qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: qemu-devel@nongnu.org
Cc: Xujun Ma <xuma@redhat.com>,
	Daniel Henrique Barboza <danielhb413@gmail.com>,
	qemu-ppc@nongnu.org, groug@kaod.org, david@gibson.dropbear.id.au
Subject: [PATCH v1 7/7] spapr.c: consider CPU core online state before allowing unplug
Date: Thu, 14 Jan 2021 15:06:28 -0300	[thread overview]
Message-ID: <20210114180628.1675603-8-danielhb413@gmail.com> (raw)
In-Reply-To: <20210114180628.1675603-1-danielhb413@gmail.com>

The only restriction we have when unplugging CPUs is to forbid unplug of
the boot cpu core. spapr_core_unplug_possible() does not contemplate the
possibility of some cores being offlined by the guest, meaning that we're
rolling the dice regarding on whether we're unplugging the last online
CPU core the guest has.

If we hit the jackpot, we're going to detach the core DRC and pulse the
hotplug IRQ, but the guest OS will refuse to release the CPU. Our
spapr_core_unplug() DRC release callback will never be called and the CPU
core object will keep existing in QEMU. No error message will be sent
to the user, but the CPU core wasn't unplugged from the guest.

If the guest OS onlines the CPU core again we won't be able to hotunplug it
either. 'dmesg' inside the guest will report a failed attempt to offline an
unknown CPU:

[  923.003994] pseries-hotplug-cpu: Failed to offline CPU <NULL>, rc: -16

This is the result of stopping the DRC state transition in the middle in the
first failed attempt.

We can avoid this, and potentially other bad things from happening, if we
avoid to attempt the unplug altogether in this scenario. Let's check for
the online/offline state of the CPU cores in the guest before allowing
the hotunplug, and forbid removing a CPU core if it's the last one online
in the guest.

Reported-by: Xujun Ma <xuma@redhat.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1911414
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a2f01c21aa..d269dcd102 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3709,9 +3709,16 @@ static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
 static int spapr_core_unplug_possible(HotplugHandler *hotplug_dev, CPUCore *cc,
                                       Error **errp)
 {
+    CPUArchId *core_slot;
+    SpaprCpuCore *core;
+    PowerPCCPU *cpu;
+    CPUState *cs;
+    bool last_cpu_online = true;
     int index;
 
-    if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index)) {
+    core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,
+                                    &index);
+    if (!core_slot) {
         error_setg(errp, "Unable to find CPU core with core-id: %d",
                    cc->core_id);
         return -1;
@@ -3722,6 +3729,36 @@ static int spapr_core_unplug_possible(HotplugHandler *hotplug_dev, CPUCore *cc,
         return -1;
     }
 
+    /* Allow for any non-boot CPU core to be unplugged if already offline */
+    core = SPAPR_CPU_CORE(core_slot->cpu);
+    cs = CPU(core->threads[0]);
+    if (cs->halted) {
+        return 0;
+    }
+
+    /*
+     * Do not allow core unplug if it's the last core online.
+     */
+    cpu = POWERPC_CPU(cs);
+    CPU_FOREACH(cs) {
+        PowerPCCPU *c = POWERPC_CPU(cs);
+
+        if (c == cpu) {
+            continue;
+        }
+
+        if (!cs->halted) {
+            last_cpu_online = false;
+            break;
+        }
+    }
+
+    if (last_cpu_online) {
+        error_setg(errp, "Unable to unplug CPU core with core-id %d: it is "
+                   "the only CPU core online in the guest", cc->core_id);
+        return -1;
+    }
+
     return 0;
 }
 
-- 
2.26.2



  parent reply	other threads:[~2021-01-14 18:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14 18:06 [PATCH v1 0/7] pseries: avoid unplug the last online CPU core + assorted fixes Daniel Henrique Barboza
2021-01-14 18:06 ` [PATCH v1 1/7] spapr.h: fix trailing whitespace in phb_placement Daniel Henrique Barboza
2021-01-15  0:42   ` David Gibson
2021-01-14 18:06 ` [PATCH v1 2/7] spapr_hcall.c: make do_client_architecture_support static Daniel Henrique Barboza
2021-01-15  0:43   ` David Gibson
2021-01-14 18:06 ` [PATCH v1 3/7] spapr_rtas.c: fix identation in rtas_ibm_nmi_interlock() string Daniel Henrique Barboza
2021-01-15  0:44   ` David Gibson
2021-01-14 18:06 ` [PATCH v1 4/7] spapr_rtas.c: fix identation of rtas_ibm_suspend_me() args Daniel Henrique Barboza
2021-01-15  0:45   ` David Gibson
2021-01-14 18:06 ` [PATCH v1 5/7] spapr_cpu_core.c: use g_auto* in spapr_create_vcpu() Daniel Henrique Barboza
2021-01-15  0:49   ` David Gibson
2021-01-14 18:06 ` [PATCH v1 6/7] spapr.c: introduce spapr_core_unplug_possible() Daniel Henrique Barboza
2021-01-15  0:52   ` David Gibson
2021-01-14 18:06 ` Daniel Henrique Barboza [this message]
2021-01-15  1:03   ` [PATCH v1 7/7] spapr.c: consider CPU core online state before allowing unplug David Gibson
2021-01-15 17:22   ` Greg Kurz
2021-01-15 18:52     ` Daniel Henrique Barboza
2021-01-18  1:18       ` David Gibson
2021-01-18 10:28         ` Daniel Henrique Barboza
2021-01-15 21:43     ` Daniel Henrique Barboza
2021-01-15 21:43     ` Daniel Henrique Barboza
2021-01-18  1:12     ` David Gibson
2021-01-18 11:03       ` Greg Kurz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210114180628.1675603-8-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=xuma@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).