[PATCH v4 0/5] accel/amdxdna: SYNC_BO correctness fixes
Taimuraz Kaitmazov <[email protected]>
| Newsgroups | org.freedesktop.lists.dri-devel |
|---|---|
| Message-ID | <[email protected]> |
Five fixes in and around amdxdna_drm_sync_bo_ioctl().
Patch 1 refuses an I/O memory mapping the driver would otherwise store as
if it were an ordinary kernel address. Patch 2 checks the device-BO range
for overflow. Patch 4 refuses a flush of an imported BO, which is why
patch 3 comes first: the ioctl answers a rejected flush with drm_WARN(),
so without it an ordinary sync on an imported BO splats. Patch 5 stops
the ioctl reporting failure for a flush that succeeded.
v3's zero-length patch has left this series. On hardware it turns out to
be a page fault in drm_clflush_virt_range() rather than the tidy-up its
commit message described, so it is a -fixes patch now, sent separately as
"accel/amdxdna: return early from a zero-length flush" with Fixes: and
Cc: stable. Patch 4 here needs its hunk, so this series wants that one
first.
Changes in v4:
- patch 1: the is_iomem check moved from the .vmap callback into
amdxdna_gem_vmap(), per Lizhi, and it logs at debug level rather than
error, since an unprivileged caller can repeat it.
- new patch 3: an unprivileged SYNC_BO with an out-of-range offset
already reaches that drm_WARN() today. Sashiko's review of v3 3/3
flagged the same thing.
- new patch 4: refuses the flush for every imported BO, as asked.
- new patch 5: the debug-BO sync I mentioned on the v2 thread. Only a BO
attached with ATTACH_DEBUG_BO has an assigned hwctx, so every other
FROM_DEVICE sync ends in -EINVAL with its flush already done. The
-EINVAL reproduces on a Strix Point NPU.
On patch 4, one consequence worth deciding before it lands. A heap BO is
created through amdxdna_drm_create_share_bo(), so a device running on
carveout memory reaches its heap through a cbuf, is_import_bo() is true of
it, and SYNC_BO on every AMDXDNA_BO_DEV now answers -EOPNOTSUPP. Today
that path flushes nothing anyway -- amdxdna_cbuf_map() fills in only the
DMA address and length, so drm_clflush_sg() walks zero pages -- so the
change is silent no-op to hard error, and XRT's dbg_buffer::sync() reaches
it without Debug.force_driver_sync. If you would rather keep our own
exporters working, I have the variant keyed on the exporter's ops, which
confines the refusal to foreign buffers. Say which you prefer.
v3: https://lore.kernel.org/all/[email protected]/
Built on drm-misc-next, each commit on its own: x86_64 with
DRM_ACCEL_AMDXDNA=m, clang 22.1.8, no warnings. Patch 5's reproducer was
run against 7.1.8's in-tree driver, where that call is unchanged.
Taimuraz Kaitmazov (5):
accel/amdxdna: refuse an I/O memory mapping of an imported BO
accel/amdxdna: check the sync range for overflow on a device BO
accel/amdxdna: do not warn when a sync request is rejected
accel/amdxdna: refuse to flush an imported BO
accel/amdxdna: do not fail a sync for a BO with no debug context
drivers/accel/amdxdna/amdxdna_ctx.c | 3 ++-
drivers/accel/amdxdna/amdxdna_gem.c | 27 +++++++++++++++++++--------
2 files changed, 21 insertions(+), 9 deletions(-)
--
2.55.0