[Qemu-devel] [PATCH 0/9] hw/misc: Clean the empty_slot device

[Qemu-devel] [PATCH 0/9] hw/misc: Clean the empty_slot device

[Philippe Mathieu-Daudé]

Hi, this is another clean-up series, paving the road for a later
series touching the GT64120 north bridge.

It makes the EMPTY_SLOT more in shape with the UNIMPLEMENTED_DEVICE,
and slighly more powerful (allowing overlapping, trace events).

Previous discussions with Artyom and Peter:

- https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00235.html
  'TYPE_EMPTY_SLOT is not UNIMPLEMENTED_DEVICE'

- https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00336.html
  '"no bus errors when this range is touched" behaviour'

- https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00292.html
  'background region with the "RAZ/WI" behaviour'

Regards,

Phil.

Philippe Mathieu-Daudé (9):
  hw/misc: Move the 'empty_slot' device to hw/misc/
  MAINTAINERS: Add the 'empty_slot' device with the 'unimp' one
  hw/misc/empty_slot: Allow overide by device with higher priority
  hw/misc/empty_slot: Add a qdev property 'size'
  hw/misc/empty_slot: Add a qdev property 'name'
  hw/misc/empty_slot: Convert debug printf()s to trace events
  hw/sparc/sun4m: Mark some devices as 'unimplemented'
  hw/sparc/sun4m: Simplify the RAM creation
  hw/misc/empty_slot: Pass the slot name as argument

 include/hw/empty_slot.h        |   7 ---
 include/hw/misc/empty_slot.h   |  33 +++++++++++
 hw/mips/mips_malta.c           |   4 +-
 hw/{core => misc}/empty_slot.c |  65 ++++++++++++--------
 hw/sparc/sun4m.c               | 105 ++++++++-------------------------
 MAINTAINERS                    |   4 +-
 hw/core/Makefile.objs          |   1 -
 hw/misc/Makefile.objs          |   1 +
 hw/misc/trace-events           |   4 ++
 9 files changed, 108 insertions(+), 116 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 (57%)

-- 
2.19.1

[Qemu-devel] [PATCH 1/9] hw/misc: Move the 'empty_slot' device to hw/misc/

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/empty_slot.h        |  7 -------
 include/hw/misc/empty_slot.h   | 32 ++++++++++++++++++++++++++++++++
 hw/mips/mips_malta.c           |  2 +-
 hw/{core => misc}/empty_slot.c |  2 +-
 hw/sparc/sun4m.c               |  2 +-
 hw/core/Makefile.objs          |  1 -
 hw/misc/Makefile.objs          |  1 +
 7 files changed, 36 insertions(+), 11 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 (98%)

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/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
new file mode 100644
index 0000000000..46483f24c3
--- /dev/null
+++ b/include/hw/misc/empty_slot.h
@@ -0,0 +1,32 @@
+/*
+ * 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
+
+/**
+ * empty_slot_init: create and map a RAZ/WI device
+ * @base: base address of the device's MMIO region
+ * @size: size of the device's MMIO region
+ *
+ * This utility function creates and maps an instance of empty slot,
+ * which is a dummy device which simply read as zero, and ignore writes.
+ * An empty slot sit on a bus, and no bus errors are generated when it is
+ * accessed.
+ * Guest accesses can be traced, using the '-trace empty_slot\*' command
+ * line argument.
+ * The device is mapped at priority -10000, which means that you can
+ * use it to cover a large region and then map other devices on top of it
+ * if necessary.
+ */
+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 37ec89b07e..7008be2e5b 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/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 98%
rename from hw/core/empty_slot.c
rename to hw/misc/empty_slot.c
index c694532046..0adf7a26dc 100644
--- a/hw/core/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -13,7 +13,7 @@
 #include "hw/hw.h"
 #include "hw/sysbus.h"
 #include "qemu/module.h"
