Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

24 participants
3677 discussions

[PATCH v6 0/9] vfio/pci: Add mmap() for DMABUFs

by Matt Evans

Hi all, The goal of this series is to enable userspace driver designs that use VFIO to export DMABUFs representing subsets of PCI device BARs, and "vend" those buffers from a primary process to other subordinate processes by fd. These processes then mmap() the buffers and their access to the device is isolated to the exported ranges. This is an improvement on sharing the VFIO device fd to subordinate processes, which would allow unfettered access. This is achieved by enabling mmap() of vfio-pci DMABUFs, passed by fd to subordinate processes. Second, a new revocation mechanism is added to allow the primary process to forcibly revoke access to previously-shared BAR spans, even if the subordinate processes haven't cleanly exited. (The related topic of safe delegation of iommufd control to the subordinate processes is not addressed here, and is follow-up work.) The background/rationale is covered in more detail in the RFC cover letters. Reviews requested that we migrate the existing VFIO PCI BAR mmap() to be backed by a DMABUF too, resulting in a common vm_ops and fault handler for mmap()s of both the VFIO device and explicitly-exported DMABUFs. This will help future iommufd emulation of VFIO Type1 peer-to-peer, making it easier to get a DMABUF for a VFIO BAR as a DMA target. mmap() conversion to use DMABUF underneath has been done for vfio-pci, but not sub-drivers: nvgrace-gpu's mmap() override path is unchanged; I kept this out of scope for now not least because I don't have a thorough test setup for this system. I would prefer to help the nvgrace-gpu maintainers enable BAR mmap() DMABUFs themselves. Notes on patches ================ vfio/pci: Remove DMABUF export dependency on vdev->memory_lock In v5 of this series [1] we discover that doing an export from the VFIO mmap() path (fundamental!) adds a dependency between mmap_lock and vdev->memory_lock(W) because export was using memory_lock(W) to protect the vdev->dmabufs list and state within, and a deadlock scenario leapt out to bite. (Details in [1].) The suggestion was to add a dedicated mutex/rwsem specifically for the DMABUFs/list, which is cleaner than overloading memory_lock(W). But whilst export could now downgrade to holding memory_lock just for _read_ to test __vfio_pci_memory_enabled(), a very similar deadlock can still arise due to a memory_lock(W) elsewhere depending on a prior memory_lock(R) to be released, and attempting to take memory_lock(R) will queue behind the (W) for fairness (effectively an R->R dependency). I'd overlooked that rwsem cannot guarantee multiple readers. To be able to export while holding mmap_lock, export cannot hold memory_lock at all. Instead of testing __vfio_pci_memory_enabled() (which tracks PCI_COMMAND.MSE and PM state), this patch tracks device-global DMABUF revocation state in a new vdev->bars_revoked flag updated by vfio_pci_dma_buf_move(). If an mmap() is somehow performed during a period when DMABUFs are all revoked, then the DMABUF is created revoked. Move() already bookends reset, PM transitions etc., so subsequent revocation state changes work as-is. This flag is also protected by the dmabuf_lock and thus memory_lock is not required to export. vfio/pci: Un-revoke DMABUFs in LOW_POWER_ENTRY_WITH_WAKEUP resume On LOW_POWER entry, DMABUFs have a move(revoke=true), but the runtime resume path didn't un-revoke. This adds a corresponding move(revoke=false), which will later turn into vfio_pci_unrevoke_bars(). NOTE: the unrevoke is reordered _before_ the eventfd_signal() in vfio_pci_core_runtime_resume(). This removes a window in which a waking waiter could have observed the DMABUF state as still revoked (or pm_runtime_engaged = true). (The UAPI docs state the event means the resume's complete, so observing otherwise seemed unintended.) Also, when DMABUFs are later mmap()ed, a waiter waking and taking a fault could have seen the unrevoked state and SIGBUS just before taking the memory_lock. (If the handler gets to acquire the lock, though, the resume sequence is complete, and the handler observes pm_runtime_engaged = false.) This fix is in this series because the issue will impact CPU access to the VMA as well (once they use DMABUFs), and so it's a strict dependency of later commits. dma-buf: Export dma_buf_set_name() Makes dma_buf_set_name() available for use (by helper patch), taking a kernel-allocated string. The pre-existing local helper becomes a wrapper copying a __user string for the set-name ioctl. vfio/pci: Add a helper to look up PFNs for DMABUFs Adds a DMABUF VMA fault handler helper to determine arbitrary-sized PFNs from ranges in DMABUF. vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA Refactors DMABUF export for use by the existing export feature, and adds a helper that creates a DMABUF corresponding to a VFIO BAR mmap() request. There was a request for decent debug naming in /proc/<pid>/maps etc. comparable to the existing VFIO names: since the VMAs are DMABUFs, they have a "dmabuf:" prefix and can't be 100% identical to before. This is a user-visible change, but this patch at least now gives us extra info on the BDF & BAR being mmap()ed. The name is installed with the new dma_buf_set_name() above. An alternative would be to add a creation-time name field to struct dma_buf_export_info, but a setter function is more useful: the name can be changed at will after init. vfio/pci: Convert BAR mmap() to use a DMABUF The vfio-pci core mmap() creates a DMABUF with the helper above, and the vm_ops fault handler uses the other helper to resolve the fault. Because this depends on DMABUF structs/code, CONFIG_VFIO_PCI_CORE needs to depend on CONFIG_DMA_SHARED_BUFFER. The CONFIG_VFIO_PCI_DMABUF still conditionally enables the export support code. NOTE: The user mmap()s a device fd, but the resulting VMA's vm_file becomes that of the DMABUF. The DMABUF takes ownership of the device file and put()s it on release, which maintains the existing behaviour of a VMA keeping the VFIO device open. BAR zapping then happens via the existing vfio_pci_dma_buf_move() path, which now needs to unmap PTEs in the DMABUF's address_space. NOTE: As local LLM reviews did, Sashiko might falsely worry about the DMABUF fd being obtained through /proc/pid/map_files and remapped, but the file is an anon inode and doesn't support the open op (-ENXIO) so AFAICT this is currently impossible. NOTE: A side-effect of this is that is_mergeable_vma() will be false between adjacent mappings of VFIO BARs; merging would require rebuilding a larger DMABUF representing the union, not just plugging the VMAs together. The DMABUF backing the BAR is an implicit/internal export, even when the CONFIG_VFIO_PCI_DMABUF feature is not included because CONFIG_PCI_P2PDMA is not available. Without P2PDMA, it's acceptable for the DMABUF to not have a P2PDMA provider. In this configuration, the VFIO DMABUF .attach prohibits any import, which avoids getting as far as a WARN in dma_buf_map_attachment(), which would fail without P2PDMA anyway. vfio/pci: Clean up BAR zap and revocation In general (see NOTE!) the vfio_pci_zap_bars() is now obsolete, since it unmaps PTEs in the VFIO device address_space which is now unused. This consolidates all calls (e.g. around reset) with the neighbouring vfio_pci_dma_buf_move()s into new functions, to revoke/unrevoke (making the steps clearer). NOTE: Because drivers can use their own vm_ops and override .mmap, the core must conservatively assume an overridden .mmap might still add PTEs to the VFIO device address_space and therefore still does the zap. A new flag, zap_bars_on_revoke, enables the zap when .mmap is overridden. A driver that does not need the zap can clear this to opt-out, e.g. if the driver calls down to the common mmap (and so uses DMABUFs). hisi-acc-vfio-pci does just this, and thus sets the opt-out flag. vfio/pci: Support mmap() of a VFIO DMABUF Adds mmap() for a DMABUF fd exported from vfio-pci. It was a goal to keep the VFIO device fd lifetime behaviour unchanged with respect to the DMABUFs. An application can close all device fds, and this will revoke/clean up all DMABUFs; then, no mappings or other access can be performed. When enabling mmap() of the DMABUFs, this means access through the VMA is also revoked. This complicates the fault handler because whilst the DMABUF exists, it has no guarantee that the corresponding VFIO device is still alive. Adds synchronisation ensuring the vdev is available before the locks in vdev are touched; this holds the device registration so that even if the buffer has been cleaned up, vdev hasn't been freed and so the locks can be safely taken. vfio/pci: Permanently revoke a DMABUF on request This is mostly a rename of `revoked` to an enum, `status`, and adding a third state for a buffer: usable, revoked temporary, revoked permanent. A new VFIO feature is added, VFIO_DEVICE_FEATURE_DMA_BUF_REVOKE, which takes a DMABUF (exported from the same device) and permanently revokes it. Thus a userspace driver can guarantee any downstream consumers of a shared fd are prevented from accessing a BAR range, and that range can be reused. NOTE: This might block userspace, waiting on importers to detach. The code doing revocation in vfio_pci_dma_buf_move() is moved, to a common function for use by ..._move() and this new feature. Testing ======= (The [RFC ONLY] userspace test program, which drives a QEMU bochs-display function, can be found in the GitHub branch below. It at least illustrates how the export, map, revoke, and close semantics interoperate. WIP on a follow-up with a proper vfio-selftests style test based on this -- this won't be part of this series.) This code has been tested in mapping DMABUFs of single/multiple ranges from multiple BARs, aliasing mmap()s, aliasing ranges across DMABUFs, vm_pgoff > 0, revocation, shutdown/cleanup scenarios, and hugepage mappings. No regressions observed on the VFIO selftests, or on our internal vfio-pci applications. VFIO on i386 has been build-tested. Thanks to Alex Mastro for building a (WIP) testcase for the mmap_lock->memory_lock issue (which is now OK...). Dear Reviewers, =============== There was a lot of finessing v5->v6, all patches had fixes/changes/cleanups/rewordings made, and so I have dropped previous R-B tags. Along the way several related issues came up that warrant more eyes, and I'd be grateful for your input: 1. The mmap fault handler takes a bunch of locks non-interruptibly, and potentially depends on a lot of DMABUF-related activities completing. I had a go at converting them to interruptible/killable forms, but pulling on the thread revealed there seems to be a wider issue if move/revoke doesn't complete in a timely fashion (due to buggy importers). Where I got to was that just updating the fault handler won't fix the user experience of an unkillable task, and move()/revocation will need thought too. I don't intend to fix this here but wanted to start discussion so we can address it in a follow up. There's now a dependency between mmap_lock in the fault handler and the DMABUF resv (which might take a while to resolve). 2. vfio_basic_config_write() has an error path if vfio_default_config_write() fails that releases memory_lock but doesn't un-revoke BARs in the case of PCI_COMMAND.MSE being cleared. When can the write fail, in practice, perhaps surprise removal? The effect on this series would be: a write of MSE=0 revokes BARs, vconfig[PCI_COMMAND]'s MSE becomes 0, but if the physical write fails then the physical MSE remains 1 and BAR VMAs stay revoked. This seemed a mess; fixing isn't as simple as un-revoking on the error path since vfio_default_config_write() has already trampled vconfig so that'd need unwinding. It felt like a catastrophic scenario where BARs staying revoked isn't a bad outcome, but want to hear your experience of the likelihood of this issue. 3. The exchange of a VFIO fd mmap()'s vma->vm_file with an implicitly created DMABUF's file has implications on LSM. For example, an mmap will be checked against the policy for a VFIO fd, but a subsequent mprotect() relates to the DMABUF file's policy (which is anon/unique to the mapping). This is pretty confusing. 4. If VFIO fd is opened O_RDONLY, it currently can't be mmap()ed (because PROT_WRITE is rejected in do_mmap(), and PROT_READ alone drops the VM_SHARED so VFIO's mmap rejects it). But it seems we can export a DMABUF from it, and then pass the resulting fd around for P2P writes. I don't know if this is intentional, relied on, or a known limitation so haven't included a change here, but: a) We could reject export unless the device fd's f_mode has O_RDWR, to reflect the abilities of P2P b) Or, instead of just failing if !O_RDWR, we limit the get_dma_buf.open_flags to the VFIO device fd's f_mode, such as: VFIO device fd perms: Export flags: Result: O_RDWR O_RDWR, O_RDONLY OK O_RDONLY O_RDONLY OK O_RDONLY O_RDWR -EPERM O_WRONLY * -EPERM * O_WRONLY -EPERM (Skipping WRONLY because a PROT_WRITE-only mmap() won't work, though it probably should be included for P2P.) If we can do at least (a) that seems good, because the knock-on effect in this series is that we can export a DMABUF RW from an O_RDONLY device fd and then succeed to mmap() the DMABUF with RW. (That said, even if one has an O_RDONLY device fd, the device state can still be changed/reset. But it feels cleaner to least prevent export for a O_RDONLY device fd, and match the device fd mmap() behaviour.) Maybe (b) is step 2: Allowing finer-grained RD/WR could be useful if there's a future goal to tie DMABUF permissions to, say, iommufd IOMMU_READ/IOMMU_WRITE permissions. I don't htink this is in place today, apologies if I'm missed something. END === This is based on v7.2. These commits are on GitHub for easier browsing, along with "[RFC ONLY] selftests: vfio: Add standalone vfio_dmabuf_mmap_test": https://github.com/metamev/linux/compare/v7.2...dev/mev/vfio-dmabuf-mmap-v6 Thanks for reading [this astonishingly-long cover letter], Matt [1] https://lore.kernel.org/kvm/9a615f22-c0d4-46ae-9654-db11e94e5fec@ozlabs.org/ ================================================================================ Changelog: v6: - Dropped patches: PCI/P2PDMA: Split pool-related cleanup out of pci_p2pdma_release() PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE - New patch, "vfio/pci: Remove DMABUF export dependency on vdev->memory_lock": memory_lock was overloaded to protect the vdev->dmabufs list, the entries' revoked status, etc. Towards the goal of not needing memory_lock in export (so not creating a mmap_lock -> memory_lock dependency due to exporting from mmap()), add a new vdev->dmabuf_lock which protects the list and (writing) the revocation status. (See details in cover letter.) - New patch, "vfio/pci: Un-revoke DMABUFs in LOW_POWER_ENTRY_WITH_WAKEUP resume": Bugfix, as the resume path didn't un-revoke. Previously affected "only" P2P, but since DMABUFs-everywhere it would affect BAR mmap()s too. - New patch, "dma-buf: Export dma_buf_set_name()": Change in dma-buf.c's dma_buf_set_name() to take a kernel-allocated string. The original IOCTL-related path wraps this to strdup the user string first. - "vfio/pci: Add a helper to look up PFNs for DMABUFs": Commit message reworded explaining vma_pgoff_adjust, return -ERANGE for (non-transient) pagesize failure instead of -EAGAIN. Add dmabuf_lock annotation and assert that the DMABUF isn't revoked (else new -ENODEV error), using a new vdev parameter (caller is expected to safely extract it from priv). Significant bugfix in the case of a VMA offset exceeding the 1TB stride between VFIO_PCI_OFFSET_MASK-sized regions, which would have silently wrapped due to being masked to 1TB. This is fixed by including the VFIO region index in the high bits of vma_pgoff_adjust for the traditional mmap() path, so the subtraction from vma->vm_pgoff in ...find_pfn() removes the region index without masking. For the DMABUF mmap() path, vma_pgoff_adjust = 0 and (thanks to no masking) large offsets can be used. - "vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA": Now permits a NULL p2pdma provider to be installed in the vfio_pci_dma_buf. The regular BAR mmap() path doesn't need a provider for CPU access; the DMABUF backing the VMA exists primarily for the CPU, and removing this check decouples vfio-pci from depending on CONFIG_PCI_P2PDMA (_without_ going through the previous hoops of splitting up P2PDMA into a _CORE subset to provide the provider). Only DMABUF import/map needs a provider, which is available when P2PDMA is available; the previous series' first two patches (to always provide a provider) are now unnecessary and have been dropped. Squashed "vfio/pci: Provide a user-facing name for BAR mappings" into this patch, because it's now just very small due to the new dma_buf_set_name(). Comment that, because of careful construction of the name length, the setting of the name can't fail; but in case assumptions change, a check/kfree prevents it leaking. (A new error path doing full unwind of the DMABUF creation is overkill.) - "vfio/pci: Convert BAR mmap() to use a DMABUF": Instead of the previous dropped P2PDMA split commits, allows pcim_p2pdma_provider() to return NULL if !CONFIG_VFIO_PCI_DMABUF (meaning !CONFIG_PCI_P2PDMA). The provider isn't used unless the DMABUF is imported; prevent import to make this fail early (rather than at map), by creating an always-fail vfio_pci_dma_buf_attach() stub. (Somewhat belt and braces, but makes clear you need P2PDMA to import a BAR VMA's DMABUF!) - "vfio/pci: Clean up BAR zap and revocation": The zap_bars_on_revoke flag is moved back to the bitfield, and is set from vfio_pci_core_init_dev() (instead of registration time). Reworked the hisi driver change to update this earlier. - "vfio/pci: Support mmap() of a VFIO DMABUF": Previous versions introduced a build issue (which was fixed by the next patch) due to a READ_ONCE of the priv->revoked bitfield member; removed. It is added by the next patch in the series (s/revoked/status/). Added an explicit rejection for mmap of a DMABUF with vma_pgoff_adjust > 0. Such buffers can only be created implicitly by the VFIO BAR mmap path, and an fd can't currently be recreated from a VMA (e.g. fished out of /proc/pid/map_files) because the anon inode ops don't support open. But check added if a future mechanism arises and for clarity. Rejecting an mmap of a DMABUF fd having vma_pgoff_adjust > 0 means vfio_pci_dma_buf_find_pfn() doesn't need to cope with the case of vma->vm_pgoff < priv->vma_pgoff_adjust and underflow. - "vfio/pci: Permanently revoke a DMABUF on request": Clarified docs for returned error values; re-added READ_ONCE(priv->status) for unlocked reads in both vfio_pci_dma_buf_attach() and vfio_pci_dma_buf_mmap(). v5: https://lore.kernel.org/all/20260715174737.15287-1-matt@ozlabs.org/ v4: https://lore.kernel.org/all/20260701171245.90111-1-matt@ozlabs.org/ v3: https://lore.kernel.org/all/20260610154327.37758-1-matt@ozlabs.org/ v2: https://lore.kernel.org/all/20260527102319.100128-1-mattev@meta.com/ v1: https://lore.kernel.org/kvm/20260416131815.2729131-1-mattev@meta.com/ RFCv2: https://lore.kernel.org/kvm/20260312184613.3710705-1-mattev@meta.com/ RFCv1: https://lore.kernel.org/all/20260226202211.929005-1-mattev@meta.com/ Tech topic: https://lore.kernel.org/linux-iommu/20250918214425.2677057-1-amastro@fb.com/ Matt Evans (9): vfio/pci: Remove DMABUF export dependency on vdev->memory_lock vfio/pci: Un-revoke DMABUFs in LOW_POWER_ENTRY_WITH_WAKEUP resume dma-buf: Export dma_buf_set_name() vfio/pci: Add a helper to look up PFNs for DMABUFs vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA vfio/pci: Convert BAR mmap() to use a DMABUF vfio/pci: Clean up BAR zap and revocation vfio/pci: Support mmap() of a VFIO DMABUF vfio/pci: Permanently revoke a DMABUF on request drivers/dma-buf/dma-buf.c | 58 +- drivers/vfio/pci/Kconfig | 4 +- drivers/vfio/pci/Makefile | 3 +- .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 14 +- drivers/vfio/pci/vfio_pci_config.c | 30 +- drivers/vfio/pci/vfio_pci_core.c | 222 +++++-- drivers/vfio/pci/vfio_pci_dmabuf.c | 607 +++++++++++++++--- drivers/vfio/pci/vfio_pci_priv.h | 54 +- include/linux/dma-buf.h | 2 + include/linux/vfio_pci_core.h | 3 + include/uapi/linux/vfio.h | 24 + 11 files changed, 851 insertions(+), 170 deletions(-) -- 2.50.1 (Apple Git-155)

1 month

Re: [PATCH v7 15/19] PCI/P2PDMA: Document TLP-class routing

by Leon Romanovsky

On Mon, Sep 21, 2026 at 04:16:56PM -0600, Logan Gunthorpe wrote: > > > On 2026-09-20 05:41, Leon Romanovsky wrote: > > +A path is bus addressable only where both directions route directly, so with > > +nothing enabled every class is. > > I don't understand the sentence. I tried to say that we need both Requests and Completions route directly. Claude suggested something like that: --------------------------------------------------------------------------- Combining the two tables gives the mapping type: a class is bus addressable when its Request and its Completions both route directly, a blocked Request is not supported because Translation Blocking rejects the Address Type wherever the Request is addressed, and anything else goes through the host bridge. With no ACS control set anywhere, every class routes directly. --------------------------------------------------------------------------- Thanks > > > Logan

1 month

Re: [PATCH v2] dma-buf/udmabuf: reinstate a reasonable default size limit

by Christian König

On 9/19/26 02:42, Jiakai Xu wrote: > Commit 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by > default") raised the default size_limit_mb from 64 to INT_MAX. With > 4K pages, creating a buffer of more than INT_MAX / 8 pages (about 1 TB > of pinned memory) now makes the two per-page pointer arrays in > init_udmabuf() exceed what kvmalloc() can handle: kvmalloc() warns and > fails when the requested size exceeds INT_MAX bytes (WARN_ON_ONCE in > __kvmalloc_node_noprof), which is trivially triggerable from > unprivileged users through the UDMABUF_CREATE ioctls: > > WARNING: CPU: 0 PID: 412 at mm/slub.c:7021 __kvmalloc_node_noprof > Call Trace: > udmabuf_create > udmabuf_ioctl_create_list > > Restore a reasonable default limit of 256 MB, which is the value > proposed in the RFC discussion that led to commit 44e9eb5a7621 That is way to low, we want to allocate multiple gigabytes using that API. Please make that (((INT_MAX / sizeof(struct folio *)) - 1) >> 20) << PAGE_SHIFT. That should allow to allocate a bit below 1TiB again without triggering the issue. Regards, Christian. > ("dma-buf/udmabuf: Disable the size limit by default") and comfortably > covers the 8K video use cases cited there, while keeping the > kvmalloc() request far below its threshold. The limit remains > runtime-tunable via the size_limit_mb module parameter for setups > that need more. > > Fixes: 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by default") > Assisted-by: OpenCode:DeepSeek-V4-Flash > Signed-off-by: Jiakai Xu <xujiakai24(a)mails.ucas.ac.cn> > --- > V1 -> V2: > - Instead of adding a pgcnt check in udmabuf_create(), restore a > reasonable default size limit (256 MB), as suggested by Christian > König. > - Drop the second Fixes tag; the regression is solely due to > 44e9eb5a7621. > - Drop the "Cc: stable" tag; 44e9eb5a7621 is only in v7.3-rc1 and has > not shipped in any released kernel yet. > > V1: https://lore.kernel.org/all/20260918013434.1849135-1-xujiakai24@mails.ucas.… > --- > drivers/dma-buf/udmabuf.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index df6dd00462423..4d90c2cb14225 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -20,9 +20,9 @@ static int list_limit = 1024; > module_param(list_limit, int, 0644); > MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024."); > > -static int size_limit_mb = INT_MAX; > +static int size_limit_mb = 256; > module_param(size_limit_mb, int, 0644); > -MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is INT_MAX."); > +MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is 256."); > > struct udmabuf { > pgoff_t pagecount; > -- > 2.34.1 > > -- > Below is the crash report: > ------------[ cut here ]------------ > WARNING: mm/slub.c:7013 at __kvmalloc_node_noprof+0x516/0x5d0 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/mm/slub.c:7013, CPU#0: syz.5.27938/167363 > Modules linked in: > CPU: 0 UID: 0 PID: 167363 Comm: syz.5.27938 Tainted: G W L 7.3.0-rc2 #1 PREEMPT(full) > Tainted: [W]=WARN, [L]=SOFTLOCKUP > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:__kvmalloc_node_noprof+0x516/0x5d0 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/mm/slub.c:7013 > Code: 05 bf fa 2d 08 49 8b 0f c6 04 08 00 65 48 ff 0d 90 fa 2d 08 0f 85 4c fd ff ff e8 05 43 8c ff e9 42 fd ff ff e8 9b 86 0e 04 90 <0f> 0b 90 31 db e9 d2 fe ff ff f3 0f 1e fa 65 8b 05 8d fa 2d 08 89 > RSP: 0018:ffffc90002bc3cf8 EFLAGS: 00010246 > RAX: 0000000000000100 RBX: 0000000000000000 RCX: 0000008000000000 > RDX: 0000008000000000 RSI: 0000000000000000 RDI: ffffffff835e3226 > RBP: 00000000ffffffff R08: 00000000000028c0 R09: 00000000ffffffff > R10: 0000000000000006 R11: 0000000000000002 R12: 0000008000000000 > R13: 00000000000028c0 R14: 0000001000000000 R15: 0000008000000000 > FS: 00007fc89ae45640(0000) GS:ffff8880f3de4000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000000110c2ff305 CR3: 000000002b3d4000 CR4: 0000000000752ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000006000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > PKRU: 80000000 > Call Trace: > <TASK> > init_udmabuf home/zzzrrll/tmp/kf_src/linux-7.3-rc2/drivers/dma-buf/udmabuf.c:193 [inline] > udmabuf_create+0x1b6/0x750 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/drivers/dma-buf/udmabuf.c:386 > udmabuf_ioctl_create home/zzzrrll/tmp/kf_src/linux-7.3-rc2/drivers/dma-buf/udmabuf.c:465 [inline] > udmabuf_ioctl+0x132/0x1d0 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/drivers/dma-buf/udmabuf.c:496 > vfs_ioctl home/zzzrrll/tmp/kf_src/linux-7.3-rc2/fs/ioctl.c:51 [inline] > __do_sys_ioctl home/zzzrrll/tmp/kf_src/linux-7.3-rc2/fs/ioctl.c:597 [inline] > __se_sys_ioctl+0xbc/0x130 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/fs/ioctl.c:583 > do_syscall_x64 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/arch/x86/entry/syscall_64.c:61 [inline] > do_syscall_64+0x12b/0x350 home/zzzrrll/tmp/kf_src/linux-7.3-rc2/arch/x86/entry/syscall_64.c:84 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > RIP: 0033:0x590d6d > Code: 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48 > RSP: 002b:00007fc89ae44fd8 EFLAGS: 00000216 ORIG_RAX: 0000000000000010 > RAX: ffffffffffffffda RBX: 0000000000611e7f RCX: 0000000000590d6d > RDX: 00002000000023c0 RSI: 0000000040187542 RDI: 000000000000000a > RBP: 00007fc89ae45010 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000216 R12: 00007fc89ae45640 > R13: 000000000000000b R14: 0000000000528d40 R15: 00007fc89ae25000 > </TASK> > ---[ end trace 0000000000000000 ]--- > --- >

