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 <peter.maydell@linaro.org>
---
 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;
 }
 
 /*
@@ -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

> On 20. Jan 2026, at 19:03, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org>

Hello,

Did put a separate implementation of a chunk of this patch as part of https://patchew.org/QEMU/20260121134114.9781-1-mohamed@unpredictable.fr/20260121134114.9781-4-mohamed@unpredictable.fr/

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
> 
> 

On 20/1/26 19:03, Peter Maydell 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 <peter.maydell@linaro.org>
> ---
>   hw/arm/virt.c | 38 +++++++++++++++++++++++++++-----------
>   1 file changed, 27 insertions(+), 11 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>