qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/14] Misc HW patches for 2025-03-11
@ 2025-03-11 19:51 Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 01/14] hw/sd/sdhci: Set reset value of interrupt registers Philippe Mathieu-Daudé
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

The following changes since commit 825b96dbcee23d134b691fc75618b59c5f53da32:

  Merge tag 'migration-20250310-pull-request' of https://gitlab.com/farosas/qemu into staging (2025-03-11 09:32:07 +0800)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/hw-misc-20250311

for you to fetch changes up to a5368f2e00c81c8c2b5dd0318293b11f8ed7c7c8:

  hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition (2025-03-11 20:03:30 +0100)

----------------------------------------------------------------
Misc HW patches

- Set correct values for MPC8569E's eSDHC (Zoltan)
- Emulate Ricoh RS5C372 RTC device (Bernhard)
- Array overflow fixes in SMSC91C111 netdev (Peter)
- Fix typo in Xen HVM (Philippe)
- Move graphic height/width/depth globals to their own file (Philippe)
- Introduce qemu_arch_available() helper (Philippe)
- Check fw_cfg's ACPI availability at runtime (Philippe)
- Remove virtio-mem dependency on CONFIG_DEVICES (Philippe)
- Sort HyperV SYNDBG API definitions (Pierrick)
- Remove need for SDHCI_VENDOR_FSL definition (Philippe)

----------------------------------------------------------------

BALATON Zoltan (1):
  hw/sd/sdhci: Set reset value of interrupt registers

Bernhard Beschow (1):
  hw/rtc: Add Ricoh RS5C372 RTC emulation

Peter Maydell (4):
  hw/net/smc91c111: Sanitize packet numbers
  hw/net/smc91c111: Sanitize packet length on tx
  hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers
  hw/net/smc91c111: Don't allow data register access to overrun buffer

Philippe Mathieu-Daudé (7):
  hw/xen/hvm: Fix Aarch64 typo
  system: Extract target-specific globals to their own compilation unit
  system: Replace arch_type global by qemu_arch_available() helper
  hw/acpi: Introduce acpi_builtin() helper
  hw/i386/fw_cfg: Check ACPI availability with acpi_builtin()
  hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition

Pierrick Bouvier (1):
  hw/hyperv/hyperv-proto: Move SYNDBG definitions from target/i386

 MAINTAINERS                      |   2 +
 include/hw/acpi/acpi.h           |   3 +
 include/hw/hyperv/hyperv-proto.h |  12 ++
 include/hw/sd/sdhci.h            |   2 +-
 include/hw/xen/arch_hvm.h        |   2 +-
 include/system/arch_init.h       |   2 +-
 target/i386/kvm/hyperv-proto.h   |  12 --
 hw/acpi/acpi-stub.c              |   8 ++
 hw/acpi/core.c                   |   5 +
 hw/arm/fsl-imx25.c               |   2 -
 hw/arm/fsl-imx6.c                |   2 -
 hw/arm/fsl-imx6ul.c              |   2 -
 hw/arm/fsl-imx7.c                |   2 -
 hw/arm/fsl-imx8mp.c              |   2 -
 hw/i386/fw_cfg.c                 |   8 +-
 hw/net/smc91c111.c               | 148 ++++++++++++++++---
 hw/ppc/e500.c                    |   1 +
 hw/rtc/rs5c372.c                 | 236 +++++++++++++++++++++++++++++++
 hw/scsi/scsi-disk.c              |   2 +-
 hw/sd/sdhci.c                    |  18 ++-
 hw/virtio/virtio-mem.c           |   6 +-
 system/arch_init.c               |  19 +--
 system/globals-target.c          |  24 ++++
 system/qdev-monitor.c            |   4 +-
 system/vl.c                      |   6 +-
 tests/qtest/rs5c372-test.c       |  43 ++++++
 hw/rtc/Kconfig                   |   5 +
 hw/rtc/meson.build               |   1 +
 hw/rtc/trace-events              |   4 +
 system/meson.build               |   1 +
 tests/qtest/meson.build          |   1 +
 31 files changed, 503 insertions(+), 82 deletions(-)
 create mode 100644 hw/rtc/rs5c372.c
 create mode 100644 system/globals-target.c
 create mode 100644 tests/qtest/rs5c372-test.c

-- 
2.47.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PULL 01/14] hw/sd/sdhci: Set reset value of interrupt registers
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 02/14] hw/rtc: Add Ricoh RS5C372 RTC emulation Philippe Mathieu-Daudé
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: BALATON Zoltan, Philippe Mathieu-Daudé

From: BALATON Zoltan <balaton@eik.bme.hu>

The interrupt enable registers are not reset to 0 on Freescale eSDHC
but some bits are enabled on reset. At least some U-Boot versions seem
to expect this and not initialise these registers before expecting
interrupts. Use existing vendor property for Freescale eSDHC and set
the reset value of the interrupt registers to match Freescale
documentation.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-ID: <20250210160329.DDA7F4E600E@zero.eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sdhci.h | 1 +
 hw/ppc/e500.c         | 1 +
 hw/sd/sdhci.c         | 4 ++++
 3 files changed, 6 insertions(+)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 38c08e28598..f722d8eb1cc 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -110,6 +110,7 @@ typedef struct SDHCIState SDHCIState;
 
 #define SDHCI_VENDOR_NONE       0
 #define SDHCI_VENDOR_IMX        1
