1. Patchew
  2. QEMU
  3. [PATCH v3 0/2] amd_iommu: Fixes
  4. View patch

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

Sairaj Kodilkar posted 2 patches 7 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
Posted by Sairaj Kodilkar 7 months ago 
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
Re: [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
Posted by Philippe Mathieu-Daudé 7 months ago 
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;
>   }
Re: [PATCH v3 1/2] hw/i386/amd_iommu: Fix device setup failure when PT is on.
Posted by Sairaj Kodilkar 7 months ago 

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.