Message-ID: <b6fc0e5f-beff-45fb-9f93-822b72094c17@ghiti.fr>
Date: Tue, 5 Aug 2025 10:50:39 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: liu.xuemei1@....com.cn, paul.walmsley@...ive.com
Cc: palmer@...belt.com, aou@...s.berkeley.edu, spersvold@...il.com,
 sudeep.holla@....com, mikisabate@...il.com, robh@...nel.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: cacheinfo: init cache levels via fetch_cache_info
 when SMP disabled

Hi Jessica,

On 8/1/25 13:45, Alexandre Ghiti wrote:
> Hi Jessica,
>
> On 8/1/25 03:32, liu.xuemei1@....com.cn wrote:
>>
>> On 7/31/25 21:29, alex@...ti.fr wrote:
>>
>> > > From: Jessica Liu <liu.xuemei1@....com.cn>
>>
>> > >
>>
>> > > As described in commit 1845d381f280 ("riscv: cacheinfo: Add back
>>
>> > > init_cache_level() function"), when CONFIG_SMP is undefined, the 
>> cache
>>
>> > > hierarchy detection needs to be performed through the 
>> init_cache_level(),
>>
>> > > whereas when CONFIG_SMP is defined, this detection is handled 
>> during the
>>
>> > > init_cpu_topology() process.
>>
>> > >
>>
>> > > Furthermore, while commit 66381d36771e ("RISC-V: Select ACPI PPTT 
>> drivers")
>>
>> > > enables cache information retrieval through the ACPI PPTT table, the
>>
>> > > init_of_cache_level() called within init_cache_level() cannot 
>> support cache
>>
>> > > hierarchy detection through ACPI PPTT. Therefore, when CONFIG_SMP is
>>
>> > > undefined, we directly invoke the fetch_cache_info function to 
>> initialize
>>
>> > > the cache levels.
>>
>> > >
>>
>> > > Signed-off-by: Jessica Liu <liu.xuemei1@....com.cn>
>>
>> > > ---
>>
>> > >   arch/riscv/kernel/cacheinfo.c | 6 +++++-
>>
>> > >   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> > >
>>
>> > > diff --git a/arch/riscv/kernel/cacheinfo.c 
>> b/arch/riscv/kernel/cacheinfo.c
>>
>> > > index 26b085dbdd07..f81ca963d177 100644
>>
>> > > --- a/arch/riscv/kernel/cacheinfo.c
>>
>> > > +++ b/arch/riscv/kernel/cacheinfo.c
>>
>> > > @@ -73,7 +73,11 @@ static void ci_leaf_init(struct cacheinfo 
>> *this_leaf,
>>
>> > >
>>
>> > >   int init_cache_level(unsigned int cpu)
>>
>> > >   {
>>
>> > > -    return init_of_cache_level(cpu);
>>
>> > > +#ifdef CONFIG_SMP
>>
>> > > +    return 0;
>>
>> > > +#endif
>>
>> > > +
>>
>> > > +    return fetch_cache_info(cpu);
>>
>> > >   }
>>
>> > >
>>
>> > >   int populate_cache_leaves(unsigned int cpu)
>>
>> >
>>
>> >
>>
>> > Is the current behaviour wrong or just redundant? If wrong, I'll add a
>>
>> > Fixes tag to backport, otherwise I won't.
>>
>> >
>>
>> > Thanks,
>>
>> >
>>
>> > Alex
>>
>>
>> Hi Alex,
>>
>>
>> The current behavior is actually wrong when using ACPI on !CONFIG_SMP
>>
>> systems. The original init_of_cache_level() cannot detect cache 
>> hierarchy
>>
>> through ACPI PPTT table, which means cache information would be missing
>>
>> in this configuration.
>>
>>
>> The patch fixes this by directly calling fetch_cache_info() when
>>
>> CONFIG_SMP is undefined, which properly handles both DT and ACPI cases.
>>
>>
>> So yes, it would be appropriate to add a Fixes tag. The commit being
>>
>> fixed is 1845d381f280 ("riscv: cacheinfo: Add back init_cache_level() 
>> function").
>>
>>
>> Please let me know if you need any additional information.
>>
>
> I'm about to send my first PR for 6.17 so I'll delay merging this one 
> for the first rc.


So I took the time this morning to look into this, and I don't really 
like the different treatment for smp, can't we just move 
init_cpu_topology() call to setup_arch() (or else) for both !smp and smp?

Thanks,

Alex


>
> Thanks for the explanation,
>
> Alex
>
>
>>
>> Best regards,
>>
>> Jessica
>>
>>
>>
>>
>>
>>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists