Re: [PATCH v5 09/19] arch_topology: Use the last level cache information from the cacheinfo

[<Conor.Dooley@microchip.com>]

On 29/06/2022 20:12, Sudeep Holla wrote:
> On Wed, Jun 29, 2022 at 06:56:29PM +0000, Conor.Dooley@microchip.com wrote:
>> On 29/06/2022 19:47, Sudeep Holla wrote:
>>> On Wed, Jun 29, 2022 at 06:18:25PM +0000, Conor.Dooley@microchip.com wrote:
>>>> On 29/06/2022 18:49, Conor.Dooley@microchip.com wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 27/06/2022 17:50, Sudeep Holla wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>
>>>>>> The cacheinfo is now initialised early along with the CPU topology
>>>>>> initialisation. Instead of relying on the LLC ID information parsed
>>>>>> separately only with ACPI PPTT elsewhere, migrate to use the similar
>>>>>> information from the cacheinfo.
>>>>>>
>>>>>> This is generic for both DT and ACPI systems. The ACPI LLC ID information
>>>>>> parsed separately can now be removed from arch specific code.
>>>>>
>>>>> Hey Sudeep,
>>>>> I bisected broken boot on PolarFire SoC to this patch in next-20220629 :/
>>>>> I suspect the issue is a missing "next-level-cache" in the the dt:
>>>>> arch/riscv/boot/dts/microchip/mpfs.dtsi
>>>
>>> Good that I included this in -next, I had not received any feedback from
>>> RISC-V even after 5 iterations.
>>
>> I'll be honest, I saw the titles and CC list and made some incorrect
>> assumptions as to whether looking at it was worthwhile! I am not at
>> this all too long and what is/isn't important to look at often is not
>> obvious to me.
> 
> No worries, that's why I thought better to include in -next to get some
> attention and I did get it this time, hurray! 😄
> 
>> But hey, our CI boots -next every day for a reason ;)
>>
> 
> Good to know and that is really great. Anyways let me know if the diff I sent
> helps. I strongly suspect that is the reason, but I may be wrong.

Aye, I'll get back to you on that one in a moment or two

> 
>>> I also see this DTS is very odd. It also
>>> states CPU0 doesn't have L1-D$ while the other 4 CPUs have L1-D$. Is that
>>> a mistake or is it the reality ?
>>
>> AFAIK, reality. It's the same for the SiFive fu540 (with which this shares
>> a core complex. See page 12:
>> https://static.dev.sifive.com/FU540-C000-v1.0.pdf
>>
>>> Another breakage in userspace cacheinfo
>>> sysfs entry of cpu0 has both I$ and D$.
>>
>> Could you clarify what this means please?
> 
> Ignore me if the cpu0 really doesn't have L1-D$. However the userspace
> sysfs cacheinfo is incomplete without linking L2, so it can be considered
> as wrong info presented to the user.

Yeah, I'll send a patch hooking up the L2.
It wasn't in the initial fu540 dtsi so I guess it was added after the
initial dts for my stuff was created based on that.

> 
> Check /sys/devices/system/cpu/cpu<n>/cache/index<i>/*.
> L2 won't be present there as the link with next-level-cache is missing.
> So userspace can interpret this as absence of L2.
> 

# cat /sys/devices/system/cpu/cpu0/cache/index0/
coherency_line_size    shared_cpu_list        type
level                  shared_cpu_map         uevent
number_of_sets         size                   ways_of_associativity
# ls /sys/devices/system/cpu/cpu0/cache/
index0  index1  uevent
# cat /sys/devices/system/cpu/cpu0/cache/index0/level 
1
# cat /sys/devices/system/cpu/cpu0/cache/index1/level 
1

cpu0 is /not/ the one with only instruction cache, that is not
running Linux.