spapr: Don't misuse DR-indicator in spapr_recover_pending_dimm_state()
With some combinations of migration and hotplug we can lost temporary state indicating how many DRCs (guest side hotplug handles) are still connected to a DIMM object in the process of removal. When we hit that situation spapr_recover_pending_dimm_state() is used to scan more extensively and work out the right number. It does this using drc->indicator state to determine what state of disconnection the DRC is in. However, this is not safe, because the indicator state is guest settable - in fact it's more-or-less a purely guest->host notification mechanism which should have no bearing on the internals of hotplug state management. So, replace the test for this with a test on drc->dev, which is a purely qemu side managed variable, and updated the same BQL critical section as the indicator state. This does introduce an off-by-one change, because the indicator state was updated before the call to spapr_lmb_release() on the current DRC, whereas drc->dev is updated afterwards. That's corrected by always decrementing the nr_lmbs value instead of only doing so in the case where we didn't have to recover information. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
This commit is contained in:
parent
f224d35be9
commit
454b580ae9
|
@ -2676,7 +2676,7 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,
|
||||||
drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
|
drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
|
||||||
addr / SPAPR_MEMORY_BLOCK_SIZE);
|
addr / SPAPR_MEMORY_BLOCK_SIZE);
|
||||||
g_assert(drc);
|
g_assert(drc);
|
||||||
if (drc->indicator_state != SPAPR_DR_INDICATOR_STATE_INACTIVE) {
|
if (drc->dev) {
|
||||||
avail_lmbs++;
|
avail_lmbs++;
|
||||||
}
|
}
|
||||||
addr += SPAPR_MEMORY_BLOCK_SIZE;
|
addr += SPAPR_MEMORY_BLOCK_SIZE;
|
||||||
|
@ -2700,10 +2700,11 @@ void spapr_lmb_release(DeviceState *dev)
|
||||||
* during the unplug process. In this case recover it. */
|
* during the unplug process. In this case recover it. */
|
||||||
if (ds == NULL) {
|
if (ds == NULL) {
|
||||||
ds = spapr_recover_pending_dimm_state(spapr, PC_DIMM(dev));
|
ds = spapr_recover_pending_dimm_state(spapr, PC_DIMM(dev));
|
||||||
if (ds->nr_lmbs) {
|
/* The DRC being examined by the caller at least must be counted */
|
||||||
return;
|
g_assert(ds->nr_lmbs);
|
||||||
}
|
}
|
||||||
} else if (--ds->nr_lmbs) {
|
|
||||||
|
if (--ds->nr_lmbs) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue