qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] amd_iommu: Fixes
@ 2025-05-16 10:05 Sairaj Kodilkar
  2025-05-16 10:05 ` [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on Sairaj Kodilkar
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-05-16 10:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	sarunkod

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
  2025-05-16 10:05 [PATCH v3 0/2] amd_iommu: Fixes Sairaj Kodilkar
@ 2025-05-16 10:05 ` Sairaj Kodilkar
  2025-05-16 14:43   ` Philippe Mathieu-Daudé
  2025-05-16 10:05 ` [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255 Sairaj Kodilkar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-05-16 10:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	sarunkod, Vasant Hegde

Commit c1f46999ef506 ("amd_iommu: Add support for pass though mode")
introduces the support for "pt" flag by enabling nodma memory when
"pt=off". This allowed VFIO devices to successfully register notifiers
by using nodma region.

But, This also broke things when guest is booted with the iommu=nopt
because, devices bypass the IOMMU and use untranslated addresses (IOVA) to
perform DMA reads/writes to the nodma memory region, ultimately resulting in
a failure to setup the devices in the guest.

Fix the above issue by always enabling the amdvi_dev_as->iommu memory region.
But this will once again cause VFIO devices to fail while registering the
notifiers with AMD IOMMU memory region.

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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255
  2025-05-16 10:05 [PATCH v3 0/2] amd_iommu: Fixes Sairaj Kodilkar
  2025-05-16 10:05 ` [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on Sairaj Kodilkar
@ 2025-05-16 10:05 ` Sairaj Kodilkar
  2025-05-16 14:38   ` Philippe Mathieu-Daudé
  2025-05-16 10:36 ` [PATCH v3 0/2] amd_iommu: Fixes Michael S. Tsirkin
  2025-06-02 20:54 ` Michael Tokarev
  3 siblings, 1 reply; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-05-16 10:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	sarunkod, Vasant Hegde, Philippe Mathieu-Daudé

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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/2] amd_iommu: Fixes
  2025-05-16 10:05 [PATCH v3 0/2] amd_iommu: Fixes Sairaj Kodilkar
  2025-05-16 10:05 ` [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on Sairaj Kodilkar
  2025-05-16 10:05 ` [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255 Sairaj Kodilkar
@ 2025-05-16 10:36 ` Michael S. Tsirkin
  2025-05-16 10:45   ` Sairaj Kodilkar
  2025-06-02 20:54 ` Michael Tokarev
  3 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2025-05-16 10:36 UTC (permalink / raw)
  To: Sairaj Kodilkar
  Cc: qemu-devel, marcel.apfelbaum, pbonzini, richard.henderson,
	eduardo, suravee.suthikulpanit, alejandro.j.jimenez,
	joao.m.martins

On Fri, May 16, 2025 at 03:35:33PM +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(-)

changelog?

> -- 
> 2.34.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/2] amd_iommu: Fixes
  2025-05-16 10:36 ` [PATCH v3 0/2] amd_iommu: Fixes Michael S. Tsirkin
@ 2025-05-16 10:45   ` Sairaj Kodilkar
  0 siblings, 0 replies; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-05-16 10:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, marcel.apfelbaum, pbonzini, richard.henderson,
	eduardo, suravee.suthikulpanit, alejandro.j.jimenez,
	joao.m.martins



On 5/16/2025 4:06 PM, Michael S. Tsirkin wrote:
> On Fri, May 16, 2025 at 03:35:33PM +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(-)
> 
> changelog?
> 

Sorry I forgot to update the cover-letter (Rookie mistake).

Changes Since v2:
Patch 1: Updated commit message [mst]
Patch 2: Updated commit message [mst]
v2: 
https://lore.kernel.org/qemu-devel/20250509064526.15500-1-sarunkod@amd.com/

Changes Since v1:
Patch 1: Updated commit message [Alejandro]
Patch 2: None
v1: 
https://lore.kernel.org/qemu-devel/20250410064447.29583-1-sarunkod@amd.com/

Regards
Sairaj Kodilkar
>> -- 
>> 2.34.1
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255
  2025-05-16 10:05 ` [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255 Sairaj Kodilkar
@ 2025-05-16 14:38   ` Philippe Mathieu-Daudé
  2025-05-19  7:35     ` Vasant Hegde
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-16 14:38 UTC (permalink / raw)
  To: Sairaj Kodilkar, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	Vasant Hegde

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
  2025-05-16 10:05 ` [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on Sairaj Kodilkar
@ 2025-05-16 14:43   ` Philippe Mathieu-Daudé
  2025-05-19  3:59     ` Sairaj Kodilkar
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-16 14:43 UTC (permalink / raw)
  To: Sairaj Kodilkar, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	Vasant Hegde

On 16/5/25 12:05, Sairaj Kodilkar wrote:
> Commit c1f46999ef506 ("amd_iommu: Add support for pass though mode")
> introduces the support for "pt" flag by enabling nodma memory when
> "pt=off". This allowed VFIO devices to successfully register notifiers
> by using nodma region.
> 
> But, This also broke things when guest is booted with the iommu=nopt
> because, devices bypass the IOMMU and use untranslated addresses (IOVA) to
> perform DMA reads/writes to the nodma memory region, ultimately resulting in
> a failure to setup the devices in the guest.
> 
> Fix the above issue by always enabling the amdvi_dev_as->iommu memory region.
> But this will once again cause VFIO devices to fail while registering the
> notifiers with AMD IOMMU memory region.
> 
> 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);

I have no clue about this device but wonder what is the usefulness of
iommu_nodma now, isn't it dead code?

> +        memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as->iommu), true);
>       }
>       return &iommu_as[devfn]->as;
>   }



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
  2025-05-16 14:43   ` Philippe Mathieu-Daudé
@ 2025-05-19  3:59     ` Sairaj Kodilkar
  0 siblings, 0 replies; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-05-19  3:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	Vasant Hegde



