Re: [PATCH 1/6] hw/m68k/irqc: Pass CPU using QOM link property

[Thomas Huth]

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>