1 month

Re: [PATCH] dma-buf/udmabuf: reinstate a reasonable default size limit

by Christian König

On 9/22/26 03:23, Jiakai Xu wrote: > Commit 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by > default") raised the default size_limit_mb from 64 to INT_MAX. > > With 4 KiB pages on a 64-bit system, creating a buffer larger than > about 1 TiB makes the two per-page pointer arrays in init_udmabuf() > exceed what kvmalloc() can handle. __kvmalloc_node_noprof() warns and > fails when the requested size exceeds INT_MAX bytes, which is > trivially triggerable by unprivileged users through the UDMABUF_CREATE > ioctls: > > WARNING: CPU: 0 PID: 412 at mm/slub.c:7021 __kvmalloc_node_noprof > Call Trace: > udmabuf_create > udmabuf_ioctl_create_list > > Instead of using a fixed megabyte value, derive the default limit from > the kvmalloc() threshold: > > (((INT_MAX / sizeof(struct folio *)) - 1) >> 20) << PAGE_SHIFT > > On a 64-bit system with 4 KiB pages, this evaluates to 1044480 MB > (1020 GiB), allowing buffers a little below 1 TiB while keeping the > per-page pointer array allocations below INT_MAX bytes. > > Fixes: 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by default") > Suggested-by: Christian König <christian.koenig(a)amd.com> > Assisted-by: OpenCode:DeepSeek-V4-Flash > Signed-off-by: Jiakai Xu <xujiakai24(a)mails.ucas.ac.cn> Reviewed-by: Christian König <christian.koenig(a)amd.com> Going to push that to drm-misc-fixes later today. Regards, Christian. > --- > V2 -> V3: > - Replace the fixed 256 MB default with a limit derived from the > kvmalloc() INT_MAX threshold, as suggested by Christian König. On > 64-bit systems with 4 KiB pages, this allows buffers just below > 1 TiB. > > V1 -> V2: > - Instead of adding a pgcnt check in udmabuf_create(), restore a > reasonable default size limit (256 MB), as suggested by Christian > König. > - Drop the second Fixes tag; the regression is solely due to > 44e9eb5a7621. > - Drop the "Cc: stable" tag; 44e9eb5a7621 is only in v7.3-rc1 and has > not shipped in any released kernel yet. > > V1: https://lore.kernel.org/all/20260918013434.1849135-1-xujiakai24@mails.ucas.… > V2: https://lore.kernel.org/all/20260919015731.2077604-1-xujiakai24@mails.ucas.… > --- > drivers/dma-buf/udmabuf.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index df6dd00462423..7ec269809fb49 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -20,9 +20,10 @@ static int list_limit = 1024; > module_param(list_limit, int, 0644); > MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024."); > > -static int size_limit_mb = INT_MAX; > +static int size_limit_mb = (((INT_MAX / sizeof(struct folio *)) - 1) >> 20) << PAGE_SHIFT; > module_param(size_limit_mb, int, 0644); > -MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is INT_MAX."); > +MODULE_PARM_DESC(size_limit_mb, > + "Max size of a dmabuf, in megabytes. Default is derived from the kvmalloc() limit."); > > struct udmabuf { > pgoff_t pagecount;

