| Message ID | 4d62bc0844576b80e00ea48e318be238a4d73eae.1603728729.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Warn user on cpu errata 832075 | expand |
On Mon, 26 Oct 2020, Bertrand Marquis wrote: > When a Cortex A57 processor is affected by CPU errata 832075, a guest > not implementing the workaround for it could deadlock the system. > Add a warning during boot informing the user that only trusted guests > should be executed on the system. > An equivalent warning is already given to the user by KVM on cores > affected by this errata. > > Also taint the hypervisor as unsecure when this errata applies and > mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > --- > SUPPORT.md | 1 + > xen/arch/arm/cpuerrata.c | 13 +++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/SUPPORT.md b/SUPPORT.md > index 5fbe5fc444..f7a3b046b0 100644 > --- a/SUPPORT.md > +++ b/SUPPORT.md > @@ -38,6 +38,7 @@ supported in this document. > ### ARM v8 > > Status: Supported > + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) > > ## Host hardware support > > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index 0430069a84..b35e8cd0b9 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -503,6 +503,19 @@ void check_local_cpu_errata(void) > void __init enable_errata_workarounds(void) > { > enable_cpu_capabilities(arm_errata); > + > +#ifdef CONFIG_ARM64_ERRATUM_832075 > + if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ) > + { > + printk_once("**** This CPU is affected by the errata 832075. ****\n" > + "**** Guests without CPU erratum workarounds ****\n" > + "**** can deadlock the system! ****\n" > + "**** Only trusted guests should be used. ****\n"); These can be on 2 lines, no need to be on 4 lines. I know that Julien wrote about printing the warning from enable_errata_workarounds but to me it looks more natural if we did it from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE. That said, I don't feel strongly about it, I am fine either way. Julien, do you have a preference? Other than that, it is fine. > + /* Taint the machine has being insecure */ > + add_taint(TAINT_MACHINE_UNSECURE); > + } > +#endif
> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Mon, 26 Oct 2020, Bertrand Marquis wrote: >> When a Cortex A57 processor is affected by CPU errata 832075, a guest >> not implementing the workaround for it could deadlock the system. >> Add a warning during boot informing the user that only trusted guests >> should be executed on the system. >> An equivalent warning is already given to the user by KVM on cores >> affected by this errata. >> >> Also taint the hypervisor as unsecure when this errata applies and >> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> --- >> SUPPORT.md | 1 + >> xen/arch/arm/cpuerrata.c | 13 +++++++++++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/SUPPORT.md b/SUPPORT.md >> index 5fbe5fc444..f7a3b046b0 100644 >> --- a/SUPPORT.md >> +++ b/SUPPORT.md >> @@ -38,6 +38,7 @@ supported in this document. >> ### ARM v8 >> >> Status: Supported >> + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) >> >> ## Host hardware support >> >> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c >> index 0430069a84..b35e8cd0b9 100644 >> --- a/xen/arch/arm/cpuerrata.c >> +++ b/xen/arch/arm/cpuerrata.c >> @@ -503,6 +503,19 @@ void check_local_cpu_errata(void) >> void __init enable_errata_workarounds(void) >> { >> enable_cpu_capabilities(arm_errata); >> + >> +#ifdef CONFIG_ARM64_ERRATUM_832075 >> + if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ) >> + { >> + printk_once("**** This CPU is affected by the errata 832075. ****\n" >> + "**** Guests without CPU erratum workarounds ****\n" >> + "**** can deadlock the system! ****\n" >> + "**** Only trusted guests should be used. ****\n"); > > These can be on 2 lines, no need to be on 4 lines. I can fix that in a v3. > > > I know that Julien wrote about printing the warning from > enable_errata_workarounds but to me it looks more natural if we did it > from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE. I have no preference either here but i kind of like this way because if we had more warnings they would allow be at the same place. I will wait for Julien answer on this before sending a v3 for this patch. Cheers Bertrand > > That said, I don't feel strongly about it, I am fine either way. Julien, > do you have a preference? > > > Other than that, it is fine. > > >> + /* Taint the machine has being insecure */ >> + add_taint(TAINT_MACHINE_UNSECURE); >> + } >> +#endif
> On Oct 28, 2020, at 8:43 AM, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: > > > >> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote: >> >> On Mon, 26 Oct 2020, Bertrand Marquis wrote: >>> When a Cortex A57 processor is affected by CPU errata 832075, a guest >>> not implementing the workaround for it could deadlock the system. >>> Add a warning during boot informing the user that only trusted guests >>> should be executed on the system. >>> An equivalent warning is already given to the user by KVM on cores >>> affected by this errata. >>> >>> Also taint the hypervisor as unsecure when this errata applies and >>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >>> >>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>> --- >>> SUPPORT.md | 1 + >>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/SUPPORT.md b/SUPPORT.md >>> index 5fbe5fc444..f7a3b046b0 100644 >>> --- a/SUPPORT.md >>> +++ b/SUPPORT.md >>> @@ -38,6 +38,7 @@ supported in this document. >>> ### ARM v8 >>> >>> Status: Supported >>> + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) I think this should be: 8<— Status, Cortex A57 r0p0-r1p1: Supported, not security supported For the Cortex A57 r0p0 - r1p1, see Errata 832075. —>8 -George
Hi George, > On 28 Oct 2020, at 09:43, George Dunlap <george.dunlap@citrix.com> wrote: > > > >> On Oct 28, 2020, at 8:43 AM, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >> >> >> >>> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> >>> On Mon, 26 Oct 2020, Bertrand Marquis wrote: >>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest >>>> not implementing the workaround for it could deadlock the system. >>>> Add a warning during boot informing the user that only trusted guests >>>> should be executed on the system. >>>> An equivalent warning is already given to the user by KVM on cores >>>> affected by this errata. >>>> >>>> Also taint the hypervisor as unsecure when this errata applies and >>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >>>> >>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>>> --- >>>> SUPPORT.md | 1 + >>>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++ >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/SUPPORT.md b/SUPPORT.md >>>> index 5fbe5fc444..f7a3b046b0 100644 >>>> --- a/SUPPORT.md >>>> +++ b/SUPPORT.md >>>> @@ -38,6 +38,7 @@ supported in this document. >>>> ### ARM v8 >>>> >>>> Status: Supported >>>> + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) > > I think this should be: > > 8<— > > Status, Cortex A57 r0p0-r1p1: Supported, not security supported > > For the Cortex A57 r0p0 - r1p1, see Errata 832075. > > —>8 > Ok I will fix that. Thanks for the review Cheers Bertrand > -George
Hi, On 28/10/2020 08:43, Bertrand Marquis wrote: > > >> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote: >> >> On Mon, 26 Oct 2020, Bertrand Marquis wrote: >>> When a Cortex A57 processor is affected by CPU errata 832075, a guest >>> not implementing the workaround for it could deadlock the system. >>> Add a warning during boot informing the user that only trusted guests >>> should be executed on the system. >>> An equivalent warning is already given to the user by KVM on cores >>> affected by this errata. >>> >>> Also taint the hypervisor as unsecure when this errata applies and >>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >>> >>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>> --- >>> SUPPORT.md | 1 + >>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/SUPPORT.md b/SUPPORT.md >>> index 5fbe5fc444..f7a3b046b0 100644 >>> --- a/SUPPORT.md >>> +++ b/SUPPORT.md >>> @@ -38,6 +38,7 @@ supported in this document. >>> ### ARM v8 >>> >>> Status: Supported >>> + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) >>> >>> ## Host hardware support >>> >>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c >>> index 0430069a84..b35e8cd0b9 100644 >>> --- a/xen/arch/arm/cpuerrata.c >>> +++ b/xen/arch/arm/cpuerrata.c >>> @@ -503,6 +503,19 @@ void check_local_cpu_errata(void) >>> void __init enable_errata_workarounds(void) >>> { >>> enable_cpu_capabilities(arm_errata); >>> + >>> +#ifdef CONFIG_ARM64_ERRATUM_832075 >>> + if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ) >>> + { >>> + printk_once("**** This CPU is affected by the errata 832075. ****\n" >>> + "**** Guests without CPU erratum workarounds ****\n" >>> + "**** can deadlock the system! ****\n" >>> + "**** Only trusted guests should be used. ****\n"); >> >> These can be on 2 lines, no need to be on 4 lines. > > I can fix that in a v3. > >> >> >> I know that Julien wrote about printing the warning from >> enable_errata_workarounds but to me it looks more natural if we did it >> from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE. > > I have no preference either here but i kind of like this way because if we had more warnings > they would allow be at the same place. So I add this placement in mind because the previous version was using warning_add() (It can't be called from non-init helper). As we are using printk_once() now, I don't really have a preference. So I would stick with what you wrote. Cheers,
Hi Bertrand, On 26/10/2020 16:21, Bertrand Marquis wrote: > When a Cortex A57 processor is affected by CPU errata 832075, a guest > not implementing the workaround for it could deadlock the system. > Add a warning during boot informing the user that only trusted guests > should be executed on the system. > An equivalent warning is already given to the user by KVM on cores > affected by this errata. > > Also taint the hypervisor as unsecure when this errata applies and > mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> Reviewed-by: Julien Grall <jgrall@amazon.com> If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit. Cheers,
On Wed, 28 Oct 2020, Julien Grall wrote: > Hi Bertrand, > > On 26/10/2020 16:21, Bertrand Marquis wrote: > > When a Cortex A57 processor is affected by CPU errata 832075, a guest > > not implementing the workaround for it could deadlock the system. > > Add a warning during boot informing the user that only trusted guests > > should be executed on the system. > > An equivalent warning is already given to the user by KVM on cores > > affected by this errata. > > > > Also taint the hypervisor as unsecure when this errata applies and > > mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md > > > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > > Reviewed-by: Julien Grall <jgrall@amazon.com> > > If you don't need to resend the series, then I would be happy to fix the typo > pointed out by George on commit. That's OK for me. Since you are at it, could you also condense the 4 lines of the warning into 2 lines on commit as well? Thanks, Stefano
Hi Julien, > On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 26/10/2020 16:21, Bertrand Marquis wrote: >> When a Cortex A57 processor is affected by CPU errata 832075, a guest >> not implementing the workaround for it could deadlock the system. >> Add a warning during boot informing the user that only trusted guests >> should be executed on the system. >> An equivalent warning is already given to the user by KVM on cores >> affected by this errata. >> Also taint the hypervisor as unsecure when this errata applies and >> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > > Reviewed-by: Julien Grall <jgrall@amazon.com> Thanks > > If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit. There is only the condensing from Stefano. If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me Cheers Bertrand > > Cheers, > > -- > Julien Grall >
On Thu, 29 Oct 2020, Bertrand Marquis wrote: > Hi Julien, > > > On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote: > > > > Hi Bertrand, > > > > On 26/10/2020 16:21, Bertrand Marquis wrote: > >> When a Cortex A57 processor is affected by CPU errata 832075, a guest > >> not implementing the workaround for it could deadlock the system. > >> Add a warning during boot informing the user that only trusted guests > >> should be executed on the system. > >> An equivalent warning is already given to the user by KVM on cores > >> affected by this errata. > >> Also taint the hypervisor as unsecure when this errata applies and > >> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md > >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > > > > Reviewed-by: Julien Grall <jgrall@amazon.com> > > Thanks > > > > > If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit. > > There is only the condensing from Stefano. > If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me I have just done the committing
> On 29 Oct 2020, at 23:32, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Thu, 29 Oct 2020, Bertrand Marquis wrote: >> Hi Julien, >> >>> On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote: >>> >>> Hi Bertrand, >>> >>> On 26/10/2020 16:21, Bertrand Marquis wrote: >>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest >>>> not implementing the workaround for it could deadlock the system. >>>> Add a warning during boot informing the user that only trusted guests >>>> should be executed on the system. >>>> An equivalent warning is already given to the user by KVM on cores >>>> affected by this errata. >>>> Also taint the hypervisor as unsecure when this errata applies and >>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md >>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>> >>> Reviewed-by: Julien Grall <jgrall@amazon.com> >> >> Thanks >> >>> >>> If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit. >> >> There is only the condensing from Stefano. >> If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me > > I have just done the committing Thanks a lot :-) Cheers Bertrand
diff --git a/SUPPORT.md b/SUPPORT.md index 5fbe5fc444..f7a3b046b0 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -38,6 +38,7 @@ supported in this document. ### ARM v8 Status: Supported + Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075) ## Host hardware support diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index 0430069a84..b35e8cd0b9 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -503,6 +503,19 @@ void check_local_cpu_errata(void) void __init enable_errata_workarounds(void) { enable_cpu_capabilities(arm_errata); + +#ifdef CONFIG_ARM64_ERRATUM_832075 + if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ) + { + printk_once("**** This CPU is affected by the errata 832075. ****\n" + "**** Guests without CPU erratum workarounds ****\n" + "**** can deadlock the system! ****\n" + "**** Only trusted guests should be used. ****\n"); + + /* Taint the machine has being insecure */ + add_taint(TAINT_MACHINE_UNSECURE); + } +#endif } static int cpu_errata_callback(struct notifier_block *nfb,
When a Cortex A57 processor is affected by CPU errata 832075, a guest not implementing the workaround for it could deadlock the system. Add a warning during boot informing the user that only trusted guests should be executed on the system. An equivalent warning is already given to the user by KVM on cores affected by this errata. Also taint the hypervisor as unsecure when this errata applies and mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> --- SUPPORT.md | 1 + xen/arch/arm/cpuerrata.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)