-#include "hw/empty_slot.h"
+#include "hw/misc/empty_slot.h"
 
 //#define DEBUG_EMPTY_SLOT
 
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 7e4f61fc3e..cc85598d5b 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -41,7 +41,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/loader.h"
 #include "elf.h"
 #include "trace.h"
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a799c83815..5122a28a3b 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
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 77b9df9796..8ed900f9f5 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-y += unimp.o
+common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_FW_CFG_DMA) += vmcoreinfo.o
 
 # ARM devices
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/9] hw/misc: Move the 'empty_slot' device to hw/misc/

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  include/hw/empty_slot.h        |  7 -------
>  include/hw/misc/empty_slot.h   | 32 ++++++++++++++++++++++++++++++++
>  hw/mips/mips_malta.c           |  2 +-
>  hw/{core => misc}/empty_slot.c |  2 +-
>  hw/sparc/sun4m.c               |  2 +-
>  hw/core/Makefile.objs          |  1 -
>  hw/misc/Makefile.objs          |  1 +
>  7 files changed, 36 insertions(+), 11 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 (98%)
>
> 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/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
> new file mode 100644
> index 0000000000..46483f24c3
> --- /dev/null
> +++ b/include/hw/misc/empty_slot.h
> @@ -0,0 +1,32 @@
> +/*
> + * 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
> +
> +/**
> + * empty_slot_init: create and map a RAZ/WI device
> + * @base: base address of the device's MMIO region
> + * @size: size of the device's MMIO region
> + *
> + * This utility function creates and maps an instance of empty slot,
> + * which is a dummy device which simply read as zero, and ignore writes.
> + * An empty slot sit on a bus, and no bus errors are generated when it is
> + * accessed.
> + * Guest accesses can be traced, using the '-trace empty_slot\*' command
> + * line argument.
> + * The device is mapped at priority -10000, which means that you can
> + * use it to cover a large region and then map other devices on top of it
> + * if necessary.
> + */
> +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 37ec89b07e..7008be2e5b 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/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 98%
> rename from hw/core/empty_slot.c
> rename to hw/misc/empty_slot.c
> index c694532046..0adf7a26dc 100644
> --- a/hw/core/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -13,7 +13,7 @@
>  #include "hw/hw.h"
>  #include "hw/sysbus.h"
>  #include "qemu/module.h"
> -#include "hw/empty_slot.h"
> +#include "hw/misc/empty_slot.h"
>
>  //#define DEBUG_EMPTY_SLOT
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 7e4f61fc3e..cc85598d5b 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -41,7 +41,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/loader.h"
>  #include "elf.h"
>  #include "trace.h"
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index a799c83815..5122a28a3b 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
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 77b9df9796..8ed900f9f5 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-y += unimp.o
> +common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
>  common-obj-$(CONFIG_FW_CFG_DMA) += vmcoreinfo.o
>
>  # ARM devices
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [RFC PATCH 2/9] MAINTAINERS: Add the 'empty_slot' device with the 'unimp' one

[Philippe Mathieu-Daudé]

The EmptySlot and UnimplementedDevice are very similar, the only
difference is how they log guest accesses.
Maintain them altogether.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Peter, are you OK with that? Do you prefer 2 distinct sections?

 MAINTAINERS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index cad58b9487..abef4a1cfc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1643,11 +1643,13 @@ F: docs/specs/vmgenid.txt
 F: tests/vmgenid-test.c
 F: stubs/vmgenid.c
 
-Unimplemented device
+Unimplemented device, empty slot device
 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
-- 
2.19.1

Re: [Qemu-devel] [RFC PATCH 2/9] MAINTAINERS: Add the 'empty_slot' device with the 'unimp' one

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The EmptySlot and UnimplementedDevice are very similar, the only
> difference is how they log guest accesses.
> Maintain them altogether.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
> Peter, are you OK with that? Do you prefer 2 distinct sections?
>
>  MAINTAINERS | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cad58b9487..abef4a1cfc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1643,11 +1643,13 @@ F: docs/specs/vmgenid.txt
>  F: tests/vmgenid-test.c
>  F: stubs/vmgenid.c
>
> -Unimplemented device
> +Unimplemented device, empty slot device
>  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
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 3/9] hw/misc/empty_slot: Allow overide by device with higher priority

[Philippe Mathieu-Daudé]

The 'empty_slot' models a ChipEnable (or ChipSelect) MMIO device
pluggable on a bus.
The bus allow such slots to be not connected ('empty), thus no
bus errors are generated when this range is accessed.

The device is mapped at priority -10000 to allow other devices
to be mapped on top of it.

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

diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index 0adf7a26dc..53299cdbd1 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -68,7 +68,11 @@ void empty_slot_init(hwaddr addr, uint64_t slot_size)
 
         qdev_init_nofail(dev);
 
-        sysbus_mmio_map(s, 0, addr);
+        /*
+         * We use a priority lower than the default UNIMPLEMENTED_DEVICE
+         * to be able to plug a UnimplementedDevice on an EmptySlot.
+         */
+        sysbus_mmio_map_overlap(s, 0, addr, -10000);
     }
 }
 
-- 
2.19.1

