Hi, Christoph,
On 1/8/23 09:36, Christoph Hellwig wrote:
Exporting access_remote_vm just seems like a horrible idea.
If a driver needs to access a different VM from a completion path
in some thread or workqueue (which I assume this does, if not please
explain the use case), it should use kthread_use_mm to associate the
mm struct and then just use all the normal uaccess helpers.
To trigger fault by IDXD device, user app frees the fault_addr pages.
After kthread_use_mm(), the IDXD driver cannot directly call
copy_to_user() to copy data to fault_addr because
the pages are freed by the app. If the IDXD driver
tries to copy to the app's fault_addr, it needs to get all the pages,
kmap, copy_to_user_page() etc, basically majority of the
__access_remote_vm() code. Without exporting access_remote_vm(),
the driver has to re-implement the function for the copy to work
in IDXD driver.
Maybe a simple exported IOMMU wrapper in iommu-sva.c can help here?
CONFIG_IOMMU_SVA is bool and needs to be set for Event Log to work.
So the wrapper is exported and can be called the IDXD driver or any
driver that is on top of IOMMU SVA.
No need to export access_remote_vm() any more.
Plus with this wrapper, no need to export iommu_sva_find()
(in an earlier patch). So the code will be more clean and concise.
Is this OK for you?
struct mm_struct
*iommu_access_remote_vm(ioasid_t pasid, unsigned long addr,
void *buf, int len, unsigned int gup_flags, int *copied)
{
struct mm_struct *mm;
mm = iommu_sva_find(pasid);
if (!IS_ERR_OR_NULL(mm)) {
/* A reference on @mm has been held. */
*copied = access_remote_vm(mm, addr, buf, len, gup_flags);
}
return mm;
}
EXPORT_SYMBOL_GPL(iommu_access_remote_vm);
Thanks.
-Fenghua
