Re: Current status of RISC-V init_cache_level()

Re: Current status of RISC-V init_cache_level()

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1190 bytes --]

On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> Hi Conor,

Firstly, please don't send me off-list mails about such things
and instead, please reply to the relevant threads on lkml.

> There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> lscpu cannot see the cache information. I found that the reason is
> that init_cache_level() is not implemented on RISC-V.

What version of the kernel are you using? I had a brief check to make
sure something had not gone awry recently and I could see them on my
system. What do the cpu nodes in your DT look like, assuming you are
on a DT system?

> The history of the patch can be found by searching, as follows:
> https://www.spinics.net/lists/arm-kernel/msg1024976.html

I don't follow. This was the first iteration of a series that was merged
about a year ago and it moved the risc-v specific cache level
initialisation code to a common location and re-uses it for arm64.

> How is the progress of this patch?

A later iteration has been merged. Please use lore.kernel.org instead of
spinics - you can search for patchsets more easily (eg by filename) and
you could see it was merged in a later form.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: Current status of RISC-V init_cache_level()

[yunhui cui]

Hi Conor,

On Thu, Jan 18, 2024 at 9:25 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> > Hi Conor,
>
> Firstly, please don't send me off-list mails about such things
> and instead, please reply to the relevant threads on lkml.
>
> > There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> > lscpu cannot see the cache information. I found that the reason is
> > that init_cache_level() is not implemented on RISC-V.
>
> What version of the kernel are you using? I had a brief check to make
> sure something had not gone awry recently and I could see them on my
> system. What do the cpu nodes in your DT look like, assuming you are
> on a DT system?

The results of top commit using linux-next on qemu, It should not
matter if it is combined with DT, because the following function flow
directly fails.
cacheinfo_sysfs_init()...init_cache_level()
int __weak init_cache_level(unsigned int cpu)
{
return -ENOENT;
}
Is it necessary to implement an init_cache_level() on RISC-V like other arches?

BTW, Does cat /proc/cpuinfo expose cache-related information, such as
cache size?

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: Current status of RISC-V init_cache_level()

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1762 bytes --]

On Fri, Jan 19, 2024 at 10:32:38AM +0800, yunhui cui wrote:
> On Thu, Jan 18, 2024 at 9:25 PM Conor Dooley <conor@kernel.org> wrote:
> > On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> >
> > Firstly, please don't send me off-list mails about such things
> > and instead, please reply to the relevant threads on lkml.
> >
> > > There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> > > lscpu cannot see the cache information. I found that the reason is
> > > that init_cache_level() is not implemented on RISC-V.
> >
> > What version of the kernel are you using? I had a brief check to make
> > sure something had not gone awry recently and I could see them on my
> > system. What do the cpu nodes in your DT look like, assuming you are
> > on a DT system?
> 
> The results of top commit using linux-next on qemu, It should not
> matter if it is combined with DT, because the following function flow
> directly fails.

Oh no, it totally does matter what the DT looks like. The default DT in
QEMU's virt machine does not populate any cache properties. On a system
where the DT populates these values, init_of_cache_level() will ensure
that the correct sysfs entries are set up.

> cacheinfo_sysfs_init()...init_cache_level()
> int __weak init_cache_level(unsigned int cpu)
> {
> return -ENOENT;
> }
> Is it necessary to implement an init_cache_level() on RISC-V like other arches?

In order to implement that function, we would need some mechanism for
finding that information. Where do you suggest we get it from, if it is not
provided to us from DT?

> BTW, Does cat /proc/cpuinfo expose cache-related information, such as
> cache size?

It does not.

Hope that helps,

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: Current status of RISC-V init_cache_level()

[yunhui cui]

Hi Conor，

On Fri, Jan 19, 2024 at 4:22 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Fri, Jan 19, 2024 at 10:32:38AM +0800, yunhui cui wrote:
> > On Thu, Jan 18, 2024 at 9:25 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> > >
> > > Firstly, please don't send me off-list mails about such things
> > > and instead, please reply to the relevant threads on lkml.
> > >
> > > > There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> > > > lscpu cannot see the cache information. I found that the reason is
> > > > that init_cache_level() is not implemented on RISC-V.
> > >
> > > What version of the kernel are you using? I had a brief check to make
> > > sure something had not gone awry recently and I could see them on my
> > > system. What do the cpu nodes in your DT look like, assuming you are
> > > on a DT system?
> >
> > The results of top commit using linux-next on qemu, It should not
> > matter if it is combined with DT, because the following function flow
> > directly fails.
>
> Oh no, it totally does matter what the DT looks like. The default DT in
> QEMU's virt machine does not populate any cache properties. On a system
> where the DT populates these values, init_of_cache_level() will ensure
> that the correct sysfs entries are set up.
>
> > cacheinfo_sysfs_init()...init_cache_level()
> > int __weak init_cache_level(unsigned int cpu)
> > {
> > return -ENOENT;
> > }
> > Is it necessary to implement an init_cache_level() on RISC-V like other arches?
>
> In order to implement that function, we would need some mechanism for
> finding that information. Where do you suggest we get it from, if it is not
> provided to us from DT?

