From: Jason Gunthorpe <jgg@mellanox.com> To: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Cc: "kenneth.w.graunke@intel.com" <kenneth.w.graunke@intel.com>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "sanjay.k.kumar@intel.com" <sanjay.k.kumar@intel.com>, "sudeep.dutt@intel.com" <sudeep.dutt@intel.com>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "jason.ekstrand@intel.com" <jason.ekstrand@intel.com>, "dave.hansen@intel.com" <dave.hansen@intel.com>, "jglisse@redhat.com" <jglisse@redhat.com>, "jon.bloomfield@intel.com" <jon.bloomfield@intel.com>, "daniel.vetter@intel.com" <daniel.vetter@intel.com>, "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "ira.weiny@intel.com" <ira.weiny@intel.com> Subject: Re: [RFC v2 05/12] drm/i915/svm: Page table mirroring support Date: Fri, 20 Dec 2019 13:45:33 +0000 [thread overview] Message-ID: <20191220134529.GW16762@mellanox.com> (raw) In-Reply-To: <20191218224147.GB17413@nvishwa1-DESK.sc.intel.com> On Wed, Dec 18, 2019 at 02:41:47PM -0800, Niranjana Vishwanathapura wrote: > > > +static u32 i915_svm_build_sg(struct i915_address_space *vm, > > > + struct hmm_range *range, > > > + struct sg_table *st) > > > +{ > > > + struct scatterlist *sg; > > > + u32 sg_page_sizes = 0; > > > + u64 i, npages; > > > + > > > + sg = NULL; > > > + st->nents = 0; > > > + npages = (range->end - range->start) / PAGE_SIZE; > > > + > > > + /* > > > + * No need to dma map the host pages and later unmap it, as > > > + * GPU is not allowed to access it with SVM. > > > + * XXX: Need to dma map host pages for integrated graphics while > > > + * extending SVM support there. > > > + */ > > > + for (i = 0; i < npages; i++) { > > > + u64 addr = range->pfns[i] & ~((1UL << range->pfn_shift) - 1); > > > + > > > + if (sg && (addr == (sg_dma_address(sg) + sg->length))) { > > > + sg->length += PAGE_SIZE; > > > + sg_dma_len(sg) += PAGE_SIZE; > > > + continue; > > > + } > > > + > > > + if (sg) > > > + sg_page_sizes |= sg->length; > > > + > > > + sg = sg ? __sg_next(sg) : st->sgl; > > > + sg_dma_address(sg) = addr; > > > + sg_dma_len(sg) = PAGE_SIZE; > > > > This still can't be like this - assigning pfn to 'dma_address' is > > fundamentally wrong. > > > > Whatever explanation you had, this needs to be fixed up first before we get > > to this patch. > > > > The pfn is converted into a device address which goes into sg_dma_address. > Ok, let me think about what else we can do here. If you combine this with the other function and make it so only DEVICE_PRIVATE pages get converted toa dma_address with out dma_map, then that would make sense. > > > +static int > > > +i915_svm_invalidate_range_start(struct mmu_notifier *mn, > > > + const struct mmu_notifier_range *update) > > > +{ > > > + struct i915_svm *svm = container_of(mn, struct i915_svm, notifier); > > > + unsigned long length = update->end - update->start; > > > + > > > + DRM_DEBUG_DRIVER("start 0x%lx length 0x%lx\n", update->start, length); > > > + if (!mmu_notifier_range_blockable(update)) > > > + return -EAGAIN; > > > + > > > + i915_gem_vm_unbind_svm_buffer(svm->vm, update->start, length); > > > + return 0; > > > +} > > > > I still think you should strive for a better design than putting a > > notifier across the entire address space.. > > > > Yah, thought it could be later optimization. > If I think about it, it has be be a new user API to set the range, > or an intermediate data structure for tracking the bound ranges. > Will look into it. Well, there are lots of options. Like I said, implicit ODP uses a level of the device page table to attach the notifier. There are many performance trade offs here, it depends what works best for your work load I suppose. But usually the fault path is the fast thing, so I would think to avoid registering mmu_intervals on it and accept the higher probability of collisions. Jason _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@mellanox.com> To: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> Cc: "kenneth.w.graunke@intel.com" <kenneth.w.graunke@intel.com>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, "sanjay.k.kumar@intel.com" <sanjay.k.kumar@intel.com>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "jason.ekstrand@intel.com" <jason.ekstrand@intel.com>, "dave.hansen@intel.com" <dave.hansen@intel.com>, "jglisse@redhat.com" <jglisse@redhat.com>, "daniel.vetter@intel.com" <daniel.vetter@intel.com>, "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "ira.weiny@intel.com" <ira.weiny@intel.com> Subject: Re: [Intel-gfx] [RFC v2 05/12] drm/i915/svm: Page table mirroring support Date: Fri, 20 Dec 2019 13:45:33 +0000 [thread overview] Message-ID: <20191220134529.GW16762@mellanox.com> (raw) In-Reply-To: <20191218224147.GB17413@nvishwa1-DESK.sc.intel.com> On Wed, Dec 18, 2019 at 02:41:47PM -0800, Niranjana Vishwanathapura wrote: > > > +static u32 i915_svm_build_sg(struct i915_address_space *vm, > > > + struct hmm_range *range, > > > + struct sg_table *st) > > > +{ > > > + struct scatterlist *sg; > > > + u32 sg_page_sizes = 0; > > > + u64 i, npages; > > > + > > > + sg = NULL; > > > + st->nents = 0; > > > + npages = (range->end - range->start) / PAGE_SIZE; > > > + > > > + /* > > > + * No need to dma map the host pages and later unmap it, as > > > + * GPU is not allowed to access it with SVM. > > > + * XXX: Need to dma map host pages for integrated graphics while > > > + * extending SVM support there. > > > + */ > > > + for (i = 0; i < npages; i++) { > > > + u64 addr = range->pfns[i] & ~((1UL << range->pfn_shift) - 1); > > > + > > > + if (sg && (addr == (sg_dma_address(sg) + sg->length))) { > > > + sg->length += PAGE_SIZE; > > > + sg_dma_len(sg) += PAGE_SIZE; > > > + continue; > > > + } > > > + > > > + if (sg) > > > + sg_page_sizes |= sg->length; > > > + > > > + sg = sg ? __sg_next(sg) : st->sgl; > > > + sg_dma_address(sg) = addr; > > > + sg_dma_len(sg) = PAGE_SIZE; > > > > This still can't be like this - assigning pfn to 'dma_address' is > > fundamentally wrong. > > > > Whatever explanation you had, this needs to be fixed up first before we get > > to this patch. > > > > The pfn is converted into a device address which goes into sg_dma_address. > Ok, let me think about what else we can do here. If you combine this with the other function and make it so only DEVICE_PRIVATE pages get converted toa dma_address with out dma_map, then that would make sense. > > > +static int > > > +i915_svm_invalidate_range_start(struct mmu_notifier *mn, > > > + const struct mmu_notifier_range *update) > > > +{ > > > + struct i915_svm *svm = container_of(mn, struct i915_svm, notifier); > > > + unsigned long length = update->end - update->start; > > > + > > > + DRM_DEBUG_DRIVER("start 0x%lx length 0x%lx\n", update->start, length); > > > + if (!mmu_notifier_range_blockable(update)) > > > + return -EAGAIN; > > > + > > > + i915_gem_vm_unbind_svm_buffer(svm->vm, update->start, length); > > > + return 0; > > > +} > > > > I still think you should strive for a better design than putting a > > notifier across the entire address space.. > > > > Yah, thought it could be later optimization. > If I think about it, it has be be a new user API to set the range, > or an intermediate data structure for tracking the bound ranges. > Will look into it. Well, there are lots of options. Like I said, implicit ODP uses a level of the device page table to attach the notifier. There are many performance trade offs here, it depends what works best for your work load I suppose. But usually the fault path is the fast thing, so I would think to avoid registering mmu_intervals on it and accept the higher probability of collisions. Jason _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-12-23 8:11 UTC|newest] Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-13 21:56 [RFC v2 00/12] drm/i915/svm: Add SVM support Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 01/12] drm/i915/svm: Add SVM documentation Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 02/12] drm/i915/svm: Runtime (RT) allocator support Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 22:58 ` Jason Ekstrand 2019-12-13 22:58 ` Jason Ekstrand 2019-12-13 23:13 ` Niranjan Vishwanathapura 2019-12-13 23:13 ` Niranjan Vishwanathapura 2019-12-14 0:36 ` Jason Ekstrand 2019-12-14 0:36 ` Jason Ekstrand 2019-12-14 10:31 ` Chris Wilson 2019-12-14 10:31 ` Chris Wilson 2019-12-16 4:13 ` Niranjan Vishwanathapura 2019-12-16 4:13 ` Niranjan Vishwanathapura 2019-12-17 18:01 ` Jason Ekstrand 2019-12-17 18:01 ` Jason Ekstrand 2019-12-18 23:25 ` Niranjana Vishwanathapura 2019-12-18 23:25 ` Niranjana Vishwanathapura 2019-12-14 10:56 ` Chris Wilson 2019-12-14 10:56 ` Chris Wilson 2019-12-16 4:15 ` Niranjan Vishwanathapura 2019-12-16 4:15 ` Niranjan Vishwanathapura 2019-12-18 22:51 ` Niranjana Vishwanathapura 2019-12-18 22:51 ` Niranjana Vishwanathapura 2019-12-17 20:18 ` Jason Gunthorpe 2019-12-17 20:18 ` [Intel-gfx] " Jason Gunthorpe 2019-12-18 23:34 ` Niranjana Vishwanathapura 2019-12-18 23:34 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 03/12] drm/i915/svm: Implicitly migrate BOs upon CPU access Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-14 10:58 ` Chris Wilson 2019-12-14 10:58 ` Chris Wilson 2019-12-16 4:17 ` Niranjan Vishwanathapura 2019-12-16 4:17 ` Niranjan Vishwanathapura 2019-12-13 21:56 ` [RFC v2 04/12] drm/i915/svm: Page table update support for SVM Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 05/12] drm/i915/svm: Page table mirroring support Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-17 20:31 ` Jason Gunthorpe 2019-12-17 20:31 ` [Intel-gfx] " Jason Gunthorpe 2019-12-18 22:41 ` Niranjana Vishwanathapura 2019-12-18 22:41 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-20 13:45 ` Jason Gunthorpe [this message] 2019-12-20 13:45 ` Jason Gunthorpe 2019-12-22 19:54 ` Niranjana Vishwanathapura 2019-12-22 19:54 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 06/12] drm/i915/svm: Device memory support Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-17 20:35 ` Jason Gunthorpe 2019-12-17 20:35 ` [Intel-gfx] " Jason Gunthorpe 2019-12-18 22:15 ` Niranjana Vishwanathapura 2019-12-18 22:15 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 07/12] drm/i915/svm: Implicitly migrate pages upon CPU fault Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 08/12] drm/i915/svm: Page copy support during migration Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 09/12] drm/i915/svm: Add functions to blitter copy SVM buffers Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 10/12] drm/i915/svm: Use blitter copy for migration Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 11/12] drm/i915/svm: Add support to en/disable SVM Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-13 21:56 ` [RFC v2 12/12] drm/i915/svm: Add page table dump support Niranjana Vishwanathapura 2019-12-13 21:56 ` [Intel-gfx] " Niranjana Vishwanathapura 2019-12-14 1:32 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/svm: Add SVM support (rev2) Patchwork 2020-01-24 8:42 ` [RFC v2 00/12] drm/i915/svm: Add SVM support Niranjana Vishwanathapura 2020-01-24 8:42 ` [Intel-gfx] " Niranjana Vishwanathapura
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191220134529.GW16762@mellanox.com \ --to=jgg@mellanox.com \ --cc=dan.j.williams@intel.com \ --cc=daniel.vetter@intel.com \ --cc=dave.hansen@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=ira.weiny@intel.com \ --cc=jason.ekstrand@intel.com \ --cc=jglisse@redhat.com \ --cc=jon.bloomfield@intel.com \ --cc=kenneth.w.graunke@intel.com \ --cc=niranjana.vishwanathapura@intel.com \ --cc=sanjay.k.kumar@intel.com \ --cc=sudeep.dutt@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.