qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses
@ 2023-10-20 15:06 Philippe Mathieu-Daudé
  2023-10-20 15:06 ` [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property Philippe Mathieu-Daudé
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

Avoid QOM objects poking at each other internals:
- Pass "link" properties
- Access MMIO via SysBus API
- Simplify using sysbus_create_simple()

Philippe Mathieu-Daudé (6):
  hw/m68k/irqc: Pass CPU using QOM link property
  hw/m68k/mcf5206: Pass CPU using QOM link property
  hw/m68k/mcf_intc: Expose MMIO region via SysBus API
  hw/m68k/mcf_intc: Pass CPU using QOM link property
  hw/m68k/next-cube: Do not open-code sysbus_create_simple()
  hw/m68k/virt: Do not open-code sysbus_create_simple()

 include/hw/intc/m68k_irqc.h |  1 +
 hw/intc/m68k_irqc.c         | 10 +++++++++-
 hw/m68k/an5206.c            |  2 ++
 hw/m68k/mcf5206.c           |  9 ++++++++-
 hw/m68k/mcf_intc.c          | 21 ++++++++++++++-------
 hw/m68k/next-cube.c         |  9 ++-------
 hw/m68k/virt.c              |  9 ++++-----
 7 files changed, 40 insertions(+), 21 deletions(-)

-- 
2.41.0



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

* [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:18   ` Thomas Huth
  2023-10-20 15:06 ` [PATCH 2/6] hw/m68k/mcf5206: " Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

Avoid the interrupt controller directly access the 'first_cpu'
global. Pass it from the board code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/intc/m68k_irqc.h |  1 +
 hw/intc/m68k_irqc.c         | 10 +++++++++-
 hw/m68k/virt.c              |  2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/intc/m68k_irqc.h b/include/hw/intc/m68k_irqc.h
index ef91f21812..693e33b0aa 100644
--- a/include/hw/intc/m68k_irqc.h
+++ b/include/hw/intc/m68k_irqc.h
@@ -33,6 +33,7 @@ typedef struct M68KIRQCState {
     SysBusDevice parent_obj;
 
     uint8_t ipr;
+    ArchCPU *cpu;
 
     /* statistics */
     uint64_t stats_irq_count[M68K_IRQC_LEVEL_NUM];
diff --git a/hw/intc/m68k_irqc.c b/hw/intc/m68k_irqc.c
index 0c515e4ecb..e09705eeaf 100644
--- a/hw/intc/m68k_irqc.c
+++ b/hw/intc/m68k_irqc.c
@@ -11,6 +11,7 @@
 #include "cpu.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
+#include "hw/qdev-properties.h"
 #include "hw/nmi.h"
 #include "hw/intc/intc.h"
 #include "hw/intc/m68k_irqc.h"
@@ -35,7 +36,7 @@ static void m68k_irqc_print_info(InterruptStatsProvider *obj, Monitor *mon)
 static void m68k_set_irq(void *opaque, int irq, int level)
 {
     M68KIRQCState *s = opaque;
-    M68kCPU *cpu = M68K_CPU(first_cpu);
+    M68kCPU *cpu = M68K_CPU(s->cpu);
     int i;
 
     if (level) {
@@ -85,12 +86,19 @@ static const VMStateDescription vmstate_m68k_irqc = {
     }
 };
 
+static Property m68k_irqc_properties[] = {
+    DEFINE_PROP_LINK("m68k-cpu", M68KIRQCState, cpu,
+                     TYPE_M68K_CPU, ArchCPU *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void m68k_irqc_class_init(ObjectClass *oc, void *data)
  {
     DeviceClass *dc = DEVICE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
     InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(oc);
 
+    device_class_set_props(dc, m68k_irqc_properties);
     nc->nmi_monitor_handler = m68k_nmi;
     dc->reset = m68k_irqc_reset;
     dc->vmsd = &vmstate_m68k_irqc;
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 2dd3c99894..da35e74bd9 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -155,6 +155,8 @@ static void virt_init(MachineState *machine)
     /* IRQ Controller */
 
     irqc_dev = qdev_new(TYPE_M68K_IRQC);
+    object_property_set_link(OBJECT(irqc_dev), "m68k-cpu",
+                             OBJECT(first_cpu), &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(irqc_dev), &error_fatal);
 
     /*
-- 
2.41.0



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

* [PATCH 2/6] hw/m68k/mcf5206: Pass CPU using QOM link property
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
  2023-10-20 15:06 ` [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:26   ` Thomas Huth
  2023-10-20 15:06 ` [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

Avoid the interrupt controller directly access the first cpu
via the qemu_get_cpu() call. Pass it from the board code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/an5206.c  | 2 ++
 hw/m68k/mcf5206.c | 9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index 11ae4c9795..f51c93088f 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -26,6 +26,8 @@ static void mcf5206_init(MemoryRegion *sysmem, uint32_t base)
     SysBusDevice *s;
 
     dev = qdev_new(TYPE_MCF5206_MBAR);
+    object_property_set_link(OBJECT(dev), "m68k-cpu",
+                             OBJECT(first_cpu), &error_abort);
     s = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(s, &error_fatal);
 
diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index 2ab1b4f059..f920ca2ceb 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -10,6 +10,7 @@
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "cpu.h"
+#include "hw/qdev-properties.h"
 #include "hw/boards.h"
 #include "hw/irq.h"
 #include "hw/m68k/mcf.h"
@@ -601,13 +602,19 @@ static void mcf5206_mbar_realize(DeviceState *dev, Error **errp)
     s->timer[1] = m5206_timer_init(s->pic[10]);
     s->uart[0] = mcf_uart_init(s->pic[12], serial_hd(0));
     s->uart[1] = mcf_uart_init(s->pic[13], serial_hd(1));
-    s->cpu = M68K_CPU(qemu_get_cpu(0));
 }
 
+static Property mcf5206_mbar_properties[] = {
+    DEFINE_PROP_LINK("m68k-cpu", m5206_mbar_state, cpu,
+                     TYPE_M68K_CPU, M68kCPU *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void mcf5206_mbar_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
+    device_class_set_props(dc, mcf5206_mbar_properties);
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->desc = "MCF5206 system integration module";
     dc->realize = mcf5206_mbar_realize;
-- 
2.41.0



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

* [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
  2023-10-20 15:06 ` [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property Philippe Mathieu-Daudé
  2023-10-20 15:06 ` [PATCH 2/6] hw/m68k/mcf5206: " Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:32   ` Thomas Huth
  2023-10-20 15:06 ` [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

QOM objects shouldn't access each other internals fields
except using the QOM API.

Here the caller of mcf_intc_init() access the MMIO region from
the MCF_INTC state. Avoid that by exposing that region via
sysbus_init_mmio(), then get it with sysbus_mmio_get_region().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/mcf_intc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 4cd30188c0..1f74ea0e14 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -173,6 +173,7 @@ static void mcf_intc_instance_init(Object *obj)
     mcf_intc_state *s = MCF_INTC(obj);
 
     memory_region_init_io(&s->iomem, obj, &mcf_intc_ops, s, "mcf", 0x100);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
 }
 
 static void mcf_intc_class_init(ObjectClass *oc, void *data)
@@ -211,7 +212,8 @@ qemu_irq *mcf_intc_init(MemoryRegion *sysmem,
     s = MCF_INTC(dev);
     s->cpu = cpu;
 
-    memory_region_add_subregion(sysmem, base, &s->iomem);
+    memory_region_add_subregion(sysmem, base,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
 
     return qemu_allocate_irqs(mcf_intc_set_irq, s, 64);
 }
-- 
2.41.0



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

* [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2023-10-20 15:06 ` [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:36   ` Thomas Huth
  2023-10-20 15:06 ` [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

QOM objects shouldn't access each other internals fields
except using the QOM API.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/mcf_intc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 1f74ea0e14..1d3b34e18c 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -14,6 +14,7 @@
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "hw/m68k/mcf.h"
+#include "hw/qdev-properties.h"
 #include "qom/object.h"
 
 #define TYPE_MCF_INTC "mcf-intc"
@@ -176,10 +177,17 @@ static void mcf_intc_instance_init(Object *obj)
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
 }
 
+static Property mcf_intc_properties[] = {
+    DEFINE_PROP_LINK("m68k-cpu", mcf_intc_state, cpu,
+                     TYPE_M68K_CPU, M68kCPU *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void mcf_intc_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
+    device_class_set_props(dc, mcf_intc_properties);
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->reset = mcf_intc_reset;
 }
@@ -204,16 +212,13 @@ qemu_irq *mcf_intc_init(MemoryRegion *sysmem,
                         M68kCPU *cpu)
 {
     DeviceState  *dev;
-    mcf_intc_state *s;
 
     dev = qdev_new(TYPE_MCF_INTC);
+    object_property_set_link(OBJECT(dev), "m68k-cpu",
+                             OBJECT(cpu), &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-
-    s = MCF_INTC(dev);
-    s->cpu = cpu;
-
     memory_region_add_subregion(sysmem, base,
                                 sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
 
-    return qemu_allocate_irqs(mcf_intc_set_irq, s, 64);
+    return qemu_allocate_irqs(mcf_intc_set_irq, dev, 64);
 }
-- 
2.41.0



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

* [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple()
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2023-10-20 15:06 ` [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:42   ` Thomas Huth
  2023-10-20 15:06 ` [PATCH 6/6] hw/m68k/virt: " Philippe Mathieu-Daudé
  2023-10-20 21:40 ` [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Richard Henderson
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

Mechanical change using the following coccinelle script:

  @@
  identifier dev;
  identifier sbd;
  expression qom_type;
  expression addr;
  @@
  -    dev = qdev_new(qom_type);
  -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
  -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
  +    dev = sysbus_create_simple(qom_type, addr, NULL);

then manually removing the 'dev' variable to avoid:

  error: variable 'dev' set but not used [-Werror,-Wunused-but-set-variable]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/next-cube.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 5d244b3b95..d17e6be8e1 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -950,7 +950,6 @@ static void next_cube_init(MachineState *machine)
     MemoryRegion *bmapm2 = g_new(MemoryRegion, 1);
     MemoryRegion *sysmem = get_system_memory();
     const char *bios_name = machine->firmware ?: ROM_FILE;
-    DeviceState *dev;
     DeviceState *pcdev;
 
     /* Initialize the cpu core */
@@ -974,9 +973,7 @@ static void next_cube_init(MachineState *machine)
     memory_region_add_subregion(sysmem, 0x04000000, machine->ram);
 
     /* Framebuffer */
-    dev = qdev_new(TYPE_NEXTFB);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x0B000000);
+    sysbus_create_simple(TYPE_NEXTFB, 0x0B000000, NULL);
 
     /* MMIO */
     sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 0, 0x02000000);
@@ -993,9 +990,7 @@ static void next_cube_init(MachineState *machine)
     memory_region_add_subregion(sysmem, 0x820c0000, bmapm2);
 
     /* KBD */
-    dev = qdev_new(TYPE_NEXTKBD);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x0200e000);
+    sysbus_create_simple(TYPE_NEXTKBD, 0x0200e000, NULL);
 
     /* Load ROM here */
     /* still not sure if the rom should also be mapped at 0x0*/
-- 
2.41.0



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

* [PATCH 6/6] hw/m68k/virt: Do not open-code sysbus_create_simple()
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2023-10-20 15:06 ` [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple() Philippe Mathieu-Daudé
@ 2023-10-20 15:06 ` Philippe Mathieu-Daudé
  2023-10-22  9:43   ` Thomas Huth
  2023-10-20 21:40 ` [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Richard Henderson
  6 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-20 15:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth,
	Philippe Mathieu-Daudé

Mechanical change using the following coccinelle script:

  @@
  identifier dev;
  expression qom_type;
  expression addr;
  expression irq;
  @@
  -    dev = qdev_new(qom_type);
  -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
  -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
  -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
  +    dev = sysbus_create_simple(qom_type, addr, irq);

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/virt.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index da35e74bd9..31038b8bb0 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -201,11 +201,8 @@ static void virt_init(MachineState *machine)
     sysbus_connect_irq(sysbus, 0, PIC_GPIO(VIRT_GF_TTY_IRQ_BASE));
 
     /* virt controller */
-    dev = qdev_new(TYPE_VIRT_CTRL);
-    sysbus = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(sysbus, &error_fatal);
-    sysbus_mmio_map(sysbus, 0, VIRT_CTRL_MMIO_BASE);
-    sysbus_connect_irq(sysbus, 0, PIC_GPIO(VIRT_CTRL_IRQ_BASE));
+    dev = sysbus_create_simple(TYPE_VIRT_CTRL, VIRT_CTRL_MMIO_BASE,
+                               PIC_GPIO(VIRT_CTRL_IRQ_BASE));
 
     /* virtio-mmio */
     io_base = VIRT_VIRTIO_MMIO_BASE;
-- 
2.41.0



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

* Re: [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses
  2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2023-10-20 15:06 ` [PATCH 6/6] hw/m68k/virt: " Philippe Mathieu-Daudé
@ 2023-10-20 21:40 ` Richard Henderson
  6 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2023-10-20 21:40 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Mark Cave-Ayland, Laurent Vivier, Thomas Huth

On 10/20/23 08:06, Philippe Mathieu-Daudé wrote:
> Avoid QOM objects poking at each other internals:
> - Pass "link" properties
> - Access MMIO via SysBus API
> - Simplify using sysbus_create_simple()
> 
> Philippe Mathieu-Daudé (6):
>    hw/m68k/irqc: Pass CPU using QOM link property
>    hw/m68k/mcf5206: Pass CPU using QOM link property
>    hw/m68k/mcf_intc: Expose MMIO region via SysBus API
>    hw/m68k/mcf_intc: Pass CPU using QOM link property
>    hw/m68k/next-cube: Do not open-code sysbus_create_simple()
>    hw/m68k/virt: Do not open-code sysbus_create_simple()

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


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

* Re: [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property
  2023-10-20 15:06 ` [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property Philippe Mathieu-Daudé
@ 2023-10-22  9:18   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:21 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> Avoid the interrupt controller directly access the 'first_cpu'
> global. Pass it from the board code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/intc/m68k_irqc.h |  1 +
>  hw/intc/m68k_irqc.c         | 10 +++++++++-
>  hw/m68k/virt.c              |  2 ++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/intc/m68k_irqc.h b/include/hw/intc/m68k_irqc.h
> index ef91f21812..693e33b0aa 100644
> --- a/include/hw/intc/m68k_irqc.h
> +++ b/include/hw/intc/m68k_irqc.h
> @@ -33,6 +33,7 @@ typedef struct M68KIRQCState {
>      SysBusDevice parent_obj;
>  
>      uint8_t ipr;
> +    ArchCPU *cpu;
>  
>      /* statistics */
>      uint64_t stats_irq_count[M68K_IRQC_LEVEL_NUM];
> diff --git a/hw/intc/m68k_irqc.c b/hw/intc/m68k_irqc.c
> index 0c515e4ecb..e09705eeaf 100644
> --- a/hw/intc/m68k_irqc.c
> +++ b/hw/intc/m68k_irqc.c
> @@ -11,6 +11,7 @@
>  #include "cpu.h"
>  #include "migration/vmstate.h"
>  #include "monitor/monitor.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/nmi.h"
>  #include "hw/intc/intc.h"
>  #include "hw/intc/m68k_irqc.h"
> @@ -35,7 +36,7 @@ static void m68k_irqc_print_info(InterruptStatsProvider *obj, Monitor *mon)
>  static void m68k_set_irq(void *opaque, int irq, int level)
>  {
>      M68KIRQCState *s = opaque;
> -    M68kCPU *cpu = M68K_CPU(first_cpu);
> +    M68kCPU *cpu = M68K_CPU(s->cpu);
>      int i;
>  
>      if (level) {
> @@ -85,12 +86,19 @@ static const VMStateDescription vmstate_m68k_irqc = {
>      }
>  };
>  
> +static Property m68k_irqc_properties[] = {
> +    DEFINE_PROP_LINK("m68k-cpu", M68KIRQCState, cpu,
> +                     TYPE_M68K_CPU, ArchCPU *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void m68k_irqc_class_init(ObjectClass *oc, void *data)
>   {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>      NMIClass *nc = NMI_CLASS(oc);
>      InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>  
> +    device_class_set_props(dc, m68k_irqc_properties);
>      nc->nmi_monitor_handler = m68k_nmi;
>      dc->reset = m68k_irqc_reset;
>      dc->vmsd = &vmstate_m68k_irqc;
> diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
> index 2dd3c99894..da35e74bd9 100644
> --- a/hw/m68k/virt.c
> +++ b/hw/m68k/virt.c
> @@ -155,6 +155,8 @@ static void virt_init(MachineState *machine)
>      /* IRQ Controller */
>  
>      irqc_dev = qdev_new(TYPE_M68K_IRQC);
> +    object_property_set_link(OBJECT(irqc_dev), "m68k-cpu",
> +                             OBJECT(first_cpu), &error_abort);

I'd rather use the local variable "cpu" instead of "first_cpu" here.

Apart from that:
Reviewed-by: Thomas Huth <huth@tuxfamily.org>



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

* Re: [PATCH 2/6] hw/m68k/mcf5206: Pass CPU using QOM link property
  2023-10-20 15:06 ` [PATCH 2/6] hw/m68k/mcf5206: " Philippe Mathieu-Daudé
@ 2023-10-22  9:26   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:26 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:22 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> Avoid the interrupt controller directly access the first cpu
> via the qemu_get_cpu() call. Pass it from the board code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/m68k/an5206.c  | 2 ++
>  hw/m68k/mcf5206.c | 9 ++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index 11ae4c9795..f51c93088f 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -26,6 +26,8 @@ static void mcf5206_init(MemoryRegion *sysmem, uint32_t base)
>      SysBusDevice *s;
>  
>      dev = qdev_new(TYPE_MCF5206_MBAR);
> +    object_property_set_link(OBJECT(dev), "m68k-cpu",
> +                             OBJECT(first_cpu), &error_abort);
>      s = SYS_BUS_DEVICE(dev);
>      sysbus_realize_and_unref(s, &error_fatal);

It might be nicer to pass "cpu" as parameter from an5206_init().

Anyway:
Tested-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Thomas Huth <huth@tuxfamily.org>


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

* Re: [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API
  2023-10-20 15:06 ` [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API Philippe Mathieu-Daudé
@ 2023-10-22  9:32   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:23 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> QOM objects shouldn't access each other internals fields
> except using the QOM API.
> 
> Here the caller of mcf_intc_init() access the MMIO region from
> the MCF_INTC state. Avoid that by exposing that region via
> sysbus_init_mmio(), then get it with sysbus_mmio_get_region().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/m68k/mcf_intc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Tested-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Thomas Huth <huth@tuxfamily.org>


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

* Re: [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property
  2023-10-20 15:06 ` [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property Philippe Mathieu-Daudé
@ 2023-10-22  9:36   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:24 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> QOM objects shouldn't access each other internals fields
> except using the QOM API.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Tested-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Thomas Huth <huth@tuxfamily.org>


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

* Re: [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple()
  2023-10-20 15:06 ` [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple() Philippe Mathieu-Daudé
@ 2023-10-22  9:42   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:25 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> Mechanical change using the following coccinelle script:
> 
>   @@
>   identifier dev;
>   identifier sbd;
>   expression qom_type;
>   expression addr;
>   @@
>   -    dev = qdev_new(qom_type);
>   -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>   -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>   +    dev = sysbus_create_simple(qom_type, addr, NULL);
> 
> then manually removing the 'dev' variable to avoid:
> 
>   error: variable 'dev' set but not used [-Werror,-Wunused-but-set-variable]
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Tested-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Thomas Huth <huth@tuxfamily.org>



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

* Re: [PATCH 6/6] hw/m68k/virt: Do not open-code sysbus_create_simple()
  2023-10-20 15:06 ` [PATCH 6/6] hw/m68k/virt: " Philippe Mathieu-Daudé
@ 2023-10-22  9:43   ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-22  9:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Mark Cave-Ayland, Laurent Vivier

Am Fri, 20 Oct 2023 17:06:26 +0200
schrieb Philippe Mathieu-Daudé <philmd@linaro.org>:

> Mechanical change using the following coccinelle script:
> 
>   @@
>   identifier dev;
>   expression qom_type;
>   expression addr;
>   expression irq;
>   @@
>   -    dev = qdev_new(qom_type);
>   -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>   -    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>   -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
>   +    dev = sysbus_create_simple(qom_type, addr, irq);
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Thomas Huth <huth@tuxfamily.org>


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

end of thread, other threads:[~2023-10-22  9:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-20 15:06 [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Philippe Mathieu-Daudé
2023-10-20 15:06 ` [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property Philippe Mathieu-Daudé
2023-10-22  9:18   ` Thomas Huth
2023-10-20 15:06 ` [PATCH 2/6] hw/m68k/mcf5206: " Philippe Mathieu-Daudé
2023-10-22  9:26   ` Thomas Huth
2023-10-20 15:06 ` [PATCH 3/6] hw/m68k/mcf_intc: Expose MMIO region via SysBus API Philippe Mathieu-Daudé
2023-10-22  9:32   ` Thomas Huth
2023-10-20 15:06 ` [PATCH 4/6] hw/m68k/mcf_intc: Pass CPU using QOM link property Philippe Mathieu-Daudé
2023-10-22  9:36   ` Thomas Huth
2023-10-20 15:06 ` [PATCH 5/6] hw/m68k/next-cube: Do not open-code sysbus_create_simple() Philippe Mathieu-Daudé
2023-10-22  9:42   ` Thomas Huth
2023-10-20 15:06 ` [PATCH 6/6] hw/m68k/virt: " Philippe Mathieu-Daudé
2023-10-22  9:43   ` Thomas Huth
2023-10-20 21:40 ` [PATCH 0/6] hw/m68k: Strengthen QOM/SysBus API uses Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).