| Message ID | 20220906083555.931806-2-ajones@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: KVM: Expose Zicbom to the guest | expand |
On 06/09/2022 09:35, Andrew Jones wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Provide an info message with the block size when the Zicbom extension is > present and the block size has been determined. Why might someone care about this? > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > --- > arch/riscv/mm/cacheflush.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > index e5b087be1577..8595baf8e403 100644 > --- a/arch/riscv/mm/cacheflush.c > +++ b/arch/riscv/mm/cacheflush.c > @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > } > } > > - if (probed_block_size) > + if (probed_block_size) { > riscv_cbom_block_size = probed_block_size; > + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > + } > } > #endif > -- > 2.37.2 >
On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: > On 06/09/2022 09:35, Andrew Jones wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Provide an info message with the block size when the Zicbom extension is > > present and the block size has been determined. > > Why might someone care about this? I was unaware of anywhere else besides hardware descriptions where this is published. And, while dmesg isn't really publishing it in a way that is useful to anything other than human readers either, it at least makes it easy for a user to check it for sanity purposes (which is what I used it for) or even for applying it if they want to write something that needs it and the OS provides U-mode access to CMO. I'm not married to the idea, though, so if people would rather have less logs than this information, then I'm fine with dropping the patch. Thanks, drew > > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > arch/riscv/mm/cacheflush.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > index e5b087be1577..8595baf8e403 100644 > > --- a/arch/riscv/mm/cacheflush.c > > +++ b/arch/riscv/mm/cacheflush.c > > @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > > } > > } > > > > - if (probed_block_size) > > + if (probed_block_size) { > > riscv_cbom_block_size = probed_block_size; > > + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > > + } > > } > > #endif > > -- > > 2.37.2 > > >
On 06/09/2022 09:55, Andrew Jones wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: >> On 06/09/2022 09:35, Andrew Jones wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Provide an info message with the block size when the Zicbom extension is >>> present and the block size has been determined. >> >> Why might someone care about this? > > I was unaware of anywhere else besides hardware descriptions where this is > published. And, while dmesg isn't really publishing it in a way that is > useful to anything other than human readers either, it at least makes it > easy for a user to check it for sanity purposes (which is what I used it > for) or even for applying it if they want to write something that needs it > and the OS provides U-mode access to CMO. > > I'm not married to the idea, though, so if people would rather have less > logs than this information, then I'm fine with dropping the patch. I don't really care either way about logging it, if it helps people to be able to see it perhaps there's a better location than dmesg - would {debug,sys}fs be overkill? I was just more interested in the motivation behind the change itself. Maybe some of the above in the commit message wuld be nice? > > Thanks, > drew > >> >>> >>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com> >>> --- >>> arch/riscv/mm/cacheflush.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c >>> index e5b087be1577..8595baf8e403 100644 >>> --- a/arch/riscv/mm/cacheflush.c >>> +++ b/arch/riscv/mm/cacheflush.c >>> @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) >>> } >>> } >>> >>> - if (probed_block_size) >>> + if (probed_block_size) { >>> riscv_cbom_block_size = probed_block_size; >>> + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); >>> + } >>> } >>> #endif >>> -- >>> 2.37.2 >>> >>
On Tue, Sep 06, 2022 at 10:55:33AM +0200, Andrew Jones wrote: > On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: > > On 06/09/2022 09:35, Andrew Jones wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > > > Provide an info message with the block size when the Zicbom extension is > > > present and the block size has been determined. > > > > Why might someone care about this? > > I was unaware of anywhere else besides hardware descriptions where this is > published. And, while dmesg isn't really publishing it in a way that is > useful to anything other than human readers either, it at least makes it > easy for a user to check it for sanity purposes (which is what I used it > for) or even for applying it if they want to write something that needs it > and the OS provides U-mode access to CMO. Of course for applying it in U-mode programs it'd probably be better to introduce a sysconf variable or similar to get it. drew > > I'm not married to the idea, though, so if people would rather have less > logs than this information, then I'm fine with dropping the patch. > > Thanks, > drew > > > > > > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > > --- > > > arch/riscv/mm/cacheflush.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > > index e5b087be1577..8595baf8e403 100644 > > > --- a/arch/riscv/mm/cacheflush.c > > > +++ b/arch/riscv/mm/cacheflush.c > > > @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > > > } > > > } > > > > > > - if (probed_block_size) > > > + if (probed_block_size) { > > > riscv_cbom_block_size = probed_block_size; > > > + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > > > + } > > > } > > > #endif > > > -- > > > 2.37.2 > > > > >
On Tue, Sep 06, 2022 at 09:00:20AM +0000, Conor.Dooley@microchip.com wrote: > On 06/09/2022 09:55, Andrew Jones wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: > >> On 06/09/2022 09:35, Andrew Jones wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> Provide an info message with the block size when the Zicbom extension is > >>> present and the block size has been determined. > >> > >> Why might someone care about this? > > > > I was unaware of anywhere else besides hardware descriptions where this is > > published. And, while dmesg isn't really publishing it in a way that is > > useful to anything other than human readers either, it at least makes it > > easy for a user to check it for sanity purposes (which is what I used it > > for) or even for applying it if they want to write something that needs it > > and the OS provides U-mode access to CMO. > > > > I'm not married to the idea, though, so if people would rather have less > > logs than this information, then I'm fine with dropping the patch. > > I don't really care either way about logging it, if it helps people to > be able to see it perhaps there's a better location than dmesg - > would {debug,sys}fs be overkill? Thinking about this some more, I think sysfs would probably be the better way to go from the start. This patch should probably be dropped and I can try to add a sysfs node. The hard part of that will be the naming... How about /sys/devices/system/cpu/cpu*/cache/cmo_block_size Thanks, drew > > I was just more interested in the motivation behind the change itself. > Maybe some of the above in the commit message wuld be nice? > > > > > Thanks, > > drew > > > >> > >>> > >>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > >>> --- > >>> arch/riscv/mm/cacheflush.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > >>> index e5b087be1577..8595baf8e403 100644 > >>> --- a/arch/riscv/mm/cacheflush.c > >>> +++ b/arch/riscv/mm/cacheflush.c > >>> @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > >>> } > >>> } > >>> > >>> - if (probed_block_size) > >>> + if (probed_block_size) { > >>> riscv_cbom_block_size = probed_block_size; > >>> + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > >>> + } > >>> } > >>> #endif > >>> -- > >>> 2.37.2 > >>> > >> >
On 06/09/2022 10:29, Andrew Jones wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Tue, Sep 06, 2022 at 09:00:20AM +0000, Conor.Dooley@microchip.com wrote: >> On 06/09/2022 09:55, Andrew Jones wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: >>>> On 06/09/2022 09:35, Andrew Jones wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>> >>>>> Provide an info message with the block size when the Zicbom extension is >>>>> present and the block size has been determined. >>>> >>>> Why might someone care about this? >>> >>> I was unaware of anywhere else besides hardware descriptions where this is >>> published. And, while dmesg isn't really publishing it in a way that is >>> useful to anything other than human readers either, it at least makes it >>> easy for a user to check it for sanity purposes (which is what I used it >>> for) or even for applying it if they want to write something that needs it >>> and the OS provides U-mode access to CMO. >>> >>> I'm not married to the idea, though, so if people would rather have less >>> logs than this information, then I'm fine with dropping the patch. >> >> I don't really care either way about logging it, if it helps people to >> be able to see it perhaps there's a better location than dmesg - >> would {debug,sys}fs be overkill? > > Thinking about this some more, I think sysfs would probably be the better > way to go from the start. This patch should probably be dropped and I > can try to add a sysfs node. The hard part of that will be the naming... > How about > > /sys/devices/system/cpu/cpu*/cache/cmo_block_size Seems sane to me, but I am oh-so-very-far from an expert here. Heiko might have a more qualified opinion. > > Thanks, > drew > >> >> I was just more interested in the motivation behind the change itself. >> Maybe some of the above in the commit message wuld be nice? >> >>> >>> Thanks, >>> drew >>> >>>> >>>>> >>>>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com> >>>>> --- >>>>> arch/riscv/mm/cacheflush.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c >>>>> index e5b087be1577..8595baf8e403 100644 >>>>> --- a/arch/riscv/mm/cacheflush.c >>>>> +++ b/arch/riscv/mm/cacheflush.c >>>>> @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) >>>>> } >>>>> } >>>>> >>>>> - if (probed_block_size) >>>>> + if (probed_block_size) { >>>>> riscv_cbom_block_size = probed_block_size; >>>>> + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); >>>>> + } >>>>> } >>>>> #endif >>>>> -- >>>>> 2.37.2 >>>>> >>>> >>
Am Dienstag, 6. September 2022, 11:42:11 CEST schrieb Conor.Dooley@microchip.com: > On 06/09/2022 10:29, Andrew Jones wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Tue, Sep 06, 2022 at 09:00:20AM +0000, Conor.Dooley@microchip.com wrote: > >> On 06/09/2022 09:55, Andrew Jones wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: > >>>> On 06/09/2022 09:35, Andrew Jones wrote: > >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>>>> > >>>>> Provide an info message with the block size when the Zicbom extension is > >>>>> present and the block size has been determined. > >>>> > >>>> Why might someone care about this? > >>> > >>> I was unaware of anywhere else besides hardware descriptions where this is > >>> published. And, while dmesg isn't really publishing it in a way that is > >>> useful to anything other than human readers either, it at least makes it > >>> easy for a user to check it for sanity purposes (which is what I used it > >>> for) or even for applying it if they want to write something that needs it > >>> and the OS provides U-mode access to CMO. > >>> > >>> I'm not married to the idea, though, so if people would rather have less > >>> logs than this information, then I'm fine with dropping the patch. > >> > >> I don't really care either way about logging it, if it helps people to > >> be able to see it perhaps there's a better location than dmesg - > >> would {debug,sys}fs be overkill? > > > > Thinking about this some more, I think sysfs would probably be the better > > way to go from the start. This patch should probably be dropped and I > > can try to add a sysfs node. The hard part of that will be the naming... > > How about > > > > /sys/devices/system/cpu/cpu*/cache/cmo_block_size > > Seems sane to me, but I am oh-so-very-far from an expert here. > Heiko might have a more qualified opinion. I guess I'd more start with a pr_debug(). When debugging you get the output and regular users won't care at all anyway. Otherwise you can also just cat /proc/device-tree/cpus/cpu\@0/riscv\,cbom-block-size | xxd -p Sysfs is whole different can of worms, as you create a new userspace facing interface, which you need to support indefinitly. Similar to Conor, I guess it would be interesting to me, what problem you're trying to solve, as in my (simple) thinking, everybody that somehow needs to check the block-size should have the knowledge to get it from any of the dt representations we already have ;-) Heiko > >> I was just more interested in the motivation behind the change itself. > >> Maybe some of the above in the commit message wuld be nice? > >> > >>> > >>> Thanks, > >>> drew > >>> > >>>> > >>>>> > >>>>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > >>>>> --- > >>>>> arch/riscv/mm/cacheflush.c | 4 +++- > >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > >>>>> index e5b087be1577..8595baf8e403 100644 > >>>>> --- a/arch/riscv/mm/cacheflush.c > >>>>> +++ b/arch/riscv/mm/cacheflush.c > >>>>> @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > >>>>> } > >>>>> } > >>>>> > >>>>> - if (probed_block_size) > >>>>> + if (probed_block_size) { > >>>>> riscv_cbom_block_size = probed_block_size; > >>>>> + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > >>>>> + } > >>>>> } > >>>>> #endif > >>>>> -- > >>>>> 2.37.2 > >>>>> > >>>> > >> > >
On Tue, Sep 06, 2022 at 04:34:49PM +0200, Heiko Stübner wrote: > Am Dienstag, 6. September 2022, 11:42:11 CEST schrieb Conor.Dooley@microchip.com: > > On 06/09/2022 10:29, Andrew Jones wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > > > On Tue, Sep 06, 2022 at 09:00:20AM +0000, Conor.Dooley@microchip.com wrote: > > >> On 06/09/2022 09:55, Andrew Jones wrote: > > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > >>> > > >>> On Tue, Sep 06, 2022 at 08:40:23AM +0000, Conor.Dooley@microchip.com wrote: > > >>>> On 06/09/2022 09:35, Andrew Jones wrote: > > >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > >>>>> > > >>>>> Provide an info message with the block size when the Zicbom extension is > > >>>>> present and the block size has been determined. > > >>>> > > >>>> Why might someone care about this? > > >>> > > >>> I was unaware of anywhere else besides hardware descriptions where this is > > >>> published. And, while dmesg isn't really publishing it in a way that is > > >>> useful to anything other than human readers either, it at least makes it > > >>> easy for a user to check it for sanity purposes (which is what I used it > > >>> for) or even for applying it if they want to write something that needs it > > >>> and the OS provides U-mode access to CMO. > > >>> > > >>> I'm not married to the idea, though, so if people would rather have less > > >>> logs than this information, then I'm fine with dropping the patch. > > >> > > >> I don't really care either way about logging it, if it helps people to > > >> be able to see it perhaps there's a better location than dmesg - > > >> would {debug,sys}fs be overkill? > > > > > > Thinking about this some more, I think sysfs would probably be the better > > > way to go from the start. This patch should probably be dropped and I > > > can try to add a sysfs node. The hard part of that will be the naming... > > > How about > > > > > > /sys/devices/system/cpu/cpu*/cache/cmo_block_size > > > > Seems sane to me, but I am oh-so-very-far from an expert here. > > Heiko might have a more qualified opinion. > > I guess I'd more start with a pr_debug(). When debugging you > get the output and regular users won't care at all anyway. > Otherwise you can also just > cat /proc/device-tree/cpus/cpu\@0/riscv\,cbom-block-size | xxd -p > > > Sysfs is whole different can of worms, as you create a new userspace > facing interface, which you need to support indefinitly. > > > Similar to Conor, I guess it would be interesting to me, what problem > you're trying to solve, as in my (simple) thinking, everybody that somehow > needs to check the block-size should have the knowledge to get it from > any of the dt representations we already have ;-) Easy to get from DT, not so easy to get from ACPI. Anyway, I'm fine with dropping this patch (I'll send a v2 without it to make that clear). If we do ever want to enable CMO in U-mode, then this will likely come up again, but then it'll have better justification to put the size somewhere that applications can easily get it (and I believe that will be sysfs). Thanks, drew > > > Heiko > > > > > >> I was just more interested in the motivation behind the change itself. > > >> Maybe some of the above in the commit message wuld be nice? > > >> > > >>> > > >>> Thanks, > > >>> drew > > >>> > > >>>> > > >>>>> > > >>>>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > >>>>> --- > > >>>>> arch/riscv/mm/cacheflush.c | 4 +++- > > >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) > > >>>>> > > >>>>> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > >>>>> index e5b087be1577..8595baf8e403 100644 > > >>>>> --- a/arch/riscv/mm/cacheflush.c > > >>>>> +++ b/arch/riscv/mm/cacheflush.c > > >>>>> @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) > > >>>>> } > > >>>>> } > > >>>>> > > >>>>> - if (probed_block_size) > > >>>>> + if (probed_block_size) { > > >>>>> riscv_cbom_block_size = probed_block_size; > > >>>>> + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); > > >>>>> + } > > >>>>> } > > >>>>> #endif > > >>>>> -- > > >>>>> 2.37.2 > > >>>>> > > >>>> > > >> > > > > > > > >
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c index e5b087be1577..8595baf8e403 100644 --- a/arch/riscv/mm/cacheflush.c +++ b/arch/riscv/mm/cacheflush.c @@ -122,7 +122,9 @@ void riscv_init_cbom_blocksize(void) } } - if (probed_block_size) + if (probed_block_size) { riscv_cbom_block_size = probed_block_size; + pr_info("riscv: Zicbom: Cache blocksize is %u bytes", probed_block_size); + } } #endif
Provide an info message with the block size when the Zicbom extension is present and the block size has been determined. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/mm/cacheflush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)