[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH for-2.6] spapr_drc: fix aborts during DRC-count ba
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH for-2.6] spapr_drc: fix aborts during DRC-count based hotplug |
Date: |
Tue, 26 Apr 2016 11:18:25 +1000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 <address@hidden>
> Cc: Bharata B Rao <address@hidden>
> Cc: address@hidden
> Cc: address@hidden
> Signed-off-by: Michael Roth <address@hidden>
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