* [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
@ 2016-11-15 22:17 David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction David Gibson
` (14 more replies)
0 siblings, 15 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
This series is a significant rework to how we handle CPU compatibility
modes on ppc.
* Information about compatibility modes was previously open coded and
scattered across a number of functions in both target-ppc and spapr
code. It's now brought together into a common table of
compatibility modes.
* There was significant conceptual confusion about what a
compatibility mode means, and how it interacts with the machine
type. This cleans that up, clarifying that a compatibility mode
(as an externally set option) only makes sense on machine types
that don't permit the guest hypervisor privilege (i.e. 'pseries')
* It was previously the user's (or management layer's) responsibility
to determine compatibility of CPUs on either end for migration.
This uses the compatibility modes to check that properly during an
incoming migration.
* Some ill-considered sanity checks broke migration from 2.6 to 2.7,
due to some new instruction classes being added. This should avoid
a repeat of that problem for 2.8 (we may be able to backport a
minimal subset to 2.7-stable to fix the existing problem).
Patches 1-3 are preliminary cleanups which could stand on their own.
Patches 4-12 are the compatibility mode cleanup proper.
So far, this has been mimimally tested. There are quite a few
migration cases to check. For example:
Basic:
1) Boot guest with -cpu host
Should go into POWER8 compat mode after CAS
Previously would have been raw mode
2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
Should go into POWER7 compat mode
3) Boot guest with -cpu host,compat=power7
Should act as (2), but print a warning
4) Boot guest via libvirt with power7 compat mode specified in XML
Should act as (3), (2) once we fix libvirt
5) Hack guest to only advertise power7 compatibility, boot with -cpu host
Should go into POWER7 compat mode after CAS
6) Hack guest to only advertise real PVRs
Should remain in POWER8 raw mode after CAS
7) Hack guest to only advertise real PVRs
Boot with -machine pseries,max-cpu-compat=power8
Should fail at CAS time
8) Hack guest to only advertise power7 compatibility, boot with -cpu host
Reboot to normal guest
Should go to power7 compat mode after CAS of boot 1
Should revert to raw mode on reboot
SHould go to power8 compat mode after CAS of boot 2
Migration:
9) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
Should work, end up running in power8 raw mode
10) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host
Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
Should work, end up running in power8 raw mode
11) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
Should work, be running in POWER7 compat after, but give warning like
(3)
12) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
Should work, be running in POWER7 compat after, no warning
13) Boot to SLOF with qemu-2.6 -machine pseries-2.6 -cpu host
Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
?
14) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host
Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
?
15) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
?
16) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
?
17) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
Migrate to qemu-2.7.z -machine pseries-2.6 -cpu host
Should work
18) Hack guest to only advertise power7 compatibility, boot with -cpu host
Boot with qemu-2.8, migrate to qemu-2.8
Should be in power7 compat mode after CAS on source, and still
in power7 compat mode on destination
Changes since RFCv1:
* Change CAS logic to prefer compatibility modes over raw mode
* Simplified by giving up on half-hearted attempts to maintain
backwards migration
* Folded migration stream changes into a single patch
* Removed some preliminary patches which are already merged
David Gibson (12):
pseries: Always use core objects for CPU construction
pseries: Make cpu_update during CAS unconditional
ppc: Clean up and QOMify hypercall emulation
ppc: Rename cpu_version to compat_pvr
ppc: Rewrite ppc_set_compat()
ppc: Rewrite ppc_get_compat_smt_threads()
ppc: Validate compatibility modes when setting
pseries: Rewrite CAS PVR compatibility logic
ppc: Add ppc_set_compat_all()
pseries: Move CPU compatibility property to machine
pseries: Reset CPU compatibility mode
ppc: Rework CPU compatibility testing across migration
hw/ppc/spapr.c | 158 ++++++++++++++++------------
hw/ppc/spapr_cpu_core.c | 85 ++++++++++-----
hw/ppc/spapr_hcall.c | 140 +++++++------------------
hw/ppc/trace-events | 2 +-
include/hw/ppc/spapr.h | 12 ++-
target-ppc/Makefile.objs | 2 +-
target-ppc/compat.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
target-ppc/cpu.h | 49 +++++++--
target-ppc/excp_helper.c | 11 +-
target-ppc/kvm.c | 4 +-
target-ppc/kvm_ppc.h | 4 +-
target-ppc/machine.c | 87 ++++++++++++++--
target-ppc/translate_init.c | 157 +++++++---------------------
13 files changed, 607 insertions(+), 353 deletions(-)
create mode 100644 target-ppc/compat.c
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-18 15:00 ` Greg Kurz
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 02/12] pseries: Make cpu_update during CAS unconditional David Gibson
` (13 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Currently the pseries machine has two paths for constructing CPUs. On
newer machine type versions, which support cpu hotplug, it constructs
cpu core objects, which in turn construct CPU threads. For older machine
versions it individually constructs the CPU threads.
This division is going to make some future changes to the cpu construction
harder, so this patch unifies them. Now cpu core objects are always
created. This requires some updates to allow core objects to be created
without a full complement of threads (since older versions allowed a
number of cpus not a multiple of the threads-per-core). Likewise it needs
some changes to the cpu core hot/cold plug path so as not to choke on the
old machine types without hotplug support.
For good measure, we move the cpu construction to its own subfunction,
spapr_init_cpus().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 125 +++++++++++++++++++++++++++---------------------
hw/ppc/spapr_cpu_core.c | 37 +++++++-------
include/hw/ppc/spapr.h | 1 -
3 files changed, 90 insertions(+), 73 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0cbab24..cbac537 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1687,11 +1687,80 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
}
}
+static void spapr_init_cpus(sPAPRMachineState *spapr)
+{
+ MachineState *machine = MACHINE(spapr);
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
+ char *type = spapr_get_cpu_core_type(machine->cpu_model);
+ int smt = kvmppc_smt_threads();
+ int spapr_max_cores, spapr_cores;
+ int i;
+
+ if (!type) {
+ error_report("Unable to find sPAPR CPU Core definition");
+ exit(1);
+ }
+
+ if (mc->query_hotpluggable_cpus) {
+ if (smp_cpus % smp_threads) {
+ error_report("smp_cpus (%u) must be multiple of threads (%u)",
+ smp_cpus, smp_threads);
+ exit(1);
+ }
+ if (max_cpus % smp_threads) {
+ error_report("max_cpus (%u) must be multiple of threads (%u)",
+ max_cpus, smp_threads);
+ exit(1);
+ }
+
+ spapr_max_cores = max_cpus / smp_threads;
+ spapr_cores = smp_cpus / smp_threads;
+ } else {
+ if (max_cpus != smp_cpus) {
+ error_report("This machine version does not support CPU hotplug");
+ exit(1);
+ }
+
+ spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
+ spapr_cores = spapr_max_cores;
+ }
+
+ spapr->cores = g_new0(Object *, spapr_max_cores);
+ for (i = 0; i < spapr_max_cores; i++) {
+ int core_id = i * smp_threads;
+
+ if (mc->query_hotpluggable_cpus) {
+ sPAPRDRConnector *drc =
+ spapr_dr_connector_new(OBJECT(spapr),
+ SPAPR_DR_CONNECTOR_TYPE_CPU,
+ (core_id / smp_threads) * smt);
+
+ qemu_register_reset(spapr_drc_reset, drc);
+ }
+
+ if (i < spapr_cores) {
+ Object *core = object_new(type);
+ int nr_threads = smp_threads;
+
+ /* Handle the partially filled core for older machine types */
+ if ((i + 1) * smp_threads >= smp_cpus) {
+ nr_threads = smp_cpus - i * smp_threads;
+ }
+
+ object_property_set_int(core, nr_threads, "nr-threads",
+ &error_fatal);
+ object_property_set_int(core, core_id, CPU_CORE_PROP_CORE_ID,
+ &error_fatal);
+ object_property_set_bool(core, true, "realized", &error_fatal);
+ }
+ }
+ g_free(type);
+}
+
/* pSeries LPAR / sPAPR hardware init */
static void ppc_spapr_init(MachineState *machine)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
- MachineClass *mc = MACHINE_GET_CLASS(machine);
sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
const char *initrd_filename = machine->initrd_filename;
@@ -1706,21 +1775,6 @@ static void ppc_spapr_init(MachineState *machine)
long load_limit, fw_size;
char *filename;
int smt = kvmppc_smt_threads();
- int spapr_cores = smp_cpus / smp_threads;
- int spapr_max_cores = max_cpus / smp_threads;
-
- if (mc->query_hotpluggable_cpus) {
- if (smp_cpus % smp_threads) {
- error_report("smp_cpus (%u) must be multiple of threads (%u)",
- smp_cpus, smp_threads);
- exit(1);
- }
- if (max_cpus % smp_threads) {
- error_report("max_cpus (%u) must be multiple of threads (%u)",
- max_cpus, smp_threads);
- exit(1);
- }
- }
msi_nonbroken = true;
@@ -1800,44 +1854,7 @@ static void ppc_spapr_init(MachineState *machine)
ppc_cpu_parse_features(machine->cpu_model);
- if (mc->query_hotpluggable_cpus) {
- char *type = spapr_get_cpu_core_type(machine->cpu_model);
-
- if (type == NULL) {
- error_report("Unable to find sPAPR CPU Core definition");
- exit(1);
- }
-
- spapr->cores = g_new0(Object *, spapr_max_cores);
- for (i = 0; i < spapr_max_cores; i++) {
- int core_id = i * smp_threads;
- sPAPRDRConnector *drc =
- spapr_dr_connector_new(OBJECT(spapr),
- SPAPR_DR_CONNECTOR_TYPE_CPU,
- (core_id / smp_threads) * smt);
-
- qemu_register_reset(spapr_drc_reset, drc);
-
- if (i < spapr_cores) {
- Object *core = object_new(type);
- object_property_set_int(core, smp_threads, "nr-threads",
- &error_fatal);
- object_property_set_int(core, core_id, CPU_CORE_PROP_CORE_ID,
- &error_fatal);
- object_property_set_bool(core, true, "realized", &error_fatal);
- }
- }
- g_free(type);
- } else {
- for (i = 0; i < smp_cpus; i++) {
- PowerPCCPU *cpu = cpu_ppc_init(machine->cpu_model);
- if (cpu == NULL) {
- error_report("Unable to find PowerPC CPU definition");
- exit(1);
- }
- spapr_cpu_init(spapr, cpu, &error_fatal);
- }
- }
+ spapr_init_cpus(spapr);
if (kvm_enabled()) {
/* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index e0c14f6..424d275 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -46,7 +46,8 @@ static void spapr_cpu_destroy(PowerPCCPU *cpu)
qemu_unregister_reset(spapr_cpu_reset, cpu);
}
-void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
+static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
+ Error **errp)
{
CPUPPCState *env = &cpu->env;
CPUState *cs = CPU(cpu);
@@ -166,11 +167,11 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+ MachineClass *mc = MACHINE_GET_CLASS(spapr);
sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
CPUCore *cc = CPU_CORE(dev);
CPUState *cs = CPU(core->threads);
sPAPRDRConnector *drc;
- sPAPRDRConnectorClass *drck;
Error *local_err = NULL;
void *fdt = NULL;
int fdt_offset = 0;
@@ -180,7 +181,7 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
spapr->cores[index] = OBJECT(dev);
- g_assert(drc);
+ g_assert(drc || !mc->query_hotpluggable_cpus);
/*
* Setup CPU DT entries only for hotplugged CPUs. For boot time or
@@ -190,13 +191,15 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
}
- drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
- drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
- if (local_err) {
- g_free(fdt);
- spapr->cores[index] = NULL;
- error_propagate(errp, local_err);
- return;
+ if (drc) {
+ sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+ drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
+ if (local_err) {
+ g_free(fdt);
+ spapr->cores[index] = NULL;
+ error_propagate(errp, local_err);
+ return;
+ }
}
if (dev->hotplugged) {
@@ -209,8 +212,11 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
/*
* Set the right DRC states for cold plugged CPU.
*/
- drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
- drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
+ if (drc) {
+ sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+ drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
+ drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
+ }
}
}
@@ -227,7 +233,7 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
const char *type = object_get_typename(OBJECT(dev));
- if (!mc->query_hotpluggable_cpus) {
+ if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
error_setg(&local_err, "CPU hotplug not supported for this machine");
goto out;
}
@@ -237,11 +243,6 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
goto out;
}
- if (cc->nr_threads != smp_threads) {
- error_setg(&local_err, "threads must be %d", smp_threads);
- goto out;
- }
-
if (cc->core_id % smp_threads) {
error_setg(&local_err, "invalid core id %d", cc->core_id);
goto out;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index bd5bcf7..f8d444d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -614,7 +614,6 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
uint32_t count, uint32_t index);
void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
uint32_t count, uint32_t index);
-void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp);
void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
sPAPRMachineState *spapr);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 02/12] pseries: Make cpu_update during CAS unconditional
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 03/12] ppc: Clean up and QOMify hypercall emulation David Gibson
` (12 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
spapr_h_cas_compose_response() includes a cpu_update parameter which
controls whether it includes updated information on the CPUs in the device
tree fragment returned from the ibm,client-architecture-support (CAS) call.
Providing the updated information is essential when CAS has negotiated
compatibility options which require different cpu information to be
presented to the guest. However, it should be safe to provide in other
cases (it will just override the existing data in the device tree with
identical data). This simplifies the code by removing the parameter and
always providing the cpu update information.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr.c | 5 +----
hw/ppc/spapr_hcall.c | 8 ++------
include/hw/ppc/spapr.h | 1 -
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cbac537..d6cc3bd 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -685,7 +685,6 @@ out:
int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
target_ulong addr, target_ulong size,
- bool cpu_update,
sPAPROptionVector *ov5_updates)
{
void *fdt, *fdt_skel;
@@ -704,9 +703,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
g_free(fdt_skel);
/* Fixup cpu nodes */
- if (cpu_update) {
- _FDT((spapr_fixup_cpu_dt(fdt, spapr)));
- }
+ _FDT((spapr_fixup_cpu_dt(fdt, spapr)));
if (spapr_dt_cas_updates(spapr, fdt, ov5_updates)) {
return -1;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 9a9bedf..3f08b8a 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -945,7 +945,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
target_ulong ov_table;
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
CPUState *cs;
- bool cpu_match = false, cpu_update = true;
+ bool cpu_match = false;
unsigned old_cpu_version = cpu_->cpu_version;
unsigned compat_lvl = 0, cpu_version = 0;
unsigned max_lvl = get_compat_level(cpu_->max_compat);
@@ -999,10 +999,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
}
}
- if (!cpu_version) {
- cpu_update = false;
- }
-
/* For the future use: here @ov_table points to the first option vector */
ov_table = list;
@@ -1028,7 +1024,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
if (!spapr->cas_reboot) {
spapr->cas_reboot =
- (spapr_h_cas_compose_response(spapr, args[1], args[2], cpu_update,
+ (spapr_h_cas_compose_response(spapr, args[1], args[2],
ov5_updates) != 0);
}
spapr_ovec_cleanup(ov5_updates);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f8d444d..04d2821 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -589,7 +589,6 @@ void spapr_events_init(sPAPRMachineState *sm);
void spapr_dt_events(sPAPRMachineState *sm, void *fdt);
int spapr_h_cas_compose_response(sPAPRMachineState *sm,
target_ulong addr, target_ulong size,
- bool cpu_update,
sPAPROptionVector *ov5_updates);
sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn);
void spapr_tce_table_enable(sPAPRTCETable *tcet,
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 03/12] ppc: Clean up and QOMify hypercall emulation
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 02/12] pseries: Make cpu_update during CAS unconditional David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 04/12] ppc: Rename cpu_version to compat_pvr David Gibson
` (11 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
The pseries machine type is a bit unusual in that it runs a paravirtualized
guest. The guest expects to interact with a hypervisor, and qemu
emulates the functions of that hypervisor directly, rather than executing
hypervisor code within the emulated system.
To implement this in TCG, we need to intercept hypercall instructions and
direct them to the machine's hypercall handlers, rather than attempting to
perform a privilege change within TCG. This is controlled by a global
hook - cpu_ppc_hypercall.
This cleanup makes the handling a little cleaner and more extensible than
a single global variable. Instead, each CPU to have hypercalls intercepted
has a pointer set to a QOM object implementing a new virtual hypervisor
interface. A method in that interface is called by TCG when it sees a
hypercall instruction. It's possible we may want to add other methods in
future.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr.c | 8 +++++---
hw/ppc/spapr_cpu_core.c | 1 +
target-ppc/cpu.h | 26 ++++++++++++++++++++++++--
target-ppc/excp_helper.c | 11 ++++-------
target-ppc/translate_init.c | 12 ++++++++++++
5 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d6cc3bd..cea5a7b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1005,7 +1005,8 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
}
-static void emulate_spapr_hypercall(PowerPCCPU *cpu)
+static void emulate_spapr_hypercall(PPCVirtualHypervisor *vhyp,
+ PowerPCCPU *cpu)
{
CPUPPCState *env = &cpu->env;
@@ -1777,8 +1778,6 @@ static void ppc_spapr_init(MachineState *machine)
QLIST_INIT(&spapr->phbs);
- cpu_ppc_hypercall = emulate_spapr_hypercall;
-
/* Allocate RMA if necessary */
rma_alloc_size = kvmppc_alloc_rma(&rma);
@@ -2609,6 +2608,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
NMIClass *nc = NMI_CLASS(oc);
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+ PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
mc->desc = "pSeries Logical Partition (PAPR compliant)";
@@ -2640,6 +2640,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
fwc->get_dev_path = spapr_get_fw_dev_path;
nc->nmi_monitor_handler = spapr_nmi;
smc->phb_placement = spapr_phb_placement;
+ vhc->hypercall = emulate_spapr_hypercall;
}
static const TypeInfo spapr_machine_info = {
@@ -2655,6 +2656,7 @@ static const TypeInfo spapr_machine_info = {
{ TYPE_FW_PATH_PROVIDER },
{ TYPE_NMI },
{ TYPE_HOTPLUG_HANDLER },
+ { TYPE_PPC_VIRTUAL_HYPERVISOR },
{ }
},
};
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 424d275..7fafdae 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -57,6 +57,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
/* Enable PAPR mode in TCG or KVM */
+ cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
cpu_ppc_set_papr(cpu);
if (cpu->max_compat) {
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 1c90adb..a655105 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1148,6 +1148,9 @@ do { \
env->wdt_period[3] = (d_); \
} while (0)
+typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
+typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
+
/**
* PowerPCCPU:
* @env: #CPUPPCState
@@ -1166,6 +1169,7 @@ struct PowerPCCPU {
int cpu_dt_id;
uint32_t max_compat;
uint32_t cpu_version;
+ PPCVirtualHypervisor *vhyp;
};
static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
@@ -1180,6 +1184,25 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr);
+struct PPCVirtualHypervisor {
+ Object parent;
+};
+
+struct PPCVirtualHypervisorClass {
+ InterfaceClass parent;
+ void (*hypercall)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
+};
+
+#define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor"
+#define PPC_VIRTUAL_HYPERVISOR(obj) \
+ OBJECT_CHECK(PPCVirtualHypervisor, (obj), TYPE_PPC_VIRTUAL_HYPERVISOR)
+#define PPC_VIRTUAL_HYPERVISOR_CLASS(klass) \
+ OBJECT_CLASS_CHECK(PPCVirtualHypervisorClass, (klass), \
+ TYPE_PPC_VIRTUAL_HYPERVISOR)
+#define PPC_VIRTUAL_HYPERVISOR_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(PPCVirtualHypervisorClass, (obj), \
+ TYPE_PPC_VIRTUAL_HYPERVISOR)
+
void ppc_cpu_do_interrupt(CPUState *cpu);
bool ppc_cpu_exec_interrupt(CPUState *cpu, int int_req);
void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
@@ -1252,6 +1275,7 @@ void store_booke_tcr (CPUPPCState *env, target_ulong val);
void store_booke_tsr (CPUPPCState *env, target_ulong val);
void ppc_tlb_invalidate_all (CPUPPCState *env);
void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr);
+void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
void cpu_ppc_set_papr(PowerPCCPU *cpu);
#endif
#endif
@@ -2421,8 +2445,6 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx)
(start + nregs > 32 && (rx >= start || rx < start + nregs - 32));
}
-extern void (*cpu_ppc_hypercall)(PowerPCCPU *);
-
void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env);
/**
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 93369d4..f4ee7aa 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -35,11 +35,6 @@
#endif
/*****************************************************************************/
-/* PowerPC Hypercall emulation */
-
-void (*cpu_ppc_hypercall)(PowerPCCPU *);
-
-/*****************************************************************************/
/* Exception processing */
#if defined(CONFIG_USER_ONLY)
void ppc_cpu_do_interrupt(CPUState *cs)
@@ -318,8 +313,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
env->nip += 4;
/* "PAPR mode" built-in hypercall emulation */
- if ((lev == 1) && cpu_ppc_hypercall) {
- cpu_ppc_hypercall(cpu);
+ if ((lev == 1) && cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ vhc->hypercall(cpu->vhyp, cpu);
return;
}
if (lev == 1) {
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 208fa1e..21899a4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8857,6 +8857,11 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
#if !defined(CONFIG_USER_ONLY)
+void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
+{
+ cpu->vhyp = vhyp;
+}
+
void cpu_ppc_set_papr(PowerPCCPU *cpu)
{
CPUPPCState *env = &cpu->env;
@@ -10587,9 +10592,16 @@ static const TypeInfo ppc_cpu_type_info = {
.class_init = ppc_cpu_class_init,
};
+static const TypeInfo ppc_vhyp_type_info = {
+ .name = TYPE_PPC_VIRTUAL_HYPERVISOR,
+ .parent = TYPE_INTERFACE,
+ .class_size = sizeof(PPCVirtualHypervisorClass),
+};
+
static void ppc_cpu_register_types(void)
{
type_register_static(&ppc_cpu_type_info);
+ type_register_static(&ppc_vhyp_type_info);
}
type_init(ppc_cpu_register_types)
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 04/12] ppc: Rename cpu_version to compat_pvr
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (2 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 03/12] ppc: Clean up and QOMify hypercall emulation David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 05/12] ppc: Rewrite ppc_set_compat() David Gibson
` (10 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
The 'cpu_version' field in PowerPCCPU is badly named. It's named after the
'cpu-version' device tree property where it is advertised, but that meaning
may not be obvious in most places it appears.
Worse, it doesn't even really correspond to that device tree property. The
property contains either the processor's PVR, or, if the CPU is running in
a compatibility mode, a special "logical PVR" representing which mode.
Rename the cpu_version field, and a number of related variables to
compat_pvr to make this clearer.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
hw/ppc/spapr.c | 4 ++--
hw/ppc/spapr_hcall.c | 30 +++++++++++++++---------------
target-ppc/cpu.h | 6 +++---
target-ppc/kvm.c | 4 ++--
target-ppc/kvm_ppc.h | 4 ++--
target-ppc/translate_init.c | 10 +++++-----
6 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cea5a7b..ca8044b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -148,8 +148,8 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
uint32_t gservers_prop[smt_threads * 2];
int index = ppc_get_vcpu_dt_id(cpu);
- if (cpu->cpu_version) {
- ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->cpu_version);
+ if (cpu->compat_pvr) {
+ ret = fdt_setprop_cell(fdt, offset, "cpu-version", cpu->compat_pvr);
if (ret < 0) {
return ret;
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 3f08b8a..35499ae 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -882,7 +882,7 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,
}
typedef struct {
- uint32_t cpu_version;
+ uint32_t compat_pvr;
Error *err;
} SetCompatState;
@@ -892,7 +892,7 @@ static void do_set_compat(CPUState *cs, run_on_cpu_data arg)
SetCompatState *s = arg.host_ptr;
cpu_synchronize_state(cs);
- ppc_set_compat(cpu, s->cpu_version, &s->err);
+ ppc_set_compat(cpu, s->compat_pvr, &s->err);
}
#define get_compat_level(cpuver) ( \
@@ -903,7 +903,7 @@ static void do_set_compat(CPUState *cs, run_on_cpu_data arg)
static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
unsigned max_lvl, unsigned *compat_lvl,
- unsigned *cpu_version)
+ unsigned *compat_pvr)
{
unsigned lvl = get_compat_level(pvr);
bool is205, is206, is207;
@@ -926,12 +926,12 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
/* User did not set the level, choose the highest */
if (*compat_lvl <= lvl) {
*compat_lvl = lvl;
- *cpu_version = pvr;
+ *compat_pvr = pvr;
}
} else if (max_lvl >= lvl) {
/* User chose the level, don't set higher than this */
*compat_lvl = lvl;
- *cpu_version = pvr;
+ *compat_pvr = pvr;
}
}
}
@@ -946,8 +946,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
CPUState *cs;
bool cpu_match = false;
- unsigned old_cpu_version = cpu_->cpu_version;
- unsigned compat_lvl = 0, cpu_version = 0;
+ unsigned old_compat_pvr = cpu_->compat_pvr;
+ unsigned compat_lvl = 0, compat_pvr = 0;
unsigned max_lvl = get_compat_level(cpu_->max_compat);
int counter;
sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates;
@@ -965,12 +965,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
if (!max_lvl &&
((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) {
cpu_match = true;
- cpu_version = 0;
- } else if (pvr == cpu_->cpu_version) {
+ compat_pvr = 0;
+ } else if (pvr == cpu_->compat_pvr) {
cpu_match = true;
- cpu_version = cpu_->cpu_version;
+ compat_pvr = cpu_->compat_pvr;
} else if (!cpu_match) {
- cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &cpu_version);
+ cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr);
}
/* Terminator record */
if (~pvr_mask & pvr) {
@@ -979,14 +979,14 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
}
/* Parsing finished */
- trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
- cpu_version, pcc->pcr_mask);
+ trace_spapr_cas_pvr(cpu_->compat_pvr, cpu_match,
+ compat_pvr, pcc->pcr_mask);
/* Update CPUs */
- if (old_cpu_version != cpu_version) {
+ if (old_compat_pvr != compat_pvr) {
CPU_FOREACH(cs) {
SetCompatState s = {
- .cpu_version = cpu_version,
+ .compat_pvr = compat_pvr,
.err = NULL,
};
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index a655105..f7656cb 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1156,7 +1156,7 @@ typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
* @env: #CPUPPCState
* @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
* @max_compat: Maximal supported logical PVR from the command line
- * @cpu_version: Current logical PVR, zero if in "raw" mode
+ * @compat_pvr: Current logical PVR, zero if in "raw" mode
*
* A PowerPC CPU.
*/
@@ -1168,7 +1168,7 @@ struct PowerPCCPU {
CPUPPCState env;
int cpu_dt_id;
uint32_t max_compat;
- uint32_t cpu_version;
+ uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
};
@@ -1243,7 +1243,7 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
#if defined(TARGET_PPC64)
-void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp);
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
#endif
/* Time-base and decrementer management */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9c4834c..15e12f3 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2103,9 +2103,9 @@ void kvmppc_set_papr(PowerPCCPU *cpu)
cap_papr = 1;
}
-int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr)
{
- return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT, &cpu_version);
+ return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT, &compat_pvr);
}
void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index bd1d78b..841a29b 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -26,7 +26,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
void kvmppc_enable_set_mode_hcall(void);
void kvmppc_enable_clear_ref_mod_hcalls(void);
void kvmppc_set_papr(PowerPCCPU *cpu);
-int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
+int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
int kvmppc_smt_threads(void);
int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
@@ -123,7 +123,7 @@ static inline void kvmppc_set_papr(PowerPCCPU *cpu)
{
}
-static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr)
{
return 0;
}
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 21899a4..c35065d 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9957,7 +9957,7 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
CPUState *cs = CPU(cpu);
int ret = MIN(cs->nr_threads, kvmppc_smt_threads());
- switch (cpu->cpu_version) {
+ switch (cpu->compat_pvr) {
case CPU_POWERPC_LOGICAL_2_05:
ret = MIN(ret, 2);
break;
@@ -9973,15 +9973,15 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
}
#ifdef TARGET_PPC64
-void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
{
int ret = 0;
CPUPPCState *env = &cpu->env;
PowerPCCPUClass *host_pcc;
- cpu->cpu_version = cpu_version;
+ cpu->compat_pvr = compat_pvr;
- switch (cpu_version) {
+ switch (compat_pvr) {
case CPU_POWERPC_LOGICAL_2_05:
env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
@@ -10004,7 +10004,7 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
}
if (kvm_enabled()) {
- ret = kvmppc_set_compat(cpu, cpu->cpu_version);
+ ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
if (ret < 0) {
error_setg_errno(errp, -ret,
"Unable to set CPU compatibility mode in KVM");
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 05/12] ppc: Rewrite ppc_set_compat()
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (3 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 04/12] ppc: Rename cpu_version to compat_pvr David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 06/12] ppc: Rewrite ppc_get_compat_smt_threads() David Gibson
` (9 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
This rewrites the ppc_set_compat() function so that instead of open coding
the various compatibility modes, it reads the relevant data from a table.
This is a first step in consolidating the information on compatibility
modes scattered across the code into a single place.
It also makes one change to the logic. The old code masked the bits
to be set in the PCR (Processor Compatibility Register) by which bits
are valid on the host CPU. This made no sense, since it was done
regardless of whether our guest CPU was the same as the host CPU or
not. Furthermore, the actual PCR bits are only relevant for TCG[1] -
KVM instead uses the compatibility mode we tell it in
kvmppc_set_compat(). When using TCG host cpu information usually
isn't even present.
While we're at it, we put the new implementation in a new file to make the
enormous translate_init.c a little smaller.
[1] Actually it doesn't even do anything in TCG, but it will if / when we
get to implementing compatibility mode logic at that level.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
target-ppc/Makefile.objs | 2 +-
target-ppc/compat.c | 91 +++++++++++++++++++++++++++++++++++++++++++++
target-ppc/cpu.h | 6 ++-
target-ppc/translate_init.c | 41 --------------------
4 files changed, 97 insertions(+), 43 deletions(-)
create mode 100644 target-ppc/compat.c
diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
index e667e69..a8c7a30 100644
--- a/target-ppc/Makefile.objs
+++ b/target-ppc/Makefile.objs
@@ -2,7 +2,7 @@ obj-y += cpu-models.o
obj-y += translate.o
ifeq ($(CONFIG_SOFTMMU),y)
obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
-obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o
+obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o compat.o
endif
obj-$(CONFIG_KVM) += kvm.o
obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
diff --git a/target-ppc/compat.c b/target-ppc/compat.c
new file mode 100644
index 0000000..f3fd9c6
--- /dev/null
+++ b/target-ppc/compat.c
@@ -0,0 +1,91 @@
+/*
+ * PowerPC CPU initialization for qemu.
+ *
+ * Copyright 2016, David Gibson, Red Hat Inc. <dgibson@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/kvm.h"
+#include "kvm_ppc.h"
+#include "sysemu/cpus.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "cpu-models.h"
+
+typedef struct {
+ uint32_t pvr;
+ uint64_t pcr;
+} CompatInfo;
+
+static const CompatInfo compat_table[] = {
+ { /* POWER6, ISA2.05 */
+ .pvr = CPU_POWERPC_LOGICAL_2_05,
+ .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
+ | PCR_TM_DIS | PCR_VSX_DIS,
+ },
+ { /* POWER7, ISA2.06 */
+ .pvr = CPU_POWERPC_LOGICAL_2_06,
+ .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ },
+ {
+ .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
+ .pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ },
+ { /* POWER8, ISA2.07 */
+ .pvr = CPU_POWERPC_LOGICAL_2_07,
+ .pcr = PCR_COMPAT_2_07,
+ },
+};
+
+static const CompatInfo *compat_by_pvr(uint32_t pvr)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
+ if (compat_table[i].pvr == pvr) {
+ return &compat_table[i];
+ }
+ }
+ return NULL;
+}
+
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
+{
+ const CompatInfo *compat = compat_by_pvr(compat_pvr);
+ CPUPPCState *env = &cpu->env;
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ uint64_t pcr;
+
+ if (!compat_pvr) {
+ pcr = 0;
+ } else if (!compat) {
+ error_setg(errp, "Unknown compatibility PVR 0x%08"PRIx32, compat_pvr);
+ return;
+ } else {
+ pcr = compat->pcr;
+ }
+
+ cpu->compat_pvr = compat_pvr;
+ env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
+
+ if (kvm_enabled()) {
+ int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Unable to set CPU compatibility mode in KVM");
+ }
+ }
+}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index f7656cb..15d5e4b 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1243,7 +1243,6 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
#if defined(TARGET_PPC64)
-void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
#endif
/* Time-base and decrementer management */
@@ -1314,6 +1313,11 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch)
return ifetch ? env->immu_idx : env->dmmu_idx;
}
+/* Compatibility modes */
+#if defined(TARGET_PPC64)
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
+#endif /* defined(TARGET_PPC64) */
+
#include "exec/cpu-all.h"
/*****************************************************************************/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index c35065d..a70eafb 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9972,47 +9972,6 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
return ret;
}
-#ifdef TARGET_PPC64
-void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
-{
- int ret = 0;
- CPUPPCState *env = &cpu->env;
- PowerPCCPUClass *host_pcc;
-
- cpu->compat_pvr = compat_pvr;
-
- switch (compat_pvr) {
- case CPU_POWERPC_LOGICAL_2_05:
- env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
- PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
- break;
- case CPU_POWERPC_LOGICAL_2_06:
- case CPU_POWERPC_LOGICAL_2_06_PLUS:
- env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
- break;
- case CPU_POWERPC_LOGICAL_2_07:
- env->spr[SPR_PCR] = PCR_COMPAT_2_07;
- break;
- default:
- env->spr[SPR_PCR] = 0;
- break;
- }
-
- host_pcc = kvm_ppc_get_host_cpu_class();
- if (host_pcc) {
- env->spr[SPR_PCR] &= host_pcc->pcr_mask;
- }
-
- if (kvm_enabled()) {
- ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
- if (ret < 0) {
- error_setg_errno(errp, -ret,
- "Unable to set CPU compatibility mode in KVM");
- }
- }
-}
-#endif
-
static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
{
ObjectClass *oc = (ObjectClass *)a;
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 06/12] ppc: Rewrite ppc_get_compat_smt_threads()
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (4 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 05/12] ppc: Rewrite ppc_set_compat() David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 07/12] ppc: Validate compatibility modes when setting David Gibson
` (8 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
To continue consolidation of compatibility mode information, this rewrites
the ppc_get_compat_smt_threads() function using the table of compatiblity
modes in target-ppc/compat.c.
It's not a direct replacement, the new ppc_compat_max_threads() function
has simpler semantics - it just returns the number of threads the cpu
model has, taking into account any compatiblity mode it is in.
This no longer takes into account kvmppc_smt_threads() as the previous
version did. That check wasn't useful because we check in
ppc_cpu_realizefn() that CPUs aren't instantiated with more threads
than kvm allows (or if we didn't things will already be broken and
this won't make it any worse).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr.c | 8 ++++----
target-ppc/compat.c | 18 ++++++++++++++++++
target-ppc/cpu.h | 2 +-
target-ppc/translate_init.c | 20 --------------------
4 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ca8044b..35a4168 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -206,6 +206,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
PowerPCCPU *cpu = POWERPC_CPU(cs);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
int index = ppc_get_vcpu_dt_id(cpu);
+ int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
if ((index % smt) != 0) {
continue;
@@ -240,8 +241,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
return ret;
}
- ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu,
- ppc_get_compat_smt_threads(cpu));
+ ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt);
if (ret < 0) {
return ret;
}
@@ -407,6 +407,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
size_t page_sizes_prop_size;
uint32_t vcpus_per_socket = smp_threads * smp_cores;
uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
+ int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
sPAPRDRConnector *drc;
sPAPRDRConnectorClass *drck;
int drc_index;
@@ -494,8 +495,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
_FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cs));
- _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu,
- ppc_get_compat_smt_threads(cpu)));
+ _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
}
static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
diff --git a/target-ppc/compat.c b/target-ppc/compat.c
index f3fd9c6..66529a6 100644
--- a/target-ppc/compat.c
+++ b/target-ppc/compat.c
@@ -28,6 +28,7 @@
typedef struct {
uint32_t pvr;
uint64_t pcr;
+ int max_threads;
} CompatInfo;
static const CompatInfo compat_table[] = {
@@ -35,18 +36,22 @@ static const CompatInfo compat_table[] = {
.pvr = CPU_POWERPC_LOGICAL_2_05,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
| PCR_TM_DIS | PCR_VSX_DIS,
+ .max_threads = 2,
},
{ /* POWER7, ISA2.06 */
.pvr = CPU_POWERPC_LOGICAL_2_06,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ .max_threads = 4,
},
{
.pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ .max_threads = 4,
},
{ /* POWER8, ISA2.07 */
.pvr = CPU_POWERPC_LOGICAL_2_07,
.pcr = PCR_COMPAT_2_07,
+ .max_threads = 8,
},
};
@@ -89,3 +94,16 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
}
}
}
+
+int ppc_compat_max_threads(PowerPCCPU *cpu)
+{
+ const CompatInfo *compat = compat_by_pvr(cpu->compat_pvr);
+ int n_threads = CPU(cpu)->nr_threads;
+
+ if (cpu->compat_pvr) {
+ g_assert(compat);
+ n_threads = MIN(n_threads, compat->max_threads);
+ }
+
+ return n_threads;
+}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 15d5e4b..cfda7b2 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1241,7 +1241,6 @@ void ppc_store_sdr1 (CPUPPCState *env, target_ulong value);
void ppc_store_msr (CPUPPCState *env, target_ulong value);
void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
-int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
#if defined(TARGET_PPC64)
#endif
@@ -1316,6 +1315,7 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch)
/* Compatibility modes */
#if defined(TARGET_PPC64)
void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
+int ppc_compat_max_threads(PowerPCCPU *cpu);
#endif /* defined(TARGET_PPC64) */
#include "exec/cpu-all.h"
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a70eafb..ba48242 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9952,26 +9952,6 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
}
}
-int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
-{
- CPUState *cs = CPU(cpu);
- int ret = MIN(cs->nr_threads, kvmppc_smt_threads());
-
- switch (cpu->compat_pvr) {
- case CPU_POWERPC_LOGICAL_2_05:
- ret = MIN(ret, 2);
- break;
- case CPU_POWERPC_LOGICAL_2_06:
- ret = MIN(ret, 4);
- break;
- case CPU_POWERPC_LOGICAL_2_07:
- ret = MIN(ret, 8);
- break;
- }
-
- return ret;
-}
-
static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
{
ObjectClass *oc = (ObjectClass *)a;
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 07/12] ppc: Validate compatibility modes when setting
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (5 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 06/12] ppc: Rewrite ppc_get_compat_smt_threads() David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 08/12] pseries: Rewrite CAS PVR compatibility logic David Gibson
` (7 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Current ppc_set_compat() will attempt to set any compatiblity mode
specified, regardless of whether it's available on the CPU. The caller is
expected to make sure it is setting a possible mode, which is awkwward
because most of the information to make that decision is at the CPU level.
This begins to clean this up by introducing a ppc_check_compat() function
which will determine if a given compatiblity mode is supported on a CPU
(and also whether it lies within specified minimum and maximum compat
levels, which will be useful later). It also contains an assertion that
the CPU has a "virtual hypervisor"[1], that is, that the guest isn't
permitted to execute hypervisor privilege code. Without that, the guest
would own the PCR and so could override any mode set here. Only machine
types which use a virtual hypervisor (i.e. 'pseries') should use
ppc_check_compat().
ppc_set_compat() is modified to validate the compatibility mode it is given
and fail if it's not available on this CPU.
[1] Or user-only mode, which also obviously doesn't allow access to the
hypervisor privileged PCR. We don't use that now, but could in future.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
target-ppc/compat.c | 41 +++++++++++++++++++++++++++++++++++++++++
target-ppc/cpu.h | 2 ++
2 files changed, 43 insertions(+)
diff --git a/target-ppc/compat.c b/target-ppc/compat.c
index 66529a6..1059555 100644
--- a/target-ppc/compat.c
+++ b/target-ppc/compat.c
@@ -28,29 +28,37 @@
typedef struct {
uint32_t pvr;
uint64_t pcr;
+ uint64_t pcr_level;
int max_threads;
} CompatInfo;
static const CompatInfo compat_table[] = {
+ /*
+ * Ordered from oldest to newest - the code relies on this
+ */
{ /* POWER6, ISA2.05 */
.pvr = CPU_POWERPC_LOGICAL_2_05,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
| PCR_TM_DIS | PCR_VSX_DIS,
+ .pcr_level = PCR_COMPAT_2_05,
.max_threads = 2,
},
{ /* POWER7, ISA2.06 */
.pvr = CPU_POWERPC_LOGICAL_2_06,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ .pcr_level = PCR_COMPAT_2_06,
.max_threads = 4,
},
{
.pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
+ .pcr_level = PCR_COMPAT_2_06,
.max_threads = 4,
},
{ /* POWER8, ISA2.07 */
.pvr = CPU_POWERPC_LOGICAL_2_07,
.pcr = PCR_COMPAT_2_07,
+ .pcr_level = PCR_COMPAT_2_07,
.max_threads = 8,
},
};
@@ -67,6 +75,35 @@ static const CompatInfo *compat_by_pvr(uint32_t pvr)
return NULL;
}
+bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
+ uint32_t min_compat_pvr, uint32_t max_compat_pvr)
+{
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+ const CompatInfo *compat = compat_by_pvr(compat_pvr);
+ const CompatInfo *min = compat_by_pvr(min_compat_pvr);
+ const CompatInfo *max = compat_by_pvr(max_compat_pvr);
+
+#if !defined(CONFIG_USER_ONLY)
+ g_assert(cpu->vhyp);
+#endif
+ g_assert(!min_compat_pvr || min);
+ g_assert(!max_compat_pvr || max);
+
+ if (!compat) {
+ /* Not a recognized logical PVR */
+ return false;
+ }
+ if ((min && (compat < min)) || (max && (compat > max))) {
+ /* Outside specified range */
+ return false;
+ }
+ if (!(pcc->pcr_supported & compat->pcr_level)) {
+ /* Not supported by this CPU */
+ return false;
+ }
+ return true;
+}
+
void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
{
const CompatInfo *compat = compat_by_pvr(compat_pvr);
@@ -79,6 +116,10 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
} else if (!compat) {
error_setg(errp, "Unknown compatibility PVR 0x%08"PRIx32, compat_pvr);
return;
+ } else if (!ppc_check_compat(cpu, compat_pvr, 0, 0)) {
+ error_setg(errp, "Compatibility PVR 0x%08"PRIx32" not valid for CPU",
+ compat_pvr);
+ return;
} else {
pcr = compat->pcr;
}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index cfda7b2..91e8be8 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1314,6 +1314,8 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch)
/* Compatibility modes */
#if defined(TARGET_PPC64)
+bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
+ uint32_t min_compat_pvr, uint32_t max_compat_pvr);
void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
int ppc_compat_max_threads(PowerPCCPU *cpu);
#endif /* defined(TARGET_PPC64) */
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 08/12] pseries: Rewrite CAS PVR compatibility logic
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (6 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 07/12] ppc: Validate compatibility modes when setting David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 09/12] ppc: Add ppc_set_compat_all() David Gibson
` (6 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
During boot, PAPR guests negotiate CPU model support with the
ibm,client-architecture-support mechanism. The logic to implement this in
qemu is very convoluted. This cleans it up to be cleaner, using the new
ppc_check_compat() call.
The new logic for choosing a compatibility mode is:
1. Usually, use the most recent compatibility mode that is
a) supported by the guest
b) supported by the CPU
and c) no later than the maximum allowed (if specified)
2. If no suitable compatibility mode was found, the guest *does*
support this CPU explicitly, and no maximum compatibility mode is
specified, then use "raw" mode for the current CPU
3. Otherwise, fail the boot.
This differs from the results of the old code: the old code preferred using
"raw" mode to a compatibility mode, whereas the new code prefers a
compatibility mode if available. Using compatibility mode preferentially
means that we're more likely to be able to migrate the guest to a similar
but not identical host.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_hcall.c | 105 +++++++++++++++++----------------------------------
hw/ppc/trace-events | 2 +-
2 files changed, 35 insertions(+), 72 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 35499ae..48b253f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -895,98 +895,61 @@ static void do_set_compat(CPUState *cs, run_on_cpu_data arg)
ppc_set_compat(cpu, s->compat_pvr, &s->err);
}
-#define get_compat_level(cpuver) ( \
- ((cpuver) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \
- ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \
- ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
- ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
-
-static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
- unsigned max_lvl, unsigned *compat_lvl,
- unsigned *compat_pvr)
-{
- unsigned lvl = get_compat_level(pvr);
- bool is205, is206, is207;
-
- if (!lvl) {
- return;
- }
-
- /* If it is a logical PVR, try to determine the highest level */
- is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) &&
- (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
- is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) &&
- ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
- (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
- is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) &&
- (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_07));
-
- if (is205 || is206 || is207) {
- if (!max_lvl) {
- /* User did not set the level, choose the highest */
- if (*compat_lvl <= lvl) {
- *compat_lvl = lvl;
- *compat_pvr = pvr;
- }
- } else if (max_lvl >= lvl) {
- /* User chose the level, don't set higher than this */
- *compat_lvl = lvl;
- *compat_pvr = pvr;
- }
- }
-}
-
-static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
+static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
target_ulong opcode,
target_ulong *args)
{
target_ulong list = ppc64_phys_to_real(args[0]);
target_ulong ov_table;
- PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
CPUState *cs;
- bool cpu_match = false;
- unsigned old_compat_pvr = cpu_->compat_pvr;
- unsigned compat_lvl = 0, compat_pvr = 0;
- unsigned max_lvl = get_compat_level(cpu_->max_compat);
- int counter;
+ bool explicit_match = false; /* Matched the CPU's real PVR */
+ uint32_t max_compat = cpu->max_compat;
+ uint32_t best_compat = 0;
+ int i;
sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates;
- /* Parse PVR list */
- for (counter = 0; counter < 512; ++counter) {
+ /*
+ * We scan the supplied table of PVRs looking for two things
+ * 1. Is our real CPU PVR in the list?
+ * 2. What's the "best" listed logical PVR
+ */
+ for (i = 0; i < 512; ++i) {
uint32_t pvr, pvr_mask;
pvr_mask = ldl_be_phys(&address_space_memory, list);
- list += 4;
- pvr = ldl_be_phys(&address_space_memory, list);
- list += 4;
-
- trace_spapr_cas_pvr_try(pvr);
- if (!max_lvl &&
- ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) {
- cpu_match = true;
- compat_pvr = 0;
- } else if (pvr == cpu_->compat_pvr) {
- cpu_match = true;
- compat_pvr = cpu_->compat_pvr;
- } else if (!cpu_match) {
- cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &compat_pvr);
- }
- /* Terminator record */
+ pvr = ldl_be_phys(&address_space_memory, list + 4);
+ list += 8;
+
if (~pvr_mask & pvr) {
- break;
+ break; /* Terminator record */
}
+
+ if ((cpu->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask)) {
+ explicit_match = true;
+ } else {
+ if (ppc_check_compat(cpu, pvr, best_compat, max_compat)) {
+ best_compat = pvr;
+ }
+ }
+ }
+
+ if ((best_compat == 0)
+ && (!explicit_match || max_compat)) {
+ /* We couldn't find a suitable compatibility mode, and either
+ * the guest doesn't support "raw" mode for this CPU, or raw
+ * mode is disabled because a maximum compat mode is set */
+ return H_HARDWARE;
}
/* Parsing finished */
- trace_spapr_cas_pvr(cpu_->compat_pvr, cpu_match,
- compat_pvr, pcc->pcr_mask);
+ trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
/* Update CPUs */
- if (old_compat_pvr != compat_pvr) {
+ if (cpu->compat_pvr != best_compat) {
CPU_FOREACH(cs) {
SetCompatState s = {
- .compat_pvr = compat_pvr,
+ .compat_pvr = best_compat,
.err = NULL,
};
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 2297ead..604ac92 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -15,7 +15,7 @@ spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
# hw/ppc/spapr_hcall.c
spapr_cas_pvr_try(uint32_t pvr) "%x"
-spapr_cas_pvr(uint32_t cur_pvr, bool cpu_match, uint32_t new_pvr, uint64_t pcr) "current=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64
+spapr_cas_pvr(uint32_t cur_pvr, bool explicit_match, uint32_t new_pvr) "current=%x, explicit_match=%u, new=%x"
# hw/ppc/spapr_iommu.c
spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 09/12] ppc: Add ppc_set_compat_all()
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (7 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 08/12] pseries: Rewrite CAS PVR compatibility logic David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine David Gibson
` (5 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Once a compatiblity mode is negotiated with the guest,
h_client_architecture_support() uses run_on_cpu() to update each CPU to
the new mode. We're going to want this logic somewhere else shortly,
so make a helper function to do this global update.
We put it in target-ppc/compat.c - it makes as much sense at the CPU level
as it does at the machine level. We also move the cpu_synchronize_state()
into ppc_set_compat(), since it doesn't really make any sense to call that
without synchronizing state.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_hcall.c | 31 +++++--------------------------
target-ppc/compat.c | 34 ++++++++++++++++++++++++++++++++++
target-ppc/cpu.h | 3 +++
3 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 48b253f..96d50ad 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -881,20 +881,6 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,
return ret;
}
-typedef struct {
- uint32_t compat_pvr;
- Error *err;
-} SetCompatState;
-
-static void do_set_compat(CPUState *cs, run_on_cpu_data arg)
-{
- PowerPCCPU *cpu = POWERPC_CPU(cs);
- SetCompatState *s = arg.host_ptr;
-
- cpu_synchronize_state(cs);
- ppc_set_compat(cpu, s->compat_pvr, &s->err);
-}
-
static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
target_ulong opcode,
@@ -902,7 +888,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
{
target_ulong list = ppc64_phys_to_real(args[0]);
target_ulong ov_table;
- CPUState *cs;
bool explicit_match = false; /* Matched the CPU's real PVR */
uint32_t max_compat = cpu->max_compat;
uint32_t best_compat = 0;
@@ -947,18 +932,12 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
/* Update CPUs */
if (cpu->compat_pvr != best_compat) {
- CPU_FOREACH(cs) {
- SetCompatState s = {
- .compat_pvr = best_compat,
- .err = NULL,
- };
+ Error *local_err = NULL;
- run_on_cpu(cs, do_set_compat, RUN_ON_CPU_HOST_PTR(&s));
-
- if (s.err) {
- error_report_err(s.err);
- return H_HARDWARE;
- }
+ ppc_set_compat_all(best_compat, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ return H_HARDWARE;
}
}
diff --git a/target-ppc/compat.c b/target-ppc/compat.c
index 1059555..8de9dfb 100644
--- a/target-ppc/compat.c
+++ b/target-ppc/compat.c
@@ -124,6 +124,8 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
pcr = compat->pcr;
}
+ cpu_synchronize_state(CPU(cpu));
+
cpu->compat_pvr = compat_pvr;
env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
@@ -136,6 +138,38 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp)
}
}
+typedef struct {
+ uint32_t compat_pvr;
+ Error *err;
+} SetCompatState;
+
+static void do_set_compat(CPUState *cs, run_on_cpu_data arg)
+{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ SetCompatState *s = arg.host_ptr;
+
+ ppc_set_compat(cpu, s->compat_pvr, &s->err);
+}
+
+void ppc_set_compat_all(uint32_t compat_pvr, Error **errp)
+{
+ CPUState *cs;
+
+ CPU_FOREACH(cs) {
+ SetCompatState s = {
+ .compat_pvr = compat_pvr,
+ .err = NULL,
+ };
+
+ run_on_cpu(cs, do_set_compat, RUN_ON_CPU_HOST_PTR(&s));
+
+ if (s.err) {
+ error_propagate(errp, s.err);
+ return;
+ }
+ }
+}
+
int ppc_compat_max_threads(PowerPCCPU *cpu)
{
const CompatInfo *compat = compat_by_pvr(cpu->compat_pvr);
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 91e8be8..201a655 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1317,6 +1317,9 @@ static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch)
bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr,
uint32_t min_compat_pvr, uint32_t max_compat_pvr);
void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
+#if !defined(CONFIG_USER_ONLY)
+void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
+#endif
int ppc_compat_max_threads(PowerPCCPU *cpu);
#endif /* defined(TARGET_PPC64) */
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (8 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 09/12] ppc: Add ppc_set_compat_all() David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-19 8:27 ` Greg Kurz
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 11/12] pseries: Reset CPU compatibility mode David Gibson
` (4 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Server class POWER CPUs have a "compat" property, which is used to set the
backwards compatibility mode for the processor. However, this only makes
sense for machine types which don't give the guest access to hypervisor
privilege - otherwise the compatibility level is under the guest's control.
To reflect this, this removes the CPU 'compat' property and instead
creates a 'max-cpu-compat' property on the pseries machine. Strictly
speaking this breaks compatibility, but AFAIK the 'compat' option was
never (directly) used with -device or device_add.
The option was used with -cpu. So, to maintain compatibility, this patch
adds a hack to the cpu option parsing to strip out any compat options
supplied with -cpu and set them on the machine property instead of the new
removed cpu property.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 6 +++-
hw/ppc/spapr_cpu_core.c | 41 ++++++++++++++++++++--
hw/ppc/spapr_hcall.c | 2 +-
include/hw/ppc/spapr.h | 10 ++++--
target-ppc/compat.c | 65 +++++++++++++++++++++++++++++++++++
target-ppc/cpu.h | 6 ++--
target-ppc/translate_init.c | 84 +++++++++++++--------------------------------
7 files changed, 144 insertions(+), 70 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 35a4168..3878baf 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1848,7 +1848,7 @@ static void ppc_spapr_init(MachineState *machine)
machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
}
- ppc_cpu_parse_features(machine->cpu_model);
+ spapr_cpu_parse_features(spapr);
spapr_init_cpus(spapr);
@@ -2190,6 +2190,10 @@ static void spapr_machine_initfn(Object *obj)
" place of standard EPOW events when possible"
" (required for memory hot-unplug support)",
NULL);
+
+ object_property_add(obj, "max-cpu-compat", "str",
+ ppc_compat_prop_get, ppc_compat_prop_set,
+ NULL, &spapr->max_compat_pvr, &error_fatal);
}
static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 7fafdae..c5dda80 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -18,6 +18,43 @@
#include "target-ppc/mmu-hash64.h"
#include "sysemu/numa.h"
+void spapr_cpu_parse_features(sPAPRMachineState *spapr)
+{
+ /*
+ * Backwards compatibility hack:
+
+ * CPUs had a "compat=" property which didn't make sense for
+ * anything except pseries. It was replaced by "max-cpu-compat"
+ * machine option. This supports old command lines like
+ * -cpu POWER8,compat=power7
+ * By stripping the compat option and applying it to the machine
+ * before passing it on to the cpu level parser.
+ */
+ gchar **inpieces;
+ int n, i;
+ gchar *compat_str = NULL;
+
+ inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
+ n = g_strv_length(inpieces);
+
+ /* inpieces[0] is the actual model string */
+ for (i = 0; i < n; i++) {
+ if (g_str_has_prefix(inpieces[i], "compat=")) {
+ compat_str = inpieces[i];
+ }
+ }
+
+ if (compat_str) {
+ char *val = compat_str + strlen("compat=");
+ object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
+ &error_fatal);
+ }
+
+ ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
+
+ g_strfreev(inpieces);
+}
+
static void spapr_cpu_reset(void *opaque)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -60,10 +97,10 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
cpu_ppc_set_papr(cpu);
- if (cpu->max_compat) {
+ if (spapr->max_compat_pvr) {
Error *local_err = NULL;
- ppc_set_compat(cpu, cpu->max_compat, &local_err);
+ ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 96d50ad..9478eae 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -889,7 +889,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
target_ulong list = ppc64_phys_to_real(args[0]);
target_ulong ov_table;
bool explicit_match = false; /* Matched the CPU's real PVR */
- uint32_t max_compat = cpu->max_compat;
+ uint32_t max_compat = spapr->max_compat_pvr;
uint32_t best_compat = 0;
int i;
sPAPROptionVector *ov5_guest, *ov5_cas_old, *ov5_updates;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 04d2821..d2a40e9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -74,15 +74,18 @@ struct sPAPRMachineState {
uint64_t rtc_offset; /* Now used only during incoming migration */
struct PPCTimebase tb;
bool has_graphics;
- sPAPROptionVector *ov5; /* QEMU-supported option vectors */
- sPAPROptionVector *ov5_cas; /* negotiated (via CAS) option vectors */
- bool cas_reboot;
Notifier epow_notifier;
QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
bool use_hotplug_event_source;
sPAPREventSource *event_sources;
+ /* ibm,client-architecture-support option negotiation */
+ bool cas_reboot;
+ sPAPROptionVector *ov5; /* QEMU-supported option vectors */
+ sPAPROptionVector *ov5_cas; /* negotiated (via CAS) option vectors */
+ uint32_t max_compat_pvr;
+
/* Migration state */
int htab_save_index;
bool htab_first_pass;
@@ -613,6 +616,7 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
uint32_t count, uint32_t index);
void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
uint32_t count, uint32_t index);
+void spapr_cpu_parse_features(sPAPRMachineState *spapr);
void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
sPAPRMachineState *spapr);
diff --git a/target-ppc/compat.c b/target-ppc/compat.c
index 8de9dfb..77caa7a 100644
--- a/target-ppc/compat.c
+++ b/target-ppc/compat.c
@@ -23,9 +23,11 @@
#include "sysemu/cpus.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "cpu-models.h"
typedef struct {
+ const char *name;
uint32_t pvr;
uint64_t pcr;
uint64_t pcr_level;
@@ -37,6 +39,7 @@ static const CompatInfo compat_table[] = {
* Ordered from oldest to newest - the code relies on this
*/
{ /* POWER6, ISA2.05 */
+ .name = "power6",
.pvr = CPU_POWERPC_LOGICAL_2_05,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05
| PCR_TM_DIS | PCR_VSX_DIS,
@@ -44,18 +47,21 @@ static const CompatInfo compat_table[] = {
.max_threads = 2,
},
{ /* POWER7, ISA2.06 */
+ .name = "power7",
.pvr = CPU_POWERPC_LOGICAL_2_06,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
.pcr_level = PCR_COMPAT_2_06,
.max_threads = 4,
},
{
+ .name = "power7+",
.pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
.pcr = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
.pcr_level = PCR_COMPAT_2_06,
.max_threads = 4,
},
{ /* POWER8, ISA2.07 */
+ .name = "power8",
.pvr = CPU_POWERPC_LOGICAL_2_07,
.pcr = PCR_COMPAT_2_07,
.pcr_level = PCR_COMPAT_2_07,
@@ -182,3 +188,62 @@ int ppc_compat_max_threads(PowerPCCPU *cpu)
return n_threads;
}
+
+void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ uint32_t compat_pvr = *((uint32_t *)opaque);
+ const char *value;
+
+ if (!compat_pvr) {
+ value = "";
+ } else {
+ const CompatInfo *compat = compat_by_pvr(compat_pvr);
+
+ g_assert(compat);
+
+ value = compat->name;
+ }
+
+ visit_type_str(v, name, (char **)&value, errp);
+}
+
+void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ Error *error = NULL;
+ char *value;
+ uint32_t compat_pvr;
+
+ visit_type_str(v, name, &value, &error);
+ if (error) {
+ error_propagate(errp, error);
+ return;
+ }
+
+ if (strcmp(value, "") == 0) {
+ compat_pvr = 0;
+ } else {
+ int i;
+ const CompatInfo *compat = NULL;
+
+ for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
+ if (strcmp(value, compat_table[i].name) == 0) {
+ compat = &compat_table[i];
+ break;
+
+ }
+ }
+
+ if (!compat) {
+ error_setg(errp, "Invalid compatibility mode \"%s\"", value);
+ goto out;
+ }
+ compat_pvr = compat->pvr;
+ }
+
+ *((uint32_t *)opaque) = compat_pvr;
+
+out:
+ g_free(value);
+}
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 201a655..51d21bb 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1155,7 +1155,6 @@ typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
* PowerPCCPU:
* @env: #CPUPPCState
* @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
- * @max_compat: Maximal supported logical PVR from the command line
* @compat_pvr: Current logical PVR, zero if in "raw" mode
*
* A PowerPC CPU.
@@ -1167,7 +1166,6 @@ struct PowerPCCPU {
CPUPPCState env;
int cpu_dt_id;
- uint32_t max_compat;
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
};
@@ -1321,6 +1319,10 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
#endif
int ppc_compat_max_threads(PowerPCCPU *cpu);
+void ppc_compat_prop_get(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error **err);
+void ppc_compat_prop_set(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error **err);
#endif /* defined(TARGET_PPC64) */
#include "exec/cpu-all.h"
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ba48242..4fce5a8 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8428,73 +8428,35 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
pcc->l1_icache_size = 0x10000;
}
-static void powerpc_get_compat(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- char *value = (char *)"";
- Property *prop = opaque;
- uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
-
- switch (*max_compat) {
- case CPU_POWERPC_LOGICAL_2_05:
- value = (char *)"power6";
- break;
- case CPU_POWERPC_LOGICAL_2_06:
- value = (char *)"power7";
- break;
- case CPU_POWERPC_LOGICAL_2_07:
- value = (char *)"power8";
- break;
- case 0:
- break;
- default:
- error_report("Internal error: compat is set to %x", *max_compat);
- abort();
- break;
- }
-
- visit_type_str(v, name, &value, errp);
-}
-
-static void powerpc_set_compat(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
+/*
+ * The CPU used to have a "compat" property which set the
+ * compatibility mode PVR. However, this was conceptually broken - it
+ * only makes sense on the pseries machine type (otherwise the guest
+ * owns the PCR and can control the compatibility mode itself). It's
+ * been replaced with the 'max-cpu-compat' property on the pseries
+ * machine type. For backwards compatibility, pseries specially
+ * parses the -cpu parameter and converts old compat= parameters into
+ * the appropriate machine parameters. This stub implementation of
+ * the parameter catches any uses on explicitly created CPUs.
+ */
+static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
{
- Error *error = NULL;
- char *value = NULL;
- Property *prop = opaque;
- uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
-
- visit_type_str(v, name, &value, &error);
- if (error) {
- error_propagate(errp, error);
- return;
- }
-
- if (strcmp(value, "power6") == 0) {
- *max_compat = CPU_POWERPC_LOGICAL_2_05;
- } else if (strcmp(value, "power7") == 0) {
- *max_compat = CPU_POWERPC_LOGICAL_2_06;
- } else if (strcmp(value, "power8") == 0) {
- *max_compat = CPU_POWERPC_LOGICAL_2_07;
- } else {
- error_setg(errp, "Invalid compatibility mode \"%s\"", value);
- }
-
- g_free(value);
+ error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
+ visit_type_null(v, name, errp);
}
-static PropertyInfo powerpc_compat_propinfo = {
+static PropertyInfo ppc_compat_deprecated_propinfo = {
.name = "str",
- .description = "compatibility mode, power6/power7/power8",
- .get = powerpc_get_compat,
- .set = powerpc_set_compat,
+ .description = "compatibility mode (deprecated)",
+ .get = getset_compat_deprecated,
+ .set = getset_compat_deprecated,
};
-
-#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
-
static Property powerpc_servercpu_properties[] = {
- DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
+ {
+ .name = "compat",
+ .info = &ppc_compat_deprecated_propinfo,
+ },
DEFINE_PROP_END_OF_LIST(),
};
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 11/12] pseries: Reset CPU compatibility mode
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (9 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration David Gibson
` (3 subsequent siblings)
14 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Currently, the CPU compatibility mode is set when the cpu is initialized,
then again when the guest negotiates features. This means if a guest
negotiates a compatibility mode, then reboots, that compatibility mode
will be retained across the reset.
Usually that will get overridden when features are negotiated on the next
boot, but it's still not really correct. This patch moves the initial set
up of the compatibility mode from cpu init to reset time. The mode *is*
retained if the reboot was caused by the feature negotiation (it might
be important in that case, though it's unlikely).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr.c | 2 ++
hw/ppc/spapr_cpu_core.c | 10 ----------
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3878baf..beef542 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1168,6 +1168,8 @@ static void ppc_spapr_reset(void)
if (!spapr->cas_reboot) {
spapr_ovec_cleanup(spapr->ov5_cas);
spapr->ov5_cas = spapr_ovec_new();
+
+ ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
}
fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index c5dda80..195a99f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -97,16 +97,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
cpu_ppc_set_papr(cpu);
- if (spapr->max_compat_pvr) {
- Error *local_err = NULL;
-
- ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- }
-
/* Set NUMA node for the added CPUs */
i = numa_get_node_for_cpu(cs->cpu_index);
if (i < nb_numa_nodes) {
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (10 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 11/12] pseries: Reset CPU compatibility mode David Gibson
@ 2016-11-15 22:17 ` David Gibson
2016-12-02 14:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2016-11-15 22:44 ` [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling no-reply
` (2 subsequent siblings)
14 siblings, 1 reply; 23+ messages in thread
From: David Gibson @ 2016-11-15 22:17 UTC (permalink / raw)
To: groug, clg, aik, mdroth, nikunj
Cc: agraf, qemu-ppc, qemu-devel, abologna, thuth, lvivier, David Gibson
Migrating between different CPU versions is quite complicated for ppc.
A long time ago, we ensure identical CPU versions at either end by checking
the PVR had the same value. However, this breaks under KVM HV, because
we always have to use the host's PVR - it's not virtualized. That would
mean we couldn't migrate between hosts with different PVRs, even if the
CPUs are close enough to compatible in practice (sometimes identical cores
with different surrounding logic have different PVRs, so this happens in
practice quite often).
So, we removed the PVR check, but instead checked that several flags
indicating supported instructions matched. This turns out to be a bad
idea, because those instruction masks are not architected information, but
essentially a TCG implementation detail. So changes to qemu internal CPU
modelling can break migration - this happened between qemu-2.6 and
qemu-2.7.
Modern server-class CPUs can be placed into compatibility modes. Now that
we're handling those properly, we finally have the information to sanely
deal with CPU compatibility across migration.
This patch bumps the migration version number for the ppc CPU removing the
instruction mask field (and some other unwise VMSTATE_EQUAL checks), and
adding the compatibility PVR to the migration stream.
We consider the CPUs compatible for migration if:
* The source was running in a compatibility mode which the destination
supports
OR * The source has a PVR matching the same qemu CPU class as the
destination, either an exact match or an approximate match determined
by the cpu class's pvr_match hook.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target-ppc/machine.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 79 insertions(+), 8 deletions(-)
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index e43cb6c..25a30d5 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -8,6 +8,7 @@
#include "helper_regs.h"
#include "mmu-hash64.h"
#include "migration/cpu.h"
+#include "qapi/error.h"
static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
{
@@ -163,6 +164,30 @@ static void cpu_pre_save(void *opaque)
}
}
+/*
+ * Determine if a given PVR is a "close enough" match to the CPU
+ * object. For TCG and KVM PR it would probably be sufficient to
+ * require an exact PVR match. However for KVM HV the user is
+ * restricted to a PVR exactly matching the host CPU. The correct way
+ * to handle this is to put the guest into an architected
+ * compatibility mode. However, to allow a more forgiving transition
+ * and migration from before this was widely done, we allow migration
+ * between sufficiently similar PVRs, as determined by the CPU class's
+ * pvr_match() hook.
+ */
+static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
+{
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+ if (pvr == pcc->pvr) {
+ return true;
+ }
+ if (pcc->pvr_match) {
+ return pcc->pvr_match(pcc, pvr);
+ }
+ return false;
+}
+
static int cpu_post_load(void *opaque, int version_id)
{
PowerPCCPU *cpu = opaque;
@@ -171,10 +196,31 @@ static int cpu_post_load(void *opaque, int version_id)
target_ulong msr;
/*
- * We always ignore the source PVR. The user or management
- * software has to take care of running QEMU in a compatible mode.
+ * If we're operating in compat mode, we should be ok as long as
+ * the destination supports the same compatiblity mode.
+ *
+ * Otherwise, however, we require that the destination has exactly
+ * the same CPU model as the source.
*/
- env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
+
+#if defined(TARGET_PPC64)
+ if (cpu->compat_pvr) {
+ Error *local_err = NULL;
+
+ ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ error_free(local_err);
+ return -1;
+ }
+ } else
+#endif
+ {
+ if (!pvr_match(cpu, env->spr[SPR_PVR])) {
+ return -1;
+ }
+ }
+
env->lr = env->spr[SPR_LR];
env->ctr = env->spr[SPR_CTR];
cpu_write_xer(env, env->spr[SPR_XER]);
@@ -527,9 +573,32 @@ static const VMStateDescription vmstate_tlbmas = {
}
};
+static bool compat_needed(void *opaque)
+{
+ PowerPCCPU *cpu = opaque;
+
+ return cpu->vhyp != NULL;
+}
+
+static const VMStateDescription vmstate_compat = {
+ .name = "cpu/compat",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = compat_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(compat_pvr, PowerPCCPU),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static bool version_before_6(void *opaque, int version_id)
+{
+ return version_id < 6;
+}
+
const VMStateDescription vmstate_ppc_cpu = {
.name = "cpu",
- .version_id = 5,
+ .version_id = 6,
.minimum_version_id = 5,
.minimum_version_id_old = 4,
.load_state_old = cpu_load_old,
@@ -561,10 +630,11 @@ const VMStateDescription vmstate_ppc_cpu = {
/* FIXME: access_type? */
/* Sanity checking */
- VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
- VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
- VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
- VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
+ VMSTATE_UNUSED_TEST(version_before_6,
+ sizeof(target_ulong) /* msr_mask */
+ + sizeof(uint64_t) /* insns_flags */
+ + sizeof(uint64_t) /* insns_flags2 */
+ + sizeof(uint32_t)), /* nb_BATs */
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription*[]) {
@@ -579,6 +649,7 @@ const VMStateDescription vmstate_ppc_cpu = {
&vmstate_tlb6xx,
&vmstate_tlbemb,
&vmstate_tlbmas,
+ &vmstate_compat,
NULL
}
};
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (11 preceding siblings ...)
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration David Gibson
@ 2016-11-15 22:44 ` no-reply
2016-11-26 0:33 ` Greg Kurz
2016-12-01 13:16 ` Greg Kurz
14 siblings, 0 replies; 23+ messages in thread
From: no-reply @ 2016-11-15 22:44 UTC (permalink / raw)
To: david
Cc: famz, groug, clg, aik, mdroth, nikunj, lvivier, thuth,
qemu-devel, agraf, abologna, qemu-ppc
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
Message-id: 1479248275-18889-1-git-send-email-david@gibson.dropbear.id.au
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1479248275-18889-1-git-send-email-david@gibson.dropbear.id.au -> patchew/1479248275-18889-1-git-send-email-david@gibson.dropbear.id.au
Switched to a new branch 'test'
a010098 ppc: Rework CPU compatibility testing across migration
830db0f pseries: Reset CPU compatibility mode
d9b2e5df pseries: Move CPU compatibility property to machine
9a1ead7 ppc: Add ppc_set_compat_all()
b9341d5 pseries: Rewrite CAS PVR compatibility logic
e183845 ppc: Validate compatibility modes when setting
0c10a17 ppc: Rewrite ppc_get_compat_smt_threads()
6741d99 ppc: Rewrite ppc_set_compat()
28872e9 ppc: Rename cpu_version to compat_pvr
6662209 ppc: Clean up and QOMify hypercall emulation
8235268 pseries: Make cpu_update during CAS unconditional
b8093c5 pseries: Always use core objects for CPU construction
=== OUTPUT BEGIN ===
Checking PATCH 1/12: pseries: Always use core objects for CPU construction...
Checking PATCH 2/12: pseries: Make cpu_update during CAS unconditional...
Checking PATCH 3/12: ppc: Clean up and QOMify hypercall emulation...
Checking PATCH 4/12: ppc: Rename cpu_version to compat_pvr...
Checking PATCH 5/12: ppc: Rewrite ppc_set_compat()...
Checking PATCH 6/12: ppc: Rewrite ppc_get_compat_smt_threads()...
Checking PATCH 7/12: ppc: Validate compatibility modes when setting...
Checking PATCH 8/12: pseries: Rewrite CAS PVR compatibility logic...
ERROR: suspect code indent for conditional statements (4, 9)
#144: FILE: hw/ppc/spapr_hcall.c:937:
+ if ((best_compat == 0)
[...]
+ /* We couldn't find a suitable compatibility mode, and either
total: 1 errors, 0 warnings, 140 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/12: ppc: Add ppc_set_compat_all()...
Checking PATCH 10/12: pseries: Move CPU compatibility property to machine...
ERROR: line over 90 characters
#370: FILE: target-ppc/translate_init.c:8445:
+ error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
total: 1 errors, 0 warnings, 330 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/12: pseries: Reset CPU compatibility mode...
Checking PATCH 12/12: ppc: Rework CPU compatibility testing across migration...
ERROR: braces {} are necessary for all arms of this statement
#98: FILE: target-ppc/machine.c:207:
+ if (cpu->compat_pvr) {
[...]
+ } else
[...]
total: 1 errors, 0 warnings, 126 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction David Gibson
@ 2016-11-18 15:00 ` Greg Kurz
0 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2016-11-18 15:00 UTC (permalink / raw)
To: David Gibson
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
On Wed, 16 Nov 2016 09:17:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> Currently the pseries machine has two paths for constructing CPUs. On
> newer machine type versions, which support cpu hotplug, it constructs
> cpu core objects, which in turn construct CPU threads. For older machine
> versions it individually constructs the CPU threads.
>
> This division is going to make some future changes to the cpu construction
> harder, so this patch unifies them. Now cpu core objects are always
> created. This requires some updates to allow core objects to be created
> without a full complement of threads (since older versions allowed a
> number of cpus not a multiple of the threads-per-core). Likewise it needs
> some changes to the cpu core hot/cold plug path so as not to choke on the
> old machine types without hotplug support.
>
> For good measure, we move the cpu construction to its own subfunction,
> spapr_init_cpus().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/ppc/spapr.c | 125 +++++++++++++++++++++++++++---------------------
> hw/ppc/spapr_cpu_core.c | 37 +++++++-------
> include/hw/ppc/spapr.h | 1 -
> 3 files changed, 90 insertions(+), 73 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0cbab24..cbac537 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1687,11 +1687,80 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
> }
> }
>
> +static void spapr_init_cpus(sPAPRMachineState *spapr)
> +{
> + MachineState *machine = MACHINE(spapr);
> + MachineClass *mc = MACHINE_GET_CLASS(machine);
> + char *type = spapr_get_cpu_core_type(machine->cpu_model);
> + int smt = kvmppc_smt_threads();
> + int spapr_max_cores, spapr_cores;
> + int i;
> +
> + if (!type) {
> + error_report("Unable to find sPAPR CPU Core definition");
> + exit(1);
> + }
> +
> + if (mc->query_hotpluggable_cpus) {
> + if (smp_cpus % smp_threads) {
> + error_report("smp_cpus (%u) must be multiple of threads (%u)",
> + smp_cpus, smp_threads);
> + exit(1);
> + }
> + if (max_cpus % smp_threads) {
> + error_report("max_cpus (%u) must be multiple of threads (%u)",
> + max_cpus, smp_threads);
> + exit(1);
> + }
> +
> + spapr_max_cores = max_cpus / smp_threads;
> + spapr_cores = smp_cpus / smp_threads;
> + } else {
> + if (max_cpus != smp_cpus) {
> + error_report("This machine version does not support CPU hotplug");
> + exit(1);
> + }
> +
> + spapr_max_cores = QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_threads;
> + spapr_cores = spapr_max_cores;
> + }
> +
> + spapr->cores = g_new0(Object *, spapr_max_cores);
> + for (i = 0; i < spapr_max_cores; i++) {
> + int core_id = i * smp_threads;
> +
> + if (mc->query_hotpluggable_cpus) {
> + sPAPRDRConnector *drc =
> + spapr_dr_connector_new(OBJECT(spapr),
> + SPAPR_DR_CONNECTOR_TYPE_CPU,
> + (core_id / smp_threads) * smt);
> +
> + qemu_register_reset(spapr_drc_reset, drc);
> + }
> +
> + if (i < spapr_cores) {
> + Object *core = object_new(type);
> + int nr_threads = smp_threads;
> +
> + /* Handle the partially filled core for older machine types */
> + if ((i + 1) * smp_threads >= smp_cpus) {
> + nr_threads = smp_cpus - i * smp_threads;
> + }
> +
> + object_property_set_int(core, nr_threads, "nr-threads",
> + &error_fatal);
> + object_property_set_int(core, core_id, CPU_CORE_PROP_CORE_ID,
> + &error_fatal);
> + object_property_set_bool(core, true, "realized", &error_fatal);
> + }
> + }
> + g_free(type);
> +}
> +
> /* pSeries LPAR / sPAPR hardware init */
> static void ppc_spapr_init(MachineState *machine)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> - MachineClass *mc = MACHINE_GET_CLASS(machine);
> sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> const char *kernel_filename = machine->kernel_filename;
> const char *initrd_filename = machine->initrd_filename;
> @@ -1706,21 +1775,6 @@ static void ppc_spapr_init(MachineState *machine)
> long load_limit, fw_size;
> char *filename;
> int smt = kvmppc_smt_threads();
> - int spapr_cores = smp_cpus / smp_threads;
> - int spapr_max_cores = max_cpus / smp_threads;
> -
> - if (mc->query_hotpluggable_cpus) {
> - if (smp_cpus % smp_threads) {
> - error_report("smp_cpus (%u) must be multiple of threads (%u)",
> - smp_cpus, smp_threads);
> - exit(1);
> - }
> - if (max_cpus % smp_threads) {
> - error_report("max_cpus (%u) must be multiple of threads (%u)",
> - max_cpus, smp_threads);
> - exit(1);
> - }
> - }
>
> msi_nonbroken = true;
>
> @@ -1800,44 +1854,7 @@ static void ppc_spapr_init(MachineState *machine)
>
> ppc_cpu_parse_features(machine->cpu_model);
>
> - if (mc->query_hotpluggable_cpus) {
> - char *type = spapr_get_cpu_core_type(machine->cpu_model);
> -
> - if (type == NULL) {
> - error_report("Unable to find sPAPR CPU Core definition");
> - exit(1);
> - }
> -
> - spapr->cores = g_new0(Object *, spapr_max_cores);
> - for (i = 0; i < spapr_max_cores; i++) {
> - int core_id = i * smp_threads;
> - sPAPRDRConnector *drc =
> - spapr_dr_connector_new(OBJECT(spapr),
> - SPAPR_DR_CONNECTOR_TYPE_CPU,
> - (core_id / smp_threads) * smt);
> -
> - qemu_register_reset(spapr_drc_reset, drc);
> -
> - if (i < spapr_cores) {
> - Object *core = object_new(type);
> - object_property_set_int(core, smp_threads, "nr-threads",
> - &error_fatal);
> - object_property_set_int(core, core_id, CPU_CORE_PROP_CORE_ID,
> - &error_fatal);
> - object_property_set_bool(core, true, "realized", &error_fatal);
> - }
> - }
> - g_free(type);
> - } else {
> - for (i = 0; i < smp_cpus; i++) {
> - PowerPCCPU *cpu = cpu_ppc_init(machine->cpu_model);
> - if (cpu == NULL) {
> - error_report("Unable to find PowerPC CPU definition");
> - exit(1);
> - }
> - spapr_cpu_init(spapr, cpu, &error_fatal);
> - }
> - }
> + spapr_init_cpus(spapr);
>
> if (kvm_enabled()) {
> /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index e0c14f6..424d275 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -46,7 +46,8 @@ static void spapr_cpu_destroy(PowerPCCPU *cpu)
> qemu_unregister_reset(spapr_cpu_reset, cpu);
> }
>
> -void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> +static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> + Error **errp)
> {
> CPUPPCState *env = &cpu->env;
> CPUState *cs = CPU(cpu);
> @@ -166,11 +167,11 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> + MachineClass *mc = MACHINE_GET_CLASS(spapr);
> sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> CPUCore *cc = CPU_CORE(dev);
> CPUState *cs = CPU(core->threads);
> sPAPRDRConnector *drc;
> - sPAPRDRConnectorClass *drck;
> Error *local_err = NULL;
> void *fdt = NULL;
> int fdt_offset = 0;
> @@ -180,7 +181,7 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * smt);
> spapr->cores[index] = OBJECT(dev);
>
> - g_assert(drc);
> + g_assert(drc || !mc->query_hotpluggable_cpus);
>
> /*
> * Setup CPU DT entries only for hotplugged CPUs. For boot time or
> @@ -190,13 +191,15 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> }
>
> - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> - drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
> - if (local_err) {
> - g_free(fdt);
> - spapr->cores[index] = NULL;
> - error_propagate(errp, local_err);
> - return;
> + if (drc) {
> + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> + drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err);
> + if (local_err) {
> + g_free(fdt);
> + spapr->cores[index] = NULL;
> + error_propagate(errp, local_err);
> + return;
> + }
> }
>
> if (dev->hotplugged) {
> @@ -209,8 +212,11 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> /*
> * Set the right DRC states for cold plugged CPU.
> */
> - drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
> - drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> + if (drc) {
> + sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> + drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
> + drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> + }
> }
> }
>
> @@ -227,7 +233,7 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> char *base_core_type = spapr_get_cpu_core_type(machine->cpu_model);
> const char *type = object_get_typename(OBJECT(dev));
>
> - if (!mc->query_hotpluggable_cpus) {
> + if (dev->hotplugged && !mc->query_hotpluggable_cpus) {
> error_setg(&local_err, "CPU hotplug not supported for this machine");
> goto out;
> }
> @@ -237,11 +243,6 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> goto out;
> }
>
> - if (cc->nr_threads != smp_threads) {
> - error_setg(&local_err, "threads must be %d", smp_threads);
> - goto out;
> - }
> -
> if (cc->core_id % smp_threads) {
> error_setg(&local_err, "invalid core id %d", cc->core_id);
> goto out;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index bd5bcf7..f8d444d 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -614,7 +614,6 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
> uint32_t count, uint32_t index);
> void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
> uint32_t count, uint32_t index);
> -void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp);
> void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> sPAPRMachineState *spapr);
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine David Gibson
@ 2016-11-19 8:27 ` Greg Kurz
0 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2016-11-19 8:27 UTC (permalink / raw)
To: David Gibson
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
Hi David,
I've started to go through the testing you suggest in the cover letter. QEMU
crashes when trying to use the deprecated "compat" CPU property.
On Wed, 16 Nov 2016 09:17:53 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> Server class POWER CPUs have a "compat" property, which is used to set the
> backwards compatibility mode for the processor. However, this only makes
> sense for machine types which don't give the guest access to hypervisor
> privilege - otherwise the compatibility level is under the guest's control.
>
> To reflect this, this removes the CPU 'compat' property and instead
> creates a 'max-cpu-compat' property on the pseries machine. Strictly
> speaking this breaks compatibility, but AFAIK the 'compat' option was
> never (directly) used with -device or device_add.
>
> The option was used with -cpu. So, to maintain compatibility, this patch
> adds a hack to the cpu option parsing to strip out any compat options
> supplied with -cpu and set them on the machine property instead of the new
> removed cpu property.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
[...]
> +static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> {
> - Error *error = NULL;
> - char *value = NULL;
> - Property *prop = opaque;
> - uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> - visit_type_str(v, name, &value, &error);
> - if (error) {
> - error_propagate(errp, error);
> - return;
> - }
> -
> - if (strcmp(value, "power6") == 0) {
> - *max_compat = CPU_POWERPC_LOGICAL_2_05;
> - } else if (strcmp(value, "power7") == 0) {
> - *max_compat = CPU_POWERPC_LOGICAL_2_06;
> - } else if (strcmp(value, "power8") == 0) {
> - *max_compat = CPU_POWERPC_LOGICAL_2_07;
> - } else {
> - error_setg(errp, "Invalid compatibility mode \"%s\"", value);
> - }
> -
> - g_free(value);
> + error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
> + visit_type_null(v, name, errp);
The string input and output visitors don't support explicit null yet. I have
a tentative patch for that.
Cheers.
--
Greg
> }
>
> -static PropertyInfo powerpc_compat_propinfo = {
> +static PropertyInfo ppc_compat_deprecated_propinfo = {
> .name = "str",
> - .description = "compatibility mode, power6/power7/power8",
> - .get = powerpc_get_compat,
> - .set = powerpc_set_compat,
> + .description = "compatibility mode (deprecated)",
> + .get = getset_compat_deprecated,
> + .set = getset_compat_deprecated,
> };
> -
> -#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
> - DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
> -
> static Property powerpc_servercpu_properties[] = {
> - DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
> + {
> + .name = "compat",
> + .info = &ppc_compat_deprecated_propinfo,
> + },
> DEFINE_PROP_END_OF_LIST(),
> };
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (12 preceding siblings ...)
2016-11-15 22:44 ` [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling no-reply
@ 2016-11-26 0:33 ` Greg Kurz
2016-11-28 4:23 ` David Gibson
2016-12-01 13:16 ` Greg Kurz
14 siblings, 1 reply; 23+ messages in thread
From: Greg Kurz @ 2016-11-26 0:33 UTC (permalink / raw)
To: David Gibson
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
On Wed, 16 Nov 2016 09:17:43 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> This series is a significant rework to how we handle CPU compatibility
> modes on ppc.
>
> * Information about compatibility modes was previously open coded and
> scattered across a number of functions in both target-ppc and spapr
> code. It's now brought together into a common table of
> compatibility modes.
>
> * There was significant conceptual confusion about what a
> compatibility mode means, and how it interacts with the machine
> type. This cleans that up, clarifying that a compatibility mode
> (as an externally set option) only makes sense on machine types
> that don't permit the guest hypervisor privilege (i.e. 'pseries')
>
> * It was previously the user's (or management layer's) responsibility
> to determine compatibility of CPUs on either end for migration.
> This uses the compatibility modes to check that properly during an
> incoming migration.
>
> * Some ill-considered sanity checks broke migration from 2.6 to 2.7,
> due to some new instruction classes being added. This should avoid
> a repeat of that problem for 2.8 (we may be able to backport a
> minimal subset to 2.7-stable to fix the existing problem).
>
> Patches 1-3 are preliminary cleanups which could stand on their own.
> Patches 4-12 are the compatibility mode cleanup proper.
>
> So far, this has been mimimally tested. There are quite a few
> migration cases to check. For example:
>
> Basic:
>
> 1) Boot guest with -cpu host
> Should go into POWER8 compat mode after CAS
> Previously would have been raw mode
>
== QEMU ==
spapr_cas_pvr current=0, explicit_match=1, new=f000004
== guest ==
cpu : POWER8 (architected), altivec supported
> 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
> Should go into POWER7 compat mode
>
== QEMU ==
spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
== guest ==
cpu : POWER7 (architected), altivec supported
> 3) Boot guest with -cpu host,compat=power7
> Should act as (2), but print a warning
>
With extra patch to add explicit null to string visitors:
qapi: add explicit null to string input and output visitors
Message-Id: <147954362297.28064.5118492606031513925.stgit@bahia>
== QEMU ==
CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine
property instead
spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
== guest ==
cpu : POWER7 (architected), altivec supported
> 4) Boot guest via libvirt with power7 compat mode specified in XML
> Should act as (3), (2) once we fix libvirt
>
Not tested yet.
> 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Should go into POWER7 compat mode after CAS
>
== QEMU ==
spapr_cas_pvr current=0, explicit_match=1, new=f000003
== guest ==
cpu : POWER7 (architected), altivec supported
> 6) Hack guest to only advertise real PVRs
> Should remain in POWER8 raw mode after CAS
>
== QEMU ==
spapr_cas_pvr current=0, explicit_match=1, new=0
== guest ==
cpu : POWER8 (raw), altivec supported
> 7) Hack guest to only advertise real PVRs
> Boot with -machine pseries,max-cpu-compat=power8
> Should fail at CAS time
>
== QEMU ==
h_client_architecture_support() returns H_HARDWARE as
expected because max-cpu-compat is set and no compat
PVR was found (even though the real PVR was found).
== guest ==
WARNING: ibm,client-architecture-support call FAILED!
but the guest boots anyway and we end up with:
cpu : POWER8 (architected), altivec supported
This looks weird since the guest explicitly said it only
supports real PVRs... raw mode like case 6) would make
more sense IMHO but patch 11/12 sets the default to max-cpu-compat
at machine reset time:
+ ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
Maybe we should at least switch to raw mode, return an error
and let the guest decide ?
Another option would be to do as specified in LoPAPR section B.6.2.3
when no acceptable PVR was found and to simply terminate the guest.
> 8) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Reboot to normal guest
> Should go to power7 compat mode after CAS of boot 1
> Should revert to raw mode on reboot
> SHould go to power8 compat mode after CAS of boot 2
>
== QEMU ==
boot 1: spapr_cas_pvr current=0, explicit_match=1, new=f000003
boot 2: spapr_cas_pvr current=0, explicit_match=1, new=f000004
== guest ==
boot 1: cpu : POWER7 (architected), altivec supported
boot 2: cpu : POWER8 (architected), altivec supported
> Migration:
>
I'll give a try to migration next week.
Cheers.
--
Greg
> 9) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
> Should work, end up running in power8 raw mode
>
> 10) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
> Should work, end up running in power8 raw mode
>
> 11) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
> Should work, be running in POWER7 compat after, but give warning like
> (3)
>
> 12) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
> Should work, be running in POWER7 compat after, no warning
>
> 13) Boot to SLOF with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
> ?
>
> 14) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
> ?
>
> 15) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
> ?
>
> 16) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
> ?
>
> 17) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.7.z -machine pseries-2.6 -cpu host
> Should work
>
> 18) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Boot with qemu-2.8, migrate to qemu-2.8
> Should be in power7 compat mode after CAS on source, and still
> in power7 compat mode on destination
>
> Changes since RFCv1:
> * Change CAS logic to prefer compatibility modes over raw mode
> * Simplified by giving up on half-hearted attempts to maintain
> backwards migration
> * Folded migration stream changes into a single patch
> * Removed some preliminary patches which are already merged
>
> David Gibson (12):
> pseries: Always use core objects for CPU construction
> pseries: Make cpu_update during CAS unconditional
> ppc: Clean up and QOMify hypercall emulation
> ppc: Rename cpu_version to compat_pvr
> ppc: Rewrite ppc_set_compat()
> ppc: Rewrite ppc_get_compat_smt_threads()
> ppc: Validate compatibility modes when setting
> pseries: Rewrite CAS PVR compatibility logic
> ppc: Add ppc_set_compat_all()
> pseries: Move CPU compatibility property to machine
> pseries: Reset CPU compatibility mode
> ppc: Rework CPU compatibility testing across migration
>
> hw/ppc/spapr.c | 158 ++++++++++++++++------------
> hw/ppc/spapr_cpu_core.c | 85 ++++++++++-----
> hw/ppc/spapr_hcall.c | 140 +++++++------------------
> hw/ppc/trace-events | 2 +-
> include/hw/ppc/spapr.h | 12 ++-
> target-ppc/Makefile.objs | 2 +-
> target-ppc/compat.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
> target-ppc/cpu.h | 49 +++++++--
> target-ppc/excp_helper.c | 11 +-
> target-ppc/kvm.c | 4 +-
> target-ppc/kvm_ppc.h | 4 +-
> target-ppc/machine.c | 87 ++++++++++++++--
> target-ppc/translate_init.c | 157 +++++++---------------------
> 13 files changed, 607 insertions(+), 353 deletions(-)
> create mode 100644 target-ppc/compat.c
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
2016-11-26 0:33 ` Greg Kurz
@ 2016-11-28 4:23 ` David Gibson
2016-11-28 4:25 ` David Gibson
0 siblings, 1 reply; 23+ messages in thread
From: David Gibson @ 2016-11-28 4:23 UTC (permalink / raw)
To: Greg Kurz
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
[-- Attachment #1: Type: text/plain, Size: 6211 bytes --]
On Sat, Nov 26, 2016 at 01:33:16AM +0100, Greg Kurz wrote:
> On Wed, 16 Nov 2016 09:17:43 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > This series is a significant rework to how we handle CPU compatibility
> > modes on ppc.
> >
> > * Information about compatibility modes was previously open coded and
> > scattered across a number of functions in both target-ppc and spapr
> > code. It's now brought together into a common table of
> > compatibility modes.
> >
> > * There was significant conceptual confusion about what a
> > compatibility mode means, and how it interacts with the machine
> > type. This cleans that up, clarifying that a compatibility mode
> > (as an externally set option) only makes sense on machine types
> > that don't permit the guest hypervisor privilege (i.e. 'pseries')
> >
> > * It was previously the user's (or management layer's) responsibility
> > to determine compatibility of CPUs on either end for migration.
> > This uses the compatibility modes to check that properly during an
> > incoming migration.
> >
> > * Some ill-considered sanity checks broke migration from 2.6 to 2.7,
> > due to some new instruction classes being added. This should avoid
> > a repeat of that problem for 2.8 (we may be able to backport a
> > minimal subset to 2.7-stable to fix the existing problem).
> >
> > Patches 1-3 are preliminary cleanups which could stand on their own.
> > Patches 4-12 are the compatibility mode cleanup proper.
> >
> > So far, this has been mimimally tested. There are quite a few
> > migration cases to check. For example:
> >
> > Basic:
> >
> > 1) Boot guest with -cpu host
> > Should go into POWER8 compat mode after CAS
> > Previously would have been raw mode
> >
Thanks for doing these detailed tests, Greg.
>
> == QEMU ==
>
> spapr_cas_pvr current=0, explicit_match=1, new=f000004
>
> == guest ==
>
> cpu : POWER8 (architected), altivec supported
>
> > 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
> > Should go into POWER7 compat mode
> >
>
> == QEMU ==
>
> spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
>
> == guest ==
>
> cpu : POWER7 (architected), altivec supported
>
> > 3) Boot guest with -cpu host,compat=power7
> > Should act as (2), but print a warning
> >
>
> With extra patch to add explicit null to string visitors:
>
> qapi: add explicit null to string input and output visitors
> Message-Id: <147954362297.28064.5118492606031513925.stgit@bahia>
>
> == QEMU ==
>
> CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine
> property instead
>
> spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
>
> == guest ==
>
> cpu : POWER7 (architected), altivec supported
>
> > 4) Boot guest via libvirt with power7 compat mode specified in XML
> > Should act as (3), (2) once we fix libvirt
> >
>
> Not tested yet.
>
> > 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
> > Should go into POWER7 compat mode after CAS
> >
>
> == QEMU ==
>
> spapr_cas_pvr current=0, explicit_match=1, new=f000003
>
> == guest ==
>
> cpu : POWER7 (architected), altivec supported
>
> > 6) Hack guest to only advertise real PVRs
> > Should remain in POWER8 raw mode after CAS
> >
>
> == QEMU ==
>
> spapr_cas_pvr current=0, explicit_match=1, new=0
>
> == guest ==
>
> cpu : POWER8 (raw), altivec supported
>
> > 7) Hack guest to only advertise real PVRs
> > Boot with -machine pseries,max-cpu-compat=power8
> > Should fail at CAS time
> >
>
> == QEMU ==
>
> h_client_architecture_support() returns H_HARDWARE as
> expected because max-cpu-compat is set and no compat
> PVR was found (even though the real PVR was found).
>
> == guest ==
>
> WARNING: ibm,client-architecture-support call FAILED!
>
> but the guest boots anyway and we end up with:
>
> cpu : POWER8 (architected), altivec supported
>
> This looks weird since the guest explicitly said it only
> supports real PVRs... raw mode like case 6) would make
> more sense IMHO but patch 11/12 sets the default to max-cpu-compat
> at machine reset time:
>
> + ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
>
> Maybe we should at least switch to raw mode, return an error
> and let the guest decide ?
>
> Another option would be to do as specified in LoPAPR section B.6.2.3
> when no acceptable PVR was found and to simply terminate the guest.
So.. I suspect this is probably good enough in practice, given that
all known guests will actually advertise compat modes. The FAILED
gives at least a hint as to what's going on.
To get it strictly correct, then yes, I think terminating the guest is
probably the PAPRishly correct thing to do, although it's not clear
quite what we do then. We don't really have a mechanism for shutting
the VM down entirely (which might surprise management), and if we
reboot we're likely to just hit the same error again.
Hence my inclination to only worry about those details if someone
starts hitting them for real.
> > 8) Hack guest to only advertise power7 compatibility, boot with -cpu host
> > Reboot to normal guest
> > Should go to power7 compat mode after CAS of boot 1
> > Should revert to raw mode on reboot
> > SHould go to power8 compat mode after CAS of boot 2
> >
>
> == QEMU ==
>
> boot 1: spapr_cas_pvr current=0, explicit_match=1, new=f000003
> boot 2: spapr_cas_pvr current=0, explicit_match=1, new=f000004
>
> == guest ==
>
> boot 1: cpu : POWER7 (architected), altivec supported
> boot 2: cpu : POWER8 (architected), altivec supported
>
> > Migration:
> >
>
> I'll give a try to migration next week.
>
> Cheers.
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
2016-11-28 4:23 ` David Gibson
@ 2016-11-28 4:25 ` David Gibson
0 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2016-11-28 4:25 UTC (permalink / raw)
To: Greg Kurz
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
[-- Attachment #1: Type: text/plain, Size: 6833 bytes --]
On Mon, Nov 28, 2016 at 03:23:46PM +1100, David Gibson wrote:
> On Sat, Nov 26, 2016 at 01:33:16AM +0100, Greg Kurz wrote:
> > On Wed, 16 Nov 2016 09:17:43 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > > This series is a significant rework to how we handle CPU compatibility
> > > modes on ppc.
> > >
> > > * Information about compatibility modes was previously open coded and
> > > scattered across a number of functions in both target-ppc and spapr
> > > code. It's now brought together into a common table of
> > > compatibility modes.
> > >
> > > * There was significant conceptual confusion about what a
> > > compatibility mode means, and how it interacts with the machine
> > > type. This cleans that up, clarifying that a compatibility mode
> > > (as an externally set option) only makes sense on machine types
> > > that don't permit the guest hypervisor privilege (i.e. 'pseries')
> > >
> > > * It was previously the user's (or management layer's) responsibility
> > > to determine compatibility of CPUs on either end for migration.
> > > This uses the compatibility modes to check that properly during an
> > > incoming migration.
> > >
> > > * Some ill-considered sanity checks broke migration from 2.6 to 2.7,
> > > due to some new instruction classes being added. This should avoid
> > > a repeat of that problem for 2.8 (we may be able to backport a
> > > minimal subset to 2.7-stable to fix the existing problem).
> > >
> > > Patches 1-3 are preliminary cleanups which could stand on their own.
> > > Patches 4-12 are the compatibility mode cleanup proper.
> > >
> > > So far, this has been mimimally tested. There are quite a few
> > > migration cases to check. For example:
> > >
> > > Basic:
> > >
> > > 1) Boot guest with -cpu host
> > > Should go into POWER8 compat mode after CAS
> > > Previously would have been raw mode
> > >
>
>
> Thanks for doing these detailed tests, Greg.
>
> >
> > == QEMU ==
> >
> > spapr_cas_pvr current=0, explicit_match=1, new=f000004
> >
> > == guest ==
> >
> > cpu : POWER8 (architected), altivec supported
> >
> > > 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
> > > Should go into POWER7 compat mode
> > >
> >
> > == QEMU ==
> >
> > spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
> >
> > == guest ==
> >
> > cpu : POWER7 (architected), altivec supported
> >
> > > 3) Boot guest with -cpu host,compat=power7
> > > Should act as (2), but print a warning
> > >
> >
> > With extra patch to add explicit null to string visitors:
> >
> > qapi: add explicit null to string input and output visitors
> > Message-Id: <147954362297.28064.5118492606031513925.stgit@bahia>
> >
> > == QEMU ==
> >
> > CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine
> > property instead
> >
> > spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
> >
> > == guest ==
> >
> > cpu : POWER7 (architected), altivec supported
> >
> > > 4) Boot guest via libvirt with power7 compat mode specified in XML
> > > Should act as (3), (2) once we fix libvirt
> > >
> >
> > Not tested yet.
> >
> > > 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
> > > Should go into POWER7 compat mode after CAS
> > >
> >
> > == QEMU ==
> >
> > spapr_cas_pvr current=0, explicit_match=1, new=f000003
> >
> > == guest ==
> >
> > cpu : POWER7 (architected), altivec supported
> >
> > > 6) Hack guest to only advertise real PVRs
> > > Should remain in POWER8 raw mode after CAS
> > >
> >
> > == QEMU ==
> >
> > spapr_cas_pvr current=0, explicit_match=1, new=0
> >
> > == guest ==
> >
> > cpu : POWER8 (raw), altivec supported
> >
> > > 7) Hack guest to only advertise real PVRs
> > > Boot with -machine pseries,max-cpu-compat=power8
> > > Should fail at CAS time
> > >
> >
> > == QEMU ==
> >
> > h_client_architecture_support() returns H_HARDWARE as
> > expected because max-cpu-compat is set and no compat
> > PVR was found (even though the real PVR was found).
> >
> > == guest ==
> >
> > WARNING: ibm,client-architecture-support call FAILED!
> >
> > but the guest boots anyway and we end up with:
> >
> > cpu : POWER8 (architected), altivec supported
> >
> > This looks weird since the guest explicitly said it only
> > supports real PVRs... raw mode like case 6) would make
> > more sense IMHO but patch 11/12 sets the default to max-cpu-compat
> > at machine reset time:
> >
> > + ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
> >
> > Maybe we should at least switch to raw mode, return an error
> > and let the guest decide ?
> >
> > Another option would be to do as specified in LoPAPR section B.6.2.3
> > when no acceptable PVR was found and to simply terminate the guest.
>
> So.. I suspect this is probably good enough in practice, given that
> all known guests will actually advertise compat modes. The FAILED
> gives at least a hint as to what's going on.
>
> To get it strictly correct, then yes, I think terminating the guest is
> probably the PAPRishly correct thing to do, although it's not clear
> quite what we do then. We don't really have a mechanism for shutting
> the VM down entirely (which might surprise management), and if we
> reboot we're likely to just hit the same error again.
>
> Hence my inclination to only worry about those details if someone
> starts hitting them for real.
Oh.. that said, I think we should at least throw in a meaningful
error_report() since that should get propagated up to the error logs
that mangement layers are likely to keep.
>
> > > 8) Hack guest to only advertise power7 compatibility, boot with -cpu host
> > > Reboot to normal guest
> > > Should go to power7 compat mode after CAS of boot 1
> > > Should revert to raw mode on reboot
> > > SHould go to power8 compat mode after CAS of boot 2
> > >
> >
> > == QEMU ==
> >
> > boot 1: spapr_cas_pvr current=0, explicit_match=1, new=f000003
> > boot 2: spapr_cas_pvr current=0, explicit_match=1, new=f000004
> >
> > == guest ==
> >
> > boot 1: cpu : POWER7 (architected), altivec supported
> > boot 2: cpu : POWER8 (architected), altivec supported
> >
> > > Migration:
> > >
> >
> > I'll give a try to migration next week.
> >
> > Cheers.
> >
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
` (13 preceding siblings ...)
2016-11-26 0:33 ` Greg Kurz
@ 2016-12-01 13:16 ` Greg Kurz
14 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2016-12-01 13:16 UTC (permalink / raw)
To: David Gibson
Cc: clg, aik, mdroth, nikunj, agraf, qemu-ppc, qemu-devel, abologna,
thuth, lvivier
On Wed, 16 Nov 2016 09:17:43 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> This series is a significant rework to how we handle CPU compatibility
> modes on ppc.
>
David,
Please find below the results of the migration tests.
> * Information about compatibility modes was previously open coded and
> scattered across a number of functions in both target-ppc and spapr
> code. It's now brought together into a common table of
> compatibility modes.
>
> * There was significant conceptual confusion about what a
> compatibility mode means, and how it interacts with the machine
> type. This cleans that up, clarifying that a compatibility mode
> (as an externally set option) only makes sense on machine types
> that don't permit the guest hypervisor privilege (i.e. 'pseries')
>
> * It was previously the user's (or management layer's) responsibility
> to determine compatibility of CPUs on either end for migration.
> This uses the compatibility modes to check that properly during an
> incoming migration.
>
> * Some ill-considered sanity checks broke migration from 2.6 to 2.7,
> due to some new instruction classes being added. This should avoid
> a repeat of that problem for 2.8 (we may be able to backport a
> minimal subset to 2.7-stable to fix the existing problem).
>
> Patches 1-3 are preliminary cleanups which could stand on their own.
> Patches 4-12 are the compatibility mode cleanup proper.
>
> So far, this has been mimimally tested. There are quite a few
> migration cases to check. For example:
>
> Basic:
>
> 1) Boot guest with -cpu host
> Should go into POWER8 compat mode after CAS
> Previously would have been raw mode
>
> 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
> Should go into POWER7 compat mode
>
> 3) Boot guest with -cpu host,compat=power7
> Should act as (2), but print a warning
>
> 4) Boot guest via libvirt with power7 compat mode specified in XML
> Should act as (3), (2) once we fix libvirt
>
> 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Should go into POWER7 compat mode after CAS
>
> 6) Hack guest to only advertise real PVRs
> Should remain in POWER8 raw mode after CAS
>
> 7) Hack guest to only advertise real PVRs
> Boot with -machine pseries,max-cpu-compat=power8
> Should fail at CAS time
>
> 8) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Reboot to normal guest
> Should go to power7 compat mode after CAS of boot 1
> Should revert to raw mode on reboot
> SHould go to power8 compat mode after CAS of boot 2
>
> Migration:
>
The QEMU command line used to test migration is as follows:
ppc64-softmmu/qemu-system-ppc64 \
-snapshot \
-nodefaults \
-no-shutdown \
-nographic \
-device virtio-blk-pci,drive=drive0 \
-drive file=/home/greg/images/fedora24-ppc64.qcow2,id=drive0,if=none \
-global virtio-blk-pci.disable-legacy=off \
-global virtio-blk-pci.disable-modern=on \
-device virtio-net,netdev=netdev0,mac=C0:FF:EE:00:00:66,id=net0 \
-netdev tap,id=netdev0,vhost=off,helper=/usr/libexec/qemu-bridge-helper \
-global virtio-net-pci.disable-legacy=off \
-global virtio-net-pci.disable-modern=on \
-m 4G \
-serial mon:stdio \
-trace spapr_cas_pvr
Note that virtio devices are explicitely configured to run in legacy
mode because I couldn't pass tests 15 and 16 otherwise, with various
issues including QEMU getting killed by OOM ! I'll focus on these
issues separately.
> 9) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
> Should work, end up running in power8 raw mode
>
== QEMU-2.6 ==
spapr_cas_pvr current=0, cpu_match=1, new=0, compat flags=6
== guest (source) ==
cpu : POWER8 (raw), altivec supported
== guest (target) ==
cpu : POWER8 (raw), altivec supported
> 10) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
> Should work, end up running in power8 raw mode
>
== QEMU-2.7 ==
spapr_cas_pvr current=0, cpu_match=1, new=0, compat flags=2000000000000006
== guest (source) ==
cpu : POWER8 (raw), altivec supported
== guest (target) ==
cpu : POWER8 (raw), altivec supported
> 11) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
> Should work, be running in POWER7 compat after, but give warning like
> (3)
>
== QEMU-2.7 ==
spapr_cas_pvr current=f000003, cpu_match=1, new=f000003, compat flags=2000000000000006
== guest (source) ==
cpu : POWER7 (architected), altivec supported
== QEMU-2.8 ==
CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead
Migration completes but the guest gets a program interrupt:
(qemu) info registers
NIP 0000000000000700 LR c0000000008309ac CTR c000000000830b40 XER 0000000020000000 CPU#0
MSR 8000000000001000 HID0 0000000000000000 HF 8000000000000000 iidx 3 didx 3
TB 00000000 00000000 DECR 00000000
GPR00 0000000000000000 0000000000000000 0000000000000000 000000007fef0000
GPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR08 0000000000000000 0000000020000000 6000000060000000 6000000060006180
GPR12 c000000000081000 0000000000000000 0000000000000000 0000000000000000
GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
CR 20000000 [ E - - - - - - - ] RES ffffffffffffffff
FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPSCR 0000000000000000
SRR0 6000000060006180 SRR1 c000000000081000 PVR 00000000004d0200 VRSAVE 0000000000000000
SPRG0 0000000000000000 SPRG1 0000000000000000 SPRG2 0000000000000000 SPRG3 0000000000000000
SPRG4 0000000000000000 SPRG5 0000000000000000 SPRG6 0000000000000000 SPRG7 0000000000000000
HSRR0 0000000000000000 HSRR1 0000000000000000
CFAR 0000000000000000
SDR1 0000000000000007 DAR 0000000000000000 DSISR 0000000000000000
Same happens with a pseries-2.6 machine.
Would you have suggestions how to debug this ? The values in SRR0 and SRR1 look
weird compared to the what is described in the ISA...
> 12) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
> Should work, be running in POWER7 compat after, no warning
>
Same as 11) except the CPU 'compat' warning for both pseries-2.6 and pseries-2.7.
It seems to be related to the compat mode itself as I also hit the error when
running with qemu-2.8 -machine pseries-2.8,max-cpu-compat=power8 on a POWER8
host.
> 13) Boot to SLOF with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
> ?
>
Migration succeeds, typing 'boot' at the SLOF prompt succeeds in booting the
system:
== QEMU-2.8 ==
spapr_cas_pvr current=0, explicit_match=1, new=f000004
== guest (target) ==
cpu : POWER8 (architected), altivec supported
> 14) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
> ?
>
Same as 13)
> 15) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
> ?
>
== QEMU-2.8 ==
CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead
Migration succeeds, but this time SLOF then boots the system automatically:
spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
== guest (target) ==
cpu : POWER7 (architected), altivec supported
The same happens with pseries-2.6.
> 16) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
> Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
> ?
>
Same as 16) except the CPU 'compat' warning for both pseries-2.6 and pseries-2.7.
> 17) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
> Migrate to qemu-2.7.z -machine pseries-2.6 -cpu host
> Should work
>
It doesn't. Migration fails on destination:
error while loading state for instance 0x0 of device 'cpu'
load of migration failed: Invalid argument
> 18) Hack guest to only advertise power7 compatibility, boot with -cpu host
> Boot with qemu-2.8, migrate to qemu-2.8
> Should be in power7 compat mode after CAS on source, and still
> in power7 compat mode on destination
>
Same failure as 11)
Cheers.
--
Greg
> Changes since RFCv1:
> * Change CAS logic to prefer compatibility modes over raw mode
> * Simplified by giving up on half-hearted attempts to maintain
> backwards migration
> * Folded migration stream changes into a single patch
> * Removed some preliminary patches which are already merged
>
> David Gibson (12):
> pseries: Always use core objects for CPU construction
> pseries: Make cpu_update during CAS unconditional
> ppc: Clean up and QOMify hypercall emulation
> ppc: Rename cpu_version to compat_pvr
> ppc: Rewrite ppc_set_compat()
> ppc: Rewrite ppc_get_compat_smt_threads()
> ppc: Validate compatibility modes when setting
> pseries: Rewrite CAS PVR compatibility logic
> ppc: Add ppc_set_compat_all()
> pseries: Move CPU compatibility property to machine
> pseries: Reset CPU compatibility mode
> ppc: Rework CPU compatibility testing across migration
>
> hw/ppc/spapr.c | 158 ++++++++++++++++------------
> hw/ppc/spapr_cpu_core.c | 85 ++++++++++-----
> hw/ppc/spapr_hcall.c | 140 +++++++------------------
> hw/ppc/trace-events | 2 +-
> include/hw/ppc/spapr.h | 12 ++-
> target-ppc/Makefile.objs | 2 +-
> target-ppc/compat.c | 249 ++++++++++++++++++++++++++++++++++++++++++++
> target-ppc/cpu.h | 49 +++++++--
> target-ppc/excp_helper.c | 11 +-
> target-ppc/kvm.c | 4 +-
> target-ppc/kvm_ppc.h | 4 +-
> target-ppc/machine.c | 87 ++++++++++++++--
> target-ppc/translate_init.c | 157 +++++++---------------------
> 13 files changed, 607 insertions(+), 353 deletions(-)
> create mode 100644 target-ppc/compat.c
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration David Gibson
@ 2016-12-02 14:48 ` Greg Kurz
2016-12-05 4:09 ` David Gibson
0 siblings, 1 reply; 23+ messages in thread
From: Greg Kurz @ 2016-12-02 14:48 UTC (permalink / raw)
To: David Gibson
Cc: clg, aik, mdroth, nikunj, lvivier, thuth, qemu-devel, abologna, qemu-ppc
On Wed, 16 Nov 2016 09:17:55 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> Migrating between different CPU versions is quite complicated for ppc.
> A long time ago, we ensure identical CPU versions at either end by checking
> the PVR had the same value. However, this breaks under KVM HV, because
> we always have to use the host's PVR - it's not virtualized. That would
> mean we couldn't migrate between hosts with different PVRs, even if the
> CPUs are close enough to compatible in practice (sometimes identical cores
> with different surrounding logic have different PVRs, so this happens in
> practice quite often).
>
> So, we removed the PVR check, but instead checked that several flags
> indicating supported instructions matched. This turns out to be a bad
> idea, because those instruction masks are not architected information, but
> essentially a TCG implementation detail. So changes to qemu internal CPU
> modelling can break migration - this happened between qemu-2.6 and
> qemu-2.7.
>
> Modern server-class CPUs can be placed into compatibility modes. Now that
> we're handling those properly, we finally have the information to sanely
> deal with CPU compatibility across migration.
>
> This patch bumps the migration version number for the ppc CPU removing the
> instruction mask field (and some other unwise VMSTATE_EQUAL checks), and
> adding the compatibility PVR to the migration stream.
>
Things have changed since you posted this RFC:
commit 16a2497bd44cac1856e259654fd304079bd1dcdc
Author: David Gibson <david@gibson.dropbear.id.au>
Date: Mon Nov 21 16:28:12 2016 +1100
target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
and
commit 146c11f16f12dbfea62cbd7f865614bb6fcbc6b5
Author: David Gibson <david@gibson.dropbear.id.au>
Date: Mon Nov 21 16:29:30 2016 +1100
target-ppc: Allow eventual removal of old migration mistakes
I guess that the version bumping isn't necessary anymore if we keep these.
I'll assume yes and rebase this patch against current master, simply dropping
the version bumping and related lines.
> We consider the CPUs compatible for migration if:
> * The source was running in a compatibility mode which the destination
> supports
> OR * The source has a PVR matching the same qemu CPU class as the
> destination, either an exact match or an approximate match determined
> by the cpu class's pvr_match hook.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target-ppc/machine.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 79 insertions(+), 8 deletions(-)
>
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index e43cb6c..25a30d5 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -8,6 +8,7 @@
> #include "helper_regs.h"
> #include "mmu-hash64.h"
> #include "migration/cpu.h"
> +#include "qapi/error.h"
>
> static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> {
> @@ -163,6 +164,30 @@ static void cpu_pre_save(void *opaque)
> }
> }
>
> +/*
> + * Determine if a given PVR is a "close enough" match to the CPU
> + * object. For TCG and KVM PR it would probably be sufficient to
> + * require an exact PVR match. However for KVM HV the user is
> + * restricted to a PVR exactly matching the host CPU. The correct way
> + * to handle this is to put the guest into an architected
> + * compatibility mode. However, to allow a more forgiving transition
> + * and migration from before this was widely done, we allow migration
> + * between sufficiently similar PVRs, as determined by the CPU class's
> + * pvr_match() hook.
> + */
> +static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
> +{
> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +
> + if (pvr == pcc->pvr) {
> + return true;
> + }
> + if (pcc->pvr_match) {
> + return pcc->pvr_match(pcc, pvr);
> + }
> + return false;
> +}
> +
> static int cpu_post_load(void *opaque, int version_id)
> {
> PowerPCCPU *cpu = opaque;
> @@ -171,10 +196,31 @@ static int cpu_post_load(void *opaque, int version_id)
> target_ulong msr;
>
> /*
> - * We always ignore the source PVR. The user or management
> - * software has to take care of running QEMU in a compatible mode.
> + * If we're operating in compat mode, we should be ok as long as
> + * the destination supports the same compatiblity mode.
> + *
> + * Otherwise, however, we require that the destination has exactly
> + * the same CPU model as the source.
> */
> - env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> +
> +#if defined(TARGET_PPC64)
> + if (cpu->compat_pvr) {
> + Error *local_err = NULL;
> +
> + ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
This calls cpu_synchronize_state(CPU(cpu)) and trashes the registers. This
is the root cause behind the program interrupts I mentioned in another mail.
Adding a sync_needed boolean argument to ppc_set_compat() seems to be enough
to get this working. So I'll just do that and rerun the tests.
Cheers.
--
Greg
> + if (local_err) {
> + error_report_err(local_err);
> + error_free(local_err);
> + return -1;
> + }
> + } else
> +#endif
> + {
> + if (!pvr_match(cpu, env->spr[SPR_PVR])) {
> + return -1;
> + }
> + }
> +
> env->lr = env->spr[SPR_LR];
> env->ctr = env->spr[SPR_CTR];
> cpu_write_xer(env, env->spr[SPR_XER]);
> @@ -527,9 +573,32 @@ static const VMStateDescription vmstate_tlbmas = {
> }
> };
>
> +static bool compat_needed(void *opaque)
> +{
> + PowerPCCPU *cpu = opaque;
> +
> + return cpu->vhyp != NULL;
> +}
> +
> +static const VMStateDescription vmstate_compat = {
> + .name = "cpu/compat",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = compat_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(compat_pvr, PowerPCCPU),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static bool version_before_6(void *opaque, int version_id)
> +{
> + return version_id < 6;
> +}
> +
> const VMStateDescription vmstate_ppc_cpu = {
> .name = "cpu",
> - .version_id = 5,
> + .version_id = 6,
> .minimum_version_id = 5,
> .minimum_version_id_old = 4,
> .load_state_old = cpu_load_old,
> @@ -561,10 +630,11 @@ const VMStateDescription vmstate_ppc_cpu = {
> /* FIXME: access_type? */
>
> /* Sanity checking */
> - VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
> - VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> - VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
> - VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
> + VMSTATE_UNUSED_TEST(version_before_6,
> + sizeof(target_ulong) /* msr_mask */
> + + sizeof(uint64_t) /* insns_flags */
> + + sizeof(uint64_t) /* insns_flags2 */
> + + sizeof(uint32_t)), /* nb_BATs */
> VMSTATE_END_OF_LIST()
> },
> .subsections = (const VMStateDescription*[]) {
> @@ -579,6 +649,7 @@ const VMStateDescription vmstate_ppc_cpu = {
> &vmstate_tlb6xx,
> &vmstate_tlbemb,
> &vmstate_tlbmas,
> + &vmstate_compat,
> NULL
> }
> };
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration
2016-12-02 14:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2016-12-05 4:09 ` David Gibson
2016-12-13 17:58 ` Greg Kurz
0 siblings, 1 reply; 23+ messages in thread
From: David Gibson @ 2016-12-05 4:09 UTC (permalink / raw)
To: Greg Kurz
Cc: clg, aik, mdroth, nikunj, lvivier, thuth, qemu-devel, abologna, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 6007 bytes --]
On Fri, Dec 02, 2016 at 03:48:25PM +0100, Greg Kurz wrote:
> On Wed, 16 Nov 2016 09:17:55 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > Migrating between different CPU versions is quite complicated for ppc.
> > A long time ago, we ensure identical CPU versions at either end by checking
> > the PVR had the same value. However, this breaks under KVM HV, because
> > we always have to use the host's PVR - it's not virtualized. That would
> > mean we couldn't migrate between hosts with different PVRs, even if the
> > CPUs are close enough to compatible in practice (sometimes identical cores
> > with different surrounding logic have different PVRs, so this happens in
> > practice quite often).
> >
> > So, we removed the PVR check, but instead checked that several flags
> > indicating supported instructions matched. This turns out to be a bad
> > idea, because those instruction masks are not architected information, but
> > essentially a TCG implementation detail. So changes to qemu internal CPU
> > modelling can break migration - this happened between qemu-2.6 and
> > qemu-2.7.
> >
> > Modern server-class CPUs can be placed into compatibility modes. Now that
> > we're handling those properly, we finally have the information to sanely
> > deal with CPU compatibility across migration.
> >
> > This patch bumps the migration version number for the ppc CPU removing the
> > instruction mask field (and some other unwise VMSTATE_EQUAL checks), and
> > adding the compatibility PVR to the migration stream.
> >
>
> Things have changed since you posted this RFC:
>
> commit 16a2497bd44cac1856e259654fd304079bd1dcdc
> Author: David Gibson <david@gibson.dropbear.id.au>
> Date: Mon Nov 21 16:28:12 2016 +1100
>
> target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
>
> and
>
> commit 146c11f16f12dbfea62cbd7f865614bb6fcbc6b5
> Author: David Gibson <david@gibson.dropbear.id.au>
> Date: Mon Nov 21 16:29:30 2016 +1100
>
> target-ppc: Allow eventual removal of old migration mistakes
>
> I guess that the version bumping isn't necessary anymore if we keep these.
>
> I'll assume yes and rebase this patch against current master, simply dropping
> the version bumping and related lines.
Yeah, I realised that breaking backwards migration was a bad idea, and
with some help from Dave Gilbert worked out how to make it possible.
I realize I'm going to have to rework my compat series in light of
these changes.
> > We consider the CPUs compatible for migration if:
> > * The source was running in a compatibility mode which the destination
> > supports
> > OR * The source has a PVR matching the same qemu CPU class as the
> > destination, either an exact match or an approximate match determined
> > by the cpu class's pvr_match hook.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > target-ppc/machine.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 79 insertions(+), 8 deletions(-)
> >
> > diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> > index e43cb6c..25a30d5 100644
> > --- a/target-ppc/machine.c
> > +++ b/target-ppc/machine.c
> > @@ -8,6 +8,7 @@
> > #include "helper_regs.h"
> > #include "mmu-hash64.h"
> > #include "migration/cpu.h"
> > +#include "qapi/error.h"
> >
> > static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> > {
> > @@ -163,6 +164,30 @@ static void cpu_pre_save(void *opaque)
> > }
> > }
> >
> > +/*
> > + * Determine if a given PVR is a "close enough" match to the CPU
> > + * object. For TCG and KVM PR it would probably be sufficient to
> > + * require an exact PVR match. However for KVM HV the user is
> > + * restricted to a PVR exactly matching the host CPU. The correct way
> > + * to handle this is to put the guest into an architected
> > + * compatibility mode. However, to allow a more forgiving transition
> > + * and migration from before this was widely done, we allow migration
> > + * between sufficiently similar PVRs, as determined by the CPU class's
> > + * pvr_match() hook.
> > + */
> > +static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
> > +{
> > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > +
> > + if (pvr == pcc->pvr) {
> > + return true;
> > + }
> > + if (pcc->pvr_match) {
> > + return pcc->pvr_match(pcc, pvr);
> > + }
> > + return false;
> > +}
> > +
> > static int cpu_post_load(void *opaque, int version_id)
> > {
> > PowerPCCPU *cpu = opaque;
> > @@ -171,10 +196,31 @@ static int cpu_post_load(void *opaque, int version_id)
> > target_ulong msr;
> >
> > /*
> > - * We always ignore the source PVR. The user or management
> > - * software has to take care of running QEMU in a compatible mode.
> > + * If we're operating in compat mode, we should be ok as long as
> > + * the destination supports the same compatiblity mode.
> > + *
> > + * Otherwise, however, we require that the destination has exactly
> > + * the same CPU model as the source.
> > */
> > - env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> > +
> > +#if defined(TARGET_PPC64)
> > + if (cpu->compat_pvr) {
> > + Error *local_err = NULL;
> > +
> > + ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
>
> This calls cpu_synchronize_state(CPU(cpu)) and trashes the registers. This
> is the root cause behind the program interrupts I mentioned in another mail.
>
> Adding a sync_needed boolean argument to ppc_set_compat() seems to be enough
> to get this working. So I'll just do that and rerun the tests.
>
> Cheers.
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration
2016-12-05 4:09 ` David Gibson
@ 2016-12-13 17:58 ` Greg Kurz
0 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2016-12-13 17:58 UTC (permalink / raw)
To: David Gibson; +Cc: lvivier, thuth, mdroth, qemu-devel, qemu-ppc, clg, abologna
[-- Attachment #1: Type: text/plain, Size: 7251 bytes --]
On Mon, 5 Dec 2016 15:09:16 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Dec 02, 2016 at 03:48:25PM +0100, Greg Kurz wrote:
> > On Wed, 16 Nov 2016 09:17:55 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > > Migrating between different CPU versions is quite complicated for ppc.
> > > A long time ago, we ensure identical CPU versions at either end by checking
> > > the PVR had the same value. However, this breaks under KVM HV, because
> > > we always have to use the host's PVR - it's not virtualized. That would
> > > mean we couldn't migrate between hosts with different PVRs, even if the
> > > CPUs are close enough to compatible in practice (sometimes identical cores
> > > with different surrounding logic have different PVRs, so this happens in
> > > practice quite often).
> > >
> > > So, we removed the PVR check, but instead checked that several flags
> > > indicating supported instructions matched. This turns out to be a bad
> > > idea, because those instruction masks are not architected information, but
> > > essentially a TCG implementation detail. So changes to qemu internal CPU
> > > modelling can break migration - this happened between qemu-2.6 and
> > > qemu-2.7.
> > >
> > > Modern server-class CPUs can be placed into compatibility modes. Now that
> > > we're handling those properly, we finally have the information to sanely
> > > deal with CPU compatibility across migration.
> > >
> > > This patch bumps the migration version number for the ppc CPU removing the
> > > instruction mask field (and some other unwise VMSTATE_EQUAL checks), and
> > > adding the compatibility PVR to the migration stream.
> > >
> >
> > Things have changed since you posted this RFC:
> >
> > commit 16a2497bd44cac1856e259654fd304079bd1dcdc
> > Author: David Gibson <david@gibson.dropbear.id.au>
> > Date: Mon Nov 21 16:28:12 2016 +1100
> >
> > target-ppc: Fix CPU migration from qemu-2.6 <-> later versions
> >
> > and
> >
> > commit 146c11f16f12dbfea62cbd7f865614bb6fcbc6b5
> > Author: David Gibson <david@gibson.dropbear.id.au>
> > Date: Mon Nov 21 16:29:30 2016 +1100
> >
> > target-ppc: Allow eventual removal of old migration mistakes
> >
> > I guess that the version bumping isn't necessary anymore if we keep these.
> >
> > I'll assume yes and rebase this patch against current master, simply dropping
> > the version bumping and related lines.
>
> Yeah, I realised that breaking backwards migration was a bad idea, and
> with some help from Dave Gilbert worked out how to make it possible.
>
> I realize I'm going to have to rework my compat series in light of
> these changes.
>
While doing so, maybe you can fold the following in your patch, and thus
make tests/qom-test happy:
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -32,6 +32,7 @@
#include "qapi/visitor.h"
#include "hw/qdev-properties.h"
#include "hw/ppc/ppc.h"
+#include "sysemu/qtest.h"
//#define PPC_DUMP_CPU
//#define PPC_DEBUG_SPR
@@ -8442,7 +8443,9 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
+ if (!qtest_enabled()) {
+ error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");
+ }
visit_type_null(v, name, errp);
}
Also a friendly reminder: cpu registers get trashed during cpu post
load (see at the end of this mail).
Cheers.
--
Greg
> > > We consider the CPUs compatible for migration if:
> > > * The source was running in a compatibility mode which the destination
> > > supports
> > > OR * The source has a PVR matching the same qemu CPU class as the
> > > destination, either an exact match or an approximate match determined
> > > by the cpu class's pvr_match hook.
> > >
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > > target-ppc/machine.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++-----
> > > 1 file changed, 79 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> > > index e43cb6c..25a30d5 100644
> > > --- a/target-ppc/machine.c
> > > +++ b/target-ppc/machine.c
> > > @@ -8,6 +8,7 @@
> > > #include "helper_regs.h"
> > > #include "mmu-hash64.h"
> > > #include "migration/cpu.h"
> > > +#include "qapi/error.h"
> > >
> > > static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> > > {
> > > @@ -163,6 +164,30 @@ static void cpu_pre_save(void *opaque)
> > > }
> > > }
> > >
> > > +/*
> > > + * Determine if a given PVR is a "close enough" match to the CPU
> > > + * object. For TCG and KVM PR it would probably be sufficient to
> > > + * require an exact PVR match. However for KVM HV the user is
> > > + * restricted to a PVR exactly matching the host CPU. The correct way
> > > + * to handle this is to put the guest into an architected
> > > + * compatibility mode. However, to allow a more forgiving transition
> > > + * and migration from before this was widely done, we allow migration
> > > + * between sufficiently similar PVRs, as determined by the CPU class's
> > > + * pvr_match() hook.
> > > + */
> > > +static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
> > > +{
> > > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > > +
> > > + if (pvr == pcc->pvr) {
> > > + return true;
> > > + }
> > > + if (pcc->pvr_match) {
> > > + return pcc->pvr_match(pcc, pvr);
> > > + }
> > > + return false;
> > > +}
> > > +
> > > static int cpu_post_load(void *opaque, int version_id)
> > > {
> > > PowerPCCPU *cpu = opaque;
> > > @@ -171,10 +196,31 @@ static int cpu_post_load(void *opaque, int version_id)
> > > target_ulong msr;
> > >
> > > /*
> > > - * We always ignore the source PVR. The user or management
> > > - * software has to take care of running QEMU in a compatible mode.
> > > + * If we're operating in compat mode, we should be ok as long as
> > > + * the destination supports the same compatiblity mode.
> > > + *
> > > + * Otherwise, however, we require that the destination has exactly
> > > + * the same CPU model as the source.
> > > */
> > > - env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> > > +
> > > +#if defined(TARGET_PPC64)
> > > + if (cpu->compat_pvr) {
> > > + Error *local_err = NULL;
> > > +
> > > + ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
> >
> > This calls cpu_synchronize_state(CPU(cpu)) and trashes the registers. This
> > is the root cause behind the program interrupts I mentioned in another mail.
> >
> > Adding a sync_needed boolean argument to ppc_set_compat() seems to be enough
> > to get this working. So I'll just do that and rerun the tests.
> >
> > Cheers.
> >
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-12-13 17:59 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-15 22:17 [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 01/12] pseries: Always use core objects for CPU construction David Gibson
2016-11-18 15:00 ` Greg Kurz
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 02/12] pseries: Make cpu_update during CAS unconditional David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 03/12] ppc: Clean up and QOMify hypercall emulation David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 04/12] ppc: Rename cpu_version to compat_pvr David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 05/12] ppc: Rewrite ppc_set_compat() David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 06/12] ppc: Rewrite ppc_get_compat_smt_threads() David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 07/12] ppc: Validate compatibility modes when setting David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 08/12] pseries: Rewrite CAS PVR compatibility logic David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 09/12] ppc: Add ppc_set_compat_all() David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 10/12] pseries: Move CPU compatibility property to machine David Gibson
2016-11-19 8:27 ` Greg Kurz
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 11/12] pseries: Reset CPU compatibility mode David Gibson
2016-11-15 22:17 ` [Qemu-devel] [RFCv2 12/12] ppc: Rework CPU compatibility testing across migration David Gibson
2016-12-02 14:48 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2016-12-05 4:09 ` David Gibson
2016-12-13 17:58 ` Greg Kurz
2016-11-15 22:44 ` [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling no-reply
2016-11-26 0:33 ` Greg Kurz
2016-11-28 4:23 ` David Gibson
2016-11-28 4:25 ` David Gibson
2016-12-01 13:16 ` Greg Kurz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.