From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 80C4737E5D4 for ; Tue, 4 Aug 2026 19:02:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.181 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785870127; cv=none; b=ZNv4IodSEj0bUQFsQz8inJtMFWB8eAdfSzNptFIJG7JGNFuJcmk5qI6WDN48dl93EJOB69J1Bvju06rW6PVcUNlBnc1edGLZ73DiO7/4+mjHbv2UjcGNe69PzHWqXflS63iJTdS0Z/kn11QIe/wU/bi6PGFpFK7kU+IRZYfKqJo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785870127; c=relaxed/simple; bh=QWLw5d3PsFELUOtZXKJPtt4CkxNmtXjChuvttT4vh7o=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=g2pOxCeYwKQUh3IZHQqORFD5DdK0i66YpzmdMCxTyaIfkpaqjudvXvXdiBmQe63RNDWA14Ur4WTvT7UYMksv33fT6Y5DoHzB0FXEU+HMbeH5AqOeJKiXFMDmjQgLUwFtaj4sQcpMybVclnQj59Dg6eXecyDKNzXIS5kabZ/55wk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=V2XPeA19; arc=none smtp.client-ip=209.85.214.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="V2XPeA19" Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-2ccdf36f63dso25035ad.0 for ; Tue, 04 Aug 2026 12:02:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1785870124; x=1786474924; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=o7hc3yljc5+LxUHQxs78+dkD3Zfo1reE/uEECJBVPac=; b=V2XPeA19gXM3fPDmK7dnIChkFnwwjxXN1IothHoQUIofnJI1gMnfkmzOggGxj1rsd2 OupN0NEGnmjWbtz6eF7r6SJFNDqzx7iQIjnT7689gnKak6T9dnv11BFnWs1r//hr/nNP Qw0wMI2ec73uAdmw1tN4In4tpWaJFKQ/dDEgx4r4PQ1f3REfkcsXhqGqoT28dWuJ4WAy NtM3+HP0DpSPWSN2VocUCR2ybCWPISC9T5PfuoHL/jFvE3HXvCI73hvR6W20vaYLescI xyY5eir22E6fPeJrESKjwEse3WRoOmVrMyI5D3mzuPwAF22qjH4wmz9ybeeL9LO45aM5 FIuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785870124; x=1786474924; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=o7hc3yljc5+LxUHQxs78+dkD3Zfo1reE/uEECJBVPac=; b=nfYLFXRK4xbo4PrBaFlS01tmaHQDk7RuMHu2bCRZM3PGYZ16//5E4XEZFE7jZxo/Wv cQfdkd/WZdpF2siOsA6a31yU7ElS0QlKcwtVYFkXcR/iHh97TF8o6yWFhOdmbU64fIam np3IP0VioPuKgbiU/bLT6eRLLrSn0JiMGTeY7JeLsvf0OBJGdy7GizAfp5XJvdSov0nC TO8r/IsZN1/fQMqekZiMI3Ugy+NLtWFikqDH48V2D8oxaEvPDIrfPjufL6+tVm3/CrO7 Tz64ocoYDWZTsBq1bCZ/i6z3pe088M8xjhpciE/HnAYNa1lSvuZke/HN6hK/m2MAn6k7 KAzw== X-Forwarded-Encrypted: i=1; AHgh+Rp1qPMIxSUy4wDS3NeQq5twpIwvOMJfkj+sb3z+Pzy8LpIydrLW8N5JSMmIAP0tJ2ySFEe9sm2U+GcGL+I=@vger.kernel.org X-Gm-Message-State: AOJu0YyM3Wtd/nyxcif/TvN62+FaaDG1Wj9gmgslVEq95d5JVsjQDYaC becdARViCmXpbc6QB1zBImcbwfEnf5ruKqENnO0DQx/hmlNLlGpvsB9ARZbtl9B8Pg== X-Gm-Gg: AR+sD10l7mh85b90ms2Se89wOkA0eYJ9TSKSsgFDXA9kKEHYTWbnMPfmORsKHm15kTZ fJsxQIGEwrusAuSeGgFiUeqT1gW2vuWAWklWo9i7f/BEDbohE1W+n3A/aGZi67ipzJ1KgjLK4Ti ed1onWf/jnfuKi7LmWAeCTZfrFp1+93x6X6ZSGG2C8vm2pLSPDq7tiTXUB6zbCLRrBaEq8ZQ/e0 X1KcWjMq7xURh5UqiR3JtV+NPQwZzcA3pVGZaDyUsIIlsmjRDGEf+d83ohAJENyJWcP6iJhEKMu cgLiQyZHmaKsozigMV9lkzU4KleZIkaCZOu5CLjyZmKAWV8UgBNEKDhiFV1wcGriC7L4eWIjH5n zlPYWvIOFo8ijkaEOXGYa78G4d6nHysNvN5rF/+0ZXbFq6yZVf8QXNfOOECTQiCQtX3xfqzrHFK TNeSP7x1iqKhD+PLqe68dJeeHuKEgIpGdhSVlDyqHembCAtUlAANxma2/5EsxEsLGBtfBmznavH kkdbe5VhbU6ObbEtDT22n8= X-Received: by 2002:a17:903:3c23:b0:2ca:cbe5:c3af with SMTP id d9443c01a7336-2d0caf6bbb3mr2044475ad.7.1785870123058; Tue, 04 Aug 2026 12:02:03 -0700 (PDT) Received: from google.com (21.168.124.34.bc.googleusercontent.com. [34.124.168.21]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-84f2e2c2326sm201840b3a.10.2026.08.04.12.01.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Aug 2026 12:02:02 -0700 (PDT) Date: Tue, 4 Aug 2026 19:01:53 +0000 From: Pranjal Shrivastava To: Matt Evans Cc: Alex Williamson , Leon Romanovsky , Jason Gunthorpe , Alex Mastro , Christian =?iso-8859-1?Q?K=F6nig?= , Bjorn Helgaas , Logan Gunthorpe , Kevin Tian , Longfang Liu , Mahmoud Adam , David Matlack , =?iso-8859-1?Q?Bj=F6rn_T=F6pel?= , Sumit Semwal , Ankit Agrawal , Alistair Popple , Vivek Kasireddy , linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, kvm@vger.kernel.org, linux-pci@vger.kernel.org Subject: Re: [PATCH v5 3/9] vfio/pci: Add a helper to look up PFNs for DMABUFs Message-ID: References: <20260715174737.15287-1-matt@ozlabs.org> <20260715174737.15287-4-matt@ozlabs.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Aug 03, 2026 at 07:22:06PM +0100, Matt Evans wrote: Hi Matt, > Hi Praan, > > On 31/07/2026 18:43, Matt Evans wrote: > > Hi Praan, > > > > On 30/07/2026 23:55, Pranjal Shrivastava wrote: > >> On Wed, Jul 15, 2026 at 06:47:26PM +0100, Matt Evans wrote: > >>> Add vfio_pci_dma_buf_find_pfn(), which a VMA fault handler can use to > >>> find a PFN. > >>> > >>> This supports multi-range DMABUFs, which typically would be used to > >>> represent scattered spans but might even represent overlapping or > >>> aliasing spans of PFNs. > >>> > >>> Because this is intended to be used in vfio_pci_core.c, we also need > >>> to expose the struct vfio_pci_dma_buf in the vfio_pci_priv.h header. > >>> > >>> Signed-off-by: Matt Evans > >>> --- > >>> drivers/vfio/pci/vfio_pci_dmabuf.c | 153 ++++++++++++++++++++++++++--- > >>> drivers/vfio/pci/vfio_pci_priv.h | 20 ++++ > >>> 2 files changed, 160 insertions(+), 13 deletions(-) > >>> > >>> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c > >>> index c16f460c01d6..7c047400dfd1 100644 > >>> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c > >>> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c > >>> @@ -9,19 +9,6 @@ > >>> > >>> MODULE_IMPORT_NS("DMA_BUF"); > >>> > >>> -struct vfio_pci_dma_buf { > >>> - struct dma_buf *dmabuf; > >>> - struct vfio_pci_core_device *vdev; > >>> - struct list_head dmabufs_elm; > >>> - size_t size; > >>> - struct phys_vec *phys_vec; > >>> - struct p2pdma_provider *provider; > >>> - u32 nr_ranges; > >>> - struct kref kref; > >>> - struct completion comp; > >>> - u8 revoked : 1; > >>> -}; > >>> - > >>> static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, > >>> struct dma_buf_attachment *attachment) > >>> { > >>> @@ -106,6 +93,146 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = { > >>> .release = vfio_pci_dma_buf_release, > >>> }; > >>> > >>> +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *priv, > >>> + struct vm_area_struct *vma, > >>> + unsigned long fault_addr, > >>> + unsigned int order, > >>> + unsigned long *out_pfn) > >>> +{ > >>> + /* > >>> + * Given a VMA (start, end, pgoffs) and a fault address, > >>> + * search the corresponding DMABUF's phys_vec[] to find the > >>> + * range representing the address's offset into the VMA, and > >>> + * its PFN. > >>> + * > >>> + * The phys_vec[] ranges represent contiguous spans of VAs > >>> + * upwards from the buffer offset 0; the actual PFNs might be > >>> + * in any order, overlap/alias, etc. Calculate an offset of > >>> + * the desired page given VMA start/pgoff and address, then > >>> + * search upwards from 0 to find which span contains it. > >>> + * > >>> + * On success, a valid PFN for a page sized by 'order' is > >>> + * returned into out_pfn. > >>> + * > >>> + * Failure occurs if: > >>> + * - A hugepage would cross the edge of the VMA, > >>> + * - A hugepage isn't entirely contained within a range > >>> + * (including where it straddles the boundary between > >>> + * ranges), > >>> + * - We find a range, but the final PFN isn't aligned to the > >>> + * requested order. > >>> + * > >>> + * Upon failure, -EAGAIN is returned and the caller is > >>> + * expected to try again with a smaller order, which will > >>> + * eventually succeed (order=0 will always work). > >>> + * > >>> + * It's suboptimal if DMABUFs are created with neighbouring > >>> + * ranges that are physically contiguous, since hugepages > >>> + * can't straddle range boundaries. (The construction of the > >>> + * ranges should merge them in this case.) > >>> + * > >>> + * Finally, vma_pgoff_adjust is used with a DMABUF created for > >>> + * a VFIO BAR mmap: a BAR mapped with vm_pgoff > 0 creates a > >>> + * DMABUF such that byte 0 of the VMA corresponds to byte 0 of > >>> + * the DMABUF and byte 'vm_pgoff << PAGE_SHIFT' into the BAR. > >>> + * To avoid double-offsetting in this scenario, subtracting > >>> + * vma_pgoff_adjust from this (non-zero) vm_pgoff generates > >>> + * the effective offset. > >>> + */ > >>> + > >>> + const unsigned long pagesize = PAGE_SIZE << order; > >>> + unsigned long vma_off = ((vma->vm_pgoff - priv->vma_pgoff_adjust) << > >>> + PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK; > >> > >> Maybe I'm getting ahead of myself here.. but it seems like this > >> restricts us to only mapping DMABUFs at offsets < 1TB due to the > >> VFIO_PCI_OFFSET_MASK (since we have HBMs on PCI devices now, hitting 1TB > >> may not be a very distant future). > > > > This is a really good question, thanks for raisiing it. It is not too > > forward-thinking at all. > > > >> While I understand this mask is needed to drop the BAR encoding in the > >> high bits. > >> > >> My worry is, if in the future a user were to export a massive > >> contiguous DMABUF (e.g., >1TB of aggregated HBM) and tried to mmap deep > >> into it (passing an offset >= 1TB), this bitwise AND would silently drop > >> the high bits, leading to silent data corruption. > > > > One of the big advantages of DMABUF export was that the range could be > > huge and unencumbered by the VFIO_PCI_OFFSET_SHIFT of the traditional > > mmap() interface. It's a way to mmap huge BARs without having to change > > the user-visible shift). So definitely this is a relevant concern. > > > >> I think we should explicitly reject such an mmap with -EINVAL like: > >> > >> +const unsigned long pagesize = PAGE_SIZE << order; > >> +unsigned long vma_off = (vma->vm_pgoff - priv->vma_pgoff_adjust) << PAGE_SHIFT; > >> > >> +/* > >> + * Prevent silent wrap-around if the user mmaps a DMABUF at an > >> + * offset greater than the VFIO index mask allows. > >> + */ > >> +if (unlikely(vma_off > VFIO_PCI_OFFSET_MASK)) > >> + return -EINVAL; > >> > >> +vma_off &= VFIO_PCI_OFFSET_MASK; > > > > Agreed, for now this absolutely should not silently wrap if the offset > > is > 1TB, and we live with the restriction that a DMABUF sized >1TB > > can't be mapped with such an offset. (We can still, say, map all of a > > 16TB DMABUF with offset=0, which is good.). I'll add a check, thanks > > for pointing this out. > > > > I suggest as something to revisit later, we flag for a DMABUF (maybe an > > evolution of vma_pgoff_adjust) to differentiate whether this masking > > needs to be applied (traditional mmap() path) or not (DMABUF mmap()), > > and then offsets can be arbitrarily large. > > Having tinkered, the two-step approach I suggested isn't great because > userspace has no easy way to know that step 2 has occurred and that > large offsets are "now supported". > > The second problem is this isn't just a limitation to not do an mmap() > with offset approaching 1TB, because you could run into trouble just > mapping, say, a 2TB BAR and then splitting it by unmapping a hole in the > middle: the second VMA now has a huge offset. > > It seems a proper fix is easy though: > > unsigned long vma_off = (vma->vm_pgoff - priv->vma_pgoff_adjust) << > PAGE_SHIFT; /* No masking! */ > > Then in vfio_pci_core_mmap_prep_dmabuf(), > > priv->vma_pgoff_adjust = vma->vm_pgoff; > > I.e., if vma_pgoff_adjust just includes the region index up high, it > cancels out the (same) index in the vm_pgoff. Thus regular mmap should > work (up to the 1TB offset), and DMABUF VMAs can have arbitrarily large > offsets. This looks good to me! I traced through both mmap paths to verify how vma_pgoff_adjust handles the offsets and it looks like this works perfectly for both. Cheers, Praan