[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH] spapr: Add forgotten capability to migration stre
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH] spapr: Add forgotten capability to migration stream |
Date: |
Fri, 17 May 2019 19:14:30 +0200 |
On Fri, 17 May 2019 14:18:23 +1000
David Gibson <address@hidden> wrote:
> spapr machine capabilities are supposed to be sent in the migration stream
> so that we can sanity check the source and destination have compatible
> configuration. Unfortunately, when we added the hpt-max-page-size
> capability, we forgot to add it to the migration state. This means that we
> can generate spurious warnings when both ends are configured for large
> pages, or potentially fail to warn if the source is configured for huge
> pages, but the destination is not.
>
> Fixes: 2309832afda "spapr: Maximum (HPT) pagesize property"
>
Sorry I didn't spot that during review :-\
> Signed-off-by: David Gibson <address@hidden>
> ---
This breaks backward migration if the cap is set to a non-default
value, since older QEMUs don't expect the "spapr/cap/hpt_maxpagesize"
subsection.
This being said, I'm not sure any other value but the current default (16)
actually works, so maybe we don't care. If so,
Reviewed-by: Greg Kurz <address@hidden>
Otherwise, I was thinking about something like this:
8<=============================================================================
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 9fc91c8f5eac..4f5becf1f3cc 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -119,6 +119,7 @@ struct SpaprMachineClass {
bool pre_2_10_has_unused_icps;
bool legacy_irq_allocation;
bool broken_host_serial_model; /* present real host info to the guest */
+ bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
uint64_t *buid, hwaddr *pio,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bcae30ad26c3..c8b3cccd5375 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4413,8 +4413,12 @@ DEFINE_SPAPR_MACHINE(4_1, "4.1", true);
*/
static void spapr_machine_4_0_class_options(MachineClass *mc)
{
+ SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
spapr_machine_4_1_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len);
+
+ smc->pre_4_1_migration = true;
}
DEFINE_SPAPR_MACHINE(4_0, "4.0", false);
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 658eb15a147b..8a77bbdcf322 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -64,6 +64,7 @@ typedef struct SpaprCapabilityInfo {
void (*apply)(SpaprMachineState *spapr, uint8_t val, Error **errp);
void (*cpu_apply)(SpaprMachineState *spapr, PowerPCCPU *cpu,
uint8_t val, Error **errp);
+ bool (*migrate_needed)(void *opaque);
} SpaprCapabilityInfo;
static void spapr_cap_get_bool(Object *obj, Visitor *v, const char *name,
@@ -350,6 +351,11 @@ static void cap_hpt_maxpagesize_apply(SpaprMachineState
*spapr,
spapr_check_pagesize(spapr, qemu_minrampagesize(), errp);
}
+static bool cap_hpt_maxpagesize_migrate_needed(void *opaque)
+{
+ return SPAPR_MACHINE_CLASS(opaque)->pre_4_1_migration;
+}
+
static bool spapr_pagesize_cb(void *opaque, uint32_t seg_pshift,
uint32_t pshift)
{
@@ -542,6 +548,7 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
.type = "int",
.apply = cap_hpt_maxpagesize_apply,
.cpu_apply = cap_hpt_maxpagesize_cpu_apply,
+ .migrate_needed = cap_hpt_maxpagesize_migrate_needed,
},
[SPAPR_CAP_NESTED_KVM_HV] = {
.name = "nested-hv",
@@ -679,8 +686,11 @@ int spapr_caps_post_migration(SpaprMachineState *spapr)
static bool spapr_cap_##sname##_needed(void *opaque) \
{ \
SpaprMachineState *spapr = opaque; \
+ bool (*needed)(void *opaque) = \
+ capability_table[cap].migrate_needed; \
\
- return spapr->cmd_line_caps[cap] && \
+ return needed ? needed(opaque) : true && \
+ spapr->cmd_line_caps[cap] && \
(spapr->eff.caps[cap] != \
spapr->def.caps[cap]); \
} \
8<============================================================================
> hw/ppc/spapr.c | 1 +
> hw/ppc/spapr_caps.c | 1 +
> include/hw/ppc/spapr.h | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8580a8dc67..bcae30ad26 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = {
> &vmstate_spapr_cap_cfpc,
> &vmstate_spapr_cap_sbbc,
> &vmstate_spapr_cap_ibs,
> + &vmstate_spapr_cap_hpt_maxpagesize,
> &vmstate_spapr_irq_map,
> &vmstate_spapr_cap_nested_kvm_hv,
> &vmstate_spapr_dtb,
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9b1c10baa6..658eb15a14 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP);
> SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC);
> SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC);
> SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS);
> +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE);
> SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
> SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
> SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 7e32f309c2..9fc91c8f5e 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp;
> extern const VMStateDescription vmstate_spapr_cap_cfpc;
> extern const VMStateDescription vmstate_spapr_cap_sbbc;
> extern const VMStateDescription vmstate_spapr_cap_ibs;
> +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize;
> extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
> extern const VMStateDescription vmstate_spapr_cap_large_decr;
> extern const VMStateDescription vmstate_spapr_cap_ccf_assist;