Re: [PATCH v3 2/4] cacheinfo: Check cache properties are present in DT

From: Florian Fainelli
Date: Thu Apr 13 2023 - 16:06:54 EST


On 4/13/23 12:50, Sudeep Holla wrote:
On Thu, Apr 13, 2023 at 11:16:37AM -0700, Florian Fainelli wrote:
On 4/13/23 02:14, Pierre Gondois wrote:
If a Device Tree (DT) is used, the presence of cache properties is
assumed. Not finding any is not considered. For arm64 platforms,
cache information can be fetched from the clidr_el1 register.
Checking whether cache information is available in the DT
allows to switch to using clidr_el1.

init_of_cache_level()
\-of_count_cache_leaves()
will assume there a 2 cache leaves (L1 data/instruction caches), which
can be different from clidr_el1 information.

cache_setup_of_node() tries to read cache properties in the DT.
If there are none, this is considered a success. Knowing no
information was available would allow to switch to using clidr_el1.

Fixes: de0df442ee49 ("cacheinfo: Check 'cache-unified' property to count cache leaves")
Reported-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>
Link: https://lore.kernel.org/all/20230404-hatred-swimmer-6fecdf33b57a@spud/
Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>

Humm, it would appear that the cache levels and topology is still provided,
despite the lack of cache properties in the Device Tree which is intended by
this patch set however we lost the size/ways/sets information, could we not
complement the missing properties here?


I am confused. How and from where the information was fetched before this
change ?

I applied Pierre's patches to my tree and then did the following:

- before means booting with the patches applied and the Device Tree providing cache information: {d,i}-cache-{size,line-size,sets} and next-level-cache

- after means removing all of those properties still with the patches applied

My expectation is that if we omit the properties in the Device Tree, we will fallback to reading that information out of clidr_el1. However as can be seen from the "before" and "after" outputs, there is loss of information, as we no longer have the cacheline size, number of sets/ways, the rest is valid though.

So my question is whether this is expected and in scope of what is being done here, or not.


If this is out of the scope of what you are doing:

Tested-by: Florian Fainelli <f.fainelli@xxxxxxxxx>


Just looking at the lscpu output before and after, it looks something is
broken. What am I missing here ?


What is broken in the "before" output? It contains the entire set of possible information we know about the caches. As for the "after", well yes there is information missing, the whole point of my email actually...
--
Florian