Message ID | 1479433227-29238-4-git-send-email-mdroth@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Quoting Michael Roth (2016-11-17 19:40:27) > With the additional of the OV5_HP_EVT option vector, we now have > certain functionality (namely, memory unplug) that checks at run-time > for whether or not the guest negotiated the option via CAS. Because > we don't currently migrate these negotiated values, we are unable > to unplug memory from a guest after it's been migrated until after > the guest is rebooted and CAS-negotiation is repeated. > > This patch fixes this by adding CAS-negotiated options to the > migration stream. We do this using a subsection, since the > negotiated value of OV5_HP_EVT is the only option currently needed > to maintain proper functionality for a running guest. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++ > hw/ppc/spapr_ovec.c | 12 ++++++++ > include/hw/ppc/spapr_ovec.h | 4 +++ > 3 files changed, 84 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 0cbab24..9e08aed 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1267,6 +1267,70 @@ static bool version_before_3(void *opaque, int version_id) > return version_id < 3; > } > > +static bool spapr_ov5_cas_needed(void *opaque) > +{ > + sPAPRMachineState *spapr = opaque; > + sPAPROptionVector *ov5_mask = spapr_ovec_new(); > + sPAPROptionVector *ov5_legacy = spapr_ovec_new(); > + sPAPROptionVector *ov5_removed = spapr_ovec_new(); > + bool cas_needed; > + > + /* Prior to the introduction of sPAPROptionVector, we had two option > + * vectors we dealt with: OV5_FORM1_AFFINITY, and OV5_DRCONF_MEMORY. > + * Both of these options encode machine topology into the device-tree > + * in such a way that the now-booted OS should still be able to interact > + * appropriately with QEMU regardless of what options were actually > + * negotiatied on the source side. > + * > + * As such, we can avoid migrating the CAS-negotiated options if these > + * are the only options available on the current machine/platform. > + * Since these are the only options available for pseries-2.7 and > + * earlier, this allows us to maintain old->new/new->old migration > + * compatibility. > + * > + * For QEMU 2.8+, there are additional CAS-negotiatable options available > + * via default pseries-2.8 machines and explicit command-line parameters. > + * Some of these options, like OV5_HP_EVT, *do* require QEMU to be aware > + * of the actual CAS-negotiated values to continue working properly. For > + * example, availability of memory unplug depends on knowing whether > + * OV5_HP_EVT was negotiated via CAS. > + * > + * Thus, for any cases where the set of available CAS-negotiatable > + * options extends beyond OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY, we > + * include the CAS-negotiated options in the migration stream. > + */ > + spapr_ovec_set(ov5_mask, OV5_FORM1_AFFINITY); > + spapr_ovec_set(ov5_mask, OV5_DRCONF_MEMORY); > + > + /* spapr_ovec_diff returns true if bits were removed. we avoid using > + * the mask itself since in the future it's possible "legacy" bits may be > + * removed via machine options, which could generate a false positive > + * that breaks migration. > + */ > + spapr_ovec_intersect(ov5_legacy, spapr->ov5, ov5_mask); > + cas_needed = spapr_ovec_diff(ov5_removed, spapr->ov5, ov5_legacy); > + > + spapr_ovec_cleanup(ov5_mask); > + spapr_ovec_cleanup(ov5_legacy); > + spapr_ovec_cleanup(ov5_removed); > + > + error_report("MIGRATION NEEDED: %d", cas_needed); Argh, sorry, I just noticed this stray debug comment that slipped in. Would you prefer a v2, or just removing it in-tree? > + > + return cas_needed; > +} > + > +static const VMStateDescription vmstate_spapr_ov5_cas = { > + .name = "spapr_option_vector_ov5_cas", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = spapr_ov5_cas_needed, > + .fields = (VMStateField[]) { > + VMSTATE_STRUCT_POINTER_V(ov5_cas, sPAPRMachineState, 1, > + vmstate_spapr_ovec, sPAPROptionVector), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > static const VMStateDescription vmstate_spapr = { > .name = "spapr", > .version_id = 3, > @@ -1282,6 +1346,10 @@ static const VMStateDescription vmstate_spapr = { > VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), > VMSTATE_END_OF_LIST() > }, > + .subsections = (const VMStateDescription*[]) { > + &vmstate_spapr_ov5_cas, > + NULL > + } > }; > > static int htab_save_setup(QEMUFile *f, void *opaque) > diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c > index c2a0d18..3eb1d59 100644 > --- a/hw/ppc/spapr_ovec.c > +++ b/hw/ppc/spapr_ovec.c > @@ -37,6 +37,17 @@ > */ > struct sPAPROptionVector { > unsigned long *bitmap; > + int32_t bitmap_size; /* only used for migration */ > +}; > + > +const VMStateDescription vmstate_spapr_ovec = { > + .name = "spapr_option_vector", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_BITMAP(bitmap, sPAPROptionVector, 1, bitmap_size), > + VMSTATE_END_OF_LIST() > + } > }; > > sPAPROptionVector *spapr_ovec_new(void) > @@ -45,6 +56,7 @@ sPAPROptionVector *spapr_ovec_new(void) > > ov = g_new0(sPAPROptionVector, 1); > ov->bitmap = bitmap_new(OV_MAXBITS); > + ov->bitmap_size = OV_MAXBITS; > > return ov; > } > diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h > index 6a06da3..355a344 100644 > --- a/include/hw/ppc/spapr_ovec.h > +++ b/include/hw/ppc/spapr_ovec.h > @@ -37,6 +37,7 @@ > #define _SPAPR_OVEC_H > > #include "cpu.h" > +#include "migration/vmstate.h" > > typedef struct sPAPROptionVector sPAPROptionVector; > > @@ -64,4 +65,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector); > int spapr_ovec_populate_dt(void *fdt, int fdt_offset, > sPAPROptionVector *ov, const char *name); > > +/* migration */ > +extern const VMStateDescription vmstate_spapr_ovec; > + > #endif /* !defined (_SPAPR_OVEC_H) */ > -- > 1.9.1 > >
On Fri, Nov 18, 2016 at 10:08:59AM -0600, Michael Roth wrote: > Quoting Michael Roth (2016-11-17 19:40:27) > > With the additional of the OV5_HP_EVT option vector, we now have > > certain functionality (namely, memory unplug) that checks at run-time > > for whether or not the guest negotiated the option via CAS. Because > > we don't currently migrate these negotiated values, we are unable > > to unplug memory from a guest after it's been migrated until after > > the guest is rebooted and CAS-negotiation is repeated. > > > > This patch fixes this by adding CAS-negotiated options to the > > migration stream. We do this using a subsection, since the > > negotiated value of OV5_HP_EVT is the only option currently needed > > to maintain proper functionality for a running guest. > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > hw/ppc/spapr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++ > > hw/ppc/spapr_ovec.c | 12 ++++++++ > > include/hw/ppc/spapr_ovec.h | 4 +++ > > 3 files changed, 84 insertions(+) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 0cbab24..9e08aed 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1267,6 +1267,70 @@ static bool version_before_3(void *opaque, int version_id) > > return version_id < 3; > > } > > > > +static bool spapr_ov5_cas_needed(void *opaque) > > +{ > > + sPAPRMachineState *spapr = opaque; > > + sPAPROptionVector *ov5_mask = spapr_ovec_new(); > > + sPAPROptionVector *ov5_legacy = spapr_ovec_new(); > > + sPAPROptionVector *ov5_removed = spapr_ovec_new(); > > + bool cas_needed; > > + > > + /* Prior to the introduction of sPAPROptionVector, we had two option > > + * vectors we dealt with: OV5_FORM1_AFFINITY, and OV5_DRCONF_MEMORY. > > + * Both of these options encode machine topology into the device-tree > > + * in such a way that the now-booted OS should still be able to interact > > + * appropriately with QEMU regardless of what options were actually > > + * negotiatied on the source side. > > + * > > + * As such, we can avoid migrating the CAS-negotiated options if these > > + * are the only options available on the current machine/platform. > > + * Since these are the only options available for pseries-2.7 and > > + * earlier, this allows us to maintain old->new/new->old migration > > + * compatibility. > > + * > > + * For QEMU 2.8+, there are additional CAS-negotiatable options available > > + * via default pseries-2.8 machines and explicit command-line parameters. > > + * Some of these options, like OV5_HP_EVT, *do* require QEMU to be aware > > + * of the actual CAS-negotiated values to continue working properly. For > > + * example, availability of memory unplug depends on knowing whether > > + * OV5_HP_EVT was negotiated via CAS. > > + * > > + * Thus, for any cases where the set of available CAS-negotiatable > > + * options extends beyond OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY, we > > + * include the CAS-negotiated options in the migration stream. > > + */ > > + spapr_ovec_set(ov5_mask, OV5_FORM1_AFFINITY); > > + spapr_ovec_set(ov5_mask, OV5_DRCONF_MEMORY); > > + > > + /* spapr_ovec_diff returns true if bits were removed. we avoid using > > + * the mask itself since in the future it's possible "legacy" bits may be > > + * removed via machine options, which could generate a false positive > > + * that breaks migration. > > + */ > > + spapr_ovec_intersect(ov5_legacy, spapr->ov5, ov5_mask); > > + cas_needed = spapr_ovec_diff(ov5_removed, spapr->ov5, ov5_legacy); > > + > > + spapr_ovec_cleanup(ov5_mask); > > + spapr_ovec_cleanup(ov5_legacy); > > + spapr_ovec_cleanup(ov5_removed); > > + > > + error_report("MIGRATION NEEDED: %d", cas_needed); > > Argh, sorry, I just noticed this stray debug comment that slipped in. > Would you prefer a v2, or just removing it in-tree? I've fixed it in tree. Thanks for pointing it out.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 0cbab24..9e08aed 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1267,6 +1267,70 @@ static bool version_before_3(void *opaque, int version_id) return version_id < 3; } +static bool spapr_ov5_cas_needed(void *opaque) +{ + sPAPRMachineState *spapr = opaque; + sPAPROptionVector *ov5_mask = spapr_ovec_new(); + sPAPROptionVector *ov5_legacy = spapr_ovec_new(); + sPAPROptionVector *ov5_removed = spapr_ovec_new(); + bool cas_needed; + + /* Prior to the introduction of sPAPROptionVector, we had two option + * vectors we dealt with: OV5_FORM1_AFFINITY, and OV5_DRCONF_MEMORY. + * Both of these options encode machine topology into the device-tree + * in such a way that the now-booted OS should still be able to interact + * appropriately with QEMU regardless of what options were actually + * negotiatied on the source side. + * + * As such, we can avoid migrating the CAS-negotiated options if these + * are the only options available on the current machine/platform. + * Since these are the only options available for pseries-2.7 and + * earlier, this allows us to maintain old->new/new->old migration + * compatibility. + * + * For QEMU 2.8+, there are additional CAS-negotiatable options available + * via default pseries-2.8 machines and explicit command-line parameters. + * Some of these options, like OV5_HP_EVT, *do* require QEMU to be aware + * of the actual CAS-negotiated values to continue working properly. For + * example, availability of memory unplug depends on knowing whether + * OV5_HP_EVT was negotiated via CAS. + * + * Thus, for any cases where the set of available CAS-negotiatable + * options extends beyond OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY, we + * include the CAS-negotiated options in the migration stream. + */ + spapr_ovec_set(ov5_mask, OV5_FORM1_AFFINITY); + spapr_ovec_set(ov5_mask, OV5_DRCONF_MEMORY); + + /* spapr_ovec_diff returns true if bits were removed. we avoid using + * the mask itself since in the future it's possible "legacy" bits may be + * removed via machine options, which could generate a false positive + * that breaks migration. + */ + spapr_ovec_intersect(ov5_legacy, spapr->ov5, ov5_mask); + cas_needed = spapr_ovec_diff(ov5_removed, spapr->ov5, ov5_legacy); + + spapr_ovec_cleanup(ov5_mask); + spapr_ovec_cleanup(ov5_legacy); + spapr_ovec_cleanup(ov5_removed); + + error_report("MIGRATION NEEDED: %d", cas_needed); + + return cas_needed; +} + +static const VMStateDescription vmstate_spapr_ov5_cas = { + .name = "spapr_option_vector_ov5_cas", + .version_id = 1, + .minimum_version_id = 1, + .needed = spapr_ov5_cas_needed, + .fields = (VMStateField[]) { + VMSTATE_STRUCT_POINTER_V(ov5_cas, sPAPRMachineState, 1, + vmstate_spapr_ovec, sPAPROptionVector), + VMSTATE_END_OF_LIST() + }, +}; + static const VMStateDescription vmstate_spapr = { .name = "spapr", .version_id = 3, @@ -1282,6 +1346,10 @@ static const VMStateDescription vmstate_spapr = { VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), VMSTATE_END_OF_LIST() }, + .subsections = (const VMStateDescription*[]) { + &vmstate_spapr_ov5_cas, + NULL + } }; static int htab_save_setup(QEMUFile *f, void *opaque) diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c index c2a0d18..3eb1d59 100644 --- a/hw/ppc/spapr_ovec.c +++ b/hw/ppc/spapr_ovec.c @@ -37,6 +37,17 @@ */ struct sPAPROptionVector { unsigned long *bitmap; + int32_t bitmap_size; /* only used for migration */ +}; + +const VMStateDescription vmstate_spapr_ovec = { + .name = "spapr_option_vector", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_BITMAP(bitmap, sPAPROptionVector, 1, bitmap_size), + VMSTATE_END_OF_LIST() + } }; sPAPROptionVector *spapr_ovec_new(void) @@ -45,6 +56,7 @@ sPAPROptionVector *spapr_ovec_new(void) ov = g_new0(sPAPROptionVector, 1); ov->bitmap = bitmap_new(OV_MAXBITS); + ov->bitmap_size = OV_MAXBITS; return ov; } diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h index 6a06da3..355a344 100644 --- a/include/hw/ppc/spapr_ovec.h +++ b/include/hw/ppc/spapr_ovec.h @@ -37,6 +37,7 @@ #define _SPAPR_OVEC_H #include "cpu.h" +#include "migration/vmstate.h" typedef struct sPAPROptionVector sPAPROptionVector; @@ -64,4 +65,7 @@ sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector); int spapr_ovec_populate_dt(void *fdt, int fdt_offset, sPAPROptionVector *ov, const char *name); +/* migration */ +extern const VMStateDescription vmstate_spapr_ovec; + #endif /* !defined (_SPAPR_OVEC_H) */
With the additional of the OV5_HP_EVT option vector, we now have certain functionality (namely, memory unplug) that checks at run-time for whether or not the guest negotiated the option via CAS. Because we don't currently migrate these negotiated values, we are unable to unplug memory from a guest after it's been migrated until after the guest is rebooted and CAS-negotiation is repeated. This patch fixes this by adding CAS-negotiated options to the migration stream. We do this using a subsection, since the negotiated value of OV5_HP_EVT is the only option currently needed to maintain proper functionality for a running guest. Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++ hw/ppc/spapr_ovec.c | 12 ++++++++ include/hw/ppc/spapr_ovec.h | 4 +++ 3 files changed, 84 insertions(+)