AMD-Vi cleanups around domain device setup

[PATCH 2/2] amd/iommu: Remove dead non-atomic update checking

Posted by Teddy Astie 2 months, 4 weeks ago

When updating a DTE, amd_iommu_setup_domain_device() would check if
the update had been non-atomic (i.e rc > 0) and throw a warning if
such non-atomic update could be dangerous.  However since commit
3fc44151d83d, rc can no longer be positive, making this branch
unreachable code.

No functional change intended.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 3a14770855..02eee4e658 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -225,24 +225,6 @@ static int __must_check amd_iommu_setup_domain_device(
             spin_unlock_irqrestore(&iommu->lock, flags);
             return rc;
         }
-        if ( rc &&
-             domain != pdev->domain &&
-             /*
-              * By non-atomically updating the DTE's domain ID field last,
-              * during a short window in time TLB entries with the old domain
-              * ID but the new page tables may have been inserted.  This could
-              * affect I/O of other devices using this same (old) domain ID.
-              * Such updating therefore is not a problem if this was the only
-              * device associated with the old domain ID.  Diverting I/O of any
-              * of a dying domain's devices to the quarantine page tables is
-              * intended anyway.
-              */
-             !pdev->domain->is_dying &&
-             pdev->domain != dom_io &&
-             (any_pdev_behind_iommu(pdev->domain, pdev, iommu) ||
-              pdev->phantom_stride) )
-            AMD_IOMMU_WARN(" %pp: reassignment may cause %pd data corruption\n",
-                           &PCI_SBDF(pdev->seg, bus, devfn), pdev->domain);
 
         /*
          * Check remaining settings are still in place from an earlier call
-- 
2.51.2



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH 2/2] amd/iommu: Remove dead non-atomic update checking

Posted by Jan Beulich 2 months, 4 weeks ago

On 12.11.2025 16:37, Teddy Astie wrote:
> When updating a DTE, amd_iommu_setup_domain_device() would check if
> the update had been non-atomic (i.e rc > 0) and throw a warning if
> such non-atomic update could be dangerous.  However since commit
> 3fc44151d83d, rc can no longer be positive, making this branch
> unreachable code.

I.e. it addresses a Misra concern and hence ...

> No functional change intended.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

... wants at least an Amends: tag, likely a Fixes: one. Then:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH 2/2] amd/iommu: Remove dead non-atomic update checking

Posted by Teddy Astie 2 months, 3 weeks ago

Le 13/11/2025 à 12:39, Jan Beulich a écrit :
> On 12.11.2025 16:37, Teddy Astie wrote:
>> When updating a DTE, amd_iommu_setup_domain_device() would check if
>> the update had been non-atomic (i.e rc > 0) and throw a warning if
>> such non-atomic update could be dangerous.  However since commit
>> 3fc44151d83d, rc can no longer be positive, making this branch
>> unreachable code.
> 
> I.e. it addresses a Misra concern and hence ...
> 
>> No functional change intended.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> 
> ... wants at least an Amends: tag, likely a Fixes: one. Then:

With :

Fixes: 3fc44151d83d ("x86/iommu: remove non-CX16 logic from DMA remapping")

?

As it is the commit that removed the rc > 0 case.

> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan
> 



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH 2/2] amd/iommu: Remove dead non-atomic update checking

Posted by Jan Beulich 2 months, 3 weeks ago

On 19.11.2025 11:22, Teddy Astie wrote:
> Le 13/11/2025 à 12:39, Jan Beulich a écrit :
>> On 12.11.2025 16:37, Teddy Astie wrote:
>>> When updating a DTE, amd_iommu_setup_domain_device() would check if
>>> the update had been non-atomic (i.e rc > 0) and throw a warning if
>>> such non-atomic update could be dangerous.  However since commit
>>> 3fc44151d83d, rc can no longer be positive, making this branch
>>> unreachable code.
>>
>> I.e. it addresses a Misra concern and hence ...
>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
>>
>> ... wants at least an Amends: tag, likely a Fixes: one. Then:
> 
> With :
> 
> Fixes: 3fc44151d83d ("x86/iommu: remove non-CX16 logic from DMA remapping")
> 
> ?
> 
> As it is the commit that removed the rc > 0 case.

Yes, that looks to be the one.

Jan

Re: [PATCH 2/2] amd/iommu: Remove dead non-atomic update checking

Posted by Jason Andryuk 2 months, 3 weeks ago

On 2025-11-13 06:37, Jan Beulich wrote:
> On 12.11.2025 16:37, Teddy Astie wrote:
>> When updating a DTE, amd_iommu_setup_domain_device() would check if
>> the update had been non-atomic (i.e rc > 0) and throw a warning if
>> such non-atomic update could be dangerous.  However since commit
>> 3fc44151d83d, rc can no longer be positive, making this branch
>> unreachable code.
> 
> I.e. it addresses a Misra concern and hence ...
> 
>> No functional change intended.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> 
> ... wants at least an Amends: tag, likely a Fixes: one. Then:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>