On Tue, Sep 16, 2025 at 12:45:17AM -0700, Chris Li wrote:
> If the liveupdate flag has LU_BUSMASTER or LU_BUSMASTER_BRIDGE, the
> device is participating in the liveupdate preserving bus master bit in the
> PCI config space command register.
>
> Avoid writing to the PCI command register for the bus master bit during
> boot up.
>
> Signed-off-by: Chris Li <chrisl@kernel.org>
> ---
> drivers/pci/liveupdate.c | 6 ++++++
> drivers/pci/pci.c | 7 +++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> index 1b12fc0649f479c6f45ffb26e6e3754f41054ea8..a09a166b6ee271b96bce763716c3b62b24f3edbb 100644
> --- a/drivers/pci/liveupdate.c
> +++ b/drivers/pci/liveupdate.c
> @@ -377,6 +377,12 @@ static void pci_dev_do_restore(struct pci_dev *dev, struct pci_dev_ser *s)
> pci_info(dev, "liveupdate restore flags %x driver: %s data: [%llx]\n",
> s->flags, s->driver_name, s->driver_data);
> list_move_tail(&dev->dev.lu.lu_next, &probe_devices);
> + if (s->flags & (LU_BUSMASTER | LU_BUSMASTER_BRIDGE)) {
> + u16 pci_command;
> +
> + pci_read_config_word(dev, PCI_COMMAND, &pci_command);
> + WARN_ON(!(pci_command & PCI_COMMAND_MASTER));
> + }
> }
>
> void pci_liveupdate_restore(struct pci_dev *dev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 9e42090fb108920995ebe34bd2535a0e23fef7fd..2339ac1bd57616a78d2105ba3a4fc72bbf49973e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2248,7 +2248,8 @@ static void do_pci_disable_device(struct pci_dev *dev)
> pci_read_config_word(dev, PCI_COMMAND, &pci_command);
> if (pci_command & PCI_COMMAND_MASTER) {
> pci_command &= ~PCI_COMMAND_MASTER;
> - pci_write_config_word(dev, PCI_COMMAND, pci_command);
> + if (!(dev->dev.lu.flags & (LU_BUSMASTER | LU_BUSMASTER_BRIDGE)))
> + pci_write_config_word(dev, PCI_COMMAND, pci_command);
I think changing the semantics of interfaces like this is a problem
because callers rely on the existing semantics, and it's hard to
reason about how this change would affect them. How would you update
the kernel-doc to reflect this change?
do_pci_disable_device() is used in the PM suspend, freeze, and
poweroff paths. I suppose those paths are allowed even when devices
have been marked with LU_BUSMASTER/LU_BUSMASTER_BRIDGE? And I assume
you probably would want the existing semantics there?
I.e., if a device has been marked with LU_BUSMASTER, you want to keep
its bus mastering enabled across a liveupdate kexec. But if we
suspend before doing the kexec, I assume we would still want to clear
bus mastering on suspend and restore bus mastering on resume?
The other path that uses do_pci_disable_device() is
pci_disable_device(), which is primarily used in driver .remove()
methods. You have to modify drivers to support liveupdate anyway, so
if we call driver .remove() methods during a liveupdate kexec, I think
you should change the .remove() method so it only calls
pci_disable_device() when you want bus mastering disabled.
> }
>
> pcibios_disable_device(dev);
> @@ -4276,7 +4277,9 @@ static void __pci_set_master(struct pci_dev *dev, bool enable)
> if (cmd != old_cmd) {
> pci_dbg(dev, "%s bus mastering\n",
> enable ? "enabling" : "disabling");
> - pci_write_config_word(dev, PCI_COMMAND, cmd);
> +
> + if (!(dev->dev.lu.flags & (LU_BUSMASTER | LU_BUSMASTER_BRIDGE)))
> + pci_write_config_word(dev, PCI_COMMAND, cmd);
> }
> dev->is_busmaster = enable;
> }
>
> --
> 2.51.0.384.g4c02a37b29-goog
>