[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 21/56] pci: acpihp: assign BSEL only to coldplugged bridges
From: |
Michael S. Tsirkin |
Subject: |
[PULL 21/56] pci: acpihp: assign BSEL only to coldplugged bridges |
Date: |
Mon, 30 Jan 2023 15:19:51 -0500 |
From: Igor Mammedov <imammedo@redhat.com>
ACPI PCI hotplug would broken after bridge hotplug and then migration
if hotplugged bridge were specified on target at command line.
Currently it's not possible since, 'hotplugged' property was made
read-only for some time now.
The issue would happen due to BSEL being assigned to all bridges
during 1st 'reset':
source seq:
1. start 'pc' machine => sets BSEL to 0 on pci.0 (host-bridge)
2. hotplug bridge, no bsel is assigned (so far is ok)
target seq:
1. start 'pc' machine with
-S -device pci-bridge,id=hp_br,hotplugged=on
BSEL gets assigned to as follows
hp_br: 0
pci.0: 1
as result hotplug requests with migrated AML generated on source
would be misdirected to 'hp_br' instead of intended pci.0
While it's not issue at the moment, it's based on implicit assumptions
* 'hotplugged' property is read-only
* 1st reset happens before QEMU drops into monitor mode
which lets add hotplugged on source bridges as hotplugged ones
(anything added at that stage counts as hotplugged
(yet another assumption))
All of it looks too fragile to me, so lets restrict BSEL only
to cold-plugged bridges explicitly.
Migration wise it shouldn't break anything since assignment order
stays the same:
* user can't specify 'hotplugged=on' on CLI
* user can't specify 'hotplugged=off' at monitor stage or later
on older QEMU versions where 'hotplugged' is RW, hotplug is broken
after migration anyways and we cannot do anything to fix that.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20230112140312.3096331-12-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/acpi/pcihp.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 99a898d9ae..5dc7377411 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -85,31 +85,40 @@ static int acpi_pcihp_get_bsel(PCIBus *bus)
}
}
-/* Assign BSEL property to all buses. In the future, this can be changed
- * to only assign to buses that support hotplug.
- */
+typedef struct {
+ unsigned bsel_alloc;
+ bool has_bridge_hotplug;
+} BSELInfo;
+
+/* Assign BSEL property only to buses that support hotplug. */
static void *acpi_set_bsel(PCIBus *bus, void *opaque)
{
- unsigned *bsel_alloc = opaque;
+ BSELInfo *info = opaque;
unsigned *bus_bsel;
+ DeviceState *br = bus->qbus.parent;
+ bool is_bridge = IS_PCI_BRIDGE(br);
+ /* hotplugged bridges can't be described in ACPI ignore them */
if (qbus_is_hotpluggable(BUS(bus))) {
- bus_bsel = g_malloc(sizeof *bus_bsel);
+ if (!is_bridge || (!br->hotplugged && info->has_bridge_hotplug)) {
+ bus_bsel = g_malloc(sizeof *bus_bsel);
- *bus_bsel = (*bsel_alloc)++;
- object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
- bus_bsel, OBJ_PROP_FLAG_READ);
+ *bus_bsel = info->bsel_alloc++;
+ object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
+ bus_bsel, OBJ_PROP_FLAG_READ);
+ }
}
- return bsel_alloc;
+ return info;
}
-static void acpi_set_pci_info(void)
+static void acpi_set_pci_info(bool has_bridge_hotplug)
{
static bool bsel_is_set;
Object *host = acpi_get_i386_pci_host();
PCIBus *bus;
- unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
+ BSELInfo info = { .bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT,
+ .has_bridge_hotplug = has_bridge_hotplug };
if (bsel_is_set) {
return;
@@ -123,7 +132,7 @@ static void acpi_set_pci_info(void)
bus = PCI_HOST_BRIDGE(host)->bus;
if (bus) {
/* Scan all PCI buses. Set property to enable acpi based hotplug. */
- pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
+ pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &info);
}
}
@@ -287,7 +296,7 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool
acpihp_root_off)
if (acpihp_root_off) {
acpi_pcihp_disable_root_bus();
}
- acpi_set_pci_info();
+ acpi_set_pci_info(!s->legacy_piix);
acpi_pcihp_update(s);
}
--
MST
- [PULL 01/56] shpc: disallow unplug when power indicator is blinking, (continued)
- [PULL 01/56] shpc: disallow unplug when power indicator is blinking, Michael S. Tsirkin, 2023/01/30
- [PULL 18/56] x86: acpi: pcihp: clean up duplicate bridge_in_acpi assignment, Michael S. Tsirkin, 2023/01/30
- [PULL 19/56] pci: acpi hotplug: rename x-native-hotplug to x-do-not-expose-native-hotplug-cap, Michael S. Tsirkin, 2023/01/30
- [PULL 24/56] tests: acpi: extend bridge tests with hotplugged bridges, Michael S. Tsirkin, 2023/01/30
- [PULL 22/56] x86: pcihp: fix invalid AML PCNT calls to hotplugged bridges, Michael S. Tsirkin, 2023/01/30
- [PULL 23/56] tests: boot_sector_test: avoid crashing if status is not available yet, Michael S. Tsirkin, 2023/01/30
- [PULL 11/56] tests: qtest: print device_add error before failing test, Michael S. Tsirkin, 2023/01/30
- [PULL 28/56] pcihp: drop pcihp_bridge_en dependency when composing PCNT method, Michael S. Tsirkin, 2023/01/30
- [PULL 27/56] tests: acpi: whitelist DSDT before refactoring acpi based PCI hotplug machinery, Michael S. Tsirkin, 2023/01/30
- [PULL 29/56] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/01/30
- [PULL 21/56] pci: acpihp: assign BSEL only to coldplugged bridges,
Michael S. Tsirkin <=
- [PULL 30/56] tests: acpi: whitelist DSDT before refactoring acpi based PCI hotplug machinery, Michael S. Tsirkin, 2023/01/30
- [PULL 31/56] pcihp: compose PCNT callchain right before its user _GPE._E01, Michael S. Tsirkin, 2023/01/30
- [PULL 33/56] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/01/30
- [PULL 38/56] pci: acpi: wire up AcpiDevAmlIf interface to generic bridge, Michael S. Tsirkin, 2023/01/30
- [PULL 35/56] tests: acpi: add endpoint devices to bridges, Michael S. Tsirkin, 2023/01/30
- [PULL 26/56] tests: acpi: add reboot cycle to bridge test, Michael S. Tsirkin, 2023/01/30
- [PULL 55/56] Revert "vhost-user: Introduce nested event loop in vhost_user_read()", Michael S. Tsirkin, 2023/01/30
- [PULL 45/56] tests: acpi: whitelist DSDT blobs before removing dynamic _DSM on coldplugged bridges, Michael S. Tsirkin, 2023/01/30
- [PULL 37/56] x86: pcihp: acpi: prepare slot ignore rule to work with self describing bridges, Michael S. Tsirkin, 2023/01/30
- [PULL 46/56] pcihp: acpi: ignore coldplugged bridges when composing hotpluggable slots, Michael S. Tsirkin, 2023/01/30