[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 05/13] xen/arm: Add command line option to control SSBD mitigation



On Tue, 5 Jun 2018, Julien Grall wrote:
> On a system where the firmware implements ARCH_WORKAROUND_2, it may be
> useful to either permanently enable or disable the workaround for cases
> where the user decides that they'd rather not get a trap overhead, and
> keep the mitigation permanently on or off instead of switching it on
> exception entry/exit. In any case, default to mitigation being enabled.
> 
> The new command line option is implemented as list of one option to
> follow x86 option and also allow to extend it more easily in the future.
> 
> Note that for convenience, the full implemention of the workaround is
> done in the .matches callback.
> 
> Lastly, a accessor is provided to know the state of the mitigation.
> 
> After this patch, there are 3 methods complementing each other to find the
> state of the mitigation:
>     - The capability ARM_SSBD indicates the platform is affected by the
>       vulnerability. This will also return false if the user decide to force
>       disabled the mitigation (spec-ctrl="ssbd=force-disable"). The
>       capability is useful for putting shortcut in place using alternative.
>     - ssbd_state indicates the global state of the mitigation (e.g
>       unknown, force enable...). The global state is required to report
>       the state to a guest.
>     - The per-cpu ssbd_callback_required indicates whether a pCPU
>       requires to call the SMC. This allows to shortcut SMC call
>       and save an entry/exit to EL3.
> 
> This is part of XSA-263.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
>     Changes in v2:
>         - Move out some code to the previous patch.
>         - Update the commit message with more background
> ---
>  docs/misc/xen-command-line.markdown | 18 ++++++++
>  xen/arch/arm/cpuerrata.c            | 91 
> +++++++++++++++++++++++++++++++++----
>  xen/include/asm-arm/cpuerrata.h     | 21 +++++++++
>  3 files changed, 122 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 8712a833a2..962028b6ed 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1756,6 +1756,24 @@ enforces the maximum theoretically necessary timeout 
> of 670ms. Any number
>  is being interpreted as a custom timeout in milliseconds. Zero or boolean
>  false disable the quirk workaround, which is also the default.
>  
> +### spec-ctrl (Arm)
> +> `= List of [ ssbd=force-disable|runtime|force-enable ]`
> +
> +Controls for speculative execution sidechannel mitigations.
> +
> +The option `ssbd=` is used to control the state of Speculative Store
> +Bypass Disable (SSBD) mitigation.
> +
> +* `ssbd=force-disable` will keep the mitigation permanently off. The guest
> +will not be able to control the state of the mitigation.
> +* `ssbd=runtime` will always turn on the mitigation when running in the
> +hypervisor context. The guest will be to turn on/off the mitigation for
> +itself by using the firmware interface ARCH\_WORKAROUND\_2.
> +* `ssbd=force-enable` will keep the mitigation permanently on. The guest will
> +not be able to control the state of the mitigation.
> +
> +By default SSBD will be mitigated at runtime (i.e `ssbd=runtime`).
> +
>  ### spec-ctrl (x86)
>  > `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
>  >              bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd}=<bool> ]`
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index aa86c7c0fe..4292008692 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -237,6 +237,41 @@ static int enable_ic_inv_hardening(void *data)
>  
>  #ifdef CONFIG_ARM_SSBD
>  
> +enum ssbd_state ssbd_state = ARM_SSBD_RUNTIME;
> +
> +static int __init parse_spec_ctrl(const char *s)
> +{
> +    const char *ss;
> +    int rc = 0;
> +
> +    do {
> +        ss = strchr(s, ',');
> +        if ( !ss )
> +            ss = strchr(s, '\0');
> +
> +        if ( !strncmp(s, "ssbd=", 5) )
> +        {
> +            s += 5;
> +
> +            if ( !strncmp(s, "force-disable", ss - s) )
> +                ssbd_state = ARM_SSBD_FORCE_DISABLE;
> +            else if ( !strncmp(s, "runtime", ss - s) )
> +                ssbd_state = ARM_SSBD_RUNTIME;
> +            else if ( !strncmp(s, "force-enable", ss - s) )
> +                ssbd_state = ARM_SSBD_FORCE_ENABLE;
> +            else
> +                rc = -EINVAL;
> +        }
> +        else
> +            rc = -EINVAL;
> +
> +        s = ss + 1;
> +    } while ( *ss );
> +
> +    return rc;
> +}
> +custom_param("spec-ctrl", parse_spec_ctrl);
> +
>  /*
>   * Assembly code may use the variable directly, so we need to make sure
>   * it fits in a register.
> @@ -251,19 +286,17 @@ static bool has_ssbd_mitigation(const struct 
> arm_cpu_capabilities *entry)
>      if ( smccc_ver < SMCCC_VERSION(1, 1) )
>          return false;
>  
> -    /*
> -     * The probe function return value is either negative (unsupported
> -     * or mitigated), positive (unaffected), or zero (requires
> -     * mitigation). We only need to do anything in the last case.
> -     */
>      arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
>                        ARM_SMCCC_ARCH_WORKAROUND_2_FID, &res);
> +

