Intel IOMMU updates for v6.17
Subsystems (2)
Lu Baolu
maintainerHi Joerg,
The following changes have been queued for v6.17-rc1. They are about new features and code refactoring, including:
- Reorganize Intel VT-d to be ready for iommupt
- Optimize iotlb_sync_map for non-caching/non-RWBF modes
- Fix missed PASID in dev TLB invalidation in cache_tag_flush_all()
- Miscellaneous cleanups
These patches are based on v6.16-rc6. Please consider them for the iommu/vt-d branch.
Best regards,
baolu
Ethan Milon (2):
iommu/vt-d: Fix missing PASID in dev TLB flush with
cache_tag_flush_all
iommu/vt-d: Deduplicate cache_tag_flush_all by reusing flush_range
Jason Gunthorpe (7):
iommu/vt-d: Lift the __pa to
domain_setup_first_level/intel_svm_set_dev_pasid()
iommu/vt-d: Fold domain_exit() into intel_iommu_domain_free() iommu/vt-d: Do not wipe out the page table NID when devices detach iommu/vt-d: Split intel_iommu_domain_alloc_paging_flags() iommu/vt-d: Create unique domain ops for each stage iommu/vt-d: Split intel_iommu_enforce_cache_coherency() iommu/vt-d: Split paging_domain_compatible()
Lu Baolu (1):
iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Vineeth Pillai (Google) (1):
iommu/vt-d: Remove the CONFIG_X86 wrapping from iommu init hook
drivers/iommu/intel/cache.c | 55 ++---- drivers/iommu/intel/dmar.c | 3 - drivers/iommu/intel/iommu.c | 336 ++++++++++++++++++++++------------- drivers/iommu/intel/iommu.h | 22 ++- drivers/iommu/intel/nested.c | 4 +- drivers/iommu/intel/pasid.c | 17 +- drivers/iommu/intel/pasid.h | 11 +- drivers/iommu/intel/svm.c | 3 +- drivers/iommu/intel/trace.h | 5 - 9 files changed, 259 insertions(+), 197 deletions(-)
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 1/11 iommu/vt-d: Remove the CONFIG_X86 wrapping from iommu init hook
From: "Vineeth Pillai (Google)" <[email protected]>
iommu init hook is wrapped in CONFI_X86 and is a remnant of dmar.c when it was a common code in "drivers/pci/dmar.c". This was added in commit (9d5ce73a64be2 x86: intel-iommu: Convert detect_intel_iommu to use iommu_init hook)
Now this is built only for x86. This config wrap could be removed.
--- drivers/iommu/intel/dmar.c | 3 --- 1 file changed, 3 deletions(-)
drivers/iommu/intel/dmar.c +0 -3
@@ -935,14 +935,11 @@ void __init detect_intel_iommu(void)pci_request_acs (); } -#ifdef CONFIG_X86 if (!ret) { x86_init.iommu.iommu_init = intel_iommu_init; x86_platform.iommu_shutdown = intel_iommu_shutdown; } -#endif -if (dmar_tbl) {acpi_put_table (dmar_tbl); dmar_tbl =NULL ;
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 2/11 iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
In reply to Lu BaoluThe iotlb_sync_map iommu ops allows drivers to perform necessary cache flushes when new mappings are established. For the Intel iommu driver, this callback specifically serves two purposes:
- To flush caches when a second-stage page table is attached to a device whose iommu is operating in caching mode (CAP_REG.CM==1).
- To explicitly flush internal write buffers to ensure updates to memory- resident remapping structures are visible to hardware (CAP_REG.RWBF==1).
However, in scenarios where neither caching mode nor the RWBF flag is active, the cache_tag_flush_range_np() helper, which is called in the iotlb_sync_map path, effectively becomes a no-op.
Despite being a no-op, cache_tag_flush_range_np() involves iterating through all cache tags of the iommu's attached to the domain, protected by a spinlock. This unnecessary execution path introduces overhead, leading to a measurable I/O performance regression. On systems with NVMes under the same bridge, performance was observed to drop from approximately ~6150 MiB/s down to ~4985 MiB/s.
Introduce a flag in the dmar_domain structure. This flag will only be set when iotlb_sync_map is required (i.e., when CM or RWBF is set). The cache_tag_flush_range_np() is called only for domains where this flag is set. This flag, once set, is immutable, given that there won't be mixed configurations in real-world scenarios where some IOMMUs in a system operate in caching mode while others do not. Theoretically, the immutability of this flag does not impact functionality.
--- drivers/iommu/intel/iommu.c | 19 ++++++++++++++++++- drivers/iommu/intel/iommu.h | 3 +++ 2 files changed, 21 insertions(+), 1 deletion(-)
drivers/iommu/intel/iommu.c +18 -1
@@ -1796,6 +1796,18 @@ static int domain_setup_first_level(struct intel_iommu *iommu, (pgd_t *)pgd, flags, old); } +static bool domain_need_iotlb_sync_map (struct dmar_domain *domain, +struct intel_iommu *iommu) +{ +if (cap_caching_mode (iommu->cap) && !domain->use_first_level) +return true; + +if (rwbf_quirk ||cap_rwbf (iommu->cap)) +return true; + +return false; +} +static int dmar_domain_attach_device (struct dmar_domain *domain,struct device *dev) {
@@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,if (ret) goto out_block_translation; + domain->iotlb_sync_map |=domain_need_iotlb_sync_map (domain, iommu); +return 0 ; out_block_translation:
@@ -3954,7 +3968,10 @@ static bool risky_device(struct pci_dev *pdev)static int intel_iommu_iotlb_sync_map (struct iommu_domain *domain,unsigned long iova,size_t size) { -cache_tag_flush_range_np (to_dmar_domain (domain), iova, iova + size -1 ); +struct dmar_domain *dmar_domain =to_dmar_domain (domain); + +if (dmar_domain->iotlb_sync_map) +cache_tag_flush_range_np (dmar_domain, iova, iova + size -1 );return 0 ; }
drivers/iommu/intel/iommu.h +3 -0
@@ -614,6 +614,9 @@ struct dmar_domain { u8 has_mappings:1; /* Has mappings configured through* iommu_map() interface. */ + u8 iotlb_sync_map:1; /* Need to flush IOTLB cache or write +* buffer when creating mappings. +*/ spinlock_t lock;/* Protect device tracking lists */ struct list_head devices;/* all devices' list */
Signature · 1 line
2.43.0
Re: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
drivers/iommu/intel/iommu.c static int dmar_domain_attach_device(struct dmar_domain *domain, · line 1848
Quoted 5 lines · Lu Baolu
drivers/iommu/intel/iommu.c +1
-0@@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,if (ret) goto out_block_translation; + domain->iotlb_sync_map |=domain_need_iotlb_sync_map (domain, iommu);
This has no locking and is in the wrong order anyhow :(
Any change to how invalidation works has to be done before attaching the HW so that the required invalidations are already happening before the HW can walk the page table.
And you need to serialize somehow with concurrent map/unmap as iommufd doesn't prevent userspace from racing attach with map/unmap.
The cache_tag_assign_domain() looks similarly wrong too, it needs to start invalidating the cache tag of the new domain, then change the context then stop invalidating the cache tag of the old domain. Otherwise there are invalidation races.
Finally, if the HW needs RWBF then this also needs to do the buffer flush in this thread before installing the context to prevent a race.
Overall this dynamic behavior may just be a bad idea, and perhaps you can live with domains having the domain->iotlb_sync_map as a static property set once during paging domain allocation.
If the iommu requires iotlb_sync_map but the domain does not have it then the attach is rejected. This reduces domain sharing possibilities, but maybe that is just fine??
Jason
Baolu Lu
maintainerRe: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Quoted 9 lines · Jason Gunthorpe
drivers/iommu/intel/iommu.c static int dmar_domain_attach_device(struct dmar_domain *domain, · line 1848
Quoted 5 lines · Lu Baolu
drivers/iommu/intel/iommu.c +1
-0@@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,if (ret) goto out_block_translation; + domain->iotlb_sync_map |=domain_need_iotlb_sync_map (domain, iommu);This has no locking and is in the wrong order anyhow :(
Quoted 6 lines
Any change to how invalidation works has to be done before attaching the HW so that the required invalidations are already happening before the HW can walk the page table.
And you need to serialize somehow with concurrent map/unmap as iommufd doesn't prevent userspace from racing attach with map/unmap.
domain->iotlb_sync_map does not change the driver's behavior. It simply indicates that there's no need to waste time calling cache_tag_flush_range_np(), as it's just a no-op.
Quoted 9 lines
The
cache_tag_assign_domain()looks similarly wrong too, it needs to start invalidating the cache tag of the new domain, then change the context then stop invalidating the cache tag of the old domain. Otherwise there are invalidation races.Finally, if the HW needs RWBF then this also needs to do the buffer flush in this thread before installing the context to prevent a race.
Quoted 7 lines
Overall this dynamic behavior may just be a bad idea, and perhaps you can live with domains having the
domain->iotlb_sync_mapas a static property set once during paging domain allocation.If the iommu requires
iotlb_sync_mapbut the domain does not have it then the attach is rejected. This reduces domain sharing possibilities, but maybe that is just fine??
I previously discussed this with Kevin, and we agreed on a phase-by- phase approach. As I mentioned, domain->iotlb_sync_map is merely a hint for the driver, preventing it from looping through all cache tags to determine if any cache invalidation work needs to be performed. We already know it's predetermined that no work needs to be done.
RWBF is only required on some early implementations where memory coherence was not yet implemented by the VT-d engine. It should be difficult to find such systems in modern environments. Thus, iotlb_sync_map is primarily relevant for nested translation that utilizes S2 shadowing page tables. This, too, is a legacy feature, as Intel has supported hardware-assisted nested translation for years.
Making iotlb_sync_map static is a feature, not an optimization. We are still evaluating the value of this, as it's only truly helpful if there are real use cases.
Thanks,
baolu
Re: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Quoted 13 lines · Baolu Lu
Quoted 12 lines · Jason Gunthorpe
drivers/iommu/intel/iommu.c static int dmar_domain_attach_device(struct dmar_domain *domain, · line 1847
Quoted 4 lines · Lu Baolu
drivers/iommu/intel/iommu.c +1
-0@@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,if (ret) goto out_block_translation; + domain->iotlb_sync_map |=domain_need_iotlb_sync_map (domain, iommu);This has no locking and is in the wrong order anyhow :(
Any change to how invalidation works has to be done before attaching the HW so that the required invalidations are already happening before the HW can walk the page table.
Quoted 6 lines
Quoted 2 lines
And you need to serialize somehow with concurrent map/unmap as iommufd doesn't prevent userspace from racing attach with map/unmap.
domain->iotlb_sync_mapdoes not change the driver's behavior. It simply indicates that there's no need to waste time callingcache_tag_flush_range_np(), as it's just a no-op.
Of course it changes the behavior, it changes what the invalidation callback does.
Without locking you have a race situation where a PGD is visible to HW that requires extra flushing and the SW is not doing the extra flushing.
Before any PGD is made visible to the HW the software must ensure all the required invalidations are happening.
Quoted 5 lines
I previously discussed this with Kevin, and we agreed on a phase-by- phase approach. As I mentioned,
domain->iotlb_sync_mapis merely a hint for the driver, preventing it from looping through all cache tags to determine if any cache invalidation work needs to be performed. We already know it's predetermined that no work needs to be done.
The iteration though the cache tags is done inside a lock so it doesn't have this race (it has the issue I mentioned setting up the cache tage list though).
Quoted 3 lines
RWBF is only required on some early implementations where memory coherence was not yet implemented by the VT-d engine. It should be difficult to find such systems in modern environments.
Then I would set it at domain creation time, check it during attach, and remove this race.
Jason
Baolu Lu
maintainerRe: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Quoted 24 lines · Jason Gunthorpe
Quoted 13 lines · Baolu Lu
Quoted 12 lines · Jason Gunthorpe
Review static int dmar_domain_attach_device(struct dmar_domain *domain, · line 1847
Quoted 4 lines · Lu Baolu
(quoted diff) +1
-0@@ -1833,6 +1845,8 @@ static int dmar_domain_attach_device(struct dmar_domain *domain, if (ret) goto out_block_translation; + domain->iotlb_sync_map |= domain_need_iotlb_sync_map(domain, iommu);This has no locking and is in the wrong order anyhow :(
Any change to how invalidation works has to be done before attaching the HW so that the required invalidations are already happening before the HW can walk the page table.
Quoted 6 lines
Quoted 2 lines
And you need to serialize somehow with concurrent map/unmap as iommufd doesn't prevent userspace from racing attach with map/unmap.
domain->iotlb_sync_mapdoes not change the driver's behavior. It simply indicates that there's no need to waste time callingcache_tag_flush_range_np(), as it's just a no-op.Of course it changes the behavior, it changes what the invalidation callback does.
Quoted 6 lines
Without locking you have a race situation where a PGD is visible to HW that requires extra flushing and the SW is not doing the extra flushing.
Before any PGD is made visible to the HW the software must ensure all the required invalidations are happening.
Oh, I understand now. If there is no synchronization between attach/ detach and map/unmap operations, the cache invalidation behavior must be determined when a domain is allocated.
Quoted 11 lines
Quoted 6 lines
I previously discussed this with Kevin, and we agreed on a phase-by- phase approach. As I mentioned,
domain->iotlb_sync_mapis merely a hint for the driver, preventing it from looping through all cache tags to determine if any cache invalidation work needs to be performed. We already know it's predetermined that no work needs to be done.The iteration though the cache tags is done inside a lock so it doesn't have this race (it has the issue I mentioned setting up the cache tage list though).
Quoted 6 lines
Quoted 3 lines
RWBF is only required on some early implementations where memory coherence was not yet implemented by the VT-d engine. It should be difficult to find such systems in modern environments.
Then I would set it at domain creation time, check it during attach, and remove this race.
How about the following changes (compiled but not tested)?
drivers/iommu/intel/iommu.c +27 -14
@@ -1780,18 +1780,6 @@ static int domain_setup_first_level(struct intel_iommu *iommu, __pa(pgd), flags, old); } -static bool domain_need_iotlb_sync_map (struct dmar_domain *domain, -struct intel_iommu *iommu) -{ -if (cap_caching_mode (iommu->cap) &&intel_domain_is_ss_paging (domain)) -return true; - -if (rwbf_quirk ||cap_rwbf (iommu->cap)) -return true; - -return false; -} -static int dmar_domain_attach_device (struct dmar_domain *domain,struct device *dev) {
@@ -1831,8 +1819,6 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,if (ret) goto out_block_translation; - domain->iotlb_sync_map |=domain_need_iotlb_sync_map (domain, iommu); -return 0 ; out_block_translation:
@@ -3352,6 +3338,14 @@ intel_iommu_domain_alloc_first_stage(struct device *dev,return ERR_CAST (dmar_domain); dmar_domain->domain.ops = &intel_fs_paging_domain_ops; +/* +* iotlb sync for map is only needed for legacy implementations that +* explicitly require flushing internal write buffers to ensure memory +* coherence. +*/ +if (rwbf_quirk ||cap_rwbf (iommu->cap)) + dmar_domain->iotlb_sync_map = true; +return &dmar_domain->domain; }
@@ -3386,6 +3380,14 @@ intel_iommu_domain_alloc_second_stage(struct device *dev,if (flags &IOMMU_HWPT_ALLOC_DIRTY_TRACKING ) dmar_domain->domain.dirty_ops = &intel_dirty_ops; +/* +* Besides the internal write buffer flush, the caching mode used for +* legacy nested translation (which utilizes shadowing page tables) +* also requires iotlb sync on map. +*/ +if (rwbf_quirk ||cap_rwbf (iommu->cap) ||cap_caching_mode (iommu->cap)) + dmar_domain->iotlb_sync_map = true; +return &dmar_domain->domain; }
@@ -3446,6 +3448,11 @@ static int paging_domain_compatible_first_stage(struct dmar_domain *dmar_domain,if (!cap_fl1gp_support (iommu->cap) && (dmar_domain->domain.pgsize_bitmap &SZ_1G ))return -EINVAL ; + +/* iotlb sync on map requirement */ +if ((rwbf_quirk ||cap_rwbf (iommu->cap)) && !dmar_domain->iotlb_sync_map) +return -EINVAL ; +return 0 ; }
@@ -3469,6 +3476,12 @@ paging_domain_compatible_second_stage(struct dmar_domain *dmar_domain,return -EINVAL ;if (!(sslps &BIT (1 )) && (dmar_domain->domain.pgsize_bitmap &SZ_1G ))return -EINVAL ; + +/* iotlb sync on map requirement */ +if ((rwbf_quirk ||cap_rwbf (iommu->cap) ||cap_caching_mode (iommu->cap)) && + !dmar_domain->iotlb_sync_map) +return -EINVAL ; +return 0 ; }
Signature · 4 lines
2.43.0 Thanks, baolu
Re: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Quoted 1 line · Baolu Lu
How about the following changes (compiled but not tested)?
Yes, exactly what I was thinking!
That leaves the cache tag linked list race..
Nicolin is working on a neat solution for SMMU I wonder if we can reuse it here too..
Jason
Baolu Lu
maintainerRe: [PATCH 02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes
Quoted 4 lines · Jason Gunthorpe
Quoted 1 line · Baolu Lu
How about the following changes (compiled but not tested)?
Yes, exactly what I was thinking!
I will submit a formal patch.
Quoted 5 lines
That leaves the cache tag linked list race..
Nicolin is working on a neat solution for SMMU I wonder if we can reuse it here too..
Okay, let's wait and see.
Thanks,
baolu
Lu Baolu
maintainerPATCH 3/11 iommu/vt-d: Lift the __pa to domain_setup_first_level/intel_svm_set_dev_pasid()
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
Pass the phys_addr_t down through the call chain from the top instead of passing a pgd_t * KVA. This moves the __pa() into domain_setup_first_level() which is the first function to obtain the pgd from the IOMMU page table in this call chain.
The SVA flow is also adjusted to get the pa of the mm->pgd.
iommput will move the __pa() into iommupt code, it never shares the KVA of the page table with the driver.
--- drivers/iommu/intel/iommu.c | 15 +++++++-------- drivers/iommu/intel/iommu.h | 7 +++---- drivers/iommu/intel/pasid.c | 17 +++++++++-------- drivers/iommu/intel/pasid.h | 11 +++++------ drivers/iommu/intel/svm.c | 2 +- 5 files changed, 25 insertions(+), 27 deletions(-)
drivers/iommu/intel/iommu.c +7 -8
@@ -1736,15 +1736,14 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8intel_context_flush_no_pasid (info, context, did); } -int __domain_setup_first_level (struct intel_iommu *iommu, -struct device *dev,ioasid_t pasid, -u16 did,pgd_t *pgd,int flags, -struct iommu_domain *old) +int __domain_setup_first_level (struct intel_iommu *iommu,struct device *dev, +ioasid_t pasid,u16 did,phys_addr_t fsptptr, +int flags,struct iommu_domain *old) {if (!old) -return intel_pasid_setup_first_level (iommu, dev, pgd, - pasid, did, flags); -return intel_pasid_replace_first_level (iommu, dev, pgd, pasid, did, +return intel_pasid_setup_first_level (iommu, dev, fsptptr, pasid, + did, flags); +return intel_pasid_replace_first_level (iommu, dev, fsptptr, pasid, did,iommu_domain_did (old, iommu), flags); }
@@ -1793,7 +1792,7 @@ static int domain_setup_first_level(struct intel_iommu *iommu,return __domain_setup_first_level (iommu, dev, pasid,domain_id_iommu (domain, iommu), - (pgd_t *)pgd, flags, old); +__pa (pgd), flags, old); } static bool domain_need_iotlb_sync_map(struct dmar_domain *domain,
drivers/iommu/intel/iommu.h +3 -4
@@ -1255,10 +1255,9 @@ domain_add_dev_pasid(struct iommu_domain *domain,void domain_remove_dev_pasid (struct iommu_domain *domain,struct device *dev,ioasid_t pasid); -int __domain_setup_first_level (struct intel_iommu *iommu, -struct device *dev,ioasid_t pasid, -u16 did,pgd_t *pgd,int flags, -struct iommu_domain *old); +int __domain_setup_first_level (struct intel_iommu *iommu,struct device *dev, +ioasid_t pasid,u16 did,phys_addr_t fsptptr, +int flags,struct iommu_domain *old);int dmar_ir_support (void );
drivers/iommu/intel/pasid.c +9 -8
@@ -348,14 +348,15 @@ static void intel_pasid_flush_present(struct intel_iommu *iommu,*/ static void pasid_pte_config_first_level (struct intel_iommu *iommu,struct pasid_entry *pte, -pgd_t *pgd,u16 did,int flags) +phys_addr_t fsptptr,u16 did, +int flags) {lockdep_assert_held (&iommu->lock);pasid_clear_entry (pte);/* Setup the first level page table pointer: */ -pasid_set_flptr (pte, (u64 )__pa (pgd)); +pasid_set_flptr (pte, fsptptr);if (flags &PASID_FLAG_FL5LP )pasid_set_flpm (pte,1 );
@@ -372,9 +373,9 @@ static void pasid_pte_config_first_level(struct intel_iommu *iommu,pasid_set_present (pte); } -int intel_pasid_setup_first_level (struct intel_iommu *iommu, -struct device *dev,pgd_t *pgd, -u32 pasid,u16 did,int flags) +int intel_pasid_setup_first_level (struct intel_iommu *iommu,struct device *dev, +phys_addr_t fsptptr,u32 pasid,u16 did, +int flags) {struct pasid_entry *pte;
@@ -402,7 +403,7 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu,return -EBUSY ; } -pasid_pte_config_first_level (iommu, pte, pgd, did, flags); +pasid_pte_config_first_level (iommu, pte, fsptptr, did, flags);spin_unlock (&iommu->lock);
@@ -412,7 +413,7 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu, }int intel_pasid_replace_first_level (struct intel_iommu *iommu, -struct device *dev,pgd_t *pgd, +struct device *dev,phys_addr_t fsptptr,u32 pasid,u16 did,u16 old_did,int flags) {
@@ -430,7 +431,7 @@ int intel_pasid_replace_first_level(struct intel_iommu *iommu,return -EINVAL ; } -pasid_pte_config_first_level (iommu, &new_pte, pgd, did, flags); +pasid_pte_config_first_level (iommu, &new_pte, fsptptr, did, flags);spin_lock (&iommu->lock); pte =intel_pasid_get_entry (dev, pasid);
drivers/iommu/intel/pasid.h +5 -6
@@ -288,9 +288,9 @@ extern unsigned int intel_pasid_max_id;int intel_pasid_alloc_table (struct device *dev);void intel_pasid_free_table (struct device *dev);struct pasid_table *intel_pasid_get_table (struct device *dev); -int intel_pasid_setup_first_level (struct intel_iommu *iommu, -struct device *dev,pgd_t *pgd, -u32 pasid,u16 did,int flags); +int intel_pasid_setup_first_level (struct intel_iommu *iommu,struct device *dev, +phys_addr_t fsptptr,u32 pasid,u16 did, +int flags);int intel_pasid_setup_second_level (struct intel_iommu *iommu,struct dmar_domain *domain,struct device *dev,u32 pasid);
@@ -302,9 +302,8 @@ int intel_pasid_setup_pass_through(struct intel_iommu *iommu,int intel_pasid_setup_nested (struct intel_iommu *iommu,struct device *dev,u32 pasid,struct dmar_domain *domain);int intel_pasid_replace_first_level (struct intel_iommu *iommu, -struct device *dev,pgd_t *pgd, -u32 pasid,u16 did,u16 old_did, -int flags); +struct device *dev,phys_addr_t fsptptr, +u32 pasid,u16 did,u16 old_did,int flags); int intel_pasid_replace_second_level(struct intel_iommu *iommu, struct dmar_domain *domain, struct device *dev, u16 old_did,
drivers/iommu/intel/svm.c +1 -1
@@ -171,7 +171,7 @@ static int intel_svm_set_dev_pasid(struct iommu_domain *domain,/* Setup the pasid table: */ sflags =cpu_feature_enabled (X86_FEATURE_LA57 ) ?PASID_FLAG_FL5LP :0 ; ret =__domain_setup_first_level (iommu, dev, pasid, -FLPT_DEFAULT_DID , mm->pgd, +FLPT_DEFAULT_DID ,__pa (mm->pgd), sflags, old);if (ret) goto out_unwind_iopf;
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 4/11 iommu/vt-d: Fold domain_exit() into intel_iommu_domain_free()
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
It has only one caller, no need for two functions.
Correct the WARN_ON() error handling to leak the entire page table if the HW is still referencing it so we don't UAF during WARN_ON recovery.
--- drivers/iommu/intel/iommu.c | 38 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 20 deletions(-)
drivers/iommu/intel/iommu.c +18 -20
@@ -1396,23 +1396,6 @@ void domain_detach_iommu(struct dmar_domain *domain, struct intel_iommu *iommu) } } -static void domain_exit (struct dmar_domain *domain) -{ -if (domain->pgd) { -struct iommu_pages_list freelist = -IOMMU_PAGES_LIST_INIT (freelist); - -domain_unmap (domain,0 ,DOMAIN_MAX_PFN (domain->gaw), &freelist); -iommu_put_pages_list (&freelist); - } - -if (WARN_ON (!list_empty (&domain->devices))) -return ; - -kfree (domain->qi_batch); -kfree (domain); -} -/* * For kdump cases, old valid entries may be cached due to the * in-flight DMA and copied pgtable, but there is no unmapping
@@ -3406,9 +3389,24 @@ static void intel_iommu_domain_free(struct iommu_domain *domain) {struct dmar_domain *dmar_domain =to_dmar_domain (domain); -WARN_ON (dmar_domain->nested_parent && - !list_empty (&dmar_domain->s1_domains)); -domain_exit (dmar_domain); +if (WARN_ON (dmar_domain->nested_parent && + !list_empty (&dmar_domain->s1_domains))) +return ; + +if (WARN_ON (!list_empty (&dmar_domain->devices))) +return ; + +if (dmar_domain->pgd) { +struct iommu_pages_list freelist = +IOMMU_PAGES_LIST_INIT (freelist); + +domain_unmap (dmar_domain,0 ,DOMAIN_MAX_PFN (dmar_domain->gaw), + &freelist); +iommu_put_pages_list (&freelist); + } + +kfree (dmar_domain->qi_batch); +kfree (dmar_domain); }int paging_domain_compatible (struct iommu_domain *domain,struct device *dev)
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 5/11 iommu/vt-d: Do not wipe out the page table NID when devices detach
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
The NID is used to control which NUMA node memory for the page table is allocated it from. It should be a permanent property of the page table when it was allocated and not change during attach/detach of devices.
--- drivers/iommu/intel/iommu.c | 1 - 1 file changed, 1 deletion(-)
drivers/iommu/intel/iommu.c +0 -1
@@ -1391,7 +1391,6 @@ void domain_detach_iommu(struct dmar_domain *domain, struct intel_iommu *iommu)if (--info->refcnt ==0 ) {ida_free (&iommu->domain_ida, info->did);xa_erase (&domain->iommu_array, iommu->seq_id); - domain->nid =NUMA_NO_NODE ;kfree (info); } }
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 6/11 iommu/vt-d: Split intel_iommu_domain_alloc_paging_flags()
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
Create stage specific functions that check the stage specific conditions if each stage can be supported.
Have intel_iommu_domain_alloc_paging_flags() call both stages in sequence until one does not return EOPNOTSUPP and prefer to use the first stage if available and suitable for the requested flags.
Move second stage only operations like nested_parent and dirty_tracking into the second stage function for clarity.
Move initialization of the iommu_domain members into paging_domain_alloc().
Drop initialization of domain->owner as the callers all do it.
--- drivers/iommu/intel/iommu.c | 98 +++++++++++++++++++++---------------- 1 file changed, 57 insertions(+), 41 deletions(-)
drivers/iommu/intel/iommu.c +57 -41
@@ -3281,10 +3281,15 @@ static struct dmar_domain *paging_domain_alloc(struct device *dev, bool first_stspin_lock_init (&domain->lock);spin_lock_init (&domain->cache_lock);xa_init (&domain->iommu_array); +INIT_LIST_HEAD (&domain->s1_domains); +spin_lock_init (&domain->s1_lock); domain->nid =dev_to_node (dev); domain->use_first_level = first_stage; + domain->domain.type =IOMMU_DOMAIN_UNMANAGED ; + domain->domain.ops = intel_iommu_ops.default_domain_ops; +/* calculate the address width */ addr_width =agaw_to_width (iommu->agaw);if (addr_width >cap_mgaw (iommu->cap))
@@ -3326,62 +3331,73 @@ static struct dmar_domain *paging_domain_alloc(struct device *dev, bool first_st }static struct iommu_domain * -intel_iommu_domain_alloc_paging_flags (struct device *dev,u32 flags, -const struct iommu_user_data *user_data) +intel_iommu_domain_alloc_first_stage (struct device *dev, +struct intel_iommu *iommu,u32 flags) +{ +struct dmar_domain *dmar_domain; + +if (flags & ~IOMMU_HWPT_ALLOC_PASID ) +return ERR_PTR (-EOPNOTSUPP ); + +/* Only SL is available in legacy mode */ +if (!sm_supported (iommu) || !ecap_flts (iommu->ecap)) +return ERR_PTR (-EOPNOTSUPP ); + + dmar_domain =paging_domain_alloc (dev, true); +if (IS_ERR (dmar_domain)) +return ERR_CAST (dmar_domain); +return &dmar_domain->domain; +} + +static struct iommu_domain * +intel_iommu_domain_alloc_second_stage (struct device *dev, +struct intel_iommu *iommu,u32 flags) { -struct device_domain_info *info =dev_iommu_priv_get (dev); -bool dirty_tracking = flags &IOMMU_HWPT_ALLOC_DIRTY_TRACKING ; -bool nested_parent = flags &IOMMU_HWPT_ALLOC_NEST_PARENT ; -struct intel_iommu *iommu = info->iommu;struct dmar_domain *dmar_domain; -struct iommu_domain *domain; -bool first_stage;if (flags & (~(IOMMU_HWPT_ALLOC_NEST_PARENT |IOMMU_HWPT_ALLOC_DIRTY_TRACKING |IOMMU_HWPT_ALLOC_PASID )))return ERR_PTR (-EOPNOTSUPP ); -if (nested_parent && !nested_supported (iommu)) -return ERR_PTR (-EOPNOTSUPP ); -if (user_data || (dirty_tracking && !ssads_supported (iommu))) + +if (((flags &IOMMU_HWPT_ALLOC_NEST_PARENT ) && + !nested_supported (iommu)) || + ((flags &IOMMU_HWPT_ALLOC_DIRTY_TRACKING ) && + !ssads_supported (iommu)))return ERR_PTR (-EOPNOTSUPP ); -/* -* Always allocate the guest compatible page table unless -* IOMMU_HWPT_ALLOC_NEST_PARENT or IOMMU_HWPT_ALLOC_DIRTY_TRACKING -* is specified. -*/ -if (nested_parent || dirty_tracking) { -if (!sm_supported (iommu) || !ecap_slts (iommu->ecap)) -return ERR_PTR (-EOPNOTSUPP ); - first_stage = false; - }else { - first_stage =first_level_by_default (iommu); - } +/* Legacy mode always supports second stage */ +if (sm_supported (iommu) && !ecap_slts (iommu->ecap)) +return ERR_PTR (-EOPNOTSUPP ); - dmar_domain =paging_domain_alloc (dev, first_stage); + dmar_domain =paging_domain_alloc (dev, false);if (IS_ERR (dmar_domain))return ERR_CAST (dmar_domain); - domain = &dmar_domain->domain; - domain->type =IOMMU_DOMAIN_UNMANAGED ; - domain->owner = &intel_iommu_ops; - domain->ops = intel_iommu_ops.default_domain_ops; -if (nested_parent) { - dmar_domain->nested_parent = true; -INIT_LIST_HEAD (&dmar_domain->s1_domains); -spin_lock_init (&dmar_domain->s1_lock); - } + dmar_domain->nested_parent = flags &IOMMU_HWPT_ALLOC_NEST_PARENT ; -if (dirty_tracking) { -if (dmar_domain->use_first_level) { -iommu_domain_free (domain); -return ERR_PTR (-EOPNOTSUPP ); - } - domain->dirty_ops = &intel_dirty_ops; - } +if (flags &IOMMU_HWPT_ALLOC_DIRTY_TRACKING ) + dmar_domain->domain.dirty_ops = &intel_dirty_ops; -return domain; +return &dmar_domain->domain; +} + +static struct iommu_domain * +intel_iommu_domain_alloc_paging_flags (struct device *dev,u32 flags, +const struct iommu_user_data *user_data) +{ +struct device_domain_info *info =dev_iommu_priv_get (dev); +struct intel_iommu *iommu = info->iommu; +struct iommu_domain *domain; + +if (user_data) +return ERR_PTR (-EOPNOTSUPP ); + +/* Prefer first stage if possible by default. */ + domain =intel_iommu_domain_alloc_first_stage (dev, iommu, flags); +if (domain !=ERR_PTR (-EOPNOTSUPP )) +return domain; +return intel_iommu_domain_alloc_second_stage (dev, iommu, flags); }static void intel_iommu_domain_free (struct iommu_domain *domain)
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 7/11 iommu/vt-d: Create unique domain ops for each stage
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
Use the domain ops pointer to tell what kind of domain it is instead of the internal use_first_level indication. This also protects against wrongly using a SVA/nested/IDENTITY/BLOCKED domain type in places they should not be.
The only remaining uses of use_first_level outside the paging domain are in paging_domain_compatible() and intel_iommu_enforce_cache_coherency().
Thus, remove the useless sets of use_first_level in intel_svm_domain_alloc() and intel_iommu_domain_alloc_nested(). None of the unique ops for these domain types ever reference it on their call chains.
Add a WARN_ON() check in domain_context_mapping_one() as it only works with second stage.
This is preparation for iommupt which will have different ops for each of the stages.
--- drivers/iommu/intel/cache.c | 5 +-- drivers/iommu/intel/iommu.c | 60 +++++++++++++++++++++++++----------- drivers/iommu/intel/iommu.h | 12 ++++++++ drivers/iommu/intel/nested.c | 4 +-- drivers/iommu/intel/svm.c | 1 - 5 files changed, 58 insertions(+), 24 deletions(-)
drivers/iommu/intel/cache.c +3 -2
@@ -370,7 +370,7 @@ static void cache_tag_flush_iotlb(struct dmar_domain *domain, struct cache_tag *struct intel_iommu *iommu = tag->iommu;u64 type =DMA_TLB_PSI_FLUSH ; -if (domain->use_first_level) { +if (intel_domain_is_fs_paging (domain)) {qi_batch_add_piotlb (iommu, tag->domain_id, tag->pasid, addr, pages, ih, domain->qi_batch);return ;
@@ -545,7 +545,8 @@ void cache_tag_flush_range_np(struct dmar_domain *domain, unsigned long start,qi_batch_flush_descs (iommu, domain->qi_batch); iommu = tag->iommu; -if (!cap_caching_mode (iommu->cap) || domain->use_first_level) { +if (!cap_caching_mode (iommu->cap) || +intel_domain_is_fs_paging (domain)) {iommu_flush_write_buffer (iommu);continue ; }
drivers/iommu/intel/iommu.c +42 -18
122 lines not on this page · raw message
drivers/iommu/intel/iommu.h +12 -0
@@ -1380,6 +1380,18 @@ struct context_entry *iommu_context_addr(struct intel_iommu *iommu, u8 bus, u8 devfn, int alloc);extern const struct iommu_ops intel_iommu_ops; +extern const struct iommu_domain_ops intel_fs_paging_domain_ops; +extern const struct iommu_domain_ops intel_ss_paging_domain_ops; + +static inline bool intel_domain_is_fs_paging (struct dmar_domain *domain) +{ +return domain->domain.ops == &intel_fs_paging_domain_ops; +} + +static inline bool intel_domain_is_ss_paging (struct dmar_domain *domain) +{ +return domain->domain.ops == &intel_ss_paging_domain_ops; +}#ifdef CONFIG_INTEL_IOMMU extern int intel_iommu_sm;
drivers/iommu/intel/nested.c +1 -3
@@ -216,8 +216,7 @@ intel_iommu_domain_alloc_nested(struct device *dev, struct iommu_domain *parent,/* Must be nested domain */ if (user_data->type !=IOMMU_HWPT_DATA_VTD_S1 )return ERR_PTR (-EOPNOTSUPP ); -if (parent->ops != intel_iommu_ops.default_domain_ops || - !s2_domain->nested_parent) +if (!intel_domain_is_ss_paging (s2_domain) || !s2_domain->nested_parent)return ERR_PTR (-EINVAL ); ret = iommu_copy_struct_from_user(&vtd, user_data,
@@ -229,7 +228,6 @@ intel_iommu_domain_alloc_nested(struct device *dev, struct iommu_domain *parent,if (!domain)return ERR_PTR (-ENOMEM ); - domain->use_first_level = true; domain->s2_domain = s2_domain; domain->s1_cfg = vtd; domain->domain.ops = &intel_nested_domain_ops;
drivers/iommu/intel/svm.c +0 -1
@@ -214,7 +214,6 @@ struct iommu_domain *intel_svm_domain_alloc(struct device *dev,return ERR_PTR (-ENOMEM ); domain->domain.ops = &intel_svm_domain_ops; - domain->use_first_level = true;INIT_LIST_HEAD (&domain->dev_pasids);INIT_LIST_HEAD (&domain->cache_tags);spin_lock_init (&domain->cache_lock);
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 8/11 iommu/vt-d: Split intel_iommu_enforce_cache_coherency()
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
First Stage and Second Stage have very different ways to deny no-snoop. The first stage uses the PGSNP bit which is global per-PASID so enabling requires loading new PASID entries for all the attached devices.
Second stage uses a bit per PTE, so enabling just requires telling future maps to set the bit.
Since we now have two domain ops we can have two functions that can directly code their required actions instead of a bunch of logic dancing around use_first_level.
Combine domain_set_force_snooping() into the new functions since they are the only caller.
--- drivers/iommu/intel/iommu.c | 47 +++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 25 deletions(-)
drivers/iommu/intel/iommu.c +22 -25
@@ -3643,44 +3643,41 @@ static bool domain_support_force_snooping(struct dmar_domain *domain)return support; } -static void domain_set_force_snooping (struct dmar_domain *domain) +static bool intel_iommu_enforce_cache_coherency_fs (struct iommu_domain *domain) { +struct dmar_domain *dmar_domain =to_dmar_domain (domain);struct device_domain_info *info; -assert_spin_locked (&domain->lock); -/* -* Second level page table supports per-PTE snoop control. The -* iommu_map() interface will handle this by setting SNP bit. -*/ -if (!domain->use_first_level) { - domain->set_pte_snp = true; -return ; - } +guard (spinlock_irqsave)(&dmar_domain->lock); -list_for_each_entry (info, &domain->devices, link) +if (dmar_domain->force_snooping) +return true; + +if (!domain_support_force_snooping (dmar_domain)) +return false; + + dmar_domain->force_snooping = true; +list_for_each_entry (info, &dmar_domain->devices, link)intel_pasid_setup_page_snoop_control (info->iommu, info->dev,IOMMU_NO_PASID ); +return true; } -static bool intel_iommu_enforce_cache_coherency (struct iommu_domain *domain) +static bool intel_iommu_enforce_cache_coherency_ss (struct iommu_domain *domain) {struct dmar_domain *dmar_domain =to_dmar_domain (domain); -unsigned long flags; -if (dmar_domain->force_snooping) -return true; - -spin_lock_irqsave (&dmar_domain->lock, flags); +guard (spinlock_irqsave)(&dmar_domain->lock);if (!domain_support_force_snooping (dmar_domain) || - (!dmar_domain->use_first_level && dmar_domain->has_mappings)) { -spin_unlock_irqrestore (&dmar_domain->lock, flags); + dmar_domain->has_mappings)return false; - } -domain_set_force_snooping (dmar_domain); +/* +* Second level page table supports per-PTE snoop control. The +* iommu_map() interface will handle this by setting SNP bit. +*/ + dmar_domain->set_pte_snp = true; dmar_domain->force_snooping = true; -spin_unlock_irqrestore (&dmar_domain->lock, flags); -return true; }
@@ -4407,7 +4404,7 @@ const struct iommu_domain_ops intel_fs_paging_domain_ops = { .iotlb_sync = intel_iommu_tlb_sync, .iova_to_phys = intel_iommu_iova_to_phys, .free = intel_iommu_domain_free, - .enforce_cache_coherency = intel_iommu_enforce_cache_coherency, + .enforce_cache_coherency = intel_iommu_enforce_cache_coherency_fs, };const struct iommu_domain_ops intel_ss_paging_domain_ops = {
@@ -4420,7 +4417,7 @@ const struct iommu_domain_ops intel_ss_paging_domain_ops = { .iotlb_sync = intel_iommu_tlb_sync, .iova_to_phys = intel_iommu_iova_to_phys, .free = intel_iommu_domain_free, - .enforce_cache_coherency = intel_iommu_enforce_cache_coherency, + .enforce_cache_coherency = intel_iommu_enforce_cache_coherency_ss, };const struct iommu_ops intel_iommu_ops = {
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 9/11 iommu/vt-d: Split paging_domain_compatible()
In reply to Lu BaoluFrom: Jason Gunthorpe <[email protected]>
Make First/Second stage specific functions that follow the same pattern in intel_iommu_domain_alloc_first/second_stage() for computing EOPNOTSUPP. This makes the code easier to understand as if we couldn't create a domain with the parameters for this IOMMU instance then we certainly are not compatible with it.
Check superpage support directly against the per-stage cap bits and the pgsize_bitmap.
Add a note that the force_snooping is read without locking. The locking needs to cover the compatible check and the add of the device to the list.
--- drivers/iommu/intel/iommu.c | 66 ++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 12 deletions(-)
drivers/iommu/intel/iommu.c +54 -12
@@ -3431,33 +3431,75 @@ static void intel_iommu_domain_free(struct iommu_domain *domain)kfree (dmar_domain); } +static int paging_domain_compatible_first_stage (struct dmar_domain *dmar_domain, +struct intel_iommu *iommu) +{ +if (WARN_ON (dmar_domain->domain.dirty_ops || + dmar_domain->nested_parent)) +return -EINVAL ; + +/* Only SL is available in legacy mode */ +if (!sm_supported (iommu) || !ecap_flts (iommu->ecap)) +return -EINVAL ; + +/* Same page size support */ +if (!cap_fl1gp_support (iommu->cap) && + (dmar_domain->domain.pgsize_bitmap &SZ_1G )) +return -EINVAL ; +return 0 ; +} + +static int +paging_domain_compatible_second_stage (struct dmar_domain *dmar_domain, +struct intel_iommu *iommu) +{ +unsigned int sslps =cap_super_page_val (iommu->cap); + +if (dmar_domain->domain.dirty_ops && !ssads_supported (iommu)) +return -EINVAL ; +if (dmar_domain->nested_parent && !nested_supported (iommu)) +return -EINVAL ; + +/* Legacy mode always supports second stage */ +if (sm_supported (iommu) && !ecap_slts (iommu->ecap)) +return -EINVAL ; + +/* Same page size support */ +if (!(sslps &BIT (0 )) && (dmar_domain->domain.pgsize_bitmap &SZ_2M )) +return -EINVAL ; +if (!(sslps &BIT (1 )) && (dmar_domain->domain.pgsize_bitmap &SZ_1G )) +return -EINVAL ; +return 0 ; +} +int paging_domain_compatible (struct iommu_domain *domain,struct device *dev) {struct device_domain_info *info =dev_iommu_priv_get (dev);struct dmar_domain *dmar_domain =to_dmar_domain (domain);struct intel_iommu *iommu = info->iommu; +int ret = -EINVAL ;int addr_width; -if (WARN_ON_ONCE (!(domain->type & __IOMMU_DOMAIN_PAGING))) -return -EPERM ; +if (intel_domain_is_fs_paging (dmar_domain)) + ret =paging_domain_compatible_first_stage (dmar_domain, iommu); +else if (intel_domain_is_ss_paging (dmar_domain)) + ret =paging_domain_compatible_second_stage (dmar_domain, iommu); +else if (WARN_ON (true)) + ret = -EINVAL ; +if (ret) +return ret; +/* +* FIXME this is locked wrong, it needs to be under the +* dmar_domain->lock +*/ if (dmar_domain->force_snooping && !ecap_sc_support (iommu->ecap))return -EINVAL ; -if (domain->dirty_ops && !ssads_supported (iommu)) -return -EINVAL ; -if (dmar_domain->iommu_coherency !=iommu_paging_structure_coherency (iommu))return -EINVAL ; -if (dmar_domain->iommu_superpage != -iommu_superpage_capability (iommu, dmar_domain->use_first_level)) -return -EINVAL ; - -if (dmar_domain->use_first_level && - (!sm_supported (iommu) || !ecap_flts (iommu->ecap))) -return -EINVAL ;/* check if this iommu agaw is sufficient for max mapped address */ addr_width =agaw_to_width (iommu->agaw);
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 10/11 iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all
In reply to Lu BaoluFrom: Ethan Milon <[email protected]>
The function cache_tag_flush_all() was originally implemented with incorrect device TLB invalidation logic that does not handle PASID, in commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
This causes regressions where full address space TLB invalidations occur with a PASID attached, such as during transparent hugepage unmapping in SVA configurations or when calling iommu_flush_iotlb_all(). In these cases, the device receives a TLB invalidation that lacks PASID.
This incorrect logic was later extracted into cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
Refactor IOTLB and Dev-IOTLB flush for batching")
The fix replaces the call to cache_tag_flush_devtlb_all() with cache_tag_flush_devtlb_psi(), which properly handles PASID.
--- drivers/iommu/intel/cache.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
drivers/iommu/intel/cache.c +1 -17
@@ -422,22 +422,6 @@ static void cache_tag_flush_devtlb_psi(struct dmar_domain *domain, struct cache_ domain->qi_batch); } -static void cache_tag_flush_devtlb_all (struct dmar_domain *domain,struct cache_tag *tag) -{ -struct intel_iommu *iommu = tag->iommu; -struct device_domain_info *info; -u16 sid; - - info =dev_iommu_priv_get (tag->dev); - sid =PCI_DEVID (info->bus, info->devfn); - -qi_batch_add_dev_iotlb (iommu, sid, info->pfsid, info->ats_qdep,0 , -MAX_AGAW_PFN_WIDTH , domain->qi_batch); -if (info->dtlb_extra_inval) -qi_batch_add_dev_iotlb (iommu, sid, info->pfsid, info->ats_qdep,0 , -MAX_AGAW_PFN_WIDTH , domain->qi_batch); -} -/* * Invalidates a range of IOVA from @start (inclusive) to @end (inclusive) * when the memory mappings in the target domain have been modified.
@@ -508,7 +492,7 @@ void cache_tag_flush_all(struct dmar_domain *domain)break ;case CACHE_TAG_DEVTLB :case CACHE_TAG_NESTING_DEVTLB : -cache_tag_flush_devtlb_all (domain, tag); +cache_tag_flush_devtlb_psi (domain, tag,0 ,MAX_AGAW_PFN_WIDTH );break ; }
Signature · 1 line
2.43.0
Lu Baolu
maintainerPATCH 11/11 iommu/vt-d: Deduplicate cache_tag_flush_all by reusing flush_range
In reply to Lu BaoluFrom: Ethan Milon <[email protected]>
The logic in cache_tag_flush_all() to iterate over cache tags and issue TLB invalidations is largely duplicated in cache_tag_flush_range(), with the only difference being the range parameters.
Extend cache_tag_flush_range() to handle a full address space flush when called with start = 0 and end = ULONG_MAX. This allows cache_tag_flush_all() to simply delegate to cache_tag_flush_range()
--- drivers/iommu/intel/cache.c | 34 ++++++++-------------------------- drivers/iommu/intel/trace.h | 5 ----- 2 files changed, 8 insertions(+), 31 deletions(-)
drivers/iommu/intel/cache.c +8 -26
@@ -434,7 +434,13 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,struct cache_tag *tag;unsigned long flags; - addr =calculate_psi_aligned_address (start, end, &pages, &mask); +if (start ==0 && end ==ULONG_MAX ) { + addr =0 ; + pages =-1 ; + mask =MAX_AGAW_PFN_WIDTH ; + }else { + addr =calculate_psi_aligned_address (start, end, &pages, &mask); + }spin_lock_irqsave (&domain->cache_lock, flags);list_for_each_entry (tag, &domain->cache_tags, node) {
@@ -475,31 +481,7 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,*/ void cache_tag_flush_all (struct dmar_domain *domain) { -struct intel_iommu *iommu =NULL ; -struct cache_tag *tag; -unsigned long flags; - -spin_lock_irqsave (&domain->cache_lock, flags); -list_for_each_entry (tag, &domain->cache_tags, node) { -if (iommu && iommu != tag->iommu) -qi_batch_flush_descs (iommu, domain->qi_batch); - iommu = tag->iommu; - -switch (tag->type) { -case CACHE_TAG_IOTLB : -case CACHE_TAG_NESTING_IOTLB : -cache_tag_flush_iotlb (domain, tag,0 ,-1 ,0 ,0 ); -break ; -case CACHE_TAG_DEVTLB : -case CACHE_TAG_NESTING_DEVTLB : -cache_tag_flush_devtlb_psi (domain, tag,0 ,MAX_AGAW_PFN_WIDTH ); -break ; - } - -trace_cache_tag_flush_all (tag); - } -qi_batch_flush_descs (iommu, domain->qi_batch); -spin_unlock_irqrestore (&domain->cache_lock, flags); +cache_tag_flush_range (domain,0 ,ULONG_MAX ,0 ); }/*
drivers/iommu/intel/trace.h +0 -5
@@ -130,11 +130,6 @@ DEFINE_EVENT(cache_tag_log, cache_tag_unassign,TP_ARGS (tag) ); -DEFINE_EVENT (cache_tag_log, cache_tag_flush_all, - TP_PROTO(struct cache_tag *tag), -TP_ARGS (tag) -); - DECLARE_EVENT_CLASS(cache_tag_flush, TP_PROTO(struct cache_tag *tag, unsigned long start, unsigned long end, unsigned long addr, unsigned long pages, unsigned long mask),
Signature · 1 line
2.43.0
Quoted 9 lines · Lu Baolu
The following changes have been queued for v6.17-rc1. They are about new features and code refactoring, including:
- Reorganize Intel VT-d to be ready for iommupt
- Optimizeiotlb_sync_mapfor non-caching/non-RWBF modes
- Fix missed PASID in dev TLB invalidation incache_tag_flush_all()
- Miscellaneous cleanups[...]
Applied to iommu (intel/vt-d), thanks!
[01/11] iommu/vt-d: Remove the CONFIG_X86 wrapping from iommu init hook
https://git.kernel.org/iommu/c/bd26cd9d815a [02/11] iommu/vt-d: Optimize iotlb_sync_map for non-caching/non-RWBF modes https://git.kernel.org/iommu/c/12724ce3fe1a [03/11] iommu/vt-d: Lift the __pa to domain_setup_first_level/intel_svm_set_dev_pasid() https://git.kernel.org/iommu/c/cd0d0e4e48d8 [04/11] iommu/vt-d: Fold domain_exit() into intel_iommu_domain_free() https://git.kernel.org/iommu/c/00939bebe51c [05/11] iommu/vt-d: Do not wipe out the page table NID when devices detach https://git.kernel.org/iommu/c/5c3687d5789c [06/11] iommu/vt-d: Split intel_iommu_domain_alloc_paging_flags() https://git.kernel.org/iommu/c/b9434ba97c44 [07/11] iommu/vt-d: Create unique domain ops for each stage https://git.kernel.org/iommu/c/b33125296b50 [08/11] iommu/vt-d: Split intel_iommu_enforce_cache_coherency() https://git.kernel.org/iommu/c/0fa6f0893466 [09/11] iommu/vt-d: Split paging_domain_compatible() https://git.kernel.org/iommu/c/85cfaacc9937 [10/11] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all https://git.kernel.org/iommu/c/3141153816bf [11/11] iommu/vt-d: Deduplicate cache_tag_flush_all by reusing flush_range https://git.kernel.org/iommu/c/e934464e098e
Cheers,
Signature · 5 lines
Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev