[Qemu-devel] [PATCH 00/15] another SysBusDevice::init to Device::realize cleanup

[Qemu-devel] [PATCH 00/15] another SysBusDevice::init to Device::realize cleanup

[Philippe Mathieu-Daudé]

Peter suggested [1] another crusade for this merge window,
then Cédric jumped on his horse [2]. My turn on my dromedary.

- convert few devices to DeviceState::realize,
- kill the empty_slot device,
- remove unuseful class_init() code [RFC, do we want to keep this?]
- few other minor fixes catched while editing

Regards,

Phil.

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg03605.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00001.html

Philippe Mathieu-Daudé (15):
  trace-events: Fix copy/paste typo
  hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events
  hw/timer/sun4v-rtc: Use DeviceState::realize rather than
    SysBusDevice::init
  hw/ssi/xilinx_spi: Use DeviceState::realize rather than
    SysBusDevice::init
  hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init
  hw/pci-host/bonito: Use DeviceState::realize rather than
    SysBusDevice::init
  hw/mips/gt64xxx_pci: Convert gt64120_reset() function into Device
    reset method
  hw/mips/gt64xxx_pci: Mark as bridge device
  hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'
  hw/sparc64/niagara: Replace 'empty_slot' by 'unimplemented_device'
  hw/sparc/sun4m: Replace 'empty_slot' by 'unimplemented_device'
  hw/core: Remove the 'empty_slot' device
  hw/alpha/typhoon: Remove unuseful code
  hw/hppa/dino: Remove unuseful code
  hw/mips/malta: Remove unuseful code

 default-configs/mips-softmmu-common.mak |   1 -
 default-configs/sparc-softmmu.mak       |   1 -
 default-configs/sparc64-softmmu.mak     |   1 -
 include/hw/empty_slot.h                 |   7 --
 hw/alpha/typhoon.c                      |  13 ---
 hw/core/empty_slot.c                    | 103 ------------------------
 hw/hppa/dino.c                          |   7 --
 hw/mips/gt64xxx_pci.c                   |  18 +----
 hw/mips/mips_malta.c                    |  17 +---
 hw/pci-host/bonito.c                    |   9 +--
 hw/sh4/sh_pci.c                         |  20 +++--
 hw/sparc/sun4m.c                        |  24 +++---
 hw/sparc64/niagara.c                    |   4 +-
 hw/ssi/xilinx_spi.c                     |   9 +--
 hw/timer/sun4v-rtc.c                    |  23 ++----
 hw/core/Makefile.objs                   |   1 -
 hw/timer/trace-events                   |   6 +-
 17 files changed, 50 insertions(+), 214 deletions(-)
 delete mode 100644 include/hw/empty_slot.h
 delete mode 100644 hw/core/empty_slot.c

-- 
2.19.0

[Qemu-devel] [PATCH 01/15] trace-events: Fix copy/paste typo

[Philippe Mathieu-Daudé]

Missed while reviewing 5dd85b4b486.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index fa4213df5b..ca9ad6321a 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -56,7 +56,7 @@ systick_timer_tick(void) "systick reload"
 systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
 systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
 
-# hw/char/cmsdk_apb_timer.c
+# hw/timer/cmsdk_apb_timer.c
 cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset"
-- 
2.19.0

[Qemu-devel] [PATCH 02/15] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/sun4v-rtc.c  | 13 +++----------
 hw/timer/trace-events |  4 ++++
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
index 310523225f..13be94f8da 100644
--- a/hw/timer/sun4v-rtc.c
+++ b/hw/timer/sun4v-rtc.c
@@ -14,15 +14,8 @@
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/timer/sun4v-rtc.h"
+#include "trace.h"
 
-//#define DEBUG_SUN4V_RTC
-
-#ifdef DEBUG_SUN4V_RTC
-#define DPRINTF(fmt, ...)                                       \
-    do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
 
 #define TYPE_SUN4V_RTC "sun4v_rtc"
 #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
@@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr,
         /* accessing the high 32 bits */
         val >>= 32;
     }
-    DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
+    trace_sun4v_rtc_read(addr, val);
     return val;
 }
 
 static void sun4v_rtc_write(void *opaque, hwaddr addr,
                              uint64_t val, unsigned size)
 {
-    DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
+    trace_sun4v_rtc_read(addr, val);
 }
 
 static const MemoryRegionOps sun4v_rtc_ops = {
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index ca9ad6321a..75bd3b1042 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -66,5 +66,9 @@ cmsdk_apb_dualtimer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK A
 cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB dualtimer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
 
+# hw/timer/sun4v-rtc.c
+sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64
+sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64
+
 # hw/timer/xlnx-zynqmp-rtc.c
 xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
-- 
2.19.0

[Qemu-devel] [PATCH 03/15] hw/timer/sun4v-rtc: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

Move from the legacy SysBusDevice::init method to using DeviceState::realize.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/sun4v-rtc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
index 13be94f8da..4e7f6a1eff 100644
--- a/hw/timer/sun4v-rtc.c
+++ b/hw/timer/sun4v-rtc.c
@@ -63,21 +63,21 @@ void sun4v_rtc_init(hwaddr addr)
     sysbus_mmio_map(s, 0, addr);
 }
 
-static int sun4v_rtc_init1(SysBusDevice *dev)
+static void sun4v_rtc_realize(DeviceState *dev, Error **errp)
 {
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     Sun4vRtc *s = SUN4V_RTC(dev);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &sun4v_rtc_ops, s,
                           "sun4v-rtc", 0x08ULL);
-    sysbus_init_mmio(dev, &s->iomem);
-    return 0;
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 static void sun4v_rtc_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = sun4v_rtc_init1;
+    dc->realize = sun4v_rtc_realize;
 }
 
 static const TypeInfo sun4v_rtc_info = {
-- 
2.19.0

[Qemu-devel] [PATCH 04/15] hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

Move from the legacy SysBusDevice::init method to using DeviceState::realize.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ssi/xilinx_spi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index 83585bc8b2..3dae303d5b 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = {
     }
 };
 
-static int xilinx_spi_init(SysBusDevice *sbd)
+static void xilinx_spi_realize(DeviceState *dev, Error **errp)
 {
-    DeviceState *dev = DEVICE(sbd);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     XilinxSPI *s = XILINX_SPI(dev);
     int i;
 
@@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
 
     fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
     fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_xilinx_spi = {
@@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = {
 static void xilinx_spi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = xilinx_spi_init;
+    dc->realize = xilinx_spi_realize;
     dc->reset = xlx_spi_reset;
     dc->props = xilinx_spi_properties;
     dc->vmsd = &vmstate_xilinx_spi;
-- 
2.19.0

[Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

Move from the legacy SysBusDevice::init method to using DeviceState::realize.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sh4/sh_pci.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index 4ec2e35500..84c52df067 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -120,16 +120,15 @@ static void sh_pci_set_irq(void *opaque, int irq_num, int level)
     qemu_set_irq(pic[irq_num], level);
 }
 
-static int sh_pci_device_init(SysBusDevice *dev)
+static void sh_pci_device_realize(PCIDevice *dev, Error **errp)
 {
-    PCIHostState *phb;
-    SHPCIState *s;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    SHPCIState *s = SH_PCI_HOST_BRIDGE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
     int i;
 
-    s = SH_PCI_HOST_BRIDGE(dev);
-    phb = PCI_HOST_BRIDGE(s);
     for (i = 0; i < 4; i++) {
-        sysbus_init_irq(dev, &s->irq[i]);
+        sysbus_init_irq(sbd, &s->irq[i]);
     }
     phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
                                      sh_pci_set_irq, sh_pci_map_irq,
@@ -143,13 +142,12 @@ static int sh_pci_device_init(SysBusDevice *dev)
                              &s->memconfig_p4, 0, 0x224);
     memory_region_init_alias(&s->isa, OBJECT(s), "sh_pci.isa",
                              get_system_io(), 0, 0x40000);
-    sysbus_init_mmio(dev, &s->memconfig_p4);
-    sysbus_init_mmio(dev, &s->memconfig_a7);
+    sysbus_init_mmio(sbd, &s->memconfig_p4);
+    sysbus_init_mmio(sbd, &s->memconfig_a7);
     s->iobr = 0xfe240000;
     memory_region_add_subregion(get_system_memory(), s->iobr, &s->isa);
 
     s->dev = pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "sh_pci_host");
-    return 0;
 }
 
 static void sh_pci_host_realize(PCIDevice *d, Error **errp)
@@ -187,9 +185,9 @@ static const TypeInfo sh_pci_host_info = {
 
 static void sh_pci_device_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    sdc->init = sh_pci_device_init;
+    k->realize = sh_pci_device_realize;
 }
 
 static const TypeInfo sh_pci_device_info = {
-- 
2.19.0

[Qemu-devel] [PATCH 06/15] hw/pci-host/bonito: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

Move from the legacy SysBusDevice::init method to using DeviceState::realize.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/bonito.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 9868e2eccc..03d1ec33e3 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -595,7 +595,7 @@ static const VMStateDescription vmstate_bonito = {
     }
 };
 
-static int bonito_pcihost_initfn(SysBusDevice *dev)
+static void bonito_pcihost_realize(PCIDevice *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
 
@@ -603,8 +603,6 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
                                      pci_bonito_set_irq, pci_bonito_map_irq,
                                      dev, get_system_memory(), get_system_io(),
                                      0x28, 32, TYPE_PCI_BUS);
-
-    return 0;
 }
 
 static void bonito_realize(PCIDevice *dev, Error **errp)
@@ -684,7 +682,6 @@ PCIBus *bonito_init(qemu_irq *pic)
     pcihost->pic = pic;
     qdev_init_nofail(dev);
 
-    /* set the pcihost pointer before bonito_initfn is called */
     d = pci_create(phb->bus, PCI_DEVFN(0, 0), TYPE_PCI_BONITO);
     s = PCI_BONITO(d);
     s->pcihost = pcihost;
@@ -726,9 +723,9 @@ static const TypeInfo bonito_info = {
 
 static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = bonito_pcihost_initfn;
+    k->realize = bonito_pcihost_realize;
 }
 
 static const TypeInfo bonito_pcihost_info = {
-- 
2.19.0

[Qemu-devel] [PATCH 07/15] hw/mips/gt64xxx_pci: Convert gt64120_reset() function into Device reset method

[Philippe Mathieu-Daudé]

Convert the gt64120_reset() function into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 24ad0ad024..dcd1a66329 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -992,9 +992,9 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
 }
 
 
-static void gt64120_reset(void *opaque)
+static void gt64120_reset(DeviceState *dev)
 {
-    GT64120State *s = opaque;
+    GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
 
     /* FIXME: Malta specific hw assumptions ahead */
 
@@ -1184,16 +1184,6 @@ PCIBus *gt64120_register(qemu_irq *pic)
     return phb->bus;
 }
 
-static int gt64120_init(SysBusDevice *dev)
-{
-    GT64120State *s;
-
-    s = GT64120_PCI_HOST_BRIDGE(dev);
-
-    qemu_register_reset(gt64120_reset, s);
-    return 0;
-}
-
 static void gt64120_pci_realize(PCIDevice *d, Error **errp)
 {
     /* FIXME: Malta specific hw assumptions ahead */
@@ -1241,9 +1231,8 @@ static const TypeInfo gt64120_pci_info = {
 static void gt64120_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
 
-    sdc->init = gt64120_init;
+    dc->reset = gt64120_reset;
     dc->vmsd = &vmstate_gt64120;
 }
 
-- 
2.19.0

[Qemu-devel] [PATCH 08/15] hw/mips/gt64xxx_pci: Mark as bridge device

[Philippe Mathieu-Daudé]

The gt64120 is currently listed as uncategorized device.
Mark it as bridge device.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/gt64xxx_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index dcd1a66329..1cd8aac658 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1232,6 +1232,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->reset = gt64120_reset;
     dc->vmsd = &vmstate_gt64120;
 }
-- 
2.19.0

[Qemu-devel] [PATCH 09/15] hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'

[Philippe Mathieu-Daudé]

The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
however the later use more recent APIs and is more widely used.

Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
maintenance.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/mips-softmmu-common.mak | 1 -
 hw/mips/mips_malta.c                    | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
index fae2347ee7..f9d664e120 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -32,7 +32,6 @@ CONFIG_PFLASH_CFI01=y
 CONFIG_I8259=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
-CONFIG_EMPTY_SLOT=y
 CONFIG_MIPS_CPS=y
 CONFIG_MIPS_ITU=y
 CONFIG_I2C=y
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 40041d5ec0..4ccfa87c35 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -53,7 +53,7 @@
 #include "sysemu/qtest.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/unimp.h"
 #include "sysemu/kvm.h"
 #include "exec/semihost.h"
 #include "hw/mips/cps.h"
@@ -1216,7 +1216,7 @@ void mips_malta_init(MachineState *machine)
     /* The whole address space decoded by the GT-64120A doesn't generate
        exception when accessing invalid memory. Create an empty slot to
        emulate this feature. */
-    empty_slot_init(0, 0x20000000);
+    create_unimplemented_device("gt64120-SysAD", 0, 0x20000000);
 
     qdev_init_nofail(dev);
 
-- 
2.19.0

[Qemu-devel] [PATCH 10/15] hw/sparc64/niagara: Replace 'empty_slot' by 'unimplemented_device'

[Philippe Mathieu-Daudé]

The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
however the later use more recent APIs and is more widely used.

Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
maintenance.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/sparc64-softmmu.mak | 1 -
 hw/sparc64/niagara.c                | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak
index 52edafe547..ce63d47046 100644
--- a/default-configs/sparc64-softmmu.mak
+++ b/default-configs/sparc64-softmmu.mak
@@ -16,5 +16,4 @@ CONFIG_SIMBA=y
 CONFIG_SUNHME=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
-CONFIG_EMPTY_SLOT=y
 CONFIG_SUN4V_RTC=y
diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index 4fa8cb2904..f8a856f611 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -29,7 +29,7 @@
 #include "hw/hw.h"
 #include "hw/boards.h"
 #include "hw/char/serial.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/unimp.h"
 #include "hw/loader.h"
 #include "hw/sparc/sparc64.h"
 #include "hw/timer/sun4v-rtc.h"
@@ -161,7 +161,7 @@ static void niagara_init(MachineState *machine)
         serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
                        serial_hd(0), DEVICE_BIG_ENDIAN);
     }
-    empty_slot_init(NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
+    create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
     sun4v_rtc_init(NIAGARA_RTC_BASE);
 }
 
-- 
2.19.0

[Qemu-devel] [PATCH 11/15] hw/sparc/sun4m: Replace 'empty_slot' by 'unimplemented_device'

[Philippe Mathieu-Daudé]

The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
however the later use more recent APIs and is more widely used.

Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
maintenance.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/sparc-softmmu.mak |  1 -
 hw/sparc/sun4m.c                  | 24 ++++++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
index 12f97eeb20..7369b54467 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -8,7 +8,6 @@ CONFIG_ESCC=y
 CONFIG_M48T59=y
 CONFIG_PTIMER=y
 CONFIG_FDC=y
-CONFIG_EMPTY_SLOT=y
 CONFIG_PCNET_COMMON=y
 CONFIG_LANCE=y
 CONFIG_TCX=y
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 3c29b68e67..ca37acf7af 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -42,7 +42,7 @@
 #include "hw/nvram/chrp_nvram.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/char/escc.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/unimp.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "trace.h"
