[PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Philippe Mathieu-Daudé]

New Spring, new opportunity to clean this device :)
(v1 was in 2018, v2 in 2019).

- lower device priority
- follow qdev model and use properties
- convert to trace events
- describe with slot name
- move under hw/misc/ and cover in MAINTAINERS

Peter, I hope you are OK adding it wit UNIMP device,
as both are very similar, and don't have much activity.

Only MIPS/SPARC32 targets use this device.

v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html

Philippe Mathieu-Daudé (7):
  hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
  hw/misc/empty_slot: Lower address space priority
  hw/misc/empty_slot: Convert 'size' field as qdev property
  hw/misc/empty_slot: Add a 'name' qdev property
  hw/misc/empty_slot: Convert debug printf() to trace event
  hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
  hw/misc/empty_slot: Name the slots when created

 include/hw/empty_slot.h        |  9 -------
 include/hw/misc/empty_slot.h   | 19 ++++++++++++++
 hw/mips/mips_malta.c           |  4 +--
 hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
 hw/sparc/sun4m.c               | 23 +++++++++++------
 MAINTAINERS                    |  4 ++-
 hw/core/Makefile.objs          |  1 -
 hw/misc/Makefile.objs          |  1 +
 hw/misc/trace-events           |  4 +++
 9 files changed, 70 insertions(+), 42 deletions(-)
 delete mode 100644 include/hw/empty_slot.h
 create mode 100644 include/hw/misc/empty_slot.h
 rename hw/{core => misc}/empty_slot.c (66%)

-- 
2.21.3

[PATCH 1/7] hw/sparc/sun4m: Use UnimplementedDevice for I/O devices

[Philippe Mathieu-Daudé]

These devices are not slots on a bus, but real I/O devices
that we do not implement. As the ISDN ROM would be a ROMD
device, also model it as UnimplementedDevice.

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sparc/sun4m.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 9838c5a183..f1494907b0 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -47,6 +47,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/char/escc.h"
 #include "hw/empty_slot.h"
+#include "hw/misc/unimp.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "elf.h"
@@ -920,7 +921,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     }
 
     if (hwdef->sx_base) {
-        empty_slot_init(hwdef->sx_base, 0x2000);
+        create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
     }
 
     nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
@@ -983,14 +984,16 @@ 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("SUNW,DBRI.prom",
+                                    hwdef->dbri_base + 0x1000, 0x30);
         /* reg space */
-        empty_slot_init(hwdef->dbri_base+0x10000, 0x100);
+        create_unimplemented_device("SUNW,DBRI",
+                                    hwdef->dbri_base + 0x10000, 0x100);
     }
 
     if (hwdef->bpp_base) {
         /* parallel port */
-        empty_slot_init(hwdef->bpp_base, 0x20);
+        create_unimplemented_device("SUNW,bpp", hwdef->bpp_base, 0x20);
     }
 
     initrd_size = 0;
-- 
2.21.3

[PATCH 2/7] hw/misc/empty_slot: Lower address space priority

[Philippe Mathieu-Daudé]

Empty slots model RAZ/WI access on a bus. Since we can still
(hot) plug devices on the bus, lower the slot priority, so
device added later is accessed first.

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

diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
index 3ba450e1ca..5ab426e965 100644
--- a/hw/core/empty_slot.c
+++ b/hw/core/empty_slot.c
@@ -67,7 +67,7 @@ void empty_slot_init(hwaddr addr, uint64_t slot_size)
 
         qdev_init_nofail(dev);
 
-        sysbus_mmio_map(s, 0, addr);
+        sysbus_mmio_map_overlap(s, 0, addr, -10000);
     }
 }
 
-- 
2.21.3

[PATCH 3/7] hw/misc/empty_slot: Convert 'size' field as qdev property

[Philippe Mathieu-Daudé]

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

diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
index 5ab426e965..0df086fe98 100644
--- a/hw/core/empty_slot.c
+++ b/hw/core/empty_slot.c
@@ -12,6 +12,7 @@
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
 #include "qemu/module.h"
