Re: [Qemu-devel] [PATCH for-2.6] spapr_drc: fix aborts during DRC-count based hotplug

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4001 bytes --]

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 <bharata@linux.vnet.ibm.com>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Cc: david@gibson.dropbear.id.au
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Michael Roth <mdroth@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]