On Tue, Mar 01, 2022 at 10:12:00AM -0500, Igor Mammedov wrote:
> Q35 switched to ACPI PCI hotplug by default in since 6.1
> machine type and migration worked as expected (with BARs
> on target being the same as on source)
>
> However native PCIe fixes [1] merged in 6.2 time, regressed
> migration part, resulting in disabled BARs on target. The
> issue affects pc-q35-6.2 and pc-q35-6.1 machine types (and
> older if qemu-6.2 binary is used on source with manually
> enabled ACPI PCI hotplug).
>
> Introduce x-pcihp-disable-pcie-slot-power-on-fixup compat
> property to keep 6.2 and older machine types in broken state
> when ACPI PCI hotplug is enabled to make sure that guest does
> see the same PCIe device and slot on src & dst.
>
> 1)
> Fixes: d5daff7d312 (pcie: implement slot power control for pcie root ports)
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
I am not sure why we need this one. What's the scenario that's broken
otherwise?
> ---
> include/hw/acpi/pcihp.h | 1 +
> hw/acpi/ich9.c | 20 ++++++++++++++++++++
> hw/acpi/pcihp.c | 11 +++++++----
> hw/core/machine.c | 4 +++-
> 4 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index af1a169fc3..2436151678 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -52,6 +52,7 @@ typedef struct AcpiPciHpState {
> bool legacy_piix;
> uint16_t io_base;
> uint16_t io_len;
> + bool disable_pcie_slot_power_on_fixup;
> } AcpiPciHpState;
>
> void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index bd9bbade70..e3bffdef71 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -430,6 +430,23 @@ static void ich9_pm_set_keep_pci_slot_hpc(Object *obj,
> bool value, Error **errp)
> s->pm.keep_pci_slot_hpc = value;
> }
>
> +static bool ich9_pm_get_disable_pcie_slot_power_on_fixup(Object *obj,
> + Error **errp)
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + return s->pm.acpi_pci_hotplug.disable_pcie_slot_power_on_fixup;
> +}
> +
> +static void ich9_pm_set_disable_pcie_slot_power_on_fixup(Object *obj,
> + bool value,
> + Error **errp)
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + s->pm.acpi_pci_hotplug.disable_pcie_slot_power_on_fixup = value;
> +}
> +
> void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> {
> static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
> @@ -469,6 +486,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs
> *pm)
> object_property_add_bool(obj, "x-keep-pci-slot-hpc",
> ich9_pm_get_keep_pci_slot_hpc,
> ich9_pm_set_keep_pci_slot_hpc);
> + object_property_add_bool(obj, "x-pcihp-disable-pcie-slot-power-on-fixup",
> + ich9_pm_get_disable_pcie_slot_power_on_fixup,
> + ich9_pm_set_disable_pcie_slot_power_on_fixup);
> }
>
> void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState
> *dev,
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 6351bd3424..4c06caf4a9 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -369,10 +369,13 @@ void acpi_pcihp_device_plug_cb(HotplugHandler
> *hotplug_dev, AcpiPciHpState *s,
> }
>
> bus = pci_get_bus(pdev);
> - bridge = pci_bridge_get_device(bus);
> - if (object_dynamic_cast(OBJECT(bridge), TYPE_PCIE_ROOT_PORT) ||
> - object_dynamic_cast(OBJECT(bridge), TYPE_XIO3130_DOWNSTREAM)) {
> - pcie_cap_slot_enable_power(bridge);
> + /* compat knob to preserve pci_config as in 6.2 & older when pcihp in
> use */
> + if (s->disable_pcie_slot_power_on_fixup == false) {
> + bridge = pci_bridge_get_device(bus);
> + if (object_dynamic_cast(OBJECT(bridge), TYPE_PCIE_ROOT_PORT) ||
> + object_dynamic_cast(OBJECT(bridge), TYPE_XIO3130_DOWNSTREAM)) {
> + pcie_cap_slot_enable_power(bridge);
> + }
> }
>
> bsel = acpi_pcihp_get_bsel(bus);
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index d856485cb4..1758b49c2f 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -37,7 +37,9 @@
> #include "hw/virtio/virtio.h"
> #include "hw/virtio/virtio-pci.h"
>
> -GlobalProperty hw_compat_6_2[] = {};
> +GlobalProperty hw_compat_6_2[] = {
> + { "ICH9-LPC", "x-pcihp-disable-pcie-slot-power-on-fixup", "on" },
> +};
> const size_t hw_compat_6_2_len = G_N_ELEMENTS(hw_compat_6_2);
>
> GlobalProperty hw_compat_6_1[] = {
> --
> 2.31.1
