* Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-10-25 10:16 ` Conor Dooley
0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2024-10-25 10:16 UTC (permalink / raw)
To: Yunhui Cui
Cc: punit.agrawal, paul.walmsley, palmer, aou, cleger, charlie, evan,
samuel.holland, andybnac, linux-riscv, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]
On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
> Like Zicboz, by enabling the corresponding bits of senvcfg,
> the instructions cbo.clean, cbo.flush, and cbo.inval can be
> executed normally in user mode.
>
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
> arch/riscv/kernel/cpufeature.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 1992ea64786e..bc850518ab41 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
> void __init riscv_user_isa_enable(void)
> {
> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> - current->thread.envcfg |= ENVCFG_CBZE;
> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
I believe we previously decided that userspace should not be allowed to
use zicbom, but that not withstanding - this is wrong. It should be
checking for Zicbom, not Zicboz.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] RISC-V: Enable Zicbom in usermode
2024-10-25 10:16 ` Conor Dooley
@ 2024-10-25 16:32 ` Jessica Clarke
-1 siblings, 0 replies; 18+ messages in thread
From: Jessica Clarke @ 2024-10-25 16:32 UTC (permalink / raw)
To: Conor Dooley
Cc: Yunhui Cui, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
> I believe we previously decided that userspace should not be allowed to
> use zicbom, but that not withstanding - this is wrong. It should be
> checking for Zicbom, not Zicboz.
Allowing clean/flush is safe but has the same problems as fence.i with
regards to migrating between harts. Allowing invalidate, unless mapped
to flush, is not safe in general unless the kernel does a lot of
flushing to avoid userspace accessing data it shouldn’t be able to see.
Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
be an invalidate). I note that the KVM code, which this likely copied
from(?), makes the same mistake, but there that is the intended
behaviour, if misleading about what the field really is.
So, with suitable caveats, allowing clean/flush could be a reasonable
thing to do (maybe useful for userspace drivers so long as they pin
themselves to a specific hart?), but invalidate should only ever be
allowed if mapped to flush.
Jess
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-10-25 16:32 ` Jessica Clarke
0 siblings, 0 replies; 18+ messages in thread
From: Jessica Clarke @ 2024-10-25 16:32 UTC (permalink / raw)
To: Conor Dooley
Cc: Yunhui Cui, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
> I believe we previously decided that userspace should not be allowed to
> use zicbom, but that not withstanding - this is wrong. It should be
> checking for Zicbom, not Zicboz.
Allowing clean/flush is safe but has the same problems as fence.i with
regards to migrating between harts. Allowing invalidate, unless mapped
to flush, is not safe in general unless the kernel does a lot of
flushing to avoid userspace accessing data it shouldn’t be able to see.
Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
be an invalidate). I note that the KVM code, which this likely copied
from(?), makes the same mistake, but there that is the intended
behaviour, if misleading about what the field really is.
So, with suitable caveats, allowing clean/flush could be a reasonable
thing to do (maybe useful for userspace drivers so long as they pin
themselves to a specific hart?), but invalidate should only ever be
allowed if mapped to flush.
Jess
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [External] Re: [PATCH] RISC-V: Enable Zicbom in usermode
2024-10-25 16:32 ` Jessica Clarke
@ 2024-10-31 8:29 ` yunhui cui
-1 siblings, 0 replies; 18+ messages in thread
From: yunhui cui @ 2024-10-31 8:29 UTC (permalink / raw)
To: Jessica Clarke
Cc: Conor Dooley, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
Hi Jessica,
On Sat, Oct 26, 2024 at 12:32 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> > On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
> >> Like Zicboz, by enabling the corresponding bits of senvcfg,
> >> the instructions cbo.clean, cbo.flush, and cbo.inval can be
> >> executed normally in user mode.
> >>
> >> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> >> ---
> >> arch/riscv/kernel/cpufeature.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >> index 1992ea64786e..bc850518ab41 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
> >> void __init riscv_user_isa_enable(void)
> >> {
> >> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> >> - current->thread.envcfg |= ENVCFG_CBZE;
> >> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
> >
> > I believe we previously decided that userspace should not be allowed to
> > use zicbom, but that not withstanding - this is wrong. It should be
> > checking for Zicbom, not Zicboz.
>
> Allowing clean/flush is safe but has the same problems as fence.i with
> regards to migrating between harts. Allowing invalidate, unless mapped
> to flush, is not safe in general unless the kernel does a lot of
> flushing to avoid userspace accessing data it shouldn’t be able to see.
>
> Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
> have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
> be an invalidate). I note that the KVM code, which this likely copied
> from(?), makes the same mistake, but there that is the intended
> behaviour, if misleading about what the field really is.
>
> So, with suitable caveats, allowing clean/flush could be a reasonable
> thing to do (maybe useful for userspace drivers so long as they pin
> themselves to a specific hart?), but invalidate should only ever be
> allowed if mapped to flush.
>
> Jess
>
Yes. The original intention is to enable clean/flush/invalid. So
ENVCFG_CBIE | ENVCFG_CBCFE is added. When one core initiates an
invalidation, other cores will also invalidate the corresponding cache
line. So do we not need to worry about this problem? Moreover,
invalidation is not found in the logic of disabling preemption in the
kernel. Or perhaps binding cores belongs to the user-space's own
logic. Can this patch be fixed as RISCV_ISA_EXT_ZICBOM and then a v2
be sent?
Thanks,
Yunhui
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [External] Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-10-31 8:29 ` yunhui cui
0 siblings, 0 replies; 18+ messages in thread
From: yunhui cui @ 2024-10-31 8:29 UTC (permalink / raw)
To: Jessica Clarke
Cc: Conor Dooley, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
Hi Jessica,
On Sat, Oct 26, 2024 at 12:32 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> > On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
> >> Like Zicboz, by enabling the corresponding bits of senvcfg,
> >> the instructions cbo.clean, cbo.flush, and cbo.inval can be
> >> executed normally in user mode.
> >>
> >> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> >> ---
> >> arch/riscv/kernel/cpufeature.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >> index 1992ea64786e..bc850518ab41 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
> >> void __init riscv_user_isa_enable(void)
> >> {
> >> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> >> - current->thread.envcfg |= ENVCFG_CBZE;
> >> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
> >
> > I believe we previously decided that userspace should not be allowed to
> > use zicbom, but that not withstanding - this is wrong. It should be
> > checking for Zicbom, not Zicboz.
>
> Allowing clean/flush is safe but has the same problems as fence.i with
> regards to migrating between harts. Allowing invalidate, unless mapped
> to flush, is not safe in general unless the kernel does a lot of
> flushing to avoid userspace accessing data it shouldn’t be able to see.
>
> Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
> have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
> be an invalidate). I note that the KVM code, which this likely copied
> from(?), makes the same mistake, but there that is the intended
> behaviour, if misleading about what the field really is.
>
> So, with suitable caveats, allowing clean/flush could be a reasonable
> thing to do (maybe useful for userspace drivers so long as they pin
> themselves to a specific hart?), but invalidate should only ever be
> allowed if mapped to flush.
>
> Jess
>
Yes. The original intention is to enable clean/flush/invalid. So
ENVCFG_CBIE | ENVCFG_CBCFE is added. When one core initiates an
invalidation, other cores will also invalidate the corresponding cache
line. So do we not need to worry about this problem? Moreover,
invalidation is not found in the logic of disabling preemption in the
kernel. Or perhaps binding cores belongs to the user-space's own
logic. Can this patch be fixed as RISCV_ISA_EXT_ZICBOM and then a v2
be sent?
Thanks,
Yunhui
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [External] Re: [PATCH] RISC-V: Enable Zicbom in usermode
2024-10-31 8:29 ` yunhui cui
@ 2024-11-13 17:37 ` Andrew Jones
-1 siblings, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2024-11-13 17:37 UTC (permalink / raw)
To: yunhui cui
Cc: Jessica Clarke, Conor Dooley, punit.agrawal, paul.walmsley,
palmer, aou, cleger, charlie, evan, samuel.holland, andybnac,
linux-riscv, linux-kernel
On Thu, Oct 31, 2024 at 04:29:49PM +0800, yunhui cui wrote:
> Hi Jessica,
>
> On Sat, Oct 26, 2024 at 12:32 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >
> > On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> > > On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
> > >> Like Zicboz, by enabling the corresponding bits of senvcfg,
> > >> the instructions cbo.clean, cbo.flush, and cbo.inval can be
> > >> executed normally in user mode.
> > >>
> > >> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > >> ---
> > >> arch/riscv/kernel/cpufeature.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > >> index 1992ea64786e..bc850518ab41 100644
> > >> --- a/arch/riscv/kernel/cpufeature.c
> > >> +++ b/arch/riscv/kernel/cpufeature.c
> > >> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
> > >> void __init riscv_user_isa_enable(void)
> > >> {
> > >> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> > >> - current->thread.envcfg |= ENVCFG_CBZE;
> > >> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
> > >
> > > I believe we previously decided that userspace should not be allowed to
> > > use zicbom, but that not withstanding - this is wrong. It should be
> > > checking for Zicbom, not Zicboz.
> >
> > Allowing clean/flush is safe but has the same problems as fence.i with
> > regards to migrating between harts. Allowing invalidate, unless mapped
> > to flush, is not safe in general unless the kernel does a lot of
> > flushing to avoid userspace accessing data it shouldn’t be able to see.
> >
> > Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
> > have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
> > be an invalidate). I note that the KVM code, which this likely copied
> > from(?), makes the same mistake, but there that is the intended
> > behaviour, if misleading about what the field really is.
> >
> > So, with suitable caveats, allowing clean/flush could be a reasonable
> > thing to do (maybe useful for userspace drivers so long as they pin
> > themselves to a specific hart?), but invalidate should only ever be
> > allowed if mapped to flush.
> >
> > Jess
> >
> Yes. The original intention is to enable clean/flush/invalid. So
> ENVCFG_CBIE | ENVCFG_CBCFE is added. When one core initiates an
> invalidation, other cores will also invalidate the corresponding cache
> line. So do we not need to worry about this problem? Moreover,
> invalidation is not found in the logic of disabling preemption in the
> kernel. Or perhaps binding cores belongs to the user-space's own
> logic. Can this patch be fixed as RISCV_ISA_EXT_ZICBOM and then a v2
> be sent?
Jessica points out that CBIE should only be set if it's 0b01 but it's
currently 0b11, so just changing RISCV_ISA_EXT_ZICBOM is insufficient
for v2.
Is there a use case for usermode to have an invalidate without a clean?
If so, then is that use case not present on Arm platforms or is there a
workaround for Arm platforms? Because Arm's 'DC IVAC' is only allowed for
EL1 and higher. To be consistent with Arm we can add cbo.flush and
cbo.clean (CBCFE) since Arm does allow exposing 'DC CIVAC' and 'DC CVAC'
to EL0. Since it looks like CBIE=0b01 would just make cbo.inval an alias
of cbo.flush, then I'm not sure it makes sense to set it, in fact it may
just confuse things.
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [External] Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-11-13 17:37 ` Andrew Jones
0 siblings, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2024-11-13 17:37 UTC (permalink / raw)
To: yunhui cui
Cc: Jessica Clarke, Conor Dooley, punit.agrawal, paul.walmsley,
palmer, aou, cleger, charlie, evan, samuel.holland, andybnac,
linux-riscv, linux-kernel
On Thu, Oct 31, 2024 at 04:29:49PM +0800, yunhui cui wrote:
> Hi Jessica,
>
> On Sat, Oct 26, 2024 at 12:32 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >
> > On 25 Oct 2024, at 11:16, Conor Dooley <conor@kernel.org> wrote:
> > > On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
> > >> Like Zicboz, by enabling the corresponding bits of senvcfg,
> > >> the instructions cbo.clean, cbo.flush, and cbo.inval can be
> > >> executed normally in user mode.
> > >>
> > >> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > >> ---
> > >> arch/riscv/kernel/cpufeature.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > >> index 1992ea64786e..bc850518ab41 100644
> > >> --- a/arch/riscv/kernel/cpufeature.c
> > >> +++ b/arch/riscv/kernel/cpufeature.c
> > >> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
> > >> void __init riscv_user_isa_enable(void)
> > >> {
> > >> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
> > >> - current->thread.envcfg |= ENVCFG_CBZE;
> > >> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
> > >
> > > I believe we previously decided that userspace should not be allowed to
> > > use zicbom, but that not withstanding - this is wrong. It should be
> > > checking for Zicbom, not Zicboz.
> >
> > Allowing clean/flush is safe but has the same problems as fence.i with
> > regards to migrating between harts. Allowing invalidate, unless mapped
> > to flush, is not safe in general unless the kernel does a lot of
> > flushing to avoid userspace accessing data it shouldn’t be able to see.
> >
> > Also, ENVCFG_CBIE is a mask for a multi-bit field, which happens to
> > have the same value as ENVCFG_CBIE_INV (i.e. really is making cbo.inval
> > be an invalidate). I note that the KVM code, which this likely copied
> > from(?), makes the same mistake, but there that is the intended
> > behaviour, if misleading about what the field really is.
> >
> > So, with suitable caveats, allowing clean/flush could be a reasonable
> > thing to do (maybe useful for userspace drivers so long as they pin
> > themselves to a specific hart?), but invalidate should only ever be
> > allowed if mapped to flush.
> >
> > Jess
> >
> Yes. The original intention is to enable clean/flush/invalid. So
> ENVCFG_CBIE | ENVCFG_CBCFE is added. When one core initiates an
> invalidation, other cores will also invalidate the corresponding cache
> line. So do we not need to worry about this problem? Moreover,
> invalidation is not found in the logic of disabling preemption in the
> kernel. Or perhaps binding cores belongs to the user-space's own
> logic. Can this patch be fixed as RISCV_ISA_EXT_ZICBOM and then a v2
> be sent?
Jessica points out that CBIE should only be set if it's 0b01 but it's
currently 0b11, so just changing RISCV_ISA_EXT_ZICBOM is insufficient
for v2.
Is there a use case for usermode to have an invalidate without a clean?
If so, then is that use case not present on Arm platforms or is there a
workaround for Arm platforms? Because Arm's 'DC IVAC' is only allowed for
EL1 and higher. To be consistent with Arm we can add cbo.flush and
cbo.clean (CBCFE) since Arm does allow exposing 'DC CIVAC' and 'DC CVAC'
to EL0. Since it looks like CBIE=0b01 would just make cbo.inval an alias
of cbo.flush, then I'm not sure it makes sense to set it, in fact it may
just confuse things.
Thanks,
drew
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] RISC-V: Enable Zicbom in usermode
2024-10-25 10:16 ` Conor Dooley
@ 2024-10-29 19:20 ` Deepak Gupta
-1 siblings, 0 replies; 18+ messages in thread
From: Deepak Gupta @ 2024-10-29 19:20 UTC (permalink / raw)
To: Conor Dooley
Cc: Yunhui Cui, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
On Fri, Oct 25, 2024 at 11:16:44AM +0100, Conor Dooley wrote:
>On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
>I believe we previously decided that userspace should not be allowed to
>use zicbom, but that not withstanding - this is wrong. It should be
>checking for Zicbom, not Zicboz.
Additional comment:
It would be good to have this (flush/clean/inval) disabled for seccomped
process or at least some sort of user abi to disable it (whenever use decides
to seccomp current task). So either
- by default disable when task is strict seccomped
OR
- introduce user abi (prctl) to disable it. so that any task trying to
lockdown itself should be able to do that.
This is particularly useful for sandbox hosting in same address space.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-10-29 19:20 ` Deepak Gupta
0 siblings, 0 replies; 18+ messages in thread
From: Deepak Gupta @ 2024-10-29 19:20 UTC (permalink / raw)
To: Conor Dooley
Cc: Yunhui Cui, punit.agrawal, paul.walmsley, palmer, aou, cleger,
charlie, evan, samuel.holland, andybnac, linux-riscv,
linux-kernel
On Fri, Oct 25, 2024 at 11:16:44AM +0100, Conor Dooley wrote:
>On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
>I believe we previously decided that userspace should not be allowed to
>use zicbom, but that not withstanding - this is wrong. It should be
>checking for Zicbom, not Zicboz.
Additional comment:
It would be good to have this (flush/clean/inval) disabled for seccomped
process or at least some sort of user abi to disable it (whenever use decides
to seccomp current task). So either
- by default disable when task is strict seccomped
OR
- introduce user abi (prctl) to disable it. so that any task trying to
lockdown itself should be able to do that.
This is particularly useful for sandbox hosting in same address space.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] RISC-V: Enable Zicbom in usermode
2024-10-25 10:16 ` Conor Dooley
@ 2024-11-13 15:11 ` Palmer Dabbelt
-1 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2024-11-13 15:11 UTC (permalink / raw)
To: Conor Dooley
Cc: cuiyunhui, punit.agrawal, Paul Walmsley, aou, cleger,
Charlie Jenkins, Evan Green, samuel.holland, andybnac,
linux-riscv, linux-kernel
On Fri, 25 Oct 2024 03:16:44 PDT (-0700), Conor Dooley wrote:
> On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
> I believe we previously decided that userspace should not be allowed to
Ya, we didn't want to expose this because it opens up a can of worms.
Is there a use case for this? It's not like this is entirely impossible
to do, it just requires a bit of thought (and should probably be gated
behind some per-process disabling).
> use zicbom, but that not withstanding - this is wrong. It should be
> checking for Zicbom, not Zicboz.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] RISC-V: Enable Zicbom in usermode
@ 2024-11-13 15:11 ` Palmer Dabbelt
0 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2024-11-13 15:11 UTC (permalink / raw)
To: Conor Dooley
Cc: cuiyunhui, punit.agrawal, Paul Walmsley, aou, cleger,
Charlie Jenkins, Evan Green, samuel.holland, andybnac,
linux-riscv, linux-kernel
On Fri, 25 Oct 2024 03:16:44 PDT (-0700), Conor Dooley wrote:
> On Fri, Oct 25, 2024 at 05:15:27PM +0800, Yunhui Cui wrote:
>> Like Zicboz, by enabling the corresponding bits of senvcfg,
>> the instructions cbo.clean, cbo.flush, and cbo.inval can be
>> executed normally in user mode.
>>
>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>> ---
>> arch/riscv/kernel/cpufeature.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1992ea64786e..bc850518ab41 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -924,7 +924,7 @@ unsigned long riscv_get_elf_hwcap(void)
>> void __init riscv_user_isa_enable(void)
>> {
>> if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
>> - current->thread.envcfg |= ENVCFG_CBZE;
>> + current->thread.envcfg |= ENVCFG_CBIE | ENVCFG_CBCFE | ENVCFG_CBZE;
>
> I believe we previously decided that userspace should not be allowed to
Ya, we didn't want to expose this because it opens up a can of worms.
Is there a use case for this? It's not like this is entirely impossible
to do, it just requires a bit of thought (and should probably be gated
behind some per-process disabling).
> use zicbom, but that not withstanding - this is wrong. It should be
> checking for Zicbom, not Zicboz.
^ permalink raw reply [flat|nested] 18+ messages in thread