Message-ID: <2025100142-slick-deserving-4aed@gregkh>
Date: Wed, 1 Oct 2025 07:13:15 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Chris Li <chrisl@...nel.org>
Cc: Pasha Tatashin <pasha.tatashin@...een.com>,
	Jason Gunthorpe <jgg@...pe.ca>, Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>, Len Brown <lenb@...nel.org>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-acpi@...r.kernel.org, David Matlack <dmatlack@...gle.com>,
	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 06/10] PCI/LUO: Save and restore driver name

On Tue, Sep 30, 2025 at 08:41:29AM -0700, Chris Li wrote:
> On Tue, Sep 30, 2025 at 6:41 AM Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> >
> > On Tue, Sep 30, 2025 at 09:02:44AM -0400, Pasha Tatashin wrote:
> > > On Mon, Sep 29, 2025 at 10:10 PM Chris Li <chrisl@...nel.org> wrote:
> > > >
> > > > On Mon, Sep 29, 2025 at 10:57 AM Jason Gunthorpe <jgg@...pe.ca> wrote:
> > > > >
> > > > > On Tue, Sep 16, 2025 at 12:45:14AM -0700, Chris Li wrote:
> > > > > > Save the PCI driver name into "struct pci_dev_ser" during the PCI
> > > > > > prepare callback.
> > > > > >
> > > > > > After kexec, use driver_set_override() to ensure the device is
> > > > > > bound only to the saved driver.
> > > > >
> > > > > This doesn't seem like a great idea, driver name should not be made
> > > > > ABI.
> > > >
> > > > Let's break it down with baby steps.
> > > >
> > > > 1) Do you agree the liveupdated PCI device needs to bind to the exact
> > > > same driver after kexec?
> > > > To me that is a firm yes. If the driver binds to another driver, we
> > > > can't expect the other driver will understand the original driver's
> > > > saved state.
> > >
> > > Hi Chris,
> > >
> > > Driver name does not have to be an ABI.
> >
> > A driver name can NEVER be an abi, please don't do that.
> 
> Can you please clarify that.
> 
> for example, the pci has this sysfs control api:
> 
> "/sys/bus/pci/devices/0000:04:00.0/driver_override" which takes the
> *driver name* as data to override what driver is allowed to bind to
> this device.
> Does this driver_override consider it as using the driver name as part
> of the abi? If not, why?

Because the bind/unbind/override was created as a debug facility for
doing kernel development and then people have turned it into a "let's
operate our massive cloud systems with this fragile feature".

We have never said that driver names will remain the same across
releases, and they have changed over time.  Device ids have also moved
from one driver to another as well, making the "control" of the device
seem to have changed names.

> What live update wants is to make that driver_override persistent over
> kexec. It does not introduce the "driver_override" API. That is
> pre-existing conditions. The PCI liveupdate just wants to use it.

That does not mean that this is the correct api to use at all.  Again,
this was a debugging aid, to help with users who wanted to add a device
id to a driver without having to rebuild it.  Don't make it something
that it was never intended to be.

Why not just make a new api as you are doing something new here?  That
way you get to define it to work exactly the way you need?

> I want to get some basic understanding before adventure into the more
> complex solutions.

You mean "real" solutions :)

> > > Drivers that support live
> > > updates should provide a live update-specific ABI to detect
> > > compatibility with the preserved data. We can use a preservation
> > > schema GUID for this.
> > >
> > > > 2) Assume the 1) is yes from you. Are you just not happy that the
> > > > kernel saves the driver name? You want user space to save it, is that
> > > > it?
> > > > How does it reference the driver after kexec otherwise?
> > >
> > > If we use GUID, drivers would advertise the GUIDs they support and we
> > > would modify the core device-driver matching process to use this
> > > information.
> > >
> > > Each driver that supports this mechanism would need to declare an
> > > array of GUIDs it is compatible with. This would be a new field in its
> > > struct pci_driver.
> > >
> > > static const guid_t my_driver_guids[] = {
> > >     GUID_INIT(0x123e4567, ...), // Schema V1
> > >     GUID_INIT(0x987a6543, ...), // Schema V2
> > >     {},
> > > };
> >
> > That's crazy, who is going to be adding all of that to all drivers?  And
> > knowing to bump this if the internal data representaion changes?  And it
> > will change underneath it without the driver even knowing?  This feels
> > really really wrong, unless I'm missing something.
> 
> The GUID is more complex than a driver name. I am fine with not using
> GUID if you are so strongly opposed to it.
> 
> You are saying don't do A(driver name) and B(GUID). I am waiting for
> the part where you say "please do C instead".

It's not my requirement to say "here is C", but rather I am saying "B is
not going to scale over time as GUIDs are a pain to manage".

> Do you have any other suggestion how to prevent the live update PCI
> device bind to a different driver after kexec? I am happy to work on
> the direction you point out and turn that into a patch for the
> discussion purpose.

Why prevent it?  Why not just have a special api just for drivers that
want to use this new feature?

thanks,

greg k-h

Powered by blists - more mailing lists