* [Qemu-devel] [PULL 01/22] target/ppc: Yet another fix for KVM-HV HPTE accessors
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 02/22] spapr: Capabilities infrastructure David Gibson
` (20 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Alexey Kardashevskiy, David Gibson
From: Alexey Kardashevskiy <aik@ozlabs.ru>
As stated in the 1ad9f0a464fe commit log, the returned entries are not
a whole PTEG. It was not a problem before 1ad9f0a464fe as it would read
a single record assuming it contains a whole PTEG but now the code tries
reading the entire PTEG and "if ((n - i) < invalid)" produces negative
values which then are converted to size_t for memset() and that throws
seg fault.
This fixes the math.
While here, fix the last @i increment as well.
Fixes: 1ad9f0a464fe "target/ppc: Fix KVM-HV HPTE accessors"
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/kvm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 4664a3ce9d..09fcb082d2 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2667,21 +2667,24 @@ void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)
hdr = (struct kvm_get_htab_header *)buf;
while ((i < n) && ((char *)hdr < (buf + rc))) {
- int invalid = hdr->n_invalid;
+ int invalid = hdr->n_invalid, valid = hdr->n_valid;
if (hdr->index != (ptex + i)) {
hw_error("kvmppc_read_hptes: Unexpected HPTE index %"PRIu32
" != (%"HWADDR_PRIu" + %d", hdr->index, ptex, i);
}
- memcpy(hptes + i, hdr + 1, HASH_PTE_SIZE_64 * hdr->n_valid);
- i += hdr->n_valid;
+ if (n - i < valid) {
+ valid = n - i;
+ }
+ memcpy(hptes + i, hdr + 1, HASH_PTE_SIZE_64 * valid);
+ i += valid;
if ((n - i) < invalid) {
invalid = n - i;
}
memset(hptes + i, 0, invalid * HASH_PTE_SIZE_64);
- i += hdr->n_invalid;
+ i += invalid;
hdr = (struct kvm_get_htab_header *)
((char *)(hdr + 1) + HASH_PTE_SIZE_64 * hdr->n_valid);
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 02/22] spapr: Capabilities infrastructure
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 01/22] target/ppc: Yet another fix for KVM-HV HPTE accessors David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 03/22] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability David Gibson
` (19 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
Because PAPR is a paravirtual environment access to certain CPU (or other)
facilities can be blocked by the hypervisor. PAPR provides ways to
advertise in the device tree whether or not those features are available to
the guest.
In some places we automatically determine whether to make a feature
available based on whether our host can support it, in most cases this is
based on limitations in the available KVM implementation.
Although we correctly advertise this to the guest, it means that host
factors might make changes to the guest visible environment which is bad:
as well as generaly reducing reproducibility, it means that a migration
between different host environments can easily go bad.
We've mostly gotten away with it because the environments considered mature
enough to be well supported (basically, KVM on POWER8) have had consistent
feature availability. But, it's still not right and some limitations on
POWER9 is going to make it more of an issue in future.
This introduces an infrastructure for defining "sPAPR capabilities". These
are set by default based on the machine version, masked by the capabilities
of the chosen cpu, but can be overriden with machine properties.
The intention is at reset time we verify that the requested capabilities
can be supported on the host (considering TCG, KVM and/or host cpu
limitations). If not we simply fail, rather than silently modifying the
advertised featureset to the guest.
This does mean that certain configurations that "worked" may now fail, but
such configurations were already more subtly broken.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/Makefile.objs | 2 +-
hw/ppc/spapr.c | 7 ++
hw/ppc/spapr_caps.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/spapr.h | 31 +++++++++
4 files changed, 220 insertions(+), 1 deletion(-)
create mode 100644 hw/ppc/spapr_caps.c
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 7efc686748..1faff853b7 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -1,7 +1,7 @@
# shared objects
obj-y += ppc.o ppc_booke.o fdt.o
# IBM pSeries (sPAPR)
-obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
+obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.o spapr_vio.o spapr_events.o
obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index dfd352c473..a6cf1234d8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1466,6 +1466,8 @@ static void spapr_machine_reset(void)
/* Check for unknown sysbus devices */
foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
+ spapr_caps_reset(spapr);
+
first_ppc_cpu = POWERPC_CPU(first_cpu);
if (kvm_enabled() && kvmppc_has_cap_mmu_radix() &&
ppc_check_compat(first_ppc_cpu, CPU_POWERPC_LOGICAL_3_00, 0,
@@ -2311,6 +2313,8 @@ static void spapr_machine_init(MachineState *machine)
char *filename;
Error *resize_hpt_err = NULL;
+ spapr_caps_validate(spapr, &error_fatal);
+
msi_nonbroken = true;
QLIST_INIT(&spapr->phbs);
@@ -3819,6 +3823,9 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
* in which LMBs are represented and hot-added
*/
mc->numa_mem_align_shift = 28;
+
+ smc->default_caps = spapr_caps(0);
+ spapr_caps_add_properties(smc, &error_abort);
}
static const TypeInfo spapr_machine_info = {
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
new file mode 100644
index 0000000000..968ba7b857
--- /dev/null
+++ b/hw/ppc/spapr_caps.c
@@ -0,0 +1,181 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition capabilities handling
+ *
+ * Copyright (c) 2017 David Gibson, Red Hat Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+
+#include "hw/ppc/spapr.h"
+
+typedef struct sPAPRCapabilityInfo {
+ const char *name;
+ const char *description;
+ uint64_t flag;
+
+ /* Make sure the virtual hardware can support this capability */
+ void (*allow)(sPAPRMachineState *spapr, Error **errp);
+
+ /* If possible, tell the virtual hardware not to allow the cap to
+ * be used at all */
+ void (*disallow)(sPAPRMachineState *spapr, Error **errp);
+} sPAPRCapabilityInfo;
+
+static sPAPRCapabilityInfo capability_table[] = {
+};
+
+static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
+ CPUState *cs)
+{
+ sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+ sPAPRCapabilities caps;
+
+ caps = smc->default_caps;
+
+ /* TODO: clamp according to cpu model */
+
+ return caps;
+}
+
+void spapr_caps_reset(sPAPRMachineState *spapr)
+{
+ Error *local_err = NULL;
+ sPAPRCapabilities caps;
+ int i;
+
+ /* First compute the actual set of caps we're running with.. */
+ caps = default_caps_with_cpu(spapr, first_cpu);
+
+ caps.mask |= spapr->forced_caps.mask;
+ caps.mask &= ~spapr->forbidden_caps.mask;
+
+ spapr->effective_caps = caps;
+
+ /* .. then apply those caps to the virtual hardware */
+
+ for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+ sPAPRCapabilityInfo *info = &capability_table[i];
+
+ if (spapr->effective_caps.mask & info->flag) {
+ /* Failure to allow a cap is fatal - if the guest doesn't
+ * have it, we'll be supplying an incorrect environment */
+ if (info->allow) {
+ info->allow(spapr, &error_fatal);
+ }
+ } else {
+ /* Failure to enforce a cap is only a warning. The guest
+ * shouldn't be using it, since it's not advertised, so it
+ * doesn't get to complain about weird behaviour if it
+ * goes ahead anyway */
+ if (info->disallow) {
+ info->disallow(spapr, &local_err);
+ }
+ if (local_err) {
+ warn_report_err(local_err);
+ local_err = NULL;
+ }
+ }
+ }
+}
+
+static void spapr_cap_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ sPAPRCapabilityInfo *cap = opaque;
+ sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+ bool value = spapr_has_cap(spapr, cap->flag);
+
+ /* TODO: Could this get called before effective_caps is finalized
+ * in spapr_caps_reset()? */
+
+ visit_type_bool(v, name, &value, errp);
+}
+
+static void spapr_cap_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ sPAPRCapabilityInfo *cap = opaque;
+ sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+ bool value;
+ Error *local_err = NULL;
+
+ visit_type_bool(v, name, &value, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ if (value) {
+ spapr->forced_caps.mask |= cap->flag;
+ } else {
+ spapr->forbidden_caps.mask |= cap->flag;
+ }
+}
+
+void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp)
+{
+ uint64_t allcaps = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+ g_assert((allcaps & capability_table[i].flag) == 0);
+ allcaps |= capability_table[i].flag;
+ }
+
+ g_assert((spapr->forced_caps.mask & ~allcaps) == 0);
+ g_assert((spapr->forbidden_caps.mask & ~allcaps) == 0);
+
+ if (spapr->forced_caps.mask & spapr->forbidden_caps.mask) {
+ error_setg(errp, "Some sPAPR capabilities set both on and off");
+ return;
+ }
+
+ /* Check for any caps incompatible with other caps. Nothing to do
+ * yet */
+}
+
+void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp)
+{
+ Error *local_err = NULL;
+ ObjectClass *klass = OBJECT_CLASS(smc);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+ sPAPRCapabilityInfo *cap = &capability_table[i];
+ const char *name = g_strdup_printf("cap-%s", cap->name);
+
+ object_class_property_add(klass, name, "bool",
+ spapr_cap_get, spapr_cap_set, NULL,
+ cap, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ object_class_property_set_description(klass, name, cap->description,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ }
+}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 14757b805e..5569caf1d4 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -50,6 +50,15 @@ typedef enum {
SPAPR_RESIZE_HPT_REQUIRED,
} sPAPRResizeHPT;
+/**
+ * Capabilities
+ */
+
+typedef struct sPAPRCapabilities sPAPRCapabilities;
+struct sPAPRCapabilities {
+ uint64_t mask;
+};
+
/**
* sPAPRMachineClass:
*/
@@ -66,6 +75,7 @@ struct sPAPRMachineClass {
hwaddr *mmio32, hwaddr *mmio64,
unsigned n_dma, uint32_t *liobns, Error **errp);
sPAPRResizeHPT resize_hpt_default;
+ sPAPRCapabilities default_caps;
};
/**
@@ -127,6 +137,9 @@ struct sPAPRMachineState {
MemoryHotplugState hotplug_memory;
const char *icp_type;
+
+ sPAPRCapabilities forced_caps, forbidden_caps;
+ sPAPRCapabilities effective_caps;
};
#define H_SUCCESS 0
@@ -724,4 +737,22 @@ int spapr_irq_alloc_block(sPAPRMachineState *spapr, int num, bool lsi,
void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
+/*
+ * Handling of optional capabilities
+ */
+static inline sPAPRCapabilities spapr_caps(uint64_t mask)
+{
+ sPAPRCapabilities caps = { mask };
+ return caps;
+}
+
+static inline bool spapr_has_cap(sPAPRMachineState *spapr, uint64_t cap)
+{
+ return !!(spapr->effective_caps.mask & cap);
+}
+
+void spapr_caps_reset(sPAPRMachineState *spapr);
+void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp);
+void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp);
+
#endif /* HW_SPAPR_H */
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 03/22] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 01/22] target/ppc: Yet another fix for KVM-HV HPTE accessors David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 02/22] spapr: Capabilities infrastructure David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 04/22] spapr: Validate capabilities on migration David Gibson
` (18 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
This adds an spapr capability bit for Hardware Transactional Memory. It is
enabled by default for pseries-2.11 and earlier machine types. with POWER8
or later CPUs (as it must be, since earlier qemu versions would implicitly
allow it). However it is disabled by default for the latest pseries-2.12
machine type.
This means that with the latest machine type, HTM will not be available,
regardless of CPU, unless it is explicitly enabled on the command line.
That change is made on the basis that:
* This way running with -M pseries,accel=tcg will start with whatever cpu
and will provide the same guest visible model as with accel=kvm.
- More specifically, this means existing make check tests don't have
to be modified to use cap-htm=off in order to run with TCG
* We hope to add a new "HTM without suspend" feature in the not too
distant future which could work on both POWER8 and POWER9 cpus, and
could be enabled by default.
* Best guesses suggest that future POWER cpus may well only support the
HTM-without-suspend model, not the (frankly, horribly overcomplicated)
POWER8 style HTM with suspend.
* Anecdotal evidence suggests problems with HTM being enabled when it
wasn't wanted are more common than being missing when it was.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 15 ++++++++++-----
hw/ppc/spapr_caps.c | 29 ++++++++++++++++++++++++++++-
include/hw/ppc/spapr.h | 3 +++
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a6cf1234d8..73310bd3ee 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -253,7 +253,9 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
}
/* Populate the "ibm,pa-features" property */
-static void spapr_populate_pa_features(PowerPCCPU *cpu, void *fdt, int offset,
+static void spapr_populate_pa_features(sPAPRMachineState *spapr,
+ PowerPCCPU *cpu,
+ void *fdt, int offset,
bool legacy_guest)
{
CPUPPCState *env = &cpu->env;
@@ -318,7 +320,7 @@ static void spapr_populate_pa_features(PowerPCCPU *cpu, void *fdt, int offset,
*/
pa_features[3] |= 0x20;
}
- if (kvmppc_has_cap_htm() && pa_size > 24) {
+ if (spapr_has_cap(spapr, SPAPR_CAP_HTM) && pa_size > 24) {
pa_features[24] |= 0x80; /* Transactional memory support */
}
if (legacy_guest && pa_size > 40) {
@@ -384,8 +386,8 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
return ret;
}
- spapr_populate_pa_features(cpu, fdt, offset,
- spapr->cas_legacy_guest_workaround);
+ spapr_populate_pa_features(spapr, cpu, fdt, offset,
+ spapr->cas_legacy_guest_workaround);
}
return ret;
}
@@ -579,7 +581,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
page_sizes_prop, page_sizes_prop_size)));
}
- spapr_populate_pa_features(cpu, fdt, offset, false);
+ spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
_FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
cs->cpu_index / vcpus_per_socket)));
@@ -3903,7 +3905,10 @@ static void spapr_machine_2_11_instance_options(MachineState *machine)
static void spapr_machine_2_11_class_options(MachineClass *mc)
{
+ sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
spapr_machine_2_12_class_options(mc);
+ smc->default_caps = spapr_caps(SPAPR_CAP_HTM);
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
}
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 968ba7b857..3b35b91a5b 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -24,6 +24,10 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
+#include "sysemu/hw_accel.h"
+#include "target/ppc/cpu.h"
+#include "cpu-models.h"
+#include "kvm_ppc.h"
#include "hw/ppc/spapr.h"
@@ -40,18 +44,41 @@ typedef struct sPAPRCapabilityInfo {
void (*disallow)(sPAPRMachineState *spapr, Error **errp);
} sPAPRCapabilityInfo;
+static void cap_htm_allow(sPAPRMachineState *spapr, Error **errp)
+{
+ if (tcg_enabled()) {
+ error_setg(errp,
+ "No Transactional Memory support in TCG, try cap-htm=off");
+ } else if (kvm_enabled() && !kvmppc_has_cap_htm()) {
+ error_setg(errp,
+"KVM implementation does not support Transactional Memory, try cap-htm=off"
+ );
+ }
+}
+
static sPAPRCapabilityInfo capability_table[] = {
+ {
+ .name = "htm",
+ .description = "Allow Hardware Transactional Memory (HTM)",
+ .flag = SPAPR_CAP_HTM,
+ .allow = cap_htm_allow,
+ /* TODO: add cap_htm_disallow */
+ },
};
static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
CPUState *cs)
{
sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
sPAPRCapabilities caps;
caps = smc->default_caps;
- /* TODO: clamp according to cpu model */
+ if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
+ 0, spapr->max_compat_pvr)) {
+ caps.mask &= ~SPAPR_CAP_HTM;
+ }
return caps;
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5569caf1d4..dc64f4ebcb 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -54,6 +54,9 @@ typedef enum {
* Capabilities
*/
+/* Hardware Transactional Memory */
+#define SPAPR_CAP_HTM 0x0000000000000001ULL
+
typedef struct sPAPRCapabilities sPAPRCapabilities;
struct sPAPRCapabilities {
uint64_t mask;
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 04/22] spapr: Validate capabilities on migration
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (2 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 03/22] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 05/22] target/ppc: Clean up probing of VMX, VSX and DFP availability on KVM David Gibson
` (17 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
Now that the "pseries" machine type implements optional capabilities (well,
one so far) there's the possibility of having different capabilities
available at either end of a migration. Although arguably a user error,
it would be nice to catch this situation and fail as gracefully as we can.
This adds code to migrate the capabilities flags. These aren't pulled
directly into the destination's configuration since what the user has
specified on the destination command line should take precedence. However,
they are checked against the destination capabilities.
If the source was using a capability which is absent on the destination,
we fail the migration, since that could easily cause a guest crash or other
bad behaviour. If the source lacked a capability which is present on the
destination we warn, but allow the migration to proceed.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 6 ++++
hw/ppc/spapr_caps.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++--
include/hw/ppc/spapr.h | 6 ++++
3 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 73310bd3ee..3451d0806d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1589,6 +1589,11 @@ static int spapr_post_load(void *opaque, int version_id)
sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
int err = 0;
+ err = spapr_caps_post_migration(spapr);
+ if (err) {
+ return err;
+ }
+
if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
CPUState *cs;
CPU_FOREACH(cs) {
@@ -1755,6 +1760,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_ov5_cas,
&vmstate_spapr_patb_entry,
&vmstate_spapr_pending_events,
+ &vmstate_spapr_caps,
NULL
}
};
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 3b35b91a5b..cad40fe49a 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
#include "sysemu/hw_accel.h"
@@ -83,6 +84,93 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
return caps;
}
+static bool spapr_caps_needed(void *opaque)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ return (spapr->forced_caps.mask != 0) || (spapr->forbidden_caps.mask != 0);
+}
+
+/* This has to be called from the top-level spapr post_load, not the
+ * caps specific one. Otherwise it wouldn't be called when the source
+ * caps are all defaults, which could still conflict with overridden
+ * caps on the destination */
+int spapr_caps_post_migration(sPAPRMachineState *spapr)
+{
+ uint64_t allcaps = 0;
+ int i;
+ bool ok = true;
+ sPAPRCapabilities dstcaps = spapr->effective_caps;
+ sPAPRCapabilities srccaps;
+
+ srccaps = default_caps_with_cpu(spapr, first_cpu);
+ srccaps.mask |= spapr->mig_forced_caps.mask;
+ srccaps.mask &= ~spapr->mig_forbidden_caps.mask;
+
+ for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+ sPAPRCapabilityInfo *info = &capability_table[i];
+
+ allcaps |= info->flag;
+
+ if ((srccaps.mask & info->flag) && !(dstcaps.mask & info->flag)) {
+ error_report("cap-%s=on in incoming stream, but off in destination",
+ info->name);
+ ok = false;
+ }
+
+ if (!(srccaps.mask & info->flag) && (dstcaps.mask & info->flag)) {
+ warn_report("cap-%s=off in incoming stream, but on in destination",
+ info->name);
+ }
+ }
+
+ if (spapr->mig_forced_caps.mask & ~allcaps) {
+ error_report(
+ "Unknown capabilities 0x%"PRIx64" enabled in incoming stream",
+ spapr->mig_forced_caps.mask & ~allcaps);
+ ok = false;
+ }
+ if (spapr->mig_forbidden_caps.mask & ~allcaps) {
+ warn_report(
+ "Unknown capabilities 0x%"PRIx64" disabled in incoming stream",
+ spapr->mig_forbidden_caps.mask & ~allcaps);
+ }
+
+ return ok ? 0 : -EINVAL;
+}
+
+static int spapr_caps_pre_save(void *opaque)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ spapr->mig_forced_caps = spapr->forced_caps;
+ spapr->mig_forbidden_caps = spapr->forbidden_caps;
+ return 0;
+}
+
+static int spapr_caps_pre_load(void *opaque)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ spapr->mig_forced_caps = spapr_caps(0);
+ spapr->mig_forbidden_caps = spapr_caps(0);
+ return 0;
+}
+
+const VMStateDescription vmstate_spapr_caps = {
+ .name = "spapr/caps",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = spapr_caps_needed,
+ .pre_save = spapr_caps_pre_save,
+ .pre_load = spapr_caps_pre_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(mig_forced_caps.mask, sPAPRMachineState),
+ VMSTATE_UINT64(mig_forbidden_caps.mask, sPAPRMachineState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
void spapr_caps_reset(sPAPRMachineState *spapr)
{
Error *local_err = NULL;
@@ -92,6 +180,11 @@ void spapr_caps_reset(sPAPRMachineState *spapr)
/* First compute the actual set of caps we're running with.. */
caps = default_caps_with_cpu(spapr, first_cpu);
+ /* Remove unnecessary forced/forbidden bits (this will help us
+ * with migration) */
+ spapr->forced_caps.mask &= ~caps.mask;
+ spapr->forbidden_caps.mask &= caps.mask;
+
caps.mask |= spapr->forced_caps.mask;
caps.mask &= ~spapr->forbidden_caps.mask;
@@ -175,9 +268,6 @@ void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp)
error_setg(errp, "Some sPAPR capabilities set both on and off");
return;
}
-
- /* Check for any caps incompatible with other caps. Nothing to do
- * yet */
}
void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index dc64f4ebcb..5c85f39c3b 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -54,6 +54,8 @@ typedef enum {
* Capabilities
*/
+/* These bits go in the migration stream, so they can't be reassigned */
+
/* Hardware Transactional Memory */
#define SPAPR_CAP_HTM 0x0000000000000001ULL
@@ -142,6 +144,7 @@ struct sPAPRMachineState {
const char *icp_type;
sPAPRCapabilities forced_caps, forbidden_caps;
+ sPAPRCapabilities mig_forced_caps, mig_forbidden_caps;
sPAPRCapabilities effective_caps;
};
@@ -743,6 +746,8 @@ qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
/*
* Handling of optional capabilities
*/
+extern const VMStateDescription vmstate_spapr_caps;
+
static inline sPAPRCapabilities spapr_caps(uint64_t mask)
{
sPAPRCapabilities caps = { mask };
@@ -757,5 +762,6 @@ static inline bool spapr_has_cap(sPAPRMachineState *spapr, uint64_t cap)
void spapr_caps_reset(sPAPRMachineState *spapr);
void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp);
void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp);
+int spapr_caps_post_migration(sPAPRMachineState *spapr);
#endif /* HW_SPAPR_H */
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 05/22] target/ppc: Clean up probing of VMX, VSX and DFP availability on KVM
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (3 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 04/22] spapr: Validate capabilities on migration David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 06/22] spapr: Handle VMX/VSX presence as an spapr capability flag David Gibson
` (16 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
When constructing the "host" cpu class we modify whether the VMX and VSX
vector extensions and DFP (Decimal Floating Point) are available
based on whether KVM can support those instructions. This can depend on
policy in the host kernel as well as on the actual host cpu capabilities.
However, the way we probe for this is not very nice: we explicitly check
the host's device tree. That works in practice, but it's not really
correct, since the device tree is a property of the host kernel's platform
which we don't really know about. We get away with it because the only
modern POWER platforms happen to encode VMX, VSX and DFP availability in
the device tree in the same way.
Arguably we should have an explicit KVM capability for this, but we haven't
needed one so far. Barring specific KVM policies which don't yet exist,
each of these instruction classes will be available in the guest if and
only if they're available in the qemu userspace process. We can determine
that from the ELF AUX vector we're supplied with.
Once reworked like this, there are no more callers for kvmppc_get_vmx() and
kvmppc_get_dfp() so remove them.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
target/ppc/kvm.c | 27 ++++++---------------------
target/ppc/kvm_ppc.h | 2 --
2 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 09fcb082d2..914be687e7 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2011,16 +2011,6 @@ uint64_t kvmppc_get_clockfreq(void)
return kvmppc_read_int_cpu_dt("clock-frequency");
}
-uint32_t kvmppc_get_vmx(void)
-{
- return kvmppc_read_int_cpu_dt("ibm,vmx");
-}
-
-uint32_t kvmppc_get_dfp(void)
-{
- return kvmppc_read_int_cpu_dt("ibm,dfp");
-}
-
static int kvmppc_get_pvinfo(CPUPPCState *env, struct kvm_ppc_pvinfo *pvinfo)
{
PowerPCCPU *cpu = ppc_env_get_cpu(env);
@@ -2404,23 +2394,18 @@ static void alter_insns(uint64_t *word, uint64_t flags, bool on)
static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
{
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
- uint32_t vmx = kvmppc_get_vmx();
- uint32_t dfp = kvmppc_get_dfp();
uint32_t dcache_size = kvmppc_read_int_cpu_dt("d-cache-size");
uint32_t icache_size = kvmppc_read_int_cpu_dt("i-cache-size");
/* Now fix up the class with information we can query from the host */
pcc->pvr = mfpvr();
- if (vmx != -1) {
- /* Only override when we know what the host supports */
- alter_insns(&pcc->insns_flags, PPC_ALTIVEC, vmx > 0);
- alter_insns(&pcc->insns_flags2, PPC2_VSX, vmx > 1);
- }
- if (dfp != -1) {
- /* Only override when we know what the host supports */
- alter_insns(&pcc->insns_flags2, PPC2_DFP, dfp);
- }
+ alter_insns(&pcc->insns_flags, PPC_ALTIVEC,
+ qemu_getauxval(AT_HWCAP) & PPC_FEATURE_HAS_ALTIVEC);
+ alter_insns(&pcc->insns_flags2, PPC2_VSX,
+ qemu_getauxval(AT_HWCAP) & PPC_FEATURE_HAS_VSX);
+ alter_insns(&pcc->insns_flags2, PPC2_DFP,
+ qemu_getauxval(AT_HWCAP) & PPC_FEATURE_HAS_DFP);
if (dcache_size != -1) {
pcc->l1_dcache_size = dcache_size;
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index d6be38ecaf..ecb55493cc 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -15,8 +15,6 @@
uint32_t kvmppc_get_tbfreq(void);
uint64_t kvmppc_get_clockfreq(void);
-uint32_t kvmppc_get_vmx(void);
-uint32_t kvmppc_get_dfp(void);
bool kvmppc_get_host_model(char **buf);
bool kvmppc_get_host_serial(char **buf);
int kvmppc_get_hasidle(CPUPPCState *env);
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 06/22] spapr: Handle VMX/VSX presence as an spapr capability flag
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (4 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 05/22] target/ppc: Clean up probing of VMX, VSX and DFP availability on KVM David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 07/22] spapr: Handle Decimal Floating Point (DFP) as an optional capability David Gibson
` (15 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
We currently have some conditionals in the spapr device tree code to decide
whether or not to advertise the availability of the VMX (aka Altivec) and
VSX vector extensions to the guest, based on whether the guest cpu has
those features.
This can lead to confusion and subtle failures on migration, since it makes
a guest visible change based only on host capabilities. We now have a
better mechanism for this, in spapr capabilities flags, which explicitly
depend on user options rather than host capabilities.
Rework the advertisement of VSX and VMX based on a new VSX capability. We
no longer bother with a conditional for VMX support, because every CPU
that's ever been supported by the pseries machine type supports VMX.
NOTE: Some userspace distributions (e.g. RHEL7.4) already rely on
availability of VSX in libc, so using cap-vsx=off may lead to a fatal
SIGILL in init.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 20 +++++++++++---------
hw/ppc/spapr_caps.c | 25 +++++++++++++++++++++++++
include/hw/ppc/spapr.h | 3 +++
3 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3451d0806d..eca9f11c91 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -557,14 +557,16 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
segs, sizeof(segs))));
}
- /* Advertise VMX/VSX (vector extensions) if available
- * 0 / no property == no vector extensions
+ /* Advertise VSX (vector extensions) if available
* 1 == VMX / Altivec available
- * 2 == VSX available */
- if (env->insns_flags & PPC_ALTIVEC) {
- uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
-
- _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
+ * 2 == VSX available
+ *
+ * Only CPUs for which we create core types in spapr_cpu_core.c
+ * are possible, and all of those have VMX */
+ if (spapr_has_cap(spapr, SPAPR_CAP_VSX)) {
+ _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 2)));
+ } else {
+ _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 1)));
}
/* Advertise DFP (Decimal Floating Point) if available
@@ -3832,7 +3834,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
*/
mc->numa_mem_align_shift = 28;
- smc->default_caps = spapr_caps(0);
+ smc->default_caps = spapr_caps(SPAPR_CAP_VSX);
spapr_caps_add_properties(smc, &error_abort);
}
@@ -3914,7 +3916,7 @@ static void spapr_machine_2_11_class_options(MachineClass *mc)
sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
spapr_machine_2_12_class_options(mc);
- smc->default_caps = spapr_caps(SPAPR_CAP_HTM);
+ smc->default_caps = spapr_caps(SPAPR_CAP_HTM | SPAPR_CAP_VSX);
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
}
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index cad40fe49a..7c855c67ad 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -57,6 +57,19 @@ static void cap_htm_allow(sPAPRMachineState *spapr, Error **errp)
}
}
+static void cap_vsx_allow(sPAPRMachineState *spapr, Error **errp)
+{
+ PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+ CPUPPCState *env = &cpu->env;
+
+ /* Allowable CPUs in spapr_cpu_core.c should already have gotten
+ * rid of anything that doesn't do VMX */
+ g_assert(env->insns_flags & PPC_ALTIVEC);
+ if (!(env->insns_flags2 & PPC2_VSX)) {
+ error_setg(errp, "VSX support not available, try cap-vsx=off");
+ }
+}
+
static sPAPRCapabilityInfo capability_table[] = {
{
.name = "htm",
@@ -65,6 +78,13 @@ static sPAPRCapabilityInfo capability_table[] = {
.allow = cap_htm_allow,
/* TODO: add cap_htm_disallow */
},
+ {
+ .name = "vsx",
+ .description = "Allow Vector Scalar Extensions (VSX)",
+ .flag = SPAPR_CAP_VSX,
+ .allow = cap_vsx_allow,
+ /* TODO: add cap_vsx_disallow */
+ },
};
static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
@@ -81,6 +101,11 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
caps.mask &= ~SPAPR_CAP_HTM;
}
+ if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
+ 0, spapr->max_compat_pvr)) {
+ caps.mask &= ~SPAPR_CAP_VSX;
+ }
+
return caps;
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5c85f39c3b..148a03d189 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -59,6 +59,9 @@ typedef enum {
/* Hardware Transactional Memory */
#define SPAPR_CAP_HTM 0x0000000000000001ULL
+/* Vector Scalar Extensions */
+#define SPAPR_CAP_VSX 0x0000000000000002ULL
+
typedef struct sPAPRCapabilities sPAPRCapabilities;
struct sPAPRCapabilities {
uint64_t mask;
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 07/22] spapr: Handle Decimal Floating Point (DFP) as an optional capability
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (5 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 06/22] spapr: Handle VMX/VSX presence as an spapr capability flag David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 08/22] hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation David Gibson
` (14 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
Decimal Floating Point has been available on POWER7 and later (server)
cpus. However, it can be disabled on the hypervisor, meaning that it's
not available to guests.
We currently handle this by conditionally advertising DFP support in the
device tree depending on whether the guest CPU model supports it - which
can also depend on what's allowed in the host for -cpu host. That can lead
to confusion on migration, since host properties are silently affecting
guest visible properties.
This patch handles it by treating it as an optional capability for the
pseries machine type.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 7 ++++---
hw/ppc/spapr_caps.c | 18 ++++++++++++++++++
include/hw/ppc/spapr.h | 3 +++
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index eca9f11c91..d1acfe8858 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -572,7 +572,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
/* Advertise DFP (Decimal Floating Point) if available
* 0 / no property == no DFP
* 1 == DFP available */
- if (env->insns_flags2 & PPC2_DFP) {
+ if (spapr_has_cap(spapr, SPAPR_CAP_DFP)) {
_FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
}
@@ -3834,7 +3834,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
*/
mc->numa_mem_align_shift = 28;
- smc->default_caps = spapr_caps(SPAPR_CAP_VSX);
+ smc->default_caps = spapr_caps(SPAPR_CAP_VSX | SPAPR_CAP_DFP);
spapr_caps_add_properties(smc, &error_abort);
}
@@ -3916,7 +3916,8 @@ static void spapr_machine_2_11_class_options(MachineClass *mc)
sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
spapr_machine_2_12_class_options(mc);
- smc->default_caps = spapr_caps(SPAPR_CAP_HTM | SPAPR_CAP_VSX);
+ smc->default_caps = spapr_caps(SPAPR_CAP_HTM | SPAPR_CAP_VSX
+ | SPAPR_CAP_DFP);
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
}
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 7c855c67ad..9d070a306c 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -70,6 +70,16 @@ static void cap_vsx_allow(sPAPRMachineState *spapr, Error **errp)
}
}
+static void cap_dfp_allow(sPAPRMachineState *spapr, Error **errp)
+{
+ PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+ CPUPPCState *env = &cpu->env;
+
+ if (!(env->insns_flags2 & PPC2_DFP)) {
+ error_setg(errp, "DFP support not available, try cap-dfp=off");
+ }
+}
+
static sPAPRCapabilityInfo capability_table[] = {
{
.name = "htm",
@@ -85,6 +95,13 @@ static sPAPRCapabilityInfo capability_table[] = {
.allow = cap_vsx_allow,
/* TODO: add cap_vsx_disallow */
},
+ {
+ .name = "dfp",
+ .description = "Allow Decimal Floating Point (DFP)",
+ .flag = SPAPR_CAP_DFP,
+ .allow = cap_dfp_allow,
+ /* TODO: add cap_dfp_disallow */
+ },
};
static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
@@ -104,6 +121,7 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
0, spapr->max_compat_pvr)) {
caps.mask &= ~SPAPR_CAP_VSX;
+ caps.mask &= ~SPAPR_CAP_DFP;
}
return caps;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 148a03d189..26ac17e641 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -62,6 +62,9 @@ typedef enum {
/* Vector Scalar Extensions */
#define SPAPR_CAP_VSX 0x0000000000000002ULL
+/* Decimal Floating Point */
+#define SPAPR_CAP_DFP 0x0000000000000004ULL
+
typedef struct sPAPRCapabilities sPAPRCapabilities;
struct sPAPRCapabilities {
uint64_t mask;
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 08/22] hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (6 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 07/22] spapr: Handle Decimal Floating Point (DFP) as an optional capability David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 09/22] spapr: Remove unnecessary 'options' field from sPAPRCapabilityInfo David Gibson
` (13 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Suraj Jitindar Singh, David Gibson
From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Currently spapr_caps are tied to boolean values (on or off). This patch
reworks the caps so that they can have any uint8 value. This allows more
capabilities with various values to be represented in the same way
internally. Capabilities are numbered in ascending order. The internal
representation of capability values is an array of uint8s in the
sPAPRMachineState, indexed by capability number.
Capabilities can have their own name, description, options, getter and
setter functions, type and allow functions. They also each have their own
section in the migration stream. Capabilities are only migrated if they
were explictly set on the command line, with the assumption that
otherwise the default will match.
On migration we ensure that the capability value on the destination
is greater than or equal to the capability value from the source. So
long at this remains the case then the migration is considered
compatible and allowed to continue.
This patch implements generic getter and setter functions for boolean
capabilities. It also converts the existings cap-htm, cap-vsx and
cap-dfp capabilities to this new format.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 45 +++++--
hw/ppc/spapr_caps.c | 322 +++++++++++++++++++++++++------------------------
include/hw/ppc/spapr.h | 45 +++----
3 files changed, 225 insertions(+), 187 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d1acfe8858..3e528fe91e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -320,7 +320,7 @@ static void spapr_populate_pa_features(sPAPRMachineState *spapr,
*/
pa_features[3] |= 0x20;
}
- if (spapr_has_cap(spapr, SPAPR_CAP_HTM) && pa_size > 24) {
+ if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) {
pa_features[24] |= 0x80; /* Transactional memory support */
}
if (legacy_guest && pa_size > 40) {
@@ -563,7 +563,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
*
* Only CPUs for which we create core types in spapr_cpu_core.c
* are possible, and all of those have VMX */
- if (spapr_has_cap(spapr, SPAPR_CAP_VSX)) {
+ if (spapr_get_cap(spapr, SPAPR_CAP_VSX) != 0) {
_FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 2)));
} else {
_FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 1)));
@@ -572,7 +572,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
/* Advertise DFP (Decimal Floating Point) if available
* 0 / no property == no DFP
* 1 == DFP available */
- if (spapr_has_cap(spapr, SPAPR_CAP_DFP)) {
+ if (spapr_get_cap(spapr, SPAPR_CAP_DFP) != 0) {
_FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
}
@@ -1586,6 +1586,18 @@ static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
}
}
+static int spapr_pre_load(void *opaque)
+{
+ int rc;
+
+ rc = spapr_caps_pre_load(opaque);
+ if (rc) {
+ return rc;
+ }
+
+ return 0;
+}
+
static int spapr_post_load(void *opaque, int version_id)
{
sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
@@ -1627,6 +1639,18 @@ static int spapr_post_load(void *opaque, int version_id)
return err;
}
+static int spapr_pre_save(void *opaque)
+{
+ int rc;
+
+ rc = spapr_caps_pre_save(opaque);
+ if (rc) {
+ return rc;
+ }
+
+ return 0;
+}
+
static bool version_before_3(void *opaque, int version_id)
{
return version_id < 3;
@@ -1747,7 +1771,9 @@ static const VMStateDescription vmstate_spapr = {
.name = "spapr",
.version_id = 3,
.minimum_version_id = 1,
+ .pre_load = spapr_pre_load,
.post_load = spapr_post_load,
+ .pre_save = spapr_pre_save,
.fields = (VMStateField[]) {
/* used to be @next_irq */
VMSTATE_UNUSED_BUFFER(version_before_3, 0, 4),
@@ -1762,7 +1788,9 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_ov5_cas,
&vmstate_spapr_patb_entry,
&vmstate_spapr_pending_events,
- &vmstate_spapr_caps,
+ &vmstate_spapr_cap_htm,
+ &vmstate_spapr_cap_vsx,
+ &vmstate_spapr_cap_dfp,
NULL
}
};
@@ -2323,8 +2351,6 @@ static void spapr_machine_init(MachineState *machine)
char *filename;
Error *resize_hpt_err = NULL;
- spapr_caps_validate(spapr, &error_fatal);
-
msi_nonbroken = true;
QLIST_INIT(&spapr->phbs);
@@ -3834,7 +3860,9 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
*/
mc->numa_mem_align_shift = 28;
- smc->default_caps = spapr_caps(SPAPR_CAP_VSX | SPAPR_CAP_DFP);
+ smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
+ smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
+ smc->default_caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_ON;
spapr_caps_add_properties(smc, &error_abort);
}
@@ -3916,8 +3944,7 @@ static void spapr_machine_2_11_class_options(MachineClass *mc)
sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
spapr_machine_2_12_class_options(mc);
- smc->default_caps = spapr_caps(SPAPR_CAP_HTM | SPAPR_CAP_VSX
- | SPAPR_CAP_DFP);
+ smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_ON;
SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11);
}
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 9d070a306c..f95a78547d 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -35,18 +35,51 @@
typedef struct sPAPRCapabilityInfo {
const char *name;
const char *description;
- uint64_t flag;
+ const char *options; /* valid capability values */
+ int index;
+ /* Getter and Setter Function Pointers */
+ ObjectPropertyAccessor *get;
+ ObjectPropertyAccessor *set;
+ const char *type;
/* Make sure the virtual hardware can support this capability */
- void (*allow)(sPAPRMachineState *spapr, Error **errp);
-
- /* If possible, tell the virtual hardware not to allow the cap to
- * be used at all */
- void (*disallow)(sPAPRMachineState *spapr, Error **errp);
+ void (*apply)(sPAPRMachineState *spapr, uint8_t val, Error **errp);
} sPAPRCapabilityInfo;
-static void cap_htm_allow(sPAPRMachineState *spapr, Error **errp)
+static void spapr_cap_get_bool(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ sPAPRCapabilityInfo *cap = opaque;
+ sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+ bool value = spapr_get_cap(spapr, cap->index) == SPAPR_CAP_ON;
+
+ visit_type_bool(v, name, &value, errp);
+}
+
+static void spapr_cap_set_bool(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
{
+ sPAPRCapabilityInfo *cap = opaque;
+ sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+ bool value;
+ Error *local_err = NULL;
+
+ visit_type_bool(v, name, &value, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ spapr->cmd_line_caps[cap->index] = true;
+ spapr->eff.caps[cap->index] = value ? SPAPR_CAP_ON : SPAPR_CAP_OFF;
+}
+
+static void cap_htm_apply(sPAPRMachineState *spapr, uint8_t val, Error **errp)
+{
+ if (!val) {
+ /* TODO: We don't support disabling htm yet */
+ return;
+ }
if (tcg_enabled()) {
error_setg(errp,
"No Transactional Memory support in TCG, try cap-htm=off");
@@ -57,11 +90,15 @@ static void cap_htm_allow(sPAPRMachineState *spapr, Error **errp)
}
}
-static void cap_vsx_allow(sPAPRMachineState *spapr, Error **errp)
+static void cap_vsx_apply(sPAPRMachineState *spapr, uint8_t val, Error **errp)
{
PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
CPUPPCState *env = &cpu->env;
+ if (!val) {
+ /* TODO: We don't support disabling vsx yet */
+ return;
+ }
/* Allowable CPUs in spapr_cpu_core.c should already have gotten
* rid of anything that doesn't do VMX */
g_assert(env->insns_flags & PPC_ALTIVEC);
@@ -70,37 +107,51 @@ static void cap_vsx_allow(sPAPRMachineState *spapr, Error **errp)
}
}
-static void cap_dfp_allow(sPAPRMachineState *spapr, Error **errp)
+static void cap_dfp_apply(sPAPRMachineState *spapr, uint8_t val, Error **errp)
{
PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
CPUPPCState *env = &cpu->env;
+ if (!val) {
+ /* TODO: We don't support disabling dfp yet */
+ return;
+ }
if (!(env->insns_flags2 & PPC2_DFP)) {
error_setg(errp, "DFP support not available, try cap-dfp=off");
}
}
-static sPAPRCapabilityInfo capability_table[] = {
- {
+
+sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
+ [SPAPR_CAP_HTM] = {
.name = "htm",
.description = "Allow Hardware Transactional Memory (HTM)",
- .flag = SPAPR_CAP_HTM,
- .allow = cap_htm_allow,
- /* TODO: add cap_htm_disallow */
+ .options = "",
+ .index = SPAPR_CAP_HTM,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_htm_apply,
},
- {
+ [SPAPR_CAP_VSX] = {
.name = "vsx",
.description = "Allow Vector Scalar Extensions (VSX)",
- .flag = SPAPR_CAP_VSX,
- .allow = cap_vsx_allow,
- /* TODO: add cap_vsx_disallow */
+ .options = "",
+ .index = SPAPR_CAP_VSX,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_vsx_apply,
},
- {
+ [SPAPR_CAP_DFP] = {
.name = "dfp",
.description = "Allow Decimal Floating Point (DFP)",
- .flag = SPAPR_CAP_DFP,
- .allow = cap_dfp_allow,
- /* TODO: add cap_dfp_disallow */
+ .options = "",
+ .index = SPAPR_CAP_DFP,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_dfp_apply,
},
};
@@ -115,23 +166,33 @@ static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
0, spapr->max_compat_pvr)) {
- caps.mask &= ~SPAPR_CAP_HTM;
+ caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
}
if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
0, spapr->max_compat_pvr)) {
- caps.mask &= ~SPAPR_CAP_VSX;
- caps.mask &= ~SPAPR_CAP_DFP;
+ caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_OFF;
+ caps.caps[SPAPR_CAP_DFP] = SPAPR_CAP_OFF;
}
return caps;
}
-static bool spapr_caps_needed(void *opaque)
+int spapr_caps_pre_load(void *opaque)
{
sPAPRMachineState *spapr = opaque;
- return (spapr->forced_caps.mask != 0) || (spapr->forbidden_caps.mask != 0);
+ /* Set to default so we can tell if this came in with the migration */
+ spapr->mig = spapr->def;
+ return 0;
+}
+
+int spapr_caps_pre_save(void *opaque)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ spapr->mig = spapr->eff;
+ return 0;
}
/* This has to be called from the top-level spapr post_load, not the
@@ -140,176 +201,121 @@ static bool spapr_caps_needed(void *opaque)
* caps on the destination */
int spapr_caps_post_migration(sPAPRMachineState *spapr)
{
- uint64_t allcaps = 0;
int i;
bool ok = true;
- sPAPRCapabilities dstcaps = spapr->effective_caps;
+ sPAPRCapabilities dstcaps = spapr->eff;
sPAPRCapabilities srccaps;
srccaps = default_caps_with_cpu(spapr, first_cpu);
- srccaps.mask |= spapr->mig_forced_caps.mask;
- srccaps.mask &= ~spapr->mig_forbidden_caps.mask;
+ for (i = 0; i < SPAPR_CAP_NUM; i++) {
+ /* If not default value then assume came in with the migration */
+ if (spapr->mig.caps[i] != spapr->def.caps[i]) {
+ srccaps.caps[i] = spapr->mig.caps[i];
+ }
+ }
- for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
+ for (i = 0; i < SPAPR_CAP_NUM; i++) {
sPAPRCapabilityInfo *info = &capability_table[i];
- allcaps |= info->flag;
-
- if ((srccaps.mask & info->flag) && !(dstcaps.mask & info->flag)) {
- error_report("cap-%s=on in incoming stream, but off in destination",
- info->name);
+ if (srccaps.caps[i] > dstcaps.caps[i]) {
+ error_report("cap-%s higher level (%d) in incoming stream than on destination (%d)",
+ info->name, srccaps.caps[i], dstcaps.caps[i]);
ok = false;
}
- if (!(srccaps.mask & info->flag) && (dstcaps.mask & info->flag)) {
- warn_report("cap-%s=off in incoming stream, but on in destination",
- info->name);
+ if (srccaps.caps[i] < dstcaps.caps[i]) {
+ warn_report("cap-%s lower level (%d) in incoming stream than on destination (%d)",
+ info->name, srccaps.caps[i], dstcaps.caps[i]);
}
}
- if (spapr->mig_forced_caps.mask & ~allcaps) {
- error_report(
- "Unknown capabilities 0x%"PRIx64" enabled in incoming stream",
- spapr->mig_forced_caps.mask & ~allcaps);
- ok = false;
- }
- if (spapr->mig_forbidden_caps.mask & ~allcaps) {
- warn_report(
- "Unknown capabilities 0x%"PRIx64" disabled in incoming stream",
- spapr->mig_forbidden_caps.mask & ~allcaps);
- }
-
return ok ? 0 : -EINVAL;
}
-static int spapr_caps_pre_save(void *opaque)
+static bool spapr_cap_htm_needed(void *opaque)
{
sPAPRMachineState *spapr = opaque;
- spapr->mig_forced_caps = spapr->forced_caps;
- spapr->mig_forbidden_caps = spapr->forbidden_caps;
- return 0;
+ return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
+ (spapr->eff.caps[SPAPR_CAP_HTM] != spapr->def.caps[SPAPR_CAP_HTM]);
}
-static int spapr_caps_pre_load(void *opaque)
+const VMStateDescription vmstate_spapr_cap_htm = {
+ .name = "spapr/cap/htm",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = spapr_cap_htm_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static bool spapr_cap_vsx_needed(void *opaque)
{
sPAPRMachineState *spapr = opaque;
- spapr->mig_forced_caps = spapr_caps(0);
- spapr->mig_forbidden_caps = spapr_caps(0);
- return 0;
+ return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
+ (spapr->eff.caps[SPAPR_CAP_VSX] != spapr->def.caps[SPAPR_CAP_VSX]);
}
-const VMStateDescription vmstate_spapr_caps = {
- .name = "spapr/caps",
+const VMStateDescription vmstate_spapr_cap_vsx = {
+ .name = "spapr/cap/vsx",
.version_id = 1,
.minimum_version_id = 1,
- .needed = spapr_caps_needed,
- .pre_save = spapr_caps_pre_save,
- .pre_load = spapr_caps_pre_load,
+ .needed = spapr_cap_vsx_needed,
.fields = (VMStateField[]) {
- VMSTATE_UINT64(mig_forced_caps.mask, sPAPRMachineState),
- VMSTATE_UINT64(mig_forbidden_caps.mask, sPAPRMachineState),
+ VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
VMSTATE_END_OF_LIST()
},
};
-void spapr_caps_reset(sPAPRMachineState *spapr)
+static bool spapr_cap_dfp_needed(void *opaque)
{
- Error *local_err = NULL;
- sPAPRCapabilities caps;
- int i;
-
- /* First compute the actual set of caps we're running with.. */
- caps = default_caps_with_cpu(spapr, first_cpu);
-
- /* Remove unnecessary forced/forbidden bits (this will help us
- * with migration) */
- spapr->forced_caps.mask &= ~caps.mask;
- spapr->forbidden_caps.mask &= caps.mask;
-
- caps.mask |= spapr->forced_caps.mask;
- caps.mask &= ~spapr->forbidden_caps.mask;
-
- spapr->effective_caps = caps;
-
- /* .. then apply those caps to the virtual hardware */
-
- for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
- sPAPRCapabilityInfo *info = &capability_table[i];
-
- if (spapr->effective_caps.mask & info->flag) {
- /* Failure to allow a cap is fatal - if the guest doesn't
- * have it, we'll be supplying an incorrect environment */
- if (info->allow) {
- info->allow(spapr, &error_fatal);
- }
- } else {
- /* Failure to enforce a cap is only a warning. The guest
- * shouldn't be using it, since it's not advertised, so it
- * doesn't get to complain about weird behaviour if it
- * goes ahead anyway */
- if (info->disallow) {
- info->disallow(spapr, &local_err);
- }
- if (local_err) {
- warn_report_err(local_err);
- local_err = NULL;
- }
- }
- }
-}
-
-static void spapr_cap_get(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- sPAPRCapabilityInfo *cap = opaque;
- sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
- bool value = spapr_has_cap(spapr, cap->flag);
-
- /* TODO: Could this get called before effective_caps is finalized
- * in spapr_caps_reset()? */
+ sPAPRMachineState *spapr = opaque;
- visit_type_bool(v, name, &value, errp);
+ return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
+ (spapr->eff.caps[SPAPR_CAP_DFP] != spapr->def.caps[SPAPR_CAP_DFP]);
}
-static void spapr_cap_set(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- sPAPRCapabilityInfo *cap = opaque;
- sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
- bool value;
- Error *local_err = NULL;
-
- visit_type_bool(v, name, &value, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
- if (value) {
- spapr->forced_caps.mask |= cap->flag;
- } else {
- spapr->forbidden_caps.mask |= cap->flag;
- }
-}
+const VMStateDescription vmstate_spapr_cap_dfp = {
+ .name = "spapr/cap/dfp",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = spapr_cap_dfp_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
+ VMSTATE_END_OF_LIST()
+ },
+};
-void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp)
+void spapr_caps_reset(sPAPRMachineState *spapr)
{
- uint64_t allcaps = 0;
+ sPAPRCapabilities default_caps;
int i;
- for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
- g_assert((allcaps & capability_table[i].flag) == 0);
- allcaps |= capability_table[i].flag;
+ /* First compute the actual set of caps we're running with.. */
+ default_caps = default_caps_with_cpu(spapr, first_cpu);
+
+ for (i = 0; i < SPAPR_CAP_NUM; i++) {
+ /* Store the defaults */
+ spapr->def.caps[i] = default_caps.caps[i];
+ /* If not set on the command line then apply the default value */
+ if (!spapr->cmd_line_caps[i]) {
+ spapr->eff.caps[i] = default_caps.caps[i];
+ }
}
- g_assert((spapr->forced_caps.mask & ~allcaps) == 0);
- g_assert((spapr->forbidden_caps.mask & ~allcaps) == 0);
+ /* .. then apply those caps to the virtual hardware */
+
+ for (i = 0; i < SPAPR_CAP_NUM; i++) {
+ sPAPRCapabilityInfo *info = &capability_table[i];
- if (spapr->forced_caps.mask & spapr->forbidden_caps.mask) {
- error_setg(errp, "Some sPAPR capabilities set both on and off");
- return;
+ /*
+ * If the apply function can't set the desired level and thinks it's
+ * fatal, it should cause that.
+ */
+ info->apply(spapr, spapr->eff.caps[i], &error_fatal);
}
}
@@ -322,17 +328,19 @@ void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp)
for (i = 0; i < ARRAY_SIZE(capability_table); i++) {
sPAPRCapabilityInfo *cap = &capability_table[i];
const char *name = g_strdup_printf("cap-%s", cap->name);
+ char *desc;
- object_class_property_add(klass, name, "bool",
- spapr_cap_get, spapr_cap_set, NULL,
- cap, &local_err);
+ object_class_property_add(klass, name, cap->type,
+ cap->get, cap->set,
+ NULL, cap, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
- object_class_property_set_description(klass, name, cap->description,
- &local_err);
+ desc = g_strdup_printf("%s%s", cap->description, cap->options);
+ object_class_property_set_description(klass, name, desc, &local_err);
+ g_free(desc);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 26ac17e641..0f5628f22e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -54,20 +54,25 @@ typedef enum {
* Capabilities
*/
-/* These bits go in the migration stream, so they can't be reassigned */
-
/* Hardware Transactional Memory */
-#define SPAPR_CAP_HTM 0x0000000000000001ULL
-
+#define SPAPR_CAP_HTM 0x00
/* Vector Scalar Extensions */
-#define SPAPR_CAP_VSX 0x0000000000000002ULL
-
+#define SPAPR_CAP_VSX 0x01
/* Decimal Floating Point */
-#define SPAPR_CAP_DFP 0x0000000000000004ULL
+#define SPAPR_CAP_DFP 0x02
+/* Num Caps */
+#define SPAPR_CAP_NUM (SPAPR_CAP_DFP + 1)
+
+/*
+ * Capability Values
+ */
+/* Bool Caps */
+#define SPAPR_CAP_OFF 0x00
+#define SPAPR_CAP_ON 0x01
typedef struct sPAPRCapabilities sPAPRCapabilities;
struct sPAPRCapabilities {
- uint64_t mask;
+ uint8_t caps[SPAPR_CAP_NUM];
};
/**
@@ -149,9 +154,8 @@ struct sPAPRMachineState {
const char *icp_type;
- sPAPRCapabilities forced_caps, forbidden_caps;
- sPAPRCapabilities mig_forced_caps, mig_forbidden_caps;
- sPAPRCapabilities effective_caps;
+ bool cmd_line_caps[SPAPR_CAP_NUM];
+ sPAPRCapabilities def, eff, mig;
};
#define H_SUCCESS 0
@@ -749,24 +753,23 @@ int spapr_irq_alloc_block(sPAPRMachineState *spapr, int num, bool lsi,
void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
+
+int spapr_caps_pre_load(void *opaque);
+int spapr_caps_pre_save(void *opaque);
+
/*
* Handling of optional capabilities
*/
-extern const VMStateDescription vmstate_spapr_caps;
-
-static inline sPAPRCapabilities spapr_caps(uint64_t mask)
-{
- sPAPRCapabilities caps = { mask };
- return caps;
-}
+extern const VMStateDescription vmstate_spapr_cap_htm;
+extern const VMStateDescription vmstate_spapr_cap_vsx;
+extern const VMStateDescription vmstate_spapr_cap_dfp;
-static inline bool spapr_has_cap(sPAPRMachineState *spapr, uint64_t cap)
+static inline uint8_t spapr_get_cap(sPAPRMachineState *spapr, int cap)
{
- return !!(spapr->effective_caps.mask & cap);
+ return spapr->eff.caps[cap];
}
void spapr_caps_reset(sPAPRMachineState *spapr);
-void spapr_caps_validate(sPAPRMachineState *spapr, Error **errp);
void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp);
int spapr_caps_post_migration(sPAPRMachineState *spapr);
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 09/22] spapr: Remove unnecessary 'options' field from sPAPRCapabilityInfo
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (7 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 08/22] hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 10/22] ppc: Change Power9 compat table to support at most 8 threads/core David Gibson
` (12 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
The options field here is intended to list the available values for the
capability. It's not used yet, because the existing capabilities are
boolean.
We're going to add capabilities that aren't, but in that case the info on
the possible values can be folded into the .description field.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_caps.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index f95a78547d..d5c9ce774a 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -35,7 +35,6 @@
typedef struct sPAPRCapabilityInfo {
const char *name;
const char *description;
- const char *options; /* valid capability values */
int index;
/* Getter and Setter Function Pointers */
@@ -126,7 +125,6 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
.description = "Allow Hardware Transactional Memory (HTM)",
- .options = "",
.index = SPAPR_CAP_HTM,
.get = spapr_cap_get_bool,
.set = spapr_cap_set_bool,
@@ -136,7 +134,6 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_VSX] = {
.name = "vsx",
.description = "Allow Vector Scalar Extensions (VSX)",
- .options = "",
.index = SPAPR_CAP_VSX,
.get = spapr_cap_get_bool,
.set = spapr_cap_set_bool,
@@ -146,7 +143,6 @@ sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_DFP] = {
.name = "dfp",
.description = "Allow Decimal Floating Point (DFP)",
- .options = "",
.index = SPAPR_CAP_DFP,
.get = spapr_cap_get_bool,
.set = spapr_cap_set_bool,
@@ -338,7 +334,7 @@ void spapr_caps_add_properties(sPAPRMachineClass *smc, Error **errp)
return;
}
- desc = g_strdup_printf("%s%s", cap->description, cap->options);
+ desc = g_strdup_printf("%s", cap->description);
object_class_property_set_description(klass, name, desc, &local_err);
g_free(desc);
if (local_err) {
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 10/22] ppc: Change Power9 compat table to support at most 8 threads/core
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (8 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 09/22] spapr: Remove unnecessary 'options' field from sPAPRCapabilityInfo David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 11/22] target/ppc: Clarify compat mode max_threads value David Gibson
` (11 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
From: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Increases the max smt mode to 8 for Power9. That's because KVM supports
smt emulation in this platform so QEMU should allow users to use it as
well.
Today if we try to pass -smp ...,threads=8, QEMU will silently truncate
it to smt4 mode and may cause a crash if we try to perform a cpu
hotplug.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
[dwg: Added an explanatory comment]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/compat.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index ad8f93c064..276b5b52c2 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -73,7 +73,14 @@ static const CompatInfo compat_table[] = {
.pvr = CPU_POWERPC_LOGICAL_3_00,
.pcr = PCR_COMPAT_3_00,
.pcr_level = PCR_COMPAT_3_00,
- .max_threads = 4,
+ /*
+ * POWER9 hardware only supports 4 threads / core, but this
+ * limit is for guests. We need to support 8 vthreads/vcore
+ * on POWER9 for POWER8 compatibility guests, and it's very
+ * confusing if half of the threads disappear from the guest
+ * if it announces it's POWER9 aware at CAS time.
+ */
+ .max_threads = 8,
},
};
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 11/22] target/ppc: Clarify compat mode max_threads value
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (9 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 10/22] ppc: Change Power9 compat table to support at most 8 threads/core David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 12/22] spapr: Allow some cases where we can't set VSMT mode in the kernel David Gibson
` (10 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
We recently had some discussions that were sidetracked for a while, because
nearly everyone misapprehended the purpose of the 'max_threads' field in
the compatiblity modes table. It's all about guest expectations, not host
expectations or support (that's handled elsewhere).
In an attempt to avoid a repeat of that confusion, rename the field to
'max_vthreads' and add an explanatory comment.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
---
hw/ppc/spapr.c | 4 ++--
target/ppc/compat.c | 25 +++++++++++++++++--------
target/ppc/cpu.h | 2 +-
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3e528fe91e..e35214bfc3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -345,7 +345,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachineState *spapr)
PowerPCCPU *cpu = POWERPC_CPU(cs);
DeviceClass *dc = DEVICE_GET_CLASS(cs);
int index = spapr_vcpu_id(cpu);
- int compat_smt = MIN(smp_threads, ppc_compat_max_threads(cpu));
+ int compat_smt = MIN(smp_threads, ppc_compat_max_vthreads(cpu));
if ((index % smt) != 0) {
continue;
@@ -503,7 +503,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));
+ int compat_smt = MIN(smp_threads, ppc_compat_max_vthreads(cpu));
sPAPRDRConnector *drc;
int drc_index;
uint32_t radix_AP_encodings[PPC_PAGE_SIZES_MAX_SZ];
diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index 276b5b52c2..807c906f68 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -32,7 +32,16 @@ typedef struct {
uint32_t pvr;
uint64_t pcr;
uint64_t pcr_level;
- int max_threads;
+
+ /*
+ * Maximum allowed virtual threads per virtual core
+ *
+ * This is to stop older guests getting confused by seeing more
+ * threads than they think the cpu can support. Usually it's
+ * equal to the number of threads supported on bare metal
+ * hardware, but not always (see POWER9).
+ */
+ int max_vthreads;
} CompatInfo;
static const CompatInfo compat_table[] = {
@@ -45,28 +54,28 @@ static const CompatInfo compat_table[] = {
.pcr = PCR_COMPAT_3_00 | 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,
+ .max_vthreads = 2,
},
{ /* POWER7, ISA2.06 */
.name = "power7",
.pvr = CPU_POWERPC_LOGICAL_2_06,
.pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
.pcr_level = PCR_COMPAT_2_06,
- .max_threads = 4,
+ .max_vthreads = 4,
},
{
.name = "power7+",
.pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
.pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
.pcr_level = PCR_COMPAT_2_06,
- .max_threads = 4,
+ .max_vthreads = 4,
},
{ /* POWER8, ISA2.07 */
.name = "power8",
.pvr = CPU_POWERPC_LOGICAL_2_07,
.pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07,
.pcr_level = PCR_COMPAT_2_07,
- .max_threads = 8,
+ .max_vthreads = 8,
},
{ /* POWER9, ISA3.00 */
.name = "power9",
@@ -80,7 +89,7 @@ static const CompatInfo compat_table[] = {
* confusing if half of the threads disappear from the guest
* if it announces it's POWER9 aware at CAS time.
*/
- .max_threads = 8,
+ .max_vthreads = 8,
},
};
@@ -192,14 +201,14 @@ void ppc_set_compat_all(uint32_t compat_pvr, Error **errp)
}
}
-int ppc_compat_max_threads(PowerPCCPU *cpu)
+int ppc_compat_max_vthreads(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);
+ n_threads = MIN(n_threads, compat->max_vthreads);
}
return n_threads;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a5e49f23e9..dc6820c5eb 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1395,7 +1395,7 @@ 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);
+int ppc_compat_max_vthreads(PowerPCCPU *cpu);
void ppc_compat_add_property(Object *obj, const char *name,
uint32_t *compat_pvr, const char *basedesc,
Error **errp);
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 12/22] spapr: Allow some cases where we can't set VSMT mode in the kernel
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (10 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 11/22] target/ppc: Clarify compat mode max_threads value David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 13/22] spapr: Adjust default VSMT value for better migration compatibility David Gibson
` (9 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
At present if we require a vsmt mode that's not equal to the kernel's
default, and the kernel doesn't let us change it (e.g. because it's an old
kernel without support) then we always fail.
But in fact we can cope with the kernel having a different vsmt as long as
a) it's >= the actual number of vthreads/vcore (so that guest threads
that are supposed to be on the same core act like it)
b) it's a submultiple of the requested vsmt mode (so that guest threads
spaced by the vsmt value will act like they're on different cores)
Allowing this case gives us a bit more freedom to adjust the vsmt behaviour
without breaking existing cases.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e35214bfc3..9e7088a192 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2314,17 +2314,29 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
if (kvm_enabled() && (spapr->vsmt != kvm_smt)) {
ret = kvmppc_set_smt_threads(spapr->vsmt);
if (ret) {
+ /* Looks like KVM isn't able to change VSMT mode */
error_setg(&local_err,
"Failed to set KVM's VSMT mode to %d (errno %d)",
spapr->vsmt, ret);
- if (!vsmt_user) {
- error_append_hint(&local_err, "On PPC, a VM with %d threads/"
- "core on a host with %d threads/core requires "
- " the use of VSMT mode %d.\n",
- smp_threads, kvm_smt, spapr->vsmt);
+ /* We can live with that if the default one is big enough
+ * for the number of threads, and a submultiple of the one
+ * we want. In this case we'll waste some vcpu ids, but
+ * behaviour will be correct */
+ if ((kvm_smt >= smp_threads) && ((spapr->vsmt % kvm_smt) == 0)) {
+ warn_report_err(local_err);
+ local_err = NULL;
+ goto out;
+ } else {
+ if (!vsmt_user) {
+ error_append_hint(&local_err,
+ "On PPC, a VM with %d threads/core"
+ " on a host with %d threads/core"
+ " requires the use of VSMT mode %d.\n",
+ smp_threads, kvm_smt, spapr->vsmt);
+ }
+ kvmppc_hint_smt_possible(&local_err);
+ goto out;
}
- kvmppc_hint_smt_possible(&local_err);
- goto out;
}
}
/* else TCG: nothing to do currently */
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 13/22] spapr: Adjust default VSMT value for better migration compatibility
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (11 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 12/22] spapr: Allow some cases where we can't set VSMT mode in the kernel David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 15/22] tests/boot-serial-test: fix powernv support David Gibson
` (8 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
David Gibson
fa98fbfc "PC: KVM: Support machine option to set VSMT mode" introduced the
"vsmt" parameter for the pseries machine type, which controls the spacing
of the vcpu ids of thread 0 for each virtual core. This was done to bring
some consistency and stability to how that was done, while still allowing
backwards compatibility for migration and otherwise.
The default value we used for vsmt was set to the max of the host's
advertised default number of threads and the number of vthreads per vcore
in the guest. This was done to continue running without extra parameters
on older KVM versions which don't allow the VSMT value to be changed.
Unfortunately, even that smaller than before leakage of host configuration
into guest visible configuration still breaks things. Specifically a guest
with 4 (or less) vthread/vcore will get a different vsmt value when
running on a POWER8 (vsmt==8) and POWER9 (vsmt==4) host. That means the
vcpu ids don't line up so you can't migrate between them, though you should
be able to.
Long term we really want to make vsmt == smp_threads for sufficiently
new machine types. However, that means that qemu will then require a
sufficiently recent KVM (one which supports changing VSMT) - that's still
not widely enough deployed to be really comfortable to do.
In the meantime we need some default that will work as often as
possible. This patch changes that default to 8 in all circumstances.
This does change guest visible behaviour (including for existing
machine versions) for many cases - just not the most common/important
case.
Following is case by case justification for why this is still the least
worst option. Note that any of the old behaviours can still be duplicated
after this patch, it's just that it requires manual intervention by
setting the vsmt property on the command line.
KVM HV on POWER8 host:
This is the overwhelmingly common case in production setups, and is
unchanged by design. POWER8 hosts will advertise a default VSMT mode
of 8, and > 8 vthreads/vcore isn't permitted
KVM HV on POWER7 host:
Will break, but POWER7s allowing KVM were never released to the public.
KVM HV on POWER9 host:
Not yet released to the public, breaking this now will reduce other
breakage later.
KVM HV on PowerPC 970:
Will theoretically break it, but it was barely supported to begin with
and already required various user visible hacks to work. Also so old
that I just don't care.
TCG:
This is the nastiest one; it means migration of TCG guests (without
manual vsmt setting) will break. Since TCG is rarely used in production
I think this is worth it for the other benefits. It does also remove
one more barrier to TCG<->KVM migration which could be interesting for
debugging applications.
KVM PR:
As with TCG, this will break migration of existing configurations,
without adding extra manual vsmt options. As with TCG, it is rare in
production so I think the benefits outweigh breakages.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9e7088a192..499ab647d8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2305,9 +2305,14 @@ static void spapr_set_vsmt_mode(sPAPRMachineState *spapr, Error **errp)
}
/* In this case, spapr->vsmt has been set by the command line */
} else {
- /* Choose a VSMT mode that may be higher than necessary but is
- * likely to be compatible with hosts that don't have VSMT. */
- spapr->vsmt = MAX(kvm_smt, smp_threads);
+ /*
+ * Default VSMT value is tricky, because we need it to be as
+ * consistent as possible (for migration), but this requires
+ * changing it for at least some existing cases. We pick 8 as
+ * the value that we'd get with KVM on POWER8, the
+ * overwhelmingly common case in production systems.
+ */
+ spapr->vsmt = 8;
}
/* KVM: If necessary, set the SMT mode: */
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 15/22] tests/boot-serial-test: fix powernv support
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (12 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 13/22] spapr: Adjust default VSMT value for better migration compatibility David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 16/22] ppc/pnv: use POWER9 DD2 processor David Gibson
` (7 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
Recent commit introduced the firmware image skiboot 5.9 which
has a different first line ouput.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
tests/boot-serial-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 663b78b950..418c5b92dc 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -72,7 +72,7 @@ static testdef_t tests[] = {
{ "ppc64", "ppce500", "", "U-Boot" },
{ "ppc64", "prep", "", "Open Hack'Ware BIOS" },
{ "ppc64", "pseries", "", "Open Firmware" },
- { "ppc64", "powernv", "-cpu POWER8", "SkiBoot" },
+ { "ppc64", "powernv", "-cpu POWER8", "OPAL" },
{ "i386", "isapc", "-cpu qemu32 -device sga", "SGABIOS" },
{ "i386", "pc", "-device sga", "SGABIOS" },
{ "i386", "q35", "-device sga", "SGABIOS" },
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 16/22] ppc/pnv: use POWER9 DD2 processor
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (13 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 15/22] tests/boot-serial-test: fix powernv support David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 17/22] ppc/pnv: change core mask for POWER9 David Gibson
` (6 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
commit 1ed9c8af501f ("target/ppc: Add POWER9 DD2.0 model information")
deprecated the POWER9 model v1.0.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/pnv.c | 2 +-
tests/pnv-xscom-test.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 9475e8479c..536162b274 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -759,7 +759,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
PnvChipClass *k = PNV_CHIP_CLASS(klass);
k->chip_type = PNV_CHIP_POWER9;
- k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
+ k->chip_cfam_id = 0x220d104900008000ull; /* P9 Nimbus DD2.0 */
k->cores_mask = POWER9_CORE_MASK;
k->core_pir = pnv_chip_core_pir_p9;
k->xscom_base = 0x00603fc00000000ull;
diff --git a/tests/pnv-xscom-test.c b/tests/pnv-xscom-test.c
index 89fa6282d3..a1a119c091 100644
--- a/tests/pnv-xscom-test.c
+++ b/tests/pnv-xscom-test.c
@@ -48,7 +48,7 @@ static const PnvChip pnv_chips[] = {
.cpu_model = "POWER9",
.xscom_base = 0x000603fc00000000ull,
.xscom_core_base = 0x0ull,
- .cfam_id = 0x100d104980000000ull,
+ .cfam_id = 0x220d104900008000ull,
.first_core = 0x20,
},
#endif
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 17/22] ppc/pnv: change core mask for POWER9
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (14 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 16/22] ppc/pnv: use POWER9 DD2 processor David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 18/22] ppc/pnv: introduce pnv*_is_power9() helpers David Gibson
` (5 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
When addressed by XSCOM, the first core has the 0x20 chiplet ID but
the CPU PIR can start at 0x0.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/pnv.c | 4 ++--
tests/pnv-xscom-test.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 536162b274..f9591cd41d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -707,9 +707,9 @@ static uint32_t pnv_chip_core_pir_p9(PnvChip *chip, uint32_t core_id)
#define POWER8_CORE_MASK (0x7e7eull)
/*
- * POWER9 has 24 cores, ids starting at 0x20
+ * POWER9 has 24 cores, ids starting at 0x0
*/
-#define POWER9_CORE_MASK (0xffffff00000000ull)
+#define POWER9_CORE_MASK (0xffffffffffffffull)
static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
{
diff --git a/tests/pnv-xscom-test.c b/tests/pnv-xscom-test.c
index a1a119c091..9d545c4718 100644
--- a/tests/pnv-xscom-test.c
+++ b/tests/pnv-xscom-test.c
@@ -49,7 +49,7 @@ static const PnvChip pnv_chips[] = {
.xscom_base = 0x000603fc00000000ull,
.xscom_core_base = 0x0ull,
.cfam_id = 0x220d104900008000ull,
- .first_core = 0x20,
+ .first_core = 0x0,
},
#endif
};
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 18/22] ppc/pnv: introduce pnv*_is_power9() helpers
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (15 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 17/22] ppc/pnv: change core mask for POWER9 David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 19/22] ppc/pnv: fix XSCOM core addressing on POWER9 David Gibson
` (4 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
These are useful when instantiating device models which are shared
between the POWER8 and the POWER9 processor families.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/pnv_xscom.c | 8 +++-----
include/hw/ppc/pnv.h | 10 ++++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index e51d634f40..99c40efecd 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -51,10 +51,9 @@ static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
static uint32_t pnv_xscom_pcba(PnvChip *chip, uint64_t addr)
{
- PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
-
addr &= (PNV_XSCOM_SIZE - 1);
- if (pcc->chip_type == PNV_CHIP_POWER9) {
+
+ if (pnv_chip_is_power9(chip)) {
return addr >> 3;
} else {
return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf);
@@ -231,7 +230,6 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset)
int xscom_offset;
ForeachPopulateArgs args;
char *name;
- PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
name = g_strdup_printf("xscom@%" PRIx64, be64_to_cpu(reg[0]));
xscom_offset = fdt_add_subnode(fdt, root_offset, name);
@@ -242,7 +240,7 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset)
_FDT((fdt_setprop_cell(fdt, xscom_offset, "#size-cells", 1)));
_FDT((fdt_setprop(fdt, xscom_offset, "reg", reg, sizeof(reg))));
- if (pcc->chip_type == PNV_CHIP_POWER9) {
+ if (pnv_chip_is_power9(chip)) {
_FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p9,
sizeof(compat_p9))));
} else {
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 61896f9fd7..f023f1ec99 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -138,6 +138,16 @@ typedef struct PnvMachineState {
Notifier powerdown_notifier;
} PnvMachineState;
+static inline bool pnv_chip_is_power9(const PnvChip *chip)
+{
+ return PNV_CHIP_GET_CLASS(chip)->chip_type == PNV_CHIP_POWER9;
+}
+
+static inline bool pnv_is_power9(PnvMachineState *pnv)
+{
+ return pnv_chip_is_power9(pnv->chips[0]);
+}
+
#define PNV_FDT_ADDR 0x01000000
#define PNV_TIMEBASE_FREQ 512000000ULL
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 19/22] ppc/pnv: fix XSCOM core addressing on POWER9
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (16 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 18/22] ppc/pnv: introduce pnv*_is_power9() helpers David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 20/22] ppc/pnv: change initrd address David Gibson
` (3 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
The XSCOM base address of the core chiplet was wrongly calculated. Use
the OPAL macros to fix that and do a couple of renames.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/pnv.c | 15 ++++++++-------
hw/ppc/pnv_core.c | 2 +-
include/hw/ppc/pnv.h | 1 -
include/hw/ppc/pnv_xscom.h | 13 +++++++++++--
tests/pnv-xscom-test.c | 27 +++++++++++++++++----------
5 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index f9591cd41d..80245f57f1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -721,7 +721,6 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
k->cores_mask = POWER8E_CORE_MASK;
k->core_pir = pnv_chip_core_pir_p8;
k->xscom_base = 0x003fc0000000000ull;
- k->xscom_core_base = 0x10000000ull;
dc->desc = "PowerNV Chip POWER8E";
}
@@ -735,7 +734,6 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
k->cores_mask = POWER8_CORE_MASK;
k->core_pir = pnv_chip_core_pir_p8;
k->xscom_base = 0x003fc0000000000ull;
- k->xscom_core_base = 0x10000000ull;
dc->desc = "PowerNV Chip POWER8";
}
@@ -749,7 +747,6 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
k->cores_mask = POWER8_CORE_MASK;
k->core_pir = pnv_chip_core_pir_p8;
k->xscom_base = 0x003fc0000000000ull;
- k->xscom_core_base = 0x10000000ull;
dc->desc = "PowerNV Chip POWER8NVL";
}
@@ -763,7 +760,6 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
k->cores_mask = POWER9_CORE_MASK;
k->core_pir = pnv_chip_core_pir_p9;
k->xscom_base = 0x00603fc00000000ull;
- k->xscom_core_base = 0x0ull;
dc->desc = "PowerNV Chip POWER9";
}
@@ -887,6 +883,7 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
&& (i < chip->nr_cores); core_hwid++) {
char core_name[32];
void *pnv_core = chip->cores + i * typesize;
+ uint64_t xscom_core_base;
if (!(chip->cores_mask & (1ull << core_hwid))) {
continue;
@@ -910,9 +907,13 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
object_unref(OBJECT(pnv_core));
/* Each core has an XSCOM MMIO region */
- pnv_xscom_add_subregion(chip,
- PNV_XSCOM_EX_CORE_BASE(pcc->xscom_core_base,
- core_hwid),
+ if (!pnv_chip_is_power9(chip)) {
+ xscom_core_base = PNV_XSCOM_EX_BASE(core_hwid);
+ } else {
+ xscom_core_base = PNV_XSCOM_P9_EC_BASE(core_hwid);
+ }
+
+ pnv_xscom_add_subregion(chip, xscom_core_base,
&PNV_CORE(pnv_core)->xscom_regs);
i++;
}
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 7e8a76df44..cbb64ad9e7 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -192,7 +192,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id);
pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), &pnv_core_xscom_ops,
- pc, name, PNV_XSCOM_EX_CORE_SIZE);
+ pc, name, PNV_XSCOM_EX_SIZE);
return;
err:
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index f023f1ec99..90759240a7 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -74,7 +74,6 @@ typedef struct PnvChipClass {
uint64_t cores_mask;
hwaddr xscom_base;
- hwaddr xscom_core_base;
uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
} PnvChipClass;
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 7252e219e2..fb1bd5df09 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -21,6 +21,8 @@
#include "qom/object.h"
+typedef struct PnvChip PnvChip;
+
typedef struct PnvXScomInterface {
Object parent;
} PnvXScomInterface;
@@ -54,8 +56,15 @@ typedef struct PnvXScomInterfaceClass {
* PCB SLAVE 0x110Fxxxx
*/
-#define PNV_XSCOM_EX_CORE_BASE(base, i) ((base) | ((uint64_t)(i) << 24))
-#define PNV_XSCOM_EX_CORE_SIZE 0x100000
+#define PNV_XSCOM_EX_CORE_BASE 0x10000000ull
+
+#define PNV_XSCOM_EX_BASE(core) \
+ (PNV_XSCOM_EX_CORE_BASE | ((uint64_t)(core) << 24))
+#define PNV_XSCOM_EX_SIZE 0x100000
+
+#define PNV_XSCOM_P9_EC_BASE(core) \
+ ((uint64_t)(((core) & 0x1F) + 0x20) << 24)
+#define PNV_XSCOM_P9_EC_SIZE 0x100000
#define PNV_XSCOM_LPC_BASE 0xb0020
#define PNV_XSCOM_LPC_SIZE 0x4
diff --git a/tests/pnv-xscom-test.c b/tests/pnv-xscom-test.c
index 9d545c4718..efb7c838b5 100644
--- a/tests/pnv-xscom-test.c
+++ b/tests/pnv-xscom-test.c
@@ -21,7 +21,6 @@ typedef struct PnvChip {
PnvChipType chip_type;
const char *cpu_model;
uint64_t xscom_base;
- uint64_t xscom_core_base;
uint64_t cfam_id;
uint32_t first_core;
} PnvChip;
@@ -31,14 +30,12 @@ static const PnvChip pnv_chips[] = {
.chip_type = PNV_CHIP_POWER8,
.cpu_model = "POWER8",
.xscom_base = 0x0003fc0000000000ull,
- .xscom_core_base = 0x10000000ull,
.cfam_id = 0x220ea04980000000ull,
.first_core = 0x1,
}, {
.chip_type = PNV_CHIP_POWER8NVL,
.cpu_model = "POWER8NVL",
.xscom_base = 0x0003fc0000000000ull,
- .xscom_core_base = 0x10000000ull,
.cfam_id = 0x120d304980000000ull,
.first_core = 0x1,
},
@@ -47,7 +44,6 @@ static const PnvChip pnv_chips[] = {
.chip_type = PNV_CHIP_POWER9,
.cpu_model = "POWER9",
.xscom_base = 0x000603fc00000000ull,
- .xscom_core_base = 0x0ull,
.cfam_id = 0x220d104900008000ull,
.first_core = 0x0,
},
@@ -89,16 +85,27 @@ static void test_cfam_id(const void *data)
qtest_quit(global_qtest);
}
-#define PNV_XSCOM_EX_CORE_BASE(chip, i) \
- ((chip)->xscom_core_base | (((uint64_t)i) << 24))
+
+#define PNV_XSCOM_EX_CORE_BASE 0x10000000ull
+#define PNV_XSCOM_EX_BASE(core) \
+ (PNV_XSCOM_EX_CORE_BASE | ((uint64_t)(core) << 24))
+#define PNV_XSCOM_P9_EC_BASE(core) \
+ ((uint64_t)(((core) & 0x1F) + 0x20) << 24)
+
#define PNV_XSCOM_EX_DTS_RESULT0 0x50000
static void test_xscom_core(const PnvChip *chip)
{
- uint32_t first_core_dts0 =
- PNV_XSCOM_EX_CORE_BASE(chip, chip->first_core) |
- PNV_XSCOM_EX_DTS_RESULT0;
- uint64_t dts0 = pnv_xscom_read(chip, first_core_dts0);
+ uint32_t first_core_dts0 = PNV_XSCOM_EX_DTS_RESULT0;
+ uint64_t dts0;
+
+ if (chip->chip_type != PNV_CHIP_POWER9) {
+ first_core_dts0 |= PNV_XSCOM_EX_BASE(chip->first_core);
+ } else {
+ first_core_dts0 |= PNV_XSCOM_P9_EC_BASE(chip->first_core);
+ }
+
+ dts0 = pnv_xscom_read(chip, first_core_dts0);
g_assert_cmphex(dts0, ==, 0x26f024f023f0000ull);
}
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 20/22] ppc/pnv: change initrd address
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (17 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 19/22] ppc/pnv: fix XSCOM core addressing on POWER9 David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 21/22] target/ppc: add support for POWER9 HILE David Gibson
` (2 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
When skiboot starts, it first clears the CPU structs for all possible
CPUs on a system :
for (i = 0; i <= cpu_max_pir; i++)
memset(&cpu_stacks[i].cpu, 0, sizeof(struct cpu_thread));
On POWER9, cpu_max_pir is quite big, 0x7fff, and the skiboot cpu_stacks
array overlaps with the memory region in which QEMU maps the initramfs
file. Move it upwards in memory to keep it safe.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/pnv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 80245f57f1..98ee3c607a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -53,7 +53,7 @@
#define FW_MAX_SIZE 0x00400000
#define KERNEL_LOAD_ADDR 0x20000000
-#define INITRD_LOAD_ADDR 0x40000000
+#define INITRD_LOAD_ADDR 0x60000000
static const char *pnv_chip_core_typename(const PnvChip *o)
{
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 21/22] target/ppc: add support for POWER9 HILE
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (18 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 20/22] ppc/pnv: change initrd address David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-17 2:25 ` [Qemu-devel] [PULL 22/22] target-ppc: Fix booke206 tlbwe TLB instruction David Gibson
2018-01-18 12:58 ` [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 Peter Maydell
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier,
Cédric Le Goater, David Gibson
From: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/cpu.h | 1 +
target/ppc/excp_helper.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index dc6820c5eb..14aaa87fe8 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2012,6 +2012,7 @@ void ppc_compat_add_property(Object *obj, const char *name,
#define HID0_DOZE (1 << 23) /* pre-2.06 */
#define HID0_NAP (1 << 22) /* pre-2.06 */
#define HID0_HILE PPC_BIT(19) /* POWER8 */
+#define HID0_POWER9_HILE PPC_BIT(4)
/*****************************************************************************/
/* PowerPC Instructions types definitions */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 37d2410726..4e548a4487 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -654,7 +654,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
}
} else if (excp_model == POWERPC_EXCP_POWER8) {
if (new_msr & MSR_HVB) {
- if (env->spr[SPR_HID0] & HID0_HILE) {
+ if (env->spr[SPR_HID0] & (HID0_HILE | HID0_POWER9_HILE)) {
new_msr |= (target_ulong)1 << MSR_LE;
}
} else if (env->spr[SPR_LPCR] & LPCR_ILE) {
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PULL 22/22] target-ppc: Fix booke206 tlbwe TLB instruction
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (19 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 21/22] target/ppc: add support for POWER9 HILE David Gibson
@ 2018-01-17 2:25 ` David Gibson
2018-01-18 12:58 ` [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 Peter Maydell
21 siblings, 0 replies; 23+ messages in thread
From: David Gibson @ 2018-01-17 2:25 UTC (permalink / raw)
To: peter.maydell
Cc: groug, surajjs, qemu-ppc, qemu-devel, joserz, lvivier, Luc MICHEL,
David Gibson
From: Luc MICHEL <luc.michel@git.antfield.fr>
When overwritting a valid TLB entry with a new one, the previous page
were not flushed in QEMU TLB, leading to incoherent mapping. This commit
fixes this.
Signed-off-by: Luc MICHEL <luc.michel@git.antfield.fr>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/mmu_helper.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 2a1f9902c9..298c15e961 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2570,6 +2570,17 @@ void helper_booke_setpid(CPUPPCState *env, uint32_t pidn, target_ulong pid)
tlb_flush(CPU(cpu));
}
+static inline void flush_page(CPUPPCState *env, ppcmas_tlb_t *tlb)
+{
+ PowerPCCPU *cpu = ppc_env_get_cpu(env);
+
+ if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
+ tlb_flush_page(CPU(cpu), tlb->mas2 & MAS2_EPN_MASK);
+ } else {
+ tlb_flush(CPU(cpu));
+ }
+}
+
void helper_booke206_tlbwe(CPUPPCState *env)
{
PowerPCCPU *cpu = ppc_env_get_cpu(env);
@@ -2628,6 +2639,21 @@ void helper_booke206_tlbwe(CPUPPCState *env)
if (msr_gs) {
cpu_abort(CPU(cpu), "missing HV implementation\n");
}
+
+ if (tlb->mas1 & MAS1_VALID) {
+ /* Invalidate the page in QEMU TLB if it was a valid entry.
+ *
+ * In "PowerPC e500 Core Family Reference Manual, Rev. 1",
+ * Section "12.4.2 TLB Write Entry (tlbwe) Instruction":
+ * (https://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf)
+ *
+ * "Note that when an L2 TLB entry is written, it may be displacing an
+ * already valid entry in the same L2 TLB location (a victim). If a
+ * valid L1 TLB entry corresponds to the L2 MMU victim entry, that L1
+ * TLB entry is automatically invalidated." */
+ flush_page(env, tlb);
+ }
+
tlb->mas7_3 = ((uint64_t)env->spr[SPR_BOOKE_MAS7] << 32) |
env->spr[SPR_BOOKE_MAS3];
tlb->mas1 = env->spr[SPR_BOOKE_MAS1];
@@ -2663,11 +2689,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
tlb->mas1 &= ~MAS1_IPROT;
}
- if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
- tlb_flush_page(CPU(cpu), tlb->mas2 & MAS2_EPN_MASK);
- } else {
- tlb_flush(CPU(cpu));
- }
+ flush_page(env, tlb);
}
static inline void booke206_tlb_to_mas(CPUPPCState *env, ppcmas_tlb_t *tlb)
--
2.14.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117
2018-01-17 2:25 [Qemu-devel] [PULL 00/22] ppc-for-2.12 queue 20180117 David Gibson
` (20 preceding siblings ...)
2018-01-17 2:25 ` [Qemu-devel] [PULL 22/22] target-ppc: Fix booke206 tlbwe TLB instruction David Gibson
@ 2018-01-18 12:58 ` Peter Maydell
21 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2018-01-18 12:58 UTC (permalink / raw)
To: David Gibson
Cc: Greg Kurz, surajjs, qemu-ppc, QEMU Developers,
Jose Ricardo Ziviani, Laurent Vivier
On 17 January 2018 at 02:25, David Gibson <david@gibson.dropbear.id.au> wrote:
> The following changes since commit 8e5dc9ba49743b46d955ec7dacb04e42ae7ada7c:
>
> Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20180116' into staging (2018-01-16 17:36:39 +0000)
>
> are available in the Git repository at:
>
> git://github.com/dgibson/qemu.git tags/ppc-for-2.12-20180117
>
> for you to fetch changes up to 2e569845bd314fc1dde83d65dc9b87e71b4d29b4:
>
> target-ppc: Fix booke206 tlbwe TLB instruction (2018-01-17 09:35:24 +1100)
>
> This pull request hasn't been through a Travis build, unfortunately,
> since Travis seems to have a gigantic backlog today. I've run all the
> rest of my usual tests, and I did get a Travis build of this branch
> before the latest minor rebase, so it's unlikely there's a problem
> there.
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread