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

[Conor.Dooley]

On 29/06/2022 20:43, Sudeep Holla wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Jun 29, 2022 at 07:25:41PM +0000, Conor.Dooley@xxxxxxxxxxxxx wrote:
>>
>>
>> On 29/06/2022 20:12, Sudeep Holla wrote:
>>> On Wed, Jun 29, 2022 at 06:56:29PM +0000, Conor.Dooley@xxxxxxxxxxxxx wrote:
>>>> On 29/06/2022 19:47, Sudeep Holla wrote:
>>>>> On Wed, Jun 29, 2022 at 06:18:25PM +0000, Conor.Dooley@xxxxxxxxxxxxx wrote:
>>>>>> On 29/06/2022 18:49, Conor.Dooley@xxxxxxxxxxxxx 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
>>
> 
> Sure, take your time.
> 
>>>
>>>>> 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.
>>
> 
> Thanks!
> 
>>>
>>> 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
>>
> Ideally there must /sys/devices/system/cpu/cpu*/cache/index2/level
> which reads 2 once you link it in the DT.

# ls /sys/devices/system/cpu/cpu0/cache/
index0  index1  index2  uevent
# cat /sys/devices/system/cpu/cpu0/cache/index2/level 
2
# cat /sys/devices/system/cpu/cpu0/cache/index1/level 
1
# cat /sys/devices/system/cpu/cpu0/cache/index0/level 
1

Sweet :)

> 
>> cpu0 is /not/ the one with only instruction cache, that is not
>> running Linux.
> 
> Ah, so there Linux runs only on cpu 1-4 ?

Correct. cpu0 supports fewer instructions & we just use it as a
monitor core for opensbi etc.

> 
> --
> Regards,
> Sudeep