+#include "hw/qdev-properties.h"
 #include "hw/empty_slot.h"
 
 //#define DEBUG_EMPTY_SLOT
@@ -57,17 +58,13 @@ 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_prop_set_uint64(dev, "size", slot_size);
         qdev_init_nofail(dev);
 
-        sysbus_mmio_map_overlap(s, 0, addr, -10000);
+        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(dev), 0, addr, -10000);
     }
 }
 
@@ -80,11 +77,17 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
 }
 
+static Property empty_slot_properties[] = {
+    DEFINE_PROP_UINT64("size", EmptySlot, size, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void empty_slot_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = empty_slot_realize;
+    device_class_set_props(dc, empty_slot_properties);
 }
 
 static const TypeInfo empty_slot_info = {
-- 
2.21.3

[PATCH 4/7] hw/misc/empty_slot: Add a 'name' qdev property

[Philippe Mathieu-Daudé]

Add a 'name' qdev property so when multiple slots are
accessed, we can notice which one is accessed.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/empty_slot.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
index 0df086fe98..576b276c4b 100644
--- a/hw/core/empty_slot.c
+++ b/hw/core/empty_slot.c
@@ -31,6 +31,7 @@ typedef struct EmptySlot {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
+    char *name;
     uint64_t size;
 } EmptySlot;
 
@@ -72,13 +73,17 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
 {
     EmptySlot *s = EMPTY_SLOT(dev);
 
+    if (s->name == NULL) {
+        s->name = g_strdup("empty-slot");
+    }
     memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
-                          "empty-slot", s->size);
+                          s->name, s->size);
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
 }
 
 static Property empty_slot_properties[] = {
     DEFINE_PROP_UINT64("size", EmptySlot, size, 0),
+    DEFINE_PROP_STRING("name", EmptySlot, name),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.21.3

[PATCH 5/7] hw/misc/empty_slot: Convert debug printf() to trace event

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/empty_slot.c | 19 ++++++++-----------
 hw/core/trace-events |  4 ++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/core/empty_slot.c b/hw/core/empty_slot.c
index 576b276c4b..d28f7f99c9 100644
--- a/hw/core/empty_slot.c
+++ b/hw/core/empty_slot.c
@@ -14,15 +14,7 @@
 #include "qemu/module.h"
 #include "hw/qdev-properties.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
+#include "trace.h"
 
 #define TYPE_EMPTY_SLOT "empty_slot"
 #define EMPTY_SLOT(obj) OBJECT_CHECK(EmptySlot, (obj), TYPE_EMPTY_SLOT)
@@ -38,14 +30,19 @@ typedef struct EmptySlot {
 static uint64_t empty_slot_read(void *opaque, hwaddr addr,
                                 unsigned size)
 {
-    DPRINTF("read from " TARGET_FMT_plx "\n", addr);
+    EmptySlot *s = EMPTY_SLOT(opaque);
+
+    trace_empty_slot_write(addr, size << 1, 0, size, s->name);
+
     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);
+    EmptySlot *s = EMPTY_SLOT(opaque);
+
+    trace_empty_slot_write(addr, size << 1, val, size, s->name);
 }
 
 static const MemoryRegionOps empty_slot_ops = {
diff --git a/hw/core/trace-events b/hw/core/trace-events
index 1ac60ede6b..bbb68fb6f0 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -34,3 +34,7 @@ clock_disconnect(const char *clk) "'%s'"
 clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', ns=%"PRIu64"->%"PRIu64
 clock_propagate(const char *clk) "'%s'"
 clock_update(const char *clk, const char *src, uint64_t val, int cb) "'%s', src='%s', ns=%"PRIu64", cb=%d"
+
+# empty_slot.c
+empty_slot_read(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "rd addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
+empty_slot_write(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "wr addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
-- 
2.21.3

[PATCH 6/7] hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS

[Philippe Mathieu-Daudé]

As this device model is very similar to the UnimplementedDevice,
maintain them altogether.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/empty_slot.h        |  9 ---------
 include/hw/misc/empty_slot.h   | 19 +++++++++++++++++++
 hw/mips/mips_malta.c           |  2 +-
 hw/{core => misc}/empty_slot.c |  4 ++--
 hw/sparc/sun4m.c               |  2 +-
 MAINTAINERS                    |  4 +++-
 hw/core/Makefile.objs          |  1 -
 hw/core/trace-events           |  4 ----
 hw/misc/Makefile.objs          |  1 +
 hw/misc/trace-events           |  4 ++++
 10 files changed, 31 insertions(+), 19 deletions(-)
 delete mode 100644 include/hw/empty_slot.h
 create mode 100644 include/hw/misc/empty_slot.h
 rename hw/{core => misc}/empty_slot.c (96%)

diff --git a/include/hw/empty_slot.h b/include/hw/empty_slot.h
deleted file mode 100644
index cb9a221aa6..0000000000
--- a/include/hw/empty_slot.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef HW_EMPTY_SLOT_H
-#define HW_EMPTY_SLOT_H
-
-#include "exec/hwaddr.h"
-
-/* empty_slot.c */
-void empty_slot_init(hwaddr addr, uint64_t slot_size);
-
-#endif
diff --git a/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
new file mode 100644
index 0000000000..b023bc2d91
--- /dev/null
+++ b/include/hw/misc/empty_slot.h
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+
+#ifndef HW_EMPTY_SLOT_H
+#define HW_EMPTY_SLOT_H
+
+#include "exec/hwaddr.h"
+
+void empty_slot_init(hwaddr addr, uint64_t slot_size);
+
+#endif
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index e4c4de1b4e..30ed3c1538 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -52,7 +52,7 @@
 #include "sysemu/runstate.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/empty_slot.h"
 #include "sysemu/kvm.h"
 #include "hw/semihosting/semihost.h"
 #include "hw/mips/cps.h"
diff --git a/hw/core/empty_slot.c b/hw/misc/empty_slot.c
similarity index 96%
rename from hw/core/empty_slot.c
rename to hw/misc/empty_slot.c
index d28f7f99c9..54be085189 100644
--- a/hw/core/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -11,9 +11,8 @@
 
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
-#include "qemu/module.h"
 #include "hw/qdev-properties.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/empty_slot.h"
 #include "trace.h"
 
 #define TYPE_EMPTY_SLOT "empty_slot"
@@ -90,6 +89,7 @@ static void empty_slot_class_init(ObjectClass *klass, void *data)
 
     dc->realize = empty_slot_realize;
     device_class_set_props(dc, empty_slot_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 }
 
 static const TypeInfo empty_slot_info = {
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index f1494907b0..6030900124 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -46,7 +46,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/empty_slot.h"
 #include "hw/misc/unimp.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
diff --git a/MAINTAINERS b/MAINTAINERS
index 1f84e3ae2c..3e0c2089f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1829,11 +1829,13 @@ F: docs/specs/vmgenid.txt
 F: tests/qtest/vmgenid-test.c
 F: stubs/vmgenid.c
 
-Unimplemented device
+Unimplemented device and empty slot
 M: Peter Maydell <peter.maydell@linaro.org>
 R: Philippe Mathieu-Daudé <f4bug@amsat.org>
 S: Maintained
+F: include/hw/misc/empty_slot.h
 F: include/hw/misc/unimp.h
+F: hw/misc/empty_slot.c
 F: hw/misc/unimp.c
 
 Standard VGA
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 1d540ed6e7..d8fee8effe 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -24,7 +24,6 @@ common-obj-$(CONFIG_SOFTMMU) += numa.o
 common-obj-$(CONFIG_SOFTMMU) += clock-vmstate.o
 obj-$(CONFIG_SOFTMMU) += machine-qmp-cmds.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_FITLOADER) += loader-fit.o
diff --git a/hw/core/trace-events b/hw/core/trace-events
index bbb68fb6f0..1ac60ede6b 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -34,7 +34,3 @@ clock_disconnect(const char *clk) "'%s'"
 clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', ns=%"PRIu64"->%"PRIu64
 clock_propagate(const char *clk) "'%s'"
 clock_update(const char *clk, const char *src, uint64_t val, int cb) "'%s', src='%s', ns=%"PRIu64", cb=%d"
-
-# empty_slot.c
-empty_slot_read(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "rd addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
-empty_slot_write(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "wr addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 68aae2eabb..e377842920 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -10,6 +10,7 @@ common-obj-$(CONFIG_EDU) += edu.o
 common-obj-$(CONFIG_PCA9552) += pca9552.o
 
 common-obj-$(CONFIG_UNIMP) += unimp.o
+common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_FW_CFG_DMA) += vmcoreinfo.o
 
 # ARM devices
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index a5862b2bed..0cb4c64ae7 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -39,6 +39,10 @@ ecc_mem_readl_ecr1(uint32_t ret) "Read event count 2 0x%08x"
 ecc_diag_mem_writeb(uint64_t addr, uint32_t val) "Write diagnostic %"PRId64" = 0x%02x"
 ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= 0x%02x"
 
+# empty_slot.c
+empty_slot_read(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "rd addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
+empty_slot_write(uint64_t addr, unsigned width, uint64_t value, unsigned size, const char *name) "wr addr:0x%04"PRIx64" data:0x%0*"PRIx64" size %u [%s]"
+
 # slavio_misc.c
 slavio_misc_update_irq_raise(void) "Raise IRQ"
 slavio_misc_update_irq_lower(void) "Lower IRQ"
-- 
2.21.3

[PATCH 7/7] hw/misc/empty_slot: Name the slots when created

[Philippe Mathieu-Daudé]

Directly set the slot name when creating the device,
to display the device name in trace events.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/empty_slot.h |  2 +-
 hw/mips/mips_malta.c         |  2 +-
 hw/misc/empty_slot.c         |  2 +-
 hw/sparc/sun4m.c             | 10 +++++++---
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
index b023bc2d91..dec56e56ae 100644
--- a/include/hw/misc/empty_slot.h
+++ b/include/hw/misc/empty_slot.h
@@ -14,6 +14,6 @@
 
 #include "exec/hwaddr.h"
 
-void empty_slot_init(hwaddr addr, uint64_t slot_size);
+void empty_slot_init(const char *name, hwaddr addr, uint64_t slot_size);
 
 #endif
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 30ed3c1538..c6e31a8fb2 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1246,7 +1246,7 @@ void mips_malta_init(MachineState *machine)
      * exception when accessing invalid memory. Create an empty slot to
      * emulate this feature.
      */
-    empty_slot_init(0, 0x20000000);
+    empty_slot_init("GT64120", 0, 0x20000000);
 
     qdev_init_nofail(dev);
 
diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index 54be085189..b568ae202b 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -50,7 +50,7 @@ static const MemoryRegionOps empty_slot_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void empty_slot_init(hwaddr addr, uint64_t slot_size)
+void empty_slot_init(const char *name, hwaddr addr, uint64_t slot_size)
 {
     if (slot_size > 0) {
         /* Only empty slots larger than 0 byte need handling. */
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 6030900124..3ef615fbbe 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -836,7 +836,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
 
     /* 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);
+        empty_slot_init("ecc", machine->ram_size,
+                        hwdef->max_mem - machine->ram_size);
     }
 
     prom_init(hwdef->slavio_base, bios_name);
@@ -867,7 +868,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
            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);
+        empty_slot_init("iommu.alias",
+                        hwdef->iommu_pad_base, hwdef->iommu_pad_len);
     }
 
     sparc32_dma_init(hwdef->dma_base,
@@ -916,7 +918,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     for (i = 0; i < MAX_VSIMMS; i++) {
         /* vsimm registers probed by OBP */
         if (hwdef->vsimm[i].reg_base) {
-            empty_slot_init(hwdef->vsimm[i].reg_base, 0x2000);
+            char *name = g_strdup_printf("vsimm[%d]", i);
+            empty_slot_init(name, hwdef->vsimm[i].reg_base, 0x2000);
+            g_free(name);
         }
     }
 
-- 
2.21.3

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Philippe Mathieu-Daudé]

ping?

On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> New Spring, new opportunity to clean this device :)
> (v1 was in 2018, v2 in 2019).
> 
> - lower device priority
> - follow qdev model and use properties
> - convert to trace events
> - describe with slot name
> - move under hw/misc/ and cover in MAINTAINERS
> 
> Peter, I hope you are OK adding it wit UNIMP device,
> as both are very similar, and don't have much activity.
> 
> Only MIPS/SPARC32 targets use this device.
> 
> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> 
> Philippe Mathieu-Daudé (7):
>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>   hw/misc/empty_slot: Lower address space priority
>   hw/misc/empty_slot: Convert 'size' field as qdev property
>   hw/misc/empty_slot: Add a 'name' qdev property
>   hw/misc/empty_slot: Convert debug printf() to trace event
>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>   hw/misc/empty_slot: Name the slots when created
> 
>  include/hw/empty_slot.h        |  9 -------
>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>  hw/mips/mips_malta.c           |  4 +--
>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>  hw/sparc/sun4m.c               | 23 +++++++++++------
>  MAINTAINERS                    |  4 ++-
>  hw/core/Makefile.objs          |  1 -
>  hw/misc/Makefile.objs          |  1 +
>  hw/misc/trace-events           |  4 +++
>  9 files changed, 70 insertions(+), 42 deletions(-)
>  delete mode 100644 include/hw/empty_slot.h
>  create mode 100644 include/hw/misc/empty_slot.h
>  rename hw/{core => misc}/empty_slot.c (66%)
> 

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Aleksandar Markovic]

нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
је написао/ла:
>
> ping?
>

I agree with all of your patches, they absolutely make sense to me,
but I would like to know Peter's opinion on such treatment of empty
slots.

I am going to give r-bs and integrate mips patches as soon as Peter
OKs the general approach. So, Peter, is Philippe's approach to empty
slots fine?

Aleksandar

> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> > New Spring, new opportunity to clean this device :)
> > (v1 was in 2018, v2 in 2019).
> >
> > - lower device priority
> > - follow qdev model and use properties
> > - convert to trace events
> > - describe with slot name
> > - move under hw/misc/ and cover in MAINTAINERS
> >
> > Peter, I hope you are OK adding it wit UNIMP device,
> > as both are very similar, and don't have much activity.
> >
> > Only MIPS/SPARC32 targets use this device.
> >
> > v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> > v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> >
> > Philippe Mathieu-Daudé (7):
> >   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
> >   hw/misc/empty_slot: Lower address space priority
> >   hw/misc/empty_slot: Convert 'size' field as qdev property
> >   hw/misc/empty_slot: Add a 'name' qdev property
> >   hw/misc/empty_slot: Convert debug printf() to trace event
> >   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
> >   hw/misc/empty_slot: Name the slots when created
> >
> >  include/hw/empty_slot.h        |  9 -------
> >  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
> >  hw/mips/mips_malta.c           |  4 +--
> >  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
> >  hw/sparc/sun4m.c               | 23 +++++++++++------
> >  MAINTAINERS                    |  4 ++-
> >  hw/core/Makefile.objs          |  1 -
> >  hw/misc/Makefile.objs          |  1 +
> >  hw/misc/trace-events           |  4 +++
> >  9 files changed, 70 insertions(+), 42 deletions(-)
> >  delete mode 100644 include/hw/empty_slot.h
> >  create mode 100644 include/hw/misc/empty_slot.h
> >  rename hw/{core => misc}/empty_slot.c (66%)
> >
>

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Philippe Mathieu-Daudé]

On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
> је написао/ла:
>>
>> ping?
>>
> 
> I agree with all of your patches, they absolutely make sense to me,
> but I would like to know Peter's opinion on such treatment of empty
> slots.
> 
> I am going to give r-bs and integrate mips patches as soon as Peter
> OKs the general approach. So, Peter, is Philippe's approach to empty
> slots fine?

Thanks Aleksandar for looking at this series.

I expect a neutral opinion from Peter.

What would be helpful is feedback from Artyom, since it authored this
device.

Artyom, do you mind Acking the series?

Thanks,

Phil.

> 
> Aleksandar
> 
>> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
>>> New Spring, new opportunity to clean this device :)
>>> (v1 was in 2018, v2 in 2019).
>>>
>>> - lower device priority
>>> - follow qdev model and use properties
>>> - convert to trace events
>>> - describe with slot name
>>> - move under hw/misc/ and cover in MAINTAINERS
>>>
>>> Peter, I hope you are OK adding it wit UNIMP device,
>>> as both are very similar, and don't have much activity.
>>>
>>> Only MIPS/SPARC32 targets use this device.
>>>
>>> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
>>> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
>>>
>>> Philippe Mathieu-Daudé (7):
>>>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>>>   hw/misc/empty_slot: Lower address space priority
>>>   hw/misc/empty_slot: Convert 'size' field as qdev property
>>>   hw/misc/empty_slot: Add a 'name' qdev property
>>>   hw/misc/empty_slot: Convert debug printf() to trace event
>>>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>>>   hw/misc/empty_slot: Name the slots when created
>>>
>>>  include/hw/empty_slot.h        |  9 -------
>>>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>>>  hw/mips/mips_malta.c           |  4 +--
>>>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>>>  hw/sparc/sun4m.c               | 23 +++++++++++------
>>>  MAINTAINERS                    |  4 ++-
>>>  hw/core/Makefile.objs          |  1 -
>>>  hw/misc/Makefile.objs          |  1 +
>>>  hw/misc/trace-events           |  4 +++
>>>  9 files changed, 70 insertions(+), 42 deletions(-)
>>>  delete mode 100644 include/hw/empty_slot.h
>>>  create mode 100644 include/hw/misc/empty_slot.h
>>>  rename hw/{core => misc}/empty_slot.c (66%)
>>>
>>
> 

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Peter Maydell]

