[Xen-devel,06/13] xen/arm: Add ARCH_WORKAROUND_2 support for guests

Message ID 20180522174254.27551-7-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: SSBD (aka Spectre-v4) mitigation (XSA-263)
Related show

Commit Message

Julien Grall May 22, 2018, 5:42 p.m.
In order to offer ARCH_WORKAROUND_2 support to guests, we need to track the
state of the workaround per-vCPU. The field 'pad' in cpu_info is now
repurposed to store flags easily accessible in assembly.

As the hypervisor will always run with the workaround enabled, we may
need to enable (on guest exit) or disable (on guest entry) the
workaround.

A follow-up patch will add fastpath for the workaround for arm64 guests.

This is part of XSA-263.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain.c         |  8 ++++++++
 xen/arch/arm/traps.c          | 20 ++++++++++++++++++++
 xen/arch/arm/vsmc.c           | 37 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/current.h |  6 +++++-
 4 files changed, 70 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini May 23, 2018, 11:24 p.m. | #1
On Tue, 22 May 2018, Julien Grall wrote:
> In order to offer ARCH_WORKAROUND_2 support to guests, we need to track the
> state of the workaround per-vCPU. The field 'pad' in cpu_info is now
> repurposed to store flags easily accessible in assembly.
> 
> As the hypervisor will always run with the workaround enabled, we may
> need to enable (on guest exit) or disable (on guest entry) the
> workaround.
> 
> A follow-up patch will add fastpath for the workaround for arm64 guests.
> 
> This is part of XSA-263.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/domain.c         |  8 ++++++++
>  xen/arch/arm/traps.c          | 20 ++++++++++++++++++++
>  xen/arch/arm/vsmc.c           | 37 +++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/current.h |  6 +++++-
>  4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index e7b33e92fb..9168195a9c 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -21,6 +21,7 @@
>  #include <xen/wait.h>
>  
>  #include <asm/alternative.h>
> +#include <asm/cpuerrata.h>
>  #include <asm/cpufeature.h>
>  #include <asm/current.h>
>  #include <asm/event.h>
> @@ -575,6 +576,13 @@ int vcpu_initialise(struct vcpu *v)
>      if ( (rc = vcpu_vtimer_init(v)) != 0 )
>          goto fail;
>  
> +    /*
> +     * The workaround 2 (i.e SSBD mitigation) is enabled by default if
> +     * supported.
> +     */
> +    if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
> +        v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
> +
>      return rc;
>  
>  fail:
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 5c18e918b0..020b0b8eef 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2011,10 +2011,23 @@ inject_abt:
>          inject_iabt_exception(regs, gva, hsr.len);
>  }
>  
> +static inline bool needs_ssbd_flip(struct vcpu *v)
> +{
> +    if ( !check_workaround_ssbd() )
> +        return false;

Why not check on get_ssbd_state() == ARM_SSBD_RUNTIME?  I am confused on
when is the right time to use the cpu capability check
(check_workaround_ssbd), when is the right time to call get_ssbd_state()
and when is the right time to call cpu_require_ssbd_mitigation().


> +    return !((v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG) &&
> +             cpu_require_ssbd_mitigation());

It looks like this won't do as intended when v->arch.cpu_info->flags = 0
and cpu_require_ssbd_mitigation() returns false, am I right?

Maybe needs_ssbd_flip() should be implemented as follows:

  return get_ssbd_state() == ARM_SSBD_RUNTIME &&
    !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)


> +}
> +
>  static void enter_hypervisor_head(struct cpu_user_regs *regs)
>  {
>      if ( guest_mode(regs) )
>      {
> +        /* If the guest has disabled the workaround, bring it back on. */
> +        if ( needs_ssbd_flip(current) )
> +            arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +
>          /*
>           * If we pended a virtual abort, preserve it until it gets cleared.
>           * See ARM ARM DDI 0487A.j D1.14.3 (Virtual Interrupts) for details,
> @@ -2260,6 +2273,13 @@ void leave_hypervisor_tail(void)
>               */
>              SYNCHRONIZE_SERROR(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
>  
> +            /*
> +             * The hypervisor runs with the workaround always present.
> +             * If the guest wants it disabled, so be it...
> +             */
> +            if ( needs_ssbd_flip(current) )
> +                arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL);
> +
>              return;
>          }
>          local_irq_enable();
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 40a80d5760..c4ccae6030 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -18,6 +18,7 @@
>  #include <xen/lib.h>
>  #include <xen/types.h>
>  #include <public/arch-arm/smccc.h>
> +#include <asm/cpuerrata.h>
>  #include <asm/cpufeature.h>
>  #include <asm/monitor.h>
>  #include <asm/regs.h>
> @@ -104,6 +105,23 @@ static bool handle_arch(struct cpu_user_regs *regs)
>              if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
>                  ret = 0;
>              break;
> +        case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
> +            switch ( get_ssbd_state() )
> +            {
> +            case ARM_SSBD_UNKNOWN:
> +            case ARM_SSBD_FORCE_DISABLE:
> +                break;
> +
> +            case ARM_SSBD_RUNTIME:
> +                ret = ARM_SMCCC_SUCCESS;
> +                break;
> +
> +            case ARM_SSBD_FORCE_ENABLE:
> +            case ARM_SSBD_MITIGATED:
> +                ret = ARM_SMCCC_NOT_REQUIRED;
> +                break;
> +            }
> +            break;
>          }
>  
>          set_user_reg(regs, 0, ret);
> @@ -114,6 +132,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
>      case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
>          /* No return value */
>          return true;
> +
> +    case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
> +    {
> +        bool enable = (uint32_t)get_user_reg(regs, 1);
> +
> +        /*
> +         * ARM_WORKAROUND_2_FID should only be called when mitigation
> +         * state can be changed at runtime.
> +         */
> +        if ( unlikely(get_ssbd_state() != ARM_SSBD_RUNTIME) )
> +            return true;
> +
> +        if ( enable )
> +            get_cpu_info()->flags |= CPUINFO_WORKAROUND_2_FLAG;
> +        else
> +            get_cpu_info()->flags &= ~CPUINFO_WORKAROUND_2_FLAG;
> +
> +        return true;
> +    }
>      }
>  
>      return false;
> diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> index 7a0971fdea..f9819b34fc 100644
> --- a/xen/include/asm-arm/current.h
> +++ b/xen/include/asm-arm/current.h
> @@ -7,6 +7,10 @@
>  #include <asm/percpu.h>
>  #include <asm/processor.h>
>  
> +/* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */
> +#define CPUINFO_WORKAROUND_2_FLAG_SHIFT   0
> +#define CPUINFO_WORKAROUND_2_FLAG (_AC(1, U) << CPUINFO_WORKAROUND_2_FLAG_SHIFT)
> +
>  #ifndef __ASSEMBLY__
>  
>  struct vcpu;
> @@ -21,7 +25,7 @@ DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
>  struct cpu_info {
>      struct cpu_user_regs guest_cpu_user_regs;
>      unsigned long elr;
> -    unsigned int pad;
> +    uint32_t flags;
>  };
>  
>  static inline struct cpu_info *get_cpu_info(void)
> -- 
> 2.11.0
>
Stefano Stabellini May 24, 2018, 12:40 a.m. | #2
On Wed, 23 May 2018, Stefano Stabellini wrote:
> On Tue, 22 May 2018, Julien Grall wrote:
> > In order to offer ARCH_WORKAROUND_2 support to guests, we need to track the
> > state of the workaround per-vCPU. The field 'pad' in cpu_info is now
> > repurposed to store flags easily accessible in assembly.
> > 
> > As the hypervisor will always run with the workaround enabled, we may
> > need to enable (on guest exit) or disable (on guest entry) the
> > workaround.
> > 
> > A follow-up patch will add fastpath for the workaround for arm64 guests.
> > 
> > This is part of XSA-263.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > ---
> >  xen/arch/arm/domain.c         |  8 ++++++++
> >  xen/arch/arm/traps.c          | 20 ++++++++++++++++++++
> >  xen/arch/arm/vsmc.c           | 37 +++++++++++++++++++++++++++++++++++++
> >  xen/include/asm-arm/current.h |  6 +++++-
> >  4 files changed, 70 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index e7b33e92fb..9168195a9c 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -21,6 +21,7 @@
> >  #include <xen/wait.h>
> >  
> >  #include <asm/alternative.h>
> > +#include <asm/cpuerrata.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/current.h>
> >  #include <asm/event.h>
> > @@ -575,6 +576,13 @@ int vcpu_initialise(struct vcpu *v)
> >      if ( (rc = vcpu_vtimer_init(v)) != 0 )
> >          goto fail;
> >  
> > +    /*
> > +     * The workaround 2 (i.e SSBD mitigation) is enabled by default if
> > +     * supported.
> > +     */
> > +    if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
> > +        v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
> > +
> >      return rc;
> >  
> >  fail:
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 5c18e918b0..020b0b8eef 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2011,10 +2011,23 @@ inject_abt:
> >          inject_iabt_exception(regs, gva, hsr.len);
> >  }
> >  
> > +static inline bool needs_ssbd_flip(struct vcpu *v)
> > +{
> > +    if ( !check_workaround_ssbd() )
> > +        return false;
> 
> Why not check on get_ssbd_state() == ARM_SSBD_RUNTIME?  I am confused on
> when is the right time to use the cpu capability check
> (check_workaround_ssbd), when is the right time to call get_ssbd_state()
> and when is the right time to call cpu_require_ssbd_mitigation().
> 
> 
> > +    return !((v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG) &&
> > +             cpu_require_ssbd_mitigation());
> 
> It looks like this won't do as intended when v->arch.cpu_info->flags = 0
> and cpu_require_ssbd_mitigation() returns false, am I right?
> 
> Maybe needs_ssbd_flip() should be implemented as follows:
> 
>   return get_ssbd_state() == ARM_SSBD_RUNTIME &&
>     !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)