@@ -863,7 +863,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     ram_init(0, machine->ram_size, hwdef->max_mem);
     /* models without ECC don't trap when missing ram is accessed */
     if (!hwdef->ecc_base) {
-        empty_slot_init(machine->ram_size, hwdef->max_mem - machine->ram_size);
+        create_unimplemented_device("ecc", machine->ram_size,
+                                    hwdef->max_mem - machine->ram_size);
     }
 
     prom_init(hwdef->slavio_base, bios_name);
@@ -892,9 +893,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     if (hwdef->iommu_pad_base) {
         /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
            Software shouldn't use aliased addresses, neither should it crash
-           when does. Using empty_slot instead of aliasing can help with
-           debugging such accesses */
-        empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
+           when does. Using the 'unimplemented device' instead of aliasing can
+           help with debugging such accesses */
+        create_unimplemented_device("iommu.alias", hwdef->iommu_pad_base,
+                                    hwdef->iommu_pad_len);
     }
 
     sparc32_dma_init(hwdef->dma_base,
@@ -944,12 +946,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     for (i = num_vsimms; i < MAX_VSIMMS; i++) {
         /* vsimm registers probed by OBP */
         if (hwdef->vsimm[i].reg_base) {
-            empty_slot_init(hwdef->vsimm[i].reg_base, 0x2000);
+            create_unimplemented_device("vsimm", hwdef->vsimm[i].reg_base,
+                                        0x2000);
         }
     }
 
     if (hwdef->sx_base) {
-        empty_slot_init(hwdef->sx_base, 0x2000);
+        create_unimplemented_device("sx", hwdef->sx_base, 0x2000);
     }
 
     nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
@@ -1012,14 +1015,15 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     if (hwdef->dbri_base) {
         /* ISDN chip with attached CS4215 audio codec */
         /* prom space */
-        empty_slot_init(hwdef->dbri_base+0x1000, 0x30);
+        create_unimplemented_device("dbri.prom", hwdef->dbri_base + 0x1000,
+                                    0x30);
         /* reg space */
-        empty_slot_init(hwdef->dbri_base+0x10000, 0x100);
+        create_unimplemented_device("dbri", hwdef->dbri_base + 0x10000, 0x100);
     }
 
     if (hwdef->bpp_base) {
         /* parallel port */
-        empty_slot_init(hwdef->bpp_base, 0x20);
+        create_unimplemented_device("bpp", hwdef->bpp_base, 0x20);
     }
 
     kernel_size = sun4m_load_kernel(machine->kernel_filename,
-- 
2.19.0

[Qemu-devel] [PATCH 12/15] hw/core: Remove the 'empty_slot' device

[Philippe Mathieu-Daudé]

All previous users of TYPE_EMPTY_SLOT now use TYPE_UNIMPLEMENTED_DEVICE.

Since TYPE_EMPTY_SLOT is no more used/referenced, remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/empty_slot.h |   7 ---
 hw/core/empty_slot.c    | 103 ----------------------------------------
 hw/core/Makefile.objs   |   1 -
 3 files changed, 111 deletions(-)
 delete mode 100644 include/hw/empty_slot.h
 delete mode 100644 hw/core/empty_slot.c

diff --git a/include/hw/empty_slot.h b/include/hw/empty_slot.h
deleted file mode 100644
index 123a9f8989..0000000000
--- a/include/hw/empty_slot.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef HW_EMPTY_SLOT_H
-#define HW_EMPTY_SLOT_H
-
-/* empty_slot.c */
-void empty_slot_init(hwaddr addr, uint64_t slot_size);
-
-#endif
diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
deleted file mode 100644
index c1b9c2b104..0000000000
--- a/hw/core/empty_slot.c
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * QEMU Empty Slot
- *
- * The empty_slot device emulates known to a bus but not connected devices.
- *
- * Copyright (c) 2010 Artyom Tarasenko
- *
- * This code is licensed under the GNU GPL v2 or (at your option) any later
- * version.
- */
-
-#include "qemu/osdep.h"
-#include "hw/hw.h"
-#include "hw/sysbus.h"
-#include "hw/empty_slot.h"
-
-//#define DEBUG_EMPTY_SLOT
-
-#ifdef DEBUG_EMPTY_SLOT
-#define DPRINTF(fmt, ...)                                       \
-    do { printf("empty_slot: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
-
-#define TYPE_EMPTY_SLOT "empty_slot"
-#define EMPTY_SLOT(obj) OBJECT_CHECK(EmptySlot, (obj), TYPE_EMPTY_SLOT)
-
-typedef struct EmptySlot {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    uint64_t size;
-} EmptySlot;
-
-static uint64_t empty_slot_read(void *opaque, hwaddr addr,
-                                unsigned size)
-{
-    DPRINTF("read from " TARGET_FMT_plx "\n", addr);
-    return 0;
-}
-
-static void empty_slot_write(void *opaque, hwaddr addr,
-                             uint64_t val, unsigned size)
-{
-    DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
-}
-
-static const MemoryRegionOps empty_slot_ops = {
-    .read = empty_slot_read,
-    .write = empty_slot_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-void empty_slot_init(hwaddr addr, uint64_t slot_size)
-{
-    if (slot_size > 0) {
-        /* Only empty slots larger than 0 byte need handling. */
-        DeviceState *dev;
-        SysBusDevice *s;
-        EmptySlot *e;
-
-        dev = qdev_create(NULL, TYPE_EMPTY_SLOT);
-        s = SYS_BUS_DEVICE(dev);
-        e = EMPTY_SLOT(dev);
-        e->size = slot_size;
-
-        qdev_init_nofail(dev);
-
-        sysbus_mmio_map(s, 0, addr);
-    }
-}
-
-static int empty_slot_init1(SysBusDevice *dev)
-{
-    EmptySlot *s = EMPTY_SLOT(dev);
-
-    memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
-                          "empty-slot", s->size);
-    sysbus_init_mmio(dev, &s->iomem);
-    return 0;
-}
-
-static void empty_slot_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-    k->init = empty_slot_init1;
-}
-
-static const TypeInfo empty_slot_info = {
-    .name          = TYPE_EMPTY_SLOT,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(EmptySlot),
-    .class_init    = empty_slot_class_init,
-};
-
-static void empty_slot_register_types(void)
-{
-    type_register_static(&empty_slot_info);
-}
-
-type_init(empty_slot_register_types)
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index eb88ca979e..fd75172a21 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -8,7 +8,6 @@ common-obj-y += irq.o
 common-obj-y += hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += nmi.o
 
-common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
-- 
2.19.0

[Qemu-devel] [RFC PATCH 13/15] hw/alpha/typhoon: Remove unuseful code

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/alpha/typhoon.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index d74b5b55e1..8004afe45b 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -932,23 +932,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
     return b;
 }
 
-static int typhoon_pcihost_init(SysBusDevice *dev)
-{
-    return 0;
-}
-
-static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-    k->init = typhoon_pcihost_init;
-}
-
 static const TypeInfo typhoon_pcihost_info = {
     .name          = TYPE_TYPHOON_PCI_HOST_BRIDGE,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(TyphoonState),
-    .class_init    = typhoon_pcihost_class_init,
 };
 
 static void typhoon_iommu_memory_region_class_init(ObjectClass *klass,
-- 
2.19.0

[Qemu-devel] [RFC PATCH 14/15] hw/hppa/dino: Remove unuseful code

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/hppa/dino.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 564b938e3a..31e09942b5 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -488,17 +488,10 @@ PCIBus *dino_init(MemoryRegion *addr_space,
     return b;
 }
 
-static int dino_pcihost_init(SysBusDevice *dev)
-{
-    return 0;
-}
-
 static void dino_pcihost_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = dino_pcihost_init;
     dc->vmsd = &vmstate_dino;
 }
 
-- 
2.19.0

[Qemu-devel] [RFC PATCH 15/15] hw/mips/malta: Remove unuseful code

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/mips_malta.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 4ccfa87c35..b6633fa141 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1422,23 +1422,10 @@ void mips_malta_init(MachineState *machine)
     pci_vga_init(pci_bus);
 }
 