+#define SDHCI_VENDOR_FSL        2
 
 /*
  * Controller does not provide transfer-complete interrupt when not
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index fe8b9f79621..69269aa24c4 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1043,6 +1043,7 @@ void ppce500_init(MachineState *machine)
         dev = qdev_new(TYPE_SYSBUS_SDHCI);
         qdev_prop_set_uint8(dev, "sd-spec-version", 2);
         qdev_prop_set_uint8(dev, "endianness", DEVICE_BIG_ENDIAN);
+        qdev_prop_set_uint8(dev, "vendor", SDHCI_VENDOR_FSL);
         s = SYS_BUS_DEVICE(dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC85XX_ESDHC_IRQ));
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1f45a77566c..fe87e18d5d2 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -307,6 +307,10 @@ static void sdhci_reset(SDHCIState *s)
     s->data_count = 0;
     s->stopped_state = sdhc_not_stopped;
     s->pending_insert_state = false;
+    if (s->vendor == SDHCI_VENDOR_FSL) {
+        s->norintstsen = 0x013f;
+        s->errintstsen = 0x117f;
+    }
 }
 
 static void sdhci_poweron_reset(DeviceState *dev)
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 02/14] hw/rtc: Add Ricoh RS5C372 RTC emulation
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 01/14] hw/sd/sdhci: Set reset value of interrupt registers Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 03/14] hw/net/smc91c111: Sanitize packet numbers Philippe Mathieu-Daudé
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Bernhard Beschow, Philippe Mathieu-Daudé, Fabiano Rosas

From: Bernhard Beschow <shentey@gmail.com>

The implementation just allows Linux to determine date and time.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Fabiano Rosas <farosas@suse.de>
Message-ID: <20250223114708.1780-19-shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS                |   2 +
 hw/rtc/rs5c372.c           | 236 +++++++++++++++++++++++++++++++++++++
 tests/qtest/rs5c372-test.c |  43 +++++++
 hw/rtc/Kconfig             |   5 +
 hw/rtc/meson.build         |   1 +
 hw/rtc/trace-events        |   4 +
 tests/qtest/meson.build    |   1 +
 7 files changed, 292 insertions(+)
 create mode 100644 hw/rtc/rs5c372.c
 create mode 100644 tests/qtest/rs5c372-test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0e5db7a5744..e34de420f08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -827,10 +827,12 @@ F: hw/arm/imx8mp-evk.c
 F: hw/arm/fsl-imx8mp.c
 F: hw/misc/imx8mp_*.c
 F: hw/pci-host/fsl_imx8m_phy.c
+F: hw/rtc/rs5c372.c
 F: include/hw/arm/fsl-imx8mp.h
 F: include/hw/misc/imx8mp_*.h
 F: include/hw/pci-host/fsl_imx8m_phy.h
 F: docs/system/arm/imx8mp-evk.rst
+F: tests/qtest/rs5c372-test.c
 
 MPS2 / MPS3
 M: Peter Maydell <peter.maydell@linaro.org>
diff --git a/hw/rtc/rs5c372.c b/hw/rtc/rs5c372.c
new file mode 100644
index 00000000000..5542f74085a
--- /dev/null
+++ b/hw/rtc/rs5c372.c
@@ -0,0 +1,236 @@
+/*
+ * Ricoh RS5C372, R222x I2C RTC
+ *
+ * Copyright (c) 2025 Bernhard Beschow <shentey@gmail.com>
+ *
+ * Based on hw/rtc/ds1338.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/i2c.h"
+#include "hw/qdev-properties.h"
+#include "hw/resettable.h"
+#include "migration/vmstate.h"
+#include "qemu/bcd.h"
+#include "qom/object.h"
+#include "system/rtc.h"
+#include "trace.h"
+
+#define NVRAM_SIZE 0x10
+
+/* Flags definitions */
+#define SECONDS_CH 0x80
+#define HOURS_PM   0x20
+#define CTRL2_24   0x20
+
+#define TYPE_RS5C372 "rs5c372"
+OBJECT_DECLARE_SIMPLE_TYPE(RS5C372State, RS5C372)
+
+struct RS5C372State {
+    I2CSlave parent_obj;
+
+    int64_t offset;
+    uint8_t wday_offset;
+    uint8_t nvram[NVRAM_SIZE];
+    uint8_t ptr;
+    uint8_t tx_format;
+    bool addr_byte;
+};
+
+static void capture_current_time(RS5C372State *s)
+{
+    /*
+     * Capture the current time into the secondary registers which will be
+     * actually read by the data transfer operation.
+     */
+    struct tm now;
+    qemu_get_timedate(&now, s->offset);
+    s->nvram[0] = to_bcd(now.tm_sec);
+    s->nvram[1] = to_bcd(now.tm_min);
+    if (s->nvram[0xf] & CTRL2_24) {
+        s->nvram[2] = to_bcd(now.tm_hour);
+    } else {
+        int tmp = now.tm_hour;
+        if (tmp % 12 == 0) {
+            tmp += 12;
+        }
+        if (tmp <= 12) {
+            s->nvram[2] = to_bcd(tmp);
+        } else {
+            s->nvram[2] = HOURS_PM | to_bcd(tmp - 12);
+        }
+    }
+    s->nvram[3] = (now.tm_wday + s->wday_offset) % 7 + 1;
+    s->nvram[4] = to_bcd(now.tm_mday);
+    s->nvram[5] = to_bcd(now.tm_mon + 1);
+    s->nvram[6] = to_bcd(now.tm_year - 100);
+}
+
+static void inc_regptr(RS5C372State *s)
+{
+    s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1);
+}
+
+static int rs5c372_event(I2CSlave *i2c, enum i2c_event event)
+{
+    RS5C372State *s = RS5C372(i2c);
+
+    switch (event) {
+    case I2C_START_RECV:
+        /*
+         * In h/w, capture happens on any START condition, not just a
+         * START_RECV, but there is no need to actually capture on
+         * START_SEND, because the guest can't get at that data
+         * without going through a START_RECV which would overwrite it.
+         */
+        capture_current_time(s);
+        s->ptr = 0xf;
+        break;
+    case I2C_START_SEND:
+        s->addr_byte = true;
+        break;
+    default:
+        break;
+    }
+
+    return 0;
+}
+
+static uint8_t rs5c372_recv(I2CSlave *i2c)
+{
+    RS5C372State *s = RS5C372(i2c);
+    uint8_t res;
+
+    res  = s->nvram[s->ptr];
+
+    trace_rs5c372_recv(s->ptr, res);
+
+    inc_regptr(s);
+    return res;
+}
+
+static int rs5c372_send(I2CSlave *i2c, uint8_t data)
+{
+    RS5C372State *s = RS5C372(i2c);
+
+    if (s->addr_byte) {
+        s->ptr = data >> 4;
+        s->tx_format = data & 0xf;
+        s->addr_byte = false;
+        return 0;
+    }
+
+    trace_rs5c372_send(s->ptr, data);
+
+    if (s->ptr < 7) {
+        /* Time register. */
+        struct tm now;
+        qemu_get_timedate(&now, s->offset);
+        switch (s->ptr) {
+        case 0:
+            now.tm_sec = from_bcd(data & 0x7f);
+            break;
+        case 1:
+            now.tm_min = from_bcd(data & 0x7f);
+            break;
+        case 2:
+            if (s->nvram[0xf] & CTRL2_24) {
+                now.tm_hour = from_bcd(data & 0x3f);
+            } else {
+                int tmp = from_bcd(data & (HOURS_PM - 1));
+                if (data & HOURS_PM) {
+                    tmp += 12;
+                }
+                if (tmp % 12 == 0) {
+                    tmp -= 12;
+                }
+                now.tm_hour = tmp;
+            }
+            break;
+        case 3:
+            {
+                /*
+                 * The day field is supposed to contain a value in the range
+                 * 1-7. Otherwise behavior is undefined.
+                 */
+                int user_wday = (data & 7) - 1;
+                s->wday_offset = (user_wday - now.tm_wday + 7) % 7;
+            }
+            break;
+        case 4:
+            now.tm_mday = from_bcd(data & 0x3f);
+            break;
+        case 5:
+            now.tm_mon = from_bcd(data & 0x1f) - 1;
+            break;
+        case 6:
+            now.tm_year = from_bcd(data) + 100;
+            break;
+        }
+        s->offset = qemu_timedate_diff(&now);
+    } else {
+        s->nvram[s->ptr] = data;
+    }
+    inc_regptr(s);
+    return 0;
+}
+
+static void rs5c372_reset_hold(Object *obj, ResetType type)
+{
+    RS5C372State *s = RS5C372(obj);
+
+    /* The clock is running and synchronized with the host */
+    s->offset = 0;
+    s->wday_offset = 0;
+    memset(s->nvram, 0, NVRAM_SIZE);
+    s->ptr = 0;
+    s->addr_byte = false;
+}
+
+static const VMStateDescription rs5c372_vmstate = {
+    .name = "rs5c372",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (const VMStateField[]) {
+        VMSTATE_I2C_SLAVE(parent_obj, RS5C372State),
+        VMSTATE_INT64(offset, RS5C372State),
+        VMSTATE_UINT8_V(wday_offset, RS5C372State, 2),
+        VMSTATE_UINT8_ARRAY(nvram, RS5C372State, NVRAM_SIZE),
+        VMSTATE_UINT8(ptr, RS5C372State),
+        VMSTATE_UINT8(tx_format, RS5C372State),
+        VMSTATE_BOOL(addr_byte, RS5C372State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void rs5c372_init(Object *obj)
+{
+    qdev_prop_set_uint8(DEVICE(obj), "address", 0x32);
+}
+
+static void rs5c372_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    k->event = rs5c372_event;
+    k->recv = rs5c372_recv;
+    k->send = rs5c372_send;
+    dc->vmsd = &rs5c372_vmstate;
+    rc->phases.hold = rs5c372_reset_hold;
+}
+
+static const TypeInfo rs5c372_types[] = {
+    {
+        .name          = TYPE_RS5C372,
+        .parent        = TYPE_I2C_SLAVE,
+        .instance_size = sizeof(RS5C372State),
+        .instance_init = rs5c372_init,
+        .class_init    = rs5c372_class_init,
+    },
+};
+
+DEFINE_TYPES(rs5c372_types)
diff --git a/tests/qtest/rs5c372-test.c b/tests/qtest/rs5c372-test.c
new file mode 100644
index 00000000000..0f6a9b68b9f
--- /dev/null
+++ b/tests/qtest/rs5c372-test.c
@@ -0,0 +1,43 @@
+/*
+ * QTest testcase for the RS5C372 RTC
+ *
+ * Copyright (c) 2025 Bernhard Beschow <shentey@gmail.com>
+ *
+ * Based on ds1338-test.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bcd.h"
+#include "libqos/i2c.h"
+
+#define RS5C372_ADDR 0x32
+
+static void rs5c372_read_date(void *obj, void *data, QGuestAllocator *alloc)
+{
+    QI2CDevice *i2cdev = obj;
+
+    uint8_t resp[0x10];
+    time_t now = time(NULL);
+    struct tm *utc = gmtime(&now);
+
+    i2c_read_block(i2cdev, 0, resp, sizeof(resp));
+
+    /* check retrieved time against local time */
+    g_assert_cmpuint(from_bcd(resp[5]), == , utc->tm_mday);
+    g_assert_cmpuint(from_bcd(resp[6]), == , 1 + utc->tm_mon);
+    g_assert_cmpuint(2000 + from_bcd(resp[7]), == , 1900 + utc->tm_year);
+}
+
+static void rs5c372_register_nodes(void)
+{
+    QOSGraphEdgeOptions opts = { };
+    add_qi2c_address(&opts, &(QI2CAddress) { RS5C372_ADDR });
+
+    qos_node_create_driver("rs5c372", i2c_device_create);
+    qos_node_consumes("rs5c372", "i2c-bus", &opts);
+    qos_add_test("read_date", "rs5c372", rs5c372_read_date, NULL);
+}
+
+libqos_init(rs5c372_register_nodes);
diff --git a/hw/rtc/Kconfig b/hw/rtc/Kconfig
index 2fe04ec1d04..315b0e4eccd 100644
--- a/hw/rtc/Kconfig
+++ b/hw/rtc/Kconfig
@@ -26,3 +26,8 @@ config GOLDFISH_RTC
 
 config LS7A_RTC
     bool
+
+config RS5C372_RTC
+    bool
+    depends on I2C
+    default y if I2C_DEVICES
diff --git a/hw/rtc/meson.build b/hw/rtc/meson.build
index 8ecc2d792c1..6c87864dc07 100644
--- a/hw/rtc/meson.build
+++ b/hw/rtc/meson.build
@@ -13,3 +13,4 @@ system_ss.add(when: 'CONFIG_GOLDFISH_RTC', if_true: files('goldfish_rtc.c'))
 system_ss.add(when: 'CONFIG_LS7A_RTC', if_true: files('ls7a_rtc.c'))
 system_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true: files('allwinner-rtc.c'))
 system_ss.add(when: 'CONFIG_MC146818RTC', if_true: files('mc146818rtc.c'))
+system_ss.add(when: 'CONFIG_RS5C372_RTC', if_true: files('rs5c372.c'))
diff --git a/hw/rtc/trace-events b/hw/rtc/trace-events
index 8012afe1021..b9f2852d35f 100644
--- a/hw/rtc/trace-events
+++ b/hw/rtc/trace-events
@@ -35,3 +35,7 @@ m48txx_nvram_mem_write(uint32_t addr, uint32_t value) "mem write addr:0x%04x val
 # goldfish_rtc.c
 goldfish_rtc_read(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64 " value 0x%08" PRIx64
 goldfish_rtc_write(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64 " value 0x%08" PRIx64
+
+# rs5c372.c
+rs5c372_recv(uint32_t addr, uint8_t value) "[0x%" PRIx32 "] -> 0x%02" PRIx8
+rs5c372_send(uint32_t addr, uint8_t value) "[0x%" PRIx32 "] <- 0x%02" PRIx8
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 8a6243382a1..9e5380ba7a2 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -298,6 +298,7 @@ qos_test_ss.add(
   'pca9552-test.c',
   'pci-test.c',
   'pcnet-test.c',
+  'rs5c372-test.c',
   'sdhci-test.c',
   'spapr-phb-test.c',
   'tmp105-test.c',
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 03/14] hw/net/smc91c111: Sanitize packet numbers
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 01/14] hw/sd/sdhci: Set reset value of interrupt registers Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 02/14] hw/rtc: Add Ricoh RS5C372 RTC emulation Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 04/14] hw/net/smc91c111: Sanitize packet length on tx Philippe Mathieu-Daudé
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Philippe Mathieu-Daudé

From: Peter Maydell <peter.maydell@linaro.org>

The smc91c111 uses packet numbers as an index into its internal
s->data[][] array. Valid packet numbers are between 0 and 3, but
the code does not generally check this, and there are various
places where the guest can hand us an arbitrary packet number
and cause an out-of-bounds access to the data array.

Add validation of packet numbers. The datasheet is not very
helpful about how guest errors like this should be handled:
it says nothing on the subject, and none of the documented
error conditions are relevant. We choose to log the situation
with LOG_GUEST_ERROR and silently ignore the attempted operation.

In the places where we are about to access the data[][] array
using a packet number and we know the number is valid because
we got it from somewhere that has already validated, we add
an assert() to document that belief.

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250228174802.1945417-2-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/smc91c111.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 0e13dfa18b2..2295c6acf25 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -118,6 +118,11 @@ static const VMStateDescription vmstate_smc91c111 = {
 #define RS_TOOSHORT     0x0400
 #define RS_MULTICAST    0x0001
 
+static inline bool packetnum_valid(int packet_num)
+{
+    return packet_num >= 0 && packet_num < NUM_PACKETS;
+}
+
 /* Update interrupt status.  */
 static void smc91c111_update(smc91c111_state *s)
 {
@@ -218,6 +223,17 @@ static void smc91c111_pop_tx_fifo_done(smc91c111_state *s)
 /* Release the memory allocated to a packet.  */
 static void smc91c111_release_packet(smc91c111_state *s, int packet)
 {
+    if (!packetnum_valid(packet)) {
+        /*
+         * Data sheet doesn't document behaviour in this guest error
+         * case, and there is no error status register to report it.
+         * Log and ignore the attempt.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "smc91c111: attempt to release invalid packet %d\n",
+                      packet);
+        return;
+    }
     s->allocated &= ~(1 << packet);
     if (s->tx_alloc == 0x80)
         smc91c111_tx_alloc(s);
@@ -239,6 +255,8 @@ static void smc91c111_do_tx(smc91c111_state *s)
         return;
     for (i = 0; i < s->tx_fifo_len; i++) {
         packetnum = s->tx_fifo[i];
+        /* queue_tx checked the packet number was valid */
+        assert(packetnum_valid(packetnum));
         p = &s->data[packetnum][0];
         /* Set status word.  */
         *(p++) = 0x01;
@@ -287,6 +305,17 @@ static void smc91c111_do_tx(smc91c111_state *s)
 /* Add a packet to the TX FIFO.  */
 static void smc91c111_queue_tx(smc91c111_state *s, int packet)
 {
+    if (!packetnum_valid(packet)) {
+        /*
+         * Datasheet doesn't document behaviour in this error case, and
+         * there's no error status register we could report it in.
+         * Log and ignore.
+         */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "smc91c111: attempt to queue invalid packet %d\n",
+                      packet);
+        return;
+    }
     if (s->tx_fifo_len == NUM_PACKETS)
         return;
     s->tx_fifo[s->tx_fifo_len++] = packet;
@@ -457,6 +486,13 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
                     n = s->rx_fifo[0];
                 else
                     n = s->packet_num;
+                if (!packetnum_valid(n)) {
+                    /* Datasheet doesn't document what to do here */
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "smc91c111: attempt to write data to invalid packet %d\n",
+                                  n);
+                    return;
+                }
                 p = s->ptr & 0x07ff;
                 if (s->ptr & 0x4000) {
                     s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x7ff);
@@ -605,6 +641,13 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
                     n = s->rx_fifo[0];
                 else
                     n = s->packet_num;
+                if (!packetnum_valid(n)) {
+                    /* Datasheet doesn't document what to do here */
+                    qemu_log_mask(LOG_GUEST_ERROR,
+                                  "smc91c111: attempt to read data from invalid packet %d\n",
+                                  n);
+                    return 0;
+                }
                 p = s->ptr & 0x07ff;
                 if (s->ptr & 0x4000) {
                     s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x07ff);
@@ -713,6 +756,8 @@ static ssize_t smc91c111_receive(NetClientState *nc, const uint8_t *buf, size_t
         return -1;
     s->rx_fifo[s->rx_fifo_len++] = packetnum;
 
+    /* allocate_packet() will not hand us back an invalid packet number */
+    assert(packetnum_valid(packetnum));
     p = &s->data[packetnum][0];
     /* ??? Multicast packets?  */
     status = 0;
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 04/14] hw/net/smc91c111: Sanitize packet length on tx
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 03/14] hw/net/smc91c111: Sanitize packet numbers Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 05/14] hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers Philippe Mathieu-Daudé
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Philippe Mathieu-Daudé

From: Peter Maydell <peter.maydell@linaro.org>

When the smc91c111 transmits a packet, it must read a control byte
which is at the end of the data area and CRC.  However, we don't
sanitize the length field in the packet buffer, so if the guest sets
the length field to something large we will try to read past the end
of the packet data buffer when we access the control byte.

As usual, the datasheet says nothing about the behaviour of the
hardware if the guest misprograms it in this way.  It says only that
the maximum valid length is 2048 bytes.  We choose to log the guest
error and silently drop the packet.

This requires us to factor out the "mark the tx packet as complete"
logic, so we can call it for this "drop packet" case as well as at
the end of the loop when we send a valid packet.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2742
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250228174802.1945417-3-peter.maydell@linaro.org>
[PMD: Update smc91c111_do_tx() as len > MAX_PACKET_SIZE]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/smc91c111.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 2295c6acf25..72ce5d8f4de 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -22,6 +22,13 @@
 
 /* Number of 2k memory pages available.  */
 #define NUM_PACKETS 4
+/*
+ * Maximum size of a data frame, including the leading status word
+ * and byte count fields and the trailing CRC, last data byte
+ * and control byte (per figure 8-1 in the Microchip Technology
+ * LAN91C111 datasheet).
+ */
+#define MAX_PACKET_SIZE 2048
 
 #define TYPE_SMC91C111 "smc91c111"
 OBJECT_DECLARE_SIMPLE_TYPE(smc91c111_state, SMC91C111)
@@ -240,6 +247,16 @@ static void smc91c111_release_packet(smc91c111_state *s, int packet)
     smc91c111_flush_queued_packets(s);
 }
 
+static void smc91c111_complete_tx_packet(smc91c111_state *s, int packetnum)
+{
+    if (s->ctr & CTR_AUTO_RELEASE) {
+        /* Race?  */
+        smc91c111_release_packet(s, packetnum);
+    } else if (s->tx_fifo_done_len < NUM_PACKETS) {
+        s->tx_fifo_done[s->tx_fifo_done_len++] = packetnum;
+    }
+}
+
 /* Flush the TX FIFO.  */
 static void smc91c111_do_tx(smc91c111_state *s)
 {
@@ -263,6 +280,17 @@ static void smc91c111_do_tx(smc91c111_state *s)
         *(p++) = 0x40;
         len = *(p++);
         len |= ((int)*(p++)) << 8;
+        if (len > MAX_PACKET_SIZE) {
+            /*
+             * Datasheet doesn't say what to do here, and there is no
+             * relevant tx error condition listed. Log, and drop the packet.
+             */
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "smc91c111: tx packet with bad length %d, dropping\n",
+                          len);
+            smc91c111_complete_tx_packet(s, packetnum);
+            continue;
+        }
         len -= 6;
         control = p[len + 1];
         if (control & 0x20)
@@ -291,11 +319,7 @@ static void smc91c111_do_tx(smc91c111_state *s)
             }
         }
 #endif
-        if (s->ctr & CTR_AUTO_RELEASE)
-            /* Race?  */
-            smc91c111_release_packet(s, packetnum);
-        else if (s->tx_fifo_done_len < NUM_PACKETS)
-            s->tx_fifo_done[s->tx_fifo_done_len++] = packetnum;
+        smc91c111_complete_tx_packet(s, packetnum);
         qemu_send_packet(qemu_get_queue(s->nic), p, len);
     }
     s->tx_fifo_len = 0;
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 05/14] hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 04/14] hw/net/smc91c111: Sanitize packet length on tx Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 06/14] hw/net/smc91c111: Don't allow data register access to overrun buffer Philippe Mathieu-Daudé
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Philippe Mathieu-Daudé

From: Peter Maydell <peter.maydell@linaro.org>

Now we have a constant for the maximum packet size, we can use it
to replace various hardcoded 2048 values.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250228174802.1945417-4-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/smc91c111.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 72ce5d8f4de..b05970d5e1c 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -58,7 +58,7 @@ struct smc91c111_state {
     int tx_fifo_done_len;
     int tx_fifo_done[NUM_PACKETS];
     /* Packet buffer memory.  */
-    uint8_t data[NUM_PACKETS][2048];
+    uint8_t data[NUM_PACKETS][MAX_PACKET_SIZE];
     uint8_t int_level;
     uint8_t int_mask;
     MemoryRegion mmio;
@@ -86,7 +86,8 @@ static const VMStateDescription vmstate_smc91c111 = {
         VMSTATE_INT32_ARRAY(rx_fifo, smc91c111_state, NUM_PACKETS),
         VMSTATE_INT32(tx_fifo_done_len, smc91c111_state),
         VMSTATE_INT32_ARRAY(tx_fifo_done, smc91c111_state, NUM_PACKETS),
-        VMSTATE_BUFFER_UNSAFE(data, smc91c111_state, 0, NUM_PACKETS * 2048),
+        VMSTATE_BUFFER_UNSAFE(data, smc91c111_state, 0,
+                              NUM_PACKETS * MAX_PACKET_SIZE),
         VMSTATE_UINT8(int_level, smc91c111_state),
         VMSTATE_UINT8(int_mask, smc91c111_state),
         VMSTATE_END_OF_LIST()
@@ -773,8 +774,9 @@ static ssize_t smc91c111_receive(NetClientState *nc, const uint8_t *buf, size_t
     if (crc)
         packetsize += 4;
     /* TODO: Flag overrun and receive errors.  */
-    if (packetsize > 2048)
+    if (packetsize > MAX_PACKET_SIZE) {
         return -1;
+    }
     packetnum = smc91c111_allocate_packet(s);
     if (packetnum == 0x80)
         return -1;
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 06/14] hw/net/smc91c111: Don't allow data register access to overrun buffer
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 05/14] hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 07/14] hw/xen/hvm: Fix Aarch64 typo Philippe Mathieu-Daudé
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Philippe Mathieu-Daudé

From: Peter Maydell <peter.maydell@linaro.org>

For accesses to the 91c111 data register, the address within the
packet's data frame is determined by a combination of the pointer
register and the offset used to access the data register, so that you
can access data at effectively wider than byte width.  The pointer
register's pointer field is 11 bits wide, which is exactly the size
to index a 2048-byte data frame.

We weren't quite getting the logic right for ensuring that we end up
with a pointer value to use in the s->data[][] array that isn't out
of bounds:

 * we correctly mask when getting the initial pointer value
 * for the "autoincrement the pointer register" case, we
   correctly mask after adding 1 so that the pointer register
   wraps back around at the 2048 byte mark
 * but for the non-autoincrement case where we have to add the
   low 2 bits of the data register offset, we don't account
   for the possibility that the pointer register is 0x7ff
   and the addition should wrap

Fix this bug by factoring out the "get the p value to use as an array
index" into a function, making it use FIELD macro names rather than
hard-coded constants, and having a utility function that does "add a
value and wrap it" that we can use both for the "autoincrement" and
"add the offset bits" codepaths.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2758
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20250228191652.1957208-1-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/smc91c111.c | 65 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index b05970d5e1c..9ce42b56155 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -13,6 +13,7 @@
 #include "net/net.h"
 #include "hw/irq.h"
 #include "hw/net/smc91c111.h"
+#include "hw/registerfields.h"
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
@@ -126,6 +127,13 @@ static const VMStateDescription vmstate_smc91c111 = {
 #define RS_TOOSHORT     0x0400
 #define RS_MULTICAST    0x0001
 
+FIELD(PTR, PTR, 0, 11)
+FIELD(PTR, NOT_EMPTY, 11, 1)
+FIELD(PTR, RESERVED, 12, 1)
+FIELD(PTR, READ, 13, 1)
+FIELD(PTR, AUTOINCR, 14, 1)
+FIELD(PTR, RCV, 15, 1)
+
 static inline bool packetnum_valid(int packet_num)
 {
     return packet_num >= 0 && packet_num < NUM_PACKETS;
@@ -372,6 +380,49 @@ static void smc91c111_reset(DeviceState *dev)
 #define SET_LOW(name, val) s->name = (s->name & 0xff00) | val
 #define SET_HIGH(name, val) s->name = (s->name & 0xff) | (val << 8)
 
+/*
+ * The pointer register's pointer is an 11 bit value (so it exactly
+ * indexes a 2048-byte data frame). Add the specified offset to it,
+ * wrapping around at the 2048 byte mark, and return the resulting
+ * wrapped value. There are flag bits in the top part of the register,
+ * but we can ignore them here as the mask will mask them out.
+ */
+static int ptr_reg_add(smc91c111_state *s, int offset)
+{
+    return (s->ptr + offset) & R_PTR_PTR_MASK;
+}
+
+/*
+ * For an access to the Data Register at @offset, return the
+ * required offset into the packet's data frame. This will
+ * perform the pointer register autoincrement if required, and
+ * guarantees to return an in-bounds offset.
+ */
+static int data_reg_ptr(smc91c111_state *s, int offset)
+{
+    int p;
+
+    if (s->ptr & R_PTR_AUTOINCR_MASK) {
+        /*
+         * Autoincrement: use the current pointer value, and
+         * increment the pointer register's pointer field.
+         */
+        p = FIELD_EX32(s->ptr, PTR, PTR);
+        s->ptr = FIELD_DP32(s->ptr, PTR, PTR, ptr_reg_add(s, 1));
+    } else {
+        /*
+         * No autoincrement: register offset determines which
+         * byte we're addressing. Setting the pointer to the top
+         * of the data buffer and then using the pointer wrapping
+         * to read the bottom byte of the buffer is not something
+         * sensible guest software will do, but the datasheet
+         * doesn't say what the behaviour is, so we don't forbid it.
+         */
+        p = ptr_reg_add(s, offset & 3);
+    }
+    return p;
+}
+
 static void smc91c111_writeb(void *opaque, hwaddr offset,
                              uint32_t value)
 {
@@ -518,12 +569,7 @@ static void smc91c111_writeb(void *opaque, hwaddr offset,
                                   n);
                     return;
                 }
-                p = s->ptr & 0x07ff;
-                if (s->ptr & 0x4000) {
-                    s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x7ff);
-                } else {
-                    p += (offset & 3);
-                }
+                p = data_reg_ptr(s, offset);
                 s->data[n][p] = value;
             }
             return;
@@ -673,12 +719,7 @@ static uint32_t smc91c111_readb(void *opaque, hwaddr offset)
                                   n);
                     return 0;
                 }
-                p = s->ptr & 0x07ff;
-                if (s->ptr & 0x4000) {
-                    s->ptr = (s->ptr & 0xf800) | ((s->ptr + 1) & 0x07ff);
-                } else {
-                    p += (offset & 3);
-                }
+                p = data_reg_ptr(s, offset);
                 return s->data[n][p];
             }
         case 12: /* Interrupt status.  */
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 07/14] hw/xen/hvm: Fix Aarch64 typo
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 06/14] hw/net/smc91c111: Don't allow data register access to overrun buffer Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 08/14] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, Pierrick Bouvier, Richard Henderson

There is no TARGET_ARM_64 definition. Luckily enough,
when TARGET_AARCH64 is defined, TARGET_ARM also is.

Fixes: 733766cd373 ("hw/arm: introduce xenpvh machine")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20250305153929.43687-2-philmd@linaro.org>
---
 include/hw/xen/arch_hvm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/xen/arch_hvm.h b/include/hw/xen/arch_hvm.h
index c7c515220d2..df39c819c8f 100644
--- a/include/hw/xen/arch_hvm.h
+++ b/include/hw/xen/arch_hvm.h
@@ -1,5 +1,5 @@
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
 #include "hw/i386/xen_arch_hvm.h"
-#elif defined(TARGET_ARM) || defined(TARGET_ARM_64)
+#elif defined(TARGET_ARM) || defined(TARGET_AARCH64)
 #include "hw/arm/xen_arch_hvm.h"
 #endif
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 08/14] system: Extract target-specific globals to their own compilation unit
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (6 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 07/14] hw/xen/hvm: Fix Aarch64 typo Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 09/14] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Alex Bennée

We shouldn't use target specific globals for machine properties.
These ones could be desugarized, as explained in [*]. While
certainly doable, not trivial nor my priority for now. Just move
them to a different file to clarify they are *globals*, like the
generic globals residing in system/globals.c.

Since arch_init.c was introduced using the MIT license (see commit
ad96090a01d), retain the same license for the new globals-target.c
file.

[*] https://lore.kernel.org/qemu-devel/e514d6db-781d-4afe-b057-9046c70044dc@redhat.com/

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20250305005225.95051-2-philmd@linaro.org>
---
 system/arch_init.c      | 14 --------------
 system/globals-target.c | 24 ++++++++++++++++++++++++
 system/meson.build      |  1 +
 3 files changed, 25 insertions(+), 14 deletions(-)
 create mode 100644 system/globals-target.c

diff --git a/system/arch_init.c b/system/arch_init.c
index b1baed18a30..b9147af93cb 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -24,18 +24,4 @@
 #include "qemu/osdep.h"
 #include "system/arch_init.h"
 
-#ifdef TARGET_SPARC
-int graphic_width = 1024;
-int graphic_height = 768;
-int graphic_depth = 8;
-#elif defined(TARGET_M68K)
-int graphic_width = 800;
-int graphic_height = 600;
-int graphic_depth = 8;
-#else
-int graphic_width = 800;
-int graphic_height = 600;
-int graphic_depth = 32;
-#endif
-
 const uint32_t arch_type = QEMU_ARCH;
diff --git a/system/globals-target.c b/system/globals-target.c
new file mode 100644
index 00000000000..989720591e7
--- /dev/null
+++ b/system/globals-target.c
@@ -0,0 +1,24 @@
+/*
+ * Global variables that should not exist (target specific)
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "system/system.h"
+
+#ifdef TARGET_SPARC
+int graphic_width = 1024;
+int graphic_height = 768;
+int graphic_depth = 8;
+#elif defined(TARGET_M68K)
+int graphic_width = 800;
+int graphic_height = 600;
+int graphic_depth = 8;
+#else
+int graphic_width = 800;
+int graphic_height = 600;
+int graphic_depth = 32;
+#endif
diff --git a/system/meson.build b/system/meson.build
index c83d80fa248..eec07a94513 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -1,6 +1,7 @@
 specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
   'arch_init.c',
   'ioport.c',
+  'globals-target.c',
   'memory.c',
   'physmem.c',
 )])
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 09/14] system: Replace arch_type global by qemu_arch_available() helper
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (7 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 08/14] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 10/14] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Richard Henderson

qemu_arch_available() is a bit simpler to understand while
reviewing than the undocumented arch_type variable.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20250305005225.95051-5-philmd@linaro.org>
---
 include/system/arch_init.h | 2 +-
 hw/scsi/scsi-disk.c        | 2 +-
 system/arch_init.c         | 5 ++++-
 system/qdev-monitor.c      | 4 ++--
 system/vl.c                | 6 +++---
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/system/arch_init.h b/include/system/arch_init.h
index d8b77440487..51e24c3091e 100644
--- a/include/system/arch_init.h
+++ b/include/system/arch_init.h
@@ -25,6 +25,6 @@ enum {
     QEMU_ARCH_LOONGARCH = (1 << 23),
 };
 
-extern const uint32_t arch_type;
+bool qemu_arch_available(unsigned qemu_arch_mask);
 
 #endif
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e7f738b4841..7c87b20e694 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -3165,7 +3165,7 @@ static void scsi_property_add_specifics(DeviceClass *dc)
     ObjectClass *oc = OBJECT_CLASS(dc);
 
     /* The loadparm property is only supported on s390x */
-    if (arch_type & QEMU_ARCH_S390X) {
+    if (qemu_arch_available(QEMU_ARCH_S390X)) {
         object_class_property_add_str(oc, "loadparm",
                                       scsi_property_get_loadparm,
                                       scsi_property_set_loadparm);
diff --git a/system/arch_init.c b/system/arch_init.c
index b9147af93cb..e85736884c9 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -24,4 +24,7 @@
 #include "qemu/osdep.h"
 #include "system/arch_init.h"
 
-const uint32_t arch_type = QEMU_ARCH;
+bool qemu_arch_available(unsigned qemu_arch_mask)
+{
+    return qemu_arch_mask & QEMU_ARCH;
+}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 856c9e8c32e..5588ed2047d 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -132,7 +132,7 @@ static const char *qdev_class_get_alias(DeviceClass *dc)
 
     for (i = 0; qdev_alias_table[i].typename; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
@@ -218,7 +218,7 @@ static const char *find_typename_by_alias(const char *alias)
 
     for (i = 0; qdev_alias_table[i].alias; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
diff --git a/system/vl.c b/system/vl.c
index 04f78466c41..ec93988a03a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -878,11 +878,11 @@ static void help(int exitcode)
             g_get_prgname());
 
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
-    if ((arch_mask) & arch_type)                               \
+    if (qemu_arch_available(arch_mask)) \
         fputs(opt_help, stdout);
 
 #define ARCHHEADING(text, arch_mask) \
-    if ((arch_mask) & arch_type)    \
+    if (qemu_arch_available(arch_mask)) \
         puts(stringify(text));
 
 #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
@@ -2929,7 +2929,7 @@ void qemu_init(int argc, char **argv)
             const QEMUOption *popt;
 
             popt = lookup_opt(argc, argv, &optarg, &optind);
-            if (!(popt->arch_mask & arch_type)) {
+            if (!qemu_arch_available(popt->arch_mask)) {
                 error_report("Option not supported for this target");
                 exit(1);
             }
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 10/14] hw/acpi: Introduce acpi_builtin() helper
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (8 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 09/14] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 11/14] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Ani Sinha

acpi_builtin() can be used to check at runtime whether
the ACPI subsystem is built in a qemu-system binary.

Reviewed-by: Ani Sinha <anisinha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20250307223949.54040-3-philmd@linaro.org>
---
 include/hw/acpi/acpi.h | 3 +++
 hw/acpi/acpi-stub.c    | 5 +++++
 hw/acpi/core.c         | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e0e51e85b41..d1a4fa2af84 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -150,6 +150,9 @@ struct ACPIREGS {
     Notifier wakeup;
 };
 
+/* Return whether ACPI subsystem is built in */
+bool acpi_builtin(void);
+
 /* PM_TMR */
 void acpi_pm_tmr_update(ACPIREGS *ar, bool enable);
 void acpi_pm_tmr_calc_overflow_time(ACPIREGS *ar);
diff --git a/hw/acpi/acpi-stub.c b/hw/acpi/acpi-stub.c
index e268ce9b1a9..790bf509e5d 100644
--- a/hw/acpi/acpi-stub.c
+++ b/hw/acpi/acpi-stub.c
@@ -25,3 +25,8 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
 {
     g_assert_not_reached();
 }
+
+bool acpi_builtin(void)
+{
+    return false;
+}
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 870391ed7c8..58f8964e130 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -78,6 +78,11 @@ static void acpi_register_config(void)
 
 opts_init(acpi_register_config);
 
+bool acpi_builtin(void)
+{
+    return true;
+}
+
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 11/14] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin()
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (9 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 10/14] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 12/14] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Ani Sinha

Define acpi_tables / acpi_tables_len stubs, then replace the
compile-time CONFIG_ACPI check in fw_cfg.c by a runtime one.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
Message-Id: <20250307223949.54040-4-philmd@linaro.org>
---
 hw/acpi/acpi-stub.c | 3 +++
 hw/i386/fw_cfg.c    | 8 ++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/acpi-stub.c b/hw/acpi/acpi-stub.c
