linux-kernel - Re: [PATCH v2 03/10] PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250925230236.GB2617119@nvidia.com>
Date: Thu, 25 Sep 2025 20:02:36 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Alex Williamson <alex.williamson@...hat.com>
Cc: Leon Romanovsky <leon@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Christian König <christian.koenig@....com>,
	dri-devel@...ts.freedesktop.org, iommu@...ts.linux.dev,
	Jens Axboe <axboe@...nel.dk>, Joerg Roedel <joro@...tes.org>,
	kvm@...r.kernel.org, linaro-mm-sig@...ts.linaro.org,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org, linux-mm@...ck.org,
	linux-pci@...r.kernel.org, Logan Gunthorpe <logang@...tatee.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Robin Murphy <robin.murphy@....com>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	Vivek Kasireddy <vivek.kasireddy@...el.com>,
	Will Deacon <will@...nel.org>
Subject: Re: [PATCH v2 03/10] PCI/P2PDMA: Refactor to separate core P2P
 functionality from memory allocation

On Thu, Sep 25, 2025 at 04:31:31PM -0600, Alex Williamson wrote:
> On Thu, 25 Sep 2025 08:53:08 -0300
> Jason Gunthorpe <jgg@...dia.com> wrote:
> 
> > On Thu, Sep 25, 2025 at 10:03:14AM +0300, Leon Romanovsky wrote:
> > 
> > > > It would at least make sense to me then to store the provider on the
> > > > vfio_pci_dma_buf object at the time of the get feature call rather than
> > > > vfio_pci_core_init_dev() though.  That would eliminate patch 08/ and
> > > > the inline #ifdefs.  
> > > 
> > > I'll change it now. If "enable" function goes to be "get" function, we
> > > won't need to store anything in vfio_pci_dma_buf too. At the end, we
> > > have exactly two lines "provider = priv->vdev->provider[priv->bar];",
> > > which can easily be changed to be "provider = pcim_p2pdma_provider(priv->vdev->pdev, priv->bar)"  
> > 
> > Not without some kind of locking change. I'd keep the
> > priv->vdev->provider[priv->bar] because setup during probe doesn't
> > need special locking.
> 
> Why do we need to store the provider on the vfio_pci_core_device at
> probe though, we can get it later via pcim_p2pdma_provider(). 

Because you'd need some new locking to prevent races.

Besides, the model here should be to call the function once during
probe and get back the allocated provider. The fact internally it is
kind of nutzo still shouldn't leak out as a property of the ABI.

I would like to remove this weird behavior where it caches things
inside the struct device. That's not normal for an API to do that, it
is only done for the genalloc path that this doesn't use.

Jason