-static int mips_malta_sysbus_device_init(SysBusDevice *sysbusdev)
-{
-    return 0;
-}
-
-static void mips_malta_class_init(ObjectClass *klass, void *data)
-{
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-
-    k->init = mips_malta_sysbus_device_init;
-}
-
 static const TypeInfo mips_malta_device = {
     .name          = TYPE_MIPS_MALTA,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(MaltaState),
-    .class_init    = mips_malta_class_init,
 };
 
 static void mips_malta_machine_init(MachineClass *mc)
-- 
2.19.0

Re: [Qemu-devel] [PATCH 11/15] hw/sparc/sun4m: Replace 'empty_slot' by 'unimplemented_device'

[Artyom Tarasenko]

Hi Philippe,

On Tue, Oct 2, 2018 at 12:10 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
> however the later use more recent APIs and is more widely used.
>
> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
> maintenance.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/sparc-softmmu.mak |  1 -
>  hw/sparc/sun4m.c                  | 24 ++++++++++++++----------
>  2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
> index 12f97eeb20..7369b54467 100644
> --- a/default-configs/sparc-softmmu.mak
> +++ b/default-configs/sparc-softmmu.mak
> @@ -8,7 +8,6 @@ CONFIG_ESCC=y
>  CONFIG_M48T59=y
>  CONFIG_PTIMER=y
>  CONFIG_FDC=y
> -CONFIG_EMPTY_SLOT=y
>  CONFIG_PCNET_COMMON=y
>  CONFIG_LANCE=y
>  CONFIG_TCX=y
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 3c29b68e67..ca37acf7af 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -42,7 +42,7 @@
>  #include "hw/nvram/chrp_nvram.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/char/escc.h"
> -#include "hw/empty_slot.h"
> +#include "hw/misc/unimp.h"
>  #include "hw/loader.h"
>  #include "elf.h"
>  #include "trace.h"
> @@ -863,7 +863,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      ram_init(0, machine->ram_size, hwdef->max_mem);
>      /* models without ECC don't trap when missing ram is accessed */
>      if (!hwdef->ecc_base) {
> -        empty_slot_init(machine->ram_size, hwdef->max_mem - machine->ram_size);
> +        create_unimplemented_device("ecc", machine->ram_size,
> +                                    hwdef->max_mem - machine->ram_size);

In this particular case of find the name UNIMPLEMENTED_DEVICE a bit
misleading: there is nothing to implement for non-installed memory
SIMMs. It's rather an unconnected device. And the name "ecc" is
misleading too. These machines are not supposed to have any ECC.
Maybe "unconnected-ram", "empty-ram-slot" or "empty-slot"?

Regards,
Artyom

>      }
>
>      prom_init(hwdef->slavio_base, bios_name);
> @@ -892,9 +893,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      if (hwdef->iommu_pad_base) {
>          /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
>             Software shouldn't use aliased addresses, neither should it crash
> -           when does. Using empty_slot instead of aliasing can help with
> -           debugging such accesses */
> -        empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
> +           when does. Using the 'unimplemented device' instead of aliasing can
> +           help with debugging such accesses */
> +        create_unimplemented_device("iommu.alias", hwdef->iommu_pad_base,
> +                                    hwdef->iommu_pad_len);
>      }
>
>      sparc32_dma_init(hwdef->dma_base,
> @@ -944,12 +946,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      for (i = num_vsimms; i < MAX_VSIMMS; i++) {
>          /* vsimm registers probed by OBP */
>          if (hwdef->vsimm[i].reg_base) {
> -            empty_slot_init(hwdef->vsimm[i].reg_base, 0x2000);
> +            create_unimplemented_device("vsimm", hwdef->vsimm[i].reg_base,
> +                                        0x2000);
>          }
>      }
>
>      if (hwdef->sx_base) {
> -        empty_slot_init(hwdef->sx_base, 0x2000);
> +        create_unimplemented_device("sx", hwdef->sx_base, 0x2000);
>      }
>
>      nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
> @@ -1012,14 +1015,15 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      if (hwdef->dbri_base) {
>          /* ISDN chip with attached CS4215 audio codec */
>          /* prom space */
> -        empty_slot_init(hwdef->dbri_base+0x1000, 0x30);
> +        create_unimplemented_device("dbri.prom", hwdef->dbri_base + 0x1000,
> +                                    0x30);
>          /* reg space */
> -        empty_slot_init(hwdef->dbri_base+0x10000, 0x100);
> +        create_unimplemented_device("dbri", hwdef->dbri_base + 0x10000, 0x100);
>      }
>
>      if (hwdef->bpp_base) {
>          /* parallel port */
> -        empty_slot_init(hwdef->bpp_base, 0x20);
> +        create_unimplemented_device("bpp", hwdef->bpp_base, 0x20);
>      }
>
>      kernel_size = sun4m_load_kernel(machine->kernel_filename,
> --
> 2.19.0
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [RFC PATCH 13/15] hw/alpha/typhoon: Remove unuseful code

[Thomas Huth]

On 2018-10-02 00:09, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/alpha/typhoon.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index d74b5b55e1..8004afe45b 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -932,23 +932,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>      return b;
>  }
>  
> -static int typhoon_pcihost_init(SysBusDevice *dev)
> -{
> -    return 0;
> -}
> -
> -static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
> -{
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> -
> -    k->init = typhoon_pcihost_init;
> -}
> -
>  static const TypeInfo typhoon_pcihost_info = {
>      .name          = TYPE_TYPHOON_PCI_HOST_BRIDGE,
>      .parent        = TYPE_PCI_HOST_BRIDGE,
>      .instance_size = sizeof(TyphoonState),
> -    .class_init    = typhoon_pcihost_class_init,
>  };
>  
>  static void typhoon_iommu_memory_region_class_init(ObjectClass *klass,

As far as I can see, this should be fine.

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

Re: [Qemu-devel] [RFC PATCH 14/15] hw/hppa/dino: Remove unuseful code

[Thomas Huth]

On 2018-10-02 00:09, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/hppa/dino.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
> index 564b938e3a..31e09942b5 100644
> --- a/hw/hppa/dino.c
> +++ b/hw/hppa/dino.c
> @@ -488,17 +488,10 @@ PCIBus *dino_init(MemoryRegion *addr_space,
>      return b;
>  }
>  
> -static int dino_pcihost_init(SysBusDevice *dev)
> -{
> -    return 0;
> -}
> -
>  static void dino_pcihost_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    k->init = dino_pcihost_init;
>      dc->vmsd = &vmstate_dino;
>  }

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

Re: [Qemu-devel] [RFC PATCH 15/15] hw/mips/malta: Remove unuseful code

[Thomas Huth]

On 2018-10-02 00:09, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/mips_malta.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 4ccfa87c35..b6633fa141 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1422,23 +1422,10 @@ void mips_malta_init(MachineState *machine)
>      pci_vga_init(pci_bus);
>  }
>  
> -static int mips_malta_sysbus_device_init(SysBusDevice *sysbusdev)
> -{
> -    return 0;
> -}
> -
> -static void mips_malta_class_init(ObjectClass *klass, void *data)
> -{
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> -
> -    k->init = mips_malta_sysbus_device_init;
> -}
> -
>  static const TypeInfo mips_malta_device = {
>      .name          = TYPE_MIPS_MALTA,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(MaltaState),
> -    .class_init    = mips_malta_class_init,
>  };
>  
>  static void mips_malta_machine_init(MachineClass *mc)

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

Re: [Qemu-devel] [PATCH 03/15] hw/timer/sun4v-rtc: Use DeviceState::realize rather than SysBusDevice::init

[Thomas Huth]

On 2018-10-02 00:09, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/sun4v-rtc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> index 13be94f8da..4e7f6a1eff 100644
> --- a/hw/timer/sun4v-rtc.c
> +++ b/hw/timer/sun4v-rtc.c
> @@ -63,21 +63,21 @@ void sun4v_rtc_init(hwaddr addr)
>      sysbus_mmio_map(s, 0, addr);
>  }
>  
> -static int sun4v_rtc_init1(SysBusDevice *dev)
> +static void sun4v_rtc_realize(DeviceState *dev, Error **errp)
>  {
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      Sun4vRtc *s = SUN4V_RTC(dev);
>  
>      memory_region_init_io(&s->iomem, OBJECT(s), &sun4v_rtc_ops, s,
>                            "sun4v-rtc", 0x08ULL);
> -    sysbus_init_mmio(dev, &s->iomem);
> -    return 0;
> +    sysbus_init_mmio(sbd, &s->iomem);
>  }
>  
>  static void sun4v_rtc_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    k->init = sun4v_rtc_init1;
> +    dc->realize = sun4v_rtc_realize;
>  }
>  
>  static const TypeInfo sun4v_rtc_info = {

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

Re: [Qemu-devel] [PATCH 04/15] hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init

[Thomas Huth]

On 2018-10-02 00:09, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/ssi/xilinx_spi.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
> index 83585bc8b2..3dae303d5b 100644
> --- a/hw/ssi/xilinx_spi.c
> +++ b/hw/ssi/xilinx_spi.c
> @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = {
>      }
>  };
>  
> -static int xilinx_spi_init(SysBusDevice *sbd)
> +static void xilinx_spi_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(sbd);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      XilinxSPI *s = XILINX_SPI(dev);
>      int i;
>  
> @@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
>  
>      fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
>      fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
> -
> -    return 0;
>  }
>  
>  static const VMStateDescription vmstate_xilinx_spi = {
> @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = {
>  static void xilinx_spi_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = xilinx_spi_init;
> +    dc->realize = xilinx_spi_realize;
>      dc->reset = xlx_spi_reset;
>      dc->props = xilinx_spi_properties;
>      dc->vmsd = &vmstate_xilinx_spi;
> 

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

Re: [Qemu-devel] [PATCH 02/15] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Artyom Tarasenko]

On Tue, Oct 2, 2018 at 12:10 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

> ---
>  hw/timer/sun4v-rtc.c  | 13 +++----------
>  hw/timer/trace-events |  4 ++++
>  2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> index 310523225f..13be94f8da 100644
> --- a/hw/timer/sun4v-rtc.c
> +++ b/hw/timer/sun4v-rtc.c
> @@ -14,15 +14,8 @@
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "hw/timer/sun4v-rtc.h"
> +#include "trace.h"
>
> -//#define DEBUG_SUN4V_RTC
> -
> -#ifdef DEBUG_SUN4V_RTC
> -#define DPRINTF(fmt, ...)                                       \
> -    do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) do {} while (0)
> -#endif
>
>  #define TYPE_SUN4V_RTC "sun4v_rtc"
>  #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
> @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr,
>          /* accessing the high 32 bits */
>          val >>= 32;
>      }
> -    DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
> +    trace_sun4v_rtc_read(addr, val);
>      return val;
>  }
>
>  static void sun4v_rtc_write(void *opaque, hwaddr addr,
>                               uint64_t val, unsigned size)
>  {
> -    DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
> +    trace_sun4v_rtc_read(addr, val);
>  }
>
>  static const MemoryRegionOps sun4v_rtc_ops = {
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index ca9ad6321a..75bd3b1042 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -66,5 +66,9 @@ cmsdk_apb_dualtimer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK A
>  cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB dualtimer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
>
> +# hw/timer/sun4v-rtc.c
> +sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64
> +sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64
> +
>  # hw/timer/xlnx-zynqmp-rtc.c
>  xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
> --
> 2.19.0
>


-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 01/15] trace-events: Fix copy/paste typo

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Missed while reviewing 5dd85b4b486.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index fa4213df5b..ca9ad6321a 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -56,7 +56,7 @@ systick_timer_tick(void) "systick reload"
>  systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>  systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>
> -# hw/char/cmsdk_apb_timer.c
> +# hw/timer/cmsdk_apb_timer.c
>  cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset"
> --
> 2.19.0

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I think I saw this in the trace file before going on holiday
and then forgot to send out a fix; thanks for the patch.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.

Comment says DeviceState::realize but the code is using
PCIDevice::realize ?

I didn't realize pci devices had their own realize method:
what's the difference between it and plain old DeviceState::realize,
which they also have, since PCIDeviceClass inherits from DeviceClass ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 07/15] hw/mips/gt64xxx_pci: Convert gt64120_reset() function into Device reset method

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Convert the gt64120_reset() function into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/gt64xxx_pci.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 08/15] hw/mips/gt64xxx_pci: Mark as bridge device

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The gt64120 is currently listed as uncategorized device.
> Mark it as bridge device.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/gt64xxx_pci.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Should this device be in hw/pci-host/ with the other PCI
host controller models ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 09/15] hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
> however the later use more recent APIs and is more widely used.
>
> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
> maintenance.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/mips-softmmu-common.mak | 1 -
>  hw/mips/mips_malta.c                    | 4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
> index fae2347ee7..f9d664e120 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -32,7 +32,6 @@ CONFIG_PFLASH_CFI01=y
>  CONFIG_I8259=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
> -CONFIG_EMPTY_SLOT=y
>  CONFIG_MIPS_CPS=y
>  CONFIG_MIPS_ITU=y
>  CONFIG_I2C=y
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 40041d5ec0..4ccfa87c35 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -53,7 +53,7 @@
>  #include "sysemu/qtest.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> -#include "hw/empty_slot.h"
> +#include "hw/misc/unimp.h"
>  #include "sysemu/kvm.h"
>  #include "exec/semihost.h"
>  #include "hw/mips/cps.h"
> @@ -1216,7 +1216,7 @@ void mips_malta_init(MachineState *machine)
>      /* The whole address space decoded by the GT-64120A doesn't generate
>         exception when accessing invalid memory. Create an empty slot to
>         emulate this feature. */
> -    empty_slot_init(0, 0x20000000);
> +    create_unimplemented_device("gt64120-SysAD", 0, 0x20000000);
>
>      qdev_init_nofail(dev);

Not sure about this one. unimplemented_device means "there should
be something here, but QEMU's model is incomplete", and if the user
asks for 'unimp' warnings via the -d option then all accesses will
generate logging. In this MIPS board, we're modelling the hardware's
actual behaviour, and we shouldn't generate debug messages that
imply that QEMU has unimplemented functionality here.

If we were writing a model of the Malta board from scratch we'd
probably do it by having the GT-64120A be modelled as a device
which was a container object that instantiated all its various
bits and pieces and mapped them into a MemoryRegion that spanned
the whole 1GB size of that part of the address space. We could
then give that MR a background region with the "RAZ/WI" behaviour
the hardware requires.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 10/15] hw/sparc64/niagara: Replace 'empty_slot' by 'unimplemented_device'

[Peter Maydell]

On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
> however the later use more recent APIs and is more widely used.
>
> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
> maintenance.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---

> @@ -161,7 +161,7 @@ static void niagara_init(MachineState *machine)
>          serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
>                         serial_hd(0), DEVICE_BIG_ENDIAN);
>      }
> -    empty_slot_init(NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
> +    create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
>      sun4v_rtc_init(NIAGARA_RTC_BASE);
>  }

Is this actually an unimplemented (missing) device, or are we
implementing hardware-defined "no bus errors when this range is
touched" behaviour ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 02/15] hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/timer/sun4v-rtc.c  | 13 +++----------
>  hw/timer/trace-events |  4 ++++
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> index 310523225f..13be94f8da 100644
> --- a/hw/timer/sun4v-rtc.c
> +++ b/hw/timer/sun4v-rtc.c
> @@ -14,15 +14,8 @@
>  #include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "hw/timer/sun4v-rtc.h"
> +#include "trace.h"
>  
> -//#define DEBUG_SUN4V_RTC
> -
> -#ifdef DEBUG_SUN4V_RTC
> -#define DPRINTF(fmt, ...)                                       \
> -    do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) do {} while (0)
> -#endif
>  
>  #define TYPE_SUN4V_RTC "sun4v_rtc"
>  #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC)
> @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr,
>          /* accessing the high 32 bits */
>          val >>= 32;
>      }
> -    DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val);
> +    trace_sun4v_rtc_read(addr, val);
>      return val;
>  }
>  
>  static void sun4v_rtc_write(void *opaque, hwaddr addr,
>                               uint64_t val, unsigned size)
>  {
> -    DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr);
> +    trace_sun4v_rtc_read(addr, val);
>  }
>  
>  static const MemoryRegionOps sun4v_rtc_ops = {
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index ca9ad6321a..75bd3b1042 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -66,5 +66,9 @@ cmsdk_apb_dualtimer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK A
>  cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB dualtimer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
>  
> +# hw/timer/sun4v-rtc.c
> +sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64
> +sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64
> +
>  # hw/timer/xlnx-zynqmp-rtc.c
>  xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
> 

Re: [Qemu-devel] [PATCH 03/15] hw/timer/sun4v-rtc: Use DeviceState::realize rather than SysBusDevice::init

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/timer/sun4v-rtc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c
> index 13be94f8da..4e7f6a1eff 100644
> --- a/hw/timer/sun4v-rtc.c
> +++ b/hw/timer/sun4v-rtc.c
> @@ -63,21 +63,21 @@ void sun4v_rtc_init(hwaddr addr)
>      sysbus_mmio_map(s, 0, addr);
>  }
>  
> -static int sun4v_rtc_init1(SysBusDevice *dev)
> +static void sun4v_rtc_realize(DeviceState *dev, Error **errp)
>  {
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      Sun4vRtc *s = SUN4V_RTC(dev);
>  
>      memory_region_init_io(&s->iomem, OBJECT(s), &sun4v_rtc_ops, s,
>                            "sun4v-rtc", 0x08ULL);
> -    sysbus_init_mmio(dev, &s->iomem);
> -    return 0;
> +    sysbus_init_mmio(sbd, &s->iomem);
>  }
>  
>  static void sun4v_rtc_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    k->init = sun4v_rtc_init1;
> +    dc->realize = sun4v_rtc_realize;
>  }
>  
>  static const TypeInfo sun4v_rtc_info = {
> 

Re: [Qemu-devel] [PATCH 04/15] hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ssi/xilinx_spi.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
> index 83585bc8b2..3dae303d5b 100644
> --- a/hw/ssi/xilinx_spi.c
> +++ b/hw/ssi/xilinx_spi.c
> @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = {
>      }
>  };
>  
> -static int xilinx_spi_init(SysBusDevice *sbd)
> +static void xilinx_spi_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(sbd);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      XilinxSPI *s = XILINX_SPI(dev);
>      int i;
>  
> @@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
>  
>      fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
>      fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
> -
> -    return 0;
>  }
>  
>  static const VMStateDescription vmstate_xilinx_spi = {
> @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = {
>  static void xilinx_spi_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = xilinx_spi_init;
> +    dc->realize = xilinx_spi_realize;
>      dc->reset = xlx_spi_reset;
>      dc->props = xilinx_spi_properties;
>      dc->vmsd = &vmstate_xilinx_spi;
> 

Re: [Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

one question below,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/sh4/sh_pci.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 4ec2e35500..84c52df067 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -120,16 +120,15 @@ static void sh_pci_set_irq(void *opaque, int irq_num, int level)
>      qemu_set_irq(pic[irq_num], level);
>  }
>  
> -static int sh_pci_device_init(SysBusDevice *dev)
> +static void sh_pci_device_realize(PCIDevice *dev, Error **errp)
>  {
> -    PCIHostState *phb;
> -    SHPCIState *s;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    SHPCIState *s = SH_PCI_HOST_BRIDGE(dev);
> +    PCIHostState *phb = PCI_HOST_BRIDGE(s);
>      int i;
>  
> -    s = SH_PCI_HOST_BRIDGE(dev);
> -    phb = PCI_HOST_BRIDGE(s);
>      for (i = 0; i < 4; i++) {

is that  PCI_NUM_PINS ? 

> -        sysbus_init_irq(dev, &s->irq[i]);
> +        sysbus_init_irq(sbd, &s->irq[i]);
>      }
>      phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
>                                       sh_pci_set_irq, sh_pci_map_irq,
> @@ -143,13 +142,12 @@ static int sh_pci_device_init(SysBusDevice *dev)
>                               &s->memconfig_p4, 0, 0x224);
>      memory_region_init_alias(&s->isa, OBJECT(s), "sh_pci.isa",
>                               get_system_io(), 0, 0x40000);
> -    sysbus_init_mmio(dev, &s->memconfig_p4);
> -    sysbus_init_mmio(dev, &s->memconfig_a7);
> +    sysbus_init_mmio(sbd, &s->memconfig_p4);
> +    sysbus_init_mmio(sbd, &s->memconfig_a7);
>      s->iobr = 0xfe240000;
>      memory_region_add_subregion(get_system_memory(), s->iobr, &s->isa);
>  
>      s->dev = pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "sh_pci_host");
> -    return 0;
>  }
>  
>  static void sh_pci_host_realize(PCIDevice *d, Error **errp)
> @@ -187,9 +185,9 @@ static const TypeInfo sh_pci_host_info = {
>  
>  static void sh_pci_device_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    sdc->init = sh_pci_device_init;
> +    k->realize = sh_pci_device_realize;
>  }
>  
>  static const TypeInfo sh_pci_device_info = {
> 

Re: [Qemu-devel] [PATCH 01/15] trace-events: Fix copy/paste typo

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Missed while reviewing 5dd85b4b486.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/timer/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index fa4213df5b..ca9ad6321a 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -56,7 +56,7 @@ systick_timer_tick(void) "systick reload"
>  systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>  systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
>  
> -# hw/char/cmsdk_apb_timer.c
> +# hw/timer/cmsdk_apb_timer.c
>  cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset"
> 

Re: [Qemu-devel] [PATCH 07/15] hw/mips/gt64xxx_pci: Convert gt64120_reset() function into Device reset method

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Convert the gt64120_reset() function into a proper Device reset method.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/mips/gt64xxx_pci.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 24ad0ad024..dcd1a66329 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -992,9 +992,9 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level)
>  }
>  
>  
> -static void gt64120_reset(void *opaque)
> +static void gt64120_reset(DeviceState *dev)
>  {
> -    GT64120State *s = opaque;
> +    GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
>  
>      /* FIXME: Malta specific hw assumptions ahead */
>  
> @@ -1184,16 +1184,6 @@ PCIBus *gt64120_register(qemu_irq *pic)
>      return phb->bus;
>  }
>  
> -static int gt64120_init(SysBusDevice *dev)
> -{
> -    GT64120State *s;
> -
> -    s = GT64120_PCI_HOST_BRIDGE(dev);
> -
> -    qemu_register_reset(gt64120_reset, s);
> -    return 0;
> -}
> -
>  static void gt64120_pci_realize(PCIDevice *d, Error **errp)
>  {
>      /* FIXME: Malta specific hw assumptions ahead */
> @@ -1241,9 +1231,8 @@ static const TypeInfo gt64120_pci_info = {
>  static void gt64120_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    sdc->init = gt64120_init;
> +    dc->reset = gt64120_reset;
>      dc->vmsd = &vmstate_gt64120;
>  }
>  
> 

Re: [Qemu-devel] [PATCH 08/15] hw/mips/gt64xxx_pci: Mark as bridge device

[Cédric Le Goater]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> The gt64120 is currently listed as uncategorized device.
> Mark it as bridge device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/mips/gt64xxx_pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index dcd1a66329..1cd8aac658 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1232,6 +1232,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>      dc->reset = gt64120_reset;
>      dc->vmsd = &vmstate_gt64120;
>  }
> 

Re: [Qemu-devel] [PATCH 10/15] hw/sparc64/niagara: Replace 'empty_slot' by 'unimplemented_device'

[Artyom Tarasenko]

On Tue, Oct 2, 2018 at 3:24 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
> > however the later use more recent APIs and is more widely used.
> >
> > Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
> > maintenance.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
>
> > @@ -161,7 +161,7 @@ static void niagara_init(MachineState *machine)
> >          serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200,
> >                         serial_hd(0), DEVICE_BIG_ENDIAN);
> >      }
> > -    empty_slot_init(NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
> > +    create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, NIAGARA_IOBSIZE);
> >      sun4v_rtc_init(NIAGARA_RTC_BASE);
> >  }
>
> Is this actually an unimplemented (missing) device, or are we
> implementing hardware-defined "no bus errors when this range is
> touched" behaviour ?

In this case it's really an unimplemented device. But in sun4m (patch
11/15) it's
"no bus errors when this range is touched" behaviour.

Artyom

--
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 09/15] hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'

[Philippe Mathieu-Daudé]

On 10/2/18 3:23 PM, Peter Maydell wrote:
> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
>> however the later use more recent APIs and is more widely used.
>>
>> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
>> maintenance.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  default-configs/mips-softmmu-common.mak | 1 -
>>  hw/mips/mips_malta.c                    | 4 ++--
>>  2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
>> index fae2347ee7..f9d664e120 100644
>> --- a/default-configs/mips-softmmu-common.mak
>> +++ b/default-configs/mips-softmmu-common.mak
>> @@ -32,7 +32,6 @@ CONFIG_PFLASH_CFI01=y
>>  CONFIG_I8259=y
>>  CONFIG_MC146818RTC=y
>>  CONFIG_ISA_TESTDEV=y
>> -CONFIG_EMPTY_SLOT=y
>>  CONFIG_MIPS_CPS=y
>>  CONFIG_MIPS_ITU=y
>>  CONFIG_I2C=y
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index 40041d5ec0..4ccfa87c35 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -53,7 +53,7 @@
>>  #include "sysemu/qtest.h"
>>  #include "qapi/error.h"
>>  #include "qemu/error-report.h"
>> -#include "hw/empty_slot.h"
>> +#include "hw/misc/unimp.h"
>>  #include "sysemu/kvm.h"
>>  #include "exec/semihost.h"
>>  #include "hw/mips/cps.h"
>> @@ -1216,7 +1216,7 @@ void mips_malta_init(MachineState *machine)
>>      /* The whole address space decoded by the GT-64120A doesn't generate
>>         exception when accessing invalid memory. Create an empty slot to
>>         emulate this feature. */
>> -    empty_slot_init(0, 0x20000000);
>> +    create_unimplemented_device("gt64120-SysAD", 0, 0x20000000);
>>
>>      qdev_init_nofail(dev);
> 
> Not sure about this one. unimplemented_device means "there should
> be something here, but QEMU's model is incomplete", and if the user
> asks for 'unimp' warnings via the -d option then all accesses will
> generate logging. In this MIPS board, we're modelling the hardware's
> actual behaviour, and we shouldn't generate debug messages that
> imply that QEMU has unimplemented functionality here.
> 
> If we were writing a model of the Malta board from scratch we'd
> probably do it by having the GT-64120A be modelled as a device
> which was a container object that instantiated all its various
> bits and pieces and mapped them into a MemoryRegion that spanned
> the whole 1GB size of that part of the address space. We could
> then give that MR a background region with the "RAZ/WI" behaviour
> the hardware requires.

Is 'git revert c7c3c9f8' a good example of your explanation?

Re: [Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

On 10/2/18 3:13 PM, Peter Maydell wrote:
> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Comment says DeviceState::realize but the code is using
> PCIDevice::realize ?
> 
> I didn't realize pci devices had their own realize method:
> what's the difference between it and plain old DeviceState::realize,
> which they also have, since PCIDeviceClass inherits from DeviceClass ?

Cc'ing Michael and Marcel.

Re: [Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Marcel Apfelbaum]

Hi Philippe, Peter

On 10/2/18 10:59 PM, Philippe Mathieu-Daudé wrote:
> On 10/2/18 3:13 PM, Peter Maydell wrote:
>> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
>> Comment says DeviceState::realize but the code is using
>> PCIDevice::realize ?
>>
>> I didn't realize pci devices had their own realize method:
>> what's the difference between it and plain old DeviceState::realize,

PCIDevice:realize handles PCI specific "realization" tasks:
* setup of the PCI configuration space
* PCI BARs configuration
* MSI initalization
*...

The PCIDevice's DeviceState::realize function named 'pci_qdev_realize'
calls PCIDevice:realize after it runs some generic initialization code.

It is possible we have this design so we would be able to move from
"qdev" to QOM.

Thanks,
Marcel

>> which they also have, since PCIDeviceClass inherits from DeviceClass ?
> Cc'ing Michael and Marcel.

Re: [Qemu-devel] [PATCH 05/15] hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

On 10/2/18 10:37 PM, Marcel Apfelbaum wrote:
> Hi Philippe, Peter
> 
> On 10/2/18 10:59 PM, Philippe Mathieu-Daudé wrote:
>> On 10/2/18 3:13 PM, Peter Maydell wrote:
>>> On 1 October 2018 at 23:09, Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> wrote:
>>>> Move from the legacy SysBusDevice::init method to using
>>>> DeviceState::realize.
>>> Comment says DeviceState::realize but the code is using
>>> PCIDevice::realize ?
>>>
>>> I didn't realize pci devices had their own realize method:
>>> what's the difference between it and plain old DeviceState::realize,
> 
> PCIDevice:realize handles PCI specific "realization" tasks:
> * setup of the PCI configuration space
> * PCI BARs configuration
> * MSI initalization
> *...
> 
> The PCIDevice's DeviceState::realize function named 'pci_qdev_realize'
> calls PCIDevice:realize after it runs some generic initialization code.
> 
> It is possible we have this design so we would be able to move from
> "qdev" to QOM.

Thanks Marcel, so this patch is incorrect.

TYPE_SH_PCI_HOST_BRIDGE inherits TYPE_PCI_HOST_BRIDGE which inherits
TYPE_SYS_BUS_DEVICE which inherits TYPE_DEVICE, so can implement
DeviceState::realize but not PCIDevice::realize.

TYPE_"sh_pci_host" inherits TYPE_PCI_DEVICE, so can implement
PCIDevice::realize.

I'll respin.

> Thanks,
> Marcel
> 
>>> which they also have, since PCIDeviceClass inherits from DeviceClass ?
>> Cc'ing Michael and Marcel.

Re: [Qemu-devel] [PATCH 06/15] hw/pci-host/bonito: Use DeviceState::realize rather than SysBusDevice::init

[Philippe Mathieu-Daudé]

On 10/2/18 12:09 AM, Philippe Mathieu-Daudé wrote:
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/pci-host/bonito.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 9868e2eccc..03d1ec33e3 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -595,7 +595,7 @@ static const VMStateDescription vmstate_bonito = {
>      }
>  };
>  
> -static int bonito_pcihost_initfn(SysBusDevice *dev)
> +static void bonito_pcihost_realize(PCIDevice *dev, Error **errp)

This patch is incorrect, see:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00378.html

>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>  
> @@ -603,8 +603,6 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
>                                       pci_bonito_set_irq, pci_bonito_map_irq,
>                                       dev, get_system_memory(), get_system_io(),
>                                       0x28, 32, TYPE_PCI_BUS);
> -
> -    return 0;
>  }
>  
>  static void bonito_realize(PCIDevice *dev, Error **errp)
> @@ -684,7 +682,6 @@ PCIBus *bonito_init(qemu_irq *pic)
>      pcihost->pic = pic;
>      qdev_init_nofail(dev);
>  
> -    /* set the pcihost pointer before bonito_initfn is called */
>      d = pci_create(phb->bus, PCI_DEVFN(0, 0), TYPE_PCI_BONITO);
>      s = PCI_BONITO(d);
>      s->pcihost = pcihost;
> @@ -726,9 +723,9 @@ static const TypeInfo bonito_info = {
>  
>  static void bonito_pcihost_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->init = bonito_pcihost_initfn;
> +    k->realize = bonito_pcihost_realize;
>  }
>  
>  static const TypeInfo bonito_pcihost_info = {
> 

Re: [Qemu-devel] [PATCH 11/15] hw/sparc/sun4m: Replace 'empty_slot' by 'unimplemented_device'

[Philippe Mathieu-Daudé]

Hi Artyom,

On 10/2/18 10:14 AM, Artyom Tarasenko wrote:
> Hi Philippe,
> 
> On Tue, Oct 2, 2018 at 12:10 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> The TYPE_EMPTY_SLOT and TYPE_UNIMPLEMENTED_DEVICE are identical devices,
>> however the later use more recent APIs and is more widely used.
>>
>> Replace 'empty_slot' by 'unimplemented_device' to simplify devices code
>> maintenance.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  default-configs/sparc-softmmu.mak |  1 -
>>  hw/sparc/sun4m.c                  | 24 ++++++++++++++----------
>>  2 files changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
>> index 12f97eeb20..7369b54467 100644
>> --- a/default-configs/sparc-softmmu.mak
>> +++ b/default-configs/sparc-softmmu.mak
>> @@ -8,7 +8,6 @@ CONFIG_ESCC=y
>>  CONFIG_M48T59=y
>>  CONFIG_PTIMER=y
>>  CONFIG_FDC=y
>> -CONFIG_EMPTY_SLOT=y
>>  CONFIG_PCNET_COMMON=y
>>  CONFIG_LANCE=y
>>  CONFIG_TCX=y
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 3c29b68e67..ca37acf7af 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -42,7 +42,7 @@
>>  #include "hw/nvram/chrp_nvram.h"
>>  #include "hw/nvram/fw_cfg.h"
>>  #include "hw/char/escc.h"
>> -#include "hw/empty_slot.h"
>> +#include "hw/misc/unimp.h"
>>  #include "hw/loader.h"
>>  #include "elf.h"
>>  #include "trace.h"
>> @@ -863,7 +863,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      ram_init(0, machine->ram_size, hwdef->max_mem);
>>      /* models without ECC don't trap when missing ram is accessed */
>>      if (!hwdef->ecc_base) {
>> -        empty_slot_init(machine->ram_size, hwdef->max_mem - machine->ram_size);
>> +        create_unimplemented_device("ecc", machine->ram_size,
>> +                                    hwdef->max_mem - machine->ram_size);
> 
> In this particular case of find the name UNIMPLEMENTED_DEVICE a bit
> misleading: there is nothing to implement for non-installed memory
> SIMMs. It's rather an unconnected device. And the name "ecc" is
> misleading too. These machines are not supposed to have any ECC.
> Maybe "unconnected-ram", "empty-ram-slot" or "empty-slot"?

OK. I'll rework on this patch later and resend in another series.

Thanks for your review!

Phil.

> Regards,
> Artyom
> 
>>      }
>>
>>      prom_init(hwdef->slavio_base, bios_name);
>> @@ -892,9 +893,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      if (hwdef->iommu_pad_base) {
>>          /* On the real hardware (SS-5, LX) the MMU is not padded, but aliased.
>>             Software shouldn't use aliased addresses, neither should it crash
>> -           when does. Using empty_slot instead of aliasing can help with
>> -           debugging such accesses */
>> -        empty_slot_init(hwdef->iommu_pad_base,hwdef->iommu_pad_len);
>> +           when does. Using the 'unimplemented device' instead of aliasing can
>> +           help with debugging such accesses */
>> +        create_unimplemented_device("iommu.alias", hwdef->iommu_pad_base,
>> +                                    hwdef->iommu_pad_len);
>>      }
>>
>>      sparc32_dma_init(hwdef->dma_base,
>> @@ -944,12 +946,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      for (i = num_vsimms; i < MAX_VSIMMS; i++) {
>>          /* vsimm registers probed by OBP */
>>          if (hwdef->vsimm[i].reg_base) {
>> -            empty_slot_init(hwdef->vsimm[i].reg_base, 0x2000);
>> +            create_unimplemented_device("vsimm", hwdef->vsimm[i].reg_base,
>> +                                        0x2000);
>>          }
>>      }
>>
>>      if (hwdef->sx_base) {
>> -        empty_slot_init(hwdef->sx_base, 0x2000);
>> +        create_unimplemented_device("sx", hwdef->sx_base, 0x2000);
>>      }
>>
>>      nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>> @@ -1012,14 +1015,15 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      if (hwdef->dbri_base) {
>>          /* ISDN chip with attached CS4215 audio codec */
>>          /* prom space */
>> -        empty_slot_init(hwdef->dbri_base+0x1000, 0x30);
>> +        create_unimplemented_device("dbri.prom", hwdef->dbri_base + 0x1000,
>> +                                    0x30);
>>          /* reg space */
>> -        empty_slot_init(hwdef->dbri_base+0x10000, 0x100);
>> +        create_unimplemented_device("dbri", hwdef->dbri_base + 0x10000, 0x100);
>>      }
>>
>>      if (hwdef->bpp_base) {
>>          /* parallel port */
>> -        empty_slot_init(hwdef->bpp_base, 0x20);
>> +        create_unimplemented_device("bpp", hwdef->bpp_base, 0x20);
>>      }
>>
>>      kernel_size = sun4m_load_kernel(machine->kernel_filename,
>> --
>> 2.19.0
>>
> 
> 

Re: [Qemu-devel] [PATCH 09/15] hw/mips/malta: Replace 'empty_slot' by 'unimplemented_device'

[Peter Maydell]

On 2 October 2018 at 20:40, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 10/2/18 3:23 PM, Peter Maydell wrote:
>> Not sure about this one. unimplemented_device means "there should
>> be something here, but QEMU's model is incomplete", and if the user
>> asks for 'unimp' warnings via the -d option then all accesses will
>> generate logging. In this MIPS board, we're modelling the hardware's
>> actual behaviour, and we shouldn't generate debug messages that
>> imply that QEMU has unimplemented functionality here.
>>
>> If we were writing a model of the Malta board from scratch we'd
>> probably do it by having the GT-64120A be modelled as a device
>> which was a container object that instantiated all its various
>> bits and pieces and mapped them into a MemoryRegion that spanned
>> the whole 1GB size of that part of the address space. We could
>> then give that MR a background region with the "RAZ/WI" behaviour
>> the hardware requires.
>
> Is 'git revert c7c3c9f8' a good example of your explanation?

That commit is OK, because the previous version was also
logging accesses as being unimplemented.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/15] hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init

[Alistair Francis]

On Mon, Oct 1, 2018 at 3:32 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ssi/xilinx_spi.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
> index 83585bc8b2..3dae303d5b 100644
> --- a/hw/ssi/xilinx_spi.c
> +++ b/hw/ssi/xilinx_spi.c
> @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = {
>      }
>  };
>
> -static int xilinx_spi_init(SysBusDevice *sbd)
> +static void xilinx_spi_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(sbd);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      XilinxSPI *s = XILINX_SPI(dev);
>      int i;
>
> @@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
>
>      fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
>      fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
> -
> -    return 0;
>  }
>
>  static const VMStateDescription vmstate_xilinx_spi = {
> @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = {
>  static void xilinx_spi_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
> -    k->init = xilinx_spi_init;
> +    dc->realize = xilinx_spi_realize;
>      dc->reset = xlx_spi_reset;
>      dc->props = xilinx_spi_properties;
>      dc->vmsd = &vmstate_xilinx_spi;
> --
> 2.19.0
>
>