All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: pbonzini@redhat.com, berrange@redhat.com, ehabkost@redhat.com,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH v2 23/24] sd: Hide the qdev-but-not-quite thing created by sd_init()
Date: Thu, 28 May 2020 13:04:43 +0200	[thread overview]
Message-ID: <20200528110444.20456-24-armbru@redhat.com> (raw)
In-Reply-To: <20200528110444.20456-1-armbru@redhat.com>

Commit 260bc9d8aa "hw/sd/sd.c: QOMify" QOMified only the device
itself, not its users.  It kept sd_init() around for non-QOMified
users.

More than four years later, three such users remain: omap1 (machines
cheetah, sx1, sx1-v1) and omap2 (machines n800, n810) are not
QOMified, and pl181 (machines integratorcp, realview-eb,
realview-eb-mpcore, realview-pb-a8 realview-pbx-a9, versatileab,
versatilepb, vexpress-a15, vexpress-a9) is not QOMified properly.

The issue I presently have with this: an "sd-card" device should plug
into an "sd-bus" (its DeviceClass member bus_type says so), but
sd_init() leaves it unplugged.  This is normally a bug (I just fixed
some instances), and I'd like to assert proper pluggedness to prevent
regressions.  However, the qdev-but-not-quite thing returned by
sd_init() would fail the assertion.  Meh.

