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

Chris Li
2025-10-03T07:26:15Z

On Thu, Oct 2, 2025 at 11:19 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Oct 02, 2025 at 01:38:56PM -0700, Chris Li wrote:
> > On Tue, Sep 30, 2025 at 8:30 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Sep 16, 2025 at 12:45:11AM -0700, Chris Li wrote:
> > > > include/linux/dev_liveupdate.h | 23 +++++
> > > > include/linux/device/driver.h | 6 ++
> > >
> > > Driver core changes under the guise of only PCI changes? Please no.
> >
> > There is a reason why I use the device struct rather than the pci_dev
> > struct even though liveupdate currently only works with PCI devices.
> > It comes down to the fact that the pci_bus and pci_host_bridge are not
> > pci_dev struct. We need something that is common across all those
> > three types of PCI related struct I care about(pci_dev, pci_bus,
> > pci_host_bridge). The device struct is just common around those. I can
> > move the dev_liveupdate struct into pci_bus, pci_host_bridge and
> > pci_dev independently. That will be more contained inside PCI, not
> > touching the device struct. The patch would be bigger because the data
> > structure is spread into different structs. Do you have a preference
> > which way to go?
>
> If you only are caring about one single driver, don't mess with a
> subsystem or the driver core, just change the driver. My objection here

It is more than just one driver, we have vfio-pci, idpf, pci-pf-stub
and possible nvme driver.
The change needs to happen in the PCI enumeration and probing as well,
that is outside of the driver code.

> was that you were claiming it was a PCI change, yet it was actually only
> touching the driver core which means that all devices in the systems for

In theory all the devices can be liveupdate preserved. But now we only
support PCI.
I can look into containing the change in PCI only and not touching the
device struct if that is what you mean. I recall I tried that
previously and failed because bus->bridge is a device struct rather
than pci_dev or pci_host_bridge. I can try harder not to touch device
structs. Patch will be bigger and more complex than this right now.
But at least the damage is limited to PCI only if successful.

> all Linux users will be affected.

I understand your concerns. I was wishing one day all devices could
support liveupdate, but that is not the case right now.

> > > Break this series out properly, get the driver core stuff working FIRST,
> > > then show how multiple busses will work with them (i.e. you usually need
> > > 3 to know if you got it right).
> >
> > Multiple buses you mean different types of bus, e.g. USB, PCI and
> > others or 3 pci_bus is good enough? Right now we have no intention to
> > support bus types other than PCI devices. The liveupdate is about
> > preserving the GPU context cross kernel upgrade. Suggestion welcome.
>
> So all of this is just for one single driver. Ugh. Just do it in the
> single driver then, don't mess with the driver core, or even the PCI

Not a single driver. It is the whole PCI core. Or in your book the
whole PCI is just one single driver?

> core. Just make it specific to the driver and then none of us will even
> notice the mess that this all creates :)

OK. Let me try that PCI only approach again and try it harder. I will
report back.

Thanks for the feedback.

Chris