Re: [PATCH] qemu: Enable I/O APIC if needed

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 12/12/24 12:37, Ján Tomko wrote:
> On a Thursday in 2024, Michal Privoznik wrote:
>> This is a follow up of my previous commits. If the number of
>> vCPUs exceeds some arbitrary value (255) then QEMU requires IOMMU
>> with EIM and interemap enabled. But in turn, intremap IOMMU
> 
> s/interemap/intremap/
> 
>> requires split I/O APIC (per virDomainDefIOMMUValidate()). Since
>> after my previous commits (e.g. v10.10.0-rc1~183) IOMMU is added
>> automagically, the I/O APIC can be also enabled automagically.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_postparse.c                         | 7 +++++++
>> tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml | 3 ---
>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
>> index 03b5ef825a..f48f172c37 100644
>> --- a/src/qemu/qemu_postparse.c
>> +++ b/src/qemu/qemu_postparse.c
>> @@ -1547,6 +1547,13 @@ qemuDomainDefEnableDefaultFeatures(virDomainDef
>> *def,
>>          * capabilities, we still want to enable this */
>>         def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON;
>>     }
>> +
>> +    /* IOMMU with intremap requires split I/O APIC */
> 
> The error message in hw/i386/x86-iommu.c suggests otherwise:
> 
>     /* Both Intel and AMD IOMMU IR only support "kernel-irqchip={off|
> split}" */
>     if (x86_iommu_ir_supported(x86_iommu) && irq_all_kernel) {
>         error_setg(errp, "Interrupt Remapping cannot work with "
>                          "kernel-irqchip=on, please use 'split|off'.");
>         return;
>     }

Does it?

./qemu-system-x86_64 \
-machine q35,acpi=off \
-accel kvm \
-cpu qemu64 \
-m size=219136k \
-smp 288,sockets=288,cores=1,threads=1 \
-display none \
-no-user-config \
-nodefaults \
-device '{"driver":"intel-iommu","id":"iommu0","intremap":"on","eim":"on"}'


qemu-system-x86_64: -device
{"driver":"intel-iommu","id":"iommu0","intremap":"on","eim":"on"}:
Interrupt Remapping cannot work with kernel-irqchip=on, please use
'split|off'.


whereas:

./qemu-system-x86_64 \
-machine q35,acpi=off,kernel_irqchip=split \
-accel kvm \
-cpu qemu64 \
-m size=219136k \
-smp 288,sockets=288,cores=1,threads=1 \
-display none \
-no-user-config  \
-nodefaults  \
-device '{"driver":"intel-iommu","id":"iommu0","intremap":"on","eim":"on"}'

starts just fine (modulo warnings that the number of vCPUs exceeds the
number of physical CPUs, but that's irrelevant for this debate).
> 
> 
>> +    if (def->iommu &&
>> +        def->iommu->intremap == VIR_TRISTATE_SWITCH_ON &&
>> +        def->features[VIR_DOMAIN_FEATURE_IOAPIC] ==
>> VIR_DOMAIN_IOAPIC_NONE) {
>> +        def->features[VIR_DOMAIN_FEATURE_IOAPIC] =
>> VIR_DOMAIN_IOAPIC_QEMU;
>> +    }
> 
> This should be only done if the IOMMU was auto-added, to not override it
> for user-specified IOMMU.

I'm not sure about that. Consider the following XML:

<domain type='kvm'>
  <name>QEMUGuest1</name>
  <memory unit='KiB'>219100</memory>
  <currentMemory unit='KiB'>219100</currentMemory>
  <vcpu placement='static'>2</vcpu>
  <os>
    <type arch='x86_64' machine='q35'>hvm</type>
    <boot dev='hd'/>
  </os>
  <devices>
    <emulator>/usr/bin/qemu-system-x86_64</emulator>
    <iommu model='intel'>
      <driver intremap='on' eim='on'/>
    </iommu>
  </devices>
</domain>

The number of vCPUs is well within the limit and the XML contains IOMMU,
yet, creating the domain fails:

error: unsupported configuration: IOMMU interrupt remapping requires
split I/O APIC (ioapic driver='qemu')


Michal