On Wed, 12 Jul 2017 15:53:10 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
> From: Laurent Vivier <lviv...@redhat.com>
>
> When migrating a guest which has already had devices hotplugged,
> libvirt typically starts the destination qemu with -incoming defer,
> adds those hotplugged devices with qmp, then initiates the incoming
> migration.
>
> This causes problems for the management of spapr DRC state. Because
> the device is treated as hotplugged, it goes into a DRC state for a
> device immediately after it's plugged, but before the guest has
> acknowledged its presence. However, chances are the guest on the
> source machine *has* acknowledged the device's presence and configured
> it.
>
> If the source has fully configured the device, then DRC state won't be
> sent in the migration stream: for maximum migration compatibility with
> earlier versions we don't migrate DRCs in coldplug-equivalent state.
> That means that the DRC effectively changes state over the migrate,
> causing problems later on.
>
> In addition, logging hotplug events for these devices isn't what we
> want because a) those events should already have been issued on the
> source host and b) the event queue should get wiped out by the
> incoming state anyway.
>
> In short, what we really want is to treat devices added before an
> incoming migration as if they were coldplugged.
>
> To do this, we first add a spapr_drc_hotplugged() helper which
> determines if the device is hotplugged in the sense relevant for DRC
> state management. We only send hotplug events when this is true.
> Second, when we add a device which isn't hotplugged in this sense, we
> force a reset of the DRC state - this ensures the DRC is in a
> coldplug-equivalent state (there isn't usually a system reset between
> these device adds and the incoming migration).
>
> This is based on an earlier patch by Laurent Vivier, cleaned up and
> extended.
>
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
Reviewed-by: Greg Kurz <gr...@kaod.org>
> hw/ppc/spapr.c | 24 ++++++++++++++++--------
> hw/ppc/spapr_drc.c | 9 ++++++---
> hw/ppc/spapr_pci.c | 4 +++-
> include/hw/ppc/spapr_drc.h | 8 ++++++++
> 4 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 12b3f09..2a059d5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2636,6 +2636,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> addr_start, uint64_t size,
> int i, fdt_offset, fdt_size;
> void *fdt;
> uint64_t addr = addr_start;
> + bool hotplugged = spapr_drc_hotplugged(dev);
> Error *local_err = NULL;
>
> for (i = 0; i < nr_lmbs; i++) {
> @@ -2659,12 +2660,15 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> addr_start, uint64_t size,
> error_propagate(errp, local_err);
> return;
> }
> + if (!hotplugged) {
> + spapr_drc_reset(drc);
> + }
> addr += SPAPR_MEMORY_BLOCK_SIZE;
> }
> /* send hotplug notification to the
> * guest only in case of hotplugged memory
> */
> - if (dev->hotplugged) {
> + if (hotplugged) {
> if (dedicated_hp_event_source) {
> drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
> addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> @@ -2998,6 +3002,7 @@ static void spapr_core_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> int smt = kvmppc_smt_threads();
> CPUArchId *core_slot;
> int index;
> + bool hotplugged = spapr_drc_hotplugged(dev);
>
> core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,
> &index);
> if (!core_slot) {
> @@ -3018,15 +3023,18 @@ static void spapr_core_plug(HotplugHandler
> *hotplug_dev, DeviceState *dev,
> error_propagate(errp, local_err);
> return;
> }
> - }
>
> - if (dev->hotplugged) {
> - /*
> - * Send hotplug notification interrupt to the guest only in case
> - * of hotplugged CPUs.
> - */
> - spapr_hotplug_req_add_by_index(drc);
> + if (hotplugged) {
> + /*
> + * Send hotplug notification interrupt to the guest only
> + * in case of hotplugged CPUs.
> + */
> + spapr_hotplug_req_add_by_index(drc);
> + } else {
> + spapr_drc_reset(drc);
> + }
> }
> +
> core_slot->cpu = OBJECT(dev);
>
> if (smc->pre_2_10_has_unused_icps) {
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index f34355d..9b07f80 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -412,10 +412,8 @@ static bool release_pending(sPAPRDRConnector *drc)
> return drc->awaiting_release;
> }
>
> -static void drc_reset(void *opaque)
> +void spapr_drc_reset(sPAPRDRConnector *drc)
> {
> - sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(opaque);
> -
> trace_spapr_drc_reset(spapr_drc_index(drc));
>
> g_free(drc->ccs);
> @@ -447,6 +445,11 @@ static void drc_reset(void *opaque)
> }
> }
>
> +static void drc_reset(void *opaque)
> +{
> + spapr_drc_reset(SPAPR_DR_CONNECTOR(opaque));
> +}
> +
> static bool spapr_drc_needed(void *opaque)
> {
> sPAPRDRConnector *drc = (sPAPRDRConnector *)opaque;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index a52dcf8..1e84c55 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1443,7 +1443,9 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
> /* If this is function 0, signal hotplug for all the device functions.
> * Otherwise defer sending the hotplug event.
> */
> - if (plugged_dev->hotplugged && PCI_FUNC(pdev->devfn) == 0) {
> + if (!spapr_drc_hotplugged(plugged_dev)) {
> + spapr_drc_reset(drc);
> + } else if (PCI_FUNC(pdev->devfn) == 0) {
> int i;
>
> for (i = 0; i < 8; i++) {
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index d15e9eb..715016b 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -15,6 +15,7 @@
>
> #include <libfdt.h>
> #include "qom/object.h"
> +#include "sysemu/sysemu.h"
> #include "hw/qdev.h"
>
> #define TYPE_SPAPR_DR_CONNECTOR "spapr-dr-connector"
> @@ -223,6 +224,13 @@ typedef struct sPAPRDRConnectorClass {
> bool (*release_pending)(sPAPRDRConnector *drc);
> } sPAPRDRConnectorClass;
>
> +static inline bool spapr_drc_hotplugged(DeviceState *dev)
> +{
> + return dev->hotplugged && !runstate_check(RUN_STATE_INMIGRATE);
> +}
> +
> +void spapr_drc_reset(sPAPRDRConnector *drc);
> +
> uint32_t spapr_drc_index(sPAPRDRConnector *drc);
> sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
>
pgprWG34rv9Gp.pgp
Description: OpenPGP digital signature
