Re: [Qemu-devel] [PATCH v7 18/19] spapr: add a 'pseries-4.0-xive' machine type

[David Gibson]

On Mon, Dec 10, 2018 at 11:17:33PM +0100, Cédric Le Goater wrote:
> On 12/9/18 8:46 PM, Cédric Le Goater wrote:
> > This pseries machine makes use of a new sPAPR IRQ backend supporting
> > the XIVE interrupt mode.
> > 
> > The guest OS is required to have support for the XIVE exploitation
> > mode of the POWER9 interrupt controller.
> > 
> > Signed-off-by: Cédric Le Goater <address@hidden>
> > ---
> >  hw/ppc/spapr.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4012ebd794a4..3cc134a0b673 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3985,6 +3985,21 @@ static void 
> > spapr_machine_4_0_class_options(MachineClass *mc)
> >  
> >  DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
> >  
> > +static void spapr_machine_4_0_xive_instance_options(MachineState *machine)
> > +{
> > +    spapr_machine_4_0_instance_options(machine);
> > +}
> > +
> > +static void spapr_machine_4_0_xive_class_options(MachineClass *mc)
> > +{
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> > +    spapr_machine_4_0_class_options(mc);> +    smc->irq = &spapr_irq_xive;
> 
> I have been adding checks on the CPU model to export the XIVE capability 
> only on POWER9 processors but it breaks some of the tests.
> 
> I was wondering if we could add a default POWER9 CPU to the -xive machine : 
> 
>   + mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> 
> and if we could change tests/cpu-plug-test.c with :
> 
>   @@ -198,8 +198,13 @@ static void add_pseries_test_case(const
>        }
>        data = g_new(PlugTestData, 1);
>        data->machine = g_strdup(mname);
>   -    data->cpu_model = "power8_v2.0";
>   -    data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
>   +    if (g_str_has_suffix(mname, "xive")) {
>   +        data->cpu_model = "power9_v2.0";
>   +        data->device_model = g_strdup("power9_v2.0-spapr-cpu-core");
>   +    } else {
>   +        data->cpu_model = "power8_v2.0";
>   +        data->device_model = g_strdup("power8_v2.0-spapr-cpu-core");
>   +    }
>        data->sockets = 2;
>        data->cores = 3;
>        data->threads = 1;
> 
> or if there is a better way ?

So, I'd actually prefer a machine option, rather than wholly separate
machine types to select xics/xive/dual.  Machine types was fine while
prototyping this, but I don't think we want to actually merge new
machine types for it.

So, instead I think we want a machine option which can be set to
xics/xive/dual, with xics being the default for earlier machine types
and dual the default for 4.0 onwards.  We can make POWER9 the default
cpu for 4.0 onwards as well, if you want.

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