From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7903C6D1A7; Tue, 30 Sep 2025 09:00:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759222853; cv=none; b=ioGcblSGgD0lEXbQHL3ngeK9xVuS8kNc3q2NPn7jb8aqIKa9FnWF+jMs18Nyr77hXyM2CJsJQH61HkJUPjPzdZQqKdmKR8mXCG1mu/zVpzvYeOpV3Mgsjt5A3+xn5jrV5z9XUcKS6HI6msoBXJenOJiVnTrUDAFAgfgilF+AI18= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759222853; c=relaxed/simple; bh=Bz59FJNr6hSVIp1bIOKqTXqbYBhR+8gIiV4mOYWby0Y=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AG2fjDpsjN6PMlA46bwoj88H0eQaqCyUT0iDRdNs/TOyrZSHcabBO0Ws+GVPRlrhgRTi1bo+lyb2o/LpIUzGib5lu8IZmRWyyGJyNFS3x6yfW4wsOe0VMlqb3kH/b0G6s6bL93uuSPXB6w8YC9Ic2j5jBpnJ/elTHj02tVrVwEE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B0lc8Tee; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B0lc8Tee" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 86F3BC4CEF0; Tue, 30 Sep 2025 09:00:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1759222853; bh=Bz59FJNr6hSVIp1bIOKqTXqbYBhR+8gIiV4mOYWby0Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=B0lc8TeeFVkyKURqnlVC8tfbwXSnH5yhCiwnKUCAVw0XfnT/duR62IEit4doNOgit w0LQw67ADMu98aes/9UNSiFxePK8v71jREbYBwtX/hzTPbV9ZNh+ClsZ59o+JePTg3 7DlSSNkO0ONIVmZkOrw6MQIgp6fEnwHYHtWOw8MrkSOFnIHrN5Kx6VbBkf0YlGbfZl MZQKCXYSs/sX24JFCduenUzxV3P1gFAK9MVDaD5Pry90wsMm4HClRFRF0dd4+CpJU+ CF4BMSEI+7doVyxI4NIwXinKl4wjhKMt6rRd8SGJfdPJXpDQlmUgZ8z+R4E8IeipQt +xlwP9a2p+IWQ== Date: Tue, 30 Sep 2025 12:00:48 +0300 From: Leon Romanovsky To: Alex Williamson Cc: Jason Gunthorpe , Andrew Morton , Bjorn Helgaas , Christian =?iso-8859-1?Q?K=F6nig?= , dri-devel@lists.freedesktop.org, iommu@lists.linux.dev, Jens Axboe , Joerg Roedel , kvm@vger.kernel.org, linaro-mm-sig@lists.linaro.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, linux-mm@kvack.org, linux-pci@vger.kernel.org, Logan Gunthorpe , Marek Szyprowski , Robin Murphy , Sumit Semwal , Vivek Kasireddy , Will Deacon Subject: Re: [PATCH v4 10/10] vfio/pci: Add dma-buf export support for MMIO regions Message-ID: <20250930090048.GG324804@unreal> References: <53f3ea1947919a5e657b4f83e74ca53aa45814d4.1759070796.git.leon@kernel.org> <20250929151749.2007b192.alex.williamson@redhat.com> Precedence: bulk X-Mailing-List: linux-media@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: <20250929151749.2007b192.alex.williamson@redhat.com> On Mon, Sep 29, 2025 at 03:17:49PM -0600, Alex Williamson wrote: > On Sun, 28 Sep 2025 17:50:20 +0300 > Leon Romanovsky wrote: > > +static int validate_dmabuf_input(struct vfio_pci_core_device *vdev, > > + struct vfio_device_feature_dma_buf *dma_buf, > > + struct vfio_region_dma_range *dma_ranges, > > + struct p2pdma_provider **provider) > > +{ > > + struct pci_dev *pdev = vdev->pdev; > > + u32 bar = dma_buf->region_index; > > + resource_size_t bar_size; > > + u64 sum; > > + int i; > > + > > + if (dma_buf->flags) > > + return -EINVAL; > > + /* > > + * For PCI the region_index is the BAR number like everything else. > > + */ > > + if (bar >= VFIO_PCI_ROM_REGION_INDEX) > > + return -ENODEV; > > + > > + *provider = pcim_p2pdma_provider(pdev, bar); > > + if (!provider) > > This needs to be IS_ERR_OR_NULL() or the function needs to settle on a > consistent error return value regardless of CONFIG_PCI_P2PDMA. pcim_p2pdma_provider() doesn't return errors after split to _init() and _get(). The more accurate check needs to be if (!*provider) and not what is written. > > > + return -EINVAL; > > + > > + bar_size = pci_resource_len(pdev, bar); > > We get to this feature via vfio_pci_core_ioctl_feature(), which is used > by several variant drivers, some of which mangle the BAR size exposed > to the user, ex. hisi_acc. I'm afraid this might actually be giving > dmabuf access to a portion of the BAR that isn't exposed otherwise. Doe you mean that part? 1185 static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev) 1186 { ... 1204 * Also the HiSilicon ACC VF devices supported by this driver on 1205 * HiSilicon hardware platforms are integrated end point devices 1206 * and the platform lacks the capability to perform any PCIe P2P 1207 * between these devices. 1208 */ 1209 1210 vf_qm->io_base = 1211 ioremap(pci_resource_start(vf_dev, VFIO_PCI_BAR2_REGION_INDEX), 1212 pci_resource_len(vf_dev, VFIO_PCI_BAR2_REGION_INDEX)); 1213 if (!vf_qm->io_base) 1214 return -EIO; 1215 According to the comment, it doesn't support p2p and in any case we will fail that platform in vfio_pci_dma_buf_attach() by taking "default" case: 34 switch (pci_p2pdma_map_type(priv->provider, attachment->dev)) { 35 case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE: 36 break; 37 case PCI_P2PDMA_MAP_BUS_ADDR: 38 /* 39 * There is no need in IOVA at all for this flow. 40 * We rely on attachment->priv == NULL as a marker 41 * for this mode. 42 */ 43 return 0; 44 default: 45 return -EINVAL; 46 } 47 > > > + for (i = 0; i < dma_buf->nr_ranges; i++) { > > + u64 offset = dma_ranges[i].offset; > > + u64 len = dma_ranges[i].length; > > + > > + if (!PAGE_ALIGNED(offset) || !PAGE_ALIGNED(len)) > > + return -EINVAL; > > + > > + if (check_add_overflow(offset, len, &sum) || sum > bar_size) > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, > > + struct vfio_device_feature_dma_buf __user *arg, > > + size_t argsz) > > +{ > > + struct vfio_device_feature_dma_buf get_dma_buf = {}; > > + struct vfio_region_dma_range *dma_ranges; > > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > + struct p2pdma_provider *provider; > > + struct vfio_pci_dma_buf *priv; > > + int ret; > > + > > + ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_GET, > > + sizeof(get_dma_buf)); > > + if (ret != 1) > > + return ret; > > + > > + if (copy_from_user(&get_dma_buf, arg, sizeof(get_dma_buf))) > > + return -EFAULT; > > + > > + if (!get_dma_buf.nr_ranges) > > + return -EINVAL; > > + > > + dma_ranges = memdup_array_user(&arg->dma_ranges, get_dma_buf.nr_ranges, > > + sizeof(*dma_ranges)); > > + if (IS_ERR(dma_ranges)) > > + return PTR_ERR(dma_ranges); > > + > > + ret = validate_dmabuf_input(vdev, &get_dma_buf, dma_ranges, &provider); > > + if (ret) > > + return ret; > > goto err_free_ranges; Thanks > > Thanks, > Alex > >