Make sd_init() hide it from QOM/qdev.  Visible in "info qom-tree",
here's the change for cheetah:

     /machine (cheetah-machine)
       [...]
       /unattached (container)
         [...]
         /device[5] (serial-mm)
           /serial (serial)
           /serial[0] (qemu:memory-region)
    -    /device[6] (sd-card)
    -    /device[7] (omap-gpio)
    +    /device[6] (omap-gpio)
         [rest of device[*] renumbered...]

Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/sd/sd.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3c06a0ac6d..7070a116ea 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -83,6 +83,10 @@ enum SDCardStates {
 struct SDState {
     DeviceState parent_obj;
 
+    /* If true, created by sd_init() for a non-qdevified caller */
+    /* TODO purge them with fire */
+    bool me_no_qdev_me_kill_mammoth_with_rocks;
+
     /* SD Memory Card Registers */
     uint32_t ocr;
     uint8_t scr[8];
@@ -129,6 +133,8 @@ struct SDState {
     bool cmd_line;
 };
 
+static void sd_realize(DeviceState *dev, Error **errp);
+
 static const char *sd_state_name(enum SDCardStates state)
 {
     static const char *state_name[] = {
@@ -590,7 +596,7 @@ static void sd_cardchange(void *opaque, bool load, Error **errp)
 {
     SDState *sd = opaque;
     DeviceState *dev = DEVICE(sd);
-    SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev));
+    SDBus *sdbus;
     bool inserted = sd_get_inserted(sd);
     bool readonly = sd_get_readonly(sd);
 
@@ -601,19 +607,17 @@ static void sd_cardchange(void *opaque, bool load, Error **errp)
         trace_sdcard_ejected();
     }
 
-    /* The IRQ notification is for legacy non-QOM SD controller devices;
-     * QOMified controllers use the SDBus APIs.
-     */
-    if (sdbus) {
-        sdbus_set_inserted(sdbus, inserted);
-        if (inserted) {
-            sdbus_set_readonly(sdbus, readonly);
-        }
-    } else {
+    if (sd->me_no_qdev_me_kill_mammoth_with_rocks) {
         qemu_set_irq(sd->inserted_cb, inserted);
         if (inserted) {
             qemu_set_irq(sd->readonly_cb, readonly);
         }
+    } else {
+        sdbus = SD_BUS(qdev_get_parent_bus(dev));
+        sdbus_set_inserted(sdbus, inserted);
+        if (inserted) {
+            sdbus_set_readonly(sdbus, readonly);
+        }
     }
 }
 
@@ -697,6 +701,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
 {
     Object *obj;
     DeviceState *dev;
+    SDState *sd;
     Error *err = NULL;
 
     obj = object_new(TYPE_SD_CARD);
@@ -707,13 +712,24 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
         return NULL;
     }
     qdev_prop_set_bit(dev, "spi", is_spi);
-    object_property_set_bool(obj, true, "realized", &err);
+
+    /*
+     * Realizing the device properly would put it into the QOM
+     * composition tree even though it is not plugged into an
+     * appropriate bus.  That's a no-no.  Hide the device from
+     * QOM/qdev, and call its qdev realize callback directly.
+     */
+    object_ref(obj);
+    object_unparent(obj);
+    sd_realize(dev, &err);
     if (err) {
         error_reportf_err(err, "sd_init failed: ");
         return NULL;
     }
 
-    return SD_CARD(dev);
+    sd = SD_CARD(dev);
+    sd->me_no_qdev_me_kill_mammoth_with_rocks = true;
+    return sd;
 }
 
 void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
-- 
2.21.3



  parent reply	other threads:[~2020-05-28 11:13 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-28 11:04 [PATCH v2 00/24] Fixes around device realization Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 01/24] arm/stm32f405: Fix realization of "stm32f2xx-adc" devices Markus Armbruster
2020-05-28 11:35   ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 02/24] display/xlnx_dp: Fix to realize "i2c-ddc" and "aux-to-i2c-bridge" Markus Armbruster
2020-06-08 14:16   ` Philippe Mathieu-Daudé
2020-06-09  5:38     ` Markus Armbruster
2020-06-09  7:42       ` Philippe Mathieu-Daudé
2020-06-09  9:35         ` Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 03/24] sd/pxa2xx_mmci: Fix to realize "pxa2xx-mmci" device Markus Armbruster
2020-06-08 14:04   ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 04/24] arm/aspeed: Compute the number of CPUs from the SoC definition Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 05/24] arm/aspeed: Rework NIC attachment Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 06/24] armv7m: Delete unused "ARM,bitband-memory" devices Markus Armbruster
2020-06-08 14:02   ` Philippe Mathieu-Daudé
2020-06-08 14:23   ` [PATCH v2 06/24] armv7m: Delete unused "ARM, bitband-memory" devices Peter Maydell
2020-05-28 11:04 ` [PATCH v2 07/24] auxbus: Fix aux-to-i2c-bridge to be a subtype of aux-slave Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 08/24] mac_via: Fix to realize "mos6522-q800-via*" devices Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 09/24] macio: Fix to realize "mos6522-cuda" and "mos6522-pmu" devices Markus Armbruster
2020-06-08 14:12   ` Philippe Mathieu-Daudé
2020-06-08 14:25   ` Peter Maydell
2020-06-09  7:05     ` Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 10/24] macio: Delete unused "macio-gpio" devices Markus Armbruster
2020-06-08 11:54   ` Mark Cave-Ayland
2020-06-09  5:42     ` Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 11/24] pnv/phb4: Delete unused "pnv-phb4-pec-stack" devices Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 12/24] MAINTAINERS: Make section PowerNV cover pci-host/pnv* as well Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 13/24] ppc4xx: Drop redundant device realization Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 14/24] macio: Put "macio-nvram" device on the macio bus Markus Armbruster
2020-06-08 14:04   ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 15/24] macio: Fix macio-bus to be a subtype of System bus Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 16/24] ppc/pnv: Put "*-pnv-chip" and "pnv-xive" on the main system bus Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 17/24] pnv/psi: Correct the pnv-psi* devices not to be sysbus devices Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 18/24] display/sm501 display/ati: Fix to realize "i2c-ddc" Markus Armbruster
2020-05-28 11:08   ` Aleksandar Markovic
2020-06-08 14:07   ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 19/24] riscv: Fix to put "riscv.hart_array" devices on sysbus Markus Armbruster
2020-05-28 11:04   ` Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 20/24] riscv: Fix type of SiFive[EU]SocState, member parent_obj Markus Armbruster
2020-05-28 11:04   ` Markus Armbruster
2020-05-28 11:04 ` [PATCH v2 21/24] sparc/leon3: Fix to put grlib,* devices on sysbus Markus Armbruster
2020-06-09  5:15   ` Philippe Mathieu-Daudé
2020-06-09  7:20     ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 22/24] qdev: Assert devices are plugged into a bus that can take them Markus Armbruster
2020-05-28 11:04 ` Markus Armbruster [this message]
2020-06-08 14:24   ` [PATCH v2 23/24] sd: Hide the qdev-but-not-quite thing created by sd_init() Philippe Mathieu-Daudé
2020-06-09  6:39     ` Markus Armbruster
2020-06-09  7:38       ` Philippe Mathieu-Daudé
2020-05-28 11:04 ` [PATCH v2 24/24] qdev: Assert onboard devices all get realized properly Markus Armbruster
2020-06-05 15:01 ` [PATCH v2 00/24] Fixes around device realization Markus Armbruster
2020-06-05 20:30   ` Paolo Bonzini
2020-06-08 11:08 ` Markus Armbruster
2020-06-08 11:44   ` Mark Cave-Ayland

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=20200528110444.20456-24-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.