On Thu, Oct 22, 2020 at 02:40:26PM +0300, Marcel Apfelbaum wrote:
> From: Marcel Apfelbaum <marcel@redhat.com>
>
> During PCIe Root Port's transition from Power-Off to Power-ON (or vice-versa)
> the "Slot Control Register" has the "Power Indicator Control"
> set to "Blinking" expressing a "power transition" mode.
>
> Any hotplug operation during the "power transition" mode is not permitted
> or at least not expected by the Guest OS leading to strange failures.
>
> Detect and refuse hotplug operations in such case.
>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> ---
> hw/pci/pcie.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 5b48bae0f6..2fe5c1473f 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -410,6 +410,7 @@ void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> PCIDevice *hotplug_pdev = PCI_DEVICE(hotplug_dev);
> uint8_t *exp_cap = hotplug_pdev->config + hotplug_pdev->exp.exp_cap;
> uint32_t sltcap = pci_get_word(exp_cap + PCI_EXP_SLTCAP);
> + uint32_t sltctl = pci_get_word(exp_cap + PCI_EXP_SLTCTL);
>
> /* Check if hot-plug is disabled on the slot */
> if (dev->hotplugged && (sltcap & PCI_EXP_SLTCAP_HPC) == 0) {
> @@ -418,6 +419,12 @@ void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
> }
>
> + if ((sltctl & PCI_EXP_SLTCTL_PIC) == PCI_EXP_SLTCTL_PWR_IND_BLINK) {
> + error_setg(errp, "Hot-plug failed: %s is in Power Transition",
> + DEVICE(hotplug_pdev)->id);
> + return;
> + }
> +
> pcie_cap_slot_plug_common(PCI_DEVICE(hotplug_dev), dev, errp);
> }
I wonder if hw/pci/shpc.c is free from this issue?
I did not encounter the issue with shpc.
However, the SHPC spec describes the Power Indicator behavior for hotplug only leaving the booting sequence out of it:
"Although the Standard Usage Model presented in Section 2.2.1.2 requires a specific
relationship between the Power Indicator and the state of the slot, this relationship is
enforced by software, not by the SHPC."
It looks like it depends on the software implementation.
Thanks,
Marcel
Roman.