On Thu, Jan 04, 2018 at 03:11:03PM -0600, Michael Roth wrote:
> Quoting Greg Kurz (2018-01-04 11:47:18)
> > On Thu, 4 Jan 2018 15:24:05 +1100
> > David Gibson <da...@gibson.dropbear.id.au> wrote:
> >
> > > Currently the pseries machine sets the compatibility mode for the
> > > guest's cpus in two places: 1) at machine reset and 2) after CAS
> > > negotiation.
> > >
> > > This means that if we set or negotiate a compatiblity mode, then
> > > hotplug a cpu, the hotplugged cpu doesn't get the right mode set and
> > > will incorrectly have the full native features.
> > >
> > > To correct this, we set the compatibility mode on a cpu when it is
> > > brought online with the 'start-cpu' RTAS call. Given that we no
> > > longer need to set the compatibility mode on all CPUs at machine
> > > reset, so we change that to only set the mode for the boot cpu.
> > >
> > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > > ---
> > > hw/ppc/spapr.c | 2 +-
> > > hw/ppc/spapr_rtas.c | 8 ++++++++
> > > 2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index e22888ba06..d1acfe8858 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1510,7 +1510,7 @@ static void spapr_machine_reset(void)
> > > spapr_ovec_cleanup(spapr->ov5_cas);
> > > spapr->ov5_cas = spapr_ovec_new();
> > >
> > > - ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> > > + ppc_set_compat(first_ppc_cpu, spapr->max_compat_pvr,
> > > &error_fatal);
> > > }
> > >
> > > fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
> > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > > index 4bb939d3d1..2ed00548c1 100644
> > > --- a/hw/ppc/spapr_rtas.c
> > > +++ b/hw/ppc/spapr_rtas.c
> > > @@ -163,6 +163,7 @@ static void rtas_start_cpu(PowerPCCPU *cpu_,
> > > sPAPRMachineState *spapr,
> > > CPUState *cs = CPU(cpu);
> > > CPUPPCState *env = &cpu->env;
> > > PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > > + Error *local_err = NULL;
> > >
> > > if (!cs->halted) {
> > > rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > > @@ -174,6 +175,13 @@ static void rtas_start_cpu(PowerPCCPU *cpu_,
> > > sPAPRMachineState *spapr,
> > > * new cpu enters */
> > > kvm_cpu_synchronize_state(cs);
> > >
> > > + /* Set compatibility mode to match existing cpus */
> > > + ppc_set_compat(cpu, POWERPC_CPU(first_cpu)->compat_pvr,
> > > &local_err);
> >
> > Is it okay to report a simple HW error to the guest here instead of aborting
> > like we do with first_cpu at reset time ?
>
> And if we don't opt for &error_fatal, we need an error_free() below.
Or better yet an error_report_err() which will display it and free
it. I'll make that change.
>
> >
> > > + if (local_err) {
> > > + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > > + return;
> > > + }
> > > +
> > > env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> > >
> > > /* Enable Power-saving mode Exit Cause exceptions for the new
> > > CPU */
> >
>
--
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
