diff options
author | Bharata B Rao <bharata@linux.vnet.ibm.com> | 2017-07-21 10:21:06 +0530 |
---|---|---|
committer | David Gibson <david@gibson.dropbear.id.au> | 2017-07-25 11:14:25 +1000 |
commit | 8d5981c4fc1342829b23666b7e7b28b4c2ce1d8a (patch) | |
tree | bae63ad39659cbbd759e5b2d5663be0c4e89c36e /hw/ppc | |
parent | e8cd4247e96bb2158ef0ae0ff20e72746b9dd32d (diff) |
spapr: Fix QEMU abort during memory unplug
Commit 0cffce56 (hw/ppc/spapr.c: adding pending_dimm_unplugs to
sPAPRMachineState) introduced a new way to track pending LMBs of DIMM
device that is marked for removal. Since this commit we can hit the
assert in spapr_pending_dimm_unplugs_add() in the following situation:
- DIMM device removal fails as the guest doesn't allow the removal.
- Subsequent attempt to remove the same DIMM would hit the assert
as the corresponding sPAPRDIMMState is still part of the
pending_dimm_unplugs list.
Fix this by removing the assert and conditionally adding the
sPAPRDIMMState to pending_dimm_unplugs list only when it is not
already present.
Fixes: 0cffce56ae3501c5783d779f97993ce478acf856
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
[dwg: Tweaked to avoid returning NULL when spapr_pending_dimm_unplugs_add()
does find an existing entry]
Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Diffstat (limited to 'hw/ppc')
-rw-r--r-- | hw/ppc/spapr.c | 38 |
1 files changed, 22 insertions, 16 deletions
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 1cb09e7c64..2a3e53d5d5 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2850,11 +2850,26 @@ static sPAPRDIMMState *spapr_pending_dimm_unplugs_find(sPAPRMachineState *s, return dimm_state; } -static void spapr_pending_dimm_unplugs_add(sPAPRMachineState *spapr, - sPAPRDIMMState *dimm_state) +static sPAPRDIMMState *spapr_pending_dimm_unplugs_add(sPAPRMachineState *spapr, + uint32_t nr_lmbs, + PCDIMMDevice *dimm) { - g_assert(!spapr_pending_dimm_unplugs_find(spapr, dimm_state->dimm)); - QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, dimm_state, next); + sPAPRDIMMState *ds = NULL; + + /* + * If this request is for a DIMM whose removal had failed earlier + * (due to guest's refusal to remove the LMBs), we would have this + * dimm already in the pending_dimm_unplugs list. In that + * case don't add again. + */ + ds = spapr_pending_dimm_unplugs_find(spapr, dimm); + if (!ds) { + ds = g_malloc0(sizeof(sPAPRDIMMState)); + ds->nr_lmbs = nr_lmbs; + ds->dimm = dimm; + QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, ds, next); + } + return ds; } static void spapr_pending_dimm_unplugs_remove(sPAPRMachineState *spapr, @@ -2875,7 +2890,6 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms, uint32_t avail_lmbs = 0; uint64_t addr_start, addr; int i; - sPAPRDIMMState *ds; addr_start = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); @@ -2891,11 +2905,7 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms, addr += SPAPR_MEMORY_BLOCK_SIZE; } - ds = g_malloc0(sizeof(sPAPRDIMMState)); - ds->nr_lmbs = avail_lmbs; - ds->dimm = dimm; - spapr_pending_dimm_unplugs_add(ms, ds); - return ds; + return spapr_pending_dimm_unplugs_add(ms, avail_lmbs, dimm); } /* Callback to be called during DRC release. */ @@ -2911,6 +2921,7 @@ void spapr_lmb_release(DeviceState *dev) * during the unplug process. In this case recover it. */ if (ds == NULL) { ds = spapr_recover_pending_dimm_state(spapr, PC_DIMM(dev)); + g_assert(ds); /* The DRC being examined by the caller at least must be counted */ g_assert(ds->nr_lmbs); } @@ -2942,18 +2953,13 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, uint64_t addr_start, addr; int i; sPAPRDRConnector *drc; - sPAPRDIMMState *ds; - addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, &local_err); if (local_err) { goto out; } - ds = g_malloc0(sizeof(sPAPRDIMMState)); - ds->nr_lmbs = nr_lmbs; - ds->dimm = dimm; - spapr_pending_dimm_unplugs_add(spapr, ds); + spapr_pending_dimm_unplugs_add(spapr, nr_lmbs, dimm); addr = addr_start; for (i = 0; i < nr_lmbs; i++) { |