qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: qemu-devel@nongnu.org
Cc: pbonzini@redhat.com, danielhb413@gmail.com, david@gibson.dropbear.id.au
Subject: [PATCH v2] ppc/spapr: cleanup -machine pseries,nvdimm=X handling
Date: Tue,  8 Dec 2020 11:46:06 -0500	[thread overview]
Message-ID: <20201208164606.4109134-1-imammedo@redhat.com> (raw)
In-Reply-To: <20201208110532.4099624-1-imammedo@redhat.com>

Since NVDIMM support was introduced on pseries machine,
it ignored machine's nvdimm=on|off option and effectively
was always enabled on machines that support NVDIMM.
Later on commit
  (28f5a716212 ppc/spapr_nvdimm: do not enable support with 'nvdimm=off')
makes QEMU error out in case user explicitly set 'nvdimm=off'
on CLI by peeking at machine_opts.

However that's a workaround and leaves 'nvdimms_state->is_enabled'
in inconsistent state (false) when it should be set true
by default.

Instead of using on machine_opts, set default to true for pseries
machine in initfn time. If user sets manually 'nvdimm=off'
it will overwrite default value to false and QEMU will error
as expected without need to peek into machine_opts.

That way pseries will have, nvdimm enabled by default and
will honor user provided 'nvdimm=on|off'.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: danielhb413@gmail.com
CC: david@gibson.dropbear.id.au
CC: pbonzini@redhat.com

v2:
  - simplify a bit more by using spapr_instance_init() to set
    default value instead of doing it in generic machine code

PS:
Patch should be applied on top of:
  [PATCH 08/15] machine: introduce MachineInitPhase
---
 hw/ppc/spapr.c        | 13 +++++++++++++
 hw/ppc/spapr_nvdimm.c | 14 +-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b7e0894019..803a6f52a2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3267,6 +3267,19 @@ static void spapr_instance_init(Object *obj)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(obj);
     SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+    MachineState *ms = MACHINE(spapr);
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+    /*
+     * NVDIMM support went live in 5.1 without considering that, in
+     * other archs, the user needs to enable NVDIMM support with the
+     * 'nvdimm' machine option and the default behavior is NVDIMM
+     * support disabled. It is too late to roll back to the standard
+     * behavior without breaking 5.1 guests.
+     */
+    if (mc->nvdimm_supported) {
+        ms->nvdimms_state->is_enabled = true;
+    }
 
     spapr->htab_fd = -1;
     spapr->use_hotplug_event_source = true;
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index a833a63b5e..66cd3dc13f 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -27,10 +27,8 @@
 #include "hw/ppc/spapr_nvdimm.h"
 #include "hw/mem/nvdimm.h"
 #include "qemu/nvdimm-utils.h"
-#include "qemu/option.h"
 #include "hw/ppc/fdt.h"
 #include "qemu/range.h"
-#include "sysemu/sysemu.h"
 #include "hw/ppc/spapr_numa.h"
 
 bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
@@ -38,7 +36,6 @@ bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
 {
     const MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
     const MachineState *ms = MACHINE(hotplug_dev);
-    const char *nvdimm_opt = qemu_opt_get(qemu_get_machine_opts(), "nvdimm");
     g_autofree char *uuidstr = NULL;
     QemuUUID uuid;
     int ret;
@@ -48,16 +45,7 @@ bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
         return false;
     }
 
