With the N:1 drm_gpusvm_pages layout, one CPU range mirrored on several
drm_devices, the caller had to invoke get_pages() once per device and
repeat the HMM fault every time.
Make get_pages() take a contiguous array of drm_gpusvm_pages plus a
count: fault once, then DMA map each instance by
drm_gpusvm_dma_map_pages() under a single read_retry gate. xe range and
userptr callers are updated.
Document the N:1 array usage in the Overview, showing how get_pages()
and drm_gpusvm_range_set_unmapped() take the whole array and its count
while the unmap and free paths stay per-instance.
Suggested-by: Matthew Brost <[email protected]>
Signed-off-by: Honglei Huang <[email protected]>
---
drivers/gpu/drm/drm_gpusvm.c | 141 ++++++++++++++++++++++++--------
drivers/gpu/drm/xe/xe_svm.c | 2 +-
drivers/gpu/drm/xe/xe_userptr.c | 2 +-
include/drm/drm_gpusvm.h | 1 +
4 files changed, 108 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 89c3061d8ef..810f801a9f7 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -80,6 +80,13 @@
* };
* };
*
+ * static struct drm_gpusvm_pages *
+ * driver_pages(struct driver_range *drange)
+ * {
+ * return drange->num_pages == 1 ? &drange->inline_pages :
+ * drange->pages;
+ * }
+ *
* In the N:1 case the driver allocates the pages array with a zeroing
* allocator (e.g. kcalloc(num_pages, ...)), initialises each entry with
* drm_gpusvm_init_pages(), and frees each entry with
@@ -89,6 +96,28 @@
* Each drm_gpusvm_pages must be zero-initialised and initialised with
* drm_gpusvm_init_pages(), called once per entry.
*
+ * The 1:1 examples below pass @num_pages == 1 and &drange->pages. In the
+ * N:1 case the driver instead passes the whole array and its count, so a
+ * single call faults the CPU range once and DMA maps it for every owning
+ * drm_device, e.g.:
+ *
+ * .. code-block:: c
+ *
+ * // GPU fault handler: one fault, one DMA mapping per device
+ * err = drm_gpusvm_get_pages(gpusvm, driver_pages(drange),
+ * drange->num_pages, gpusvm->mm,
+ * &range->notifier->notifier,
+ * drm_gpusvm_range_start(range),
+ * drm_gpusvm_range_end(range), &ctx);
+ *
+ * // Notifier callback: mark every instance unmapped in one call
+ * drm_gpusvm_range_set_unmapped(range, driver_pages(drange),
+ * drange->num_pages, mmu_range);
+ *
+ * The unmap and free paths stay per-instance: iterate @num_pages over
+ * driver_pages(drange) and call drm_gpusvm_unmap_pages() /
+ * drm_gpusvm_free_pages() for each entry.
+ *
* - Operations:
* Define the interface for driver-specific GPU SVM operations such as
* range allocation, notifier allocation, and invalidations.
@@ -232,7 +261,7 @@
* goto retry;
* }
*
- * err = drm_gpusvm_get_pages(gpusvm, &drange->pages,
+ * err = drm_gpusvm_get_pages(gpusvm, &drange->pages, 1,
* gpusvm->mm,
&range->notifier->notifier,
* drm_gpusvm_range_start(range),
* drm_gpusvm_range_end(range), &ctx);
@@ -1417,25 +1446,34 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid);
/**
* drm_gpusvm_pages_valid_unlocked() - GPU SVM pages valid unlocked
* @gpusvm: Pointer to the GPU SVM structure
- * @svm_pages: Pointer to the GPU SVM pages structure
+ * @svm_pages: Array of GPU SVM pages structures
+ * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages
*
- * This function determines if a GPU SVM pages are valid. Expected be called
+ * This function determines if every GPU SVM pages instance is valid, dropping
+ * the stale dma_addr array of any instance which is not. Expected be called
* without holding gpusvm->notifier_lock.
*
- * Return: True if GPU SVM pages are valid, False otherwise
+ * Return: True if all GPU SVM pages are valid, False otherwise
*/
static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
- struct drm_gpusvm_pages *svm_pages)
+ struct drm_gpusvm_pages *svm_pages,
+ unsigned int num_pages)
{
- bool pages_valid;
+ bool pages_valid = true;
+ unsigned int p;
- if (!svm_pages->dma_addr)
- return false;
+ for (p = 0; p < num_pages; ++p) {
+ if (!svm_pages[p].dma_addr)
+ return false;
+ }
drm_gpusvm_notifier_lock(gpusvm);
- pages_valid = drm_gpusvm_pages_valid(gpusvm, svm_pages);
- if (!pages_valid)
- __drm_gpusvm_free_pages(gpusvm, svm_pages);
+ for (p = 0; p < num_pages; ++p) {
+ if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
+ continue;
+ __drm_gpusvm_free_pages(gpusvm, &svm_pages[p]);
+ pages_valid = false;
+ }
drm_gpusvm_notifier_unlock(gpusvm);
return pages_valid;
@@ -1451,8 +1489,9 @@ static bool drm_gpusvm_pages_valid_unlocked(struct
drm_gpusvm *gpusvm,
* @dma_dir: DMA data direction for the mappings
*
* Map the faulted @pfns into @svm_pages for DMA access through its owning
- * drm_device. Must be called under the notifier lock. On failure this unwinds
- * the partial mapping of this instance before returning.
+ * drm_device. Must be called under the notifier lock and only for an instance
+ * without a live mapping. On failure this unwinds the partial mapping of this
+ * instance before returning.
*
* Return: 0 on success, negative error code on failure.
*/
@@ -1475,6 +1514,9 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm
*gpusvm,
lockdep_assert_held(&gpusvm->notifier_lock);
+ *state = (struct dma_iova_state){};
+ svm_pages->state_offset = 0;
+
flags.__flags = svm_pages->flags.__flags;
for (i = 0, j = 0; i < npages; ++j) {
@@ -1603,20 +1645,29 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm
*gpusvm,
/**
* drm_gpusvm_get_pages() - Get pages and populate GPU SVM pages struct
* @gpusvm: Pointer to the GPU SVM structure
- * @svm_pages: The SVM pages to populate. This will contain the dma-addresses
+ * @svm_pages: Array of SVM pages instances to populate with dma addresses
+ * @num_pages: Number of drm_gpusvm_pages instances in @svm_pages
* @mm: The mm corresponding to the CPU range
* @notifier: The corresponding notifier for the given CPU range
* @pages_start: Start CPU address for the pages
* @pages_end: End CPU address for the pages (exclusive)
* @ctx: GPU SVM context
*
- * This function gets and maps pages for CPU range and ensures they are
- * mapped for DMA access.
+ * This function gets and maps pages for a CPU range and ensures they are
+ * mapped for DMA access. The HMM fault for the CPU range is performed once,
+ * the DMA mapping by drm_gpusvm_dma_map_pages() is then done per instance,
+ * one per owning drm_device. The retry against notifier races is kept here
+ * in common code so drivers never open code it.
+ * The common 1:1 case passes @num_pages == 1.
+ *
+ * On error the instances mapped before the failing one stay mapped, so the
+ * caller must unmap and free every instance regardless of the return value.
*
* Return: 0 on success, negative error code on failure.
*/
int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct drm_gpusvm_pages *svm_pages,
+ unsigned int num_pages,
struct mm_struct *mm,
struct mmu_interval_notifier *notifier,
unsigned long pages_start, unsigned long pages_end,
@@ -1638,9 +1689,11 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
int err = 0;
enum dma_data_direction dma_dir = ctx->read_only ? DMA_TO_DEVICE :
DMA_BIDIRECTIONAL;
+ unsigned int p;
- if (!svm_pages->drm)
- return -EINVAL;
+ for (p = 0; p < num_pages; ++p)
+ if (!svm_pages[p].drm)
+ return -EINVAL;
retry:
remaining = timeout - jiffies;
@@ -1649,7 +1702,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
return -EBUSY;
hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
+
+ if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
goto set_seqno;
pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
@@ -1667,18 +1721,17 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
if (err)
goto err_free;
- if (!svm_pages->dma_addr) {
- svm_pages->dma_addr =
- kvzalloc_objs(*svm_pages->dma_addr, npages);
- if (!svm_pages->dma_addr) {
+ for (p = 0; p < num_pages; ++p) {
+ if (svm_pages[p].dma_addr)
+ continue;
+ svm_pages[p].dma_addr =
+ kvzalloc_objs(*svm_pages[p].dma_addr, npages);
+ if (!svm_pages[p].dma_addr) {
err = -ENOMEM;
goto err_free;
}
}
- svm_pages->state = (struct dma_iova_state){};
- svm_pages->state_offset = 0;
-
/*
* Perform all dma mappings under the notifier lock to not
* access freed pages. A notifier will either block on
@@ -1686,10 +1739,12 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
*/
drm_gpusvm_notifier_lock(gpusvm);
- if (svm_pages->flags.unmapped) {
- drm_gpusvm_notifier_unlock(gpusvm);
- err = -EFAULT;
- goto err_free;
+ for (p = 0; p < num_pages; ++p) {
+ if (svm_pages[p].flags.unmapped) {
+ drm_gpusvm_notifier_unlock(gpusvm);
+ err = -EFAULT;
+ goto err_free;
+ }
