All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pranjal Shrivastava <praan@google.com>
To: Matt Evans <matt@ozlabs.org>
Cc: "Alex Williamson" <alex@shazbot.org>,
	"Jason Gunthorpe" <jgg@nvidia.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"Leon Romanovsky" <leon@kernel.org>,
	"Alex Mastro" <amastro@fb.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Logan Gunthorpe" <logang@deltatee.com>,
	"Mahmoud Adam" <mngyadam@amazon.de>,
	"David Matlack" <dmatlack@google.com>,
	"Björn Töpel" <bjorn@kernel.org>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Ankit Agrawal" <ankita@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Kasireddy, Vivek" <vivek.kasireddy@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH v3 6/9] vfio/pci: Clean up BAR zap and revocation
Date: Tue, 23 Jun 2026 12:35:19 +0000	[thread overview]
Message-ID: <ajp9hwIw657qxji5@google.com> (raw)
In-Reply-To: <fbb2b1a6-594f-48eb-887f-7cf0cdd4c336@ozlabs.org>

On Tue, Jun 23, 2026 at 12:08:30PM +0100, Matt Evans wrote:
> Hi Alex,
> 
> On 23/06/2026 00:13, Alex Williamson wrote:
> > On Fri, 19 Jun 2026 16:13:17 +0100
> > Matt Evans <matt@ozlabs.org> wrote:
> > 
> >> Hi Jason,
> >>
> >> On 19/06/2026 14:31, Jason Gunthorpe wrote:
> >>> On Thu, Jun 18, 2026 at 05:02:58PM +0100, Matt Evans wrote:
> >>>   
> >>>> My understanding is that the sequences above wake a device that happens
> >>>> to have previously been put into D3, and AFAICT it could only have got
> >>>> there because of a previous vfio_pci_set_power_state().  Seems its only
> >>>> caller is from the emulation of PCI_PM_CTRL using
> >>>> vfio_lock_and_set_power_state(), and this zaps/revokes BAR access before
> >>>> a transition to D3.  Similarly, an attempt to access a BAR via an
> >>>> ioctl/through vfio_pci_core_do_io_rw() fails the D3 check in
> >>>> __vfio_pci_memory_enabled(), and besides will try to take the memory_lock.  
> >>>
> >>> I thought the general design was the bars were made inaccessible
> >>> before going to a low power state, and remain inaccessible while it is
> >>> in low power?
> >>>
> >>> So the order of D0 doesn't matter. If it is not in D0 then there is no
> >>> mappings and zap/revoke is a NOP.
> >>>
> >>> If is it in D0 then it doesn't matter because D0 is a nop.  
> >> Yes, that's what I'm getting at. :)  If it's in D3 then BARs are
> >> inaccessible, so as long as we go into D0 before the DMABUF move, the
> >> order of the zap relative to the "go to D0" doesn't matter.
> > 
> > I believe this is correct as well, but importantly we cannot assume
> > that a stray read or write just returns -1 or gets dropped.  This is
> > exactly why we have such hard protections against the user accessing
> > the device while it's disabled.  Not all platforms, even within
> > architectures that might otherwise be considered lenient of such
> > accesses, consider this benign and might escalate to system level
> > faults.
> 
> We are in enthusiastic agreement here.
> 
> > Let's be careful not to frame this as "the access doesn't matter
> > anyway", the answer is instead that non-D0 devices already lack any
> > mappings to access the device.  Thanks,
> 
> I agree that is not the right thing to say, for exactly that reason.
> (For avoidance of any doubt, I didn't say that :) )
> 
> Thanks for confirming the behaviour.  I hope Praan and Kevin are
> satisfied that this patch doesn't cause the issues they first worried
> about (the changed order of the zap relative to the D0 transition
> doesn't have a detrimental effect because of the existing inaccessibility).
> 
> Alex, I'll post v4 soon, but if you have any comments in the pipeline
> please shout and I'll hold off awhile.

I think the discussion addresses my concerns. I'm in agreement as well.

Thanks,
Praan

  reply	other threads:[~2026-06-23 12:35 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 15:43 [PATCH v3 0/9] vfio/pci: Add mmap() for DMABUFs Matt Evans
2026-06-10 15:43 ` [PATCH v3 1/9] PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE Matt Evans
2026-06-10 18:39   ` Leon Romanovsky
2026-06-11 16:07   ` Bjorn Helgaas
2026-06-11 17:44     ` Matt Evans
2026-06-11 18:37   ` Pranjal Shrivastava
2026-06-12  3:39     ` Tian, Kevin
2026-06-12 14:31       ` Matt Evans
2026-06-23 15:48         ` Robin Murphy
2026-06-23 15:59           ` Matt Evans
2026-06-23 17:47             ` Matt Evans
2026-06-17  7:47   ` Tian, Kevin
2026-06-10 15:43 ` [PATCH v3 2/9] vfio/pci: Add a helper to look up PFNs for DMABUFs Matt Evans
2026-06-11 20:30   ` Pranjal Shrivastava
2026-06-12 17:37     ` Alex Williamson
2026-06-12 18:21       ` Pranjal Shrivastava
2026-06-15 14:27     ` Matt Evans
2026-06-15 15:07       ` Pranjal Shrivastava
2026-06-12  8:42   ` Tian, Kevin
2026-06-15 18:04     ` Matt Evans
2026-06-16  9:28       ` Tian, Kevin
2026-06-16 11:48         ` Matt Evans
2026-06-10 15:43 ` [PATCH v3 3/9] vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA Matt Evans
2026-06-12  8:43   ` Tian, Kevin
2026-06-12  9:20   ` Pranjal Shrivastava
2026-06-10 15:43 ` [PATCH v3 4/9] vfio/pci: Convert BAR mmap() to use a DMABUF Matt Evans
2026-06-12  8:46   ` Tian, Kevin
2026-06-15 15:33     ` Matt Evans
2026-06-12 10:41   ` Pranjal Shrivastava
2026-06-12 15:22     ` Matt Evans
2026-06-12 19:43       ` Pranjal Shrivastava
2026-06-10 15:43 ` [PATCH v3 5/9] vfio/pci: Provide a user-facing name for BAR mappings Matt Evans
2026-06-12  8:46   ` Tian, Kevin
2026-06-12 14:06   ` Pranjal Shrivastava
2026-06-15 15:13     ` Matt Evans
2026-06-10 15:43 ` [PATCH v3 6/9] vfio/pci: Clean up BAR zap and revocation Matt Evans
2026-06-12 19:39   ` Pranjal Shrivastava
2026-06-16  9:48     ` Tian, Kevin
2026-06-16 18:51       ` Pranjal Shrivastava
2026-06-17  6:22         ` Tian, Kevin
2026-06-17 12:16           ` Jason Gunthorpe
2026-06-18 16:02           ` Matt Evans
2026-06-19 13:31             ` Jason Gunthorpe
2026-06-19 15:13               ` Matt Evans
2026-06-22 23:13                 ` Alex Williamson
2026-06-23 11:08                   ` Matt Evans
2026-06-23 12:35                     ` Pranjal Shrivastava [this message]
2026-06-18 16:06     ` Matt Evans
2026-06-23 12:38       ` Pranjal Shrivastava
2026-06-16  9:18   ` Tian, Kevin
2026-06-10 15:43 ` [PATCH v3 7/9] vfio/pci: Support mmap() of a VFIO DMABUF Matt Evans
2026-06-12 20:35   ` Pranjal Shrivastava
2026-06-16 15:45     ` Matt Evans
2026-06-16  9:20   ` Tian, Kevin
2026-06-10 15:43 ` [PATCH v3 8/9] vfio/pci: Permanently revoke a DMABUF on request Matt Evans
2026-06-16  8:05   ` Pranjal Shrivastava
2026-06-17 16:22     ` Matt Evans
2026-06-16  9:26   ` Tian, Kevin
2026-06-17 16:08     ` Matt Evans
2026-06-10 15:43 ` [PATCH v3 9/9] vfio/pci: Add mmap() attributes to DMABUF feature Matt Evans
2026-06-16  8:47   ` Pranjal Shrivastava
2026-06-16 11:37     ` Matt Evans
2026-06-16 19:09       ` Pranjal Shrivastava
2026-06-16  9:26   ` Tian, Kevin
2026-06-12  8:27 ` [PATCH v3 0/9] vfio/pci: Add mmap() for DMABUFs Tian, Kevin
2026-06-12 15:11   ` Matt Evans
2026-06-12 15:17     ` Pranjal Shrivastava

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajp9hwIw657qxji5@google.com \
    --to=praan@google.com \
    --cc=alex@shazbot.org \
    --cc=amastro@fb.com \
    --cc=ankita@nvidia.com \
    --cc=apopple@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn@kernel.org \
    --cc=christian.koenig@amd.com \
    --cc=dmatlack@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jgg@nvidia.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=leon@kernel.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=logang@deltatee.com \
    --cc=matt@ozlabs.org \
    --cc=mngyadam@amazon.de \
    --cc=sumit.semwal@linaro.org \
    --cc=vivek.kasireddy@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.