Message-ID: <CACePvbXheW_GGiyc5sN3SnEkCVVeNH4UvMORtMf5rZhCp_4OLg@mail.gmail.com>
Date: Thu, 2 Oct 2025 22:17:25 -0700
From: Chris Li <chrisl@...nel.org>
To: David Matlack <dmatlack@...gle.com>
Cc: Jason Gunthorpe <jgg@...pe.ca>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Danilo Krummrich <dakr@...nel.org>, Len Brown <lenb@...nel.org>, 
	Pasha Tatashin <pasha.tatashin@...een.com>, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org, 
	Pasha Tatashin <tatashin@...gle.com>, Jason Miu <jasonmiu@...gle.com>, 
	Vipin Sharma <vipinsh@...gle.com>, Saeed Mahameed <saeedm@...dia.com>, 
	Adithya Jayachandran <ajayachandra@...dia.com>, Parav Pandit <parav@...dia.com>, William Tu <witu@...dia.com>, 
	Mike Rapoport <rppt@...nel.org>, Leon Romanovsky <leon@...nel.org>
Subject: Re: [PATCH v2 03/10] PCI/LUO: Forward prepare()/freeze()/cancel()
 callbacks to driver

On Thu, Oct 2, 2025 at 2:31 PM David Matlack <dmatlack@...gle.com> wrote:
>
> On Thu, Oct 2, 2025 at 1:58 PM Chris Li <chrisl@...nel.org> wrote:
> >
> > On Thu, Oct 2, 2025 at 11:54 AM David Matlack <dmatlack@...gle.com> wrote:
> > > > Then don't do generic devices until we get iommufd done and you have a
> > > > meaningful in-tree driver to consume what you are adding.
> > >
> > > I agree with Jason. I don't think we can reasonably make the argument
> > > that we need this series until we have actualy use-cases for it.
> > >
> > > I think we should focus on vfio-pci device preservation next, and use
> > > that to incrementally drive whatever changes are necessary to the PCI
> > > and generic device layer bit by bit.
> >
> > The feedback I got for the PCI V1 was to make it as minimal as
> > possible. We agree preserving the BUS MASTER bit is the first minimal
> > step. That is what I did in the V2 phase I series. Only the bus
> > master. I think the pci-lu-test driver did demo the bus master bit, it
> > is not vfio yet. At least that was the plan shared in the upstream
> > alignment meeting.
>
> What do the driver callbacks in patch 3 and patches 5-8 have to do
> with preserving the bus master bit? That's half the series.

I was thinking the pcie driver is for preserving the bus master bit on
the bridge. I just realized that the actual bus master bit preserve is
at the pci_dev level. The pcie driver call back is effectively no op.
Yes, I can delete those from the series and not save driver data at
all. Points taken.
>
> >
> > > For example, once we a basic vfio-pci device preservation working, we
> > > can start thinking about how to handle when that device is a VF, and we
> > > have to start also preserving the SR-IOV state on the PF and get the PF
> >
> > SR-IOV is a much bigger step than the BUS Master bit. I recall at one
> > point in the upstream discussion meeting that we don't do SR-IOV as
> > the first step. I am not opposed to it, we need to get to vfio and
> > SR-IOV eventually. I just feel that the PCI + VFIO + SR-IOV will be a
> > much bigger series. I worry the series size is not friendly for
> > reviewers. I wish there could be smaller incremental steps digestible.
>
> SR-IOV is not a first step, of course. That's not what I said. I'm
> saying SR-IOV is future work that could justify some of the larger
> changes in this series (e.g. driver callbacks). So I am suggesting we
> revisit those changes when we are working on SR-IOV.

Just to confirm my understanding aligned with you. We remove the
driver callbacks in PCI series until the vfio SR-IOV to add them back.

> > > driver involved in the process. At that point we can discuss how to
> > > solve that specific problem. Maybe the solution will look something like
> > > this series, maybe it will look like something else. There is open
> > > design space.
> >
> > Yes doable, just will delay the LUO/PCI series by a bit and a much
> > bigger series.
>
> There will not be one "LUO/PCI series". There will be many incremental steps.

Oh, that is good then. I can still keep the PCI series as one incremental step.

>
> >
> > > Without approaching it this way, I don't see how we can't reasonably
> > > argue that anything in this series is necessary. And I suspect some
> > > parts of this series truly are unnecessary, at least in the short term.
> >
> > You have me on the double negatives, always not very good at those.
> > If the bigger series is what we want, I can do that. Just will have
> > some latency to get the VFIO.
>
> Oops, typo on my end. I meant "I don't see how we _can_ reasonably
> argue". I am saying we don't have enough justification (yet) for a lot
> of the code changes in this series.

Ack for removing driver callbacks in this series.

> > > In our internal implementation, the only dependent device that truly
> > > needed to participate is the PF driver when a VF is preserved.
> > > Everything else (e.g. pcieport callbacks) have just been no-ops.
> >
> > Your VF device does not need to preserve DMA? If you want to preserve
> > DMA the bus master bit is required, and the pcieport driver for the
> > PCI-PCI bridge is also required. I am not sure pure VF and PF without
> > any DMA makes practical sense.
>
> I'm saying the only drivers that actually needed to implement Live
> Update driver callbacks have been vfio-pci and PF drivers. The
> vfio-pci support doesn't exist yet upstream, and we are planning to
> use FD preservation there. So we don't know if driver callbacks will
> be needed for that. And we don't care about PF drivers until we get to
> supporting SR-IOV. So the driver callbacks all seem unnecessary at
> this point.

Ack.

>
> I totally agree we need to avoid clearing the bus master bit. Let's
> focus on that.

Ack.

Chris

Powered by blists - more mailing lists