Re: [PATCH v3 11/11] hw/sparc64/cpu: Initialize GPIO before realizing CPU devices

[Mark Cave-Ayland]

On 09/02/2024 11:34, Peter Maydell wrote:

On Thu, 8 Feb 2024 at 18:14, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:

Inline cpu_create() in order to call
qdev_init_gpio_in_named_with_opaque()
before the CPU is realized.

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

diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index 72f0849f50..3091cde586 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -24,6 +24,7 @@


  #include "qemu/osdep.h"
+#include "qapi/error.h"
  #include "cpu.h"
  #include "hw/boards.h"
  #include "hw/sparc/sparc64.h"
@@ -271,9 +272,10 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_type, 
uint64_t prom_addr)
      uint32_t  stick_frequency = 100 * 1000000;
      uint32_t hstick_frequency = 100 * 1000000;

-    cpu = SPARC_CPU(cpu_create(cpu_type));
+    cpu = SPARC_CPU(object_new(cpu_type));
      qdev_init_gpio_in_named(DEVICE(cpu), sparc64_cpu_set_ivec_irq,
                              "ivec-irq", IVEC_MAX);
+    qdev_realize(DEVICE(cpu), NULL, &error_fatal);
      env = &cpu->env;

      env->tick = cpu_timer_create("tick", cpu, tick_irq,
--
2.41.0

For the purposes of letting us enforce the "init GPIOs
before realize, not after" rule,
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>

but it looks like this code is adding a GPIO to a
device from code that's not actually part of the
implementation of the device. Ideally most of the code in
this file should be rolled into the CPU itself in target/sparc.

I suspect the reason the code is arranged like this is because IVECs aren't part of 
the core SPARC 64-bit architecture specification, although they happen to be 
implemented by the CPUs used by QEMU. Perhaps this would be better be handled on a 
CPU model basis, but I agree this shouldn't be a blocker for this patch.


ATB,

Mark.