History

iommu: Remove iommu_sva_ops::mm_exit()

After binding a device to an mm, device drivers currently need to register a mm_exit handler. This function is called when the mm exits, to gracefully stop DMA targeting the address space and flush page faults to the IOMMU. This is deemed too complex for the MMU release() notifier, which may be triggered by any mmput() invocation, from about 120 callsites [1]. The upcoming SVA module has an example of such complexity: the I/O Page Fault handler would need to call mmput_async() instead of mmput() after handling an IOPF, to avoid triggering the release() notifier which would in turn drain the IOPF queue and lock up. Another concern is the DMA stop function taking too long, up to several minutes [2]. For some mmput() callers this may disturb other users. For example, if the OOM killer picks the mm bound to a device as the victim and that mm's memory is locked, if the release() takes too long, it might choose additional innocent victims to kill. To simplify the MMU release notifier, don't forward the notification to device drivers. Since they don't stop DMA on mm exit anymore, the PASID lifetime is extended: (1) The device driver calls bind(). A PASID is allocated. Here any DMA fault is handled by mm, and on error we don't print anything to dmesg. Userspace can easily trigger errors by issuing DMA on unmapped buffers. (2) exit_mmap(), for example the process took a SIGKILL. This step doesn't happen during normal operations. Remove the pgd from the PASID table, since the page tables are about to be freed. Invalidate the IOTLBs. Here the device may still perform DMA on the address space. Incoming transactions are aborted but faults aren't printed out. ATS Translation Requests return Successful Translation Completions with R=W=0. PRI Page Requests return with Invalid Request. (3) The device driver stops DMA, possibly following release of a fd, and calls unbind(). PASID table is cleared, IOTLB invalidated if necessary. The page fault queues are drained, and the PASID is freed. If DMA for that PASID is still running here, something went seriously wrong and errors should be reported. For now remove iommu_sva_ops entirely. We might need to re-introduce them at some point, for example to notify device drivers of unhandled IOPF. [1] https://lore.kernel.org/linux-iommu/20200306174239.GM31668@ziepe.ca/ [2] https://lore.kernel.org/linux-iommu/4d68da96-0ad5-b412-5987-2f7a6aa796c3@amd.com/

Jean-Philippe Brucker committed 6 years ago

Commit edcc40d1 parent fb015622 files · +0 −41Browse files

Showing the changes to include/linux (1 of 2 files). Show all

Unified · Split

include/linux/iommu.h+0−30
@@ -53,8 +53,6 @@ struct iommu_fault_event; · 52 unmodified lines
53 53
54typedef int (*iommu_fault_handler_t)(struct iommu_domain *,54typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
55 struct device *, unsigned long, int, void *);55 struct device *, unsigned long, int, void *);
56typedef int (*iommu_mm_exit_handler_t)(struct device *dev, struct iommu_sva *,
57 void *);
58typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault *, void *);56typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault *, void *);
59 57
60struct iommu_domain_geometry {58struct iommu_domain_geometry {
@@ -171,25 +169,6 @@ enum iommu_dev_features { · 110 unmodified lines
171 169
172#define IOMMU_PASID_INVALID (-1U)170#define IOMMU_PASID_INVALID (-1U)
173 171
174/**
175 * struct iommu_sva_ops - device driver callbacks for an SVA context
176 *
177 * @mm_exit: called when the mm is about to be torn down by exit_mmap. After
178 * @mm_exit returns, the device must not issue any more transaction
179 * with the PASID given as argument.
180 *
181 * The @mm_exit handler is allowed to sleep. Be careful about the
182 * locks taken in @mm_exit, because they might lead to deadlocks if
183 * they are also held when dropping references to the mm. Consider the
184 * following call chain:
185 * mutex_lock(A); mmput(mm) -> exit_mm() -> @mm_exit() -> mutex_lock(A)
186 * Using mmput_async() prevents this scenario.
187 *
188 */
189struct iommu_sva_ops {
190 iommu_mm_exit_handler_t mm_exit;
191};
192
193#ifdef CONFIG_IOMMU_API172#ifdef CONFIG_IOMMU_API
194 173
195/**174/**
@@ -616,7 +595,6 @@ struct iommu_fwspec { · 420 unmodified lines
616 */595 */
617struct iommu_sva {596struct iommu_sva {
618 struct device *dev;597 struct device *dev;
619 const struct iommu_sva_ops *ops;
620};598};
621 599
622int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,600int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
@@ -664,8 +642,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, · 41 unmodified lines
664 struct mm_struct *mm,642 struct mm_struct *mm,
665 void *drvdata);643 void *drvdata);
666void iommu_sva_unbind_device(struct iommu_sva *handle);644void iommu_sva_unbind_device(struct iommu_sva *handle);
667int iommu_sva_set_ops(struct iommu_sva *handle,
668 const struct iommu_sva_ops *ops);
669int iommu_sva_get_pasid(struct iommu_sva *handle);645int iommu_sva_get_pasid(struct iommu_sva *handle);
670 646
671#else /* CONFIG_IOMMU_API */647#else /* CONFIG_IOMMU_API */
@@ -1069,12 +1045,6 @@ static inline void iommu_sva_unbind_device(struct iommu_sva *handle) · 397 unmodified lines
1069{1045{
1070}1046}
1071 1047
1072static inline int iommu_sva_set_ops(struct iommu_sva *handle,
1073 const struct iommu_sva_ops *ops)
1074{
1075 return -EINVAL;
1076}
1077
1078static inline int iommu_sva_get_pasid(struct iommu_sva *handle)1048static inline int iommu_sva_get_pasid(struct iommu_sva *handle)
1079{1049{
1080 return IOMMU_PASID_INVALID;1050 return IOMMU_PASID_INVALID;

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t