Re: [Qemu-devel] [PATCH 3/9] hw/misc/empty_slot: Allow overide by device with higher priority

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The 'empty_slot' models a ChipEnable (or ChipSelect) MMIO device
> pluggable on a bus.
> The bus allow such slots to be not connected ('empty), thus no
> bus errors are generated when this range is accessed.
>
> The device is mapped at priority -10000 to allow other devices
> to be mapped on top of it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  hw/misc/empty_slot.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
> index 0adf7a26dc..53299cdbd1 100644
> --- a/hw/misc/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -68,7 +68,11 @@ void empty_slot_init(hwaddr addr, uint64_t slot_size)
>
>          qdev_init_nofail(dev);
>
> -        sysbus_mmio_map(s, 0, addr);
> +        /*
> +         * We use a priority lower than the default UNIMPLEMENTED_DEVICE
> +         * to be able to plug a UnimplementedDevice on an EmptySlot.
> +         */
> +        sysbus_mmio_map_overlap(s, 0, addr, -10000);
>      }
>  }
>
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 4/9] hw/misc/empty_slot: Add a qdev property 'size'

[Philippe Mathieu-Daudé]

Add a qdev 'size' property, check the size is not zero in the
realize() function, simplify the empty_slot_init() logic.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/empty_slot.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index 53299cdbd1..ef0a7b99ba 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/hw.h"
 #include "hw/sysbus.h"
 #include "qemu/module.h"
@@ -55,41 +56,45 @@ static const MemoryRegionOps empty_slot_ops = {
 
 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);
-
-        /*
-         * We use a priority lower than the default UNIMPLEMENTED_DEVICE
-         * to be able to plug a UnimplementedDevice on an EmptySlot.
-         */
-        sysbus_mmio_map_overlap(s, 0, addr, -10000);
-    }
+    DeviceState *dev;
+
+    dev = qdev_create(NULL, TYPE_EMPTY_SLOT);
+
+    qdev_prop_set_uint64(dev, "size", slot_size);
+    qdev_init_nofail(dev);
+
+    /*
+     * We use a priority lower than the default UNIMPLEMENTED_DEVICE
+     * to be able to plug a UnimplementedDevice on an EmptySlot.
+     */
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(dev), 0, addr, -10000);
 }
 
 static void empty_slot_realize(DeviceState *dev, Error **errp)
 {
     EmptySlot *s = EMPTY_SLOT(dev);
 
+    if (s->size == 0) {
+        error_setg(errp, "property 'size' not specified or zero");
+        return;
+    }
+
     memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
                           "empty-slot", 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_END_OF_LIST(),
+};
+
 static void empty_slot_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = empty_slot_realize;
+    dc->props = empty_slot_properties;
 }
 
 static const TypeInfo empty_slot_info = {
-- 
2.19.1

Re: [Qemu-devel] [PATCH 4/9] hw/misc/empty_slot: Add a qdev property 'size'

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Add a qdev 'size' property, check the size is not zero in the
> realize() function, simplify the empty_slot_init() logic.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  hw/misc/empty_slot.c | 43 ++++++++++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
> index 53299cdbd1..ef0a7b99ba 100644
> --- a/hw/misc/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "hw/hw.h"
>  #include "hw/sysbus.h"
>  #include "qemu/module.h"
> @@ -55,41 +56,45 @@ static const MemoryRegionOps empty_slot_ops = {
>
>  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);
> -
> -        /*
> -         * We use a priority lower than the default UNIMPLEMENTED_DEVICE
> -         * to be able to plug a UnimplementedDevice on an EmptySlot.
> -         */
> -        sysbus_mmio_map_overlap(s, 0, addr, -10000);
> -    }
> +    DeviceState *dev;
> +
> +    dev = qdev_create(NULL, TYPE_EMPTY_SLOT);
> +
> +    qdev_prop_set_uint64(dev, "size", slot_size);
> +    qdev_init_nofail(dev);
> +
> +    /*
> +     * We use a priority lower than the default UNIMPLEMENTED_DEVICE
> +     * to be able to plug a UnimplementedDevice on an EmptySlot.
> +     */
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(dev), 0, addr, -10000);
>  }
>
>  static void empty_slot_realize(DeviceState *dev, Error **errp)
>  {
>      EmptySlot *s = EMPTY_SLOT(dev);
>
> +    if (s->size == 0) {
> +        error_setg(errp, "property 'size' not specified or zero");
> +        return;
> +    }
> +
>      memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
>                            "empty-slot", 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_END_OF_LIST(),
> +};
> +
>  static void empty_slot_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->realize = empty_slot_realize;
> +    dc->props = empty_slot_properties;
>  }
>
>  static const TypeInfo empty_slot_info = {
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 5/9] hw/misc/empty_slot: Add a qdev property 'name'