On Sun, 24 May 2020 at 21:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> > I agree with all of your patches, they absolutely make sense to me,
> > but I would like to know Peter's opinion on such treatment of empty
> > slots.
> >
> > I am going to give r-bs and integrate mips patches as soon as Peter
> > OKs the general approach. So, Peter, is Philippe's approach to empty
> > slots fine?
>
> Thanks Aleksandar for looking at this series.
>
> I expect a neutral opinion from Peter.

Yes. In particular I cannot be the bottleneck for all design
choices in QEMU: that doesn't scale. Mostly you can assume
that if I have a strong opinion then I'll provide it, and
otherwise I prefer it if the people who care enough to
maintain code and write patches to tidy it up make the choices.

(Empty slots seemed a bit odd to me last time I looked at them,
but if they work for mips and sparc that's fine with me.)

thanks
-- PMM

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Artyom Tarasenko]

On Sun, May 24, 2020 at 10:21 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> > нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
> > је написао/ла:
> >>
> >> ping?
> >>
> >
> > I agree with all of your patches, they absolutely make sense to me,
> > but I would like to know Peter's opinion on such treatment of empty
> > slots.
> >
> > I am going to give r-bs and integrate mips patches as soon as Peter
> > OKs the general approach. So, Peter, is Philippe's approach to empty
> > slots fine?
>
> Thanks Aleksandar for looking at this series.
>
> I expect a neutral opinion from Peter.
>
> What would be helpful is feedback from Artyom, since it authored this
> device.
>
> Artyom, do you mind Acking the series?

Thanks for asking and sorry for the delay. My mailer played a bad trick on me.
The patches definitely make sense:

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



> Thanks,
>
> Phil.
>
> >
> > Aleksandar
> >
> >> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> >>> New Spring, new opportunity to clean this device :)
> >>> (v1 was in 2018, v2 in 2019).
> >>>
> >>> - lower device priority
> >>> - follow qdev model and use properties
> >>> - convert to trace events
> >>> - describe with slot name
> >>> - move under hw/misc/ and cover in MAINTAINERS
> >>>
> >>> Peter, I hope you are OK adding it wit UNIMP device,
> >>> as both are very similar, and don't have much activity.
> >>>
> >>> Only MIPS/SPARC32 targets use this device.
> >>>
> >>> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> >>> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> >>>
> >>> Philippe Mathieu-Daudé (7):
> >>>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
> >>>   hw/misc/empty_slot: Lower address space priority
> >>>   hw/misc/empty_slot: Convert 'size' field as qdev property
> >>>   hw/misc/empty_slot: Add a 'name' qdev property
> >>>   hw/misc/empty_slot: Convert debug printf() to trace event
> >>>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
> >>>   hw/misc/empty_slot: Name the slots when created
> >>>
> >>>  include/hw/empty_slot.h        |  9 -------
> >>>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
> >>>  hw/mips/mips_malta.c           |  4 +--
> >>>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
> >>>  hw/sparc/sun4m.c               | 23 +++++++++++------
> >>>  MAINTAINERS                    |  4 ++-
> >>>  hw/core/Makefile.objs          |  1 -
> >>>  hw/misc/Makefile.objs          |  1 +
> >>>  hw/misc/trace-events           |  4 +++
> >>>  9 files changed, 70 insertions(+), 42 deletions(-)
> >>>  delete mode 100644 include/hw/empty_slot.h
> >>>  create mode 100644 include/hw/misc/empty_slot.h
> >>>  rename hw/{core => misc}/empty_slot.c (66%)
> >>>
> >>
> >
>


