From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

The XTSup mode enables x2APIC support for AMD IOMMU, which is needed
to support vcpu w/ APIC ID > 255.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
---
v2:
- Fixed non-kvm build issue by adding a check for kvm_irqchip_is_split()

 hw/i386/amd_iommu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 9095146525e6..24eebf053df0 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -32,6 +32,7 @@
 #include "trace.h"
 #include "hw/i386/apic-msidef.h"
 #include "hw/qdev-properties.h"
+#include "kvm/kvm_i386.h"
 
 /* used AMD-Vi MMIO registers */
 const char *amdvi_mmio_low[] = {
@@ -1651,6 +1652,16 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
                                         &s->mr_ir, 1);
 
+    /* AMD IOMMU with x2APIC mode requires xtsup=on */
+    if (x86ms->apic_id_limit > 255 && !s->xtsup) {
+        error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
+        exit(EXIT_FAILURE);
+    }
+    if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
+        error_report("AMD IOMMU xt=on requires support on the KVM side");
+        exit(EXIT_FAILURE);
+    }
+
     pci_setup_iommu(bus, &amdvi_iommu_ops, s);
     amdvi_init(s);
 }
-- 
2.43.5

On 9/16/24 10:31, Santosh Shukla wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> The XTSup mode enables x2APIC support for AMD IOMMU, which is needed
> to support vcpu w/ APIC ID > 255.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
> ---
> v2:
> - Fixed non-kvm build issue by adding a check for kvm_irqchip_is_split()
> 
>   hw/i386/amd_iommu.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 9095146525e6..24eebf053df0 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -32,6 +32,7 @@
>   #include "trace.h"
>   #include "hw/i386/apic-msidef.h"
>   #include "hw/qdev-properties.h"
> +#include "kvm/kvm_i386.h"
>   
>   /* used AMD-Vi MMIO registers */
>   const char *amdvi_mmio_low[] = {
> @@ -1651,6 +1652,16 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
>                                           &s->mr_ir, 1);
>   
> +    /* AMD IOMMU with x2APIC mode requires xtsup=on */
> +    if (x86ms->apic_id_limit > 255 && !s->xtsup) {
> +        error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
> +        exit(EXIT_FAILURE);
> +    }
> +    if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
> +        error_report("AMD IOMMU xt=on requires support on the KVM side");

Use "xtsup=on" on the error message, "xt" is not a valid option IIUC.

Alejandro

> +        exit(EXIT_FAILURE);
> +    }
> +
>       pci_setup_iommu(bus, &amdvi_iommu_ops, s);
>       amdvi_init(s);
>   }

On 9/21/2024 1:56 AM, Alejandro Jimenez wrote:
> 
> 
> On 9/16/24 10:31, Santosh Shukla wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> The XTSup mode enables x2APIC support for AMD IOMMU, which is needed
>> to support vcpu w/ APIC ID > 255.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
>> ---
>> v2:
>> - Fixed non-kvm build issue by adding a check for kvm_irqchip_is_split()
>>
>>   hw/i386/amd_iommu.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 9095146525e6..24eebf053df0 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -32,6 +32,7 @@
>>   #include "trace.h"
>>   #include "hw/i386/apic-msidef.h"
>>   #include "hw/qdev-properties.h"
>> +#include "kvm/kvm_i386.h"
>>     /* used AMD-Vi MMIO registers */
>>   const char *amdvi_mmio_low[] = {
>> @@ -1651,6 +1652,16 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
>>       memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
>>                                           &s->mr_ir, 1);
>>   +    /* AMD IOMMU with x2APIC mode requires xtsup=on */
>> +    if (x86ms->apic_id_limit > 255 && !s->xtsup) {
>> +        error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +    if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
>> +        error_report("AMD IOMMU xt=on requires support on the KVM side");
> 
> Use "xtsup=on" on the error message, "xt" is not a valid option IIUC.
> 

Yes - V3, thank you for spotting.

Thanks,
Santosh

> Alejandro
> 
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>>       pci_setup_iommu(bus, &amdvi_iommu_ops, s);
>>       amdvi_init(s);
>>   }