With the intention of supporting systems where not all CPUs need/have
the workaround, then it should be:

   return cpu_require_ssbd_mitigation() &&
     !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)
 

> > +}
> > +
> >  static void enter_hypervisor_head(struct cpu_user_regs *regs)
> >  {
> >      if ( guest_mode(regs) )
> >      {
> > +        /* If the guest has disabled the workaround, bring it back on. */
> > +        if ( needs_ssbd_flip(current) )
> > +            arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> > +
> >          /*
> >           * If we pended a virtual abort, preserve it until it gets cleared.
> >           * See ARM ARM DDI 0487A.j D1.14.3 (Virtual Interrupts) for details,
> > @@ -2260,6 +2273,13 @@ void leave_hypervisor_tail(void)
> >               */
> >              SYNCHRONIZE_SERROR(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
> >  
> > +            /*
> > +             * The hypervisor runs with the workaround always present.
> > +             * If the guest wants it disabled, so be it...
> > +             */
> > +            if ( needs_ssbd_flip(current) )
> > +                arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL);
> > +
> >              return;
> >          }
> >          local_irq_enable();
> > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> > index 40a80d5760..c4ccae6030 100644
> > --- a/xen/arch/arm/vsmc.c
> > +++ b/xen/arch/arm/vsmc.c
> > @@ -18,6 +18,7 @@
> >  #include <xen/lib.h>
> >  #include <xen/types.h>
> >  #include <public/arch-arm/smccc.h>
> > +#include <asm/cpuerrata.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/monitor.h>
> >  #include <asm/regs.h>
> > @@ -104,6 +105,23 @@ static bool handle_arch(struct cpu_user_regs *regs)
> >              if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
> >                  ret = 0;
> >              break;
> > +        case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
> > +            switch ( get_ssbd_state() )
> > +            {
> > +            case ARM_SSBD_UNKNOWN:
> > +            case ARM_SSBD_FORCE_DISABLE:
> > +                break;
> > +
> > +            case ARM_SSBD_RUNTIME:
> > +                ret = ARM_SMCCC_SUCCESS;
> > +                break;
> > +
> > +            case ARM_SSBD_FORCE_ENABLE:
> > +            case ARM_SSBD_MITIGATED:
> > +                ret = ARM_SMCCC_NOT_REQUIRED;
> > +                break;
> > +            }
> > +            break;
> >          }
> >  
> >          set_user_reg(regs, 0, ret);
> > @@ -114,6 +132,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
> >      case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
> >          /* No return value */
> >          return true;
> > +
> > +    case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
> > +    {
> > +        bool enable = (uint32_t)get_user_reg(regs, 1);
> > +
> > +        /*
> > +         * ARM_WORKAROUND_2_FID should only be called when mitigation
> > +         * state can be changed at runtime.
> > +         */
> > +        if ( unlikely(get_ssbd_state() != ARM_SSBD_RUNTIME) )
> > +            return true;
> > +
> > +        if ( enable )
> > +            get_cpu_info()->flags |= CPUINFO_WORKAROUND_2_FLAG;
> > +        else
> > +            get_cpu_info()->flags &= ~CPUINFO_WORKAROUND_2_FLAG;
> > +
> > +        return true;
> > +    }
> >      }
> >  
> >      return false;
> > diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> > index 7a0971fdea..f9819b34fc 100644
> > --- a/xen/include/asm-arm/current.h
> > +++ b/xen/include/asm-arm/current.h
> > @@ -7,6 +7,10 @@
> >  #include <asm/percpu.h>
> >  #include <asm/processor.h>
> >  
> > +/* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */
> > +#define CPUINFO_WORKAROUND_2_FLAG_SHIFT   0
> > +#define CPUINFO_WORKAROUND_2_FLAG (_AC(1, U) << CPUINFO_WORKAROUND_2_FLAG_SHIFT)
> > +
> >  #ifndef __ASSEMBLY__
> >  
> >  struct vcpu;
> > @@ -21,7 +25,7 @@ DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
> >  struct cpu_info {
> >      struct cpu_user_regs guest_cpu_user_regs;
> >      unsigned long elr;
> > -    unsigned int pad;
> > +    uint32_t flags;
> >  };
> >  
> >  static inline struct cpu_info *get_cpu_info(void)
> > -- 
> > 2.11.0
> > 
>
Julien Grall May 24, 2018, 10 a.m. | #3
Hi Stefano,

