* [Qemu-devel] [PATCHv2] spapr: Correct compatibility mode setting for hotplugged CPUs @ 2018-01-05 3:38 David Gibson 2018-01-05 11:19 ` Greg Kurz 0 siblings, 1 reply; 3+ messages in thread From: David Gibson @ 2018-01-05 3:38 UTC (permalink / raw) To: groug, mdroth, surajjs; +Cc: qemu-ppc, qemu-devel, satheera, David Gibson 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 <david@gibson.dropbear.id.au> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes since v1: * Add an error_report_err() to display and free the error from ppc_set_compat() in rtas_start_cpu(). 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); + 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 */ -- 2.14.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCHv2] spapr: Correct compatibility mode setting for hotplugged CPUs 2018-01-05 3:38 [Qemu-devel] [PATCHv2] spapr: Correct compatibility mode setting for hotplugged CPUs David Gibson @ 2018-01-05 11:19 ` Greg Kurz 2018-01-05 12:49 ` satheesh rajendran 0 siblings, 1 reply; 3+ messages in thread From: Greg Kurz @ 2018-01-05 11:19 UTC (permalink / raw) To: David Gibson Cc: mdroth, surajjs, qemu-ppc, qemu-devel, satheera, Daniel Henrique Barboza On Fri, 5 Jan 2018 14:38:06 +1100 David Gibson <david@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 <david@gibson.dropbear.id.au> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > Changes since v1: > * Add an error_report_err() to display and free the error from > ppc_set_compat() in rtas_start_cpu(). > It looks like this change wasn't committed when git-send-email was invoked, and you missed Daniel's comment (typo in commit message) and R-b... > > 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); > + if (local_err) { With a call to error_report_err() here, you can add: Reviewed-by: Greg Kurz <groug@kaod.org> > + 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 */ ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCHv2] spapr: Correct compatibility mode setting for hotplugged CPUs 2018-01-05 11:19 ` Greg Kurz @ 2018-01-05 12:49 ` satheesh rajendran 0 siblings, 0 replies; 3+ messages in thread From: satheesh rajendran @ 2018-01-05 12:49 UTC (permalink / raw) To: Greg Kurz Cc: David Gibson, surajjs, satheera, qemu-devel, mdroth, qemu-ppc, Daniel Henrique Barboza On Fri, Jan 05, 2018 at 12:19:14PM +0100, Greg Kurz wrote: > On Fri, 5 Jan 2018 14:38:06 +1100 > David Gibson <david@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 <david@gibson.dropbear.id.au> > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > --- Tested in upstream with this patch and found fixed. ./ppc64-softmmu/qemu-system-ppc64 -M pseries,max-cpu-compat=power8,accel=kvm \ -smp 1,cores=1,threads=1,sockets=1,maxcpus=2 \ -m 2048 -nographic -enable-kvm /home/sath/images/hostos-ppc64le.qcow2 \ -monitor stdio -serial /dev/pts/7 QEMU 2.11.50 monitor - type 'help' for more information (qemu) device_add host-spapr-cpu-core,id=core1,core-id=1 (qemu) inside guest: # taskset -c 1 ./a.out Illegal instruction Reported-and-tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> Regards, -Satheesh > > > > Changes since v1: > > * Add an error_report_err() to display and free the error from > > ppc_set_compat() in rtas_start_cpu(). > > > > It looks like this change wasn't committed when git-send-email was > invoked, and you missed Daniel's comment (typo in commit message) > and R-b... > > > > > 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); > > + if (local_err) { > > With a call to error_report_err() here, you can add: > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > + 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 */ > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-05 12:49 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-05 3:38 [Qemu-devel] [PATCHv2] spapr: Correct compatibility mode setting for hotplugged CPUs David Gibson 2018-01-05 11:19 ` Greg Kurz 2018-01-05 12:49 ` satheesh rajendran
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).