Hi Alex,
>> Hi Jessica,
>>
>> On 8/1/25 03:32, liu.xuemei1@xxxxxxxxxx wrote:
>>>
>>> On 7/31/25 21:29, alex@xxxxxxxx wrote:
>>>
>>> > > From: Jessica Liu <liu.xuemei1@xxxxxxxxxx>
>>>
>>> > >
>>>
>>> > > 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@xxxxxxxxxx>
>>>
>>> > > ---
>>>
>>> > > 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
Thank you for your feedback and suggestion. I understand your desire
to have a unified approach for both SMP and !SMP. However, after
careful consideration, I still believe that handling them separately
is the more appropriate solution.
The current method of obtaining cache information in
`init_cpu_topology()` is specific to RISC-V and ARM64. If we move
`init_cpu_topology()` to cover both SMP and !SMP, it may require
modifying the generic boot sequence. This could inadvertently affect
other architectures that do not rely on `init_cpu_topology()` for
cache initialization, leading to potential regressions and maintenance
issues.
The `setup_arch()` function is called early in the boot process,
and at this stage, the ACPI subsystem has not been fully initialized.
Specifically, the ACPI tables (including PPTT) are not yet parsed.
Therefore, if we call `init_cpu_topology()` from `setup_arch()`, it
would not be able to retrieve cache information from the ACPI PPTT table.
I hope this clarifies my train of thought. I'm open to further discussion and
alternative suggestions that can address the issue properly.
Best regards,
Jessica