-    /*
-     * NVDIMM support went live in 5.1 without considering that, in
-     * other archs, the user needs to enable NVDIMM support with the
-     * 'nvdimm' machine option and the default behavior is NVDIMM
-     * support disabled. It is too late to roll back to the standard
-     * behavior without breaking 5.1 guests. What we can do is to
-     * ensure that, if the user sets nvdimm=off, we error out
-     * regardless of being 5.1 or newer.
-     */
-    if (!ms->nvdimms_state->is_enabled && nvdimm_opt) {
+    if (!ms->nvdimms_state->is_enabled) {
         error_setg(errp, "nvdimm device found but 'nvdimm=off' was set");
         return false;
     }
-- 
2.27.0



  reply	other threads:[~2020-12-08 17:08 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02  8:18 [PATCH 00/15] Finish cleaning up qemu_init Paolo Bonzini
2020-12-02  8:18 ` [PATCH 01/15] remove preconfig state Paolo Bonzini
2020-12-07 13:57   ` Igor Mammedov
2020-12-07 14:11     ` Paolo Bonzini
2020-12-07 15:14       ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 02/15] vl: remove separate preconfig main_loop Paolo Bonzini
2020-12-07 14:02   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 03/15] vl: allow -incoming defer with -preconfig Paolo Bonzini
2020-12-02  8:18 ` [PATCH 04/15] vl: extract softmmu/runstate.c Paolo Bonzini
2020-12-02  8:18 ` [PATCH 05/15] vl: extract softmmu/globals.c Paolo Bonzini
2020-12-02  8:18 ` [PATCH 06/15] vl: move all generic initialization out of vl.c Paolo Bonzini
2020-12-07 14:19   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 07/15] chardev: do not use machine_init_done Paolo Bonzini
2020-12-07 15:15   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 08/15] machine: introduce MachineInitPhase Paolo Bonzini
2020-12-07 15:28   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 09/15] machine: record whether nvdimm= was set Paolo Bonzini
2020-12-07 15:40   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 10/15] vl: make qemu_get_machine_opts static Paolo Bonzini
2020-12-07 16:07   ` Igor Mammedov
2020-12-07 16:38     ` Paolo Bonzini
2020-12-08  2:32     ` Daniel Henrique Barboza
2020-12-08 10:55       ` Igor Mammedov
2020-12-08 11:05       ` [PATCH] ppc/spapr: cleanup -machine pseries,nvdimm=X handling Igor Mammedov
2020-12-08 16:46         ` Igor Mammedov [this message]
2020-12-08 17:24           ` [PATCH v2] " Daniel Henrique Barboza
2020-12-08 18:35             ` Igor Mammedov
2020-12-08  2:16   ` [PATCH 10/15] vl: make qemu_get_machine_opts static Daniel Henrique Barboza
2020-12-08  8:13     ` Paolo Bonzini
2020-12-02  8:18 ` [PATCH 11/15] qtest: add a QOM object for qtest Paolo Bonzini
2020-12-07 16:24   ` Igor Mammedov
2020-12-07 16:43     ` Paolo Bonzini
2020-12-07 16:57       ` Igor Mammedov
2020-12-07 17:22         ` Paolo Bonzini
2020-12-08 11:11           ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 12/15] plugin: propagate errors Paolo Bonzini
2020-12-02 11:33   ` Alex Bennée
2020-12-07 16:53   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 13/15] memory: allow creating MemoryRegions before accelerators Paolo Bonzini
2020-12-07 16:38   ` Igor Mammedov
2020-12-07 16:40     ` Paolo Bonzini
2020-12-07 17:06   ` Igor Mammedov
2020-12-02  8:18 ` [PATCH 14/15] null-machine: do not create a default memdev Paolo Bonzini
2020-12-07 16:43   ` Igor Mammedov
2020-12-11 23:24     ` Paolo Bonzini
2020-12-14 11:53       ` Igor Mammedov
2020-12-14 13:24         ` Paolo Bonzini
2020-12-02  8:18 ` [PATCH 15/15] monitor: allow quitting while in preconfig state Paolo Bonzini
2020-12-07 16:45   ` Igor Mammedov
2020-12-07 14:12 ` [PATCH 00/15] Finish cleaning up qemu_init no-reply

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=20201208164606.4109134-1-imammedo@redhat.com \
    --to=imammedo@redhat.com \
    --cc=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@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).