From: Ben Skeggs <skeggsb@gmail.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: linux-mm@kvack.org, ML nouveau <nouveau@lists.freedesktop.org>,
Ben Skeggs <bskeggs@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
Ralph Campbell <rcampbell@nvidia.com>,
linux-doc@vger.kernel.org, John Hubbard <jhubbard@nvidia.com>,
bsingharora@gmail.com, LKML <linux-kernel@vger.kernel.org>,
ML dri-devel <dri-devel@lists.freedesktop.org>,
Christoph Hellwig <hch@infradead.org>,
jglisse@redhat.com, willy@infradead.org,
Jason Gunthorpe <jgg@nvidia.com>
Subject: Re: [PATCH v8 8/8] nouveau/svm: Implement atomic SVM access
Date: Fri, 21 May 2021 14:04:08 +1000 [thread overview]
Message-ID: <CACAvsv6hKbmHMHmJSxMyij_4Y1v51Zo5NAfLtDyaQyiamtGOLQ@mail.gmail.com> (raw)
In-Reply-To: <20210407084238.20443-9-apopple@nvidia.com>
On Wed, 7 Apr 2021 at 18:43, Alistair Popple <apopple@nvidia.com> wrote:
>
> Some NVIDIA GPUs do not support direct atomic access to system memory
> via PCIe. Instead this must be emulated by granting the GPU exclusive
> access to the memory. This is achieved by replacing CPU page table
> entries with special swap entries that fault on userspace access.
>
> The driver then grants the GPU permission to update the page undergoing
> atomic access via the GPU page tables. When CPU access to the page is
> required a CPU fault is raised which calls into the device driver via
> MMU notifiers to revoke the atomic access. The original page table
> entries are then restored allowing CPU access to proceed.
>
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
The Nouveau bits at least look good to me.
For patches 7/8:
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>
>
> ---
>
> v7:
> * Removed magic values for fault access levels
> * Improved readability of fault comparison code
>
> v4:
> * Check that page table entries haven't changed before mapping on the
> device
> ---
> drivers/gpu/drm/nouveau/include/nvif/if000c.h | 1 +
> drivers/gpu/drm/nouveau/nouveau_svm.c | 126 ++++++++++++++++--
> drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 1 +
> .../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c | 6 +
> 4 files changed, 123 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvif/if000c.h b/drivers/gpu/drm/nouveau/include/nvif/if000c.h
> index d6dd40f21eed..9c7ff56831c5 100644
> --- a/drivers/gpu/drm/nouveau/include/nvif/if000c.h
> +++ b/drivers/gpu/drm/nouveau/include/nvif/if000c.h
> @@ -77,6 +77,7 @@ struct nvif_vmm_pfnmap_v0 {
> #define NVIF_VMM_PFNMAP_V0_APER 0x00000000000000f0ULL
> #define NVIF_VMM_PFNMAP_V0_HOST 0x0000000000000000ULL
> #define NVIF_VMM_PFNMAP_V0_VRAM 0x0000000000000010ULL
> +#define NVIF_VMM_PFNMAP_V0_A 0x0000000000000004ULL
> #define NVIF_VMM_PFNMAP_V0_W 0x0000000000000002ULL
> #define NVIF_VMM_PFNMAP_V0_V 0x0000000000000001ULL
> #define NVIF_VMM_PFNMAP_V0_NONE 0x0000000000000000ULL
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index a195e48c9aee..81526d65b4e2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -35,6 +35,7 @@
> #include <linux/sched/mm.h>
> #include <linux/sort.h>
> #include <linux/hmm.h>
> +#include <linux/rmap.h>
>
> struct nouveau_svm {
> struct nouveau_drm *drm;
> @@ -67,6 +68,11 @@ struct nouveau_svm {
> } buffer[1];
> };
>
> +#define FAULT_ACCESS_READ 0
> +#define FAULT_ACCESS_WRITE 1
> +#define FAULT_ACCESS_ATOMIC 2
> +#define FAULT_ACCESS_PREFETCH 3
> +
> #define SVM_DBG(s,f,a...) NV_DEBUG((s)->drm, "svm: "f"\n", ##a)
> #define SVM_ERR(s,f,a...) NV_WARN((s)->drm, "svm: "f"\n", ##a)
>
> @@ -411,6 +417,24 @@ nouveau_svm_fault_cancel_fault(struct nouveau_svm *svm,
> fault->client);
> }
>
> +static int
> +nouveau_svm_fault_priority(u8 fault)
> +{
> + switch (fault) {
> + case FAULT_ACCESS_PREFETCH:
> + return 0;
> + case FAULT_ACCESS_READ:
> + return 1;
> + case FAULT_ACCESS_WRITE:
> + return 2;
> + case FAULT_ACCESS_ATOMIC:
> + return 3;
> + default:
> + WARN_ON_ONCE(1);
> + return -1;
> + }
> +}
> +
> static int
> nouveau_svm_fault_cmp(const void *a, const void *b)
> {
> @@ -421,9 +445,8 @@ nouveau_svm_fault_cmp(const void *a, const void *b)
> return ret;
> if ((ret = (s64)fa->addr - fb->addr))
> return ret;
> - /*XXX: atomic? */
> - return (fa->access == 0 || fa->access == 3) -
> - (fb->access == 0 || fb->access == 3);
> + return nouveau_svm_fault_priority(fa->access) -
> + nouveau_svm_fault_priority(fb->access);
> }
>
> static void
> @@ -487,6 +510,10 @@ static bool nouveau_svm_range_invalidate(struct mmu_interval_notifier *mni,
> struct svm_notifier *sn =
> container_of(mni, struct svm_notifier, notifier);
>
> + if (range->event == MMU_NOTIFY_EXCLUSIVE &&
> + range->owner == sn->svmm->vmm->cli->drm->dev)
> + return true;
> +
> /*
> * serializes the update to mni->invalidate_seq done by caller and
> * prevents invalidation of the PTE from progressing while HW is being
> @@ -555,6 +582,71 @@ static void nouveau_hmm_convert_pfn(struct nouveau_drm *drm,
> args->p.phys[0] |= NVIF_VMM_PFNMAP_V0_W;
> }
>
> +static int nouveau_atomic_range_fault(struct nouveau_svmm *svmm,
> + struct nouveau_drm *drm,
> + struct nouveau_pfnmap_args *args, u32 size,
> + struct svm_notifier *notifier)
> +{
> + unsigned long timeout =
> + jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> + struct mm_struct *mm = svmm->notifier.mm;
> + struct page *page;
> + unsigned long start = args->p.addr;
> + unsigned long notifier_seq;
> + int ret = 0;
> +
> + ret = mmu_interval_notifier_insert(¬ifier->notifier, mm,
> + args->p.addr, args->p.size,
> + &nouveau_svm_mni_ops);
> + if (ret)
> + return ret;
> +
> + while (true) {
> + if (time_after(jiffies, timeout)) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + notifier_seq = mmu_interval_read_begin(¬ifier->notifier);
> + mmap_read_lock(mm);
> + make_device_exclusive_range(mm, start, start + PAGE_SIZE,
> + &page, drm->dev);
> + mmap_read_unlock(mm);
> + if (!page) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + mutex_lock(&svmm->mutex);
> + if (!mmu_interval_read_retry(¬ifier->notifier,
> + notifier_seq))
> + break;
> + mutex_unlock(&svmm->mutex);
> + }
> +
> + /* Map the page on the GPU. */
> + args->p.page = 12;
> + args->p.size = PAGE_SIZE;
> + args->p.addr = start;
> + args->p.phys[0] = page_to_phys(page) |
> + NVIF_VMM_PFNMAP_V0_V |
> + NVIF_VMM_PFNMAP_V0_W |
> + NVIF_VMM_PFNMAP_V0_A |
> + NVIF_VMM_PFNMAP_V0_HOST;
> +
> + svmm->vmm->vmm.object.client->super = true;
> + ret = nvif_object_ioctl(&svmm->vmm->vmm.object, args, size, NULL);
> + svmm->vmm->vmm.object.client->super = false;
> + mutex_unlock(&svmm->mutex);
> +
> + unlock_page(page);
> + put_page(page);
> +
> +out:
> + mmu_interval_notifier_remove(¬ifier->notifier);
> + return ret;
> +}
> +
> static int nouveau_range_fault(struct nouveau_svmm *svmm,
> struct nouveau_drm *drm,
> struct nouveau_pfnmap_args *args, u32 size,
> @@ -637,7 +729,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
> unsigned long hmm_flags;
> u64 inst, start, limit;
> int fi, fn;
> - int replay = 0, ret;
> + int replay = 0, atomic = 0, ret;
>
> /* Parse available fault buffer entries into a cache, and update
> * the GET pointer so HW can reuse the entries.
> @@ -718,12 +810,14 @@ nouveau_svm_fault(struct nvif_notify *notify)
> /*
> * Determine required permissions based on GPU fault
> * access flags.
> - * XXX: atomic?
> */
> switch (buffer->fault[fi]->access) {
> case 0: /* READ. */
> hmm_flags = HMM_PFN_REQ_FAULT;
> break;
> + case 2: /* ATOMIC. */
> + atomic = true;
> + break;
> case 3: /* PREFETCH. */
> hmm_flags = 0;
> break;
> @@ -739,8 +833,14 @@ nouveau_svm_fault(struct nvif_notify *notify)
> }
>
> notifier.svmm = svmm;
> - ret = nouveau_range_fault(svmm, svm->drm, &args.i,
> - sizeof(args), hmm_flags, ¬ifier);
> + if (atomic)
> + ret = nouveau_atomic_range_fault(svmm, svm->drm,
> + &args.i, sizeof(args),
> + ¬ifier);
> + else
> + ret = nouveau_range_fault(svmm, svm->drm, &args.i,
> + sizeof(args), hmm_flags,
> + ¬ifier);
> mmput(mm);
>
> limit = args.i.p.addr + args.i.p.size;
> @@ -756,11 +856,15 @@ nouveau_svm_fault(struct nvif_notify *notify)
> */
> if (buffer->fault[fn]->svmm != svmm ||
> buffer->fault[fn]->addr >= limit ||
> - (buffer->fault[fi]->access == 0 /* READ. */ &&
> + (buffer->fault[fi]->access == FAULT_ACCESS_READ &&
> !(args.phys[0] & NVIF_VMM_PFNMAP_V0_V)) ||
> - (buffer->fault[fi]->access != 0 /* READ. */ &&
> - buffer->fault[fi]->access != 3 /* PREFETCH. */ &&
> - !(args.phys[0] & NVIF_VMM_PFNMAP_V0_W)))
> + (buffer->fault[fi]->access != FAULT_ACCESS_READ &&
> + buffer->fault[fi]->access != FAULT_ACCESS_PREFETCH &&
> + !(args.phys[0] & NVIF_VMM_PFNMAP_V0_W)) ||
> + (buffer->fault[fi]->access != FAULT_ACCESS_READ &&
> + buffer->fault[fi]->access != FAULT_ACCESS_WRITE &&
> + buffer->fault[fi]->access != FAULT_ACCESS_PREFETCH &&
> + !(args.phys[0] & NVIF_VMM_PFNMAP_V0_A)))
> break;
> }
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> index a2b179568970..f6188aa9171c 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h
> @@ -178,6 +178,7 @@ void nvkm_vmm_unmap_region(struct nvkm_vmm *, struct nvkm_vma *);
> #define NVKM_VMM_PFN_APER 0x00000000000000f0ULL
> #define NVKM_VMM_PFN_HOST 0x0000000000000000ULL
> #define NVKM_VMM_PFN_VRAM 0x0000000000000010ULL
> +#define NVKM_VMM_PFN_A 0x0000000000000004ULL
> #define NVKM_VMM_PFN_W 0x0000000000000002ULL
> #define NVKM_VMM_PFN_V 0x0000000000000001ULL
> #define NVKM_VMM_PFN_NONE 0x0000000000000000ULL
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
> index 236db5570771..f02abd9cb4dd 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c
> @@ -88,6 +88,9 @@ gp100_vmm_pgt_pfn(struct nvkm_vmm *vmm, struct nvkm_mmu_pt *pt,
> if (!(*map->pfn & NVKM_VMM_PFN_W))
> data |= BIT_ULL(6); /* RO. */
>
> + if (!(*map->pfn & NVKM_VMM_PFN_A))
> + data |= BIT_ULL(7); /* Atomic disable. */
> +
> if (!(*map->pfn & NVKM_VMM_PFN_VRAM)) {
> addr = *map->pfn >> NVKM_VMM_PFN_ADDR_SHIFT;
> addr = dma_map_page(dev, pfn_to_page(addr), 0,
> @@ -322,6 +325,9 @@ gp100_vmm_pd0_pfn(struct nvkm_vmm *vmm, struct nvkm_mmu_pt *pt,
> if (!(*map->pfn & NVKM_VMM_PFN_W))
> data |= BIT_ULL(6); /* RO. */
>
> + if (!(*map->pfn & NVKM_VMM_PFN_A))
> + data |= BIT_ULL(7); /* Atomic disable. */
> +
> if (!(*map->pfn & NVKM_VMM_PFN_VRAM)) {
> addr = *map->pfn >> NVKM_VMM_PFN_ADDR_SHIFT;
> addr = dma_map_page(dev, pfn_to_page(addr), 0,
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-05-21 4:04 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-07 8:42 [PATCH v8 0/8] Add support for SVM atomics in Nouveau Alistair Popple
2021-04-07 8:42 ` [PATCH v8 1/8] mm: Remove special swap entry functions Alistair Popple
2021-05-18 2:17 ` Peter Xu
2021-05-18 11:58 ` Alistair Popple
2021-05-18 14:17 ` Peter Xu
2021-04-07 8:42 ` [PATCH v8 2/8] mm/swapops: Rework swap entry manipulation code Alistair Popple
2021-04-07 8:42 ` [PATCH v8 3/8] mm/rmap: Split try_to_munlock from try_to_unmap Alistair Popple
2021-05-18 20:04 ` Liam Howlett
2021-05-19 12:38 ` Alistair Popple
2021-05-20 20:24 ` Liam Howlett
2021-05-21 2:23 ` Alistair Popple
2021-04-07 8:42 ` [PATCH v8 4/8] mm/rmap: Split migration into its own function Alistair Popple
2021-04-07 8:42 ` [PATCH v8 5/8] mm: Device exclusive memory access Alistair Popple
2021-05-18 2:08 ` Peter Xu
2021-05-18 13:19 ` Alistair Popple
2021-05-18 17:27 ` Peter Xu
2021-05-18 17:33 ` Jason Gunthorpe
2021-05-18 18:01 ` Peter Xu
2021-05-18 19:45 ` Jason Gunthorpe
2021-05-18 20:29 ` Peter Xu
2021-05-18 23:03 ` Jason Gunthorpe
2021-05-18 23:45 ` Peter Xu
2021-05-19 11:04 ` Alistair Popple
2021-05-19 12:15 ` Peter Xu
2021-05-19 13:11 ` Alistair Popple
2021-05-19 14:04 ` Peter Xu
2021-05-19 13:28 ` Jason Gunthorpe
2021-05-19 14:09 ` Peter Xu
2021-05-19 18:11 ` Jason Gunthorpe
2021-05-19 11:35 ` Alistair Popple
2021-05-19 12:21 ` Peter Xu
2021-05-19 12:46 ` Alistair Popple
2021-05-21 6:53 ` Alistair Popple
2021-05-18 21:16 ` Peter Xu
2021-05-19 10:49 ` Alistair Popple
2021-05-19 12:24 ` Peter Xu
2021-05-19 12:46 ` Alistair Popple
2021-04-07 8:42 ` [PATCH v8 6/8] mm: Selftests for exclusive device memory Alistair Popple
2021-04-07 8:42 ` [PATCH v8 7/8] nouveau/svm: Refactor nouveau_range_fault Alistair Popple
2021-04-07 8:42 ` [PATCH v8 8/8] nouveau/svm: Implement atomic SVM access Alistair Popple
2021-05-21 4:04 ` Ben Skeggs [this message]
2021-05-06 7:43 ` [PATCH v8 0/8] Add support for SVM atomics in Nouveau Alistair Popple
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=CACAvsv6hKbmHMHmJSxMyij_4Y1v51Zo5NAfLtDyaQyiamtGOLQ@mail.gmail.com \
--to=skeggsb@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=apopple@nvidia.com \
--cc=bsingharora@gmail.com \
--cc=bskeggs@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@infradead.org \
--cc=jgg@nvidia.com \
--cc=jglisse@redhat.com \
--cc=jhubbard@nvidia.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nouveau@lists.freedesktop.org \
--cc=rcampbell@nvidia.com \
--cc=willy@infradead.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).