On 24/05/18 01:40, Stefano Stabellini wrote:
> On Wed, 23 May 2018, Stefano Stabellini wrote:
>> On Tue, 22 May 2018, Julien Grall wrote:
>>> In order to offer ARCH_WORKAROUND_2 support to guests, we need to track the
>>> state of the workaround per-vCPU. The field 'pad' in cpu_info is now
>>> repurposed to store flags easily accessible in assembly.
>>>
>>> As the hypervisor will always run with the workaround enabled, we may
>>> need to enable (on guest exit) or disable (on guest entry) the
>>> workaround.
>>>
>>> A follow-up patch will add fastpath for the workaround for arm64 guests.
>>>
>>> This is part of XSA-263.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>> ---
>>>   xen/arch/arm/domain.c         |  8 ++++++++
>>>   xen/arch/arm/traps.c          | 20 ++++++++++++++++++++
>>>   xen/arch/arm/vsmc.c           | 37 +++++++++++++++++++++++++++++++++++++
>>>   xen/include/asm-arm/current.h |  6 +++++-
>>>   4 files changed, 70 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>> index e7b33e92fb..9168195a9c 100644
>>> --- a/xen/arch/arm/domain.c
>>> +++ b/xen/arch/arm/domain.c
>>> @@ -21,6 +21,7 @@
>>>   #include <xen/wait.h>
>>>   
>>>   #include <asm/alternative.h>
>>> +#include <asm/cpuerrata.h>
>>>   #include <asm/cpufeature.h>
>>>   #include <asm/current.h>
>>>   #include <asm/event.h>
>>> @@ -575,6 +576,13 @@ int vcpu_initialise(struct vcpu *v)
>>>       if ( (rc = vcpu_vtimer_init(v)) != 0 )
>>>           goto fail;
>>>   
>>> +    /*
>>> +     * The workaround 2 (i.e SSBD mitigation) is enabled by default if
>>> +     * supported.
>>> +     */
>>> +    if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
>>> +        v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
>>> +
>>>       return rc;
>>>   
>>>   fail:
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index 5c18e918b0..020b0b8eef 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -2011,10 +2011,23 @@ inject_abt:
>>>           inject_iabt_exception(regs, gva, hsr.len);
>>>   }
>>>   
>>> +static inline bool needs_ssbd_flip(struct vcpu *v)
>>> +{
>>> +    if ( !check_workaround_ssbd() )
>>> +        return false;
>>
>> Why not check on get_ssbd_state() == ARM_SSBD_RUNTIME? 

get_ssbd_state() would introduce an overhead for each entry/exit even on 
platform not affected. check_workaround_ssbd() remove this overhead by 
using an alternative.

> I am confused on
>> when is the right time to use the cpu capability check
>> (check_workaround_ssbd), when is the right time to call get_ssbd_state()
>> and when is the right time to call cpu_require_ssbd_mitigation().

See my answer in the previous patches.

>>
>>
>>> +    return !((v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG) &&
>>> +             cpu_require_ssbd_mitigation());
>>
>> It looks like this won't do as intended when v->arch.cpu_info->flags = 0
>> and cpu_require_ssbd_mitigation() returns false, am I right?
>>
>> Maybe needs_ssbd_flip() should be implemented as follows:
>>
>>    return get_ssbd_state() == ARM_SSBD_RUNTIME &&
>>      !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)
> 
> With the intention of supporting systems where not all CPUs need/have
> the workaround, then it should be:
> 
>     return cpu_require_ssbd_mitigation() &&
>       !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)