[Philippe Mathieu-Daudé]

If few commits empty_slot_init() will take 'name' as argument.
Meanwhile, initialize it as 'empty-slot'.

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

diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index ef0a7b99ba..c32241a9e5 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -32,6 +32,7 @@ typedef struct EmptySlot {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
+    char *name;
     uint64_t size;
 } EmptySlot;
 
@@ -78,14 +79,18 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "property 'size' not specified or zero");
         return;
     }
+    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.19.1

Re: [Qemu-devel] [PATCH 5/9] hw/misc/empty_slot: Add a qdev property 'name'

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> If few commits empty_slot_init() will take 'name' as argument.
> Meanwhile, initialize it as 'empty-slot'.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  hw/misc/empty_slot.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
> index ef0a7b99ba..c32241a9e5 100644
> --- a/hw/misc/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -32,6 +32,7 @@ typedef struct EmptySlot {
>      SysBusDevice parent_obj;
>
>      MemoryRegion iomem;
> +    char *name;
>      uint64_t size;
>  } EmptySlot;
>
> @@ -78,14 +79,18 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
>          error_setg(errp, "property 'size' not specified or zero");
>          return;
>      }
> +    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.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 6/9] hw/misc/empty_slot: Convert debug printf()s to trace events

[Philippe Mathieu-Daudé]

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

diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index c32241a9e5..b810655554 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -15,15 +15,7 @@
 #include "hw/sysbus.h"
 #include "qemu/module.h"
 #include "hw/misc/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)
@@ -39,14 +31,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/misc/trace-events b/hw/misc/trace-events
index 47e1bccf71..b81135ab1e 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -1,5 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# 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]"
+
 # eccmemctl.c
 ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
 ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
-- 
2.19.1

Re: [Qemu-devel] [PATCH 6/9] hw/misc/empty_slot: Convert debug printf()s to trace events

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 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/misc/empty_slot.c | 19 ++++++++-----------
>  hw/misc/trace-events |  4 ++++
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
> index c32241a9e5..b810655554 100644
> --- a/hw/misc/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -15,15 +15,7 @@
>  #include "hw/sysbus.h"
>  #include "qemu/module.h"
>  #include "hw/misc/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)
> @@ -39,14 +31,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/misc/trace-events b/hw/misc/trace-events
> index 47e1bccf71..b81135ab1e 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -1,5 +1,9 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>
> +# 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]"
> +
>  # eccmemctl.c
>  ecc_mem_writel_mer(uint32_t val) "Write memory enable 0x%08x"
>  ecc_mem_writel_mdr(uint32_t val) "Write memory delay 0x%08x"
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 7/9] hw/sparc/sun4m: Mark some devices as 'unimplemented'

[Philippe Mathieu-Daudé]

These devices are not slots on a bus, but real devices that
we do not implement.

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 cc85598d5b..0df5a8edfc 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -42,6 +42,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/char/escc.h"
 #include "hw/misc/empty_slot.h"
+#include "hw/misc/unimp.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "trace.h"
@@ -970,7 +971,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("sx", hwdef->sx_base, 0x2000);
     }
 
     nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
@@ -1033,14 +1034,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("parallel", hwdef->bpp_base, 0x20);
     }
 
     initrd_size = 0;
-- 
2.19.1

Re: [Qemu-devel] [PATCH 7/9] hw/sparc/sun4m: Mark some devices as 'unimplemented'

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> These devices are not slots on a bus, but real devices that
> we do not implement.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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


> ---
>  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 cc85598d5b..0df5a8edfc 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -42,6 +42,7 @@
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/char/escc.h"
>  #include "hw/misc/empty_slot.h"
> +#include "hw/misc/unimp.h"
>  #include "hw/loader.h"
>  #include "elf.h"
>  #include "trace.h"
> @@ -970,7 +971,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("sx", hwdef->sx_base, 0x2000);
>      }
>
>      nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
> @@ -1033,14 +1034,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("parallel", hwdef->bpp_base, 0x20);
>      }
>
>      initrd_size = 0;
> --
> 2.19.1
>


--
Regards,
Artyom Tarasenko

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

[Qemu-devel] [PATCH 8/9] hw/sparc/sun4m: Simplify the RAM creation

[Philippe Mathieu-Daudé]

