From 351d24dd01468e63230c68aa27adfebbaf052f3e Mon Sep 17 00:00:00 2001

From: Serhii Popovych <spopovyc@redhat.com>

Date: Thu, 11 Jan 2018 10:37:46 +0100

Subject: [PATCH 05/12] hw/ppc/spapr.c: abort unplug_request if previous unplug

 isn't done

RH-Author: Serhii Popovych <spopovyc@redhat.com>

Message-id: <1515667066-41734-1-git-send-email-spopovyc@redhat.com>

Patchwork-id: 78547

O-Subject: [RHV7.5 qemu-kvm-rhev PATCH] hw/ppc/spapr.c: abort unplug_request if previous unplug isn't done

Bugzilla: 1528173

RH-Acked-by: Laurent Vivier <lvivier@redhat.com>

RH-Acked-by: David Gibson <dgibson@redhat.com>

RH-Acked-by: Thomas Huth <thuth@redhat.com>

From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

LMB removal is completed only when the spapr_lmb_release callback

is called after all DRCs of the dimm are detached. During this

time, it is possible that a unplug request for the same dimm

arrives, trying to detach DRCs that were detached by the guest

in the first unplug_request.

BQL doesn't help in this case - the lock will prevent any concurrent

removal from happening until the end of spapr_memory_unplug_request

only. What happens is that the second unplug_request ends up calling

spapr_drc_detach in a DRC that were detached already, causing an

assert error in spapr_drc_detach (e.g

https://bugs.launchpad.net/qemu/+bug/1718118).

spapr_lmb_release uses a structure called sPAPRDIMMState, stored in the

spapr->pending_dimm_unplugs QTAIL, to track how many LMB DRCs are left

to be detached by the guest. When there are no more DRCs left, this

structure is deleted and the pc-dimm unplug handler is called to

finish the process.

This patch reuses the sPAPRDIMMState to allow unplug_request to know

if there is an ongoing unplug process for a given dimm, aborting the

unplug request in this case, by doing the following changes:

- in spapr_lmb_release callback, move the dimm state removal to the

end, after pc-dimm unplug handler. With this change we can check for

the existence of the dimm state to see if the unplug process is

done.

- use spapr_pending_dimm_unplugs_find in spapr_memory_unplug_request

to check if the dimm state exists. If positive, there is an unplug

operation already in progress for this dimm, meaning that we should

abort it and warn the user about it.

Fixes: https://bugs.launchpad.net/qemu/+bug/1718118

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

(cherry picked from commit 2a129767ebb13ffc29dad6a8e8e6eec06dc38b25)

Signed-off-by: Serhii Popovych <spopovyc@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 hw/ppc/spapr.c | 16 ++++++++++++++--

 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

index d7fac62..e276632 100644

--- a/hw/ppc/spapr.c

+++ b/hw/ppc/spapr.c

@@ -3070,14 +3070,13 @@ void spapr_lmb_release(DeviceState *dev)

         return;

     }

-    spapr_pending_dimm_unplugs_remove(spapr, ds);

-

     /*

      * Now that all the LMBs have been removed by the guest, call the

      * pc-dimm unplug handler to cleanup up the pc-dimm device.

      */

     pc_dimm_memory_unplug(dev, &spapr->hotplug_memory, mr);

     object_unparent(OBJECT(dev));

+    spapr_pending_dimm_unplugs_remove(spapr, ds);

 }

 static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev,

@@ -3106,6 +3105,19 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev,

         goto out;

     }

+    /*

+     * An existing pending dimm state for this DIMM means that there is an

+     * unplug operation in progress, waiting for the spapr_lmb_release

+     * callback to complete the job (BQL can't cover that far). In this case,

+     * bail out to avoid detaching DRCs that were already released.

+     */

+    if (spapr_pending_dimm_unplugs_find(spapr, dimm)) {

+        error_setg(&local_err,

+                   "Memory unplug already in progress for device %s",

+                   dev->id);

+        goto out;

+    }

+

     spapr_pending_dimm_unplugs_add(spapr, nr_lmbs, dimm);

     addr = addr_start;

-- 

1.8.3.1