[PATCH 1/1] pcie: Do not set power state for some hot-plugged devices

* [PATCH 1/1] pcie: Do not set power state for some hot-plugged devices
@ 2021-12-14 21:53 Annie Li
  2021-12-15  6:05 ` Gerd Hoffmann
  0 siblings, 1 reply; 6+ messages in thread
From: Annie Li @ 2021-12-14 21:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: annie.li, kraxel

After the PCIe device is hot-plugged, the device's power state is
initialized as ON. However, the device isn't powered on yet, i.e.
the PCI_EXP_SYSCTL_PCC bit isn't set to PCI_EXP_SLTCTL_PWR_ON.
Later on, its power state will set back to OFF due to the non
PCI_EXP_SLTCTL_PWR_ON state. The device is invisible until
PCI_EXP_SLTCTL_PWR_ON is set.

This may be appropriate for general PCIe hot-plug cases. However,
if the device is hot-plugged when the VM is in VM_STATE_PRELAUNCH
state, especially the system disk device, the firmware will fail
to find the system disk. As a result, the guest fails to boot.

An extra flag(set_power) is added in this patch to indicate if
pci_set_power is needed. After the device is powered
on(PCI_EXP_SLTCTL_PWR_ON), its power state will be set as normal
devices.

Fixes: 090b32b8dae6 ("implement slot power control for pcie root ports")

Signed-off-by: Annie Li <annie.li@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 hw/pci/pci.c         |  1 +
 hw/pci/pcie.c        | 29 +++++++++++++++++++++++++++--
 include/hw/pci/pci.h |  1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e5993c1ef5..b61c547291 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2186,6 +2186,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         return;
     }
 
+    pci_dev->set_power = true;
     pci_set_power(pci_dev, true);
 }
 
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index d7d73a31e4..e4ff23f3b9 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -28,6 +28,7 @@
 #include "hw/pci/pcie_regs.h"
 #include "hw/pci/pcie_port.h"
 #include "qemu/range.h"
+#include "sysemu/runstate.h"
 
 //#define DEBUG_PCIE
 #ifdef DEBUG_PCIE
@@ -385,8 +386,20 @@ static void pcie_cap_update_power(PCIDevice *hotplug_dev)
         power = (sltctl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_ON;
     }
 
-    pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
-                        pcie_set_power_device, &power);
+    /*
+     * For devices hot-plugged in RUN_STATE_PRELAUNCH state, set_power is
+     * set to false to avoid unnecessary power state changes before the device
+     * is powered on. After the device is powered on, set_power has to be
+     * set back to true to allow general power state changes.
+     */
+    if (!hotplug_dev->set_power && power) {
+        hotplug_dev->set_power = true;
+    }
+
+    if (hotplug_dev->set_power) {
+        pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
+                            pcie_set_power_device, &power);
+    }
 }
 
 /*
@@ -475,6 +488,18 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
         }
         pcie_cap_slot_event(hotplug_pdev,
                             PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+
+        /*
+         * After the system disk device is hot-plugged during
+         * RUN_STATE_PRELAUNCH state, its power state will be set to OFF
+         * before the device is actually powered on. The device is invisible
+         * during this period. Hence the firmware won't find the system
+         * disk to boot. The set_power is set to false to avoid setting the
+         * power state to OFF.
+         */
+        if (runstate_check(RUN_STATE_PRELAUNCH)) {
+            hotplug_pdev->set_power = false;
+        }
         pcie_cap_update_power(hotplug_pdev);
     }
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e7cdf2d5ec..753df3523e 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -269,6 +269,7 @@ struct PCIDevice {
     DeviceState qdev;
     bool partially_hotplugged;
     bool has_power;
+    bool set_power;
 
     /* PCI config space */
     uint8_t *config;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread