Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Nareshkumar Gollakoti <naresh.kumar.g@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, <himal.prasad.ghimiray@intel.com>
Subject: Re: [PATCH 0/6] ATS/PASID SVA support and PRI-less fault servicing
Date: Wed, 21 Oct 2026 14:24:49 -0700	[thread overview]
Message-ID: <atktoYr4H6wjnR4m@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20261021041117.1774877-1-naresh.kumar.g@intel.com>

On Wed, Oct 21, 2026 at 09:41:11AM +0530, Nareshkumar Gollakoti wrote:
> This series adds initial Shared Virtual Addressing (SVA) support for xe
> on platforms where ATS and PASID are available but PRI is not enabled or
> not supported.
> 
> For SVA, the device must be able to issue memory transactions tagged
> with a PASID so that address translation is performed in the process
> address space rather than in a driver owned address space. This series
> adds the xe side infrastructure needed for that flow: VM support for SVA
> PASID state, PASID retriveal helpers, and programming of CTX_SYS_PASID
> for ATS and PASID enabled contexts.
> 
> On systems with PRI, device page faults can be surfaced and resolved
> through the hardware page request interface, so the driver does not need
> to implement a separate fault servicing path for SVA.
> On non PRI systems, that hardware assisted fault path is unavailable.In
> that case, the driver still needs a way to resolve faults against the
> backing process address space so execution can proceed.
> 
> To support that case, this series introduces a CPU mirror VMA and uses
> it as part of a driver managed SVM path for SVA faults without PRI.The
> purpose of the CPU mirror VMA is to provide a CPU side VMA
> representation for the address range being accesses through SVA,so the
> driver can rely on the kernel MM/VMA machinery to validate mappings and
> resolve backing pages for fault handling in the absence of PRI.In
> other words, since the device cannot issue PRI requests, the driver
> needs a CPU visible VMA context that mirrors the GPU SVA range closely
> enough to let faults be service through normal MM paths.
> 
> At a high level, this series does the following:
> 
>  1. log ATS/PASID enablement state during probe,
>  2. add VM infrastructure needed for SVA PASID support,
>  3. add a helper to retrieve the VM SVA PASID,
>  4. program CTX_SYS_PASID for ATS/PASID-enabled contexts,
>  5. add a PCI helper for ATS/PASID operation without PRI,
>  6. add a CPU mirror VMA for handling SVA faults on non PRI platforms
> 
> Patches 1-5 provide the ATS/PASID and VM/context plumbing needed for
> this flow and have been validated internally.Patch 6 adds the non PRI
> fault servicing path, and feedback on that overall approach would be
> especially appreciated.
> 

Many concerns with this series, and I'm also generally quite confused
about the flows here. Let's take a step back and dig into what you're
trying to accomplish and why.

Prior to this series:

VM bind sets up PPGTTs.
- System memory uses DMA addresses (via dma-map) with DM=0 in the entry.
  These are GPAs (no pasid) translated by the IOMMU (or via ATS).
- Device memory uses DPAs with DM=1 in the entry.
SVM is configured via VM bind and CPU page-table mirroring.
- Pagefault or prefetches create mappings, data may or may not migrate.

After this series, all I have is questions and none are explained by the
cover-letter or patch commits:

1. Is a user expected to call VM bind? It seems the answer is yes.

2. Do we need any changes to the PPGTT setup? bspec53912 seems to
indicate that we do, but I'm not seeing any PPGTT setup changes in this
series. At the very minimum, I'd expect userptr to not use dma-address
and rely a PPGTT entry with P=0, FF=0 so the PASID walk occurs.

3. Are any programming model changes required in the UMD? If so, what
are they? I'm also concerned that this becomes a blind opt-in whenever
ATS + PASID are supported, as this would surely affect performance and a
UMD should have an explicit opt-in or opt-out mechanism.

4. Is SVM just implicitly enabled? Patch 6 seems to suggest that it is.
However, if I'm reading bspec53912 correctly and force-fault is not set
in the PPGTT, non-present PPGTT entries that correspond to valid CPU
pages would simply be translated and never migrated. If we miss in the
CPU page tables (e.g., the CPU has never touched the page), migration
could still occur, but only in 4K chunks, which would be terrible for
performance.

5. If SVM is explicitly enabled (e.g., through CPU page-table mirroring
via a CPU bind), we still have all the same issues described in #4 with
migration largely being bypassed. However, we'd at least hit the CPU
mirror and could perform larger migrations.

6. The most important question is: why is this being posted at all?
Unless we have hard data showing a performance benefit, I'd prefer not
to carry this code in the upstream tree. I could see some very narrow
use cases (e.g., userptr or SVM with a preferred system-memory location)
where avoiding a DMA map might provide a small performance win. However,
we'd likely need to rework things so that FF=1 is the default SVM
configuration. Alternatively, if we wanted access-counter-driven
migration rather than first-touch migration, this might make sense. 

Matt


> Himal Prasad Ghimiray (1):
>   drm/xe/vm: Setup SVA infrastructure for PASID support
> 
> Nareshkumar Gollakoti (5):
>   drm/xe/pci: log ATS/PASID enablement state at probe
>   drm/xe/vm: add helper to retrieve VM SVA PASID
>   drm/xe/lrc: program CTX_SYS_PASID for ATS/PASID-enabled contexts
>   drm/xe/pci: add helper for ATS/PASID without PRI
>   drm/xe/pagefault: add CPU mirror VMA for SVA faults without PRI
> 
>  drivers/gpu/drm/xe/regs/xe_lrc_layout.h |  6 +++
>  drivers/gpu/drm/xe/xe_lrc.c             | 37 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_pagefault.c       | 49 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_pagefault_types.h |  6 +++
>  drivers/gpu/drm/xe/xe_pci.c             |  7 ++++
>  drivers/gpu/drm/xe/xe_pci.h             | 15 +++++++
>  drivers/gpu/drm/xe/xe_vm.c              | 56 +++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_vm.h              |  2 +
>  drivers/gpu/drm/xe/xe_vm_types.h        |  6 +++
>  9 files changed, 184 insertions(+)
> 
> -- 
> 2.43.0
> 

      parent reply	other threads:[~2026-10-21 21:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21  4:11 [PATCH 0/6] ATS/PASID SVA support and PRI-less fault servicing Nareshkumar Gollakoti
2026-10-21  4:11 ` [PATCH 1/6] drm/xe/pci: log ATS/PASID enablement state at probe Nareshkumar Gollakoti
2026-10-21  4:32   ` sashiko-bot
2026-10-21  4:11 ` [PATCH 2/6] drm/xe/vm: Setup SVA infrastructure for PASID support Nareshkumar Gollakoti
2026-10-21  4:30   ` sashiko-bot
2026-10-21  4:11 ` [PATCH 3/6] drm/xe/vm: add helper to retrieve VM SVA PASID Nareshkumar Gollakoti
2026-10-21  4:11 ` [PATCH 4/6] drm/xe/lrc: program CTX_SYS_PASID for ATS/PASID-enabled contexts Nareshkumar Gollakoti
2026-10-21  4:11 ` [PATCH 5/6] drm/xe/pci: add helper for ATS/PASID without PRI Nareshkumar Gollakoti
2026-10-21  4:11 ` [PATCH 6/6] drm/xe/pagefault: add CPU mirror VMA for SVA faults " Nareshkumar Gollakoti
2026-10-21  4:35   ` sashiko-bot
2026-10-21  4:21 ` ✗ CI.KUnit: failure for ATS/PASID SVA support and PRI-less fault servicing Patchwork
2026-10-21 15:25 ` [PATCH 0/6] " Thomas Hellström
2026-10-22  6:30   ` Kumar G, Naresh
2026-10-22 11:12     ` Thomas Hellström
2026-10-21 21:24 ` Matthew Brost [this message]

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=atktoYr4H6wjnR4m@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=naresh.kumar.g@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: 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