From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f48.google.com (mail-qv1-f48.google.com [209.85.219.48]) (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 15B5D27FD7D for ; Tue, 18 Nov 2025 14:28:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763476135; cv=none; b=rT8jKlO2riS1XvYa7uaMuLaqmkIHlV6TZzDckTEoj44ZW0i5bTjQLvflhPsNEuzRAG2xMZVC0KHJIakGSuj2dvVxCKZc50SXtI5jH3mx6S+jomQLTASDfG2i7PSVZ3h580ReKYayGjUiBAuIVQL+BIDEKtxgsrdU8w0CN0xjA00= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763476135; c=relaxed/simple; bh=Vw+jMoLrz3CPi3RZadAUQ9OZDIQrFYnK7RrpZ+3jFFU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lKC+z7pE3rNhA4ojrhvGv0fFZaXxLx4dTTjkeoVYOj5SXyZCpuAM7JupmnCcdHiq7odwqvDs5SrH0PXYhin9ynuKqV/QskzaB5nAF7MvpyjQ2KPf55Lj2CfsuiVeHyFhPMa0p/WinwI8Ri6yB/D3yiDnLVoxAIYy8bTlnei5jZo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca; spf=pass smtp.mailfrom=ziepe.ca; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b=jbgNlztq; arc=none smtp.client-ip=209.85.219.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ziepe.ca Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="jbgNlztq" Received: by mail-qv1-f48.google.com with SMTP id 6a1803df08f44-880503ab181so55043126d6.2 for ; Tue, 18 Nov 2025 06:28:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; t=1763476131; x=1764080931; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=35WpspOv1fAWFINmOzi0AcgEQL1oV2/QEYSJcrH4aSg=; b=jbgNlztqEGj3WmXe7Jo3l8FaT1DvF8AaIYPg93dVoF6UZRCE01BN9/Pnq+azINjQid LpaMmzcr/XUyOOp2ThaI47IxSCnWlvwIwG94wN+XZ7LGqOd6cdwVFXyTanROgMLBbQ/6 jzJ8MKocS/iXqpfC3Wey6NGpOtxYUOzxcm2sbBZdIsSp05BjVhevLq428KKvAXPl3xL5 8RIG9SSDGshJkAbsY3vW7n/jSgIOdS7J00+Bl+R1/qeSCQMNP5hD5M8oSOaDEz/Aheqg FSaz76ZiLI+pEwl6Es2xp9bla/Vfo6GtgTw9bOQLBkvuDaGebbESWQ07NgaAWjGPJun8 9Ycw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763476131; x=1764080931; h=in-reply-to:content-disposition: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; bh=35WpspOv1fAWFINmOzi0AcgEQL1oV2/QEYSJcrH4aSg=; b=etcM9oI5o+XMJCIEiFv06qF0BeZybkg4qe8ExLI6GBc6VBY2iC346QJ5BBmaoL+zjp S07jY1/YS719kA6o5ZzTKFfeUigpwKO3Tt8CYhP42THSlBUBjeF3G6xV49s8rVD0Bmng mF50cGjYQesOKGGOiUHX51mA95aloRcjFO7PEaqRmnsyZSJC2SGiDM5w26zoD8+Fr1q4 2HizOXMd4+8jMc8l084XioPqlYwvmSn8vvyACjZLRInwORGwlbCkGZ+5eYgXUZFOwk4V 8y8qwjLIXWlbFrK0dw/tEtG8o6t6Wu9gOtDNObVucjd4KoL5pAXe1jMMp3sIFLscS3hf Vpgg== X-Forwarded-Encrypted: i=1; AJvYcCWu/gywjcnag5N3SWoWfO+1BiEbNZOz52tHjceW9gxtOviNrvfHRjpOz/PEXcIlwKb8LorlyH0W0Yp/8HQ=@vger.kernel.org X-Gm-Message-State: AOJu0YwMSg9D/L9KMu0T2oGN0pQ7N1Uq0PTTqLW/itgUFbwSZELob285 abRO10T7fUs25ZoilkjnSqdmWf4k8iJwcF2Eh5crWRsyqqDKKvb1az254ExOS/Q+ohY= X-Gm-Gg: ASbGnctH8ZQEB/iEefz7ZFxeIUZa/xCsaAgxMJmeZFcz4EFS+VYdMq+X4f/P6mM6k7Z 0KftVnmlnZWw/UeYZ3aUU4w24c3HkQl9rI2a4+OTn1C9OZRyoKufO+1F0DLRigYxoqs69VpLcMR 9G1nnYifUY1u39toxEg5aQIWa5Qp6HrYtYvG9YSvSDqCUmrlYAIq6461wlZf7Z8gOXKH8FOAbfh 3fV8SjqXZd0aR/Khsx9dJD9P4GLr8qZgLkHDsKjWHiosuLAM6LeYMsPcjyPbnnpje3T2NtEQ3On Yz9zAvxMOITfZ3z774cWTUkGdoQuW512czAebU6Eces87mQswrKp6v6J3WtI6CGA4xtHHSwrX7e ULNj5KueDvZd4uba9F8D9ISDqIh4ijBMuD1aseke47WqNZ8ibI0WNfZeAS9vZ7bfDecsEQrWhEs deEFNGRgbHmnsRneJlB1M2IA+0ElkArNxkYZLoXEy/sEZBqsbjdkmBHwBrhji7atZjoRM= X-Google-Smtp-Source: AGHT+IF+NrfSbYwvwiNh+CC3YngHFJHpwzxC37jBlKkv0sHaTL3i1cld4gWS47y25QRA75BxBaWSvQ== X-Received: by 2002:a05:6214:62a:b0:81b:bf92:8df9 with SMTP id 6a1803df08f44-8829269e086mr234228876d6.43.1763476131063; Tue, 18 Nov 2025 06:28:51 -0800 (PST) Received: from ziepe.ca (hlfxns017vw-47-55-120-4.dhcp-dynamic.fibreop.ns.bellaliant.net. [47.55.120.4]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-8828652efa4sm114860276d6.39.2025.11.18.06.28.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Nov 2025 06:28:50 -0800 (PST) Received: from jgg by wakko with local (Exim 4.97) (envelope-from ) id 1vLMhN-00000000NEc-3Dxs; Tue, 18 Nov 2025 10:28:49 -0400 Date: Tue, 18 Nov 2025 10:28:49 -0400 From: Jason Gunthorpe To: "Tian, Kevin" Cc: Leon Romanovsky , Bjorn Helgaas , Logan Gunthorpe , Jens Axboe , Robin Murphy , Joerg Roedel , Will Deacon , Marek Szyprowski , Andrew Morton , Jonathan Corbet , Sumit Semwal , Christian =?utf-8?B?S8O2bmln?= , Kees Cook , "Gustavo A. R. Silva" , Ankit Agrawal , Yishai Hadas , Shameer Kolothum , Alex Williamson , Krishnakant Jaju , Matt Ochs , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-block@vger.kernel.org" , "iommu@lists.linux.dev" , "linux-mm@kvack.org" , "linux-doc@vger.kernel.org" , "linux-media@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "linaro-mm-sig@lists.linaro.org" , "kvm@vger.kernel.org" , "linux-hardening@vger.kernel.org" , "Kasireddy, Vivek" Subject: Re: [PATCH v8 10/11] vfio/pci: Add dma-buf export support for MMIO regions Message-ID: <20251118142849.GG17968@ziepe.ca> References: <20251111-dmabuf-vfio-v8-0-fd9aa5df478f@nvidia.com> <20251111-dmabuf-vfio-v8-10-fd9aa5df478f@nvidia.com> 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 Tue, Nov 18, 2025 at 07:33:23AM +0000, Tian, Kevin wrote: > > From: Leon Romanovsky > > Sent: Tuesday, November 11, 2025 5:58 PM > > > > - if (!new_mem) > > + if (!new_mem) { > > vfio_pci_zap_and_down_write_memory_lock(vdev); > > - else > > + vfio_pci_dma_buf_move(vdev, true); > > + } else { > > down_write(&vdev->memory_lock); > > + } > > shouldn't we notify move before zapping the bars? otherwise there is > still a small window in between where the exporter already has the > mapping cleared while the importer still keeps it... zapping the VMA and moving/revoking the DMABUF are independent operations that can happen in any order. They effect different kinds of users. The VMA zap prevents CPU access from userspace, the DMABUF move prevents DMA access from devices. The order has to be like the above because vfio_pci_dma_buf_move() must be called under the memory lock and vfio_pci_zap_and_down_write_memory_lock() gets the memory lock.. > > +static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) > > +{ > > + struct vfio_pci_dma_buf *priv = dmabuf->priv; > > + > > + /* > > + * Either this or vfio_pci_dma_buf_cleanup() will remove from the list. > > + * The refcount prevents both. > > which refcount? I thought it's vdev->memory_lock preventing the race... Refcount on the dmabuf > > +int vfio_pci_core_fill_phys_vec(struct dma_buf_phys_vec *phys_vec, > > + struct vfio_region_dma_range *dma_ranges, > > + size_t nr_ranges, phys_addr_t start, > > + phys_addr_t len) > > +{ > > + phys_addr_t max_addr; > > + unsigned int i; > > + > > + max_addr = start + len; > > + for (i = 0; i < nr_ranges; i++) { > > + phys_addr_t end; > > + > > + if (!dma_ranges[i].length) > > + return -EINVAL; > > Looks redundant as there is already a check in validate_dmabuf_input(). Agree > > +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 vfio_pci_dma_buf *priv; > > + size_t length; > > + int ret; > > + > > + if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys) > > + return -EOPNOTSUPP; > > + > > + 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 || get_dma_buf.flags) > > + return -EINVAL; > > unknown flag bits get -EOPNOTSUPP. Agree > > + > > +void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) > > +{ > > + struct vfio_pci_dma_buf *priv; > > + struct vfio_pci_dma_buf *tmp; > > + > > + down_write(&vdev->memory_lock); > > + list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) > > { > > + if (!get_file_active(&priv->dmabuf->file)) > > + continue; > > + > > + dma_resv_lock(priv->dmabuf->resv, NULL); > > + list_del_init(&priv->dmabufs_elm); > > + priv->vdev = NULL; > > + priv->revoked = true; > > + dma_buf_move_notify(priv->dmabuf); > > + dma_resv_unlock(priv->dmabuf->resv); > > + vfio_device_put_registration(&vdev->vdev); > > + fput(priv->dmabuf->file); > > dma_buf_put(priv->dmabuf), consistent with other places. Someone else said this, I don't agree, the above got the get via get_file_active() instead of a dma_buf version.. So we should pair with get_file_active() vs fput(). Christian rejected the idea of adding a dmabuf wrapper for get_file_active(), oh well. > > +struct vfio_device_feature_dma_buf { > > + __u32 region_index; > > + __u32 open_flags; > > + __u32 flags; > > Usually the 'flags' field is put in the start (following argsz if existing). Yeah, but doesn't really matter. Thanks, Jason