Yes, I did the exact same error I reported to Marc on the KVM side :/. I 
will update the patch.

Cheers,
Stefano Stabellini May 25, 2018, 8:51 p.m. | #4
On Thu, 24 May 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 24/05/18 01:40, Stefano Stabellini wrote:
> > On Wed, 23 May 2018, Stefano Stabellini wrote:
> > > On Tue, 22 May 2018, Julien Grall wrote:
> > > > In order to offer ARCH_WORKAROUND_2 support to guests, we need to track
> > > > the
> > > > state of the workaround per-vCPU. The field 'pad' in cpu_info is now
> > > > repurposed to store flags easily accessible in assembly.
> > > > 
> > > > As the hypervisor will always run with the workaround enabled, we may
> > > > need to enable (on guest exit) or disable (on guest entry) the
> > > > workaround.
> > > > 
> > > > A follow-up patch will add fastpath for the workaround for arm64 guests.
> > > > 
> > > > This is part of XSA-263.
> > > > 
> > > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > > ---
> > > >   xen/arch/arm/domain.c         |  8 ++++++++
> > > >   xen/arch/arm/traps.c          | 20 ++++++++++++++++++++
> > > >   xen/arch/arm/vsmc.c           | 37
> > > > +++++++++++++++++++++++++++++++++++++
> > > >   xen/include/asm-arm/current.h |  6 +++++-
> > > >   4 files changed, 70 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > > > index e7b33e92fb..9168195a9c 100644
> > > > --- a/xen/arch/arm/domain.c
> > > > +++ b/xen/arch/arm/domain.c
> > > > @@ -21,6 +21,7 @@
> > > >   #include <xen/wait.h>
> > > >     #include <asm/alternative.h>
> > > > +#include <asm/cpuerrata.h>
> > > >   #include <asm/cpufeature.h>
> > > >   #include <asm/current.h>
> > > >   #include <asm/event.h>
> > > > @@ -575,6 +576,13 @@ int vcpu_initialise(struct vcpu *v)
> > > >       if ( (rc = vcpu_vtimer_init(v)) != 0 )
> > > >           goto fail;
> > > >   +    /*
> > > > +     * The workaround 2 (i.e SSBD mitigation) is enabled by default if
> > > > +     * supported.
> > > > +     */
> > > > +    if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
> > > > +        v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
> > > > +
> > > >       return rc;
> > > >     fail:
> > > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > > index 5c18e918b0..020b0b8eef 100644
> > > > --- a/xen/arch/arm/traps.c
> > > > +++ b/xen/arch/arm/traps.c
> > > > @@ -2011,10 +2011,23 @@ inject_abt:
> > > >           inject_iabt_exception(regs, gva, hsr.len);
> > > >   }
> > > >   +static inline bool needs_ssbd_flip(struct vcpu *v)
> > > > +{
> > > > +    if ( !check_workaround_ssbd() )
> > > > +        return false;
> > > 
> > > Why not check on get_ssbd_state() == ARM_SSBD_RUNTIME? 
> 
> get_ssbd_state() would introduce an overhead for each entry/exit even on
> platform not affected. check_workaround_ssbd() remove this overhead by using
> an alternative.

Ah yes, great idea.


> > I am confused on
> > > when is the right time to use the cpu capability check
> > > (check_workaround_ssbd), when is the right time to call get_ssbd_state()
> > > and when is the right time to call cpu_require_ssbd_mitigation().
> 
> See my answer in the previous patches.

I understand now, I didn't realize we wanted to go into the details of
improving big.LITTLE scenarios with different erratas on the differnt
CPUs.


> > > 
> > > 
> > > > +    return !((v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG) &&
> > > > +             cpu_require_ssbd_mitigation());
> > > 
> > > It looks like this won't do as intended when v->arch.cpu_info->flags = 0
> > > and cpu_require_ssbd_mitigation() returns false, am I right?
> > > 
> > > Maybe needs_ssbd_flip() should be implemented as follows:
> > > 
> > >    return get_ssbd_state() == ARM_SSBD_RUNTIME &&
> > >      !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)
> > 
> > With the intention of supporting systems where not all CPUs need/have
> > the workaround, then it should be:
> > 
> >     return cpu_require_ssbd_mitigation() &&
> >       !(v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG)
> 
> Yes, I did the exact same error I reported to Marc on the KVM side :/. I will
> update the patch.

