qemu-devel.nongnu.org archive mirror
 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 v3 23/24] sd: Hide the qdev-but-not-quite thing created by sd_init()
Date: Tue,  9 Jun 2020 14:23:38 +0200	[thread overview]
Message-ID: <20200609122339.937862-24-armbru@redhat.com> (raw)
In-Reply-To: <20200609122339.937862-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.26.2



  parent reply	other threads:[~2020-06-09 12:29 UTC|newest]

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

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=20200609122339.937862-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 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).