qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: qemu-devel@nongnu.org
Cc: Igor Mammedov <imammedo@redhat.com>,
	qemu-ppc@nongnu.org, Greg Kurz <groug@kaod.org>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH for-6.0 v2 2/4] spapr: Abort if ppc_set_compat() fails for hot-plugged CPUs
Date: Tue,  1 Dec 2020 12:37:26 +0100	[thread overview]
Message-ID: <20201201113728.885700-3-groug@kaod.org> (raw)
In-Reply-To: <20201201113728.885700-1-groug@kaod.org>

When a CPU is hot-plugged, we set its compat mode to match the boot
CPU, which was either set by machine reset or by CAS. This is currently
handled in the plug handler after the core got realized. Potential errors
of ppc_set_compat() are propagated to the hot-plug logic.

Handling errors this late in the hot-plug sequence is generally frown
upon. Ideally, we should do sanity checks in a pre-plug handler and pass
&error_abort to ppc_set_compat() in the plug handler.

We can filter out some error cases of ppc_set_compat() by calling
ppc_check_compat() at pre-plug. But ppc_set_compat() also sets the
compat register in KVM, and KVM doesn't provide any API that would
allow to check valid compat mode settings beforehand.

However, at this point we know that the compat mode was already
successfully set for the boot CPU. Since this all boils down to
setting a register with the very same value that was valid
for the boot CPU, it should definitely not fail for hot-plugged
CPUS.

Pass &error_abort to ppc_set_compat().

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 57c6eecc56d6..3a32918dd9a9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3787,15 +3787,13 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
 
     /*
      * Set compatibility mode to match the boot CPU, which was either set
-     * by the machine reset code or by CAS.
+     * by the machine reset code or by CAS. This really shouldn't fail at
+     * this point.
      */
     if (hotplugged) {
         for (i = 0; i < cc->nr_threads; i++) {
-            if (ppc_set_compat(core->threads[i],
-                               POWERPC_CPU(first_cpu)->compat_pvr,
-                               errp) < 0) {
-                return;
-            }
+            ppc_set_compat(core->threads[i], POWERPC_CPU(first_cpu)->compat_pvr,
+                           &error_abort);
         }
     }
 
-- 
2.26.2



  parent reply	other threads:[~2020-12-01 11:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-01 11:37 [PATCH for-6.0 v2 0/4] spapr: Perform hotplug sanity checks at pre-plug Greg Kurz
2020-12-01 11:37 ` [PATCH for-6.0 v2 1/4] spapr: Fix pre-2.10 dummy ICP hack Greg Kurz
2020-12-01 11:37 ` Greg Kurz [this message]
2020-12-01 11:37 ` [PATCH for-6.0 v2 3/4] spapr: Simplify error path of spapr_core_plug() Greg Kurz
2020-12-01 11:37 ` [PATCH for-6.0 v2 4/4] spapr: spapr_drc_attach() cannot fail Greg Kurz
2020-12-02  3:16 ` [PATCH for-6.0 v2 0/4] spapr: Perform hotplug sanity checks at pre-plug David Gibson

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=20201201113728.885700-3-groug@kaod.org \
    --to=groug@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=imammedo@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /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).