Now than the empty_slot device can be overlapped, use it to cover
the maximum memory range.
We can simplify now the main RAM is created.
The TYPE_SUN4M_MEMORY is not migratable, simply remove it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sparc/sun4m.c | 85 ++++++++----------------------------------------
 1 file changed, 13 insertions(+), 72 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 0df5a8edfc..d55753d5cb 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -767,71 +767,6 @@ static const TypeInfo prom_info = {
     .class_init    = prom_class_init,
 };
 
-#define TYPE_SUN4M_MEMORY "memory"
-#define SUN4M_RAM(obj) OBJECT_CHECK(RamDevice, (obj), TYPE_SUN4M_MEMORY)
-
-typedef struct RamDevice {
-    SysBusDevice parent_obj;
-
-    MemoryRegion ram;
-    uint64_t size;
-} RamDevice;
-
-/* System RAM */
-static void ram_realize(DeviceState *dev, Error **errp)
-{
-    RamDevice *d = SUN4M_RAM(dev);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-
-    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
-                                         d->size);
-    sysbus_init_mmio(sbd, &d->ram);
-}
-
-static void ram_init(hwaddr addr, ram_addr_t RAM_size,
-                     uint64_t max_mem)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-    RamDevice *d;
-
-    /* allocate RAM */
-    if ((uint64_t)RAM_size > max_mem) {
-        error_report("Too much memory for this machine: %" PRId64 ","
-                     " maximum %" PRId64,
-                     RAM_size / MiB, max_mem / MiB);
-        exit(1);
-    }
-    dev = qdev_create(NULL, "memory");
-    s = SYS_BUS_DEVICE(dev);
-
-    d = SUN4M_RAM(dev);
-    d->size = RAM_size;
-    qdev_init_nofail(dev);
-
-    sysbus_mmio_map(s, 0, addr);
-}
-
-static Property ram_properties[] = {
-    DEFINE_PROP_UINT64("size", RamDevice, size, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void ram_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    dc->realize = ram_realize;
-    dc->props = ram_properties;
-}
-
-static const TypeInfo ram_info = {
-    .name          = TYPE_SUN4M_MEMORY,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(RamDevice),
-    .class_init    = ram_class_init,
-};
-
 static void cpu_devinit(const char *cpu_type, unsigned int id,
                         uint64_t prom_addr, qemu_irq **cpu_irqs)
 {
@@ -872,6 +807,19 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     FWCfgState *fw_cfg;
     DeviceState *dev;
     SysBusDevice *s;
+    MemoryRegion ram;
+
+    if ((uint64_t)machine->ram_size > hwdef->max_mem) {
+        error_report("Too much memory for this machine: %" PRId64 ","
+                     " maximum %" PRId64,
+                     machine->ram_size / MiB, hwdef->max_mem / MiB);
+        exit(1);
+    }
+    memory_region_allocate_system_memory(&ram, OBJECT(machine), "sun4m.ram",
+                                         machine->ram_size);
+    memory_region_add_subregion(get_system_memory(), 0x00000000, &ram);
+    /* models without ECC don't trap when missing ram is accessed */
+    empty_slot_init(0x00000000, hwdef->max_mem);
 
     /* init CPUs */
     for(i = 0; i < smp_cpus; i++) {
@@ -881,13 +829,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     for (i = smp_cpus; i < MAX_CPUS; i++)
         cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
 
-
     /* set up devices */
-    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);
-    }
 
     prom_init(hwdef->slavio_base, bios_name);
 
@@ -1561,7 +1503,6 @@ static void sun4m_register_types(void)
     type_register_static(&idreg_info);
     type_register_static(&afx_info);
     type_register_static(&prom_info);
-    type_register_static(&ram_info);
 
     type_register_static(&ss5_type);
     type_register_static(&ss10_type);
-- 
2.19.1

Re: [Qemu-devel] [PATCH 8/9] hw/sparc/sun4m: Simplify the RAM creation

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Now than the empty_slot device can be overlapped, use it to cover
> the maximum memory range.

Whilie this is true for SS-5, the SS-20 does trap on missing RAM, so
empty_slot_init must be conditional.
nack.

> We can simplify now the main RAM is created.
> The TYPE_SUN4M_MEMORY is not migratable, simply remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sparc/sun4m.c | 85 ++++++++----------------------------------------
>  1 file changed, 13 insertions(+), 72 deletions(-)
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 0df5a8edfc..d55753d5cb 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -767,71 +767,6 @@ static const TypeInfo prom_info = {
>      .class_init    = prom_class_init,
>  };
>
> -#define TYPE_SUN4M_MEMORY "memory"
> -#define SUN4M_RAM(obj) OBJECT_CHECK(RamDevice, (obj), TYPE_SUN4M_MEMORY)
> -
> -typedef struct RamDevice {
> -    SysBusDevice parent_obj;
> -
> -    MemoryRegion ram;
> -    uint64_t size;
> -} RamDevice;
> -
> -/* System RAM */
> -static void ram_realize(DeviceState *dev, Error **errp)
> -{
> -    RamDevice *d = SUN4M_RAM(dev);
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -
> -    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
> -                                         d->size);
> -    sysbus_init_mmio(sbd, &d->ram);
> -}
> -
> -static void ram_init(hwaddr addr, ram_addr_t RAM_size,
> -                     uint64_t max_mem)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -    RamDevice *d;
> -
> -    /* allocate RAM */
> -    if ((uint64_t)RAM_size > max_mem) {
> -        error_report("Too much memory for this machine: %" PRId64 ","
> -                     " maximum %" PRId64,
> -                     RAM_size / MiB, max_mem / MiB);
> -        exit(1);
> -    }
> -    dev = qdev_create(NULL, "memory");
> -    s = SYS_BUS_DEVICE(dev);
> -
> -    d = SUN4M_RAM(dev);
> -    d->size = RAM_size;
> -    qdev_init_nofail(dev);
> -
> -    sysbus_mmio_map(s, 0, addr);
> -}
> -
> -static Property ram_properties[] = {
> -    DEFINE_PROP_UINT64("size", RamDevice, size, 0),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void ram_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -
> -    dc->realize = ram_realize;
> -    dc->props = ram_properties;
> -}
> -
> -static const TypeInfo ram_info = {
> -    .name          = TYPE_SUN4M_MEMORY,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(RamDevice),
> -    .class_init    = ram_class_init,
> -};
> -
>  static void cpu_devinit(const char *cpu_type, unsigned int id,
>                          uint64_t prom_addr, qemu_irq **cpu_irqs)
>  {
> @@ -872,6 +807,19 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      FWCfgState *fw_cfg;
>      DeviceState *dev;
>      SysBusDevice *s;
> +    MemoryRegion ram;
> +
> +    if ((uint64_t)machine->ram_size > hwdef->max_mem) {
> +        error_report("Too much memory for this machine: %" PRId64 ","
> +                     " maximum %" PRId64,
> +                     machine->ram_size / MiB, hwdef->max_mem / MiB);
> +        exit(1);
> +    }
> +    memory_region_allocate_system_memory(&ram, OBJECT(machine), "sun4m.ram",
> +                                         machine->ram_size);
> +    memory_region_add_subregion(get_system_memory(), 0x00000000, &ram);
> +    /* models without ECC don't trap when missing ram is accessed */
> +    empty_slot_init(0x00000000, hwdef->max_mem);
>
>      /* init CPUs */
>      for(i = 0; i < smp_cpus; i++) {
> @@ -881,13 +829,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      for (i = smp_cpus; i < MAX_CPUS; i++)
>          cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
>
> -
>      /* set up devices */
> -    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);
> -    }
>
>      prom_init(hwdef->slavio_base, bios_name);
>
> @@ -1561,7 +1503,6 @@ static void sun4m_register_types(void)
>      type_register_static(&idreg_info);
>      type_register_static(&afx_info);
>      type_register_static(&prom_info);
> -    type_register_static(&ram_info);
>
>      type_register_static(&ss5_type);
>      type_register_static(&ss10_type);
> --
> 2.19.1
>


