qemu: Enable I/O APIC if needed

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

Posted by Michal Privoznik 1 week, 2 days ago

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
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@redhat.com>
---
 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 */
+    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;
+    }
 }
 
 
diff --git a/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml b/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
index fa3aaf0d44..5abc40e566 100644
--- a/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
+++ b/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
@@ -8,9 +8,6 @@
     <type arch='x86_64' machine='q35'>hvm</type>
     <boot dev='hd'/>
   </os>
-  <features>
-    <ioapic driver='qemu'/>
-  </features>
   <cpu mode='custom' match='exact' check='none'>
     <model fallback='forbid'>qemu64</model>
   </cpu>
-- 
2.45.2

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

Posted by Ján Tomko 1 week, 2 days ago

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@redhat.com>
>---
> 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;
     }


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

(Sadly, this is one of those enums that does not distinguish between off
and not specified)

Jano

> }
>
>
>diff --git a/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml b/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
>index fa3aaf0d44..5abc40e566 100644
>--- a/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
>+++ b/tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml
>@@ -8,9 +8,6 @@
>     <type arch='x86_64' machine='q35'>hvm</type>
>     <boot dev='hd'/>
>   </os>
>-  <features>
>-    <ioapic driver='qemu'/>
>-  </features>
>   <cpu mode='custom' match='exact' check='none'>
>     <model fallback='forbid'>qemu64</model>
>   </cpu>
>-- 
>2.45.2
>

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

Posted by Michal Prívozník 1 week, 2 days ago

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@redhat.com>
>> ---
>> 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

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

Posted by Ján Tomko 5 days, 2 hours ago

On a Thursday in 2024, Michal Prívozník wrote:
>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/
>>

With the typo fixed:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

>>> 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@redhat.com>
>>> ---
>>> src/qemu/qemu_postparse.c                         | 7 +++++++
>>> tests/qemuxmlconfdata/intel-iommu-eim-autoadd.xml | 3 ---
>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>