PCI: liveupdate: PCI core support for Live Update

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing kho_is_enabled() check before calling KHO preservation APIs in PCI Live Update FLB operations. --

commit b0cb2fb154fba1aa85d63570fe0186e243353fb7
Author: David Matlack <[email protected]>

PCI: liveupdate: Set up FLB handler for the PCI core

This commit sets up a File-Lifecycle-Bound (FLB) handler for the PCI core to participate in the preservation of PCI devices across Live Update. It allocates and preserves a pci_ser structure using KHO when the first PCI device file is preserved.

Quoted 4 lines

drivers/pci/liveupdate.c new file +0 -0

new file mode 100644

[ ... ]

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+static int pci_flb_preserve(struct liveupdate_flb_op_args *args)
+{
+     struct pci_dev *dev = NULL;
+     u32 max_nr_devices = 0;
+     struct pci_ser *ser;
+     unsigned long size;
+
+     /*
+      * Allocate enough space to preserve all devices that are currently
+      * present on the system. Extra padding can be added to this in the

drivers/pci/liveupdate.c line 70

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+      * future to increase the chances that there is enough room to preserve
+      * devices that are not yet present on the system (e.g. VFs, hot-plugged
+      * devices).
+      */
+     for_each_pci_dev(dev)
+             max_nr_devices++;
+
+     size = struct_size_t(struct pci_ser, devices, max_nr_devices);
+
+     ser = kho_alloc_preserve(size);

[Severity: Medium]
Should this code verify kho_is_enabled() before calling kho_alloc_preserve()?

The commit message states:
"Note: There is no reason to check for kho_is_enabled() since it can be assumed to return true. If KHO was not enabled then Live Update would not be enabled and these routines would never run."

Does this assumption conflict with the Kexec Handover subsystem guidelines?

The guidelines mandate that all callers of serialization APIs must explicitly gate their usage with kho_is_enabled().

Calling preservation APIs like kho_alloc_preserve() without the check can silently add tracking state that will never be used if the subsystem is unexpectedly inactive.

Quoted 56 lines · On Fri, May 22, 2026 at 1:44 PM <[email protected]> wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing kho_is_enabled() check before calling KHO preservation APIs in PCI Live Update FLB operations. --

commit b0cb2fb154fba1aa85d63570fe0186e243353fb7
Author: David Matlack <[email protected]>

PCI: liveupdate: Set up FLB handler for the PCI core

This commit sets up a File-Lifecycle-Bound (FLB) handler for the PCI core to participate in the preservation of PCI devices across Live Update. It allocates and preserves a pci_ser structure using KHO when the first PCI device file is preserved.

Quoted 4 lines

drivers/pci/liveupdate.c new file +0 -0

new file mode 100644

[ ... ]

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+static int pci_flb_preserve(struct liveupdate_flb_op_args *args)
+{
+     struct pci_dev *dev = NULL;
+     u32 max_nr_devices = 0;
+     struct pci_ser *ser;
+     unsigned long size;
+
+     /*
+      * Allocate enough space to preserve all devices that are currently
+      * present on the system. Extra padding can be added to this in the

drivers/pci/liveupdate.c line 70

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+      * future to increase the chances that there is enough room to preserve
+      * devices that are not yet present on the system (e.g. VFs, hot-plugged
+      * devices).
+      */
+     for_each_pci_dev(dev)
+             max_nr_devices++;
+
+     size = struct_size_t(struct pci_ser, devices, max_nr_devices);
+
+     ser = kho_alloc_preserve(size);

[Severity: Medium]
Should this code verify kho_is_enabled() before calling kho_alloc_preserve()?

The commit message states:
"Note: There is no reason to check for kho_is_enabled() since it can be assumed to return true. If KHO was not enabled then Live Update would not be enabled and these routines would never run."

Does this assumption conflict with the Kexec Handover subsystem guidelines?

The guidelines mandate that all callers of serialization APIs must explicitly gate their usage with kho_is_enabled().

Quoted 3 lines

Calling preservation APIs like kho_alloc_preserve() without the check can silently add tracking state that will never be used if the subsystem is unexpectedly inactive.

Sashiko keeps leaving this comment. It seems like a false positive to me. KHO maintainers, perhaps the KHO documentation needs adjustment?

Quoted 57 lines · David Matlack

Quoted 56 lines · On Fri, May 22, 2026 at 1:44 PM <[email protected]> wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing kho_is_enabled() check before calling KHO preservation APIs in PCI Live Update FLB operations. --

commit b0cb2fb154fba1aa85d63570fe0186e243353fb7
Author: David Matlack <[email protected]>

PCI: liveupdate: Set up FLB handler for the PCI core

This commit sets up a File-Lifecycle-Bound (FLB) handler for the PCI core to participate in the preservation of PCI devices across Live Update. It allocates and preserves a pci_ser structure using KHO when the first PCI device file is preserved.

Quoted 4 lines

drivers/pci/liveupdate.c new file +0 -0

new file mode 100644

[ ... ]

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+static int pci_flb_preserve(struct liveupdate_flb_op_args *args)
+{
+     struct pci_dev *dev = NULL;
+     u32 max_nr_devices = 0;
+     struct pci_ser *ser;
+     unsigned long size;
+
+     /*
+      * Allocate enough space to preserve all devices that are currently
+      * present on the system. Extra padding can be added to this in the

drivers/pci/liveupdate.c line 70

Quoted 10 lines

drivers/pci/liveupdate.c new file +10 -0

+      * future to increase the chances that there is enough room to preserve
+      * devices that are not yet present on the system (e.g. VFs, hot-plugged
+      * devices).
+      */
+     for_each_pci_dev(dev)
+             max_nr_devices++;
+
+     size = struct_size_t(struct pci_ser, devices, max_nr_devices);
+
+     ser = kho_alloc_preserve(size);

[Severity: Medium]
Should this code verify kho_is_enabled() before calling kho_alloc_preserve()?

The commit message states:
"Note: There is no reason to check for kho_is_enabled() since it can be assumed to return true. If KHO was not enabled then Live Update would not be enabled and these routines would never run."

Does this assumption conflict with the Kexec Handover subsystem guidelines?

The guidelines mandate that all callers of serialization APIs must explicitly gate their usage with kho_is_enabled().

Quoted 6 lines

Quoted 3 lines

Calling preservation APIs like kho_alloc_preserve() without the check can silently add tracking state that will never be used if the subsystem is unexpectedly inactive.

Sashiko keeps leaving this comment. It seems like a false positive to me. KHO maintainers, perhaps the KHO documentation needs adjustment?

TBH, I don't remember such guidelines, but patches are welcome ;-)

Set up a File-Lifecycle-Bound (FLB) handler for the PCI core to enable it to participate in the preservation of PCI devices across Live Update. Essentially, this commit enables the PCI core to allocate a struct (struct pci_ser) and preserve it across a Live Update whenever at least one device is preserved.

Preserving PCI devices across Live Update is built on top of the Live Update Orchestrator's (LUO) support for file preservation. Drivers are expected to expose a file to userspace to represent a single PCI device and support preservation of that file. This is intended primarily to support preservation of PCI devices bound to VFIO drivers.

This commit enables drivers to register their liveupdate_file_handler with the PCI core so that the PCI core can do its own tracking and enforcement of which devices are preserved.

  pci_liveupdate_register_flb(driver_file_handler);
  pci_liveupdate_unregister_flb(driver_file_handler);

When the first file (with a handler registered with the PCI core) is preserved, the PCI core will be notified to allocate its tracking struct (pci_ser). When the last file is unpreserved (i.e. preservation cancelled) the PCI core will be notified to free struct pci_ser.

This struct is preserved across a Live Update using KHO and can be fetched by the PCI core during early boot (e.g. during device enumeration) so that it knows which devices were preserved.

Note: This commit only allocates struct pci_ser and preserves it across Live Update. A subsequent commit will add an API for drivers to tell the PCI core exactly which devices are being preserved.

Note: There is no reason to check for kho_is_enabled() since it can be assumed to return true. If KHO was not enabled then Live Update would not be enabled and these routines would never run.

include/linux/kho/abi/pci.h new file +22 -0

+/**
+ * struct pci_dev_ser - Serialized state about a single PCI device.
+ *
+ * @domain: The device's PCI domain number (segment).
+ * @bdf: The device's PCI bus, device, and function number.
+ * @padding: Padding to naturally align struct pci_dev_ser.
+ */
+struct pci_dev_ser {
+	u32 domain;
+	u16 bdf;
+	u16 padding;
+} __packed;
+
+/**
+ * struct pci_ser - PCI Subsystem Live Update State
+ *
+ * This struct tracks state about all devices that are being preserved across
+ * a Live Update for the next kernel.
+ *
+ * @max_nr_devices: The length of the devices[] flexible array.
+ * @nr_devices: The number of devices that were preserved.
+ * @devices: Flexible array of pci_dev_ser structs for each device.

Quoted 36 lines · David Matlack

Set up a File-Lifecycle-Bound (FLB) handler for the PCI core to enable it to participate in the preservation of PCI devices across Live Update. Essentially, this commit enables the PCI core to allocate a struct (struct pci_ser) and preserve it across a Live Update whenever at least one device is preserved.

Preserving PCI devices across Live Update is built on top of the Live Update Orchestrator's (LUO) support for file preservation. Drivers are expected to expose a file to userspace to represent a single PCI device and support preservation of that file. This is intended primarily to support preservation of PCI devices bound to VFIO drivers.

This commit enables drivers to register their liveupdate_file_handler with the PCI core so that the PCI core can do its own tracking and enforcement of which devices are preserved.

  pci_liveupdate_register_flb(driver_file_handler);
  pci_liveupdate_unregister_flb(driver_file_handler);

When the first file (with a handler registered with the PCI core) is preserved, the PCI core will be notified to allocate its tracking struct (pci_ser). When the last file is unpreserved (i.e. preservation cancelled) the PCI core will be notified to free struct pci_ser.

This struct is preserved across a Live Update using KHO and can be fetched by the PCI core during early boot (e.g. during device enumeration) so that it knows which devices were preserved.

Note: This commit only allocates struct pci_ser and preserves it across Live Update. A subsequent commit will add an API for drivers to tell the PCI core exactly which devices are being preserved.

Note: There is no reason to check for kho_is_enabled() since it can be assumed to return true. If KHO was not enabled then Live Update would not be enabled and these routines would never run.

[...]

Quoted 29 lines

include/linux/kho/abi/pci.h new file +29 -0

+/**
+ * struct pci_dev_ser - Serialized state about a single PCI device.
+ *
+ * @domain: The device's PCI domain number (segment).
+ * @bdf: The device's PCI bus, device, and function number.
+ * @padding: Padding to naturally align struct pci_dev_ser.
+ */
+struct pci_dev_ser {
+	u32 domain;
+	u16 bdf;
+	u16 padding;
+} __packed;
+
+/**
+ * struct pci_ser - PCI Subsystem Live Update State
+ *
+ * This struct tracks state about all devices that are being preserved across
+ * a Live Update for the next kernel.
+ *
+ * @max_nr_devices: The length of the devices[] flexible array.
+ * @nr_devices: The number of devices that were preserved.
+ * @devices: Flexible array of pci_dev_ser structs for each device.
+ */
+struct pci_ser {
+	u32 max_nr_devices;
+	u32 nr_devices;
+	struct pci_dev_ser devices[];
+} __packed;
+

include/linux/kho/abi/pci.h line 59

Quoted 3 lines

include/linux/kho/abi/pci.h new file +3 -0

+/* Ensure all elements of devices[] are naturally aligned. */
+static_assert(offsetof(struct pci_ser, devices) % sizeof(unsigned long) == 0);
+static_assert(sizeof(struct pci_dev_ser) % sizeof(unsigned long) == 0);

Minor Nit: Shall we consider using specific bitwidth types here? I'm wondering if down the line another u32 field is added to struct pci_dev_ser.. in that case on a 32-bit machine 12 % 4 == 0 but on a 64-bit machine 12 % 8 != 0..

[...]

With the nit:

Reviewed-by Pranjal Shrivastava <[email protected]>

Thanks,
Praan

Quoted 13 lines · Pranjal Shrivastava

Quoted 12 lines · David Matlack

Set up a File-Lifecycle-Bound (FLB) handler for the PCI core to enable it to participate in the preservation of PCI devices across Live Update. Essentially, this commit enables the PCI core to allocate a struct (struct pci_ser) and preserve it across a Live Update whenever at least one device is preserved.

Preserving PCI devices across Live Update is built on top of the Live Update Orchestrator's (LUO) support for file preservation. Drivers are expected to expose a file to userspace to represent a single PCI device and support preservation of that file. This is intended primarily to support preservation of PCI devices bound to VFIO drivers.

Quoted 15 lines

Quoted 15 lines

+ * struct pci_ser - PCI Subsystem Live Update State
+ *
+ * This struct tracks state about all devices that are being preserved across
+ * a Live Update for the next kernel.
+ *
+ * @max_nr_devices: The length of the devices[] flexible array.
+ * @nr_devices: The number of devices that were preserved.
+ * @devices: Flexible array of pci_dev_ser structs for each device.
+ */

include/linux/kho/abi/pci.h new file +6 -0

+struct pci_ser {
+	u32 max_nr_devices;
+	u32 nr_devices;
+	struct pci_dev_ser devices[];
+} __packed;
+

Quoted 8 lines

include/linux/kho/abi/pci.h line 59

Quoted 3 lines

include/linux/kho/abi/pci.h new file +3 -0

+/* Ensure all elements of devices[] are naturally aligned. */
+static_assert(offsetof(struct pci_ser, devices) % sizeof(unsigned long) == 0);
+static_assert(sizeof(struct pci_dev_ser) % sizeof(unsigned long) == 0);

Minor Nit: Shall we consider using specific bitwidth types here? I'm wondering if down the line another u32 field is added to struct pci_dev_ser.. in that case on a 32-bit machine 12 % 4 == 0 but on a 64-bit machine 12 % 8 != 0..

I think natural alignment is what matters for efficient access of the array elements. So failing the assert only on 64-bit architectures seems like the correct behavior.

drivers/pci/liveupdate.c new file +15 -0

new file mode 100644
index 000000000000..737e7b9366db
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (c) 2026, Google LLC.
+ * David Matlack <[email protected]>
+ */
+
+/**
+ * DOC: PCI Live Update
+ *
+ * The PCI subsystem participates in the Live Update process to enable drivers
+ * to preserve their PCI devices across kexec.
+ *
+ * File-Lifecycle-Bound (FLB) Data
+ * ===============================

+ *
+ * PCI device preservation across Live Update is built on top of the Live Update
+ * Orchestrator's (LUO) support for file preservation across kexec. Drivers

include/linux/pci_liveupdate.h new file +0 -0

new file mode 100644
index 000000000000..8ec98beefcb4
@@ -0,0 +1,30 @@

[ ... skip 24 lines ... ]

Quoted 8 lines · David Matlack

MAINTAINERS +0 -0

index 2fb1c75afd16..6c618830cf61 100644
@@ -20530,6 +20530,16 @@ L:	[email protected]
 S:	Supported
 F:	Documentation/PCI/pci-error-recovery.rst
 

MAINTAINERS L: [email protected] · line 20534

Quoted 2 lines

MAINTAINERS +2 -0

+PCI LIVE UPDATE
+M:	David Matlack <[email protected]>

Please add Pratyush, Mike, and myself so we are notified directly of incoming patches, the same as with other areas where the liveupdate/ tree is specified.

Quoted 5 lines · Pasha Tatashin

On Fri, 22 May 2026 20:23:59 +0000, David Matlack <[email protected]> wrote:

Please add Pratyush, Mike, and myself so we are notified directly of incoming patches, the same as with other areas where the liveupdate/ tree is specified.

Or we can add PCI liveupdate files to LIVEUPDATE entry.

Quoted 9 lines · Mike Rapoport

Quoted 5 lines · Pasha Tatashin

On Fri, 22 May 2026 20:23:59 +0000, David Matlack <[email protected]> wrote:

Please add Pratyush, Mike, and myself so we are notified directly of incoming patches, the same as with other areas where the liveupdate/ tree is specified.

Or we can add PCI liveupdate files to LIVEUPDATE entry.

That will not work, as we cannot serve as maintainers for PCI/VFIO/IOMMU/KVM, etc. David Matlack will be the maintainer for the PCI components, and we will accept patches once they have been approved by him.

The simplification we could do is to create an email alias for the live-update tree maintainers. This would allow us to use a single entry instead of listing all three of us individually.

Quoted 16 lines · Pasha Tatashin

Quoted 9 lines · Mike Rapoport

Quoted 5 lines · Pasha Tatashin

On Fri, 22 May 2026 20:23:59 +0000, David Matlack <[email protected]> wrote:

Please add Pratyush, Mike, and myself so we are notified directly of incoming patches, the same as with other areas where the liveupdate/ tree is specified.

Or we can add PCI liveupdate files to LIVEUPDATE entry.

That will not work, as we cannot serve as maintainers for PCI/VFIO/IOMMU/KVM, etc. David Matlack will be the maintainer for the PCI components, and we will accept patches once they have been approved by him.

Quoted 3 lines

The simplification we could do is to create an email alias for the live-update tree maintainers. This would allow us to use a single entry instead of listing all three of us individually.

We could create a Live Update mailing list for all code that can be CCed on all patches that must be merged through the Live Update tree. I would also be interested in subscribing to that list.

Quoted 21 lines · David Matlack

Quoted 19 lines · Pasha Tatashin

Quoted 9 lines · Mike Rapoport

Quoted 5 lines · Pasha Tatashin

On Fri, 22 May 2026 20:23:59 +0000, David Matlack <[email protected]> wrote:

Please add Pratyush, Mike, and myself so we are notified directly of incoming patches, the same as with other areas where the liveupdate/ tree is specified.

Or we can add PCI liveupdate files to LIVEUPDATE entry.

That will not work, as we cannot serve as maintainers for PCI/VFIO/IOMMU/KVM, etc. David Matlack will be the maintainer for the PCI components, and we will accept patches once they have been approved by him.

The simplification we could do is to create an email alias for the live-update tree maintainers. This would allow us to use a single entry instead of listing all three of us individually.

Quoted 3 lines

We could create a Live Update mailing list for all code that can be CCed on all patches that must be merged through the Live Update tree. I would also be interested in subscribing to that list.

+1. I'd like if there's a specific Live Update mailing list for submissions & discussion about the Live Update tree.

Quoted 2 lines · David Matlack

+ * PCI device preservation across Live Update is built on top of the Live Update
+ * Orchestrator's (LUO) support for file preservation across kexec. Drivers

I prefer to just use acronyms FLB, and LUO, but have links to the actual documentations about them.

So, something like this:

* :ref:FLB <flb> Data
* =====================
*
* PCI device preservation across Live Update is built on top of the
* :ref:LUO <luo> support for file preservation across kexec. Drivers

And also add _luo and _flb to Documentation/core-api/liveupdate.rst

.. _luo:

========================
Live Update Orchestrator
========================

.. _flb:

LUO File Lifecycle Bound Global Data
====================================

Review

Quoted 7 lines

[ ... skip 17 lines ... ]
+ *
+ * * ``pci_liveupdate_register_flb(driver_file_handler)``
+ * * ``pci_liveupdate_unregister_flb(driver_file_handler)``
+ */

(quoted diff) +2 -0

+
+#define pr_fmt(fmt) "PCI: liveupdate: " fmt

Nit, may be:

drivers/pci/liveupdate.c line 48

Quoted 8 lines

drivers/pci/liveupdate.c new file +7 -0

+
+#include <linux/io.h>
+#include <linux/kexec_handover.h>
+#include <linux/kho/abi/pci.h>
+#include <linux/liveupdate.h>
+#include <linux/mutex.h>
+#include <linux/mm.h>

Please sort alphabetically.

drivers/pci/liveupdate.c line 66

Quoted 8 lines

[ ... skip 12 lines ... ]

drivers/pci/liveupdate.c new file +6 -0

+	 * future to increase the chances that there is enough room to preserve
+	 * devices that are not yet present on the system (e.g. VFs, hot-plugged
+	 * devices).
+	 */
+	for_each_pci_dev(dev)
+		max_nr_devices++;

I think, we want to use kho_block [1] (it is in liveupdate/next branch) to allow number of supported devices to be dynamic.

To support this, we would redefine the ABI and tracking structures like so:

/* include/linux/kho/abi/pci.h */
struct pci_ser {
u64 devices; /* Phys address of the first block header of kho_block_set */
u64 nr_devices; /* Total count of active preserved devices */
} __packed;

/* drivers/pci/liveupdate.c */
struct pci_flb_outgoing {
struct pci_ser *ser; /* Points to the FDT/KHO-allocated ABI struct */
struct kho_block_set block_set; /* Controls the active blocks on the fly */
};

In __pci_liveupdate_preserve_device() , we would search for and reuse any inactive pci_dev_ser slot first, and only call kho_block_set_grow() to expand if no inactive slots are available.

In pci_liveupdate_unpreserve_device(), we would simply mark the pci_dev_ser as inactive.

Quoted 9 lines

include/linux/pci_liveupdate.h new file +0 -0

new file mode 100644
index 000000000000..8ec98beefcb4
@@ -0,0 +1,30 @@

[ ... skip 24 lines ... ]

include/linux/pci_liveupdate.h line 30

Quoted 6 lines

include/linux/pci_liveupdate.h new file +6 -0

+static inline void pci_liveupdate_unregister_flb(struct liveupdate_file_handler *fh)
+{
+}
+#endif
+
+#endif /* LINUX_PCI_LIVEUPDATE_H */

[1] https://lore.kernel.org/all/[email protected]/

Preserving: In __pci_liveupdate_preserve_device() , we would search for
Unpreserving: In pci_liveupdate_unpreserve_device(), we would simply

Preserving: In __pci_liveupdate_preserve_device() , we would search for
Unpreserving: In pci_liveupdate_unpreserve_device(), we would simply

Signature · 1 line

Pasha Tatashin <[email protected]>

Quoted 25 lines · Pasha Tatashin

Quoted 1 line · David Matlack

+ * PCI device preservation across Live Update is built on top of the Live Update

I prefer to just use acronyms FLB, and LUO, but have links to the actual documentations about them.

So, something like this:

* :ref:FLB <flb> Data
* =====================
*
* PCI device preservation across Live Update is built on top of the
* :ref:LUO <luo> support for file preservation across kexec. Drivers

And also add _luo and _flb to Documentation/core-api/liveupdate.rst

.. _luo:

========================
Live Update Orchestrator
========================

Quoted 1 line

.. _flb:

Will do.

I guess I will need to add another patch to add the link references to liveupdate.rst?

Quoted 5 lines

LUO File Lifecycle Bound Global Data
====================================

Nit, may be:

Did you have a suggestion here that got lost?

Quoted 2 lines

Please sort alphabetically.

Will do.

Quoted 18 lines

I think, we want to use kho_block [1] (it is in liveupdate/next branch) to allow number of supported devices to be dynamic.

To support this, we would redefine the ABI and tracking structures like so:

/* include/linux/kho/abi/pci.h */
struct pci_ser {
u64 devices; /* Phys address of the first block header of kho_block_set */
u64 nr_devices; /* Total count of active preserved devices */
} __packed;

/* drivers/pci/liveupdate.c */
struct pci_flb_outgoing {
struct pci_ser *ser; /* Points to the FDT/KHO-allocated ABI struct */
struct kho_block_set block_set; /* Controls the active blocks on the fly */
};

Quoted 6 lines

In __pci_liveupdate_preserve_device() , we would search for and reuse any inactive pci_dev_ser slot first, and only call kho_block_set_grow() to expand if no inactive slots are available.

In pci_liveupdate_unpreserve_device(), we would simply mark the pci_dev_ser as inactive.

Makes sense at a high level. I'll work on switching kho_block for v7 and get back to you if I hit any issues.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Acquiring a sleepable rw_semaphore (rwsem_write) during PCI device release, which can occur in atomic contexts. --

drivers/pci/liveupdate.c static struct liveupdate_flb pci_liveupdate_flb = { · line 281

Quoted 9 lines

drivers/pci/liveupdate.c +9 -0

+void pci_liveupdate_cleanup_device(struct pci_dev *dev)
+{
+     /*
+      * It should be safe to READ_ONCE() outside of the rwsem during cleanup
+      * since there should no longer be any references to @dev on the system.
+      */
+     if (READ_ONCE(dev->liveupdate.outgoing)) {
+             pci_WARN(dev, 1, "Destroying outgoing-preserved device!\n");
+             pci_liveupdate_unpreserve(dev);

[Severity: High]
Can calling pci_liveupdate_unpreserve() here cause a sleep-in-atomic bug?

pci_liveupdate_unpreserve() unconditionally acquires a sleepable rw_semaphore:
guard(rwsem_write)(&pci_liveupdate.rwsem).

Since pci_liveupdate_cleanup_device() is called from pci_release_dev(), this runs when the last reference to a struct pci_dev is dropped via pci_dev_put().

Because pci_dev_put() can be called from atomic contexts (such as hardware interrupt handlers or while holding spinlocks) to drop references acquired during lookups, acquiring a sleepable lock here could trigger a scheduling-while-atomic BUG or deadlock.

drivers/pci/probe.c static void pci_release_dev(struct device *dev) · line 2497

Quoted 8 lines

drivers/pci/probe.c +1 -0

@@ -2490,6 +2491,7 @@ static void pci_release_dev(struct device *dev)
 
      pci_dev = to_pci_dev(dev);
      pci_release_capabilities(pci_dev);
+     pci_liveupdate_cleanup_device(pci_dev);
      pci_release_of_node(pci_dev);
      pcibios_release_device(pci_dev);
      pci_bus_put(pci_dev->bus);

Gemini found 3 files where pci_dev_put() is called under a spinlock:

- drivers/net/ethernet/microsoft/mana/gdma_main.c (mana_driver_exit())
- arch/x86/pci/i386.c (pcibios_fw_addr_list_del())
- arch/powerpc/platforms/pasemi/dma_lib.c (pasemi_dma_init())

OTOH, pci_release_dev() does not seem atomic-safe. On s390, it can take the zpci_add_remove_lock mutex via pcibios_release_device(). And on all architectures it can call kernfs_remove() via pci_bus_put(), which takes multiple semaphores.

Add APIs to allow drivers to notify the PCI core of which devices are being preserved across a Live Update for the next kernel, i.e. "outgoing" devices.

Drivers must notify the PCI core when devices are preserved so that the PCI core can update its FLB data (struct pci_ser) and track the list of outgoing devices. pci_liveupdate_preserve() notifies the PCI core that a device must be preserved across Live Update. pci_liveupdate_unpreserve() reverses this (cancels the preservation of the device).

This tracking ensures the PCI core is fully aware of which devices may need special handling during shutdown and kexec, and so that it can be handed off to the next kernel.

Signed-off-by David Matlack <[email protected]>

/**
* struct pci_dev_ser - Serialized state about a single PCI device. *
* @domain: The device's PCI domain number (segment).
* @bdf: The device's PCI bus, device, and function number.
- * @padding: Padding to naturally align struct pci_dev_ser.
+ * @refcount: Reference count used by the PCI core to keep track of whether it

+ *            is done using a device's struct pci_dev_ser. The value of the
+ *            refcount is equal to 1 when the struct pci_dev_ser is in use, and
+ *            0 otherwise.

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] pci_flb_retrieve() frees the incoming preserved memory (ser) on failure, causing a Use-After-Free when the core retries.
- [Medium] Leaked reference to the incoming FLB when a device's preserved refcount is corrupted.
--

commit 5af02c95c00f317cc5c4fa203b8b79373f2759a5
Author: David Matlack <[email protected]>

PCI: liveupdate: Track incoming preserved PCI devices

During PCI enumeration, the PCI core fetches serialized state from the Live Update Orchestrator to identify incoming preserved devices. This patch adds tracking and lifecycle management APIs for these devices.

Quoted 4 lines

drivers/pci/liveupdate.c +0 -0

index 065d5af822f74..96c43b84532cc 100644

[ ... ]

Quoted 28 lines

drivers/pci/liveupdate.c +23 -1

@@ -128,13 +157,49 @@ static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args)
 
 static int pci_flb_retrieve(struct liveupdate_flb_op_args *args)
 {
-     args->obj = phys_to_virt(args->data);
+     struct pci_ser *ser = phys_to_virt(args->data);
+     struct pci_flb_incoming *incoming;
+     int ret = -ENOMEM;
+     u32 i;
+
+     incoming = kmalloc_obj(*incoming);
+     if (!incoming)
+             goto err_restore_free;
+
+     incoming->ser = ser;
+     xa_init(&incoming->xa);
+
+     for (i = 0; i < incoming->ser->max_nr_devices; i++) {
+             struct pci_dev_ser *dev_ser = &incoming->ser->devices[i];
+             unsigned long key;
+
+             if (!dev_ser->refcount)
+                     continue;
+
+             key = pci_ser_xa_key(dev_ser->domain, dev_ser->bdf);
+             ret = xa_insert(&incoming->xa, key, dev_ser, GFP_KERNEL);
+             if (ret)
+                     goto err_xa_destroy;

drivers/pci/liveupdate.c static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args) · line 192

Quoted 10 lines

drivers/pci/liveupdate.c +9 -0

+     }
+
+     args->obj = incoming;
      return 0;
+
+err_xa_destroy:
+     xa_destroy(&incoming->xa);
+     kfree(incoming);
+err_restore_free:
+     kho_restore_free(ser);

[Severity: Critical]
Does this error path create a use-after-free if the retrieval is retried?

If pci_flb_retrieve() fails here (e.g. from -ENOMEM), kho_restore_free() frees the preserved physical memory for the incoming state. However, the Live Update Orchestrator propagates this error without marking the FLB as retrieved or finished.

When the next PCI device is enumerated, pci_liveupdate_setup_device() calls pci_liveupdate_flb_get_incoming() again, which invokes pci_flb_retrieve() a second time with the exact same data address. The function will then attempt to read incoming->ser->max_nr_devices from memory that was already freed.

Quoted 3 lines

drivers/pci/liveupdate.c +1 -0

+     return ret;
 }

[ ... ]

Quoted 11 lines

drivers/pci/liveupdate.c +7 -0

@@ -280,7 +430,83 @@ void pci_liveupdate_cleanup_device(struct pci_dev *dev)
              pci_WARN(dev, 1, "Destroying outgoing-preserved device!\n");
              pci_liveupdate_unpreserve(dev);
      }
+
+     if (READ_ONCE(dev->liveupdate.incoming)) {
+             pci_WARN(dev, 1, "Destroying incoming-preserved device!\n");
+             pci_liveupdate_finish(dev);
+     }
+}
+

drivers/pci/liveupdate.c void pci_liveupdate_cleanup_device(struct pci_dev *dev) · line 449

Quoted 10 lines

drivers/pci/liveupdate.c +10 -0

+static void pci_liveupdate_finish_device(struct pci_ser *ser, struct pci_dev *dev)
+{
+     if (!dev->liveupdate.incoming) {
+             pci_warn(dev, "Cannot finish preserving an unpreserved device\n");
+             return;
+     }
+
+     if (dev->liveupdate.incoming->refcount != 1) {
+             pci_WARN(dev, 1, "Preserved device has a corrupted refcount!\n");
+             return;

[Severity: Medium]
Can this early return leak a reference to the incoming FLB?

In pci_liveupdate_setup_device(), a reference is held to protect dev->liveupdate.incoming. This reference is expected to be released via pci_liveupdate_flb_put_incoming() at the end of this function.

By returning early here when the refcount is corrupted, we skip the put call, leaking the reference. Furthermore, dev->liveupdate.incoming is left non-NULL, which means pci_liveupdate_cleanup_device() might later try to finish it again and hit this same leak. Should this error path drop the reference and clear the incoming pointer?

During PCI enumeration, the previous kernel might have passed state about devices that were preserved across kexec. The PCI core needs to fetch this state to identify which devices are "incoming" and require special handling.

Add pci_liveupdate_setup_device() which is called during device setup to fetch the serialized state (struct pci_ser) from the Live Update Orchestrator. The first time this happens, pci_flb_retrieve() will run and convert the array of pci_dev_ser structs into an xarray so that it can be looked up efficiently.

If a device is found in the xarray, the PCI core stores a pointer to its state in dev->liveupdate_incoming and holds a reference to the incoming FLB until pci_liveupdate_finish() is called by the driver.

This ensures proper lifecycle management for incoming preserved devices and allows the PCI core and drivers to apply specific Live Update logic to them in subsequent commits.

Drivers can check if a device is an incoming preserved device (e.g. during probe) by calling pci_liveupdate_is_incoming().

CONFIG_64BIT is now required to enable CONFIG_PCI_LIVEUPDATE so that the domain and bdf can be guaranteed to fit in an unsigned long and be used as the xarray key.

Signed-off-by David Matlack <[email protected]>

---
 MAINTAINERS                    |   1 +
 drivers/pci/Kconfig            |   2 +-
 drivers/pci/liveupdate.c       | 230 ++++++++++++++++++++++++++++++++-
 drivers/pci/liveupdate.h       |   5 +
 drivers/pci/probe.c            |   3 +
 include/linux/pci_liveupdate.h |  13 ++
 6 files changed, 251 insertions(+), 3 deletions(-)

MAINTAINERS +1 -0

index 6c618830cf61..0e262c0ceb43 100644
@@ -20537,6 +20537,7 @@ L:	[email protected]
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/liveupdate/linux.git
 F:	drivers/pci/liveupdate.c
+F:	drivers/pci/liveupdate.h
 F:	include/linux/kho/abi/pci.h
 F:	include/linux/pci_liveupdate.h
 

drivers/pci/Kconfig +0 -0

index 10c9b65aa242..e68ae5c172d4 100644

drivers/pci/Kconfig +0 -0

 	help
 	  Enable PCI core support for preserving PCI devices across Live
 	  Update. This, in combination with support in a device's driver,

static int pci_flb_retrieve(struct liveupdate_flb_op_args *args) {

drivers/pci/Kconfig +23 -1

-	args->obj = phys_to_virt(args->data);
+	struct pci_ser *ser = phys_to_virt(args->data);
+	struct pci_flb_incoming *incoming;
+	int ret = -ENOMEM;
+	u32 i;
+
+	incoming = kmalloc_obj(*incoming);
+	if (!incoming)
+		goto err_restore_free;
+
+	incoming->ser = ser;
+	xa_init(&incoming->xa);
+
+	for (i = 0; i < incoming->ser->max_nr_devices; i++) {
+		struct pci_dev_ser *dev_ser = &incoming->ser->devices[i];
+		unsigned long key;
+
+		if (!dev_ser->refcount)
+			continue;
+
+		key = pci_ser_xa_key(dev_ser->domain, dev_ser->bdf);
+		ret = xa_insert(&incoming->xa, key, dev_ser, GFP_KERNEL);
+		if (ret)
+			goto err_xa_destroy;

Quoted 54 lines · David Matlack

During PCI enumeration, the previous kernel might have passed state about devices that were preserved across kexec. The PCI core needs to fetch this state to identify which devices are "incoming" and require special handling.

Add pci_liveupdate_setup_device() which is called during device setup to fetch the serialized state (struct pci_ser) from the Live Update Orchestrator. The first time this happens, pci_flb_retrieve() will run and convert the array of pci_dev_ser structs into an xarray so that it can be looked up efficiently.

If a device is found in the xarray, the PCI core stores a pointer to its state in dev->liveupdate_incoming and holds a reference to the incoming FLB until pci_liveupdate_finish() is called by the driver.

This ensures proper lifecycle management for incoming preserved devices and allows the PCI core and drivers to apply specific Live Update logic to them in subsequent commits.

Drivers can check if a device is an incoming preserved device (e.g. during probe) by calling pci_liveupdate_is_incoming().

CONFIG_64BIT is now required to enable CONFIG_PCI_LIVEUPDATE so that the domain and bdf can be guaranteed to fit in an unsigned long and be used as the xarray key.

Signed-off-by David Matlack <[email protected]>

---
 MAINTAINERS                    |   1 +
 drivers/pci/Kconfig            |   2 +-
 drivers/pci/liveupdate.c       | 230 ++++++++++++++++++++++++++++++++-
 drivers/pci/liveupdate.h       |   5 +
 drivers/pci/probe.c            |   3 +
 include/linux/pci_liveupdate.h |  13 ++
 6 files changed, 251 insertions(+), 3 deletions(-)

MAINTAINERS +1 -0

index 6c618830cf61..0e262c0ceb43 100644
@@ -20537,6 +20537,7 @@ L:	[email protected]
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/liveupdate/linux.git
 F:	drivers/pci/liveupdate.c
+F:	drivers/pci/liveupdate.h
 F:	include/linux/kho/abi/pci.h
 F:	include/linux/pci_liveupdate.h
 

drivers/pci/Kconfig +0 -0

index 10c9b65aa242..e68ae5c172d4 100644
@@ -330,7 +330,7 @@ config VGA_ARB_MAX_GPUS
 

drivers/pci/Kconfig config VGA_ARB_MAX_GPUS · line 333

Quoted 4 lines

drivers/pci/Kconfig +1 -1

 config PCI_LIVEUPDATE
 	bool "PCI Live Update Support"
-	depends on PCI && LIVEUPDATE
+	depends on PCI && LIVEUPDATE && 64BIT

I see that the static assertions in Patch 1 work because of the 64BIT enforcement here. In that case, should we have the assertions check u64?

Quoted 5 lines

drivers/pci/Kconfig +0 -0

 	help
 	  Enable PCI core support for preserving PCI devices across Live
 	  Update. This, in combination with support in a device's driver,

[...]

Quoted 26 lines

static int pci_flb_retrieve(struct liveupdate_flb_op_args *args) {

drivers/pci/Kconfig +23 -1

-	args->obj = phys_to_virt(args->data);
+	struct pci_ser *ser = phys_to_virt(args->data);
+	struct pci_flb_incoming *incoming;
+	int ret = -ENOMEM;
+	u32 i;
+
+	incoming = kmalloc_obj(*incoming);
+	if (!incoming)
+		goto err_restore_free;
+
+	incoming->ser = ser;
+	xa_init(&incoming->xa);
+
+	for (i = 0; i < incoming->ser->max_nr_devices; i++) {
+		struct pci_dev_ser *dev_ser = &incoming->ser->devices[i];
+		unsigned long key;
+
+		if (!dev_ser->refcount)
+			continue;
+
+		key = pci_ser_xa_key(dev_ser->domain, dev_ser->bdf);
+		ret = xa_insert(&incoming->xa, key, dev_ser, GFP_KERNEL);
+		if (ret)
+			goto err_xa_destroy;

drivers/pci/Kconfig

Quoted 10 lines

drivers/pci/Kconfig +9 -0

+	}
+
+	args->obj = incoming;
 	return 0;
+
+err_xa_destroy:
+	xa_destroy(&incoming->xa);
+	kfree(incoming);
+err_restore_free:
+	kho_restore_free(ser);

I tend to partly agree with Sashiko[1] here.. it raises a policy-hole. We may need a policy here, the options I have in mind are:

1. Retrieve shall ONLY be tried once, if it fails (like -ENOMEM in the xArray alloc), it's a liveupdate failure. We can't retry liveupdate.

2. Retrying retrieve is allowed.

The only downside with option 1 is, the user may want flexibility due to certain subsystems OR may choose NOT to use the proposed LUOd and instead have its own user-space component which might try funny things or have a different use-case.

In such a situation, the system may have transiently run out of memory during the kexec transition (for e.g. a subsystem uses GFP_ATOMIC to allocate memory and temporarily runs out of the atomic pool). [Note we removed it in IOMMU v1 [2] but subsystems may have a use-case for it]

If the kernel frees the KHO page on the first failure, it removes any chance of recovery. :/

Thus, it might make sense to let the user decide if it wants to fail the liveupdate or retry again based on the failure type / source?

[...]

The changes LGTM, except for policy-based, kho_restore_free discussion.

Reviewed-by Pranjal Shrivastava <[email protected]>

Thanks,
Praan

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Quoted 55 lines · Pranjal Shrivastava

Quoted 54 lines · David Matlack

During PCI enumeration, the previous kernel might have passed state about devices that were preserved across kexec. The PCI core needs to fetch this state to identify which devices are "incoming" and require special handling.

Add pci_liveupdate_setup_device() which is called during device setup to fetch the serialized state (struct pci_ser) from the Live Update Orchestrator. The first time this happens, pci_flb_retrieve() will run and convert the array of pci_dev_ser structs into an xarray so that it can be looked up efficiently.

If a device is found in the xarray, the PCI core stores a pointer to its state in dev->liveupdate_incoming and holds a reference to the incoming FLB until pci_liveupdate_finish() is called by the driver.

This ensures proper lifecycle management for incoming preserved devices and allows the PCI core and drivers to apply specific Live Update logic to them in subsequent commits.

Drivers can check if a device is an incoming preserved device (e.g. during probe) by calling pci_liveupdate_is_incoming().

CONFIG_64BIT is now required to enable CONFIG_PCI_LIVEUPDATE so that the domain and bdf can be guaranteed to fit in an unsigned long and be used as the xarray key.

Signed-off-by David Matlack <[email protected]>

---
 MAINTAINERS                    |   1 +
 drivers/pci/Kconfig            |   2 +-
 drivers/pci/liveupdate.c       | 230 ++++++++++++++++++++++++++++++++-
 drivers/pci/liveupdate.h       |   5 +
 drivers/pci/probe.c            |   3 +
 include/linux/pci_liveupdate.h |  13 ++
 6 files changed, 251 insertions(+), 3 deletions(-)

MAINTAINERS +1 -0

index 6c618830cf61..0e262c0ceb43 100644
@@ -20537,6 +20537,7 @@ L:	[email protected]
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/liveupdate/linux.git
 F:	drivers/pci/liveupdate.c
+F:	drivers/pci/liveupdate.h
 F:	include/linux/kho/abi/pci.h
 F:	include/linux/pci_liveupdate.h
 

drivers/pci/Kconfig +0 -0

index 10c9b65aa242..e68ae5c172d4 100644
@@ -330,7 +330,7 @@ config VGA_ARB_MAX_GPUS
 

Quoted 7 lines

drivers/pci/Kconfig config VGA_ARB_MAX_GPUS · line 333

Quoted 4 lines

drivers/pci/Kconfig +1 -1

 config PCI_LIVEUPDATE
 	bool "PCI Live Update Support"
-	depends on PCI && LIVEUPDATE
+	depends on PCI && LIVEUPDATE && 64BIT

I see that the static assertions in Patch 1 work because of the 64BIT enforcement here. In that case, should we have the assertions check u64?

The static asserts have nothing to do with the 64BIT enforcement here. The static asserts just verify that the array elements in struct pci_ser are naturally aligned (unsigned long) so they can be accessed efficiently. The requirement here for CONFIG_64BIT is for the xarray key.

Theoretically if we got the xarray to work with 32-bit architectures then we could drop the CONFIG_64BIT requirement here.

Quoted 20 lines

drivers/pci/Kconfig config VGA_ARB_MAX_GPUS · line 334

Quoted 1 line

drivers/pci/Kconfig +1 -0

+	kho_restore_free(ser);

I tend to partly agree with Sashiko[1] here.. it raises a policy-hole. We may need a policy here, the options I have in mind are:

1. Retrieve shall ONLY be tried once, if it fails (like -ENOMEM in the xArray alloc), it's a liveupdate failure. We can't retry liveupdate.

2. Retrying retrieve is allowed.

The only downside with option 1 is, the user may want flexibility due to certain subsystems OR may choose NOT to use the proposed LUOd and instead have its own user-space component which might try funny things or have a different use-case.

In such a situation, the system may have transiently run out of memory during the kexec transition (for e.g. a subsystem uses GFP_ATOMIC to allocate memory and temporarily runs out of the atomic pool). [Note we removed it in IOMMU v1 [2] but subsystems may have a use-case for it]

Quoted 5 lines

If the kernel frees the KHO page on the first failure, it removes any chance of recovery. :/

Thus, it might make sense to let the user decide if it wants to fail the liveupdate or retry again based on the failure type / source?

The plan is to have LUO enforce that retrieve() is only called once:

https://lore.kernel.org/kexec/[email protected]/

Supporting retry gets complicated since there's many different places where retrieve() could have failed.

Quoted 6 lines · David Matlack

drivers/pci/Kconfig +0 -0

index 10c9b65aa242..e68ae5c172d4 100644
@@ -330,7 +330,7 @@ config VGA_ARB_MAX_GPUS
 

drivers/pci/Kconfig config VGA_ARB_MAX_GPUS · line 333

Quoted 4 lines

drivers/pci/Kconfig +1 -1

 config PCI_LIVEUPDATE
 	bool "PCI Live Update Support"
-	depends on PCI && LIVEUPDATE
+	depends on PCI && LIVEUPDATE && 64BIT

Please move this to the first patch, fewer changes between patches, and also KHO does not support anything but 64-bit mode.

Quoted 8 lines

drivers/pci/liveupdate.c +0 -0

index 065d5af822f7..96c43b84532c 100644
@@ -128,13 +157,49 @@ static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args)

[ ... skip 31 lines ... ]

drivers/pci/liveupdate.c static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args) · line 192

Quoted 6 lines

drivers/pci/liveupdate.c +6 -0

+
+err_xa_destroy:
+	xa_destroy(&incoming->xa);
+	kfree(incoming);
+err_restore_free:
+	kho_restore_free(ser);

This is the pattern we have been enforcing in other places in LUO. If the first retrieval fails, return the same error thereafter.

Quoted 4 lines

drivers/pci/liveupdate.c +0 -0

@@ -270,6 +335,91 @@ void pci_liveupdate_unpreserve(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);
 

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 341

Quoted 4 lines

drivers/pci/liveupdate.c +4 -0

+static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
+{
+	struct pci_flb_incoming *incoming = NULL;
+	int ret;

Maybe make the error return static, and avoid another search through compatible FLBs if it failed before?

1. Add "saved_err;"; if it is set, return it right away.
2. Change all errors to use goto save_err;, and at the end of the function, assign ret to saved_err;

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 362

Quoted 7 lines

[ ... skip 15 lines ... ]

drivers/pci/liveupdate.c +6 -0

+	 * This could mean that no PCI FLB data was passed by the previous
+	 * kernel, but it could also mean the previous kernel used a different
+	 * compatibility string (i.e. a different ABI).
+	 */
+	if (ret == -ENOENT) {
+		pr_info_once("No incoming FLB matched %s\n", pci_liveupdate_flb.compatible);

I would assume this is very normal, e.g., no devices were preserved but memfd+hugetlb was preserved. Maybe use pr_debug_once().

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 371

Quoted 10 lines

drivers/pci/liveupdate.c +9 -0

+		return NULL;
+	}
+
+	/*
+	 * There is incoming FLB data that matches pci_liveupdate_flb.compatible
+	 * but it cannot be retrieved.
+	 */
+	if (ret) {
+		WARN_ONCE(ret, "Failed to retrieve incoming FLB data\n");

No need to print backtrace, please just print a warning:
pr_warn_once("Failed to retrieve incoming FLB data: %pe\n", ERR_PTR(ret));

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 411

Quoted 8 lines

[ ... skip 34 lines ... ]

drivers/pci/liveupdate.c +6 -0

+	 * through pci_liveupdate_finish(). This can happen if PCI core probes
+	 * the same device multiple times, e.g. due to hotplug.
+	 */
+	if (!dev_ser->refcount) {
+		pci_liveupdate_flb_put_incoming();
+		return;

Pleaes use 'goto put_incoming'

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 420

Quoted 10 lines

drivers/pci/liveupdate.c +9 -0

+	}
+
+	pci_info(dev, "Device was preserved by previous kernel across Live Update\n");
+	dev->liveupdate.incoming = dev_ser;
+
+	/*
+	 * Hold the ref on the incoming FLB until pci_liveupdate_finish() so
+	 * that dev->liveupdate.incoming does not get freed while it is in use.
+	 */

How would that work? If finish is not called FLB stays around until the next reboot.

Signature · 2 lines

-- 
Pasha Tatashin <[email protected]>

Quoted 9 lines · Pasha Tatashin

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 341

Quoted 4 lines · David Matlack

drivers/pci/liveupdate.c +4 -0

+static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
+{
+   struct pci_flb_incoming *incoming = NULL;
+   int ret;

Maybe make the error return static, and avoid another search through compatible FLBs if it failed before?

Good idea, will do.

Quoted 1 line · Pasha Tatashin

On Fri, 22 May 2026 20:24:01 +0000, David Matlack <[email protected]> wrote:

Quoted 4 lines

Quoted 4 lines

drivers/pci/liveupdate.c +4 -0

+	}
+
+	pci_info(dev, "Device was preserved by previous kernel across Live Update\n");
+	dev->liveupdate.incoming = dev_ser;

Quoted 8 lines

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 420

Quoted 5 lines

drivers/pci/liveupdate.c +5 -0

+
+	/*
+	 * Hold the ref on the incoming FLB until pci_liveupdate_finish() so
+	 * that dev->liveupdate.incoming does not get freed while it is in use.
+	 */

How would that work? If finish is not called FLB stays around until the next reboot.

True... I think if the PCI core trusts drivers to call pci_liveupdate_finish() then we don't need to hold onto the incoming reference here.

Hi David,

Quoted 2 lines · David Matlack

Quoted 1 line · Pasha Tatashin

On Fri, 22 May 2026 20:24:01 +0000, David Matlack <[email protected]> wrote:

[...]

Quoted 13 lines

Quoted 4 lines

Quoted 4 lines

drivers/pci/liveupdate.c +4 -0

+  }
+
+  pci_info(dev, "Device was preserved by previous kernel across Live Update\n");
+  dev->liveupdate.incoming = dev_ser;

Quoted 8 lines

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 420

Quoted 5 lines

drivers/pci/liveupdate.c +5 -0

+
+  /*
+   * Hold the ref on the incoming FLB until pci_liveupdate_finish() so
+   * that dev->liveupdate.incoming does not get freed while it is in use.
+   */

How would that work? If finish is not called FLB stays around until the next reboot.

Quoted 3 lines

True... I think if the PCI core trusts drivers to call pci_liveupdate_finish() then we don't need to hold onto the incoming reference here.

That was my point when I was arguing against refcounts on outgoing FLBs. This is very easy to abuse, especially when we are talking about device drivers. And this refcounting mechanism makes the FLB no longer file-lifecycle-bound, since now it is entirely up to drivers to decide the lifecycle of this data.

I have been thinking about this a bit more in the last couple days, and I wonder if we are doing this right. Here's an idea I have been thinking of.

We should make live update a first class citizen in PCI. Instead of patching in liveupdate via the liveupdate.incoming field, and letting drivers figure out when to use it, we should separate out probe and retrieve paths entirely.

Probe and retrieve are fundamentally different operations. While they may share some common initialization logic for the _software_ state, how they interface with the hardware is completely different. I think mixing the two will result in driver code being more spaghetti by having liveupdate checks sprayed out all over.

This series doesn't add support for any drivers, but looking at some of the code we have downstream, I see this problem. The liveupdate code is all over the place in the driver and it is very hard to wrap one's head around how the device is actually retrieved.

So I think PCI core should track preserved devices, and if the device is preserved, it should skip the probe and wait for retrieve. Retrieve does the full initialization of the device. This fits in with the LUO model as well. You can make retrieve a callback of struct pci_driver and do some wrappers to talk with LUO, so device drivers don't directly interface with LUO at all.

We should do similar things on the shutdown path. Shutdown is a fundamentally different operation from freeze, and so we should separate them out as well.

This solves the lifetime problem as well. When PCI core is initializing, it knows for sure that no retrievals are going to happen. That's because none of the drivers have registered yet. So it can safely access the FLB and initialize its state. After that, drivers can register themselves and start accepting retrieve() calls. Once the last driver goes away, the FLB is freed automatically.

I am sorry for suggesting a big refactor at v6, but the early versions looked good to me at the time, and I only thought more deeply about this when trying to figure out how we can make the lifetimes cleaner.

What do you think? Does this make sense?

--
Regards,
Pratyush Yadav

Quoted 25 lines · Pratyush Yadav

Hi David,

Quoted 2 lines · David Matlack

Quoted 1 line · Pasha Tatashin

On Fri, 22 May 2026 20:24:01 +0000, David Matlack <[email protected]> wrote:

[...]

Quoted 13 lines

Quoted 4 lines

Quoted 4 lines

drivers/pci/liveupdate.c +4 -0

+  }
+
+  pci_info(dev, "Device was preserved by previous kernel across Live Update\n");
+  dev->liveupdate.incoming = dev_ser;

Quoted 8 lines

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 420

Quoted 5 lines

drivers/pci/liveupdate.c +5 -0

+
+  /*
+   * Hold the ref on the incoming FLB until pci_liveupdate_finish() so
+   * that dev->liveupdate.incoming does not get freed while it is in use.
+   */

How would that work? If finish is not called FLB stays around until the next reboot.

Quoted 3 lines

True... I think if the PCI core trusts drivers to call pci_liveupdate_finish() then we don't need to hold onto the incoming reference here.

Quoted 5 lines

That was my point when I was arguing against refcounts on outgoing FLBs. This is very easy to abuse, especially when we are talking about device drivers. And this refcounting mechanism makes the FLB no longer file-lifecycle-bound, since now it is entirely up to drivers to decide the lifecycle of this data.

The PCI core holds a reference to the incoming FLB for as long as it maintains a pointer to that FLB in struct pci_dev (dev->liveupdate.incoming). The lifetime of that pointer is aligned with the lifetime of the file as long as the driver calls pci_liveupdate_finish() in its file finish() callback.

If there is a bug in the driver that causes it to not call pci_liveupdate_finish() then the FLB will leak past the file yes. But the alternative would be to leak a pointer to freed memory in dev->liveupdate.incoming, which could lead to UAF.

Leaking the FLB seems safer than UAF, which is why I went for the refcounting approach.

Another approach entirely would be to drop the dev->liveupdate.incoming and do the xarray lookup everytime instead.

Quoted 11 lines

I have been thinking about this a bit more in the last couple days, and I wonder if we are doing this right. Here's an idea I have been thinking of.

We should make live update a first class citizen in PCI. Instead of patching in liveupdate via the liveupdate.incoming field, and letting drivers figure out when to use it, we should separate out probe and retrieve paths entirely.

Quoted 5 lines

Probe and retrieve are fundamentally different operations. While they may share some common initialization logic for the _software_ state, how they interface with the hardware is completely different. I think mixing the two will result in driver code being more spaghetti by having liveupdate checks sprayed out all over.

We are only planning on supporting Live Update for VFIO drivers for the forseeable future. The VFIO work during probe is almost entirely software state setup. The only hardware logic we need to "if" out in the vfio-pci driver's probe() is putting the device into a low-power mode via the runtime power manager. So I don't think we will get any benefit from this approach, and it would be a lot more intrusive to both the PCI core driver framework, and VFIO itself, to support this.

Quoted 4 lines

This series doesn't add support for any drivers, but looking at some of the code we have downstream, I see this problem. The liveupdate code is all over the place in the driver and it is very hard to wrap one's head around how the device is actually retrieved.

You can find the vfio-pci driver changes here:

https://lore.kernel.org/kvm/[email protected]/

Let's keep the discussion focused on upstream VFIO drivers since that is all we are planning to support right now due to LUO's requirement of file-based preservation. The downstream driver changes we are carrying is not reflective of what we want to support upstream.

Quoted 10 lines

So I think PCI core should track preserved devices, and if the device is preserved, it should skip the probe and wait for retrieve. Retrieve does the full initialization of the device. This fits in with the LUO model as well. You can make retrieve a callback of struct pci_driver and do some wrappers to talk with LUO, so device drivers don't directly interface with LUO at all.

We should do similar things on the shutdown path. Shutdown is a fundamentally different operation from freeze, and so we should separate them out as well.

This is speculative. In practice, we haven't needed to change VFIO's shutdown() or probe() functions so far. The only change I anticipate needing is skipping runtime power management "put" during probe() I mentioned above.

If we actually made retrieve() a first-class callback and used that instead of probe(), VFIO would internally just call its probe() function because that would be the cleanest way to set up all the software state it needs to manage the device.

Quoted 7 lines

This solves the lifetime problem as well. When PCI core is initializing, it knows for sure that no retrievals are going to happen. That's because none of the drivers have registered yet. So it can safely access the FLB and initialize its state. After that, drivers can register themselves and start accepting retrieve() calls. Once the last driver goes away, the FLB is freed automatically.

It's not so simple. The PCI core does not really initialize itself. Scanning devices gets triggered externally, e.g. by ACPI, device trees, runtime hotplug events, etc., and that is when the PCI core gets notified about a device. None of this is synchronized with "when drivers have registered", which I assume you are referring to registering with LUO.

Quoted 4 lines

I am sorry for suggesting a big refactor at v6, but the early versions looked good to me at the time, and I only thought more deeply about this when trying to figure out how we can make the lifetimes cleaner.

During PCI enumeration, the previous kernel might have passed state about devices that were preserved across kexec. The PCI core needs to fetch this state to identify which devices are "incoming" and require special handling.

Add pci_liveupdate_setup_device() which is called during device setup to fetch the serialized state (struct pci_ser) from the Live Update Orchestrator. The first time this happens, pci_flb_retrieve() will run and convert the array of pci_dev_ser structs into an xarray so that it can be looked up efficiently.

If a device is found in the xarray, the PCI core stores a pointer to its state in dev->liveupdate_incoming and holds a reference to the incoming FLB until pci_liveupdate_finish() is called by the driver.

This ensures proper lifecycle management for incoming preserved devices and allows the PCI core and drivers to apply specific Live Update logic to them in subsequent commits.

Drivers can check if a device is an incoming preserved device (e.g. during probe) by calling pci_liveupdate_is_incoming().

CONFIG_64BIT is now required to enable CONFIG_PCI_LIVEUPDATE so that the domain and bdf can be guaranteed to fit in an unsigned long and be used as the xarray key.

Signed-off-by David Matlack <[email protected]>

---

MAINTAINERS                    |   1 +
drivers/pci/Kconfig            |   2 +-
drivers/pci/liveupdate.c       | 230 ++++++++++++++++++++++++++++++++-
drivers/pci/liveupdate.h       |   5 +
drivers/pci/probe.c            |   3 +
include/linux/pci_liveupdate.h |  13 ++
6 files changed, 251 insertions(+), 3 deletions(-)

static int pci_flb_retrieve(struct liveupdate_flb_op_args *args) {

drivers/pci/liveupdate.c +26 -1

-	args->obj = phys_to_virt(args->data);
+	struct pci_ser *ser = phys_to_virt(args->data);
+	struct pci_flb_incoming *incoming;
+	int ret = -ENOMEM;
+	u32 i;
+
+	incoming = kmalloc_obj(*incoming);
+	if (!incoming)
+		goto err_restore_free;
+
+	incoming->ser = ser;
+	xa_init(&incoming->xa);
+
+	for (i = 0; i < incoming->ser->max_nr_devices; i++) {
+		struct pci_dev_ser *dev_ser = &incoming->ser->devices[i];
+		unsigned long key;
+
+		if (!dev_ser->refcount)
+			continue;
+
+		key = pci_ser_xa_key(dev_ser->domain, dev_ser->bdf);
+		ret = xa_insert(&incoming->xa, key, dev_ser, GFP_KERNEL);
+		if (ret)
+			goto err_xa_destroy;
+	}
+
+	args->obj = incoming;

	return 0;

}

static void pci_flb_finish(struct liveupdate_flb_op_args *args) {

drivers/pci/liveupdate.c +5 -1

-	kho_restore_free(args->obj);
+	struct pci_flb_incoming *incoming = args->obj;
+
+	xa_destroy(&incoming->xa);
+	kho_restore_free(incoming->ser);
+	kfree(incoming);

}

static struct liveupdate_flb_ops pci_liveupdate_flb_ops = {

drivers/pci/liveupdate.c +24 -0

@@ -270,6 +335,91 @@ void pci_liveupdate_unpreserve(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);
 
+static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
+{
+	struct pci_flb_incoming *incoming = NULL;
+	int ret;
+
+	ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&incoming);
+
+	/* Live Update is not enabled. */
+	if (ret == -EOPNOTSUPP)
+		return NULL;
+
+	/* Live Update is enabled, but there is no incoming FLB data. */
+	if (ret == -ENODATA)
+		return NULL;
+
+	/*
+	 * Live Update is enabled and there is incoming FLB data, but none of it
+	 * matches pci_liveupdate_flb.compatible.
+	 *
+	 * This could mean that no PCI FLB data was passed by the previous
+	 * kernel, but it could also mean the previous kernel used a different
+	 * compatibility string (i.e. a different ABI).
+	 */
+	if (ret == -ENOENT) {

drivers/pci/liveupdate.c +6 -0

+		return NULL;
+	}
+
+	return incoming;
+}
+

include/linux/pci_liveupdate.h +5 -0

+
+static inline bool pci_liveupdate_is_incoming(struct pci_dev *dev)
+{
+	return false;
+}

#endif

#endif /* LINUX_PCI_LIVEUPDATE_H */

Signature · 1 line

2.54.0.746.g67dd491aae-goog

Quoted 36 lines · David Matlack

During PCI enumeration, the previous kernel might have passed state about devices that were preserved across kexec. The PCI core needs to fetch this state to identify which devices are "incoming" and require special handling.

Add pci_liveupdate_setup_device() which is called during device setup to fetch the serialized state (struct pci_ser) from the Live Update Orchestrator. The first time this happens, pci_flb_retrieve() will run and convert the array of pci_dev_ser structs into an xarray so that it can be looked up efficiently.

If a device is found in the xarray, the PCI core stores a pointer to its state in dev->liveupdate_incoming and holds a reference to the incoming FLB until pci_liveupdate_finish() is called by the driver.

This ensures proper lifecycle management for incoming preserved devices and allows the PCI core and drivers to apply specific Live Update logic to them in subsequent commits.

Drivers can check if a device is an incoming preserved device (e.g. during probe) by calling pci_liveupdate_is_incoming().

CONFIG_64BIT is now required to enable CONFIG_PCI_LIVEUPDATE so that the domain and bdf can be guaranteed to fit in an unsigned long and be used as the xarray key.

Signed-off-by David Matlack <[email protected]>

---

MAINTAINERS                    |   1 +
drivers/pci/Kconfig            |   2 +-
drivers/pci/liveupdate.c       | 230 ++++++++++++++++++++++++++++++++-
drivers/pci/liveupdate.h       |   5 +
drivers/pci/probe.c            |   3 +
include/linux/pci_liveupdate.h |  13 ++
6 files changed, 251 insertions(+), 3 deletions(-)

[snip]

Quoted 28 lines

static int pci_flb_retrieve(struct liveupdate_flb_op_args *args) {

drivers/pci/liveupdate.c +24 -1

-      args->obj = phys_to_virt(args->data);
+      struct pci_ser *ser = phys_to_virt(args->data);
+      struct pci_flb_incoming *incoming;
+      int ret = -ENOMEM;
+      u32 i;
+
+      incoming = kmalloc_obj(*incoming);
+      if (!incoming)
+              goto err_restore_free;
+
+      incoming->ser = ser;
+      xa_init(&incoming->xa);
+
+      for (i = 0; i < incoming->ser->max_nr_devices; i++) {
+              struct pci_dev_ser *dev_ser = &incoming->ser->devices[i];
+              unsigned long key;
+
+              if (!dev_ser->refcount)
+                      continue;
+
+              key = pci_ser_xa_key(dev_ser->domain, dev_ser->bdf);
+              ret = xa_insert(&incoming->xa, key, dev_ser, GFP_KERNEL);
+              if (ret)
+                      goto err_xa_destroy;
+      }

drivers/pci/liveupdate.c static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args) · line 193

Quoted 10 lines

drivers/pci/liveupdate.c +9 -0

+
+      args->obj = incoming;
      return 0;
+
+err_xa_destroy:
+      xa_destroy(&incoming->xa);
+      kfree(incoming);
+err_restore_free:
+      kho_restore_free(ser);
+      return ret;

Hmm.. This is interesting, so the KHO state is freed and it cannot be reused. I see you already pointed out that we are putting an LUO policy to say that the retry is not allowed.

But what should be the behaviour of liveupdate in this regard? Let the system boot in a normal way? This might break other subsystems as they might depend on PCIe restoring state properly. Also I think some of the PCIe state, like device-id, BAR addresses, ACLs etc, might be used as source of truth by other components.

For example, lets say FLB retrieve() of PCIe fails, but succeeds for VFIO/IOMMU, now VFIO/IOMMU are restoring state of a device that is not restored/preserved?

Should this be considered fatal?

Quoted 48 lines

}

static void pci_flb_finish(struct liveupdate_flb_op_args *args) {

drivers/pci/liveupdate.c +5 -1

-      kho_restore_free(args->obj);
+      struct pci_flb_incoming *incoming = args->obj;
+
+      xa_destroy(&incoming->xa);
+      kho_restore_free(incoming->ser);
+      kfree(incoming);

}

static struct liveupdate_flb_ops pci_liveupdate_flb_ops = {

drivers/pci/liveupdate.c +31 -0

@@ -270,6 +335,91 @@ void pci_liveupdate_unpreserve(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);
 
+static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
+{
+      struct pci_flb_incoming *incoming = NULL;
+      int ret;
+
+      ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&incoming);
+
+      /* Live Update is not enabled. */
+      if (ret == -EOPNOTSUPP)
+              return NULL;
+
+      /* Live Update is enabled, but there is no incoming FLB data. */
+      if (ret == -ENODATA)
+              return NULL;
+
+      /*
+       * Live Update is enabled and there is incoming FLB data, but none of it
+       * matches pci_liveupdate_flb.compatible.
+       *
+       * This could mean that no PCI FLB data was passed by the previous
+       * kernel, but it could also mean the previous kernel used a different
+       * compatibility string (i.e. a different ABI).
+       */
+      if (ret == -ENOENT) {
+              pr_info_once("No incoming FLB matched %s\n", pci_liveupdate_flb.compatible);
+              return NULL;
+      }
+
+      /*
+       * There is incoming FLB data that matches pci_liveupdate_flb.compatible
+       * but it cannot be retrieved.

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 371

Quoted 3 lines

drivers/pci/liveupdate.c +3 -0

+       */
+      if (ret) {
+              WARN_ONCE(ret, "Failed to retrieve incoming FLB data\n");

I think this should probably be considered fatal as mentioned above or the caller of this function should get an error so it can fail. I think retrievel of preserved state should generally not fail unless there is memory corruption or ABI is incompatible.

Quoted 6 lines

drivers/pci/liveupdate.c +6 -0

+              return NULL;
+      }
+
+      return incoming;
+}
+

[snip]

Quoted 11 lines

include/linux/pci_liveupdate.h +5 -0

+
+static inline bool pci_liveupdate_is_incoming(struct pci_dev *dev)
+{
+      return false;
+}

#endif

#endif /* LINUX_PCI_LIVEUPDATE_H */
--
2.54.0.746.g67dd491aae-goog

Sami

Quoted 1 line · Samiullah Khawaja

Quoted 11 lines

Hmm.. This is interesting, so the KHO state is freed and it cannot be reused. I see you already pointed out that we are putting an LUO policy to say that the retry is not allowed.

But what should be the behaviour of liveupdate in this regard? Let the system boot in a normal way? This might break other subsystems as they might depend on PCIe restoring state properly. Also I think some of the PCIe state, like device-id, BAR addresses, ACLs etc, might be used as source of truth by other components.

Quoted 5 lines

For example, lets say FLB retrieve() of PCIe fails, but succeeds for VFIO/IOMMU, now VFIO/IOMMU are restoring state of a device that is not restored/preserved?

Should this be considered fatal?

If PCI FLB retrieve fails then there are certain things that cannot be guaranteed, such as BDF (B specifically) remaining constant. This could lead to memory corruption as the IOMMU may have live translations in place for those specific RequesterIDs. And, in the future, preserved devices may be doing P2P which depends on BARs not moving. If the PCI core cannot retrieve the FLB saved by the previous kernel, it cannot make these guarantees.

So yeah I think you're right that PCI core should treat FLB retrieve as fatal and just panic.

Quoted 49 lines

Quoted 49 lines

}

static void pci_flb_finish(struct liveupdate_flb_op_args *args) {

drivers/pci/liveupdate.c +5 -1

-      kho_restore_free(args->obj);
+      struct pci_flb_incoming *incoming = args->obj;
+
+      xa_destroy(&incoming->xa);
+      kho_restore_free(incoming->ser);
+      kfree(incoming);

}

static struct liveupdate_flb_ops pci_liveupdate_flb_ops = {

drivers/pci/liveupdate.c +31 -0

@@ -270,6 +335,91 @@ void pci_liveupdate_unpreserve(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);
 
+static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
+{
+      struct pci_flb_incoming *incoming = NULL;
+      int ret;
+
+      ret = liveupdate_flb_get_incoming(&pci_liveupdate_flb, (void **)&incoming);
+
+      /* Live Update is not enabled. */
+      if (ret == -EOPNOTSUPP)
+              return NULL;
+
+      /* Live Update is enabled, but there is no incoming FLB data. */
+      if (ret == -ENODATA)
+              return NULL;
+
+      /*
+       * Live Update is enabled and there is incoming FLB data, but none of it
+       * matches pci_liveupdate_flb.compatible.
+       *
+       * This could mean that no PCI FLB data was passed by the previous
+       * kernel, but it could also mean the previous kernel used a different
+       * compatibility string (i.e. a different ABI).
+       */
+      if (ret == -ENOENT) {
+              pr_info_once("No incoming FLB matched %s\n", pci_liveupdate_flb.compatible);
+              return NULL;
+      }
+
+      /*
+       * There is incoming FLB data that matches pci_liveupdate_flb.compatible
+       * but it cannot be retrieved.

Quoted 8 lines

drivers/pci/liveupdate.c void pci_liveupdate_unpreserve(struct pci_dev *dev) · line 371

Quoted 3 lines

drivers/pci/liveupdate.c +3 -0

+       */
+      if (ret) {
+              WARN_ONCE(ret, "Failed to retrieve incoming FLB data\n");

I think this should probably be considered fatal as mentioned above or the caller of this function should get an error so it can fail. I think retrievel of preserved state should generally not fail unless there is memory corruption or ABI is incompatible.

Yeah. I think I will just call panic() here to cover all cases.

Quoted 22 lines

Quoted 7 lines

drivers/pci/liveupdate.c +6 -0

+              return NULL;
+      }
+
+      return incoming;
+}
+

[snip]

Quoted 11 lines

include/linux/pci_liveupdate.h +5 -0

+
+static inline bool pci_liveupdate_is_incoming(struct pci_dev *dev)
+{
+      return false;
+}

#endif

#endif /* LINUX_PCI_LIVEUPDATE_H */
--
2.54.0.746.g67dd491aae-goog

Sami

Document how driver binding works during a Live Update and what the PCI core expects of drivers and users. Note that this is only a description of the current division of responsibilities. These can change in the future if we decide.

Signed-off-by David Matlack <[email protected]>

---
 drivers/pci/liveupdate.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

drivers/pci/liveupdate.c +16 -0

index 96c43b84532c..4f2ec6ffdd16 100644
@@ -70,6 +70,22 @@
  * preserved. These may be relaxed in the future:
  *
  *  * The device cannot be a Virtual Function (VF).
+ *
+ * Driver Binding
+ * ==============
+ *
+ * In the outgoing kernel, it is the driver's responsibility to ensure that it
+ * does not release a device between pci_liveupdate_preserve() and
+ * pci_liveupdate_unpreserve().
+ *
+ * In the incoming kernel, it is the driver's responsibility to ensure that it
+ * does not release a preserved device between probe() and
+ * pci_liveupdate_finish().
+ *
+ * It is the user's responsibility to ensure that incoming preserved devices are
+ * bound to the correct driver. i.e. The PCI core does not protect against a
+ * device getting preserved by driver A in the outgoing kernel and then getting
+ * bound to driver B in the incoming kernel.
  */
 
 #define pr_fmt(fmt) "PCI: liveupdate: " fmt

Document how driver binding works during a Live Update and what the PCI core expects of drivers and users. Note that this is only a description of the current division of responsibilities. These can change in the future if we decide.

Signed-off-by David Matlack <[email protected]>

Document how driver binding works during a Live Update and what the PCI core expects of drivers and users. Note that this is only a description of the current division of responsibilities. These can change in the future if we decide.

Document how driver binding works during a Live Update and what the PCI core expects of drivers and users. Note that this is only a description of the current division of responsibilities. These can change in the future if we decide.

Signed-off-by David Matlack <[email protected]>

---
drivers/pci/liveupdate.c | 16 ++++++++++++++++

1 file changed, 16 insertions(+)

drivers/pci/liveupdate.c +16 -0

index 96c43b84532c..4f2ec6ffdd16 100644
@@ -70,6 +70,22 @@
 * preserved. These may be relaxed in the future:
 *
 *  * The device cannot be a Virtual Function (VF).
+ *
+ * Driver Binding
+ * ==============
+ *
+ * In the outgoing kernel, it is the driver's responsibility to ensure that it
+ * does not release a device between pci_liveupdate_preserve() and
+ * pci_liveupdate_unpreserve().
+ *
+ * In the incoming kernel, it is the driver's responsibility to ensure that it
+ * does not release a preserved device between probe() and
+ * pci_liveupdate_finish().
+ *
+ * It is the user's responsibility to ensure that incoming preserved devices are
+ * bound to the correct driver. i.e. The PCI core does not protect against a
+ * device getting preserved by driver A in the outgoing kernel and then getting
+ * bound to driver B in the incoming kernel.
 */
 
 #define pr_fmt(fmt) "PCI: liveupdate: " fmt

Signature · 1 line

2.54.0.746.g67dd491aae-goog

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Mixing goto-based control flow with scope-based cleanup helpers (scoped_guard) in the function pci_liveupdate_scan_bridge_begin. --

commit 12ae1f010eb68b0654e38ce2800545f03bf1b835
Author: David Matlack <[email protected]>

PCI: liveupdate: Keep bus numbers constant during Live Update

During a Live Update, preserved devices must be allowed to continue performing memory transactions so the kernel cannot change the fabric topology. To keep bus numbers constant, this patch makes the PCI core inherit the secondary and subordinate bus numbers assigned to bridges during scanning if any PCI devices are being preserved.

Quoted 2 lines

drivers/pci/liveupdate.c +0 -0

[ ... ]

Quoted 30 lines

drivers/pci/liveupdate.c +30 -0

+bool pci_liveupdate_scan_bridge_begin(struct pci_bus *bus, struct pci_dev *dev,
+                                   int pass)
+{
+     struct pci_dev *parent = bus->self;
+
+     /*
+      * On the second pass, reuse the value that was set on the first pass
+      * so that the passes are consistent with one another.
+      */
+     if (pass)
+             return dev->liveupdate.inherit_buses;
+
+     /*
+      * If the parent bridge is being forced to inherit its bus numbers
+      * during this scan then this bridge must as well, otherwise the PCI
+      * core could expand this bridge's reservation beyond its parent (which
+      * cannot expand).
+      */
+     if (parent && parent->liveupdate.inherit_buses) {
+             dev->liveupdate.inherit_buses = true;
+             goto out;
+     }
+
+     /*
+      * Otherwise, if there are any incoming preserved devices, force the
+      * bus numbers to be inherited to avoid changing the bus numbers
+      * assigned to those devices during enumeration.
+      *
+      * To keep things simple, inherit bus numbers on all bridges if any PCI
+      * devices are incoming, to ensure that no bridge's reservation is

drivers/pci/liveupdate.c static void pci_liveupdate_flb_put_incoming(void) · line 453

Quoted 10 lines

drivers/pci/liveupdate.c +10 -0

+      * expanded to overlap with a preserved device downstream of a different
+      * bridge.
+      */
+     scoped_guard(rwsem_read, &pci_liveupdate.rwsem) {
+             struct pci_flb_incoming *incoming;
+
+             incoming = pci_liveupdate_flb_get_incoming();
+             if (!incoming) {
+                     dev->liveupdate.inherit_buses = false;
+                     goto out;

[Severity: Low]
Does this mix goto-based control flow with scope-based cleanup helpers?

According to the cleanup subsystem guidelines, functions should either use scope-based cleanups or goto-based cleanups, but never both in the same function.

Could these goto statements be replaced by directly returning the boolean value instead?

During a Live Update, preserved devices must be allowed to continue performing memory transactions so the kernel cannot change the fabric topology, including bus numbers, since that would require disabling and flushing any memory transactions first.

To keep bus numbers constant, always inherit the secondary and subordinate bus numbers assigned to bridges during scanning, instead of assigning new ones, if any PCI devices are being preserved. Note that the kernel inherits bus numbers even on bridges without any downstream endpoints that were preserved. This avoids accidentally assigning a bridge a new window that overlaps with a preserved device that is downstream of a different bridge.

If a bridge is scanned with a broken topology or has no bus numbers set during a Live Update, refuse to assign it new bus numbers and refuse to enumerate devices below it until the Live Update is finished. This is a safety measure to prevent topology conflicts.

Require that CONFIG_CARDBUS is not enabled to enable CONFIG_PCI_LIVEUPDATE since inheriting bus numbers on PCI-to-CardBus bridges requires additional work but is not a priority at the moment.

Signed-off-by David Matlack <[email protected]>

---
 .../admin-guide/kernel-parameters.txt         |  6 +-
 drivers/pci/Kconfig                           |  2 +-
 drivers/pci/liveupdate.c                      | 83 ++++++++++++++++++-
 drivers/pci/liveupdate.h                      | 14 ++++
 drivers/pci/probe.c                           | 17 +++-
 include/linux/pci_liveupdate.h                |  4 +
 6 files changed, 119 insertions(+), 7 deletions(-)

drivers/pci/liveupdate.c +7 -0

+		pci_liveupdate_flb_put_incoming();
+	}
+
+out:
+	return dev->liveupdate.inherit_buses;
+}
+

drivers/pci/probe.c +1 -2

 		/*

@@ -1497,8 +1501,7 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 		 * do in the second pass.
 		 */
 		if (!pass) {
-			if (pcibios_assign_all_busses() || broken)
-
+			if (assign_new_buses || broken)
 				/*
 				 * Temporarily disable forwarding of the
 				 * configuration cycles on all bridges in