-- 
Regards,
Artyom Tarasenko

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

Re: [Qemu-devel] [PATCH 8/9] hw/sparc/sun4m: Simplify the RAM creation

[Philippe Mathieu-Daudé]

On 6/25/19 9:14 AM, Artyom Tarasenko wrote:
> On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Now than the empty_slot device can be overlapped, use it to cover
>> the maximum memory range.
> 
> Whilie this is true for SS-5, the SS-20 does trap on missing RAM, so
> empty_slot_init must be conditional.
> nack.

Oh, I totally forgot the 'if (!hwdef->ecc_base)'...

Thanks Artyom!

>> We can simplify now the main RAM is created.
>> The TYPE_SUN4M_MEMORY is not migratable, simply remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sparc/sun4m.c | 85 ++++++++----------------------------------------
>>  1 file changed, 13 insertions(+), 72 deletions(-)
>>
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 0df5a8edfc..d55753d5cb 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -767,71 +767,6 @@ static const TypeInfo prom_info = {
>>      .class_init    = prom_class_init,
>>  };
>>
>> -#define TYPE_SUN4M_MEMORY "memory"
>> -#define SUN4M_RAM(obj) OBJECT_CHECK(RamDevice, (obj), TYPE_SUN4M_MEMORY)
>> -
>> -typedef struct RamDevice {
>> -    SysBusDevice parent_obj;
>> -
>> -    MemoryRegion ram;
>> -    uint64_t size;
>> -} RamDevice;
>> -
>> -/* System RAM */
>> -static void ram_realize(DeviceState *dev, Error **errp)
>> -{
>> -    RamDevice *d = SUN4M_RAM(dev);
>> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> -
>> -    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
>> -                                         d->size);
>> -    sysbus_init_mmio(sbd, &d->ram);
>> -}
>> -
>> -static void ram_init(hwaddr addr, ram_addr_t RAM_size,
>> -                     uint64_t max_mem)
>> -{
>> -    DeviceState *dev;
>> -    SysBusDevice *s;
>> -    RamDevice *d;
>> -
>> -    /* allocate RAM */
>> -    if ((uint64_t)RAM_size > max_mem) {
>> -        error_report("Too much memory for this machine: %" PRId64 ","
>> -                     " maximum %" PRId64,
>> -                     RAM_size / MiB, max_mem / MiB);
>> -        exit(1);
>> -    }
>> -    dev = qdev_create(NULL, "memory");
>> -    s = SYS_BUS_DEVICE(dev);
>> -
>> -    d = SUN4M_RAM(dev);
>> -    d->size = RAM_size;
>> -    qdev_init_nofail(dev);
>> -
>> -    sysbus_mmio_map(s, 0, addr);
>> -}
>> -
>> -static Property ram_properties[] = {
>> -    DEFINE_PROP_UINT64("size", RamDevice, size, 0),
>> -    DEFINE_PROP_END_OF_LIST(),
>> -};
>> -
>> -static void ram_class_init(ObjectClass *klass, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -
>> -    dc->realize = ram_realize;
>> -    dc->props = ram_properties;
>> -}
>> -
>> -static const TypeInfo ram_info = {
>> -    .name          = TYPE_SUN4M_MEMORY,
>> -    .parent        = TYPE_SYS_BUS_DEVICE,
>> -    .instance_size = sizeof(RamDevice),
>> -    .class_init    = ram_class_init,
>> -};
>> -
>>  static void cpu_devinit(const char *cpu_type, unsigned int id,
>>                          uint64_t prom_addr, qemu_irq **cpu_irqs)
>>  {
>> @@ -872,6 +807,19 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      FWCfgState *fw_cfg;
>>      DeviceState *dev;
>>      SysBusDevice *s;
>> +    MemoryRegion ram;
>> +
>> +    if ((uint64_t)machine->ram_size > hwdef->max_mem) {
>> +        error_report("Too much memory for this machine: %" PRId64 ","
>> +                     " maximum %" PRId64,
>> +                     machine->ram_size / MiB, hwdef->max_mem / MiB);
>> +        exit(1);
>> +    }
>> +    memory_region_allocate_system_memory(&ram, OBJECT(machine), "sun4m.ram",
>> +                                         machine->ram_size);
>> +    memory_region_add_subregion(get_system_memory(), 0x00000000, &ram);
>> +    /* models without ECC don't trap when missing ram is accessed */
>> +    empty_slot_init(0x00000000, hwdef->max_mem);
>>
>>      /* init CPUs */
>>      for(i = 0; i < smp_cpus; i++) {
>> @@ -881,13 +829,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>      for (i = smp_cpus; i < MAX_CPUS; i++)
>>          cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
>>
>> -
>>      /* set up devices */
>> -    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);
>> -    }
>>
>>      prom_init(hwdef->slavio_base, bios_name);
>>
>> @@ -1561,7 +1503,6 @@ static void sun4m_register_types(void)
>>      type_register_static(&idreg_info);
>>      type_register_static(&afx_info);
>>      type_register_static(&prom_info);
>> -    type_register_static(&ram_info);
>>
>>      type_register_static(&ss5_type);
>>      type_register_static(&ss10_type);
>> --
>> 2.19.1
>>
> 
> 

