On Tue, Dec 17, 2019 at 08:35:47PM +0000, Jason Gunthorpe wrote:
On Fri, Dec 13, 2019 at 01:56:08PM -0800, Niranjana Vishwanathapura wrote:
@@ -169,6 +170,11 @@ static int i915_range_fault(struct svm_notifier *sn,
return ret;
}
+ /* For dgfx, ensure the range is in device local memory only */
+ regions = i915_dmem_convert_pfn(vm->i915, &range);
+ if (!regions || (IS_DGFX(vm->i915) && (regions & REGION_SMEM)))
+ return -EINVAL;
+
This is not OK, as I said before, the driver cannot de-reference pfns
before doing the retry check, under lock.
Thanks.
Ok, will push it down and do it after validating the range.
+
+int i915_dmem_convert_pfn(struct drm_i915_private *dev_priv,
+ struct hmm_range *range)
+{
+ unsigned long i, npages;
+ int regions = 0;
+
+ npages = (range->end - range->start) >> PAGE_SHIFT;
+ for (i = 0; i < npages; ++i) {
+ struct i915_buddy_block *block;
+ struct intel_memory_region *mem;
+ struct page *page;
+ u64 addr;
+
+ page = hmm_device_entry_to_page(range, range->pfns[i]);
^^^^^^^^^^^^^^^^^^^^^^
For instance, that cannot be done on a speculatively loaded page.
This also looks like it suffers from the same bug as
Ok.
+ if (!page)
+ continue;
+
+ if (!(range->pfns[i] & range->flags[HMM_PFN_DEVICE_PRIVATE])) {
+ regions |= REGION_SMEM;
+ continue;
+ }
+
+ if (!i915_dmem_page(dev_priv, page)) {
+ WARN(1, "Some unknown device memory !\n");
Why is that a WARN? The user could put other device memory in the
address space. You need to 'segfault' the GPU execution if this happens.
OK, will return an error here if user is trying to bind here.
I agree, we need to segfault the GPU if it is GPU fault handling.
+ range->pfns[i] = 0;
+ continue;
+ }
+
+ regions |= REGION_LMEM;
+ block = page->zone_device_data;
+ mem = block->private;
+ addr = mem->region.start +
+ i915_buddy_block_offset(block);
+ addr += (page_to_pfn(page) - block->pfn_first) << PAGE_SHIFT;
+
+ range->pfns[i] &= ~range->flags[HMM_PFN_DEVICE_PRIVATE];
+ range->pfns[i] &= ((1UL << range->pfn_shift) - 1);
+ range->pfns[i] |= (addr >> PAGE_SHIFT) << range->pfn_shift;
This makes more sense as a direct manipulation of the sgl, not sure
why this routine is split out from the sgl builder?
Ok, yah, let me merge it with sgl building.
Thanks,
Niranjana
Jason
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
