[Qemu-devel] [PATCH v3 2/4] hw/sd: add card-reparenting function

Clement Deschamps posted 4 patches 8 years, 11 months ago
[Qemu-devel] [PATCH v3 2/4] hw/sd: add card-reparenting function
Posted by Clement Deschamps 8 years, 11 months ago
Provide a new function sdbus_reparent_card() in sd core for reparenting
a card from a SDBus to another one.

This function is required by the raspi platform, where the two SD
controllers can be dynamically switched.

Signed-off-by: Clement Deschamps <clement.deschamps@antfield.fr>
---
 hw/sd/core.c       | 30 ++++++++++++++++++++++++++++++
 include/hw/sd/sd.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/hw/sd/core.c b/hw/sd/core.c
index 14c2bdf27b..a8f24f505f 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -131,6 +131,36 @@ void sdbus_set_readonly(SDBus *sdbus, bool readonly)
     }
 }
 
+void sdbus_reparent_card(SDBus *from, SDBus *to)
+{
+    BusChild *kid = QTAILQ_FIRST(&from->qbus.children);
+    SDState *card;
+    SDCardClass *sc;
+    bool readonly;
+
+    /* We directly reparent the card object rather than implementing this
+     * as a hotpluggable connection because we don't want to expose SD cards
+     * to users as being hotpluggable, and we can get away with it in this
+     * limited use case. This could perhaps be implemented more cleanly in
+     * future by adding support to the hotplug infrastructure for "device
+     * can be hotplugged only via code, not by user".
+     */
+
+    if (!kid) {
+        return;
+    }
+
+    card = SD_CARD(kid->child);
+    sc = SD_CARD_GET_CLASS(card);
+    readonly = sc->get_readonly(card);
+
+    sdbus_set_inserted(from, false);
+    object_unparent(OBJECT(kid));
+    qdev_set_parent_bus(DEVICE(card), &to->qbus);
+    sdbus_set_inserted(to, true);
+    sdbus_set_readonly(to, readonly);
+}
+
 static const TypeInfo sd_bus_info = {
     .name = TYPE_SD_BUS,
     .parent = TYPE_BUS,
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 79909b2478..54eaf9b554 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -140,6 +140,7 @@ uint8_t sdbus_read_data(SDBus *sd);
 bool sdbus_data_ready(SDBus *sd);
 bool sdbus_get_inserted(SDBus *sd);
 bool sdbus_get_readonly(SDBus *sd);
+void sdbus_reparent_card(SDBus *from, SDBus *to);
 
 /* Functions to be used by SD devices to report back to qdevified controllers */
 void sdbus_set_inserted(SDBus *sd, bool inserted);
-- 
2.11.1


Re: [Qemu-devel] [PATCH v3 2/4] hw/sd: add card-reparenting function
Posted by Peter Maydell 8 years, 11 months ago
On 24 February 2017 at 16:40, Clement Deschamps
<clement.deschamps@antfield.fr> wrote:
> Provide a new function sdbus_reparent_card() in sd core for reparenting
> a card from a SDBus to another one.
>
> This function is required by the raspi platform, where the two SD
> controllers can be dynamically switched.
>
> Signed-off-by: Clement Deschamps <clement.deschamps@antfield.fr>
> ---
>  hw/sd/core.c       | 30 ++++++++++++++++++++++++++++++
>  include/hw/sd/sd.h |  1 +
>  2 files changed, 31 insertions(+)
>

It turns out we got the logic wrong here -- it causes random
crashes on OSX, and this valgrind error on Linux:

==19695== Invalid read of size 8
==19695==    at 0x7DD59C: object_unparent (object.c:446)
==19695==    by 0x6E920A: sdbus_reparent_card (core.c:158)
==19695==    by 0x3ED0D5: bcm2835_gpio_reset (bcm2835_gpio.c:268)
==19695==    by 0x5DE10E: device_reset (qdev.c:1148)
==19695==    by 0x5DBFA3: qdev_reset_one (qdev.c:295)
==19695==    by 0x5DCBA7: qdev_walk_children (qdev.c:610)
==19695==    by 0x5E0D4A: qbus_walk_children (bus.c:59)
==19695==    by 0x5DC0BD: qbus_reset_all (qdev.c:321)
==19695==    by 0x5DC0E0: qbus_reset_all_fn (qdev.c:327)
==19695==    by 0x5E1690: qemu_devices_reset (reset.c:69)
==19695==    by 0x5258C6: qemu_system_reset (vl.c:1695)
==19695==    by 0x52D915: main (vl.c:4645)
==19695==  Address 0x354c9bc0 is 0 bytes after a block of size 32 alloc'd
==19695==    at 0x4C2FB55: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19695==    by 0x1B7A5770: g_malloc0 (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==19695==    by 0x5DB77C: bus_add_child (qdev.c:85)
==19695==    by 0x5DB8C9: qdev_set_parent_bus (qdev.c:107)
==19695==    by 0x5DBA17: qdev_try_create (qdev.c:153)
==19695==    by 0x5DB8EF: qdev_create (qdev.c:117)
==19695==    by 0x49E3AB: raspi2_init (raspi.c:152)
==19695==    by 0x52D68C: main (vl.c:4523)

I've dropped it (and the following patches) from the pull request
for the moment and will have a look at what the fix is.

thanks
-- PMM