AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Huang <danielhu@amd.com>
To: <amd-gfx@lists.freedesktop.org>
Cc: "Felix Kuehling" <Felix.Kuehling@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Daniel Huang" <danielhu@amd.com>
Subject: [PATCH v3 0/2] drm/amdkfd: bound SVM mappings by the GPU address limits
Date: Tue, 20 Oct 2026 21:28:59 +0800	[thread overview]
Message-ID: <20261020132901.23157-1-danielhu@amd.com> (raw)

KFD SVM maps memory in GPUVM at the same virtual address used by the
CPU, but no code bounds that address against what the GPU can actually
represent. kfd_ioctl_svm() validates only page alignment and a non-zero
size, and svm_range_map_to_gpu() passes the PFNs straight to
amdgpu_vm_update_range(). The BO mapping path, by contrast, validates
through amdgpu_vm_verify_parameters().

Patch 1 is the crash reported in [1]: with an explicit amdgpu.vm_size
override the GPUVM can be smaller than the CPU address range, and the
page-table walker indexes past the root page-table entries. It disables
SVM when the GPUVM cannot cover AMDGPU_GMC_HOLE_START.

Patch 2 addresses a gap in that check, found by the Sashiko AI review
bot [3]. The comparison is against the GPU's own address hole, so on a
system whose CPU address space is larger than the GPU's it succeeds and
SVM stays enabled, while individual addresses can still fall outside the
GPUVM. Every current GMC generation uses a 4-level, 48-bit GPUVM except
one gfx12 variant, so a 5-level paging CPU reaches this whenever
userspace passes an mmap hint above DEFAULT_MAP_WINDOW.

Changes since v2 [2]:
- Dropped the Fixes: tag and the stable Cc from both patches, and
  dropped stable@vger.kernel.org from the Cc list of this posting. The
  crash requires an explicit amdgpu.vm_size override, which exists for
  driver testing rather than normal end-user configuration, so this is
  not a conventional bug fix (Christian).
- Patch 1 now states in its changelog why GMC v6/v7/v8 cannot be
  affected: kgd2kfd_init_zone_device() returns early for GC IP versions
  below 9.0.1, leaving kfd.pgmap.type at zero, so those parts never
  advertised HSA_CAP_SVMAPI_SUPPORTED to begin with and the new
  condition cannot change their behaviour. Thanks to Felix for
  confirming that KFD SVM has never been supported before GFXv9.
- Patch 2 rewritten following Felix's review. v2 checked every range
  against every GPU at map time, and svm_range_map_to_gpus() skipped a
  GPU that could not represent the range without reporting anything to
  userspace. v3 instead determines the smallest GPUVM among the SVM
  capable GPUs of the process in svm_range_list_init(), stores it in
  svm_range_list, and validates where ranges are created.
- Canonicalised the Closes: URL in patch 1; the previous repository
  redirects to [1].

Three points in patch 2 where I did not follow the suggestion exactly,
flagged here so they are easy to object to:

- The set-attribute check went into svm_range_is_valid() rather than
  svm_range_add(). svm_range_set_attr() calls it immediately before
  svm_range_add(), it already takes (p, start, size) in pages and
  already computes the inclusive last page, it runs before anything is
  allocated, and it additionally covers svm_range_get_attr(). Happy to
  move it into svm_range_add() if you prefer the literal placement.

- svm_range_create_unregistered_range() clamps rather than rejects. Its
  range boundaries widen the fault address to the granularity, so a
  fault that is itself in range can produce a last page that is not.
  Rejecting the widened range would fail a legitimate fault, so only
  the fault address itself is rejected and the widened range is clamped.

- One late check is kept, the hard -EINVAL in svm_range_map_to_gpu(),
  immediately before amdgpu_vm_update_range() walks the page tables.
  With the creation-time validation in place it should be unreachable;
  it is there as a backstop on the exact path that produced the GPF in
  [1]. Say the word and I will drop it.

Note that a process wide minimum makes a GPU with a larger GPUVM
unusable above the smallest limit in the process, even for ranges that
only that GPU would map. Given the 4-level, 48-bit uniformity noted
above this should not be observable on real systems, but it is a
behaviour change and I would rather state it than have it found later.

[1] https://github.com/ROCm/TheRock/issues/9302
[2] https://lore.kernel.org/amd-gfx/20261016170546.88570-1-danielhu@amd.com/
[3] https://sashiko.dev/#/patchset/20261016145129.69645-1-danielhu%40amd.com

Per Documentation/process/generated-content.rst: an AI coding assistant
was used throughout this series, as recorded by the Assisted-by tags. It
diagnosed the fault path ordering that determines where patch 1 emits
its warning, designed the creation-time validation and its call sites in
patch 2, and drafted both changelogs and this cover letter. I reviewed
and tested the result and take responsibility for it.

Patch 1 is tested on a Radeon 780M as described in its notes. Patch 2 is
built for CONFIG_HSA_AMD_SVM=y and =n and otherwise compile-tested only;
I do not have a 5-level paging host with a 4-level GPU to exercise the
out-of-range path directly, so review of the bounds arithmetic would be
welcome.

Daniel Huang (2):
  drm/amdkfd: disable SVM when the GPUVM cannot cover CPU addresses
  drm/amdkfd: bound SVM ranges by the smallest GPU address limit

 drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  4 +++
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c      | 44 +++++++++++++++++++++--
 drivers/gpu/drm/amd/amdkfd/kfd_svm.h      | 23 ++++++++++--
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 11 +++++-
 4 files changed, 76 insertions(+), 6 deletions(-)


base-commit: fe9f3ed36c83dcc24b4ca3e8913edd6e33a89150
-- 
2.53.0


             reply	other threads:[~2026-10-20  5:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 13:28 Daniel Huang [this message]
2026-10-20 13:29 ` [PATCH v3 1/2] drm/amdkfd: disable SVM when the GPUVM cannot cover CPU addresses Daniel Huang
2026-10-20 18:30   ` Felix Kuehling
2026-10-20 13:29 ` [PATCH v3 2/2] drm/amdkfd: bound SVM ranges by the smallest GPU address limit Daniel Huang
2026-10-20 18:41   ` Felix Kuehling

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=20261020132901.23157-1-danielhu@amd.com \
    --to=danielhu@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.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