[Qemu-devel] [PATCH 9/9] hw/misc/empty_slot: Pass the slot name as argument

[Philippe Mathieu-Daudé]

Use the slot name to have more meaningful tracing logs.

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

diff --git a/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
index 46483f24c3..d0cd989d2d 100644
--- a/include/hw/misc/empty_slot.h
+++ b/include/hw/misc/empty_slot.h
@@ -14,6 +14,7 @@
 
 /**
  * empty_slot_init: create and map a RAZ/WI device
+ * @name: name of the device for debug logging
  * @base: base address of the device's MMIO region
  * @size: size of the device's MMIO region
  *
@@ -27,6 +28,6 @@
  * use it to cover a large region and then map other devices on top of it
  * if necessary.
  */
-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 7008be2e5b..51db5212be 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1212,7 +1212,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);
+    empty_slot_init("gt64120-ad", 0x00000000, 0x20000000);
 
     qdev_init_nofail(dev);
 
diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
index b810655554..396f9dd06a 100644
--- a/hw/misc/empty_slot.c
+++ b/hw/misc/empty_slot.c
@@ -52,12 +52,13 @@ 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)
 {
     DeviceState *dev;
 
     dev = qdev_create(NULL, TYPE_EMPTY_SLOT);
 
+    qdev_prop_set_string(dev, "name", name);
     qdev_prop_set_uint64(dev, "size", slot_size);
     qdev_init_nofail(dev);
 
@@ -77,7 +78,8 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
         return;
     }
     if (s->name == NULL) {
-        s->name = g_strdup("empty-slot");
+        error_setg(errp, "property 'name' not specified");
+        return;
     }
 
     memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index d55753d5cb..b17be3ddb8 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -819,7 +819,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), 0x00000000, &ram);
     /* models without ECC don't trap when missing ram is accessed */
