Re: [PATCH v2 03/10] PCI/LUO: Forward prepare()/freeze()/cancel() callbacks to driver

David Matlack
2025-10-02T21:31:38Z

On Thu, Oct 2, 2025 at 1:58 PM Chris Li <chrisl@kernel.org> wrote:
>
> On Thu, Oct 2, 2025 at 11:54 AM David Matlack <dmatlack@google.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.

>
> > 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.

>
> > 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.

>
> > 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.

>
> > 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.

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