index 790bf509e5d..fd0b62fad9e 100644
--- a/hw/acpi/acpi-stub.c
+++ b/hw/acpi/acpi-stub.c
@@ -21,6 +21,9 @@
 #include "qemu/osdep.h"
 #include "hw/acpi/acpi.h"
 
+char unsigned *acpi_tables;
+size_t acpi_tables_len;
+
 void acpi_table_add(const QemuOpts *opts, Error **errp)
 {
     g_assert_not_reached();
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index d08aefa0291..a7f1b60b98c 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -145,10 +145,10 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
      */
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, ms->ram_size);
-#ifdef CONFIG_ACPI
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
-                     acpi_tables, acpi_tables_len);
-#endif
+    if (acpi_builtin()) {
+        fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
+                         acpi_tables, acpi_tables_len);
+    }
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_fw_cfg, sizeof(hpet_fw_cfg));
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 12/14] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (10 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 11/14] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 13/14] hw/hyperv/hyperv-proto: Move SYNDBG definitions from target/i386 Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition Philippe Mathieu-Daudé
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, David Hildenbrand, Pierrick Bouvier

Rather than checking ACPI availability at compile time by
checking the CONFIG_ACPI definition from CONFIG_DEVICES,
check at runtime via acpi_builtin().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Message-Id: <20250307223949.54040-5-philmd@linaro.org>
---
 hw/virtio/virtio-mem.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 7b140add765..5f57eccbb66 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -28,7 +28,7 @@
 #include "migration/misc.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
-#include CONFIG_DEVICES
+#include "hw/acpi/acpi.h"
 #include "trace.h"
 
 static const VMStateDescription vmstate_virtio_mem_device_early;
