[PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Dave Olson]

From: Dave Olson <olson@cumulusnetworks.com>

Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
The missing entry caused lscpu to error out on e500v2 devices, and probably others
 error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size

Signed-off-by: Dave Olson <olson@cumulusnetworks.com>

---

diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index 40198d5..9ca1e9a 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -72,7 +72,7 @@ static const struct cache_type_info cache_type_info[] = {
 		 * must be equal on unified caches, so just use
 		 * d-cache properties. */
 		.name            = "Unified",
-		.size_prop       = "d-cache-size",
+		.size_prop       = "cache-size",
 		.line_size_props = { "d-cache-line-size",
 				     "d-cache-block-size", },
 		.nr_sets_prop    = "d-cache-sets",

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Michael Ellerman]

On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
> From: Dave Olson <olson@cumulusnetworks.com>
> 
> Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
> This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
> The missing entry caused lscpu to error out on e500v2 devices, and probably others
>  error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
> The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size

Can you convince me that this is not going to break other machines that have
"d-cache-size" but not "cache-size"?
 
cheers

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Dave Olson]

Michael Ellerman <mpe@ellerman.id.au> wrote:

> On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
> > From: Dave Olson <olson@cumulusnetworks.com>
> > 
> > Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
> > This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
> > The missing entry caused lscpu to error out on e500v2 devices, and probably others
> >  error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
> > The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size
> 
> Can you convince me that this is not going to break other machines that have
> "d-cache-size" but not "cache-size"?

I'm unable to find any dts file that uses d-cache-size for the L2
unified cache.  All in the powerpc tree in arch/powerpc/boot/dts/*
are using cache-size in the L2 description for the cache size.

As best as I can tell from looking around, this is universal.

Dave Olson
olson@cumulusnetworks.com

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Benjamin Herrenschmidt]

On Mon, 2015-02-09 at 15:43 -0800, Dave Olson wrote:
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
> > > From: Dave Olson <olson@cumulusnetworks.com>
> > > 
> > > Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
> > > This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
> > > The missing entry caused lscpu to error out on e500v2 devices, and probably others
> > >  error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
> > > The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size
> > 
> > Can you convince me that this is not going to break other machines that have
> > "d-cache-size" but not "cache-size"?
> 
> I'm unable to find any dts file that uses d-cache-size for the L2
> unified cache.  All in the powerpc tree in arch/powerpc/boot/dts/*
> are using cache-size in the L2 description for the cache size.
> 
> As best as I can tell from looking around, this is universal.

It may be universal for embedded machines using DTS in the kernel tree
but it's definitely not true of any Mac or server machine (from which
there is no DTS in the kernel as we get the DT from the firmware).

Ben.

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Michael Ellerman]

On Tue, 2015-02-10 at 11:12 +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2015-02-09 at 15:43 -0800, Dave Olson wrote:
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > 
> > > On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
> > > > From: Dave Olson <olson@cumulusnetworks.com>
> > > > 
> > > > Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
> > > > This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
> > > > The missing entry caused lscpu to error out on e500v2 devices, and probably others
> > > >  error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
> > > > The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size
> > > 
> > > Can you convince me that this is not going to break other machines that have
> > > "d-cache-size" but not "cache-size"?
> > 
> > I'm unable to find any dts file that uses d-cache-size for the L2
> > unified cache.  All in the powerpc tree in arch/powerpc/boot/dts/*
> > are using cache-size in the L2 description for the cache size.
> > 
> > As best as I can tell from looking around, this is universal.
>
> 
> It may be universal for embedded machines using DTS in the kernel tree
> but it's definitely not true of any Mac or server machine (from which
> there is no DTS in the kernel as we get the DT from the firmware).

Right.

$ grep machine /proc/cpuinfo 
machine		: PowerNV 8247-22L

$ lsprop /proc/device-tree/cpus/l2-cache@20000020
name             "l2-cache"
status           "okay"
reg              20000020 (536870944)
phandle          00000005
linux,phandle    00000005
l2-cache         00000006
i-cache-size     00080000 (524288)
i-cache-sets     00000008
device_type      "cache"
d-cache-size     00080000 (524288)
d-cache-sets     00000008
cache-unified   


cheers

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Dave Olson]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2015-02-09 at 15:43 -0800, Dave Olson wrote:
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > 
> > > On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
> > > > From: Dave Olson <olson@cumulusnetworks.com>
> > > > 
> > > > Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
> > > > This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
> > > > The missing entry caused lscpu to error out on e500v2 devices, and probably others
> > > >  error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory
> > > > The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size
> > > 
> > > Can you convince me that this is not going to break other machines that have
> > > "d-cache-size" but not "cache-size"?
> > 
> > I'm unable to find any dts file that uses d-cache-size for the L2
> > unified cache.  All in the powerpc tree in arch/powerpc/boot/dts/*
> > are using cache-size in the L2 description for the cache size.
> > 
> > As best as I can tell from looking around, this is universal.
> 
> It may be universal for embedded machines using DTS in the kernel tree
> but it's definitely not true of any Mac or server machine (from which
> there is no DTS in the kernel as we get the DT from the firmware).

OK, now that I understand that's the case, I'll have to go back and
re-do the patch to handle both cache-size and d-cache-size for the
L2 cache (using whichever is present).

I don't have any power Macs to use for testing, would one of you be
willing and able to verify the patch on a power Mac?

The patch below fixes my problem, and I don't think it will break
platforms like the PowerPC Mac that use d-cache-size
=====
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index a3c684b..0d1f879 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -200,6 +200,10 @@ static int cache_size(const struct cache *cache, unsigned int *ret)
 	propname = cache_type_info[cache->type].size_prop;
 
 	cache_size = of_get_property(cache->ofnode, propname, NULL);
+	if (!cache_size && cache->type == CACHE_TYPE_UNIFIED) {
+        /* most embedded systems with L2 use "cache-size", allow that also */
+        cache_size = of_get_property(cache->ofnode, "cache-size", NULL);
+    }
 	if (!cache_size)
 		return -ENODEV;
 