1 month

[PATCH V2] accel/amdxdna: Drop dma-buf wrapping for ubuf

by Lizhi Hou

A ubuf buffer is a range of existing userspace memory passed in through CREATE_BO. The driver currently wraps those pages in a dma-buf, then imports that dma-buf as a GEM object. The extra layer does not add a sharing path: ubuf is not exportable, and the pages never belonged to the driver in the first place. Make ubuf a DRM GEM private object and tracks the user VA with HMM instead of a long-term pin. - Create the BO directly from the VA table without going through dma-buf. Export stays unsupported. - Drop pin_user_pages(FOLL_LONGTERM). Create registers an MMU interval notifier over the user range and marks it invalid. Command submit faults the range in if it is still invalid. That only works with PASID, which is the only mode where ubuf is supported. Signed-off-by: Lizhi Hou <lizhi.hou(a)amd.com> --- V2: Fix sashiko comment: remove holding process's virtual address space. drivers/accel/amdxdna/amdxdna_gem.c | 85 ++++++----- drivers/accel/amdxdna/amdxdna_gem.h | 12 ++ drivers/accel/amdxdna/amdxdna_ubuf.c | 207 ++++++++++----------------- drivers/accel/amdxdna/amdxdna_ubuf.h | 8 +- 4 files changed, 138 insertions(+), 174 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c index f4832337ec31..a3d9a54c6563 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.c +++ b/drivers/accel/amdxdna/amdxdna_gem.c @@ -164,8 +164,7 @@ void amdxdna_gem_heap_free(struct amdxdna_client *client, struct amdxdna_gem_obj mutex_unlock(&client->mm_lock); } -static void -amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo) +void amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo) { mutex_destroy(&abo->lock); kfree(abo); @@ -190,7 +189,9 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo) if (!abo->mem.kva) { ret = drm_gem_vmap(to_gobj(abo), &map); - if (ret) + if (ret == -EOPNOTSUPP) + XDNA_DBG(xdna, "Vmap bo is not supported"); + else if (ret) XDNA_ERR(xdna, "Vmap bo failed, ret %d", ret); else abo->mem.kva = map.vaddr; @@ -291,7 +292,7 @@ static void amdxdna_hmm_unregister(struct amdxdna_gem_obj *abo, up_write(&xdna->notifier_lock); } -static void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force) +void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); struct amdxdna_umap *mapp, *tmp; @@ -335,12 +336,11 @@ static void amdxdna_hmm_unreg_work(struct work_struct *work) amdxdna_hmm_unreg_umaps(abo, false); } -static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, - struct vm_area_struct *vma) +int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, struct vm_area_struct *vma, + size_t offset, size_t len) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); - unsigned long len = vma->vm_end - vma->vm_start; - unsigned long addr = vma->vm_start; + unsigned long addr = vma->vm_start + offset; struct amdxdna_umap *mapp; unsigned long nr_pages; int ret; @@ -359,7 +359,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, down_read(&xdna->notifier_lock); list_for_each_entry(mapp, &abo->mem.umap_list, node) { - if (compare_range(mapp, current->mm, addr, addr + len)) { + if (compare_range(mapp, vma->vm_mm, addr, addr + len)) { up_read(&xdna->notifier_lock); return 0; } @@ -371,15 +371,15 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, return -ENOMEM; nr_pages = (PAGE_ALIGN(addr + len) - (addr & PAGE_MASK)) >> PAGE_SHIFT; - mapp->range.hmm_pfns = kvzalloc_objs(*mapp->range.hmm_pfns, nr_pages); + mapp->range.hmm_pfns = kvzalloc_objs(*mapp->range.hmm_pfns, nr_pages, GFP_KERNEL_ACCOUNT); if (!mapp->range.hmm_pfns) { ret = -ENOMEM; goto free_map; } mapp->range.notifier = &mapp->notifier; - mapp->range.start = vma->vm_start; - mapp->range.end = vma->vm_end; + mapp->range.start = addr; + mapp->range.end = addr + len; /* * Access permissions are fixed at mmap() time. Changing them later * with mprotect() is not supported: the range keeps requesting the @@ -393,7 +393,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, kref_init(&mapp->refcnt); ret = mmu_interval_notifier_insert_locked(&mapp->notifier, - current->mm, + vma->vm_mm, addr, len, &amdxdna_hmm_ops); @@ -417,7 +417,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, return ret; } -static struct amdxdna_gem_obj * +struct amdxdna_gem_obj * amdxdna_gem_create_obj(struct drm_device *dev, size_t size) { struct amdxdna_gem_obj *abo; @@ -462,8 +462,9 @@ static void amdxdna_gem_dev_obj_free(struct drm_gem_object *gobj) amdxdna_gem_destroy_obj(abo); } -static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, - struct vm_area_struct *vma) +void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, + struct mm_struct *mm, + unsigned long start, unsigned long end) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); struct amdxdna_umap *mapp; @@ -471,7 +472,7 @@ static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, down_write(&xdna->notifier_lock); abo->mem.map_invalid = true; list_for_each_entry(mapp, &abo->mem.umap_list, node) { - if (compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end)) { + if (compare_range(mapp, mm, start, end)) { mapp->invalid = true; break; } @@ -502,7 +503,7 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo, return ret; } - amdxdna_mark_mapp_invalid(abo, vma); + amdxdna_mark_mapp_invalid(abo, vma->vm_mm, vma->vm_start, vma->vm_end); /* Drop the reference drm_gem_mmap_obj() acquired.*/ drm_gem_object_put(to_gobj(abo)); @@ -539,7 +540,7 @@ static int amdxdna_gem_obj_mmap(struct drm_gem_object *gobj, drm_vma_node_offset_addr(&gobj->vma_node), abo->type, vma->vm_start, gobj->size); - ret = amdxdna_hmm_register(abo, vma); + ret = amdxdna_hmm_register(abo, vma, 0, vma->vm_end - vma->vm_start); if (ret) return ret; @@ -682,7 +683,7 @@ static struct dma_buf *amdxdna_gem_prime_export(struct drm_gem_object *gobj, int struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); - if (abo->private_buffer) + if (is_private_bo(abo)) return ERR_PTR(-EOPNOTSUPP); if (abo->dma_buf) { @@ -766,7 +767,7 @@ static void amdxdna_gem_obj_free(struct drm_gem_object *gobj) drm_gem_shmem_free(&abo->base); } -static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp) +int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp) { struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev); struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); @@ -805,7 +806,7 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi return 0; } -static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp) +void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp) { struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); struct amdxdna_client *client = NULL; @@ -991,40 +992,46 @@ amdxdna_gem_create_shmem_object(struct drm_device *dev, struct amdxdna_drm_creat } static struct amdxdna_gem_obj * -amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create_bo *args) +amdxdna_gem_create_ubuf_object(struct drm_device *dev, + struct amdxdna_drm_create_bo *args, + struct drm_file *filp) { + struct amdxdna_client *client = filp->driver_priv; struct amdxdna_dev *xdna = to_xdna_dev(dev); struct amdxdna_drm_va_tbl va_tbl; struct amdxdna_gem_obj *abo; struct drm_gem_object *gobj; struct dma_buf *dma_buf; + if (args->type == AMDXDNA_BO_DEV_HEAP || args->type == AMDXDNA_BO_CMD) + return ERR_PTR(-EOPNOTSUPP); + if (copy_from_user(&va_tbl, u64_to_user_ptr(args->vaddr), sizeof(va_tbl))) { XDNA_DBG(xdna, "Access va table failed"); return ERR_PTR(-EINVAL); } if (va_tbl.num_entries) { - dma_buf = amdxdna_get_ubuf(dev, va_tbl.num_entries, - u64_to_user_ptr(args->vaddr + sizeof(va_tbl))); + abo = amdxdna_alloc_ubuf_bo(client, va_tbl.num_entries, + u64_to_user_ptr(args->vaddr + sizeof(va_tbl))); + if (IS_ERR(abo)) + return abo; } else { dma_buf = dma_buf_get(va_tbl.dmabuf_fd); - } + if (IS_ERR(dma_buf)) + return ERR_CAST(dma_buf); - if (IS_ERR(dma_buf)) - return ERR_CAST(dma_buf); + gobj = amdxdna_gem_prime_import(dev, dma_buf); + if (IS_ERR(gobj)) { + dma_buf_put(dma_buf); + return ERR_CAST(gobj); + } - gobj = amdxdna_gem_prime_import(dev, dma_buf); - if (IS_ERR(gobj)) { dma_buf_put(dma_buf); - return ERR_CAST(gobj); + abo = to_xdna_obj(gobj); } - dma_buf_put(dma_buf); - - abo = to_xdna_obj(gobj); abo->private_buffer = true; - return abo; } @@ -1112,7 +1119,7 @@ amdxdna_drm_create_share_bo(struct drm_device *dev, struct amdxdna_gem_obj *abo; if (args->vaddr) - abo = amdxdna_gem_create_ubuf_object(dev, args); + abo = amdxdna_gem_create_ubuf_object(dev, args, filp); else if (amdxdna_use_carveout(to_xdna_dev(dev))) abo = amdxdna_gem_create_cbuf_object(dev, args); else @@ -1293,7 +1300,7 @@ static int amdxdna_bo_pin(struct amdxdna_gem_obj *abo) struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); int ret; - if (is_import_bo(abo)) + if (is_import_bo(abo) || is_private_bo(abo)) return 0; ret = drm_gem_shmem_pin(&abo->base); @@ -1306,7 +1313,7 @@ static void amdxdna_bo_unpin(struct amdxdna_gem_obj *abo) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); - if (is_import_bo(abo)) + if (is_import_bo(abo) || is_private_bo(abo)) return; drm_gem_shmem_unpin(&abo->base); @@ -1406,7 +1413,7 @@ int amdxdna_drm_get_bo_info_ioctl(struct drm_device *dev, void *data, struct drm args->vaddr = amdxdna_gem_uva(abo); args->xdna_addr = amdxdna_gem_dev_addr(abo); - if (abo->type != AMDXDNA_BO_DEV) + if (abo->type != AMDXDNA_BO_DEV && !is_private_bo(abo)) args->map_offset = drm_vma_node_offset_addr(&gobj->vma_node); else args->map_offset = AMDXDNA_INVALID_ADDR; diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h index 9b4aa21a37c9..815348a3c429 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.h +++ b/drivers/accel/amdxdna/amdxdna_gem.h @@ -60,6 +60,7 @@ struct amdxdna_gem_obj { #define to_gobj(obj) (&(obj)->base.base) #define is_import_bo(obj) ((obj)->attach) +#define is_private_bo(obj) ((obj)->private_buffer) static inline struct amdxdna_gem_obj *to_xdna_obj(struct drm_gem_object *gobj) { @@ -112,10 +113,21 @@ amdxdna_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf); struct amdxdna_gem_obj * amdxdna_drm_create_dev_bo(struct drm_device *dev, struct amdxdna_drm_create_bo *args, struct drm_file *filp); +struct amdxdna_gem_obj * +amdxdna_gem_create_obj(struct drm_device *dev, size_t size); +void amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo); +void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force); int amdxdna_gem_pin_nolock(struct amdxdna_gem_obj *abo); int amdxdna_gem_pin(struct amdxdna_gem_obj *abo); void amdxdna_gem_unpin(struct amdxdna_gem_obj *abo); +int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp); +void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp); +int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, struct vm_area_struct *vma, + size_t offset, size_t len); +void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, + struct mm_struct *mm, + unsigned long start, unsigned long end); int amdxdna_drm_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); int amdxdna_drm_get_bo_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c index 0e0cd69cd1fb..5c291786d981 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.c +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c @@ -6,182 +6,127 @@ #include <drm/amdxdna_accel.h> #include <drm/drm_device.h> #include <drm/drm_print.h> -#include <linux/dma-buf.h> #include <linux/overflow.h> #include <linux/pagemap.h> #include <linux/vmalloc.h> +#include "amdxdna_gem.h" #include "amdxdna_pci_drv.h" #include "amdxdna_ubuf.h" -struct amdxdna_ubuf_priv { - struct page **pages; - u64 nr_pages; - struct mm_struct *mm; -}; - -static struct sg_table *amdxdna_ubuf_map(struct dma_buf_attachment *attach, - enum dma_data_direction direction) +static int amdxdna_ubuf_hmm_register(struct amdxdna_client *client, + struct amdxdna_gem_obj *abo, + struct amdxdna_drm_va_entry *va_ent) { - struct amdxdna_ubuf_priv *ubuf = attach->dmabuf->priv; - struct sg_table *sg; + struct vm_area_struct *vma; int ret; - sg = kzalloc_obj(*sg); - if (!sg) - return ERR_PTR(-ENOMEM); - - ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->nr_pages, 0, - ubuf->nr_pages << PAGE_SHIFT, GFP_KERNEL); - if (ret) - goto err_free_sg; + mmap_write_lock(client->mm); + vma = find_vma(client->mm, va_ent->vaddr); + if (!vma || vma->vm_start > va_ent->vaddr || + vma->vm_end - va_ent->vaddr < va_ent->len) { + ret = -EINVAL; + goto unlock; + } - ret = dma_map_sgtable(attach->dev, sg, direction, 0); - if (ret) - goto err_free_table; + ret = amdxdna_hmm_register(abo, vma, va_ent->vaddr - vma->vm_start, va_ent->len); - return sg; +unlock: + mmap_write_unlock(client->mm); -err_free_table: - sg_free_table(sg); -err_free_sg: - kfree(sg); - return ERR_PTR(ret); + return ret; } -static void amdxdna_ubuf_unmap(struct dma_buf_attachment *attach, - struct sg_table *sg, - enum dma_data_direction direction) +static void amdxdna_gem_ubuf_obj_free(struct drm_gem_object *gobj) { - dma_unmap_sgtable(attach->dev, sg, direction, 0); - sg_free_table(sg); - kfree(sg); + struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); + + amdxdna_hmm_unreg_umaps(abo, true); + cancel_work_sync(&abo->hmm_unreg_work); + + drm_gem_object_release(gobj); + amdxdna_gem_destroy_obj(abo); } -static void amdxdna_ubuf_release(struct dma_buf *dbuf) +static struct dma_buf *amdxdna_gem_ubuf_obj_export(struct drm_gem_object *gobj, int flags) { - struct amdxdna_ubuf_priv *ubuf = dbuf->priv; - - unpin_user_pages(ubuf->pages, ubuf->nr_pages); - kvfree(ubuf->pages); - atomic64_sub(ubuf->nr_pages, &ubuf->mm->pinned_vm); - mmdrop(ubuf->mm); - kfree(ubuf); + return ERR_PTR(-EOPNOTSUPP); } -static const struct dma_buf_ops amdxdna_ubuf_dmabuf_ops = { - .map_dma_buf = amdxdna_ubuf_map, - .unmap_dma_buf = amdxdna_ubuf_unmap, - .release = amdxdna_ubuf_release, +static const struct drm_gem_object_funcs amdxdna_gem_ubuf_obj_funcs = { + .free = amdxdna_gem_ubuf_obj_free, + .open = amdxdna_gem_obj_open, + .close = amdxdna_gem_obj_close, + .export = amdxdna_gem_ubuf_obj_export, }; -struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, - u32 num_entries, void __user *va_entries) +struct amdxdna_gem_obj *amdxdna_alloc_ubuf_bo(struct amdxdna_client *client, + u32 num_entries, void __user *va_entries) { - struct amdxdna_dev *xdna = to_xdna_dev(dev); - unsigned long lock_limit, new_pinned; + struct amdxdna_dev *xdna = client->xdna; struct amdxdna_drm_va_entry *va_ent; - struct amdxdna_ubuf_priv *ubuf; - u32 npages, start = 0; - struct dma_buf *dbuf; - int i, ret; - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + struct amdxdna_gem_obj *abo; + size_t bufsize; + long ret; - if (!can_do_mlock()) - return ERR_PTR(-EPERM); + if (!amdxdna_pasid_on(client)) + return ERR_PTR(-EOPNOTSUPP); - ubuf = kzalloc_obj(*ubuf); - if (!ubuf) - return ERR_PTR(-ENOMEM); + /* + * There is not any valid case to use more than 1 entry. + * Hardcode maximum entries to 1. + */ + if (num_entries > 1) + return ERR_PTR(-EINVAL); - ubuf->mm = current->mm; - mmgrab(ubuf->mm); + if (current->mm != client->mm) + return ERR_PTR(-EINVAL); - va_ent = kvzalloc_objs(*va_ent, num_entries); - if (!va_ent) { - ret = -ENOMEM; - goto free_ubuf; - } + va_ent = kvzalloc_obj(*va_ent); + if (!va_ent) + return ERR_PTR(-ENOMEM); - if (copy_from_user(va_ent, va_entries, sizeof(*va_ent) * num_entries)) { + if (copy_from_user(va_ent, va_entries, sizeof(*va_ent))) { XDNA_DBG(xdna, "Access va entries failed"); ret = -EINVAL; goto free_ent; } - for (i = 0, exp_info.size = 0; i < num_entries; i++) { - if (!IS_ALIGNED(va_ent[i].vaddr, PAGE_SIZE) || - !IS_ALIGNED(va_ent[i].len, PAGE_SIZE)) { - XDNA_ERR(xdna, "Invalid address or len %llx, %llx", - va_ent[i].vaddr, va_ent[i].len); - ret = -EINVAL; - goto free_ent; - } - - if (check_add_overflow(exp_info.size, va_ent[i].len, &exp_info.size)) { - ret = -EINVAL; - goto free_ent; - } - } - - ubuf->nr_pages = exp_info.size >> PAGE_SHIFT; - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; - new_pinned = atomic64_add_return(ubuf->nr_pages, &ubuf->mm->pinned_vm); - if (new_pinned > lock_limit && !capable(CAP_IPC_LOCK)) { - XDNA_DBG(xdna, "New pin %ld, limit %ld, cap %d", - new_pinned, lock_limit, capable(CAP_IPC_LOCK)); - ret = -ENOMEM; - goto sub_pin_cnt; + if (!IS_ALIGNED(va_ent->vaddr, PAGE_SIZE) || + !IS_ALIGNED(va_ent->len, PAGE_SIZE) || + !va_ent->len || + check_add_overflow(va_ent->vaddr, va_ent->len, &bufsize)) { + XDNA_DBG(xdna, "Invalid address or len %llx, %llx", + va_ent->vaddr, va_ent->len); + ret = -EINVAL; + goto free_ent; } - ubuf->pages = kvmalloc_objs(*ubuf->pages, ubuf->nr_pages); - if (!ubuf->pages) { - ret = -ENOMEM; - goto sub_pin_cnt; + bufsize = va_ent->len; + abo = amdxdna_gem_create_obj(&xdna->ddev, bufsize); + if (IS_ERR(abo)) { + ret = PTR_ERR(abo); + goto free_ent; } - for (i = 0; i < num_entries; i++) { - npages = va_ent[i].len >> PAGE_SHIFT; - - ret = pin_user_pages_fast(va_ent[i].vaddr, npages, - FOLL_WRITE | FOLL_LONGTERM, - &ubuf->pages[start]); - if (ret >= 0) { - start += ret; - if (ret != npages) { - XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, npages); - ret = -ENOMEM; - goto destroy_pages; - } - } else { - XDNA_ERR(xdna, "Failed to pin pages ret %d", ret); - goto destroy_pages; - } - } + abo->type = AMDXDNA_BO_SHARE; + abo->mem.uva = va_ent->vaddr; + to_gobj(abo)->funcs = &amdxdna_gem_ubuf_obj_funcs; + drm_gem_private_object_init(&xdna->ddev, to_gobj(abo), bufsize); - exp_info.ops = &amdxdna_ubuf_dmabuf_ops; - exp_info.priv = ubuf; - exp_info.flags = O_RDWR | O_CLOEXEC; + ret = amdxdna_ubuf_hmm_register(client, abo, va_ent); + if (ret) + goto put_obj; - dbuf = dma_buf_export(&exp_info); - if (IS_ERR(dbuf)) { - ret = PTR_ERR(dbuf); - goto destroy_pages; - } + amdxdna_mark_mapp_invalid(abo, client->mm, va_ent->vaddr, + va_ent->vaddr + va_ent->len); kvfree(va_ent); - return dbuf; + return abo; -destroy_pages: - if (start) - unpin_user_pages(ubuf->pages, start); - kvfree(ubuf->pages); -sub_pin_cnt: - atomic64_sub(ubuf->nr_pages, &ubuf->mm->pinned_vm); +put_obj: + drm_gem_object_put(to_gobj(abo)); free_ent: kvfree(va_ent); -free_ubuf: - mmdrop(ubuf->mm); - kfree(ubuf); return ERR_PTR(ret); } diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.h b/drivers/accel/amdxdna/amdxdna_ubuf.h index 8900a6dc4371..f6335603da34 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.h +++ b/drivers/accel/amdxdna/amdxdna_ubuf.h @@ -5,10 +5,10 @@ #ifndef _AMDXDNA_UBUF_H_ #define _AMDXDNA_UBUF_H_ -#include <drm/drm_device.h> -#include <linux/dma-buf.h> +#include "amdxdna_gem.h" +#include "amdxdna_pci_drv.h" -struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, - u32 num_entries, void __user *va_entries); +struct amdxdna_gem_obj *amdxdna_alloc_ubuf_bo(struct amdxdna_client *client, + u32 num_entries, void __user *va_entries); #endif /* _AMDXDNA_UBUF_H_ */ -- 2.34.1