--
Regards,
Artyom Tarasenko

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

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning

[Philippe Mathieu-Daudé]

On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> New Spring, new opportunity to clean this device :)
> (v1 was in 2018, v2 in 2019).
> 
> - lower device priority
> - follow qdev model and use properties
> - convert to trace events
> - describe with slot name
> - move under hw/misc/ and cover in MAINTAINERS
> 
> Peter, I hope you are OK adding it wit UNIMP device,
> as both are very similar, and don't have much activity.
> 
> Only MIPS/SPARC32 targets use this device.
> 
> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> 
> Philippe Mathieu-Daudé (7):
>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>   hw/misc/empty_slot: Lower address space priority
>   hw/misc/empty_slot: Convert 'size' field as qdev property
>   hw/misc/empty_slot: Add a 'name' qdev property
>   hw/misc/empty_slot: Convert debug printf() to trace event
>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>   hw/misc/empty_slot: Name the slots when created
> 
>  include/hw/empty_slot.h        |  9 -------
>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>  hw/mips/mips_malta.c           |  4 +--
>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>  hw/sparc/sun4m.c               | 23 +++++++++++------
>  MAINTAINERS                    |  4 ++-
>  hw/core/Makefile.objs          |  1 -
>  hw/misc/Makefile.objs          |  1 +
>  hw/misc/trace-events           |  4 +++
>  9 files changed, 70 insertions(+), 42 deletions(-)
>  delete mode 100644 include/hw/empty_slot.h
>  create mode 100644 include/hw/misc/empty_slot.h
>  rename hw/{core => misc}/empty_slot.c (66%)
> 

Thanks - except the MAINTAINERS change (merging empty_slot with
unimp device) which Peter did not ack - series applied to for
the next (temporary) sparc-next pull request.