=====

Thanks,

Dave Olson
olson@cumulusnetworks.com

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Benjamin Herrenschmidt]

On Tue, 2015-02-10 at 00:00 -0800, Dave Olson wrote:
> 
> OK, now that I understand that's the case, I'll have to go back and
> re-do the patch to handle both cache-size and d-cache-size for the
> L2 cache (using whichever is present).

I notice that you also didn't modify all the other properties, I would
assume you need to also updates in that area ? Maybe you should
duplicate the whole structure and have the code look for both.

> I don't have any power Macs to use for testing, would one of you be
> willing and able to verify the patch on a power Mac?
> 
> The patch below fixes my problem, and I don't think it will break
> platforms like the PowerPC Mac that use d-cache-size

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Dave Olson]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2015-02-10 at 00:00 -0800, Dave Olson wrote:
> > 
> > OK, now that I understand that's the case, I'll have to go back and
> > re-do the patch to handle both cache-size and d-cache-size for the
> > L2 cache (using whichever is present).
> 
> I notice that you also didn't modify all the other properties, I would
> assume you need to also updates in that area ? Maybe you should
> duplicate the whole structure and have the code look for both.

Since we have line_size_props, I can bump that from 2 to 4
entries, and add "cache_line_size" and "cache_block_size",
instead of an explict check.

I could change size_prop, and nr_sets_prop to be a structure like
line_size_props, if you think that's cleaner than the explict
check for "cache-size", and "cache-sets" in the functions.

These 3 seem to be the only ones at issue, and I should have checked
futher to realize that sets and line size were missing.

What's the preference for the other 2 missing items?

> > I don't have any power Macs to use for testing, would one of you be
> > willing and able to verify the patch on a power Mac?

Dave Olson
olson@cumulusnetworks.com

Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu

[Benjamin Herrenschmidt]

On Tue, 2015-02-10 at 08:55 -0800, Dave Olson wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > On Tue, 2015-02-10 at 00:00 -0800, Dave Olson wrote:
> > > 
> > > OK, now that I understand that's the case, I'll have to go back and
> > > re-do the patch to handle both cache-size and d-cache-size for the
> > > L2 cache (using whichever is present).
> > 
> > I notice that you also didn't modify all the other properties, I would
> > assume you need to also updates in that area ? Maybe you should
> > duplicate the whole structure and have the code look for both.
> 
> Since we have line_size_props, I can bump that from 2 to 4
> entries, and add "cache_line_size" and "cache_block_size",
> instead of an explict check.
> 
> I could change size_prop, and nr_sets_prop to be a structure like
> line_size_props, if you think that's cleaner than the explict
> check for "cache-size", and "cache-sets" in the functions.
> 
> These 3 seem to be the only ones at issue, and I should have checked
> futher to realize that sets and line size were missing.
> 
> What's the preference for the other 2 missing items?

Up to you, but I'm thinking at this point, isn't it worth duplicating
the whole struct and using which ever matches on the first entry ?

> > > I don't have any power Macs to use for testing, would one of you be
> > > willing and able to verify the patch on a power Mac?
> 
> Dave Olson
> olson@cumulusnetworks.com