Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

9 participants
3613 discussions

[PATCH v8 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. This is achieved by allowing the processes to mmap() the DMABUFs; their access to the device is isolated to the exported ranges. The primary userspace driver process can forcibly revoke access to previously-shared buffers upon cleanup (without requiring cooperation from the subordinate processes). This is an improvement on sharing the VFIO device fd to subordinate processes, which would allow unfettered access. See the RFCs for background. The existing VFIO PCI BAR mmap() becomes backed by a DMABUF too, keeping common vm_ops and fault handler for VMAs from 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. Below are per-patch notes & background info, and at the bottom are several related questions that reviewers may like to consider (worth at least skipping to #1, a possible bug). Notes on patches ================ 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. It's also worth noting that one reason for nvgrace-gpu's special BAR handling is to map RESMEM with a WC memory type. This is unchanged for existing mmap() but WC is not currently supported for mapping DMABUFs exported for that BAR; all DMABUF VMAs currently have the same memory type and control of UC/WC for a mapping is future work. vfio/pci: Remove DMABUF export dependency on vdev->memory_lock In v5 of this series 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: https://lore.kernel.org/kvm/9a615f22-c0d4-46ae-9654-db11e94e5fec@ozlabs.org/ 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(R) 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() to remove 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: Provide 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. 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: Revoke a DMABUF on request from userspace 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 used 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 prior mmap_lock->memory_lock issue. END === This is based on v7.3-rc6 These commits are on GitHub for easier browsing, along with "[RFC ONLY] selftests: vfio: Add standalone vfio_dmabuf_mmap_test": https://github.com/evansm7/linux/compare/v7.3-rc6...dev/mev/vfio-dmabuf-mma… Also please see... https://lore.kernel.org/all/f8efaabd-c06e-4f04-8cfa-489c148e37ba@ozlabs.org/ ("[PATCH] dma-buf: Annul dmabuf->file on file release") ...which fixes a stale file pointer issue that could affect VFIO's DMABUF export. Although the issue exists prior to this series, the addition of another dmabuf->file dereference adds additional unpleasant failure modes (for example, unmap_mapping_range() the wrong file, or good old random data dereference, etc.) Thanks for reading, Matt ================================================================================ Changelog: v8: - Rebased, on v7.3-rc6 - "dma-buf: Provide dma_buf_set_name()": Comment and "DOC: locking convention" updates. dma_buf_set_name() is is listed in the "do not hold resv" category v7: https://lore.kernel.org/all/20260924152159.49702-1-matt@ozlabs.org/ v6: https://lore.kernel.org/all/20260911214200.33793-1-matt@ozlabs.org/ 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: Provide 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: Revoke a DMABUF on request from userspace drivers/dma-buf/dma-buf.c | 84 ++- 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 | 609 +++++++++++++++--- drivers/vfio/pci/vfio_pci_priv.h | 54 +- include/linux/dma-buf.h | 16 +- include/linux/vfio_pci_core.h | 3 + include/uapi/linux/vfio.h | 24 + 11 files changed, 870 insertions(+), 193 deletions(-) -- 2.47.3

30 minutes

[PATCH] dma-buf: Annul dmabuf->file on file release

by Matt Evans

The dmabuf release path is split between file and dentry release: dma_buf_file_release() is called shortly before the file is freed, and dma_buf_release() calls an exporter's dmabuf->ops->release when the dentry is freed. However, the dentry can outlive the file (for example, if opened with O_PATH), meaning .release might be called some time after the file is freed. This presents a window, when closing a DMABUF file, in which dmabuf->file points to freed memory yet .release op has not yet been called. For VFIO, if a buffer's .release has not been called it's considered still active and subject to move/cleanup. If so, it attempts to get_file_active() on dmabuf->file: a file close with dentry held open will call this function with a stale pointer, a UAF. To make this pattern safe, set dmabuf->file to NULL in dma_buf_file_release() to reflect that the associated file is now dead even if the DMABUF is not yet gone. A get_file_active() ... fput() sequence concurrent with a file close will only execute as one of: - Gets the file before file_ref_put() (dmabuf->file valid) - Observes dmabuf->file pointing to a file, but it's DEAD (no file) - Observes dmabuf->file = NULL (no file) Originally, drivers could assume dmabuf->file was valid until .release was called from fops->release. This assumption was no longer valid after 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops"), which moved the callback to the dentry release (by which point the file might have been freed). With this commit, drivers must still consider that dmabuf->file could be NULL before .release. Fixes: 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops") Signed-off-by: Matt Evans <matt(a)ozlabs.org> --- Hi, This issue was found (by Claude Opus 5.5) in the context of VFIO's DMABUF export path. VFIO iterates live DMABUFs with a get_file_active()/fput() block, which now becomes safe if the file is closed (and memory freed!) yet DMABUF .release hasn't yet occurred. However, there are a couple of other places that directly use dmabuf->file and seem able to race a closing file (i.e. without holding the file reference)? If this is so, they'd be a UAF today; with this patch that goes away, but instead of a stale pointer dmabuf->file could be NULL: 1. drivers/gpu/drm/vmwgfx/ttm_object.c:get_dma_buf_unless_doomed() file_ref_get(&dmabuf->file->f_ref); on a non-refcounted DMABUF The commit message of 90ee6ed776c0 ("fs: port files to file_ref") hints this might be more subtle than replacing it with a get_file() variant (so as to accept a NULL file *). 2. drivers/gpu/drm/i915/gvt/dmabuf.c:intel_vgpu_get_dmabuf() gvt_dbg_dpy(... file_count(dmabuf->file) ...); Respective vmwgfx & i915 maintainers, what is your view? Or, indeed, if anyone sees any other questionable uses of dmabuf->file. Thanks, Matt drivers/dma-buf/dma-buf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 4c9add51f9ef..726639130477 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -193,11 +193,15 @@ static void dma_buf_release(struct dentry *dentry) static int dma_buf_file_release(struct inode *inode, struct file *file) { + struct dma_buf *dmabuf = file->private_data; + if (!is_dma_buf_file(file)) return -EINVAL; - __dma_buf_list_del(file->private_data); + __dma_buf_list_del(dmabuf); + /* Must be observed by __get_file_rcu() before file_free() */ + smp_store_mb(dmabuf->file, NULL); return 0; } -- 2.47.3

2 hours, 1 minute

[PATCH v9 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, and treat a client with ATS enabled as translating unless its driver declares per-mapping ATS. This completes the P2PDMA side; dma-buf and mlx5 follow separately. Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com> --- Changes in v9: - Removed dmabuf patches for now. - Removed mlx5 per-mapping ATS declaration for now; it comes back with the dmabuf patches. - Link to v8: https://patch.msgid.link/20260928-fix-p2p-acs-v4-0-v8-0-404453b9c435@nvidia… Changes in v8: - Added extra Reviewed-by from Logan. - Added patches to take care of ATS per-device vs. per-mapping option - Link to v7: https://patch.msgid.link/20260920-fix-p2p-acs-v4-0-v7-0-ca0828ab697c@nvidia… Changes in v7: - Removed "Document pdev->p2pdma lifetime rules" patch, it gives nothing after p2pmem fix. - Split dmabuf patch. - Tushar retested the series, so added his Tested-by. - Added Logan's ROB tags and fixed minor documentation issues pointed by him. - Link to v6: https://lore.kernel.org/all/20260914-fix-p2p-acs-v4-0-v6-0-5ef07ec9ef06@nvi… 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 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 PCI/P2PDMA: Let a client declare that it selects ATS per mapping PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled PCI/P2PDMA: Test the routing of clients with ATS enabled Documentation/admin-guide/kernel-parameters.txt | 15 +- Documentation/driver-api/pci/p2pdma.rst | 80 +++ drivers/pci/Kconfig | 15 + drivers/pci/Makefile | 1 + drivers/pci/p2pdma.c | 719 ++++++++++++++++++-- drivers/pci/pci.c | 7 +- drivers/pci/pci.h | 58 ++ drivers/pci/pci_acs_test.c | 860 ++++++++++++++++++++++++ drivers/pci/quirks.c | 6 +- include/linux/pci-p2pdma.h | 12 +- 10 files changed, 1687 insertions(+), 86 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>

2 hours, 56 minutes

Re: [PATCH] udmabuf: Disable the list length limit by default

by Christian König

On 10/6/26 02:48, Val Packett wrote: > Shared memory buffers for software-rendered GUI applications are often > large and highly fragmented in memory. To get a shareable handle to such > a buffer from a guest VM, a virtio-gpu device backend can use the > UDMABUF_CREATE_LIST ioctl with the memfds backing guest RAM and the list > of guest physical pages sent by the guest driver. That list reaches 80K > entries for a maximized window on a 4K display! > > Like 44e9eb5a7621 ("dma-buf/udmabuf: Disable the size limit by default") > did for the size limit, change the default to INT_MAX since the amount > of protection provided by this limit seems dubious. > > Signed-off-by: Val Packett <val(a)invisiblethingslab.com> > --- > 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 df6dd0046242..589031133588 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -16,9 +16,9 @@ > #include <linux/vmalloc.h> > #include <linux/iosys-map.h> > > -static int list_limit = 1024; > +static int list_limit = INT_MAX; > module_param(list_limit, int, 0644); > -MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is 1024."); > +MODULE_PARM_DESC(list_limit, "udmabuf_create_list->count limit. Default is INT_MAX."); This will completely overflow the size calculation in udmabuf_ioctl_create_list(). Please fix udmabuf_ioctl_create_list() to use memdup_array_user() and use a reasonable limit here. Something like 128k should probably do. Apart from that I think we need to start using huge and giant pages for virtio-gpu on the client side, we already had it multiple times that we hit limits with that in multiple places. Sharing 80k individual 4k pointers is really not very efficient. Regards, Christian. > > static int size_limit_mb = INT_MAX; > module_param(size_limit_mb, int, 0644);

1 day, 2 hours

Re: [RFC PATCH 2/6] dma-buf: Add get_phys() to describe a physical run

by Christian König

On 10/5/26 15:20, Fred Griffoul wrote: > On 10/5/26 Christian König wrote: >> Well filling page tables by the importer is an absolutely clear NO-GO >> for the DMA-buf design, we have gotten down that path already and it >> took us years to remove this functionality again. > > Understood, thanks for the quick and clear answers on this patch and on > patch 3. > >> The problem you are facing here is that dma_buf_mmap() doesn't work >> because you don't have a VMA. > > Right: the memory has no struct page and is deliberately not mapped in > the host, so there is no VMA to hand to KVM. > > I'll post a new version of this series. It drops get_phys(), the ranged > invalidation and the guest_memfd dma-buf import (patches 2-5), and it > does not touch drivers/dma-buf, as you suggested in the PAL discussion: > > https://lore.kernel.org/all/c413710b-4c28-4ed8-88ec-aeb8c4482011@amd.com/ > > KVM gets its frames through the KVM-private guest_memfd provider > interface from David's series. iommufd gets them through a private > interface with the exporter, as it already does for vfio-pci. The new > version adds a small registry in iommufd for that, which replaces the > symbol_get() of the sample in David's series. The dma-buf is then only > the handle and the existing revoke. Yeah that approach sounds totally sane to me. We have drivers which stuff all kind of resources, not just memory, into a DMA-buf. That ranges from MMIO BARs to trigger FW operations (doorbells) over full HW blocks (ordered appends, global wave sync etc...) where two or more applications need to share which one is used. That you then have a exporter private interface is for accessing this is perfectly ok. The problem you are facing here is more that this is not limited to one driver/module but multiple ones and you don't want module inter dependencies because of the symbols. So symbol_get() is probably the right thing to do as long as we don't have something like weak symbols or similar. Regards, Christian. > > Thanks, > Fred

1 day, 18 hours

Re: [RFC PATCH 3/6] dma-buf: Add ranged mapping invalidation

by Christian König

On 10/5/26 11:55, Fred Griffoul wrote: > From: Fred Griffoul <fgriffo(a)amazon.co.uk> > > dma_buf_invalidate_mappings() tells every importer that the whole > buffer changed. An exporter that changes one part of its memory cannot > say which bytes changed, so importers throw away mappings that are > still valid. > > Add an exporter helper that invalidates a byte range, and an importer > callback that receives it. The callback means that the address, the > attributes or the backing of the range changed. If part of the range is > no longer backed, get_phys() returns -ENOENT for it. Importers must stop > using their old answer before the callback returns. Importers that do > not implement the callback still receive a whole-buffer invalidation. Yeah that is exactly one of the reasons why we don't allow that. Clear NAK to the whole approach. See the reply to patch #2 for a detailed description. Regards, Christian. > > Signed-off-by: Fred Griffoul <fgriffo(a)amazon.co.uk> > --- > drivers/dma-buf/dma-buf.c | 30 ++++++++++++++++++++++++++++++ > include/linux/dma-buf.h | 16 ++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 66b85d53ed22..e7010163eb2f 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1389,6 +1389,36 @@ void dma_buf_invalidate_mappings(struct dma_buf *dmabuf) > } > EXPORT_SYMBOL_NS_GPL(dma_buf_invalidate_mappings, "DMA_BUF"); > > +/** > + * dma_buf_invalidate_mappings_range - notify attachments that a range changed > + * @dmabuf: buffer whose layout changed > + * @offset: first changed byte > + * @length: number of changed bytes > + * > + * Importers with a ranged callback stop using their old mappings of the range > + * before returning. Other importers receive the existing whole-buffer > + * callback, which is correct but coarser. The reservation lock must be held. > + */ > +void dma_buf_invalidate_mappings_range(struct dma_buf *dmabuf, > + unsigned long offset, > + unsigned long length) > +{ > + struct dma_buf_attachment *attach; > + > + dma_resv_assert_held(dmabuf->resv); > + list_for_each_entry(attach, &dmabuf->attachments, node) { > + const struct dma_buf_attach_ops *ops = attach->importer_ops; > + > + if (!ops) > + continue; > + if (ops->invalidate_mappings_range) > + ops->invalidate_mappings_range(attach, offset, length); > + else if (ops->invalidate_mappings) > + ops->invalidate_mappings(attach); > + } > +} > +EXPORT_SYMBOL_NS_GPL(dma_buf_invalidate_mappings_range, "DMA_BUF"); > + > /** > * dma_buf_get_phys - describe the run that starts at an offset > * @attach: attachment to query > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index b223962e20c2..55c3fe60a0ba 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -485,6 +485,19 @@ struct dma_buf_attach_ops { > * required behavior. > */ > void (*invalidate_mappings)(struct dma_buf_attachment *attach); > + > + /** > + * @invalidate_mappings_range: [optional] a byte range changed > + * > + * The exporter changed the address, attributes or backing of > + * [@offset, @offset + @length). The importer must stop using its old > + * answer for that range before returning. > + * Importers without this callback receive @invalidate_mappings for > + * the whole buffer instead. > + */ > + void (*invalidate_mappings_range)(struct dma_buf_attachment *attach, > + unsigned long offset, > + unsigned long length); > }; > > /** > @@ -600,6 +613,9 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *, > void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *, > enum dma_data_direction); > void dma_buf_invalidate_mappings(struct dma_buf *dma_buf); > +void dma_buf_invalidate_mappings_range(struct dma_buf *dma_buf, > + unsigned long offset, > + unsigned long length); > bool dma_buf_attach_revocable(struct dma_buf_attachment *attach); > /* bits 0-7: memory type (a value, not flags) */ > #define DMA_BUF_PHYS_ATTR_TYPE_MASK GENMASK(7, 0) > -- > 2.47.3 >

1 day, 23 hours

Re: [RFC PATCH 2/6] dma-buf: Add get_phys() to describe a physical run

by Christian König

On 10/5/26 11:55, Fred Griffoul wrote: > From: Fred Griffoul <fgriffo(a)amazon.co.uk> > > iommufd and KVM write physical addresses into their own page tables. > To do so, they must ask the exporter which frames back an offset of a > dma-buf, whether that memory is RAM or MMIO, and whether it may be > written. Well filling page tables by the importer is an absolutely clear NO-GO for the DMA-buf design, we have gotten down that path already and it took us years to remove this functionality again. The problem you are facing here is that dma_buf_mmap() doesn't work because you don't have a VMA. I can understand the reasoning that you don't want to have a VMA, but I don't think that this is a valid justification to add complexity to DMA-buf and especially bring an approach back which we have already deprecated. A possible solution might be Jasons patch set to directly negotiate exposing PCI BAR regions as DMA-buf, but that certainly needs more discussion. But the approach outlined here is an absolutely clear NAK from my side. Regards, Christian. > > Add a get_phys() operation. The importer passes an offset and a maximum > length. The exporter reports one run: the frames that start at the > offset, are backed and physically contiguous, and share one attribute > word. The run never exceeds the length. The exporter may end it early, > so importers must not assume that it is the longest possible run. > > get_phys() returns -ENOENT when the byte at the offset is not backed, > and -ENODEV when the buffer is revoked. The caller holds the > reservation, and either pins the attachment or handles revocation. A > reported frame stays valid until an invalidation that covers it > returns. > > The attribute word holds the memory type and a READONLY flag. Zero > means writable RAM. Importers refuse unknown types, reserved bits and > unknown flags, so attributes added later fail safely. Two flag bits are > reserved: one for holes and one for confidential memory. > > Convert vfio-pci, the iommufd selftest exporter and the KVM sample. > iommufd behaves as before: it maps a buffer only when one writable run > covers all of it. > > Signed-off-by: Fred Griffoul <fgriffo(a)amazon.co.uk> > --- > drivers/dma-buf/dma-buf.c | 47 +++++++++++ > drivers/iommu/iommufd/iommufd_private.h | 8 -- > drivers/iommu/iommufd/iommufd_test.h | 16 ++++ > drivers/iommu/iommufd/pages.c | 74 ++++------------- > drivers/iommu/iommufd/selftest.c | 106 ++++++++++++++++++++---- > drivers/vfio/pci/vfio_pci_dmabuf.c | 58 ++++++------- > include/linux/dma-buf.h | 54 ++++++++++++ > include/linux/vfio_pci_core.h | 3 - > samples/kvm/gmem_provider.c | 39 ++++----- > 9 files changed, 267 insertions(+), 138 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index d504c636dc29..66b85d53ed22 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1389,6 +1389,53 @@ void dma_buf_invalidate_mappings(struct dma_buf *dmabuf) > } > EXPORT_SYMBOL_NS_GPL(dma_buf_invalidate_mappings, "DMA_BUF"); > > +/** > + * dma_buf_get_phys - describe the run that starts at an offset > + * @attach: attachment to query > + * @offset: first buffer byte to describe > + * @len: maximum number of bytes to describe > + * @phys: physical address and length of the run > + * @attr: DMA_BUF_PHYS_ATTR_* word of the run > + * > + * A run is the longest stretch of backed bytes starting at @offset whose > + * frames are physically contiguous and share one attribute word. On success > + * *@phys starts at the byte at @offset and covers at most @len bytes; it may > + * be shorter than the run. > + * > + * The dma-buf reservation must be held. The attachment must be pinned or have > + * revocable importer operations. A frame remains valid until a covering > + * invalidation callback returns; the exporter must invalidate a changed range > + * before reusing its old frames. > + * > + * Returns: > + * > + * 0 on success, -ENOENT if the byte at @offset is not backed, -ENODEV if the > + * buffer is revoked, -EOPNOTSUPP if the exporter cannot describe itself this > + * way, or another negative error code. > + */ > +int dma_buf_get_phys(struct dma_buf_attachment *attach, u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr) > +{ > + u64 end; > + int ret; > + > + if (WARN_ON_ONCE(!attach || !attach->dmabuf || !phys || !attr)) > + return -EINVAL; > + if (!len || check_add_overflow(offset, len, &end) || > + end > attach->dmabuf->size) > + return -EINVAL; > + > + dma_resv_assert_held(attach->dmabuf->resv); > + if (!attach->dmabuf->ops->get_phys) > + return -EOPNOTSUPP; > + > + ret = attach->dmabuf->ops->get_phys(attach, offset, len, phys, attr); > + if (!ret && WARN_ON_ONCE(!phys->len || phys->len > len)) > + return -EIO; > + return ret; > +} > +EXPORT_SYMBOL_NS_GPL(dma_buf_get_phys, "DMA_BUF"); > + > /** > * DOC: cpu access > * > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h > index 43fbc5bed8de..5cded585c227 100644 > --- a/drivers/iommu/iommufd/iommufd_private.h > +++ b/drivers/iommu/iommufd/iommufd_private.h > @@ -716,8 +716,6 @@ bool iommufd_should_fail(void); > int __init iommufd_test_init(void); > void iommufd_test_exit(void); > bool iommufd_selftest_is_mock_dev(struct device *dev); > -int iommufd_test_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys); > #else > static inline void iommufd_test_syz_conv_iova_id(struct iommufd_ucmd *ucmd, > unsigned int ioas_id, > @@ -739,11 +737,5 @@ static inline bool iommufd_selftest_is_mock_dev(struct device *dev) > { > return false; > } > -static inline int > -iommufd_test_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys) > -{ > - return -EOPNOTSUPP; > -} > #endif > #endif > diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h > index 52b78cbcc920..28fd9c43edc4 100644 > --- a/drivers/iommu/iommufd/iommufd_test.h > +++ b/drivers/iommu/iommufd/iommufd_test.h > @@ -31,6 +31,8 @@ enum { > IOMMU_TEST_OP_PASID_CHECK_HWPT, > IOMMU_TEST_OP_DMABUF_GET, > IOMMU_TEST_OP_DMABUF_REVOKE, > + IOMMU_TEST_OP_MD_CHECK_MAPPED, > + IOMMU_TEST_OP_MD_IOVA_TO_PHYS, > }; > > enum { > @@ -193,6 +195,20 @@ struct iommu_test_cmd { > __s32 dmabuf_fd; > __u32 revoked; > } dmabuf_revoke; > + struct { > + /* > + * 1: every page in [iova, iova+length) must be mapped; > + * 0: none of them may be. Mixed is an error. > + */ > + __u32 mapped; > + __u32 __reserved; > + __aligned_u64 iova; > + __aligned_u64 length; > + } check_mapped; > + struct { > + __aligned_u64 iova; > + __aligned_u64 out_phys; /* 0 if unmapped */ > + } iova_to_phys; > }; > __u32 last; > }; > diff --git a/drivers/iommu/iommufd/pages.c b/drivers/iommu/iommufd/pages.c > index f9b2ae6d7e96..196d1bb330c2 100644 > --- a/drivers/iommu/iommufd/pages.c > +++ b/drivers/iommu/iommufd/pages.c > @@ -1463,68 +1463,12 @@ static const struct dma_buf_attach_ops iopt_dmabuf_attach_revoke_ops = { > .invalidate_mappings = iopt_revoke_notify, > }; > > -/* > - * iommufd and vfio have a circular dependency. Future work for a phys > - * based private interconnect will remove this. > - */ > -/* > - * Look up the exporter's phys accessor for iommufd's private-interconnect > - * path. Also fills *is_cpu_ram: true if the exporter's memory is normal > - * cache-coherent RAM (needs BATCH_CPU_MEMORY / IOMMU_CACHE), false for MMIO > - * (needs BATCH_MMIO / IOMMU_MMIO). This will be replaced by a formal > - * exporter op that returns phys + memory type together. > - */ > -static int > -sym_vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys, bool *is_cpu_ram) > -{ > - typeof(&vfio_pci_dma_buf_iommufd_map) fn; > - int rc; > - > - rc = iommufd_test_dma_buf_iommufd_map(attachment, phys); > - if (rc != -EOPNOTSUPP) { > - *is_cpu_ram = false; /* test hook mimics VFIO MMIO */ > - return rc; > - } > - > - /* > - * Prototype: try the sample gmem provider's dma-buf exporter. This > - * mirrors the vfio-pci private-interconnect hook, and (like it) is > - * meant to be replaced by a formal negotiated exporter op returning > - * phys + memory type. The provider serves RAM, so mark it CPU_RAM. > - */ > - { > - extern int gmem_provider_dma_buf_iommufd_map( > - struct dma_buf_attachment *, struct phys_vec *); > - typeof(&gmem_provider_dma_buf_iommufd_map) gfn; > - > - gfn = symbol_get(gmem_provider_dma_buf_iommufd_map); > - if (gfn) { > - rc = gfn(attachment, phys); > - symbol_put(gmem_provider_dma_buf_iommufd_map); > - if (rc != -EOPNOTSUPP) { > - *is_cpu_ram = true; > - return rc; > - } > - } > - } > - > - if (!IS_ENABLED(CONFIG_VFIO_PCI_DMABUF)) > - return -EOPNOTSUPP; > - > - fn = symbol_get(vfio_pci_dma_buf_iommufd_map); > - if (!fn) > - return -EOPNOTSUPP; > - rc = fn(attachment, phys); > - symbol_put(vfio_pci_dma_buf_iommufd_map); > - *is_cpu_ram = false; /* VFIO PCI dma-buf carries BAR (MMIO) memory */ > - return rc; > -} > - > static int iopt_map_dmabuf(struct iommufd_ctx *ictx, struct iopt_pages *pages, > struct dma_buf *dmabuf) > { > struct dma_buf_attachment *attach; > + struct phys_vec pv; > + u32 attr; > int rc; > > attach = dma_buf_dynamic_attach(dmabuf, iommufd_global_device(), > @@ -1546,10 +1490,20 @@ static int iopt_map_dmabuf(struct iommufd_ctx *ictx, struct iopt_pages *pages, > if (rc) > goto err_detach; > > - rc = sym_vfio_pci_dma_buf_iommufd_map(attach, &pages->dmabuf.phys, > - &pages->dmabuf.is_cpu_ram); > + /* One backed, writable run covering the buffer: refuse the rest. */ > + rc = dma_buf_get_phys(attach, 0, dmabuf->size, &pv, &attr); > + if (rc == -ENOENT) > + rc = -EOPNOTSUPP; > if (rc) > goto err_unpin; > + if (pv.len != dmabuf->size || !dma_buf_phys_attr_known(attr) || > + (attr & DMA_BUF_PHYS_ATTR_FLAGS_MASK)) { > + rc = -EOPNOTSUPP; > + goto err_unpin; > + } > + pages->dmabuf.phys = pv; > + pages->dmabuf.is_cpu_ram = > + dma_buf_phys_attr_type(attr) == DMA_BUF_PHYS_ATTR_RAM; > > dma_resv_unlock(dmabuf->resv); > > diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c > index af07c642a526..0899272d1e66 100644 > --- a/drivers/iommu/iommufd/selftest.c > +++ b/drivers/iommu/iommufd/selftest.c > @@ -1962,32 +1962,31 @@ static void iommufd_test_dma_buf_release(struct dma_buf *dmabuf) > kfree(priv); > } > > -static const struct dma_buf_ops iommufd_test_dmabuf_ops = { > - .attach = iommufd_test_dma_buf_attach, > - .detach = iommufd_test_dma_buf_detach, > - .map_dma_buf = iommufd_test_dma_buf_map, > - .release = iommufd_test_dma_buf_release, > - .unmap_dma_buf = iommufd_test_dma_buf_unmap, > -}; > - > -int iommufd_test_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys) > +static int iommufd_test_dma_buf_get_phys(struct dma_buf_attachment *attachment, > + u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr) > { > struct iommufd_test_dma_buf *priv = attachment->dmabuf->priv; > > dma_resv_assert_held(attachment->dmabuf->resv); > - > - if (attachment->dmabuf->ops != &iommufd_test_dmabuf_ops) > - return -EOPNOTSUPP; > - > if (priv->revoked) > return -ENODEV; > > - phys->paddr = virt_to_phys(priv->memory); > - phys->len = priv->length; > + phys->paddr = virt_to_phys(priv->memory) + offset; > + phys->len = len; > + *attr = DMA_BUF_PHYS_ATTR_MMIO; > return 0; > } > > +static const struct dma_buf_ops iommufd_test_dmabuf_ops = { > + .attach = iommufd_test_dma_buf_attach, > + .detach = iommufd_test_dma_buf_detach, > + .map_dma_buf = iommufd_test_dma_buf_map, > + .release = iommufd_test_dma_buf_release, > + .unmap_dma_buf = iommufd_test_dma_buf_unmap, > + .get_phys = iommufd_test_dma_buf_get_phys, > +}; > + > static int iommufd_test_dmabuf_get(struct iommufd_ucmd *ucmd, > unsigned int open_flags, > size_t len) > @@ -2031,6 +2030,73 @@ static int iommufd_test_dmabuf_get(struct iommufd_ucmd *ucmd, > return rc; > } > > +/* > + * Report the physical address the mock domain resolves @iova to, or 0 if > + * it is unmapped. Lets a test check that two IOVAs share one frame (a > + * scratch substitution) without knowing the frame in advance. > + */ > +static int iommufd_test_md_iova_to_phys(struct iommufd_ucmd *ucmd, > + unsigned int mockpt_id, > + unsigned long iova) > +{ > + struct iommu_test_cmd *cmd = ucmd->cmd; > + struct iommufd_hw_pagetable *hwpt; > + struct mock_iommu_domain *mock; > + unsigned int page_size; > + int rc; > + > + hwpt = get_md_pagetable(ucmd, mockpt_id, &mock); > + if (IS_ERR(hwpt)) > + return PTR_ERR(hwpt); > + > + page_size = 1 << __ffs(mock->domain.pgsize_bitmap); > + if (iova % page_size) { > + rc = -EINVAL; > + goto out_put; > + } > + cmd->iova_to_phys.out_phys = > + mock->domain.ops->iova_to_phys(&mock->domain, iova); > + rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd)); > +out_put: > + iommufd_put_object(ucmd->ictx, &hwpt->obj); > + return rc; > +} > + > +static int iommufd_test_md_check_mapped(struct iommufd_ucmd *ucmd, > + unsigned int mockpt_id, > + unsigned long iova, size_t length, > + bool mapped) > +{ > + struct iommufd_hw_pagetable *hwpt; > + struct mock_iommu_domain *mock; > + unsigned int page_size; > + int rc = 0; > + > + hwpt = get_md_pagetable(ucmd, mockpt_id, &mock); > + if (IS_ERR(hwpt)) > + return PTR_ERR(hwpt); > + > + page_size = 1 << __ffs(mock->domain.pgsize_bitmap); > + if (iova % page_size || length % page_size || !length) { > + rc = -EINVAL; > + goto out_put; > + } > + > + for (; length; length -= page_size, iova += page_size) { > + bool is_mapped = > + mock->domain.ops->iova_to_phys(&mock->domain, iova) != 0; > + > + if (is_mapped != mapped) { > + rc = -ENOENT; > + goto out_put; > + } > + } > + > +out_put: > + iommufd_put_object(ucmd->ictx, &hwpt->obj); > + return rc; > +} > + > static int iommufd_test_dmabuf_revoke(struct iommufd_ucmd *ucmd, int fd, > bool revoked) > { > @@ -2143,6 +2209,14 @@ int iommufd_test(struct iommufd_ucmd *ucmd) > return iommufd_test_dmabuf_revoke(ucmd, > cmd->dmabuf_revoke.dmabuf_fd, > cmd->dmabuf_revoke.revoked); > + case IOMMU_TEST_OP_MD_CHECK_MAPPED: > + return iommufd_test_md_check_mapped(ucmd, cmd->id, > + cmd->check_mapped.iova, > + cmd->check_mapped.length, > + cmd->check_mapped.mapped); > + case IOMMU_TEST_OP_MD_IOVA_TO_PHYS: > + return iommufd_test_md_iova_to_phys(ucmd, cmd->id, > + cmd->iova_to_phys.iova); > default: > return -EOPNOTSUPP; > } > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c > index c16f460c01d6..381c3d338e8c 100644 > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c > @@ -99,46 +99,46 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) > kfree(priv); > } > > -static const struct dma_buf_ops vfio_pci_dmabuf_ops = { > - .attach = vfio_pci_dma_buf_attach, > - .map_dma_buf = vfio_pci_dma_buf_map, > - .unmap_dma_buf = vfio_pci_dma_buf_unmap, > - .release = vfio_pci_dma_buf_release, > -}; > - > /* > - * This is a temporary "private interconnect" between VFIO DMABUF and iommufd. > - * It allows the two co-operating drivers to exchange the physical address of > - * the BAR. This is to be replaced with a formal DMABUF system for negotiated > - * interconnect types. > + * Report the BAR's physical range for importers which program it into their own > + * translation tables, such as iommufd. A BAR is MMIO, never cache-coherent RAM. > * > - * If this function succeeds the following are true: > - * - There is one physical range and it is pointing to MMIO > - * - When move_notify is called it means revoke, not move, vfio_dma_buf_map > - * will fail if it is currently revoked > + * When move_notify is called it means revoke, not move, so this fails while > + * revoked and vfio_dma_buf_map() does the same. > */ > -int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys) > +static int vfio_pci_dma_buf_get_phys(struct dma_buf_attachment *attachment, > + u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr) > { > - struct vfio_pci_dma_buf *priv; > + struct vfio_pci_dma_buf *priv = attachment->dmabuf->priv; > + u32 i; > > dma_resv_assert_held(attachment->dmabuf->resv); > - > - if (attachment->dmabuf->ops != &vfio_pci_dmabuf_ops) > - return -EOPNOTSUPP; > - > - priv = attachment->dmabuf->priv; > if (priv->revoked) > return -ENODEV; > > - /* More than one range to iommufd will require proper DMABUF support */ > - if (priv->nr_ranges != 1) > - return -EOPNOTSUPP; > - > - *phys = priv->phys_vec[0]; > + /* Report from @offset to the end of the BAR range containing it. */ > + for (i = 0; i < priv->nr_ranges; i++) { > + if (offset < priv->phys_vec[i].len) > + break; > + offset -= priv->phys_vec[i].len; > + } > + if (i == priv->nr_ranges) > + return -EINVAL; > + phys->paddr = priv->phys_vec[i].paddr + offset; > + phys->len = min_t(u64, priv->phys_vec[i].len - offset, len); > + *attr = DMA_BUF_PHYS_ATTR_MMIO; > return 0; > } > -EXPORT_SYMBOL_FOR_MODULES(vfio_pci_dma_buf_iommufd_map, "iommufd"); > + > +static const struct dma_buf_ops vfio_pci_dmabuf_ops = { > + .attach = vfio_pci_dma_buf_attach, > + .map_dma_buf = vfio_pci_dma_buf_map, > + .unmap_dma_buf = vfio_pci_dma_buf_unmap, > + .release = vfio_pci_dma_buf_release, > + .get_phys = vfio_pci_dma_buf_get_phys, > +}; > + > > int vfio_pci_core_fill_phys_vec(struct phys_vec *phys_vec, > struct vfio_region_dma_range *dma_ranges, > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index d1203da56fc5..b223962e20c2 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -13,6 +13,7 @@ > #ifndef __DMA_BUF_H__ > #define __DMA_BUF_H__ > > +#include <linux/bitfield.h> > #include <linux/iosys-map.h> > #include <linux/file.h> > #include <linux/err.h> > @@ -23,6 +24,7 @@ > #include <linux/dma-fence.h> > #include <linux/wait.h> > #include <linux/pci-p2pdma.h> > +#include <linux/types.h> > > struct device; > struct dma_buf; > @@ -182,6 +184,29 @@ struct dma_buf_ops { > struct sg_table *, > enum dma_data_direction); > > + /** > + * @get_phys: > + * > + * Describe the run that starts at @offset, for an importer that > + * programs its own translation tables. A run is the longest stretch > + * of backed bytes whose frames are physically contiguous and share > + * one attribute word. Report it in *@phys, starting at the byte at > + * @offset and clipped at @offset + @len, with its DMA_BUF_PHYS_ATTR_* > + * word in *@attr. The exporter may stop before the end of the run; > + * importers must not assume the reported run is maximal. > + * > + * Return 0 on success, -ENOENT if the byte at @offset is not backed, > + * -ENODEV if the buffer is revoked, or another negative error. Do not > + * wait for memory to become available. > + * > + * The dma-buf reservation is held. The attachment must be pinned or > + * have revocable importer operations. A reported frame remains valid > + * until a covering invalidation callback returns; an exporter must > + * invalidate every change before reusing an old frame. > + */ > + int (*get_phys)(struct dma_buf_attachment *attach, u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr); > + > /* TODO: Add try_map_dma_buf version, to return immed with -EBUSY > * if the call would block. > */ > @@ -576,6 +601,35 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table *, > enum dma_data_direction); > void dma_buf_invalidate_mappings(struct dma_buf *dma_buf); > bool dma_buf_attach_revocable(struct dma_buf_attachment *attach); > +/* bits 0-7: memory type (a value, not flags) */ > +#define DMA_BUF_PHYS_ATTR_TYPE_MASK GENMASK(7, 0) > +#define DMA_BUF_PHYS_ATTR_RAM 0x00 /* cache-coherent system RAM */ > +#define DMA_BUF_PHYS_ATTR_MMIO 0x01 /* device MMIO, uncached */ > +/* bits 8-15: reserved for a second value field; must be zero */ > +#define DMA_BUF_PHYS_ATTR_RSVD_MASK GENMASK(15, 8) > +/* bits 16-31: flags; undefined bits must be zero */ > +#define DMA_BUF_PHYS_ATTR_READONLY BIT(16) > +/* BIT(17): reserved (hole, for importers that walk across gaps) */ > +/* BIT(18): reserved (private, for confidential computing) */ > +#define DMA_BUF_PHYS_ATTR_FLAGS_MASK (DMA_BUF_PHYS_ATTR_READONLY) > + > +static inline u32 dma_buf_phys_attr_type(u32 attrs) > +{ > + return FIELD_GET(DMA_BUF_PHYS_ATTR_TYPE_MASK, attrs); > +} > + > +static inline bool dma_buf_phys_attr_known(u32 attrs) > +{ > + return dma_buf_phys_attr_type(attrs) <= DMA_BUF_PHYS_ATTR_MMIO && > + !(attrs & DMA_BUF_PHYS_ATTR_RSVD_MASK) && > + !(attrs & ~(DMA_BUF_PHYS_ATTR_TYPE_MASK | > + DMA_BUF_PHYS_ATTR_RSVD_MASK | > + DMA_BUF_PHYS_ATTR_FLAGS_MASK)); > +} > + > +int dma_buf_get_phys(struct dma_buf_attachment *attach, u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr); > + > int dma_buf_begin_cpu_access(struct dma_buf *dma_buf, > enum dma_data_direction dir); > int dma_buf_end_cpu_access(struct dma_buf *dma_buf, > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > index 9a1674c152aa..2a1d13abdb0c 100644 > --- a/include/linux/vfio_pci_core.h > +++ b/include/linux/vfio_pci_core.h > @@ -257,7 +257,4 @@ vfio_pci_core_get_iomap(struct vfio_pci_core_device *vdev, unsigned int bar) > return vdev->barmap[bar]; > } > > -int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, > - struct phys_vec *phys); > - > #endif /* VFIO_PCI_CORE_H */ > diff --git a/samples/kvm/gmem_provider.c b/samples/kvm/gmem_provider.c > index 75197c088762..b6824fe5d228 100644 > --- a/samples/kvm/gmem_provider.c > +++ b/samples/kvm/gmem_provider.c > @@ -487,35 +487,30 @@ static void gmem_dma_buf_release(struct dma_buf *dmabuf) > kfree(priv); > } > > -static const struct dma_buf_ops gmem_dma_buf_ops = { > - .attach = gmem_dma_buf_attach, > - .map_dma_buf = gmem_dma_buf_map, > - .unmap_dma_buf = gmem_dma_buf_unmap, > - .release = gmem_dma_buf_release, > -}; > - > -/* > - * Private interconnect for iommufd (mirrors vfio_pci_dma_buf_iommufd_map). > - * Returns the single contiguous phys range for the exported region so iommufd > - * can program the IOMMU directly, bypassing the DMA API. > - */ > -int gmem_provider_dma_buf_iommufd_map(struct dma_buf_attachment *attach, > - struct phys_vec *phys); > -int gmem_provider_dma_buf_iommufd_map(struct dma_buf_attachment *attach, > - struct phys_vec *phys) > +/* Report this flat sample region through the generic dma-buf operation. */ > +static int gmem_dma_buf_get_phys(struct dma_buf_attachment *attach, > + u64 offset, u64 len, > + struct phys_vec *phys, u32 *attr) > { > - struct gmem_dmabuf *priv; > + struct gmem_dmabuf *priv = attach->dmabuf->priv; > > dma_resv_assert_held(attach->dmabuf->resv); > - if (attach->dmabuf->ops != &gmem_dma_buf_ops) > - return -EOPNOTSUPP; > - priv = attach->dmabuf->priv; > if (priv->revoked) > return -ENODEV; > - *phys = priv->phys; > + > + phys->paddr = priv->phys.paddr + offset; > + phys->len = len; > + *attr = DMA_BUF_PHYS_ATTR_RAM; > return 0; > } > -EXPORT_SYMBOL_FOR_MODULES(gmem_provider_dma_buf_iommufd_map, "iommufd"); > + > +static const struct dma_buf_ops gmem_dma_buf_ops = { > + .attach = gmem_dma_buf_attach, > + .map_dma_buf = gmem_dma_buf_map, > + .unmap_dma_buf = gmem_dma_buf_unmap, > + .release = gmem_dma_buf_release, > + .get_phys = gmem_dma_buf_get_phys, > +}; > > /* Called with info->dmabufs_lock held on the revoke path. */ > static void gmem_dma_buf_revoke_all(struct gmem_info *info) > -- > 2.47.3 >

1 day, 23 hours

Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests

by Christoph Hellwig

On Mon, Sep 28, 2026 at 02:32:22PM +0100, Pavel Begunkov wrote: > +static void nvme_dmabuf_map_sync_for_cpu(struct nvme_dev *nvme_dev, > + struct request *req) > +{ > + struct device *dev = nvme_dev->dev; > + enum dma_data_direction dma_dir; > + struct bio *bio = req->bio; > + struct nvme_dmabuf_map *map = to_nvme_dmabuf_map(bio->bi_dmabuf_map); > + > + dma_dir = rq_data_dir(req) == READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE; > + dma_sync_sgtable_for_cpu(dev, map->sgt, dma_dir); This can use rq_dma_dir in the argument. Same for the other DMA API calls. Btw, given how larger the dmabuf code is now, have you considered splitting it into a new nvme-pci-dmabuf.c file?

2 days

Re: [PATCH v7 06/13] nvme-pci: implement dma-buf backed requests

by Christophe JAILLET

Le 28/09/2026 à 15:32, Pavel Begunkov a écrit : > Enable BIO_DMABUF_MAP backed requests. On registration we map the > dma-buf and store it as a prp list, which is then used to initialise > requests. All attached contexts are stored in a new list dmabuf_ctxs, > and additions/removals are synchronised with dmabuf_lock. > > Suggested-by: Keith Busch <kbusch(a)kernel.org> > Signed-off-by: Pavel Begunkov <asml.silence(a)gmail.com> Hi, a few nitpick below, should it help. > --- > drivers/nvme/host/core.c | 12 ++ > drivers/nvme/host/nvme.h | 2 + > drivers/nvme/host/pci.c | 308 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 322 insertions(+) > [...] > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index a953c0697f99..e58bdd9a4098 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -27,6 +27,8 @@ > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/io-64-nonatomic-hi-lo.h> > #include <linux/sed-opal.h> > +#include <linux/dma-buf-io.h> > +#include <linux/dma-resv.h> Move up, to keep better alphabetical order ? > > #include "trace.h" > #include "nvme.h" > @@ -318,6 +320,8 @@ struct nvme_dev { > bool hmb; > struct sg_table *hmb_sgt; > mempool_t *dmavec_mempool; > + struct list_head dmabuf_ctxs; > + struct mutex dmabuf_lock; > > /* shadow doorbell buffer support: */ > __le32 *dbbuf_dbs; > @@ -397,6 +401,13 @@ struct nvme_queue { > struct completion delete_done; > }; > > +struct nvme_dmabuf_map { > + struct dma_buf_io_map base; > + struct sg_table *sgt; > + unsigned nr_entries; > + dma_addr_t dma_list[]; Add __counted_by(nr_entries) and update nvme_dma_buf_io_map() so that nr_entries is set at the right time ? > +}; > + > /* bits for iod->flags */ > enum nvme_iod_flags { > /* this command has been aborted by the timeout handler */ ... > +static struct dma_buf_io_map *nvme_dma_buf_io_map(struct dma_buf_io_ctx *ctx) > +{ > + unsigned nr_entries = ctx->dmabuf->size / NVME_CTRL_PAGE_SIZE; > + struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv; > + struct dma_buf_attachment *attach = nvme_ctx->attach; > + unsigned long tmp, i = 0; > + struct nvme_dmabuf_map *map; > + struct scatterlist *sg; > + struct sg_table *sgt; > + int ret; > + > + dma_resv_assert_held(ctx->dmabuf->resv); > + > + map = kvmalloc_flex(*map, dma_list, nr_entries); > + if (!map) > + return ERR_PTR(-ENOMEM); > + > + sgt = dma_buf_map_attachment(attach, ctx->dir); > + if (IS_ERR(sgt)) { > + ret = PTR_ERR(sgt); > + sgt = NULL; > + goto err; > + } > + > + for_each_sgtable_dma_sg(sgt, sg, tmp) { > + dma_addr_t dma_addr = sg_dma_address(sg); > + unsigned long sg_len = sg_dma_len(sg); > + > + if ((sg_len % NVME_CTRL_PAGE_SIZE) || > + (dma_addr % NVME_CTRL_PAGE_SIZE)) { > + ret = -EINVAL; > + goto err; > + } > + while (sg_len) { > + map->dma_list[i++] = dma_addr; > + dma_addr += NVME_CTRL_PAGE_SIZE; > + sg_len -= NVME_CTRL_PAGE_SIZE; > + } > + } > + > + ret = dma_buf_io_init_map(ctx, &map->base, sgt); > + if (ret) > + goto err; > + map->nr_entries = nr_entries; > + map->sgt = sgt; > + return &map->base; > +err: > + if (sgt) > + dma_buf_unmap_attachment(attach, sgt, ctx->dir); > + kfree(map); kvfree()? > + return ERR_PTR(ret); > +} ... CJ

2 days

Re: [PATCH v7 05/13] block: add dma-buf support for raw bdev

by Christoph Hellwig

On Mon, Sep 28, 2026 at 02:32:21PM +0100, Pavel Begunkov wrote: > Add a simple proxy implementation of init_dma_buf_io_ctx() forwarding > the call to a new struct block_device_operations operation. Also reject > dma-buf backed iterators for buffered IO. > > Reviewed-by: Christoph Hellwig <hch(a)lst.de> > [pavel: reject dma-buf without O_DIRECT] > Signed-off-by: Pavel Begunkov <asml.silence(a)gmail.com> The Sashiko report about the errno on fallback here looks real, and is probably easily fixed by setting a proper errno.

2 days

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig