[PATCH] aspeed/smc: Only wire flash devices at reset

[PATCH] aspeed/smc: Only wire flash devices at reset

[Cédric Le Goater]

The Aspeed machines have many Static Memory Controllers (SMC), up to
8, which can only drive flash memory devices. Commit 27a2c66c92ec
("aspeed/smc: Wire CS lines at reset") tried to ease the definitions
of these devices by allowing flash devices from the command line to be
attached to a SSI bus. For that, the wiring of the CS lines of the
Aspeed SMC controller was moved at reset. Two assumptions are made
though, first that the device has a SSI_GPIO_CS GPIO line, which is
not always the case, and second that it is flash device.

Correct this problem by ensuring that the devices attached to the bus
are the correct flash type. This fixes a QEMU abort when devices
without a CS line, such as the max111x, are passed on the command
line.

While at it, export TYPE_M25P80 used in the Xilinx Versal Virtual
machine.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2228
Fixes: 27a2c66c92ec ("aspeed/smc: Wire CS lines at reset")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/block/flash.h  | 2 ++
 hw/arm/xlnx-versal-virt.c | 3 ++-
 hw/block/m25p80.c         | 1 -
 hw/ssi/aspeed_smc.c       | 9 +++++++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index de93756cbe8f..2b5ccd92f463 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -78,6 +78,8 @@ extern const VMStateDescription vmstate_ecc_state;
 
 /* m25p80.c */
 
+#define TYPE_M25P80 "m25p80-generic"
+
 BlockBackend *m25p80_get_blk(DeviceState *dev);
 
 #endif
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index bfaed1aebfc6..962f98fee2ea 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -13,6 +13,7 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "sysemu/device_tree.h"
+#include "hw/block/flash.h"
 #include "hw/boards.h"
 #include "hw/sysbus.h"
 #include "hw/arm/fdt.h"
@@ -759,7 +760,7 @@ static void versal_virt_init(MachineState *machine)
             flash_klass = object_class_by_name(s->ospi_model);
             if (!flash_klass ||
                 object_class_is_abstract(flash_klass) ||
-                !object_class_dynamic_cast(flash_klass, "m25p80-generic")) {
+                !object_class_dynamic_cast(flash_klass, TYPE_M25P80)) {
                 error_setg(&error_fatal, "'%s' is either abstract or"
                        " not a subtype of m25p80", s->ospi_model);
                 return;
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 08a00a6d9b89..8dec134832a1 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -515,7 +515,6 @@ struct M25P80Class {
     FlashPartInfo *pi;
 };
 
-#define TYPE_M25P80 "m25p80-generic"
 OBJECT_DECLARE_TYPE(Flash, M25P80Class, M25P80)
 
 static inline Manufacturer get_man(Flash *s)
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 0dff1d910778..de57e690e124 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/block/flash.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
@@ -711,6 +712,14 @@ static void aspeed_smc_reset(DeviceState *d)
     for (i = 0; i < asc->cs_num_max; i++) {
         DeviceState *dev = ssi_get_cs(s->spi, i);
         if (dev) {
+            Object *o = OBJECT(dev);
+
+            if (!object_dynamic_cast(o, TYPE_M25P80)) {
+                warn_report("Aspeed SMC %s.%d : Invalid %s device type",
+                            BUS(s->spi)->name, i, object_get_typename(o));
+                continue;
+            }
+
             qemu_irq cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
             qdev_connect_gpio_out_named(DEVICE(s), "cs", i, cs_line);
         }
-- 
2.44.0

Re: [PATCH] aspeed/smc: Only wire flash devices at reset

[Thomas Huth]

On 19/03/2024 08.33, Cédric Le Goater wrote:
> The Aspeed machines have many Static Memory Controllers (SMC), up to
> 8, which can only drive flash memory devices. Commit 27a2c66c92ec
> ("aspeed/smc: Wire CS lines at reset") tried to ease the definitions
> of these devices by allowing flash devices from the command line to be
> attached to a SSI bus. For that, the wiring of the CS lines of the
> Aspeed SMC controller was moved at reset. Two assumptions are made
> though, first that the device has a SSI_GPIO_CS GPIO line, which is
> not always the case, and second that it is flash device.
> 
> Correct this problem by ensuring that the devices attached to the bus
> are the correct flash type. This fixes a QEMU abort when devices
> without a CS line, such as the max111x, are passed on the command
> line.
> 
> While at it, export TYPE_M25P80 used in the Xilinx Versal Virtual
> machine.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2228
> Fixes: 27a2c66c92ec ("aspeed/smc: Wire CS lines at reset")
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---

Thanks!

Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>