Messages in this thread | | | Date | Thu, 6 Apr 2023 09:31:24 +0200 | Subject | Re: [PATCH 2/3] cacheinfo: Check cache properties are present in DT | From | Pierre Gondois <> |
| |
Hello Conor,
On 4/4/23 21:29, Conor Dooley wrote: > Hey Pierre, > > On Mon, Mar 27, 2023 at 01:59:50PM +0200, 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. >> > > Alex reported seeing a bunch of messages in his boot log in QEMU since > -rc1 which appears to be the fault of, as far as I can tell, e0df442ee49 > ("cacheinfo: Check 'cache-unified' property to count cache leaves") > like: > cacheinfo: Unable to detect cache hierarchy for CPU N > > The RISC-V QEMU virt machine doesn't define any cache properties of any > sort in the dtb, and unlike the arm64 virt machine I tried (a72) doesn't > have some registers that cache info is discoverable from. > When we call of_count_cache_leaves() from init_of_cache_level() and > there are of course no reasons to increment leaves, we hit the return 2 > case you mention above, setting num_leaves to 2. > > As you mention, when we hit cache_setup_of_node(), levels is not going > to be set to one, so we trigger the condition (this_leaf->level != 1) > and, as there are no cache nodes, break out of the loop without > incrementing index. Index is therefore less than 2, and thus we return > -ENOENT. > This is of course propagated back out to detect_cache_attributes() and > triggers the "Unable to detect..." printout :( > > With this patch(set), the spurious error prints go away, but we are left > with a "Early cacheinfo failed, ret = -22" which will need to be fixed. > > So I think this also needs to be: > Fixes: de0df442ee49 ("cacheinfo: Check 'cache-unified' property to count cache leaves") > > Probably also needs a: > Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com> > since he's found an actual, rather than theoretical, problem!
Ok yes indeed, I will do this and the other comments you made,
Regards, Pierre
> > Cheers, > Conor. >
| |