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

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Wed, 2018-02-14 at 15:20 +0100, Greg Kurz wrote:
> On Wed, 14 Feb 2018 17:51:35 +1100
> Suraj Jitindar Singh <sjitindarsingh@gmail.com> 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 <sjitindarsingh@gmail.com>
> > ---
> >  hw/ppc/spapr.c      | 11 ++++++++++-
> >  hw/ppc/spapr_caps.c | 10 ++++++++++
> >  include/hw/compat.h |  2 ++
> >  3 files changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 969db6cde2..e2ebb76242 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3941,13 +3941,20 @@ static const TypeInfo spapr_machine_info =
> > {
> >  /*
> >   * pseries-2.12
> >   */
> > +#define
> > SPAPR_COMPAT_2_12                                              \
> > +    HW_COMPAT_2_12
> > +
> >  static void spapr_machine_2_12_instance_options(MachineState
> > *machine)
> >  {
> >  }
> >  
> >  static void spapr_machine_2_12_class_options(MachineClass *mc)
> >  {
> > -    /* Defaults for the latest behaviour inherited from the base
> > class */
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > +    smc->default_caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_WORKAROUND;
> > +    smc->default_caps.caps[SPAPR_CAP_SBBC] = SPAPR_CAP_WORKAROUND;
> 
> As written in the comment, the default for the latest machine type
> should
> be set at the base class level, in spapr_machine_class_init()... not
> sure
> to understand why you set it here...

Ah yes, my bad.

> 
> > +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_12);
> 
> And so, you shouldn't need to setup 2.12 compat mode before 2.12 was
> even
> released :)

Fair enough :)

> 
> >  }
> >  
> >  DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> > @@ -3969,6 +3976,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 05997b0842..c25c2bca52 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -281,11 +281,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;
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 7f31850dfa..13238239da 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -1,6 +1,8 @@
> >  #ifndef HW_COMPAT_H
> >  #define HW_COMPAT_H
> >  
> > +#define HW_COMPAT_2_12
> > +
> >  #define HW_COMPAT_2_11 \
> >      {\
> >          .driver   = "hpet",\
> 
>