Re: [Qemu-devel] [QEMU-PPC] [PATCH V4 2/2] ppc/spapr-caps: For pseries-2.12 change spapr-cap defaults

[David Gibson]

On Fri, Feb 16, 2018 at 01:33:28PM +1100, Suraj Jitindar Singh wrote:
> For the pseries-2.12 machine type, make the spapr-caps SPAPR_CAP_CFPC
> and SPAPR_CAP_SBBC default to workaround. Thus if the host is capable
> the guest will be able to take advantage of these workarounds by default.
> Otherwise if the host doesn't have these capabilities qemu will fail to
> start and they will have to be explicitly disabled on the command line
> with:
> -machine pseries,cap-cfpc=broken,cap-sbbc=broken
> 
> Signed-off-by: Suraj Jitindar Singh <address@hidden>

I've decided to hold off on this for a little bit longer for two
reasons: 1) I realised how badly it will break things for machines
which don't yet updated firmware (both POWER8 and POWER9) which
includes a very large proportion of test machines.  2) I've looked
more closely at what x86 has done.  For themthe mitigations are
controlled by CPU options, not machine level, but the "plain" types
are still the unmitigated variants, with variants for the versions
with microcode updated with mitigations.  See
https://www.qemu.org/2018/02/14/qemu-2-11-1-and-spectre-update/

> 
> ---
> 
> V2 -> V3:
>  - Set caps to workaround in the class default rather than the
>    pseries-2.12 initialiser.
> ---
>  hw/ppc/spapr.c      |  6 ++++--
>  hw/ppc/spapr_caps.c | 10 ++++++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 83c9d66dd5..69f59aabf1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3915,8 +3915,8 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>      smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>      smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_ON;
> -    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
> -    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
> +    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
> +    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
>      smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN;
>      spapr_caps_add_properties(smc, &error_abort);
>  }
> @@ -4000,6 +4000,8 @@ static void 
> spapr_machine_2_11_class_options(MachineClass *mc)
>  
>      spapr_machine_2_12_class_options(mc);
>      smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_ON;
> +    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
> +    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
>  }
>  
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 99a4b71d19..7b0ecb3eca 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -283,11 +283,21 @@ static sPAPRCapabilities 
> default_caps_with_cpu(sPAPRMachineState *spapr,
>  
>      caps = smc->default_caps;
>  
> +    if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00,
> +                          0, spapr->max_compat_pvr)) {
> +        caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
> +    }
> +
>      if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
>                            0, spapr->max_compat_pvr)) {
>          caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>      }
>  
> +    if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06_PLUS,
> +                          0, spapr->max_compat_pvr)) {
> +        caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_BROKEN;
> +    }
> +
>      if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
>                            0, spapr->max_compat_pvr)) {
>          caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_OFF;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature