[PATCH v2 0/2] amd_iommu: Fixes

[PATCH v2 0/2] amd_iommu: Fixes

[Sairaj Kodilkar]

Fix following two issues in the amd viommu
1. The guest fails to setup the passthrough device when for following setup
   because amd iommu enables the no DMA memory region even when guest is 
   using DMA remapping mode.

    -device amd-iommu,intremap=on,xtsup=on,pt=on \
    -device vfio-pci,host=<DEVID> \

    and guest forcing DMA remap mode e.g. 'iommu.passthrough=0'

    which will cause failures from QEMU:

    qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
    qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
    qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
    qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
    qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address


2. The guest fails to boot with xtsup=on and <= 255 vCPUs, because amd_iommu
   does not enable x2apic mode.

base commit 56c6e249b6988c1b6edc2dd34ebb0f1e570a1365 (v10.0.0-rc3)

Sairaj Kodilkar (1):
  hw/i386/amd_iommu: Fix device setup failure when PT is on.

Vasant Hegde (1):
  hw/i386/amd_iommu: Fix xtsup when vcpus < 255

 hw/i386/amd_iommu.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

-- 
2.34.1

Re: [PATCH v2 0/2] amd_iommu: Fixes

[Michael S. Tsirkin]

On Fri, May 09, 2025 at 12:15:24PM +0530, Sairaj Kodilkar wrote:
> Fix following two issues in the amd viommu
> 1. The guest fails to setup the passthrough device when for following setup
>    because amd iommu enables the no DMA memory region even when guest is 
>    using DMA remapping mode.
> 
>     -device amd-iommu,intremap=on,xtsup=on,pt=on \
>     -device vfio-pci,host=<DEVID> \
> 
>     and guest forcing DMA remap mode e.g. 'iommu.passthrough=0'
> 
>     which will cause failures from QEMU:
> 
>     qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>     qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>     qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>     qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>     qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
> 
> 
> 2. The guest fails to boot with xtsup=on and <= 255 vCPUs, because amd_iommu
>    does not enable x2apic mode.
> 
> base commit 56c6e249b6988c1b6edc2dd34ebb0f1e570a1365 (v10.0.0-rc3)
> 
> Sairaj Kodilkar (1):
>   hw/i386/amd_iommu: Fix device setup failure when PT is on.
> 
> Vasant Hegde (1):
>   hw/i386/amd_iommu: Fix xtsup when vcpus < 255
> 
>  hw/i386/amd_iommu.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Patches themselves were not posted to list.
Only the cover letter.

> -- 
> 2.34.1

Re: [PATCH v2 0/2] amd_iommu: Fixes

[Sairaj Kodilkar]

On 5/11/2025 11:22 PM, Michael S. Tsirkin wrote:
> On Fri, May 09, 2025 at 12:15:24PM +0530, Sairaj Kodilkar wrote:
>> Fix following two issues in the amd viommu
>> 1. The guest fails to setup the passthrough device when for following setup
>>     because amd iommu enables the no DMA memory region even when guest is
>>     using DMA remapping mode.
>>
>>      -device amd-iommu,intremap=on,xtsup=on,pt=on \
>>      -device vfio-pci,host=<DEVID> \
>>
>>      and guest forcing DMA remap mode e.g. 'iommu.passthrough=0'
>>
>>      which will cause failures from QEMU:
>>
>>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>>      qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>>      qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>>
>>
>> 2. The guest fails to boot with xtsup=on and <= 255 vCPUs, because amd_iommu
>>     does not enable x2apic mode.
>>
>> base commit 56c6e249b6988c1b6edc2dd34ebb0f1e570a1365 (v10.0.0-rc3)
>>
>> Sairaj Kodilkar (1):
>>    hw/i386/amd_iommu: Fix device setup failure when PT is on.
>>
>> Vasant Hegde (1):
>>    hw/i386/amd_iommu: Fix xtsup when vcpus < 255
>>
>>   hw/i386/amd_iommu.c | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> Patches themselves were not posted to list.
> Only the cover letter.

Sorry, messed up SMTP settings for git. Sent the remaining patches

> 
>> -- 
>> 2.34.1
> 

[PATCH v2 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.

[Sairaj Kodilkar]

Current amd_iommu enables the iommu_nodma address space when pt_supported
flag is on. This causes device to bypass the IOMMU and use untranslated
address to perform DMA when guest kernel uses DMA mode, resulting in
failure to setup the devices in the guest.

Fix the issue by always enabling amdvi_dev_as->iommu, which is explicitly
created as an IOMMU memory region (i.e. a memory region with mr->is_iommu
== true), and it is meant to support DMA remapping. It is relying on the
"side effect" that VFIO will try to register notifiers for memory regions
that are an "IOMMU" (i.e. pass the check in memory_region_is_iommu()),
and later fail when trying to register the notifier.

Fixes: c1f46999ef506 ("amd_iommu: Add support for pass though mode")
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 hw/i386/amd_iommu.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 5f9b95279997..df8ba5d39ada 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1426,7 +1426,6 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
     AMDVIState *s = opaque;
     AMDVIAddressSpace **iommu_as, *amdvi_dev_as;
     int bus_num = pci_bus_num(bus);
-    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
     iommu_as = s->address_spaces[bus_num];
 
@@ -1486,15 +1485,8 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
                                             AMDVI_INT_ADDR_FIRST,
                                             &amdvi_dev_as->iommu_ir, 1);
 
-        if (!x86_iommu->pt_supported) {
-            memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
-            memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
-                                      true);
-        } else {
-            memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
-                                      false);
-            memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, true);
-        }
+        memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
+        memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu), true);
     }
     return &iommu_as[devfn]->as;
 }
