> On 20. Jan 2026, at 19:03, Peter Maydell <[email protected]> wrote:
> 
> The virt board might have multiple different ways to handle MSI
> interrupts: via an ITS, via the GICv2M device, or not at all.  The
> logic to select which of these we use is confusing because it is
> controlled by a mix of versioned-board compatibility flags, board
> option flags, and open-coded logic inside the create_gic() and
> create_its() functions.
> 
> Currently we set VirtMachineState::msi_controller as the very last
> part of this, inside create_its() or create_gicv2m().  This field is
> then used only in the hotplug pre-plug callback function.
> 
> As a first step in making this clearer to understand, move the logic
> into a single finalize_msi_controller() function, which sets
> VirtMachineState::msi_controller.  The actual machine creation code
> can then look only at that field.  (This is a parallel to what we do
> with the GIC, where finalize_gic_version() sets the
> VirtMachineState::gic_version field.)
> 
> Signed-off-by: Peter Maydell <[email protected]>

Hello,

Did put a separate implementation of a chunk of this patch as part of 
https://patchew.org/QEMU/[email protected]/[email protected]/

Thank you,

> ---
> hw/arm/virt.c | 38 +++++++++++++++++++++++++++-----------
> 1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 4badc1a734..b55297455f 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -738,13 +738,6 @@ static void create_its(VirtMachineState *vms)
>     DeviceState *dev;
> 
>     assert(vms->its);
> -    if (!kvm_irqchip_in_kernel() && !vms->tcg_its) {
> -        /*
> -         * Do nothing if ITS is neither supported by the host nor emulated by
> -         * the machine.
> -         */
> -        return;
> -    }
>     dev = qdev_new(its_class_name());
> 
> @@ -754,7 +747,6 @@ static void create_its(VirtMachineState *vms)
>     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base);
> 
>     fdt_add_its_gic_node(vms);
> -    vms->msi_controller = VIRT_MSI_CTRL_ITS;
> }
> 
> static void create_v2m(VirtMachineState *vms)
> @@ -775,7 +767,6 @@ static void create_v2m(VirtMachineState *vms)
>     }
> 
>     fdt_add_v2m_gic_node(vms);
> -    vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
> }
> 

Parts above still apply even after that patch.

> /*
> @@ -957,10 +948,15 @@ static void create_gic(VirtMachineState *vms, 
> MemoryRegion *mem)
> 
>     fdt_add_gic_node(vms);
> 
> -    if (vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) {
> +    switch (vms->msi_controller) {
> +    case VIRT_MSI_CTRL_NONE:
> +        break;
> +    case VIRT_MSI_CTRL_ITS:
>         create_its(vms);
> -    } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +        break;
> +    case VIRT_MSI_CTRL_GICV2M:
>         create_v2m(vms);
> +        break;
>     }
> }
> 
> @@ -2079,6 +2075,24 @@ static VirtGICType finalize_gic_version_do(const char 
> *accel_name,
>     return gic_version;
> }
> 
> +static void finalize_msi_controller(VirtMachineState *vms)
> +{
> +    /*
> +     * Determine the final msi_controller according to
> +     * the relevant user settings and compat data. Called
> +     * after finalizing the GIC version.
> +     */
> +    if (vms->gic_version != VIRT_GIC_VERSION_2 && vms->its) {
> +        if (!kvm_irqchip_in_kernel() && !vms->tcg_its) {
> +            vms->msi_controller = VIRT_MSI_CTRL_NONE;
> +        } else {
> +            vms->msi_controller = VIRT_MSI_CTRL_ITS;
> +        }
> +    } else if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +        vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
> +    }
> +}
> +
> /*
>  * finalize_gic_version - Determines the final gic_version
>  * according to the gic-version property
> @@ -2251,6 +2265,8 @@ static void machvirt_init(MachineState *machine)
>      * KVM is not available yet
>      */
>     finalize_gic_version(vms);
> +    /* MSI controller type depends on GIC version */
> +    finalize_msi_controller(vms);
> 
>     if (vms->secure) {
>         /*
> -- 
> 2.47.3
> 
> 

Reply via email to