[v3] amd_iommu: Fixes

[PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

Posted by Sairaj Kodilkar 7 months ago

From: Vasant Hegde <vasant.hegde@amd.com>

If vCPUs > 255 then x86 common code (x86_cpus_init()) call kvm_enable_x2apic().
But if vCPUs <= 255 then the common code won't calls kvm_enable_x2apic().

This is because commit 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM
checks on XTSup feature") removed the call to kvm_enable_x2apic when xtsup
is "on", which break things when guest is booted with x2apic mode and
there are <= 255 vCPUs.

Fix this by adding back kvm_enable_x2apic() call when xtsup=on.

Fixes: 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM checks on XTSup feature")
Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Tested-by: Tested-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
---
 hw/i386/amd_iommu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index df8ba5d39ada..af85706b8a0d 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1649,6 +1649,14 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
         exit(EXIT_FAILURE);
     }
 
+    if (s->xtsup) {
+        if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
+            error_report("AMD IOMMU xtsup=on requires x2APIC support on "
+                          "the KVM side");
+            exit(EXIT_FAILURE);
+        }
+    }
+
     pci_setup_iommu(bus, &amdvi_iommu_ops, s);
     amdvi_init(s);
 }
-- 
2.34.1

Re: [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

Posted by Philippe Mathieu-Daudé 7 months ago

Hi,

On 16/5/25 12:05, Sairaj Kodilkar wrote:
> From: Vasant Hegde <vasant.hegde@amd.com>
> 
> If vCPUs > 255 then x86 common code (x86_cpus_init()) call kvm_enable_x2apic().
> But if vCPUs <= 255 then the common code won't calls kvm_enable_x2apic().
> 
> This is because commit 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM
> checks on XTSup feature") removed the call to kvm_enable_x2apic when xtsup
> is "on", which break things when guest is booted with x2apic mode and
> there are <= 255 vCPUs.
> 
> Fix this by adding back kvm_enable_x2apic() call when xtsup=on.
> 
> Fixes: 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM checks on XTSup feature")
> Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Tested-by: Tested-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> ---
>   hw/i386/amd_iommu.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index df8ba5d39ada..af85706b8a0d 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1649,6 +1649,14 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>           exit(EXIT_FAILURE);
>       }
>   
> +    if (s->xtsup) {

I suppose we need:

        if (s->xtsup && kvm_enabled()) {

otherwise that will trigger back the problem I tried to fix.
Did you try building QEMU with KVM disabled?

> +        if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
> +            error_report("AMD IOMMU xtsup=on requires x2APIC support on "
> +                          "the KVM side");
> +            exit(EXIT_FAILURE);
> +        }
> +    }
> +
>       pci_setup_iommu(bus, &amdvi_iommu_ops, s);
>       amdvi_init(s);
>   }

Re: [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

Posted by Vasant Hegde 7 months ago

Hi Philippe,


On 5/16/2025 8:08 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 16/5/25 12:05, Sairaj Kodilkar wrote:
>> From: Vasant Hegde <vasant.hegde@amd.com>
>>
>> If vCPUs > 255 then x86 common code (x86_cpus_init()) call kvm_enable_x2apic().
>> But if vCPUs <= 255 then the common code won't calls kvm_enable_x2apic().
>>
>> This is because commit 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM
>> checks on XTSup feature") removed the call to kvm_enable_x2apic when xtsup
>> is "on", which break things when guest is booted with x2apic mode and
>> there are <= 255 vCPUs.
>>
>> Fix this by adding back kvm_enable_x2apic() call when xtsup=on.
>>
>> Fixes: 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM checks on XTSup
>> feature")
>> Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> Tested-by: Tested-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Cc: Joao Martins <joao.m.martins@oracle.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
>> ---
>>   hw/i386/amd_iommu.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index df8ba5d39ada..af85706b8a0d 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -1649,6 +1649,14 @@ static void amdvi_sysbus_realize(DeviceState *dev,
>> Error **errp)
>>           exit(EXIT_FAILURE);
>>       }
>>   +    if (s->xtsup) {
> 
> I suppose we need:
> 
>        if (s->xtsup && kvm_enabled()) {
> 
> otherwise that will trigger back the problem I tried to fix.
> Did you try building QEMU with KVM disabled?

My understanding is if KVM is disabled then kvm_irqchip_is_split() will return
false and we exit on below condition check.

Yes. I had tried `configure --disable-kvm` and it was built fine.

-Vasant

> 
>> +        if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
>> +            error_report("AMD IOMMU xtsup=on requires x2APIC support on "
>> +                          "the KVM side");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +    }
>> +
>>       pci_setup_iommu(bus, &amdvi_iommu_ops, s);
>>       amdvi_init(s);
>>   }
>