-    empty_slot_init(0x00000000, hwdef->max_mem);
+    empty_slot_init("dram", 0x00000000, hwdef->max_mem);
 
     /* init CPUs */
     for(i = 0; i < smp_cpus; i++) {
@@ -859,7 +859,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,
@@ -908,7 +909,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.19.1

Re: [Qemu-devel] [PATCH 9/9] hw/misc/empty_slot: Pass the slot name as argument

[Artyom Tarasenko]

On Tue, Jun 25, 2019 at 12:01 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Use the slot name to have more meaningful tracing logs.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
>  include/hw/misc/empty_slot.h | 3 ++-
>  hw/mips/mips_malta.c         | 2 +-
>  hw/misc/empty_slot.c         | 6 ++++--
>  hw/sparc/sun4m.c             | 9 ++++++---
>  4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/misc/empty_slot.h b/include/hw/misc/empty_slot.h
> index 46483f24c3..d0cd989d2d 100644
> --- a/include/hw/misc/empty_slot.h
> +++ b/include/hw/misc/empty_slot.h
> @@ -14,6 +14,7 @@
>
>  /**
>   * empty_slot_init: create and map a RAZ/WI device
> + * @name: name of the device for debug logging
>   * @base: base address of the device's MMIO region
>   * @size: size of the device's MMIO region
>   *
> @@ -27,6 +28,6 @@
>   * use it to cover a large region and then map other devices on top of it
>   * if necessary.
>   */
> -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 7008be2e5b..51db5212be 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1212,7 +1212,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);
> +    empty_slot_init("gt64120-ad", 0x00000000, 0x20000000);
>
>      qdev_init_nofail(dev);
>
> diff --git a/hw/misc/empty_slot.c b/hw/misc/empty_slot.c
> index b810655554..396f9dd06a 100644
> --- a/hw/misc/empty_slot.c
> +++ b/hw/misc/empty_slot.c
> @@ -52,12 +52,13 @@ 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)
>  {
>      DeviceState *dev;
>
>      dev = qdev_create(NULL, TYPE_EMPTY_SLOT);
>
> +    qdev_prop_set_string(dev, "name", name);
>      qdev_prop_set_uint64(dev, "size", slot_size);
>      qdev_init_nofail(dev);
>
> @@ -77,7 +78,8 @@ static void empty_slot_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>      if (s->name == NULL) {
> -        s->name = g_strdup("empty-slot");
> +        error_setg(errp, "property 'name' not specified");
> +        return;
>      }
>
>      memory_region_init_io(&s->iomem, OBJECT(s), &empty_slot_ops, s,
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index d55753d5cb..b17be3ddb8 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -819,7 +819,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>                                           machine->ram_size);
>      memory_region_add_subregion(get_system_memory(), 0x00000000, &ram);
>      /* models without ECC don't trap when missing ram is accessed */
> -    empty_slot_init(0x00000000, hwdef->max_mem);
> +    empty_slot_init("dram", 0x00000000, hwdef->max_mem);
>
>      /* init CPUs */
>      for(i = 0; i < smp_cpus; i++) {
> @@ -859,7 +859,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,
> @@ -908,7 +909,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.19.1
>


-- 
Regards,
Artyom Tarasenko

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