Patch

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index e7b33e92fb..9168195a9c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -21,6 +21,7 @@ 
 #include <xen/wait.h>
 
 #include <asm/alternative.h>
+#include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
 #include <asm/current.h>
 #include <asm/event.h>
@@ -575,6 +576,13 @@  int vcpu_initialise(struct vcpu *v)
     if ( (rc = vcpu_vtimer_init(v)) != 0 )
         goto fail;
 
+    /*
+     * The workaround 2 (i.e SSBD mitigation) is enabled by default if
+     * supported.
+     */
+    if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
+        v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
+
     return rc;
 
 fail:
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 5c18e918b0..020b0b8eef 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2011,10 +2011,23 @@  inject_abt:
         inject_iabt_exception(regs, gva, hsr.len);
 }
 
+static inline bool needs_ssbd_flip(struct vcpu *v)
+{
+    if ( !check_workaround_ssbd() )
+        return false;
+
+    return !((v->arch.cpu_info->flags & CPUINFO_WORKAROUND_2_FLAG) &&
+             cpu_require_ssbd_mitigation());
+}
+
 static void enter_hypervisor_head(struct cpu_user_regs *regs)
 {
     if ( guest_mode(regs) )
     {
+        /* If the guest has disabled the workaround, bring it back on. */
+        if ( needs_ssbd_flip(current) )
+            arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
+
         /*
          * If we pended a virtual abort, preserve it until it gets cleared.
          * See ARM ARM DDI 0487A.j D1.14.3 (Virtual Interrupts) for details,
@@ -2260,6 +2273,13 @@  void leave_hypervisor_tail(void)
              */
             SYNCHRONIZE_SERROR(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
 
+            /*
+             * The hypervisor runs with the workaround always present.
+             * If the guest wants it disabled, so be it...
+             */
+            if ( needs_ssbd_flip(current) )
+                arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL);
+
             return;
         }
         local_irq_enable();
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 40a80d5760..c4ccae6030 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -18,6 +18,7 @@ 
 #include <xen/lib.h>
 #include <xen/types.h>
 #include <public/arch-arm/smccc.h>
+#include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
 #include <asm/monitor.h>
 #include <asm/regs.h>
@@ -104,6 +105,23 @@  static bool handle_arch(struct cpu_user_regs *regs)
             if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
                 ret = 0;
             break;
+        case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
+            switch ( get_ssbd_state() )
+            {
+            case ARM_SSBD_UNKNOWN:
+            case ARM_SSBD_FORCE_DISABLE:
+                break;
+
+            case ARM_SSBD_RUNTIME:
+                ret = ARM_SMCCC_SUCCESS;
+                break;
+
+            case ARM_SSBD_FORCE_ENABLE:
+            case ARM_SSBD_MITIGATED:
+                ret = ARM_SMCCC_NOT_REQUIRED;
+                break;
+            }
+            break;
         }
 
         set_user_reg(regs, 0, ret);
@@ -114,6 +132,25 @@  static bool handle_arch(struct cpu_user_regs *regs)
     case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
         /* No return value */
         return true;
+
+    case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
+    {
+        bool enable = (uint32_t)get_user_reg(regs, 1);
+
+        /*
+         * ARM_WORKAROUND_2_FID should only be called when mitigation
+         * state can be changed at runtime.
+         */
+        if ( unlikely(get_ssbd_state() != ARM_SSBD_RUNTIME) )
+            return true;
+
+        if ( enable )
+            get_cpu_info()->flags |= CPUINFO_WORKAROUND_2_FLAG;
+        else
+            get_cpu_info()->flags &= ~CPUINFO_WORKAROUND_2_FLAG;
+
+        return true;
+    }
     }
 
     return false;
diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 7a0971fdea..f9819b34fc 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -7,6 +7,10 @@ 
 #include <asm/percpu.h>
 #include <asm/processor.h>
 
+/* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */
+#define CPUINFO_WORKAROUND_2_FLAG_SHIFT   0
+#define CPUINFO_WORKAROUND_2_FLAG (_AC(1, U) << CPUINFO_WORKAROUND_2_FLAG_SHIFT)
+
 #ifndef __ASSEMBLY__
 
 struct vcpu;
@@ -21,7 +25,7 @@  DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
 struct cpu_info {
     struct cpu_user_regs guest_cpu_user_regs;
     unsigned long elr;
-    unsigned int pad;
+    uint32_t flags;
 };
 
 static inline struct cpu_info *get_cpu_info(void)