@@ -883,10 +883,8 @@ static uint64_t virtio_mem_get_features(VirtIODevice *vdev, uint64_t features,
     MachineState *ms = MACHINE(qdev_get_machine());
     VirtIOMEM *vmem = VIRTIO_MEM(vdev);
 
-    if (ms->numa_state) {
-#if defined(CONFIG_ACPI)
+    if (ms->numa_state && acpi_builtin()) {
         virtio_add_feature(&features, VIRTIO_MEM_F_ACPI_PXM);
-#endif
     }
     assert(vmem->unplugged_inaccessible != ON_OFF_AUTO_AUTO);
     if (vmem->unplugged_inaccessible == ON_OFF_AUTO_ON) {
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 13/14] hw/hyperv/hyperv-proto: Move SYNDBG definitions from target/i386
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (11 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 12/14] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 19:51 ` [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition Philippe Mathieu-Daudé
  13 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Pierrick Bouvier, Philippe Mathieu-Daudé

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Allows SYNDBG definitions to be available for common compilation units.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Message-ID: <20250307215623.524987-5-pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/hyperv/hyperv-proto.h | 12 ++++++++++++
 target/i386/kvm/hyperv-proto.h   | 12 ------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/hw/hyperv/hyperv-proto.h b/include/hw/hyperv/hyperv-proto.h
index 4a2297307b0..fffc5ce342f 100644
--- a/include/hw/hyperv/hyperv-proto.h
+++ b/include/hw/hyperv/hyperv-proto.h
@@ -61,6 +61,18 @@
 #define HV_MESSAGE_X64_APIC_EOI               0x80010004
 #define HV_MESSAGE_X64_LEGACY_FP_ERROR        0x80010005
 
+/*
+ * Hyper-V Synthetic debug options MSR
+ */
+#define HV_X64_MSR_SYNDBG_CONTROL               0x400000F1
+#define HV_X64_MSR_SYNDBG_STATUS                0x400000F2
+#define HV_X64_MSR_SYNDBG_SEND_BUFFER           0x400000F3
+#define HV_X64_MSR_SYNDBG_RECV_BUFFER           0x400000F4
+#define HV_X64_MSR_SYNDBG_PENDING_BUFFER        0x400000F5
+#define HV_X64_MSR_SYNDBG_OPTIONS               0x400000FF
+
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS         BIT(2)
+
 /*
  * Message flags
  */
diff --git a/target/i386/kvm/hyperv-proto.h b/target/i386/kvm/hyperv-proto.h
index 464fbf09e35..a9f056f2f3e 100644
--- a/target/i386/kvm/hyperv-proto.h
+++ b/target/i386/kvm/hyperv-proto.h
@@ -151,18 +151,6 @@
 #define HV_X64_MSR_STIMER3_CONFIG               0x400000B6
 #define HV_X64_MSR_STIMER3_COUNT                0x400000B7
 
-/*
- * Hyper-V Synthetic debug options MSR
- */
-#define HV_X64_MSR_SYNDBG_CONTROL               0x400000F1
-#define HV_X64_MSR_SYNDBG_STATUS                0x400000F2
-#define HV_X64_MSR_SYNDBG_SEND_BUFFER           0x400000F3
-#define HV_X64_MSR_SYNDBG_RECV_BUFFER           0x400000F4
-#define HV_X64_MSR_SYNDBG_PENDING_BUFFER        0x400000F5
-#define HV_X64_MSR_SYNDBG_OPTIONS               0x400000FF
-
-#define HV_X64_SYNDBG_OPTION_USE_HCALLS         BIT(2)
-
 /*
  * Guest crash notification MSRs
  */
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition
  2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
                   ` (12 preceding siblings ...)
  2025-03-11 19:51 ` [PULL 13/14] hw/hyperv/hyperv-proto: Move SYNDBG definitions from target/i386 Philippe Mathieu-Daudé
@ 2025-03-11 19:51 ` Philippe Mathieu-Daudé
  2025-03-11 21:00   ` BALATON Zoltan
  13 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-11 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, BALATON Zoltan, Bernhard Beschow

All instances of TYPE_IMX_USDHC set vendor=SDHCI_VENDOR_IMX.
No need to special-case it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Message-Id: <20250308213640.13138-3-philmd@linaro.org>
---
 include/hw/sd/sdhci.h |  1 -
 hw/arm/fsl-imx25.c    |  2 --
 hw/arm/fsl-imx6.c     |  2 --
 hw/arm/fsl-imx6ul.c   |  2 --
 hw/arm/fsl-imx7.c     |  2 --
 hw/arm/fsl-imx8mp.c   |  2 --
 hw/sd/sdhci.c         | 14 ++++----------
 7 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index f722d8eb1cc..51fb30ea528 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -109,7 +109,6 @@ struct SDHCIState {
 typedef struct SDHCIState SDHCIState;
 
 #define SDHCI_VENDOR_NONE       0
-#define SDHCI_VENDOR_IMX        1
 #define SDHCI_VENDOR_FSL        2
 
 /*
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 5359a6d8d3b..02214ca1a1c 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -243,8 +243,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
                                  &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), "capareg",
                                  IMX25_ESDHC_CAPABILITIES, &error_abort);
-        object_property_set_uint(OBJECT(&s->esdhc[i]), "vendor",
-                                 SDHCI_VENDOR_IMX, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), errp)) {
             return;
         }
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index dc86338b3a5..a114dc0d63d 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -327,8 +327,6 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
                                  &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), "capareg",
                                  IMX6_ESDHC_CAPABILITIES, &error_abort);
-        object_property_set_uint(OBJECT(&s->esdhc[i]), "vendor",
-                                 SDHCI_VENDOR_IMX, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), errp)) {
             return;
         }
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 34c4aa15cd0..ce8d3ef535f 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -531,8 +531,6 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
             FSL_IMX6UL_USDHC2_IRQ,
         };
 
-        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
-                                 SDHCI_VENDOR_IMX, &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), &error_abort);
 
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->usdhc[i]), 0,
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 3374018cde0..ed1f10bca26 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -471,8 +471,6 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
             FSL_IMX7_USDHC3_IRQ,
         };
 
-        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
-                                 SDHCI_VENDOR_IMX, &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), &error_abort);
 
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->usdhc[i]), 0,
diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
index 1ea98e14635..c3f6da63220 100644
--- a/hw/arm/fsl-imx8mp.c
+++ b/hw/arm/fsl-imx8mp.c
@@ -524,8 +524,6 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error **errp)
             { fsl_imx8mp_memmap[FSL_IMX8MP_USDHC3].addr, FSL_IMX8MP_USDHC3_IRQ },
         };
 
-        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
-                                 SDHCI_VENDOR_IMX, &error_abort);
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), errp)) {
             return;
         }
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index fe87e18d5d2..69baf73ae9b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1735,16 +1735,10 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
 
     case USDHC_VENDOR_SPEC:
         s->vendor_spec = value;
-        switch (s->vendor) {
-        case SDHCI_VENDOR_IMX:
-            if (value & USDHC_IMX_FRC_SDCLK_ON) {
-                s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
-            } else {
-                s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
-            }
-            break;
-        default:
-            break;
+        if (value & USDHC_IMX_FRC_SDCLK_ON) {
+            s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
+        } else {
+            s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
         }
         break;
 
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition
  2025-03-11 19:51 ` [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition Philippe Mathieu-Daudé
@ 2025-03-11 21:00   ` BALATON Zoltan
  0 siblings, 0 replies; 16+ messages in thread
From: BALATON Zoltan @ 2025-03-11 21:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Bernhard Beschow

[-- Attachment #1: Type: text/plain, Size: 4977 bytes --]

On Tue, 11 Mar 2025, Philippe Mathieu-Daudé wrote:
> All instances of TYPE_IMX_USDHC set vendor=SDHCI_VENDOR_IMX.
> No need to special-case it.

Typo in subject. It's actually SDHCI_VENDOR_IMX which is removed by this 
patch.

Regards,
BALATON Zoltan

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Bernhard Beschow <shentey@gmail.com>
> Message-Id: <20250308213640.13138-3-philmd@linaro.org>
> ---
> include/hw/sd/sdhci.h |  1 -
> hw/arm/fsl-imx25.c    |  2 --
> hw/arm/fsl-imx6.c     |  2 --
> hw/arm/fsl-imx6ul.c   |  2 --
> hw/arm/fsl-imx7.c     |  2 --
> hw/arm/fsl-imx8mp.c   |  2 --
> hw/sd/sdhci.c         | 14 ++++----------
> 7 files changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index f722d8eb1cc..51fb30ea528 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -109,7 +109,6 @@ struct SDHCIState {
> typedef struct SDHCIState SDHCIState;
>
> #define SDHCI_VENDOR_NONE       0
> -#define SDHCI_VENDOR_IMX        1
> #define SDHCI_VENDOR_FSL        2
>
> /*
> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
> index 5359a6d8d3b..02214ca1a1c 100644
> --- a/hw/arm/fsl-imx25.c
> +++ b/hw/arm/fsl-imx25.c
> @@ -243,8 +243,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
>                                  &error_abort);
>         object_property_set_uint(OBJECT(&s->esdhc[i]), "capareg",
>                                  IMX25_ESDHC_CAPABILITIES, &error_abort);
> -        object_property_set_uint(OBJECT(&s->esdhc[i]), "vendor",
> -                                 SDHCI_VENDOR_IMX, &error_abort);
>         if (!sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), errp)) {
>             return;
>         }
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index dc86338b3a5..a114dc0d63d 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -327,8 +327,6 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>                                  &error_abort);
>         object_property_set_uint(OBJECT(&s->esdhc[i]), "capareg",
>                                  IMX6_ESDHC_CAPABILITIES, &error_abort);
> -        object_property_set_uint(OBJECT(&s->esdhc[i]), "vendor",
> -                                 SDHCI_VENDOR_IMX, &error_abort);
>         if (!sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), errp)) {
>             return;
>         }
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 34c4aa15cd0..ce8d3ef535f 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -531,8 +531,6 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>             FSL_IMX6UL_USDHC2_IRQ,
>         };
>
> -        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
> -                                 SDHCI_VENDOR_IMX, &error_abort);
>         sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), &error_abort);
>
>         sysbus_mmio_map(SYS_BUS_DEVICE(&s->usdhc[i]), 0,
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 3374018cde0..ed1f10bca26 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -471,8 +471,6 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>             FSL_IMX7_USDHC3_IRQ,
>         };
>
> -        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
> -                                 SDHCI_VENDOR_IMX, &error_abort);
>         sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), &error_abort);
>
>         sysbus_mmio_map(SYS_BUS_DEVICE(&s->usdhc[i]), 0,
> diff --git a/hw/arm/fsl-imx8mp.c b/hw/arm/fsl-imx8mp.c
> index 1ea98e14635..c3f6da63220 100644
> --- a/hw/arm/fsl-imx8mp.c
> +++ b/hw/arm/fsl-imx8mp.c
> @@ -524,8 +524,6 @@ static void fsl_imx8mp_realize(DeviceState *dev, Error **errp)
>             { fsl_imx8mp_memmap[FSL_IMX8MP_USDHC3].addr, FSL_IMX8MP_USDHC3_IRQ },
>         };
>
> -        object_property_set_uint(OBJECT(&s->usdhc[i]), "vendor",
> -                                 SDHCI_VENDOR_IMX, &error_abort);
>         if (!sysbus_realize(SYS_BUS_DEVICE(&s->usdhc[i]), errp)) {
>             return;
>         }
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index fe87e18d5d2..69baf73ae9b 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1735,16 +1735,10 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>
>     case USDHC_VENDOR_SPEC:
>         s->vendor_spec = value;
> -        switch (s->vendor) {
> -        case SDHCI_VENDOR_IMX:
> -            if (value & USDHC_IMX_FRC_SDCLK_ON) {
> -                s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
> -            } else {
> -                s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
> -            }
> -            break;
> -        default:
> -            break;
> +        if (value & USDHC_IMX_FRC_SDCLK_ON) {
> +            s->prnsts &= ~SDHC_IMX_CLOCK_GATE_OFF;
> +        } else {
> +            s->prnsts |= SDHC_IMX_CLOCK_GATE_OFF;
>         }
>         break;
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-03-11 21:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-11 19:51 [PULL 00/14] Misc HW patches for 2025-03-11 Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 01/14] hw/sd/sdhci: Set reset value of interrupt registers Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 02/14] hw/rtc: Add Ricoh RS5C372 RTC emulation Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 03/14] hw/net/smc91c111: Sanitize packet numbers Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 04/14] hw/net/smc91c111: Sanitize packet length on tx Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 05/14] hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 06/14] hw/net/smc91c111: Don't allow data register access to overrun buffer Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 07/14] hw/xen/hvm: Fix Aarch64 typo Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 08/14] system: Extract target-specific globals to their own compilation unit Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 09/14] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 10/14] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 11/14] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 12/14] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 13/14] hw/hyperv/hyperv-proto: Move SYNDBG definitions from target/i386 Philippe Mathieu-Daudé
2025-03-11 19:51 ` [PULL 14/14] hw/sd/sdhci: Remove need for SDHCI_VENDOR_FSL definition Philippe Mathieu-Daudé
2025-03-11 21:00   ` BALATON Zoltan

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