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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

> On 14 Oct 2020, at 12:11, Julien Grall <julien@xxxxxxx> 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

> 
>> 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@xxxxxxx>
>> ---
>>  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