spurious change


>      switch ( (int)res.a0 )
>      {
>      case ARM_SMCCC_NOT_SUPPORTED:
> +        ssbd_state = ARM_SSBD_UNKNOWN;
>          return false;
>  
>      case ARM_SMCCC_NOT_REQUIRED:
> +        ssbd_state = ARM_SSBD_MITIGATED;
>          return false;
>  
>      case ARM_SMCCC_SUCCESS:
> @@ -271,7 +304,7 @@ static bool has_ssbd_mitigation(const struct 
> arm_cpu_capabilities *entry)
>          break;
>  
>      case 1: /* Mitigation not required on this CPU. */
> -        required = true;
> +        required = false;
>          break;
>  
>      default:
> @@ -279,8 +312,49 @@ static bool has_ssbd_mitigation(const struct 
> arm_cpu_capabilities *entry)
>          return false;
>      }
>  
> -    if ( required )
> -        this_cpu(ssbd_callback_required) = 1;
> +    switch ( ssbd_state )
> +    {
> +    case ARM_SSBD_FORCE_DISABLE:
> +    {
> +        static bool once = true;
> +
> +        if ( once )
> +            printk("%s disabled from command-line\n", entry->desc);
> +        once = false;
> +
> +        arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL);
> +        required = false;
> +
> +        break;
> +    }
> +
> +    case ARM_SSBD_RUNTIME:
> +        if ( required )
> +        {
> +            this_cpu(ssbd_callback_required) = 1;
> +            arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +        }
> +
> +        break;
> +
> +    case ARM_SSBD_FORCE_ENABLE:
> +    {
> +        static bool once = true;
> +
> +        if ( once )
> +            printk("%s forced from command-line\n", entry->desc);
> +        once = false;
> +
> +        arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +        required = true;
> +
> +        break;
> +    }
> +
> +    default:
> +        ASSERT_UNREACHABLE();
> +        return false;
> +    }
>  
>      return required;
>  }
> @@ -389,6 +463,7 @@ static const struct arm_cpu_capabilities arm_errata[] = {
>  #endif
>  #ifdef CONFIG_ARM_SSBD
>      {
> +        .desc = "Speculative Store Bypass Disabled",
>          .capability = ARM_SSBD,
>          .matches = has_ssbd_mitigation,
>      },
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index e628d3ff56..7fbb3dc0be 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -31,10 +31,26 @@ CHECK_WORKAROUND_HELPER(ssbd, ARM_SSBD, CONFIG_ARM_SSBD)
>  
>  #undef CHECK_WORKAROUND_HELPER
>  
> +enum ssbd_state
> +{
> +    ARM_SSBD_UNKNOWN,
> +    ARM_SSBD_FORCE_DISABLE,
> +    ARM_SSBD_RUNTIME,
> +    ARM_SSBD_FORCE_ENABLE,
> +    ARM_SSBD_MITIGATED,
> +};
> +
>  #ifdef CONFIG_ARM_SSBD
>  
>  #include <asm/current.h>
>  
> +extern enum ssbd_state ssbd_state;
> +
> +static inline enum ssbd_state get_ssbd_state(void)
> +{
> +    return ssbd_state;
> +}
> +
>  DECLARE_PER_CPU(register_t, ssbd_callback_required);
>  
>  static inline bool cpu_require_ssbd_mitigation(void)
> @@ -49,6 +65,11 @@ static inline bool cpu_require_ssbd_mitigation(void)
>      return false;
>  }
>  
> +static inline enum ssbd_state get_sbdd_state(void)

the mistype is still present


> +{
> +    return ARM_SSBD_UNKNOWN;
> +}
> +
>  #endif
>  
>  #endif /* __ARM_CPUERRATA_H__ */
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.