On Wed, 21 Jan 2026 at 10:19, Mohamed Mediouni <[email protected]> wrote:
>
>
>
> > On 21. Jan 2026, at 10:27, Peter Maydell <[email protected]> wrote:
> >
> > On Tue, 20 Jan 2026 at 18:44, Mohamed Mediouni <[email protected]> 
> > wrote:
> >>
> >>
> >>
> >>> On 20. Jan 2026, at 19:02, Peter Maydell <[email protected]> wrote:
> >>
> >>
> >>> The MSI controller selection logic is already pretty confusing,
> >>> and this is making it more complicated. I don't think we should
> >>> need this machine back compatibility, because currently there
> >>> is no setup where you can have a GICv3 that doesn't also have
> >>> either an ITS or no MSI controller.
> >>>
> >> Hello,
> >>
> >> This was to replicate the “no_tcg_its” where this was done when 
> >> introducing TCG.
> >> And for the purpose of VM save/restore on HVF with the older machine type 
> >> continuing to work as expected.
> >
> > no_tcg_its is there for backwards compatibility: on older
> > QEMU versions before we added the TCG ITS, if you said
> > "-machine virt,gic-version=3 -accel tcg" you didn't get an
> > ITS; we wanted to change the default in new machine versions,
> > so we needed the back compat switch.
> >
> > What is the command line that works today and where we
> > want to go from "default to GICv3 without ITS or v2m"
> > to "default to GICv3 with v2m" ? I might have missed it
> > but back last year when I was looking at the current
> > configs I didn't think we had one.
> Hello,
>
> None, was thinking about what its=off should mean but perhaps
> the right behaviour to do there is to have it mean no MSI-X controller.

Yeah, this is a bit tricky. I think that at the moment
we allow "gic-version=2,its=off" and you still get a gicv2m.
So we need to retain that behaviour (though I doubt anybody
is deliberately using it), which I think means we do need
the its option to be an on/off/auto one. On the other
hand that means that the finalize_msi_controller() logic
(see https://patchew.org/QEMU/[email protected]/
that I posted yesterday) can look relatively neat:

   if (vms->its != auto) {
       /*
        * User set the legacy "its" option, which traditionally
        * meant "ignored if GICv2 (always provide GICv2m), enable
        * or disable ITS for GICv3".
        * Translate into its "msi" option equivalent:
        *  - "its=no" means "msi=gicv2m" for a GICv2, "msi=none"
        *    for GICv3 and up.
        *  - "its=yes" means "msi=auto"
        */
       if (vms->msi_controller != VIRT_MSI_CTRL_NOSEL) {
           error("msi and its options cannot be used together");
           exit(1);
       }
       if (vms->its == off) {
           if (vms->gic_version == VIRT_GIC_VERSION_2) {
               vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
           } else {
               vms->msi_controller = VIRT_MSI_CTRL_NONE;
           }
       }
       /* for vms->its == on we leave msi_controller at NOSEL */
   }

   if (vms->msi_controller != VIRT_MSI_CTRL_NOSEL) {
       /*
         * User specified an "msi" option: check what they
         * specified, and use it.
         */
        if (vms->msi_controller == VIRT_MSI_CTRL_ITS &&
            vms->gic_version == VIRT_GIC_VERSION_2) {
            error_report("A GICv2 cannot use an ITS");
            exit(1);
        }
        /* other error checks here as needed */
        return;
    }

    /* logic to pick the best msi_controller goes here */

This puts all the option handling code in one function that
has three parts:
 * deal with the legacy its= option by converting it to
   the equivalent msi= option
 * error check any explicitly set msi option
 * pick an msi for the user if they didn't specify

Then the rest of the code in virt.c doesn't need to care
about the option flags or back-compat stuff directly, it
can look at just the final resulting msi_controller setting.

thanks
-- PMM

Reply via email to