On 5/16/2025 8:13 PM, Philippe Mathieu-Daudé wrote:
> On 16/5/25 12:05, Sairaj Kodilkar wrote:
>> Commit c1f46999ef506 ("amd_iommu: Add support for pass though mode")
>> introduces the support for "pt" flag by enabling nodma memory when
>> "pt=off". This allowed VFIO devices to successfully register notifiers
>> by using nodma region.
>>
>> But, This also broke things when guest is booted with the iommu=nopt
>> because, devices bypass the IOMMU and use untranslated addresses 
>> (IOVA) to
>> perform DMA reads/writes to the nodma memory region, ultimately 
>> resulting in
>> a failure to setup the devices in the guest.
>>
>> Fix the above issue by always enabling the amdvi_dev_as->iommu memory 
>> region.
>> But this will once again cause VFIO devices to fail while registering the
>> notifiers with AMD IOMMU memory region.
>>
>> 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);
> 
> I have no clue about this device but wonder what is the usefulness of
> iommu_nodma now, isn't it dead code?
> 

Hi Philippe,

Indeed the iommu_nodma is dead. The reason I did not remove the
iommu_nodma region completely is that, Alejandro's DMA remapping patches
[1] uses this region to dynamically switch the address space.

[1] 
https://lore.kernel.org/qemu-devel/20250502021605.1795985-1-alejandro.j.jimenez@oracle.com/

Thanks
Sairaj

>> +        memory_region_set_enabled(MEMORY_REGION(&amdvi_dev_as- 
>> >iommu), true);
>>       }
>>       return &iommu_as[devfn]->as;
>>   }
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255
  2025-05-16 14:38   ` Philippe Mathieu-Daudé
@ 2025-05-19  7:35     ` Vasant Hegde
  0 siblings, 0 replies; 11+ messages in thread
From: Vasant Hegde @ 2025-05-19  7:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Sairaj Kodilkar, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/2] amd_iommu: Fixes
  2025-05-16 10:05 [PATCH v3 0/2] amd_iommu: Fixes Sairaj Kodilkar
                   ` (2 preceding siblings ...)
  2025-05-16 10:36 ` [PATCH v3 0/2] amd_iommu: Fixes Michael S. Tsirkin
@ 2025-06-02 20:54 ` Michael Tokarev
  2025-06-10  8:33   ` Sairaj Kodilkar
  3 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2025-06-02 20:54 UTC (permalink / raw)
  To: Sairaj Kodilkar, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	qemu-stable

On 16.05.2025 13:05, 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

Hi!

Is this a qemu-stable material (for 10.0.x)?

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/2] amd_iommu: Fixes
  2025-06-02 20:54 ` Michael Tokarev
@ 2025-06-10  8:33   ` Sairaj Kodilkar
  0 siblings, 0 replies; 11+ messages in thread
From: Sairaj Kodilkar @ 2025-06-10  8:33 UTC (permalink / raw)
  To: Michael Tokarev, qemu-devel
  Cc: mst, marcel.apfelbaum, pbonzini, richard.henderson, eduardo,
	suravee.suthikulpanit, alejandro.j.jimenez, joao.m.martins,
	qemu-stable



On 6/3/2025 2:24 AM, Michael Tokarev wrote:
> On 16.05.2025 13:05, 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
> 
> Hi!
> 
> Is this a qemu-stable material (for 10.0.x)?

Yes it is

Thanks
Sairaj Kodilkar




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-06-10  8:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-16 10:05 [PATCH v3 0/2] amd_iommu: Fixes Sairaj Kodilkar
2025-05-16 10:05 ` [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on Sairaj Kodilkar
2025-05-16 14:43   ` Philippe Mathieu-Daudé
2025-05-19  3:59     ` Sairaj Kodilkar
2025-05-16 10:05 ` [PATCH v3 2/2] hw/i386/amd_iommu: Fix xtsup when vcpus < 255 Sairaj Kodilkar
2025-05-16 14:38   ` Philippe Mathieu-Daudé
2025-05-19  7:35     ` Vasant Hegde
2025-05-16 10:36 ` [PATCH v3 0/2] amd_iommu: Fixes Michael S. Tsirkin
2025-05-16 10:45   ` Sairaj Kodilkar
2025-06-02 20:54 ` Michael Tokarev
2025-06-10  8:33   ` Sairaj Kodilkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).