[Qemu-devel] [PATCH] hw/arm/raspi: avoid reparenting the sd card during qbus tree reset

[Damien Hedde]

In the raspi machine, the sd card can be on several sd bus (in reality
there is one bus but several controllers). It is initially created in
the "sd-bus" child in the gpio peripheral. Then is moved (parent bus
changes) during machine reset in the "sdhci-bus". It can be moved again
by software between the "sdhci-bus" and another bus ("bcm2835-sdhost-bus").
Here's the corresponding qbus tree of the raspi machine:
 + sysbus
   * bcm2835_gpio
     + sd-bus
       * sd-card
   * bcm2835-sdhost
     + bcm2835-sdhost-bus
   * generic-sdhci
     + sdhci-bus

During the initial machine reset, the sd card is moved. Since reset is
based on qbus tree, moving the card during the reset seems odd (it changes
the qbus tree). In this case, because of the order the qbus tree is
walked, the sd card ends up being reset twice; the effective reset order call
follows:
 1 sd-card
 2 sd-bus
 3 bcm2835_gpio        -> move the sd-card to sdhci_bus
 4 bcm2835-sdhost-bus
 5 bcm2835-sdhost
 6 sd-card             (again)
 7 sdhci-bus
 8 generic-sdhci

This patch adds a raspi machine reset method which moves the sd card
to the sdhci-bus before doing the whole reset (which will try to do the
move too). By anticipating the move we avoid changing the qdev tree while
resetting it.

In consequence the step 1 is skipped in the previous list: when reset starts
the sd-card is already not a child of bcm2835_gpio.

Signed-off-by: Damien Hedde <address@hidden>
---
This is a follow-up of this discussion
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg06954.html

I did not have much comments about my last proposal so I thought I'd finalize
a corresponding patch since it is small instead of doing a ping.

Feel free to comment,
Damien
---
 hw/arm/raspi.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 74c062d05e..19b032546b 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -21,6 +21,7 @@
 #include "hw/loader.h"
 #include "hw/arm/boot.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/reset.h"
 
 #define SMPBOOT_ADDR    0x300 /* this should leave enough space for ATAGS */
 #define MVBAR_ADDR      0x400 /* secure vectors */
@@ -214,6 +215,32 @@ static void raspi_init(MachineState *machine, int version)
     setup_boot(machine, version, machine->ram_size - vcram_size);
 }
 
+static void raspi_reset(MachineState *machine)
+{
+    BCM2835GpioState *gpio;
+
+    gpio = BCM2835_GPIO(object_resolve_path("gpio", NULL));
+
+    /*
+     * Put the sd-card on sdhci bus as the bcm2835_gpio's reset
+     * procedure will do during qemu_devices_reset().
+     *
+     * Note: we do this here to avoid doing it during following
+     * qemu_devices_reset() so that we don't modify the qbus tree during the
+     * reset (which is based on it).
+     *
+     * Note: sd-card can be on sdbus, sdbus_sdhci or sdbus_sdhost. So at least
+     * one of the following sdbus_reparent_card is useless. This is not a
+     * problem because sdbus_reparent_card is a no-op if the source does not
+     * has a card.
+     */
+    sdbus_reparent_card(&gpio->sdbus, gpio->sdbus_sdhci);
+    sdbus_reparent_card(gpio->sdbus_sdhost, gpio->sdbus_sdhci);
+
+    /* then do the classical reset */
+    qemu_devices_reset();
+}
+
 static void raspi2_init(MachineState *machine)
 {
     raspi_init(machine, 2);
@@ -223,6 +250,7 @@ static void raspi2_machine_init(MachineClass *mc)
 {
     mc->desc = "Raspberry Pi 2";
     mc->init = raspi2_init;
+    mc->reset = raspi_reset;
     mc->block_default_type = IF_SD;
     mc->no_parallel = 1;
     mc->no_floppy = 1;
@@ -245,6 +273,7 @@ static void raspi3_machine_init(MachineClass *mc)
 {
     mc->desc = "Raspberry Pi 3";
     mc->init = raspi3_init;
+    mc->reset = raspi_reset;
     mc->block_default_type = IF_SD;
     mc->no_parallel = 1;
     mc->no_floppy = 1;
-- 
2.22.0