It's not that we don't get cache information from DT. What I want to
express is that we need to implement init_cache_level() on RISC-V,
otherwise cacheinfo_cpu_online() will return from
detect_cache_attributes() and will not execute cache_add_dev(), so it
will not be in/ sys/devices/system/cpu/cpux/ creates a "cache" node.

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: Current status of RISC-V init_cache_level()

[yunhui cui]

Hi Conor,

On Fri, Jan 19, 2024 at 4:59 PM yunhui cui <cuiyunhui@bytedance.com> wrote:
>
> Hi Conor，
>
> On Fri, Jan 19, 2024 at 4:22 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > On Fri, Jan 19, 2024 at 10:32:38AM +0800, yunhui cui wrote:
> > > On Thu, Jan 18, 2024 at 9:25 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> > > >
> > > > Firstly, please don't send me off-list mails about such things
> > > > and instead, please reply to the relevant threads on lkml.
> > > >
> > > > > There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> > > > > lscpu cannot see the cache information. I found that the reason is
> > > > > that init_cache_level() is not implemented on RISC-V.
> > > >
> > > > What version of the kernel are you using? I had a brief check to make
> > > > sure something had not gone awry recently and I could see them on my
> > > > system. What do the cpu nodes in your DT look like, assuming you are
> > > > on a DT system?
> > >
> > > The results of top commit using linux-next on qemu, It should not
> > > matter if it is combined with DT, because the following function flow
> > > directly fails.
> >
> > Oh no, it totally does matter what the DT looks like. The default DT in
> > QEMU's virt machine does not populate any cache properties. On a system
> > where the DT populates these values, init_of_cache_level() will ensure
> > that the correct sysfs entries are set up.
> >
> > > cacheinfo_sysfs_init()...init_cache_level()
> > > int __weak init_cache_level(unsigned int cpu)
> > > {
> > > return -ENOENT;
> > > }
> > > Is it necessary to implement an init_cache_level() on RISC-V like other arches?
> >
> > In order to implement that function, we would need some mechanism for
> > finding that information. Where do you suggest we get it from, if it is not
> > provided to us from DT?
>
> It's not that we don't get cache information from DT. What I want to
> express is that we need to implement init_cache_level() on RISC-V,
> otherwise cacheinfo_cpu_online() will return from
> detect_cache_attributes() and will not execute cache_add_dev(), so it
> will not be in/ sys/devices/system/cpu/cpux/ creates a "cache" node.

What do you think? If so, I will post a patch to fix it.

Thanks,
Yunhui

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [External] Re: Current status of RISC-V init_cache_level()

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3018 bytes --]

On Fri, Jan 19, 2024 at 05:24:48PM +0800, yunhui cui wrote:
> Hi Conor,
> 
> On Fri, Jan 19, 2024 at 4:59 PM yunhui cui <cuiyunhui@bytedance.com> wrote:
> >
> > Hi Conor，
> >
> > On Fri, Jan 19, 2024 at 4:22 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> > >
> > > On Fri, Jan 19, 2024 at 10:32:38AM +0800, yunhui cui wrote:
> > > > On Thu, Jan 18, 2024 at 9:25 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > On Thu, Jan 18, 2024 at 07:40:42PM +0800, yunhui cui wrote:
> > > > >
> > > > > Firstly, please don't send me off-list mails about such things
> > > > > and instead, please reply to the relevant threads on lkml.
> > > > >
> > > > > > There is no cache subdirectory in /sys/devices/system/cpu/cpu0/, so
> > > > > > lscpu cannot see the cache information. I found that the reason is
> > > > > > that init_cache_level() is not implemented on RISC-V.
> > > > >
> > > > > What version of the kernel are you using? I had a brief check to make
> > > > > sure something had not gone awry recently and I could see them on my
> > > > > system. What do the cpu nodes in your DT look like, assuming you are
> > > > > on a DT system?
> > > >
> > > > The results of top commit using linux-next on qemu, It should not
> > > > matter if it is combined with DT, because the following function flow
> > > > directly fails.
> > >
> > > Oh no, it totally does matter what the DT looks like. The default DT in
> > > QEMU's virt machine does not populate any cache properties. On a system
> > > where the DT populates these values, init_of_cache_level() will ensure
> > > that the correct sysfs entries are set up.
> > >
> > > > cacheinfo_sysfs_init()...init_cache_level()
> > > > int __weak init_cache_level(unsigned int cpu)
> > > > {
> > > > return -ENOENT;
> > > > }
> > > > Is it necessary to implement an init_cache_level() on RISC-V like other arches?
> > >
> > > In order to implement that function, we would need some mechanism for
> > > finding that information. Where do you suggest we get it from, if it is not
> > > provided to us from DT?
> >
> > It's not that we don't get cache information from DT. What I want to
> > express is that we need to implement init_cache_level() on RISC-V,
> > otherwise cacheinfo_cpu_online() will return from
> > detect_cache_attributes() and will not execute cache_add_dev(), so it
> > will not be in/ sys/devices/system/cpu/cpux/ creates a "cache" node.
> 
> What do you think? If so, I will post a patch to fix it.

By all means if you have a patch that you think solves a problem, post
it. Be sure to CC Pierre and Sudeep if you do. Also explain exactly the
situation in which you encountered it and how to reproduce.

I briefly tested a hotunplug and replug on my setup here and the cache
information was populated correctly on replug, so there must be some
specific situation in which this is hit that I am not aware of as the
hotplug side of things I am not super au fait with.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv