Re: [PATCH] xen/arm: Warn user on cpu errata 832075

[Stefano Stabellini <sstabellini@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]

On Wed, 14 Oct 2020, Bertrand Marquis wrote:
> > On 14 Oct 2020, at 12:11, Julien Grall <julien@xen.org> wrote:
> > 
> > Hi Bertrand,
> > 
> > On 14/10/2020 11:41, 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.
> > 
> > I think we should update SUPPORT.MD to say we will not security support those processors. Stefano, what do you think?
> 
> That could make sense to do that yes.
> If Stefano confirms then i can do this in a v2

Yes, I confirm

 
> >> An equivalent warning is already given to the user by KVM on cores
> >> affected by this errata.
> > 
> > I don't seem to find the warning in Linux. Do you have a link?
> 
> sure:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abf532cceaca9c21a148498091f87de1b8ae9b49
> 
> > 
> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >> ---
> >>  xen/arch/arm/cpuerrata.c | 21 +++++++++++++++++++++
> >>  1 file changed, 21 insertions(+)
> >> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> >> index 6c09017515..8f9ab6dde1 100644
> >> --- a/xen/arch/arm/cpuerrata.c
> >> +++ b/xen/arch/arm/cpuerrata.c
> >> @@ -240,6 +240,26 @@ static int enable_ic_inv_hardening(void *data)
> >>    #endif
> >>  +#ifdef CONFIG_ARM64_ERRATUM_832075
> >> +
> >> +static int warn_device_load_acquire_errata(void *data)
> >> +{
> >> +    static bool warned = false;
> >> +
> >> +    if ( !warned )
> >> +    {
> >> +        warning_add("This CPU is affected by the errata 832075.\n"
> >> +                    "Guests without required CPU erratum workarounds\n"
> >> +                    "can deadlock the system!\n"
> >> +                    "Only trusted guests should be used on this system.\n");
> >> +        warned = true;
> > 
> > I was going to suggest to use WARN_ON_ONCE() but it looks like it never made upstream :(.
> 
> I did do this as it was done in the smc warning function (that’s why i pushed a patch for it).
> 
> Cheers
> Bertrand
> 
> > 
> >> +    }
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +#endif
> >> +
> >>  #ifdef CONFIG_ARM_SSBD
> >>    enum ssbd_state ssbd_state = ARM_SSBD_RUNTIME;
> >> @@ -419,6 +439,7 @@ static const struct arm_cpu_capabilities arm_errata[] = {
> >>          .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
> >>          MIDR_RANGE(MIDR_CORTEX_A57, 0x00,
> >>                     (1 << MIDR_VARIANT_SHIFT) | 2),
> >> +        .enable = warn_device_load_acquire_errata,
> >>      },
> >>  #endif
> >>  #ifdef CONFIG_ARM64_ERRATUM_834220
> > 
> > -- 
> > Julien Grall
> 
>