-- 
2.34.1

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

[Sairaj Kodilkar]

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 it won't call kvm_enable_x2apic().

Booting guest in x2apic mode, amd-iommu,xtsup=on and <= 255 vCPUs is
broken as it fails to call kvm_enable_x2apic().

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>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.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 v2 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

[Michael S. Tsirkin]

On Mon, May 12, 2025 at 09:36:51AM +0530, 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 it won't call kvm_enable_x2apic().
> 
> Booting guest in x2apic mode, amd-iommu,xtsup=on and <= 255 vCPUs is
> broken as it fails to call kvm_enable_x2apic().
> 
> 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>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>


You did not actually Cc Philippe.

> Cc: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Reviewed-by: Joao Martins <joao.m.martins@oracle.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 v2 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

[Michael S. Tsirkin]

On Mon, May 12, 2025 at 09:36:51AM +0530, Sairaj Kodilkar wrote:
> From: Vasant Hegde <vasant.hegde@amd.com>

A bit of history is missing here. Something like:


Before commit 8c6619f3e692 ("hw/i386/amd_iommu: Simplify non-KVM checks
on XTSup feature") amd_iommu.c used to call kvm_enable_x2apic() when
xtsup is on, to support x2apic mode.


> 
> If vCPUs > 255 then x86 common code (x86_cpus_init()) call kvm_enable_x2apic().

calls

> But if vCPUs <= 255 then it won't call kvm_enable_x2apic().

it -> the common code

> Booting guest in x2apic mode, amd-iommu,xtsup=on and <= 255 vCPUs is
> broken as it fails to call kvm_enable_x2apic().

You mean
 "is now broken as kvm_enable_x2apic() is required for x2apic to work".



> 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>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Reviewed-by: Joao Martins <joao.m.martins@oracle.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 v2 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255

[Joao Martins]

On 12/05/2025 05:06, 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 it won't call kvm_enable_x2apic().
> 
> Booting guest in x2apic mode, amd-iommu,xtsup=on and <= 255 vCPUs is
> broken as it fails to call kvm_enable_x2apic().
> 
> 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>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

FWIW I think v2 missed Alejandro's Tested-by

> ---
>  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);
>  }

Re: [PATCH v2 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.

[Michael S. Tsirkin]

On Mon, May 12, 2025 at 09:35:37AM +0530, Sairaj Kodilkar wrote:
> Current amd_iommu enables the iommu_nodma address space when pt_supported

Currently,

> flag is on. This causes device to bypass the IOMMU and use untranslated

devices

> address to perform DMA when guest kernel uses DMA mode, resulting in

the guest

> failure to setup the devices in the guest.

a failure

> 
> Fix the issue by always enabling amdvi_dev_as->iommu, which is explicitly
> created as an IOMMU memory region (i.e. a memory region with mr->is_iommu
> == true), and it is meant to support DMA remapping. It is relying on the
> "side effect" that VFIO will try to register notifiers for memory regions
> that are an "IOMMU" (i.e. pass the check in memory_region_is_iommu()),
> and later fail when trying to register the notifier.

what will fail? what is meant for what? confused.

> 
> Fixes: c1f46999ef506 ("amd_iommu: Add support for pass though mode")

explain the connection with this commit pls. did it work
prior to that?

> Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  hw/i386/amd_iommu.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 5f9b95279997..df8ba5d39ada 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1426,7 +1426,6 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>      AMDVIState *s = opaque;
>      AMDVIAddressSpace **iommu_as, *amdvi_dev_as;
>      int bus_num = pci_bus_num(bus);
> -    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>  
>      iommu_as = s->address_spaces[bus_num];
>  
> @@ -1486,15 +1485,8 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>                                              AMDVI_INT_ADDR_FIRST,
>                                              &amdvi_dev_as->iommu_ir, 1);
>  
> -        if (!x86_iommu->pt_supported) {
> -            memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
> -            memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
> -                                      true);
> -        } else {
> -            memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu),
> -                                      false);
> -            memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, true);
> -        }
> +        memory_region_set_enabled(&amdvi_dev_as->iommu_nodma, false);
> +        memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu), true);
>      }
>      return &iommu_as[devfn]->as;
>  }
> -- 
> 2.34.1

Re: [PATCH v2 0/2] amd_iommu: Fixes

[Sairaj Kodilkar]

++ Philippe Mathieu-Daudé

On 5/9/2025 12:15 PM, Sairaj Kodilkar wrote:
> Fix following two issues in the amd viommu
> 1. The guest fails to setup the passthrough device when for following setup
>     because amd iommu enables the no DMA memory region even when guest is
>     using DMA remapping mode.
> 
>      -device amd-iommu,intremap=on,xtsup=on,pt=on \
>      -device vfio-pci,host=<DEVID> \
> 
>      and guest forcing DMA remap mode e.g. 'iommu.passthrough=0'
> 
>      which will cause failures from QEMU:
> 
>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>      qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
>      qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
>      qemu-system-x86_64: AHCI: Failed to start DMA engine: bad command list buffer address
> 
> 
> 2. The guest fails to boot with xtsup=on and <= 255 vCPUs, because amd_iommu
>     does not enable x2apic mode.
> 
> base commit 56c6e249b6988c1b6edc2dd34ebb0f1e570a1365 (v10.0.0-rc3)
> 
> Sairaj Kodilkar (1):
>    hw/i386/amd_iommu: Fix device setup failure when PT is on.
> 
> Vasant Hegde (1):
>    hw/i386/amd_iommu: Fix xtsup when vcpus < 255
> 
>   hw/i386/amd_iommu.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
>