1 month

[PATCH V1] accel/amdxdna: Drop dma-buf wrapping for ubuf

by Lizhi Hou

A ubuf buffer is a range of existing userspace memory passed in through CREATE_BO. The driver currently wraps those pages in a dma-buf, then imports that dma-buf as a GEM object. The extra layer does not add a sharing path: ubuf is not exportable, and the pages never belonged to the driver in the first place. Make ubuf a DRM GEM private object and tracks the user VA with HMM instead of a long-term pin. - Create the BO directly from the VA table without going through dma-buf. Export stays unsupported. - Drop pin_user_pages(FOLL_LONGTERM). Create registers an MMU interval notifier over the user range and marks it invalid. Command submit faults the range in if it is still invalid. That only works with PASID, which is the only mode where ubuf is supported. Signed-off-by: Lizhi Hou <lizhi.hou(a)amd.com> --- drivers/accel/amdxdna/amdxdna_gem.c | 85 ++++++----- drivers/accel/amdxdna/amdxdna_gem.h | 13 ++ drivers/accel/amdxdna/amdxdna_ubuf.c | 211 +++++++++++---------------- drivers/accel/amdxdna/amdxdna_ubuf.h | 8 +- 4 files changed, 145 insertions(+), 172 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c index f4832337ec31..a3d9a54c6563 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.c +++ b/drivers/accel/amdxdna/amdxdna_gem.c @@ -164,8 +164,7 @@ void amdxdna_gem_heap_free(struct amdxdna_client *client, struct amdxdna_gem_obj mutex_unlock(&client->mm_lock); } -static void -amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo) +void amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo) { mutex_destroy(&abo->lock); kfree(abo); @@ -190,7 +189,9 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo) if (!abo->mem.kva) { ret = drm_gem_vmap(to_gobj(abo), &map); - if (ret) + if (ret == -EOPNOTSUPP) + XDNA_DBG(xdna, "Vmap bo is not supported"); + else if (ret) XDNA_ERR(xdna, "Vmap bo failed, ret %d", ret); else abo->mem.kva = map.vaddr; @@ -291,7 +292,7 @@ static void amdxdna_hmm_unregister(struct amdxdna_gem_obj *abo, up_write(&xdna->notifier_lock); } -static void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force) +void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); struct amdxdna_umap *mapp, *tmp; @@ -335,12 +336,11 @@ static void amdxdna_hmm_unreg_work(struct work_struct *work) amdxdna_hmm_unreg_umaps(abo, false); } -static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, - struct vm_area_struct *vma) +int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, struct vm_area_struct *vma, + size_t offset, size_t len) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); - unsigned long len = vma->vm_end - vma->vm_start; - unsigned long addr = vma->vm_start; + unsigned long addr = vma->vm_start + offset; struct amdxdna_umap *mapp; unsigned long nr_pages; int ret; @@ -359,7 +359,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, down_read(&xdna->notifier_lock); list_for_each_entry(mapp, &abo->mem.umap_list, node) { - if (compare_range(mapp, current->mm, addr, addr + len)) { + if (compare_range(mapp, vma->vm_mm, addr, addr + len)) { up_read(&xdna->notifier_lock); return 0; } @@ -371,15 +371,15 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, return -ENOMEM; nr_pages = (PAGE_ALIGN(addr + len) - (addr & PAGE_MASK)) >> PAGE_SHIFT; - mapp->range.hmm_pfns = kvzalloc_objs(*mapp->range.hmm_pfns, nr_pages); + mapp->range.hmm_pfns = kvzalloc_objs(*mapp->range.hmm_pfns, nr_pages, GFP_KERNEL_ACCOUNT); if (!mapp->range.hmm_pfns) { ret = -ENOMEM; goto free_map; } mapp->range.notifier = &mapp->notifier; - mapp->range.start = vma->vm_start; - mapp->range.end = vma->vm_end; + mapp->range.start = addr; + mapp->range.end = addr + len; /* * Access permissions are fixed at mmap() time. Changing them later * with mprotect() is not supported: the range keeps requesting the @@ -393,7 +393,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, kref_init(&mapp->refcnt); ret = mmu_interval_notifier_insert_locked(&mapp->notifier, - current->mm, + vma->vm_mm, addr, len, &amdxdna_hmm_ops); @@ -417,7 +417,7 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, return ret; } -static struct amdxdna_gem_obj * +struct amdxdna_gem_obj * amdxdna_gem_create_obj(struct drm_device *dev, size_t size) { struct amdxdna_gem_obj *abo; @@ -462,8 +462,9 @@ static void amdxdna_gem_dev_obj_free(struct drm_gem_object *gobj) amdxdna_gem_destroy_obj(abo); } -static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, - struct vm_area_struct *vma) +void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, + struct mm_struct *mm, + unsigned long start, unsigned long end) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); struct amdxdna_umap *mapp; @@ -471,7 +472,7 @@ static void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, down_write(&xdna->notifier_lock); abo->mem.map_invalid = true; list_for_each_entry(mapp, &abo->mem.umap_list, node) { - if (compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end)) { + if (compare_range(mapp, mm, start, end)) { mapp->invalid = true; break; } @@ -502,7 +503,7 @@ static int amdxdna_insert_pages(struct amdxdna_gem_obj *abo, return ret; } - amdxdna_mark_mapp_invalid(abo, vma); + amdxdna_mark_mapp_invalid(abo, vma->vm_mm, vma->vm_start, vma->vm_end); /* Drop the reference drm_gem_mmap_obj() acquired.*/ drm_gem_object_put(to_gobj(abo)); @@ -539,7 +540,7 @@ static int amdxdna_gem_obj_mmap(struct drm_gem_object *gobj, drm_vma_node_offset_addr(&gobj->vma_node), abo->type, vma->vm_start, gobj->size); - ret = amdxdna_hmm_register(abo, vma); + ret = amdxdna_hmm_register(abo, vma, 0, vma->vm_end - vma->vm_start); if (ret) return ret; @@ -682,7 +683,7 @@ static struct dma_buf *amdxdna_gem_prime_export(struct drm_gem_object *gobj, int struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); DEFINE_DMA_BUF_EXPORT_INFO(exp_info); - if (abo->private_buffer) + if (is_private_bo(abo)) return ERR_PTR(-EOPNOTSUPP); if (abo->dma_buf) { @@ -766,7 +767,7 @@ static void amdxdna_gem_obj_free(struct drm_gem_object *gobj) drm_gem_shmem_free(&abo->base); } -static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp) +int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp) { struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev); struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); @@ -805,7 +806,7 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi return 0; } -static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp) +void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp) { struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); struct amdxdna_client *client = NULL; @@ -991,40 +992,46 @@ amdxdna_gem_create_shmem_object(struct drm_device *dev, struct amdxdna_drm_creat } static struct amdxdna_gem_obj * -amdxdna_gem_create_ubuf_object(struct drm_device *dev, struct amdxdna_drm_create_bo *args) +amdxdna_gem_create_ubuf_object(struct drm_device *dev, + struct amdxdna_drm_create_bo *args, + struct drm_file *filp) { + struct amdxdna_client *client = filp->driver_priv; struct amdxdna_dev *xdna = to_xdna_dev(dev); struct amdxdna_drm_va_tbl va_tbl; struct amdxdna_gem_obj *abo; struct drm_gem_object *gobj; struct dma_buf *dma_buf; + if (args->type == AMDXDNA_BO_DEV_HEAP || args->type == AMDXDNA_BO_CMD) + return ERR_PTR(-EOPNOTSUPP); + if (copy_from_user(&va_tbl, u64_to_user_ptr(args->vaddr), sizeof(va_tbl))) { XDNA_DBG(xdna, "Access va table failed"); return ERR_PTR(-EINVAL); } if (va_tbl.num_entries) { - dma_buf = amdxdna_get_ubuf(dev, va_tbl.num_entries, - u64_to_user_ptr(args->vaddr + sizeof(va_tbl))); + abo = amdxdna_alloc_ubuf_bo(client, va_tbl.num_entries, + u64_to_user_ptr(args->vaddr + sizeof(va_tbl))); + if (IS_ERR(abo)) + return abo; } else { dma_buf = dma_buf_get(va_tbl.dmabuf_fd); - } + if (IS_ERR(dma_buf)) + return ERR_CAST(dma_buf); - if (IS_ERR(dma_buf)) - return ERR_CAST(dma_buf); + gobj = amdxdna_gem_prime_import(dev, dma_buf); + if (IS_ERR(gobj)) { + dma_buf_put(dma_buf); + return ERR_CAST(gobj); + } - gobj = amdxdna_gem_prime_import(dev, dma_buf); - if (IS_ERR(gobj)) { dma_buf_put(dma_buf); - return ERR_CAST(gobj); + abo = to_xdna_obj(gobj); } - dma_buf_put(dma_buf); - - abo = to_xdna_obj(gobj); abo->private_buffer = true; - return abo; } @@ -1112,7 +1119,7 @@ amdxdna_drm_create_share_bo(struct drm_device *dev, struct amdxdna_gem_obj *abo; if (args->vaddr) - abo = amdxdna_gem_create_ubuf_object(dev, args); + abo = amdxdna_gem_create_ubuf_object(dev, args, filp); else if (amdxdna_use_carveout(to_xdna_dev(dev))) abo = amdxdna_gem_create_cbuf_object(dev, args); else @@ -1293,7 +1300,7 @@ static int amdxdna_bo_pin(struct amdxdna_gem_obj *abo) struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); int ret; - if (is_import_bo(abo)) + if (is_import_bo(abo) || is_private_bo(abo)) return 0; ret = drm_gem_shmem_pin(&abo->base); @@ -1306,7 +1313,7 @@ static void amdxdna_bo_unpin(struct amdxdna_gem_obj *abo) { struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev); - if (is_import_bo(abo)) + if (is_import_bo(abo) || is_private_bo(abo)) return; drm_gem_shmem_unpin(&abo->base); @@ -1406,7 +1413,7 @@ int amdxdna_drm_get_bo_info_ioctl(struct drm_device *dev, void *data, struct drm args->vaddr = amdxdna_gem_uva(abo); args->xdna_addr = amdxdna_gem_dev_addr(abo); - if (abo->type != AMDXDNA_BO_DEV) + if (abo->type != AMDXDNA_BO_DEV && !is_private_bo(abo)) args->map_offset = drm_vma_node_offset_addr(&gobj->vma_node); else args->map_offset = AMDXDNA_INVALID_ADDR; diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h index 9b4aa21a37c9..619b3fa5211c 100644 --- a/drivers/accel/amdxdna/amdxdna_gem.h +++ b/drivers/accel/amdxdna/amdxdna_gem.h @@ -32,6 +32,7 @@ struct amdxdna_mem { * without taking notifier_lock. */ u64 uva; + struct mm_struct *mm; }; struct amdxdna_gem_obj { @@ -60,6 +61,7 @@ struct amdxdna_gem_obj { #define to_gobj(obj) (&(obj)->base.base) #define is_import_bo(obj) ((obj)->attach) +#define is_private_bo(obj) ((obj)->private_buffer) static inline struct amdxdna_gem_obj *to_xdna_obj(struct drm_gem_object *gobj) { @@ -112,10 +114,21 @@ amdxdna_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf); struct amdxdna_gem_obj * amdxdna_drm_create_dev_bo(struct drm_device *dev, struct amdxdna_drm_create_bo *args, struct drm_file *filp); +struct amdxdna_gem_obj * +amdxdna_gem_create_obj(struct drm_device *dev, size_t size); +void amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo); +void amdxdna_hmm_unreg_umaps(struct amdxdna_gem_obj *abo, bool force); int amdxdna_gem_pin_nolock(struct amdxdna_gem_obj *abo); int amdxdna_gem_pin(struct amdxdna_gem_obj *abo); void amdxdna_gem_unpin(struct amdxdna_gem_obj *abo); +int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *filp); +void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp); +int amdxdna_hmm_register(struct amdxdna_gem_obj *abo, struct vm_area_struct *vma, + size_t offset, size_t len); +void amdxdna_mark_mapp_invalid(struct amdxdna_gem_obj *abo, + struct mm_struct *mm, + unsigned long start, unsigned long end); int amdxdna_drm_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); int amdxdna_drm_get_bo_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.c b/drivers/accel/amdxdna/amdxdna_ubuf.c index 0e0cd69cd1fb..cff5d319ad60 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.c +++ b/drivers/accel/amdxdna/amdxdna_ubuf.c @@ -6,182 +6,135 @@ #include <drm/amdxdna_accel.h> #include <drm/drm_device.h> #include <drm/drm_print.h> -#include <linux/dma-buf.h> #include <linux/overflow.h> #include <linux/pagemap.h> #include <linux/vmalloc.h> +#include "amdxdna_gem.h" #include "amdxdna_pci_drv.h" #include "amdxdna_ubuf.h" -struct amdxdna_ubuf_priv { - struct page **pages; - u64 nr_pages; - struct mm_struct *mm; -}; - -static struct sg_table *amdxdna_ubuf_map(struct dma_buf_attachment *attach, - enum dma_data_direction direction) +static int amdxdna_ubuf_hmm_register(struct amdxdna_gem_obj *abo, + struct amdxdna_drm_va_entry *va_ent) { - struct amdxdna_ubuf_priv *ubuf = attach->dmabuf->priv; - struct sg_table *sg; + struct vm_area_struct *vma; int ret; - sg = kzalloc_obj(*sg); - if (!sg) - return ERR_PTR(-ENOMEM); - - ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->nr_pages, 0, - ubuf->nr_pages << PAGE_SHIFT, GFP_KERNEL); - if (ret) - goto err_free_sg; + mmap_write_lock(abo->mem.mm); + vma = find_vma(abo->mem.mm, va_ent->vaddr); + if (!vma || vma->vm_start > va_ent->vaddr || + vma->vm_end - va_ent->vaddr < va_ent->len) { + ret = -EINVAL; + goto unlock; + } - ret = dma_map_sgtable(attach->dev, sg, direction, 0); - if (ret) - goto err_free_table; + ret = amdxdna_hmm_register(abo, vma, va_ent->vaddr - vma->vm_start, va_ent->len); - return sg; +unlock: + mmap_write_unlock(abo->mem.mm); -err_free_table: - sg_free_table(sg); -err_free_sg: - kfree(sg); - return ERR_PTR(ret); + return ret; } -static void amdxdna_ubuf_unmap(struct dma_buf_attachment *attach, - struct sg_table *sg, - enum dma_data_direction direction) +static void amdxdna_gem_ubuf_obj_free(struct drm_gem_object *gobj) { - dma_unmap_sgtable(attach->dev, sg, direction, 0); - sg_free_table(sg); - kfree(sg); + struct amdxdna_gem_obj *abo = to_xdna_obj(gobj); + + amdxdna_hmm_unreg_umaps(abo, true); + cancel_work_sync(&abo->hmm_unreg_work); + + if (abo->mem.mm) + mmput(abo->mem.mm); + drm_gem_object_release(gobj); + amdxdna_gem_destroy_obj(abo); } -static void amdxdna_ubuf_release(struct dma_buf *dbuf) +static struct dma_buf *amdxdna_gem_ubuf_obj_export(struct drm_gem_object *gobj, int flags) { - struct amdxdna_ubuf_priv *ubuf = dbuf->priv; - - unpin_user_pages(ubuf->pages, ubuf->nr_pages); - kvfree(ubuf->pages); - atomic64_sub(ubuf->nr_pages, &ubuf->mm->pinned_vm); - mmdrop(ubuf->mm); - kfree(ubuf); + return ERR_PTR(-EOPNOTSUPP); } -static const struct dma_buf_ops amdxdna_ubuf_dmabuf_ops = { - .map_dma_buf = amdxdna_ubuf_map, - .unmap_dma_buf = amdxdna_ubuf_unmap, - .release = amdxdna_ubuf_release, +static const struct drm_gem_object_funcs amdxdna_gem_ubuf_obj_funcs = { + .free = amdxdna_gem_ubuf_obj_free, + .open = amdxdna_gem_obj_open, + .close = amdxdna_gem_obj_close, + .export = amdxdna_gem_ubuf_obj_export, }; -struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, - u32 num_entries, void __user *va_entries) +struct amdxdna_gem_obj *amdxdna_alloc_ubuf_bo(struct amdxdna_client *client, + u32 num_entries, void __user *va_entries) { - struct amdxdna_dev *xdna = to_xdna_dev(dev); - unsigned long lock_limit, new_pinned; + struct amdxdna_dev *xdna = client->xdna; struct amdxdna_drm_va_entry *va_ent; - struct amdxdna_ubuf_priv *ubuf; - u32 npages, start = 0; - struct dma_buf *dbuf; - int i, ret; - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + struct amdxdna_gem_obj *abo; + size_t bufsize; + long ret; - if (!can_do_mlock()) - return ERR_PTR(-EPERM); + if (!amdxdna_pasid_on(client)) + return ERR_PTR(-EOPNOTSUPP); - ubuf = kzalloc_obj(*ubuf); - if (!ubuf) - return ERR_PTR(-ENOMEM); + /* + * There is not any valid case to use more than 1 entry. + * Hardcode maximum entries to 1. + */ + if (num_entries > 1) + return ERR_PTR(-EINVAL); - ubuf->mm = current->mm; - mmgrab(ubuf->mm); + if (current->mm != client->mm) + return ERR_PTR(-EINVAL); - va_ent = kvzalloc_objs(*va_ent, num_entries); - if (!va_ent) { - ret = -ENOMEM; - goto free_ubuf; - } + va_ent = kvzalloc_obj(*va_ent); + if (!va_ent) + return ERR_PTR(-ENOMEM); - if (copy_from_user(va_ent, va_entries, sizeof(*va_ent) * num_entries)) { + if (copy_from_user(va_ent, va_entries, sizeof(*va_ent))) { XDNA_DBG(xdna, "Access va entries failed"); ret = -EINVAL; goto free_ent; } - for (i = 0, exp_info.size = 0; i < num_entries; i++) { - if (!IS_ALIGNED(va_ent[i].vaddr, PAGE_SIZE) || - !IS_ALIGNED(va_ent[i].len, PAGE_SIZE)) { - XDNA_ERR(xdna, "Invalid address or len %llx, %llx", - va_ent[i].vaddr, va_ent[i].len); - ret = -EINVAL; - goto free_ent; - } - - if (check_add_overflow(exp_info.size, va_ent[i].len, &exp_info.size)) { - ret = -EINVAL; - goto free_ent; - } + if (!IS_ALIGNED(va_ent->vaddr, PAGE_SIZE) || + !IS_ALIGNED(va_ent->len, PAGE_SIZE) || + !va_ent->len || + check_add_overflow(va_ent->vaddr, va_ent->len, &bufsize)) { + XDNA_DBG(xdna, "Invalid address or len %llx, %llx", + va_ent->vaddr, va_ent->len); + ret = -EINVAL; + goto free_ent; } - ubuf->nr_pages = exp_info.size >> PAGE_SHIFT; - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; - new_pinned = atomic64_add_return(ubuf->nr_pages, &ubuf->mm->pinned_vm); - if (new_pinned > lock_limit && !capable(CAP_IPC_LOCK)) { - XDNA_DBG(xdna, "New pin %ld, limit %ld, cap %d", - new_pinned, lock_limit, capable(CAP_IPC_LOCK)); - ret = -ENOMEM; - goto sub_pin_cnt; + bufsize = va_ent->len; + abo = amdxdna_gem_create_obj(&xdna->ddev, bufsize); + if (IS_ERR(abo)) { + ret = PTR_ERR(abo); + goto free_ent; } - ubuf->pages = kvmalloc_objs(*ubuf->pages, ubuf->nr_pages); - if (!ubuf->pages) { - ret = -ENOMEM; - goto sub_pin_cnt; - } + abo->type = AMDXDNA_BO_SHARE; + abo->mem.uva = va_ent->vaddr; + to_gobj(abo)->funcs = &amdxdna_gem_ubuf_obj_funcs; + drm_gem_private_object_init(&xdna->ddev, to_gobj(abo), bufsize); - for (i = 0; i < num_entries; i++) { - npages = va_ent[i].len >> PAGE_SHIFT; - - ret = pin_user_pages_fast(va_ent[i].vaddr, npages, - FOLL_WRITE | FOLL_LONGTERM, - &ubuf->pages[start]); - if (ret >= 0) { - start += ret; - if (ret != npages) { - XDNA_ERR(xdna, "Partially pinned pages %d/%u", ret, npages); - ret = -ENOMEM; - goto destroy_pages; - } - } else { - XDNA_ERR(xdna, "Failed to pin pages ret %d", ret); - goto destroy_pages; - } + if (!mmget_not_zero(client->mm)) { + ret = -EFAULT; + goto put_obj; } - exp_info.ops = &amdxdna_ubuf_dmabuf_ops; - exp_info.priv = ubuf; - exp_info.flags = O_RDWR | O_CLOEXEC; + abo->mem.mm = client->mm; - dbuf = dma_buf_export(&exp_info); - if (IS_ERR(dbuf)) { - ret = PTR_ERR(dbuf); - goto destroy_pages; - } + ret = amdxdna_ubuf_hmm_register(abo, va_ent); + if (ret) + goto put_obj; + + amdxdna_mark_mapp_invalid(abo, abo->mem.mm, va_ent->vaddr, + va_ent->vaddr + va_ent->len); kvfree(va_ent); - return dbuf; + return abo; -destroy_pages: - if (start) - unpin_user_pages(ubuf->pages, start); - kvfree(ubuf->pages); -sub_pin_cnt: - atomic64_sub(ubuf->nr_pages, &ubuf->mm->pinned_vm); +put_obj: + drm_gem_object_put(to_gobj(abo)); free_ent: kvfree(va_ent); -free_ubuf: - mmdrop(ubuf->mm); - kfree(ubuf); return ERR_PTR(ret); } diff --git a/drivers/accel/amdxdna/amdxdna_ubuf.h b/drivers/accel/amdxdna/amdxdna_ubuf.h index 8900a6dc4371..f6335603da34 100644 --- a/drivers/accel/amdxdna/amdxdna_ubuf.h +++ b/drivers/accel/amdxdna/amdxdna_ubuf.h @@ -5,10 +5,10 @@ #ifndef _AMDXDNA_UBUF_H_ #define _AMDXDNA_UBUF_H_ -#include <drm/drm_device.h> -#include <linux/dma-buf.h> +#include "amdxdna_gem.h" +#include "amdxdna_pci_drv.h" -struct dma_buf *amdxdna_get_ubuf(struct drm_device *dev, - u32 num_entries, void __user *va_entries); +struct amdxdna_gem_obj *amdxdna_alloc_ubuf_bo(struct amdxdna_client *client, + u32 num_entries, void __user *va_entries); #endif /* _AMDXDNA_UBUF_H_ */ -- 2.34.1

1 month

Re: [PATCH v6 18/18] RDMA/mlx5: Ask P2PDMA whether ATS takes a direct peer-to-peer route

by Thomas Hellström

On Mon, 2026-09-21 at 10:10 -0300, Jason Gunthorpe wrote: > On Mon, Sep 21, 2026 at 08:40:41AM +0200, Thomas Hellström wrote: > > On Fri, 2026-09-18 at 14:05 -0300, Jason Gunthorpe wrote: > > > On Fri, Sep 18, 2026 at 03:42:28PM +0200, Thomas Hellström wrote: > > > > > > > > 1) Xe attachment check if pci_p2pdma_distance() returns OK for > > > > the > > > > path. Then Xe always sets up dma-addresses using > > > > dma_map_resource(). > > > > > > Open coding pci_p2pdma_distance() in drivers is a hack. Using > > > dma_map_resource() like this was never "allowed". > > > > > > We've fixed things so these hacks are not needed, the drivers > > > need to > > > move over to things like dma_buf_phys_vec_to_sgt() and the hmm > > > helpers > > > to use the DMA API correctly. > > > > > > > 1a) Let's say the importer (Xe) has ATS enabled, and the > > > > exporter > > > > is > > > > supposed to map dma_addresses. > > > > In between there is a switch that allows direct traffic. > > > > Now with your patch series, How would the exporter know that Xe > > > > has > > > > ATS > > > > enabled and therefore should return IOVA mappings rather than > > > > the > > > > BUS > > > > addresses representing the shortcut. > > > > > > The thing that does the DMA map must be aware of what PCI TLPs > > > the > > > PCIe initiator will use to compute the proper P2P mapping mode. > > > > > > ATS always needs an IOVA but that still doesn't mean the P2P will > > > work. > > > > > > > It seems to me that a pci-device settable flag "ATS always > > > > enabled" > > > > should be enough to fix both issues? > > > > > > It should be be per-mapping to support the NIC workflow that > > > isn't a > > > global operation. > > > > OK. It looked to me from the series like the NIC workflow was to > > adapt > > its ATS settings per mapping depending on the p2p mapping type > > returned > > rather than vice versa. If so it would just clear "ATS always > > enabled". > > > > Admittedly a per-mapping flag would be more flexible, but also > > would > > require changed internal APIs? > > Ah you didn't send this to the list, was it intentional? No, Just hit the wrong reply-button. +lists again. > > mlx5 has both cases, some DMA is flexable and can do either, some is > non-ATS only. OK. > > Jason Thanks, Thomas

1 month

[PATCH v6 00/18] PCI/P2PDMA: Route peer-to-peer DMA by TLP class

by Leon Romanovsky

PCI P2PDMA applies Request and Completion Redirect throughout both paths. This misclassifies asymmetric and nested switches, and reports one answer for every kind of TLP. Three ACS controls act on TLP attributes the client chooses rather than on the topology: Translation Blocking and Direct Translated P2P act on a Request's Address Type, and Completion Redirect skips Completions carrying Relaxed Ordering. Evaluate each direction at the path divergence, decide every class from the one walk, expose the provider to dma-buf importers, and let mlx5 ask rather than assume. Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com> --- Changes in v6: - Changed DMABUF to use callbacks and not directly stored pointer. - Link to v5: https://patch.msgid.link/20260910-fix-p2p-acs-v4-0-v5-0-856087f63c0d@nvidia… Changes in v5: - Rebase on the posted fixes. - Dropped tags from changed patches. - Remove Egress Control Vector interpretation and coverage. - Keep enabled Egress Control conservative as a Request redirect. - Use pci_dbg()/dev_dbg() for diagnostics and drop the "debug" prefix. - Removed code comments from "Document the pdev->p2pdma lifetime and RCU rules" patch and reduced description to actual lifetime explanation. - Added note that Linux assumes that TLPs are in strict-ordering and untranslated. - Added code to calculate p2p paths per-TLP type. - Converted mlx5 to use that new proposed API. - Link to https://patch.msgid.link/20260821-fix-p2p-acs-v4-0-v4-0-94426b96de73@nvidia… Changes in v4: - Reject ACS Violations and unreadable routing state instead of treating them as host-bridge redirects - Added Tested-by tags from Tushar Dave - Added support to asymmetric ACS routing - Limited redirect checks to the two ports at the path divergence - Added standalone ACS routing diagnostics for hardware retesting - Dropped " PCI: Account for Direct Translated P2P in ACS isolation checks" patch - Link to v3: https://patch.msgid.link/20260811-fix-p2p-acs-v3-0-efc488ee7c03@nvidia.com Changes in v3: - Fixed pci_p2pdma_add_resource() error unwinding - Made pdev->p2pdma teardown wait unconditionally for RCU readers - Restricted pci_p2pmem_find_many() to pool-backed providers - Documented the pdev->p2pdma lifetime and RCU rules - Fixed calc_map_type_and_dist() handling of the verbose argument - Required the ACS port and target to share a bus before indexing the Egress Control Vector - Gave pci_acs_enabled() and pci_acs_path_enabled() a scope, so the ACS Direct Translated P2P rule no longer stops pci_enable_pasid() from enabling PASID - Dropped "Report ACS ports when the paths share no upstream bridge": the mapping type cannot change without a shared upstream bridge, so the pci=disable_acs_redir= hint was not actionable there and the ACS walk only cost config space reads - Folded the Request Redirect rule into pci_acs_rr_ineffective(), so pci_acs_flags_enabled() and the Intel SPT PCH quirk share one copy - Renamed pci_acs_egress_ctrl_set() to pci_acs_egress_ctrl_is_set(), it reads the bit rather than setting it - Reworded the blocked-path warning: ACS may also leave the direct route indeterminate rather than blocked - Added KUnit coverage for the shared-bus guard, a device with no ACS capability and an unreadable ACS Control register - Added the missing Fixes: tags, a second one on the pci_p2pdma_add_resource() unwinding fix (the dangling devres action dates to f58ef9d1d135) and one on the Egress Control isolation change - Link to v2: https://patch.msgid.link/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com Changes in v2: - Added Logan's ROB tags - Added commas in Documentation patch - Link to v1: https://patch.msgid.link/20260802-fix-p2p-acs-v1-0-a7c5eb64fff6@nvidia.com --- Leon Romanovsky (18): PCI/P2PDMA: Document pdev->p2pdma lifetime rules PCI/P2PDMA: Document the TLP attribute assumptions PCI/P2PDMA: Derive routing from directional ACS controls PCI: Reject unreadable ACS controls in isolation checks PCI/P2PDMA: Evaluate ACS controls at the path divergence PCI/P2PDMA: Document directional ACS routing PCI/P2PDMA: Collect the path's ACS controls before deciding PCI/P2PDMA: Answer routing per TLP class PCI/P2PDMA: Route Relaxed Ordering Completions directly PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking PCI/P2PDMA: Route Translated Requests under Direct Translated P2P PCI/P2PDMA: Log detailed ACS routing diagnostics PCI/P2PDMA: Add KUnit tests for the ACS routing decisions PCI/P2PDMA: Test the ACS P2P routing walk PCI: Add KUnit coverage for ACS isolation checks PCI/P2PDMA: Document TLP-class routing dma-buf: Let importers ask how peer-to-peer traffic is routed RDMA/mlx5: Ask P2PDMA whether ATS takes a direct peer-to-peer route Documentation/admin-guide/kernel-parameters.txt | 9 +- Documentation/driver-api/pci/p2pdma.rst | 69 +++ drivers/dma-buf/dma-buf-mapping.c | 32 ++ drivers/infiniband/core/uverbs_std_types_dmabuf.c | 12 + drivers/infiniband/hw/mlx5/mlx5_ib.h | 36 +- drivers/infiniband/hw/mlx5/mr.c | 40 ++ drivers/pci/Kconfig | 15 + drivers/pci/Makefile | 1 + drivers/pci/p2pdma.c | 637 +++++++++++++++++++--- drivers/pci/pci.c | 7 +- drivers/pci/pci.h | 26 + drivers/pci/pci_acs_test.c | 609 +++++++++++++++++++++ drivers/pci/quirks.c | 6 +- drivers/vfio/pci/vfio_pci_dmabuf.c | 12 + include/linux/dma-buf-mapping.h | 3 + include/linux/dma-buf.h | 17 + include/linux/pci-p2pdma.h | 57 +- 17 files changed, 1467 insertions(+), 121 deletions(-) --- base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72 change-id: 20260821-fix-p2p-acs-v4-0-e72455e3a261 prerequisite-message-id: <20260830-batch-p2p-fixes-v1-0-5044e8dfbe2e(a)nvidia.com> prerequisite-patch-id: 6b25c7fcf164cdfc14e9fac5b908d97fcf6509d7 prerequisite-patch-id: 0d083c281001365aae4b35544cf28891a6ab9a96 prerequisite-patch-id: bfd9dabf271f3cc9a3a61f46387d20c20311363d prerequisite-patch-id: fad0275efc722830fc591509506c0a5e4f581073 prerequisite-patch-id: 0c83bee688fec1f6d1564654df7c630fa6a4a978 Best regards, -- Leon Romanovsky <leonro(a)nvidia.com>

1 month

Re: [PATCH v6 5/9] vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA

by Matt Evans

Hi Longfang, On 15/09/2026 13:16, liulongfang wrote: > On 2026/9/12 5:41, Matt Evans wrote: >> This helper, vfio_pci_core_mmap_prep_dmabuf(), creates a single-range >> DMABUF for the purpose of mapping a PCI BAR. This is used in a future >> commit by VFIO's ordinary mmap() path. >> >> This function transfers ownership of the VFIO device fd to the >> DMABUF, which fput()s when it's released. >> >> Refactor the existing vfio_pci_core_feature_dma_buf() to split out >> export code common to the two paths, VFIO_DEVICE_FEATURE_DMA_BUF and >> this new VFIO_BAR mmap(). >> >> By exchanging the VMA file, we lose the original device path in >> /proc/<pid>/maps, lsof, etc. Generate a debug-oriented synthetic >> 'filename' for BAR mappings based on the cdev, plus BDF, plus resource >> index. (This does not apply to explicitly-exported DMABUFs which are >> named by DMA_BUF_SET_NAME.) >> >> Signed-off-by: Matt Evans <matt(a)ozlabs.org> >> --- >> drivers/vfio/pci/vfio_pci_dmabuf.c | 211 +++++++++++++++++++++++------ >> drivers/vfio/pci/vfio_pci_priv.h | 5 + >> 2 files changed, 171 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c >> index 9f10b10fc436..faa9239e66f8 100644 >> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c >> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c >> @@ -3,6 +3,7 @@ >> */ >> #include <linux/dma-buf-mapping.h> >> #include <linux/pci-p2pdma.h> >> +#include <linux/dma-buf.h> >> #include <linux/dma-resv.h> >> >> #include "vfio_pci_priv.h" >> @@ -82,6 +83,8 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) >> up_write(&priv->vdev->dmabuf_lock); >> vfio_device_put_registration(&priv->vdev->vdev); >> } >> + if (priv->vfile) >> + fput(priv->vfile); >> kfree(priv->phys_vec); >> kfree(priv); >> } >> @@ -246,6 +249,167 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_core_device *vdev, >> return ret; >> } >> >> +/* >> + * Create a DMABUF corresponding to priv, add it to vdev->dmabufs list >> + * for tracking (meaning cleanup or revocation will zap it), and take >> + * a vfio_device registration. >> + */ >> +static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, >> + struct vfio_pci_dma_buf *priv, u32 flags) >> +{ >> + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); >> + >> + if (!vfio_device_try_get_registration(&vdev->vdev)) >> + return -ENODEV; >> + >> + exp_info.ops = &vfio_pci_dmabuf_ops; >> + exp_info.size = priv->size; >> + exp_info.flags = flags; >> + exp_info.priv = priv; >> + >> + priv->dmabuf = dma_buf_export(&exp_info); >> + if (IS_ERR(priv->dmabuf)) { >> + vfio_device_put_registration(&vdev->vdev); >> + return PTR_ERR(priv->dmabuf); >> + } >> + >> + kref_init(&priv->kref); >> + init_completion(&priv->comp); >> + >> + /* dma_buf_put() now frees priv */ >> + INIT_LIST_HEAD(&priv->dmabufs_elm); >> + >> + /* >> + * dmabuf_lock synchronises access (R) or updates (W) to the >> + * vdev->dmabufs list and to bars_revoked (see below). The >> + * revocation state of DMABUF elements in the list is written >> + * holding both dmabuf_lock(W) and resv, and tested with >> + * either. >> + * >> + * (memory_lock, if held ->) dmabuf_lock -> resv >> + * >> + * NOTE: memory_lock is strictly avoided here, to avoid a >> + * dependency on memory_lock when mmap_lock is held, when >> + * mmap() leads to export. vfio-pci variant drivers are >> + * permitted to hold memory_lock across actions that might >> + * fault (such as user access); a deadlock could result when >> + * that fault path attempts to take mmap_lock (if held by an >> + * export waiting for memory_lock). >> + * >> + * vdev->bars_revoked tracks the BAR revocation status updated >> + * via vfio_pci_dma_buf_move(), so the initial DMABUF state >> + * follows the same criteria that later update the DMABUF >> + * state (BAR zap, etc.). >> + */ >> + lockdep_assert_not_held(&vdev->memory_lock); >> + >> + down_write(&vdev->dmabuf_lock); >> + dma_resv_lock(priv->dmabuf->resv, NULL); >> + priv->revoked = vdev->bars_revoked; >> + list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs); >> + dma_resv_unlock(priv->dmabuf->resv); >> + up_write(&vdev->dmabuf_lock); >> + >> + return 0; >> +} >> + >> +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, >> + struct vm_area_struct *vma, >> + u64 phys_start, u64 req_len, >> + unsigned int res_index) >> +{ >> + struct vfio_pci_dma_buf *priv; >> + unsigned long vma_pgoff = vma->vm_pgoff & (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT); >> + char *bufname; >> + int ret; >> + >> + priv = kzalloc_obj(*priv); >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->phys_vec = kzalloc_obj(*priv->phys_vec); >> + if (!priv->phys_vec) { >> + ret = -ENOMEM; >> + goto err_free_priv; >> + } >> + >> + /* >> + * Maximum size of the friendly debug name is >> + * vfio1048575:ffff:ff:1f.7/5 = 26. This fits within >> + * DMA_BUF_NAME_LEN, so dma_buf_set_name() below won't fail. >> + */ >> + bufname = kasprintf(GFP_KERNEL, "%s:%s/%x", >> + dev_name(&vdev->vdev.device), pci_name(vdev->pdev), >> + res_index); >> + >> + if (!bufname) { >> + ret = -ENOMEM; >> + goto err_free_phys; >> + } >> + >> + /* >> + * The DMABUF begins from the mmap()'s BAR offset, i.e. the >> + * start of the VMA corresponds to byte 0 of the DMABUF and >> + * byte (vma_pgoff << PAGE_SHIFT) of the BAR. >> + * >> + * vfio_pci_dma_buf_find_pfn() reverses this offset using >> + * vma_pgoff_adjust, so that ultimately a fault's offset from >> + * the start of the _VMA_ has a consistent usage whether the >> + * VMA originates from an mmap() of the VFIO device here or a >> + * direct DMABUF mmap(). Note vma_pgoff_adjust also includes >> + * the encoded VFIO region index, which cancels out the index >> + * encoded in vm_pgoff. >> + */ >> + priv->vdev = vdev; >> + priv->size = req_len; >> + priv->nr_ranges = 1; >> + priv->vma_pgoff_adjust = vma->vm_pgoff; >> + >> + priv->provider = pcim_p2pdma_provider(vdev->pdev, res_index); >> + if (!priv->provider) { >> + ret = -EINVAL; >> + goto err_free_name; >> + } >> + >> + priv->phys_vec[0].paddr = phys_start + ((u64)vma_pgoff << PAGE_SHIFT); >> + priv->phys_vec[0].len = priv->size; >> + >> + ret = vfio_pci_dmabuf_export(vdev, priv, O_RDWR); >> + if (ret) >> + goto err_free_name; >> + > > In the current patch, the PCIe device's BAR2 configuration space can be mapped as a DMABUF. > However, on an OS with a 64K page size, if a VF device's BAR2 is smaller than 64K, > a problem arises where the space is forced to page-align to 64K, it will causing the VM to > access memory beyond the actual size of the VF device's BAR2 space. > > How does your solution handle these cases where the BAR2 space is smaller than the Host OS's page size? Even on a 4K host, there can be BARs < PAGE_SIZE so 64K (or 16K) hosts aren't a new case. These small BARs cannot be mmap()ed and DMABUFs cannot be exported from them. (vfio_pci_core_mmap() errors out when !bar_mmap_supported[index]. And, a DMABUF needs to be an aligned multiple of PAGE_SIZE, plus vfio_pci_core_fill_phys_vec() won't allow a DMABUF to be created off the end of a BAR.) So, although this series allows a DMABUF to be mmap()ed, the preexisting checks prevent a sub-page DMABUF from existing and so there is no new route to mapping a sub-page BAR. What's the concern on BAR2 specifically, out of interest? This logic is applied to all resources equally, and tests pci_resource_len(...) so there shouldn't be a PF/VF distinction either. Matt > > Thanks. > Longfang. > >> + if (dma_buf_set_name(priv->dmabuf, bufname)) { >> + /* Shouldn't happen, but don't leak if it does: */ >> + dev_dbg_ratelimited(&vdev->pdev->dev, >> + "Failed to set map name '%s'\n", >> + bufname); >> + kfree(bufname); >> + } >> + >> + /* >> + * Ownership of the DMABUF file transfers to the VMA so that >> + * other users can locate the DMABUF via a VA. Ownership of >> + * the original VFIO device file being mmap()ed transfers to >> + * priv, and is put when the DMABUF is released. This >> + * intentionally does not use get_file()/vma_set_file() >> + * because the references are already held, and ownership >> + * moves. >> + */ >> + priv->vfile = vma->vm_file; >> + vma->vm_file = priv->dmabuf->file; >> + vma->vm_private_data = priv; >> + >> + return 0; >> + >> +err_free_name: >> + kfree(bufname); >> +err_free_phys: >> + kfree(priv->phys_vec); >> +err_free_priv: >> + kfree(priv); >> + return ret; >> +} >> + >> /* >> * This is a temporary "private interconnect" between VFIO DMABUF and iommufd. >> * It allows the two co-operating drivers to exchange the physical address of >> @@ -364,7 +528,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, >> { >> struct vfio_device_feature_dma_buf get_dma_buf = {}; >> struct vfio_region_dma_range *dma_ranges; >> - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); >> struct vfio_pci_dma_buf *priv; >> size_t length; >> int ret; >> @@ -424,49 +587,9 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, >> kfree(dma_ranges); >> dma_ranges = NULL; >> >> - if (!vfio_device_try_get_registration(&vdev->vdev)) { >> - ret = -ENODEV; >> + ret = vfio_pci_dmabuf_export(vdev, priv, get_dma_buf.open_flags); >> + if (ret) >> goto err_free_phys; >> - } >> - >> - exp_info.ops = &vfio_pci_dmabuf_ops; >> - exp_info.size = priv->size; >> - exp_info.flags = get_dma_buf.open_flags; >> - exp_info.priv = priv; >> - >> - priv->dmabuf = dma_buf_export(&exp_info); >> - if (IS_ERR(priv->dmabuf)) { >> - ret = PTR_ERR(priv->dmabuf); >> - goto err_dev_put; >> - } >> - >> - kref_init(&priv->kref); >> - init_completion(&priv->comp); >> - >> - /* dma_buf_put() now frees priv */ >> - INIT_LIST_HEAD(&priv->dmabufs_elm); >> - >> - /* >> - * dmabuf_lock synchronises access (R) or updates (W) to the >> - * vdev->dmabufs list and to bars_revoked (see below). The >> - * revocation state of DMABUF elements in the list is written >> - * holding both dmabuf_lock(W) and resv, and tested with >> - * either. >> - * >> - * dmabuf_lock -> resv >> - * >> - * vdev->bars_revoked tracks the BAR revocation status updated >> - * via vfio_pci_dma_buf_move(), so the initial DMABUF state >> - * follows the same criteria that later update the DMABUF >> - * state (BAR zap, etc.). >> - */ >> - down_write(&vdev->dmabuf_lock); >> - dma_resv_lock(priv->dmabuf->resv, NULL); >> - priv->revoked = vdev->bars_revoked; >> - list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs); >> - dma_resv_unlock(priv->dmabuf->resv); >> - up_write(&vdev->dmabuf_lock); >> - >> /* >> * dma_buf_fd() consumes the reference, when the file closes the dmabuf >> * will be released. >> @@ -477,8 +600,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, >> >> return ret; >> >> -err_dev_put: >> - vfio_device_put_registration(&vdev->vdev); >> err_free_phys: >> kfree(priv->phys_vec); >> err_free_priv: >> diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h >> index 48d9f574a3df..3ec676e12e21 100644 >> --- a/drivers/vfio/pci/vfio_pci_priv.h >> +++ b/drivers/vfio/pci/vfio_pci_priv.h >> @@ -30,6 +30,7 @@ struct vfio_pci_dma_buf { >> size_t size; >> struct phys_vec *phys_vec; >> struct p2pdma_provider *provider; >> + struct file *vfile; >> u32 nr_ranges; >> struct kref kref; >> struct completion comp; >> @@ -134,6 +135,10 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_core_device *vdev, >> unsigned long address, >> unsigned int order, >> unsigned long *out_pfn); >> +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, >> + struct vm_area_struct *vma, >> + u64 phys_start, u64 req_len, >> + unsigned int res_index); >> >> #ifdef CONFIG_VFIO_PCI_DMABUF >> int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, >>

1 month

Re: [PATCH] dma-buf: don't WARN_ON_ONCE() on a failing dmabuf->ops->vmap()

by Christian König

On 9/21/26 12:09, Janghyuck Kim wrote: > dma_buf_vmap() escalates any non-zero return from dmabuf->ops->vmap() > into a WARN_ON_ONCE(). But a failing vmap() is not, by itself, > evidence of a violated invariant or a driver bug -- it's an ordinary > runtime failure of a fallible operation, no different from any other > allocation-shaped kernel API that can return an error. The dma-buf > core has no visibility into what a given exporter's vmap() > implementation considers a normal failure versus an internal bug; > that distinction, if it matters at all, belongs to the exporter, not > to this generic passthrough. Well not quite, apart from some not so relevant HW workarounds the vmap callback is mostly used for fbdev emulation. > -ENOMEM is a concrete, easily reproduced example of this. Page > allocation on the vmap()/vmalloc() path can fail purely because the > calling task has a fatal signal pending: should_reclaim_retry() in > mm/page_alloc.c intentionally gives up reclaim and returns false > once fatal_signal_pending(current) is true, so that a task that is > already being killed (e.g. by a userspace low-memory killer) isn't > stuck retrying reclaim on its way out. The resulting -ENOMEM from > dmabuf->ops->vmap() is expected behaviour in that case, not a bug in > the exporter -- and it is far from the only way ops->vmap() can > legitimately fail (an exporter may just as well refuse to vmap a > buffer for other, unrelated reasons of its own). > > This is exactly the kind of condition Documentation/process/coding-style.rst > warns against for WARN*(): > > WARN*() must not be used for a condition that is expected to > trigger easily, for example, by user space actions. > > A failing vmap() is reachable purely by driving the system into low > memory and then issuing an ioctl that happens to map a dma-buf -- no > driver misbehaviour required. Turning that into a WARN_ON_ONCE(), > and on panic_on_warn kernels into an outright panic, doesn't help > debugging; it just turns an otherwise-recoverable error into a fatal > one. WOW STOP! A driver allowing to vmap() through an IOCTL is a major no-go! vmap() on a DMA-buf has very very limited use cases which should mostly only be trigger able as superuser. At least on 32bit system you can otherwise trivially exhaust the vmap area of the kernel and that is even worse than a simple WARN_ON_ONCE(). Regards, Christian. > > Drop the WARN_ON_ONCE() and simply propagate the error, consistent > with how other allocation-failure paths in the kernel are handled. > The dma_buf_vmap_unlocked()/dma_buf_vmap() contract is unchanged: > both still return the negative errno from dmabuf->ops->vmap() to the > caller. Only the WARN_ON_ONCE() splat (and, transitively, the panic > on panic_on_warn kernels) is removed. > > Signed-off-by: Janghyuck Kim <janghyuck.kim(a)samsung.com> > --- > drivers/dma-buf/dma-buf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 36cdc342627e..252495761a21 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1670,7 +1670,7 @@ int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map) > BUG_ON(iosys_map_is_set(&dmabuf->vmap_ptr)); > > ret = dmabuf->ops->vmap(dmabuf, &ptr); > - if (WARN_ON_ONCE(ret)) > + if (ret) > return ret; > > dmabuf->vmap_ptr = ptr; > -- > 2.34.1 >

1 month

← Newer
1
...
8
9
10
11
12
13
14
...
368
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig