On Mon, Apr 25, 2016 at 05:24:25PM -0500, Michael Roth wrote: > CPU/memory resources can be signalled en-masse via > spapr_hotplug_req_add_by_count(), and when doing so, actually change > the meaning of the 'drc' parameter passed to > spapr_hotplug_req_event() to be a count rather than an index. > > f40eb92 added a hook in spapr_hotplug_req_event() to record when a > device had been 'signalled' to the guest, but that code assumes that > drc is always an index. In cases where it's a count, such as memory > hotplug, the DRC lookup will fail, leading to an assert. > > Fix this by only explicitly setting the signalled state for cases where > we are doing PCI hotplug. > > For other resources types, since we cannot selectively track whether a > resource has been signalled in cases where we signal attach as a count, > set the 'signalled' state to true immediately upon making the > resource available via drck->attach(). > > Reported-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > Cc: Bharata B Rao <bhar...@linux.vnet.ibm.com> > Cc: da...@gibson.dropbear.id.au > Cc: qemu-...@nongnu.org > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
Applued to 2.6, I'll send a pull request today.
> ---
> Really sorry for the way last-minute fix, but without this memory hotplug
> is totally broken :( Hoping to get this in for Wednesday's RC4, which
> I think will be the final before release.
> ---
> hw/ppc/spapr_drc.c | 12 +++++++++++-
> hw/ppc/spapr_events.c | 7 +++----
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 3173940..1f5f1d7 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -364,7 +364,17 @@ static void attach(sPAPRDRConnector *drc, DeviceState
> *d, void *fdt,
> drc->fdt = fdt;
> drc->fdt_start_offset = fdt_start_offset;
> drc->configured = coldplug;
> - drc->signalled = coldplug;
> + /* 'logical' DR resources such as memory/cpus are in some cases treated
> + * as a pool of resources from which the guest is free to choose from
> + * based on only a count. for resources that can be assigned in this
> + * fashion, we must assume the resource is signalled immediately
> + * since a single hotplug request might make an arbitrary number of
> + * such attached resources available to the guest, as opposed to
> + * 'physical' DR resources such as PCI where each device/resource is
> + * signalled individually.
> + */
> + drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI)
> + ? true : coldplug;
>
> object_property_add_link(OBJECT(drc), "device",
> object_get_typename(OBJECT(drc->dev)),
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 269ab7e..049fb1b 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -442,6 +442,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id,
> uint8_t hp_action,
> switch (drc_type) {
> case SPAPR_DR_CONNECTOR_TYPE_PCI:
> hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PCI;
> + if (hp->hotplug_action == RTAS_LOG_V6_HP_ACTION_ADD) {
> + spapr_hotplug_set_signalled(drc);
> + }
> break;
> case SPAPR_DR_CONNECTOR_TYPE_LMB:
> hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_MEMORY;
> @@ -462,10 +465,6 @@ static void spapr_hotplug_req_event(uint8_t hp_id,
> uint8_t hp_action,
>
> rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
>
> - if (hp->hotplug_action == RTAS_LOG_V6_HP_ACTION_ADD) {
> - spapr_hotplug_set_signalled(drc);
> - }
> -
> qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
