* [RFC PATCH v4 1/7] hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-13 6:26 ` [RFC PATCH v4 2/7] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
PCI Express does not allow hot-plug on pcie.0. Check for Q35 in
acpi_pcihp_disable_root_bus() to be able to forbid hot-plug using the
'acpi-root-pci-hotplug' flag.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/pcihp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 4999277d57..09f531e941 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -122,13 +122,14 @@ static void acpi_set_pci_info(void)
static void acpi_pcihp_disable_root_bus(void)
{
static bool root_hp_disabled;
+ Object *host = acpi_get_i386_pci_host();
PCIBus *bus;
if (root_hp_disabled) {
return;
}
- bus = find_i440fx();
+ bus = PCI_HOST_BRIDGE(host)->bus;
if (bus) {
/* setting the hotplug handler to NULL makes the bus non-hotpluggable */
qbus_set_hotplug_handler(BUS(bus), NULL);
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC PATCH v4 2/7] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
2021-05-13 6:26 ` [RFC PATCH v4 1/7] hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35 Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-13 6:26 ` [RFC PATCH v4 3/7] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
Implement notifications and gpe to support q35 ACPI PCI hot-plug.
Use 0xcc4 - 0xcd7 range for 'acpi-pci-hotplug' io ports.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.h | 4 ++++
include/hw/acpi/ich9.h | 2 ++
include/hw/acpi/pcihp.h | 3 ++-
hw/acpi/pcihp.c | 6 +++---
hw/acpi/piix4.c | 4 +++-
hw/i386/acpi-build.c | 30 +++++++++++++++++++-----------
6 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 74df5fc612..487ec7710f 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -5,6 +5,10 @@
extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
+/* PCI Hot-plug registers bases. See docs/spec/acpi_pci_hotplug.txt */
+#define ACPI_PCIHP_SEJ_BASE 0x8
+#define ACPI_PCIHP_BNMR_BASE 0x10
+
void acpi_setup(void);
#endif
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index df519e40b5..596120d97f 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -28,6 +28,8 @@
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/acpi/tco.h"
+#define ACPI_PCIHP_ADDR_ICH9 0x0cc4
+
typedef struct ICH9LPCPMRegs {
/*
* In ich9 spec says that pm1_cnt register is 32bit width and
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index 2dd90aea30..af1a169fc3 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -55,7 +55,8 @@ typedef struct AcpiPciHpState {
} AcpiPciHpState;
void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root,
- MemoryRegion *address_space_io, bool bridges_enabled);
+ MemoryRegion *address_space_io, bool bridges_enabled,
+ uint16_t io_base);
void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp);
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 09f531e941..a55992ed9f 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -37,7 +37,6 @@
#include "qom/qom-qobject.h"
#include "trace.h"
-#define ACPI_PCIHP_ADDR 0xae00
#define ACPI_PCIHP_SIZE 0x0018
#define PCI_UP_BASE 0x0000
#define PCI_DOWN_BASE 0x0004
@@ -489,10 +488,11 @@ static const MemoryRegionOps acpi_pcihp_io_ops = {
};
void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus,
- MemoryRegion *address_space_io, bool bridges_enabled)
+ MemoryRegion *address_space_io, bool bridges_enabled,
+ uint16_t io_base)
{
s->io_len = ACPI_PCIHP_SIZE;
- s->io_base = ACPI_PCIHP_ADDR;
+ s->io_base = io_base;
s->root = root_bus;
s->legacy_piix = !bridges_enabled;
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0bd23d74e2..48f7a1edbc 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -49,6 +49,8 @@
#define GPE_BASE 0xafe0
#define GPE_LEN 4
+#define ACPI_PCIHP_ADDR_PIIX4 0xae00
+
struct pci_status {
uint32_t up; /* deprecated, maintained for migration compatibility */
uint32_t down;
@@ -607,7 +609,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
if (s->use_acpi_hotplug_bridge || s->use_acpi_root_pci_hotplug) {
acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
- s->use_acpi_hotplug_bridge);
+ s->use_acpi_hotplug_bridge, ACPI_PCIHP_ADDR_PIIX4);
}
s->cpu_hotplug_legacy = true;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index bfecb0038c..1cc5c3b791 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -217,10 +217,6 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
/* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
pm->fadt.rev = 1;
pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
- pm->pcihp_io_base =
- object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
- pm->pcihp_io_len =
- object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
}
if (lpc) {
uint64_t smi_features = object_property_get_uint(lpc,
@@ -236,6 +232,10 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
pm->smi_on_cpu_unplug =
!!(smi_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOT_UNPLUG_BIT));
}
+ pm->pcihp_io_base =
+ object_property_get_uint(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
+ pm->pcihp_io_len =
+ object_property_get_uint(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
/* The above need not be conditional on machine type because the reset port
* happens to be the same on PIIX (pc) and ICH9 (q35). */
@@ -388,6 +388,9 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
if (!pdev) {
if (bsel) { /* add hotplug slots for non present devices */
+ if (pci_bus_is_express(bus) && slot > 0) {
+ break;
+ }
dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
@@ -512,7 +515,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
QLIST_FOREACH(sec, &bus->child, sibling) {
int32_t devfn = sec->parent_dev->devfn;
- if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
+ if (pci_bus_is_root(sec)) {
continue;
}
@@ -1242,7 +1245,7 @@ static void build_piix4_isa_bridge(Aml *table)
aml_append(table, scope);
}
-static void build_piix4_pci_hotplug(Aml *table)
+static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
{
Aml *scope;
Aml *field;
@@ -1251,20 +1254,22 @@ static void build_piix4_pci_hotplug(Aml *table)
scope = aml_scope("_SB.PCI0");
aml_append(scope,
- aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x08));
+ aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(pcihp_addr), 0x08));
field = aml_field("PCST", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("PCIU", 32));
aml_append(field, aml_named_field("PCID", 32));
aml_append(scope, field);
aml_append(scope,
- aml_operation_region("SEJ", AML_SYSTEM_IO, aml_int(0xae08), 0x04));
+ aml_operation_region("SEJ", AML_SYSTEM_IO,
+ aml_int(pcihp_addr + ACPI_PCIHP_SEJ_BASE), 0x04));
field = aml_field("SEJ", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("B0EJ", 32));
aml_append(scope, field);
aml_append(scope,
- aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x08));
+ aml_operation_region("BNMR", AML_SYSTEM_IO,
+ aml_int(pcihp_addr + ACPI_PCIHP_BNMR_BASE), 0x08));
field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
aml_append(field, aml_named_field("BNUM", 32));
aml_append(field, aml_named_field("PIDX", 32));
@@ -1396,7 +1401,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
build_piix4_isa_bridge(dsdt);
build_isa_devices_aml(dsdt);
if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
- build_piix4_pci_hotplug(dsdt);
+ build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base);
}
build_piix4_pci0_int(dsdt);
} else {
@@ -1444,6 +1449,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
}
build_q35_isa_bridge(dsdt);
build_isa_devices_aml(dsdt);
+ if (pm->pcihp_bridge_en) {
+ build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base);
+ }
build_q35_pci0_int(dsdt);
if (pcms->smbus && !pcmc->do_not_add_smb_acpi) {
build_smb0(dsdt, pcms->smbus, ICH9_SMB_DEV, ICH9_SMB_FUNC);
@@ -1478,7 +1486,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
{
aml_append(scope, aml_name_decl("_HID", aml_string("ACPI0006")));
- if (misc->is_piix4 && (pm->pcihp_bridge_en || pm->pcihp_root_en)) {
+ if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
method = aml_method("_E01", 0, AML_NOTSERIALIZED);
aml_append(method,
aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF));
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC PATCH v4 3/7] hw/acpi/ich9: Enable ACPI PCI hot-plug
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
2021-05-13 6:26 ` [RFC PATCH v4 1/7] hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35 Julia Suvorova
2021-05-13 6:26 ` [RFC PATCH v4 2/7] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-20 13:46 ` Igor Mammedov
2021-05-13 6:26 ` [RFC PATCH v4 4/7] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
` (6 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
Add acpi_pcihp to ich9_pm as part of
'acpi-pci-hotplug-with-bridge-support' option. Set default to false.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
hw/i386/acpi-build.h | 1 +
include/hw/acpi/ich9.h | 3 ++
hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++++++++++++++
hw/acpi/pcihp.c | 5 +++-
hw/i386/acpi-build.c | 2 +-
5 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
index 487ec7710f..0dce155c8c 100644
--- a/hw/i386/acpi-build.h
+++ b/hw/i386/acpi-build.h
@@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
#define ACPI_PCIHP_BNMR_BASE 0x10
void acpi_setup(void);
+Object *acpi_get_i386_pci_host(void);
#endif
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 596120d97f..a329ce43ab 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -24,6 +24,7 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/cpu_hotplug.h"
#include "hw/acpi/cpu.h"
+#include "hw/acpi/pcihp.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/acpi/tco.h"
@@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs {
AcpiCpuHotplug gpe_cpu;
CPUHotplugState cpuhp_state;
+ bool use_acpi_hotplug_bridge;
+ AcpiPciHpState acpi_pci_hotplug;
MemHotplugState acpi_memory_hotplug;
uint8_t disable_s3;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 4daa79ec8d..f6819c4f2a 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = {
}
};
+static bool vmstate_test_use_pcihp(void *opaque)
+{
+ ICH9LPCPMRegs *s = opaque;
+
+ return s->use_acpi_hotplug_bridge;
+}
+
+static const VMStateDescription vmstate_pcihp_state = {
+ .name = "ich9_pm/pcihp",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = vmstate_test_use_pcihp,
+ .fields = (VMStateField[]) {
+ VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug,
+ ICH9LPCPMRegs,
+ NULL, NULL),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
const VMStateDescription vmstate_ich9_pm = {
.name = "ich9_pm",
.version_id = 1,
@@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = {
&vmstate_memhp_state,
&vmstate_tco_io_state,
&vmstate_cpuhp_state,
+ &vmstate_pcihp_state,
NULL
}
};
@@ -259,6 +280,7 @@ static void pm_reset(void *opaque)
}
pm->smi_en_wmask = ~0;
+ acpi_pcihp_reset(&pm->acpi_pci_hotplug, true);
acpi_update_sci(&pm->acpi_regs, pm->irq);
}
@@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
pm->enable_tco = true;
acpi_pm_tco_init(&pm->tco_regs, &pm->io);
+ if (pm->use_acpi_hotplug_bridge) {
+ acpi_pcihp_init(OBJECT(lpc_pci),
+ &pm->acpi_pci_hotplug,
+ pci_get_bus(lpc_pci),
+ pci_address_space_io(lpc_pci),
+ true,
+ ACPI_PCIHP_ADDR_ICH9);
+
+ qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)),
+ OBJECT(lpc_pci));
+ }
+
pm->irq = sci_irq;
qemu_register_reset(pm_reset, pm);
pm->powerdown_notifier.notify = pm_powerdown_req;
@@ -368,6 +402,21 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp)
s->pm.enable_tco = value;
}
+static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp)
+{
+ ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
+
+ return s->pm.use_acpi_hotplug_bridge;
+}
+
+static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value,
+ Error **errp)
+{
+ ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
+
+ s->pm.use_acpi_hotplug_bridge = value;
+}
+
void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
{
static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
@@ -376,6 +425,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
pm->disable_s3 = 0;
pm->disable_s4 = 0;
pm->s4_val = 2;
+ pm->use_acpi_hotplug_bridge = false;
object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
&pm->pm_io_base, OBJ_PROP_FLAG_READ);
@@ -399,6 +449,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED,
ich9_pm_get_enable_tco,
ich9_pm_set_enable_tco);
+ object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support",
+ ich9_pm_get_acpi_pci_hotplug,
+ ich9_pm_set_acpi_pci_hotplug);
}
void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
@@ -406,6 +459,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
{
ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+ acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp);
+ return;
+ }
+
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
!lpc->pm.acpi_memory_hotplug.is_enabled) {
error_setg(errp,
@@ -441,6 +499,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
} else {
acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp);
}
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+ acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug,
+ dev, errp);
} else {
error_setg(errp, "acpi: device plug request for not supported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -473,6 +534,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state,
dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+ acpi_pcihp_device_unplug_request_cb(hotplug_dev,
+ &lpc->pm.acpi_pci_hotplug,
+ dev, errp);
} else {
error_setg(errp, "acpi: device unplug request for not supported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -490,6 +555,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
!lpc->pm.cpu_hotplug_legacy) {
acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+ acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug,
+ dev, errp);
} else {
error_setg(errp, "acpi: device unplug for not supported device"
" type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index a55992ed9f..5355618608 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -30,6 +30,8 @@
#include "hw/pci-host/i440fx.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_host.h"
+#include "hw/i386/acpi-build.h"
#include "hw/acpi/acpi.h"
#include "hw/pci/pci_bus.h"
#include "migration/vmstate.h"
@@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
static void acpi_set_pci_info(void)
{
static bool bsel_is_set;
+ Object *host = acpi_get_i386_pci_host();
PCIBus *bus;
unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
@@ -111,7 +114,7 @@ static void acpi_set_pci_info(void)
}
bsel_is_set = true;
- bus = find_i440fx(); /* TODO: Q35 support */
+ 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);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1cc5c3b791..a2e883f75c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -295,7 +295,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
* Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
* On i386 arch we only have two pci hosts, so we can look only for them.
*/
-static Object *acpi_get_i386_pci_host(void)
+Object *acpi_get_i386_pci_host(void)
{
PCIHostState *host;
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 3/7] hw/acpi/ich9: Enable ACPI PCI hot-plug
2021-05-13 6:26 ` [RFC PATCH v4 3/7] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
@ 2021-05-20 13:46 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-05-20 13:46 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Eduardo Habkost, qemu-devel, Michael S. Tsirkin
On Thu, 13 May 2021 08:26:38 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> Add acpi_pcihp to ich9_pm as part of
> 'acpi-pci-hotplug-with-bridge-support' option. Set default to false.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Modulo code style nit below, looks fine to me:
With that fixed,
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.h | 1 +
> include/hw/acpi/ich9.h | 3 ++
> hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/pcihp.c | 5 +++-
> hw/i386/acpi-build.c | 2 +-
> 5 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h
> index 487ec7710f..0dce155c8c 100644
> --- a/hw/i386/acpi-build.h
> +++ b/hw/i386/acpi-build.h
> @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio;
> #define ACPI_PCIHP_BNMR_BASE 0x10
>
> void acpi_setup(void);
> +Object *acpi_get_i386_pci_host(void);
>
> #endif
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index 596120d97f..a329ce43ab 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -24,6 +24,7 @@
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/cpu_hotplug.h"
> #include "hw/acpi/cpu.h"
> +#include "hw/acpi/pcihp.h"
> #include "hw/acpi/memory_hotplug.h"
> #include "hw/acpi/acpi_dev_interface.h"
> #include "hw/acpi/tco.h"
> @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs {
> AcpiCpuHotplug gpe_cpu;
> CPUHotplugState cpuhp_state;
>
> + bool use_acpi_hotplug_bridge;
> + AcpiPciHpState acpi_pci_hotplug;
> MemHotplugState acpi_memory_hotplug;
>
> uint8_t disable_s3;
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 4daa79ec8d..f6819c4f2a 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = {
> }
> };
>
> +static bool vmstate_test_use_pcihp(void *opaque)
> +{
> + ICH9LPCPMRegs *s = opaque;
> +
> + return s->use_acpi_hotplug_bridge;
> +}
> +
> +static const VMStateDescription vmstate_pcihp_state = {
> + .name = "ich9_pm/pcihp",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = vmstate_test_use_pcihp,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug,
> + ICH9LPCPMRegs,
> + NULL, NULL),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> const VMStateDescription vmstate_ich9_pm = {
> .name = "ich9_pm",
> .version_id = 1,
> @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = {
> &vmstate_memhp_state,
> &vmstate_tco_io_state,
> &vmstate_cpuhp_state,
> + &vmstate_pcihp_state,
> NULL
> }
> };
> @@ -259,6 +280,7 @@ static void pm_reset(void *opaque)
> }
> pm->smi_en_wmask = ~0;
>
> + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true);
> acpi_update_sci(&pm->acpi_regs, pm->irq);
> }
>
> @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> pm->enable_tco = true;
> acpi_pm_tco_init(&pm->tco_regs, &pm->io);
>
> + if (pm->use_acpi_hotplug_bridge) {
> + acpi_pcihp_init(OBJECT(lpc_pci),
> + &pm->acpi_pci_hotplug,
> + pci_get_bus(lpc_pci),
> + pci_address_space_io(lpc_pci),
> + true,
> + ACPI_PCIHP_ADDR_ICH9);
> +
> + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)),
> + OBJECT(lpc_pci));
> + }
> +
> pm->irq = sci_irq;
> qemu_register_reset(pm_reset, pm);
> pm->powerdown_notifier.notify = pm_powerdown_req;
> @@ -368,6 +402,21 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp)
> s->pm.enable_tco = value;
> }
>
> +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp)
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + return s->pm.use_acpi_hotplug_bridge;
> +}
> +
> +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value,
> + Error **errp)
misaligned line
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + s->pm.use_acpi_hotplug_bridge = value;
> +}
> +
> void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> {
> static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
> @@ -376,6 +425,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> pm->disable_s3 = 0;
> pm->disable_s4 = 0;
> pm->s4_val = 2;
> + pm->use_acpi_hotplug_bridge = false;
>
> object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> @@ -399,6 +449,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED,
> ich9_pm_get_enable_tco,
> ich9_pm_set_enable_tco);
> + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support",
> + ich9_pm_get_acpi_pci_hotplug,
> + ich9_pm_set_acpi_pci_hotplug);
> }
>
> void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> @@ -406,6 +459,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> {
> ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
>
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp);
> + return;
> + }
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
> !lpc->pm.acpi_memory_hotplug.is_enabled) {
> error_setg(errp,
> @@ -441,6 +499,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> } else {
> acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp);
> }
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug,
> + dev, errp);
> } else {
> error_setg(errp, "acpi: device plug request for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -473,6 +534,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>
> acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state,
> dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> + acpi_pcihp_device_unplug_request_cb(hotplug_dev,
> + &lpc->pm.acpi_pci_hotplug,
> + dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug request for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -490,6 +555,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
> !lpc->pm.cpu_hotplug_legacy) {
> acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug,
> + dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug for not supported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index a55992ed9f..5355618608 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -30,6 +30,8 @@
> #include "hw/pci-host/i440fx.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_host.h"
> +#include "hw/i386/acpi-build.h"
> #include "hw/acpi/acpi.h"
> #include "hw/pci/pci_bus.h"
> #include "migration/vmstate.h"
> @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque)
> static void acpi_set_pci_info(void)
> {
> static bool bsel_is_set;
> + Object *host = acpi_get_i386_pci_host();
> PCIBus *bus;
> unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
>
> @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void)
> }
> bsel_is_set = true;
>
> - bus = find_i440fx(); /* TODO: Q35 support */
> + 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);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 1cc5c3b791..a2e883f75c 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -295,7 +295,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
> * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE.
> * On i386 arch we only have two pci hosts, so we can look only for them.
> */
> -static Object *acpi_get_i386_pci_host(void)
> +Object *acpi_get_i386_pci_host(void)
> {
> PCIHostState *host;
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v4 4/7] hw/pci/pcie: Do not set HPC flag if acpihp is used
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (2 preceding siblings ...)
2021-05-13 6:26 ` [RFC PATCH v4 3/7] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-20 14:14 ` Igor Mammedov
2021-05-13 6:26 ` [RFC PATCH v4 5/7] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
` (5 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
Instead of changing the hot-plug type in _OSC register, do not
set the 'Hot-Plug Capable' flag. This way guest will choose ACPI
hot-plug if it is preferred and leave the option to use SHPC with
pcie-pci-bridge.
The ability to control hot-plug for each downstream port is retained,
while 'hotplug=off' on the port means all hot-plug types are disabled.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
include/hw/boards.h | 1 +
hw/acpi/pcihp.c | 8 ++++++++
hw/core/machine.c | 19 +++++++++++++++++++
hw/i386/pc_q35.c | 8 ++++++++
hw/pci/pcie.c | 11 ++++++++++-
5 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3d55d2bd62..a20ca5ab37 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -334,6 +334,7 @@ struct MachineState {
CpuTopology smp;
struct NVDIMMState *nvdimms_state;
struct NumaState *numa_state;
+ bool native_pci_hotplug;
};
#define DEFINE_MACHINE(namestr, machine_initfn) \
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 5355618608..7a6bc1b31e 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -31,6 +31,7 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_port.h"
#include "hw/i386/acpi-build.h"
#include "hw/acpi/acpi.h"
#include "hw/pci/pci_bus.h"
@@ -332,6 +333,13 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
+ /* Remove all hot-plug handlers if hot-plug is disabled on slot */
+ if (object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT) &&
+ !PCIE_SLOT(pdev)->hotplug) {
+ qbus_set_hotplug_handler(BUS(sec), NULL);
+ return;
+ }
+
qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev));
/* We don't have to overwrite any other hotplug handler yet */
assert(QLIST_EMPTY(&sec->child));
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1bf0e687b9..a3411f0e04 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -581,6 +581,21 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
ms->ram_memdev_id = g_strdup(value);
}
+static void machine_set_native_pci_hotplug(Object *obj, bool value,
+ Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+
+ ms->native_pci_hotplug = value;
+}
+
+static bool machine_get_native_pci_hotplug(Object *obj, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+
+ return ms->native_pci_hotplug;
+}
+
static void machine_init_notify(Notifier *notifier, void *data)
{
@@ -891,6 +906,10 @@ static void machine_class_init(ObjectClass *oc, void *data)
object_class_property_set_description(oc, "confidential-guest-support",
"Set confidential guest scheme to support");
+ object_class_property_add_bool(oc, "x-native-pci-hotplug",
+ machine_get_native_pci_hotplug,
+ machine_set_native_pci_hotplug);
+
/* For compatibility */
object_class_property_add_str(oc, "memory-encryption",
machine_get_memory_encryption, machine_set_memory_encryption);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 46a0f196f4..1059ebfdc7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -136,6 +136,7 @@ static void pc_q35_init(MachineState *machine)
ram_addr_t lowmem;
DriveInfo *hd[MAX_SATA_PORTS];
MachineClass *mc = MACHINE_GET_CLASS(machine);
+ bool acpi_pcihp;
/* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
* and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -236,6 +237,13 @@ static void pc_q35_init(MachineState *machine)
object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
OBJECT(lpc), &error_abort);
+ acpi_pcihp = object_property_get_bool(OBJECT(lpc),
+ "acpi-pci-hotplug-with-bridge-support",
+ NULL);
+
+ object_property_set_bool(OBJECT(machine), "x-native-pci-hotplug",
+ !acpi_pcihp, NULL);
+
/* irq lines */
gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index fd0fa157e8..be331310c3 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -517,6 +517,9 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
{
uint32_t pos = dev->exp.exp_cap;
+ bool native_pcihp_enabled = object_property_get_bool(qdev_get_machine(),
+ "x-native-pci-hotplug",
+ NULL);
pci_word_test_and_set_mask(dev->config + pos + PCI_EXP_FLAGS,
PCI_EXP_FLAGS_SLOT);
@@ -529,7 +532,13 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
PCI_EXP_SLTCAP_PIP |
PCI_EXP_SLTCAP_AIP |
PCI_EXP_SLTCAP_ABP);
- if (s->hotplug) {
+
+ /*
+ * Enable native hot-plug on all hot-plugged bridges unless
+ * hot-plug is disabled on the slot.
+ */
+ if (s->hotplug &&
+ (native_pcihp_enabled || DEVICE(dev)->hotplugged)) {
pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
PCI_EXP_SLTCAP_HPS |
PCI_EXP_SLTCAP_HPC);
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 4/7] hw/pci/pcie: Do not set HPC flag if acpihp is used
2021-05-13 6:26 ` [RFC PATCH v4 4/7] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
@ 2021-05-20 14:14 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-05-20 14:14 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Michael S. Tsirkin, qemu-devel, Eduardo Habkost
On Thu, 13 May 2021 08:26:39 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> Instead of changing the hot-plug type in _OSC register, do not
> set the 'Hot-Plug Capable' flag. This way guest will choose ACPI
> hot-plug if it is preferred and leave the option to use SHPC with
> pcie-pci-bridge.
>
> The ability to control hot-plug for each downstream port is retained,
> while 'hotplug=off' on the port means all hot-plug types are disabled.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> include/hw/boards.h | 1 +
> hw/acpi/pcihp.c | 8 ++++++++
> hw/core/machine.c | 19 +++++++++++++++++++
> hw/i386/pc_q35.c | 8 ++++++++
> hw/pci/pcie.c | 11 ++++++++++-
> 5 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3d55d2bd62..a20ca5ab37 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -334,6 +334,7 @@ struct MachineState {
> CpuTopology smp;
> struct NVDIMMState *nvdimms_state;
> struct NumaState *numa_state;
> + bool native_pci_hotplug;
> };
>
> #define DEFINE_MACHINE(namestr, machine_initfn) \
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 5355618608..7a6bc1b31e 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -31,6 +31,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bridge.h"
> #include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_port.h"
> #include "hw/i386/acpi-build.h"
> #include "hw/acpi/acpi.h"
> #include "hw/pci/pci_bus.h"
> @@ -332,6 +333,13 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
> object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
> PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>
> + /* Remove all hot-plug handlers if hot-plug is disabled on slot */
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PCIE_SLOT) &&
> + !PCIE_SLOT(pdev)->hotplug) {
> + qbus_set_hotplug_handler(BUS(sec), NULL);
> + return;
> + }
> +
> qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev));
> /* We don't have to overwrite any other hotplug handler yet */
> assert(QLIST_EMPTY(&sec->child));
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 1bf0e687b9..a3411f0e04 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -581,6 +581,21 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
> ms->ram_memdev_id = g_strdup(value);
> }
>
> +static void machine_set_native_pci_hotplug(Object *obj, bool value,
> + Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + ms->native_pci_hotplug = value;
> +}
> +
> +static bool machine_get_native_pci_hotplug(Object *obj, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> +
> + return ms->native_pci_hotplug;
> +}
> +
>
> static void machine_init_notify(Notifier *notifier, void *data)
> {
> @@ -891,6 +906,10 @@ static void machine_class_init(ObjectClass *oc, void *data)
> object_class_property_set_description(oc, "confidential-guest-support",
> "Set confidential guest scheme to support");
>
> + object_class_property_add_bool(oc, "x-native-pci-hotplug",
> + machine_get_native_pci_hotplug,
> + machine_set_native_pci_hotplug);
> +
> /* For compatibility */
> object_class_property_add_str(oc, "memory-encryption",
> machine_get_memory_encryption, machine_set_memory_encryption);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 46a0f196f4..1059ebfdc7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -136,6 +136,7 @@ static void pc_q35_init(MachineState *machine)
> ram_addr_t lowmem;
> DriveInfo *hd[MAX_SATA_PORTS];
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> + bool acpi_pcihp;
>
> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> @@ -236,6 +237,13 @@ static void pc_q35_init(MachineState *machine)
> object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
> OBJECT(lpc), &error_abort);
>
> + acpi_pcihp = object_property_get_bool(OBJECT(lpc),
> + "acpi-pci-hotplug-with-bridge-support",
> + NULL);
> +
> + object_property_set_bool(OBJECT(machine), "x-native-pci-hotplug",
> + !acpi_pcihp, NULL);
> /* irq lines */
> gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index fd0fa157e8..be331310c3 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -517,6 +517,9 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
> void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> {
> uint32_t pos = dev->exp.exp_cap;
> + bool native_pcihp_enabled = object_property_get_bool(qdev_get_machine(),
> + "x-native-pci-hotplug",
> + NULL);
how about replacing of q35_machine -> machine -> qdev_get_machine() with
PCIESlot::enable_native_hotplug property
and then in pc_q35_init() just adding compat/global property at runtime
if(acpi_hotplug)
object_register_sugar_prop(TYPE_PCIE_ROOT_PORT, "enable_native_hotplug",
"false", true);
after that every port created will have that property set automatically.
or something like this.
It's also hackish but a bit better as you don't have pollute generic machine
code with "x-native-pci-hotplug" and don't have to call qdev_get_machine() for
every port.
> pci_word_test_and_set_mask(dev->config + pos + PCI_EXP_FLAGS,
> PCI_EXP_FLAGS_SLOT);
> @@ -529,7 +532,13 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> PCI_EXP_SLTCAP_PIP |
> PCI_EXP_SLTCAP_AIP |
> PCI_EXP_SLTCAP_ABP);
> - if (s->hotplug) {
> +
> + /*
> + * Enable native hot-plug on all hot-plugged bridges unless
> + * hot-plug is disabled on the slot.
> + */
> + if (s->hotplug &&
> + (native_pcihp_enabled || DEVICE(dev)->hotplugged)) {
> pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
> PCI_EXP_SLTCAP_HPS |
> PCI_EXP_SLTCAP_HPC);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v4 5/7] bios-tables-test: Allow changes in DSDT ACPI tables
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (3 preceding siblings ...)
2021-05-13 6:26 ` [RFC PATCH v4 4/7] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-20 14:14 ` Igor Mammedov
2021-05-13 6:26 ` [RFC PATCH v4 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
` (4 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
All DSDT Q35 tables will be modified because ACPI hot-plug is enabled
by default.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..c5167f48af 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,12 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.tis",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/q35/DSDT.numamem",
+"tests/data/acpi/q35/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.nohpet",
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 5/7] bios-tables-test: Allow changes in DSDT ACPI tables
2021-05-13 6:26 ` [RFC PATCH v4 5/7] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
@ 2021-05-20 14:14 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-05-20 14:14 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Eduardo Habkost, qemu-devel, Michael S. Tsirkin
On Thu, 13 May 2021 08:26:40 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> All DSDT Q35 tables will be modified because ACPI hot-plug is enabled
> by default.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..c5167f48af 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,12 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/DSDT",
> +"tests/data/acpi/q35/DSDT.tis",
> +"tests/data/acpi/q35/DSDT.bridge",
> +"tests/data/acpi/q35/DSDT.mmio64",
> +"tests/data/acpi/q35/DSDT.ipmibt",
> +"tests/data/acpi/q35/DSDT.cphp",
> +"tests/data/acpi/q35/DSDT.memhp",
> +"tests/data/acpi/q35/DSDT.acpihmat",
> +"tests/data/acpi/q35/DSDT.numamem",
> +"tests/data/acpi/q35/DSDT.dimmpxm",
> +"tests/data/acpi/q35/DSDT.nohpet",
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v4 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (4 preceding siblings ...)
2021-05-13 6:26 ` [RFC PATCH v4 5/7] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-20 14:23 ` Igor Mammedov
2021-05-13 6:26 ` [RFC PATCH v4 7/7] bios-tables-test: Update golden binaries Julia Suvorova
` (3 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
Q35 has three different types of PCI devices hot-plug: PCIe Native,
SHPC Native and ACPI hot-plug. This patch changes the default choice
for cold-plugged bridges from PCIe Native to ACPI Hot-plug with
ability to use SHPC and PCIe Native for hot-plugged bridges.
This is a list of the PCIe Native hot-plug issues that led to this
change:
* no racy behavior during boot (see 110c477c2ed)
* no delay during deleting - after the actual power off software
must wait at least 1 second before indicating about it. This case
is quite important for users, it even has its own bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1594168
* no timer-based behavior - in addition to the previous example,
the attention button has a 5-second waiting period, during which
the operation can be canceled with a second press. While this
looks fine for manual button control, automation will result in
the need to queue or drop events, and the software receiving
events in all sort of unspecified combinations of attention/power
indicator states, which is racy and uppredictable.
* fixes or reduces the likelihood of the bug:
* https://bugzilla.redhat.com/show_bug.cgi?id=1833187
* https://bugzilla.redhat.com/show_bug.cgi?id=1657077
* https://bugzilla.redhat.com/show_bug.cgi?id=1669931
* https://bugzilla.redhat.com/show_bug.cgi?id=1678290
To return to PCIe Native hot-plug:
-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
hw/acpi/ich9.c | 2 +-
hw/i386/pc.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index f6819c4f2a..e7b2cd9719 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -425,7 +425,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
pm->disable_s3 = 0;
pm->disable_s4 = 0;
pm->s4_val = 2;
- pm->use_acpi_hotplug_bridge = false;
+ pm->use_acpi_hotplug_bridge = true;
object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
&pm->pm_io_base, OBJ_PROP_FLAG_READ);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8cfaf216e7..5c2d3d11a2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -94,7 +94,9 @@
#include "trace.h"
#include CONFIG_DEVICES
-GlobalProperty pc_compat_6_0[] = {};
+GlobalProperty pc_compat_6_0[] = {
+ { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
+};
const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
GlobalProperty pc_compat_5_2[] = {
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-05-13 6:26 ` [RFC PATCH v4 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
@ 2021-05-20 14:23 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-05-20 14:23 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Michael S. Tsirkin, qemu-devel, Eduardo Habkost
On Thu, 13 May 2021 08:26:41 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> Q35 has three different types of PCI devices hot-plug: PCIe Native,
> SHPC Native and ACPI hot-plug. This patch changes the default choice
> for cold-plugged bridges from PCIe Native to ACPI Hot-plug with
> ability to use SHPC and PCIe Native for hot-plugged bridges.
>
> This is a list of the PCIe Native hot-plug issues that led to this
> change:
> * no racy behavior during boot (see 110c477c2ed)
> * no delay during deleting - after the actual power off software
> must wait at least 1 second before indicating about it. This case
> is quite important for users, it even has its own bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> * no timer-based behavior - in addition to the previous example,
> the attention button has a 5-second waiting period, during which
> the operation can be canceled with a second press. While this
> looks fine for manual button control, automation will result in
> the need to queue or drop events, and the software receiving
> events in all sort of unspecified combinations of attention/power
> indicator states, which is racy and uppredictable.
> * fixes or reduces the likelihood of the bug:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1833187
> * https://bugzilla.redhat.com/show_bug.cgi?id=1657077
> * https://bugzilla.redhat.com/show_bug.cgi?id=1669931
> * https://bugzilla.redhat.com/show_bug.cgi?id=1678290
>
> To return to PCIe Native hot-plug:
> -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/ich9.c | 2 +-
> hw/i386/pc.c | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index f6819c4f2a..e7b2cd9719 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -425,7 +425,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> pm->disable_s3 = 0;
> pm->disable_s4 = 0;
> pm->s4_val = 2;
> - pm->use_acpi_hotplug_bridge = false;
> + pm->use_acpi_hotplug_bridge = true;
>
> object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8cfaf216e7..5c2d3d11a2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -94,7 +94,9 @@
> #include "trace.h"
> #include CONFIG_DEVICES
>
> -GlobalProperty pc_compat_6_0[] = {};
> +GlobalProperty pc_compat_6_0[] = {
> + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> +};
> const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
>
> GlobalProperty pc_compat_5_2[] = {
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH v4 7/7] bios-tables-test: Update golden binaries
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (5 preceding siblings ...)
2021-05-13 6:26 ` [RFC PATCH v4 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
@ 2021-05-13 6:26 ` Julia Suvorova
2021-05-20 14:37 ` [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Igor Mammedov
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-05-13 6:26 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Julia Suvorova, Eduardo Habkost, Michael S. Tsirkin
Add ACPI hot-plug registers to DSDT Q35 tables.
Changes in the tables:
+ Scope (_SB.PCI0)
+ {
+ OperationRegion (PCST, SystemIO, 0x0CC4, 0x08)
+ Field (PCST, DWordAcc, NoLock, WriteAsZeros)
+ {
+ PCIU, 32,
+ PCID, 32
+ }
+
+ OperationRegion (SEJ, SystemIO, 0x0CCC, 0x04)
+ Field (SEJ, DWordAcc, NoLock, WriteAsZeros)
+ {
+ B0EJ, 32
+ }
+
+ OperationRegion (BNMR, SystemIO, 0x0CD4, 0x08)
+ Field (BNMR, DWordAcc, NoLock, WriteAsZeros)
+ {
+ BNUM, 32,
+ PIDX, 32
+ }
+
+ Mutex (BLCK, 0x00)
+ Method (PCEJ, 2, NotSerialized)
+ {
+ Acquire (BLCK, 0xFFFF)
+ BNUM = Arg0
+ B0EJ = (One << Arg1)
+ Release (BLCK)
+ Return (Zero)
+ }
+
+ Method (AIDX, 2, NotSerialized)
+ {
+ Acquire (BLCK, 0xFFFF)
+ BNUM = Arg0
+ PIDX = (One << Arg1)
+ Local0 = PIDX /* \_SB_.PCI0.PIDX */
+ Release (BLCK)
+ Return (Local0)
+ }
+
+ Method (PDSM, 6, Serialized)
+ {
+ If ((Arg0 == ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d") /* Device Labeling Interface */))
+ {
+ Local0 = AIDX (Arg4, Arg5)
+ If ((Arg2 == Zero))
+ {
+ If ((Arg1 == 0x02))
+ {
+ If (!((Local0 == Zero) | (Local0 == 0xFFFFFFFF)))
+ {
+ Return (Buffer (One)
+ {
+ 0x81 // .
+ })
+ }
+ }
+
+ Return (Buffer (One)
+ {
+ 0x00 // .
+ })
+ }
+ ElseIf ((Arg2 == 0x07))
+ {
+ Local1 = Package (0x02)
+ {
+ Zero,
+ ""
+ }
+ Local1 [Zero] = Local0
+ Return (Local1)
+ }
+ }
+ }
+ }
+
...
Scope (_GPE)
{
Name (_HID, "ACPI0006" /* GPE Block Device */) // _HID: Hardware ID
+ Method (_E01, 0, NotSerialized) // _Exx: Edge-Triggered GPE, xx=0x00-0xFF
+ {
+ Acquire (\_SB.PCI0.BLCK, 0xFFFF)
+ \_SB.PCI0.PCNT ()
+ Release (\_SB.PCI0.BLCK)
+ }
...
+
+ Device (PHPR)
+ {
+ Name (_HID, "PNP0A06" /* Generic Container Device */) // _HID: Hardware ID
+ Name (_UID, "PCI Hotplug resources") // _UID: Unique ID
+ Name (_STA, 0x0B) // _STA: Status
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ IO (Decode16,
+ 0x0CC4, // Range Minimum
+ 0x0CC4, // Range Maximum
+ 0x01, // Alignment
+ 0x18, // Length
+ )
+ })
+ }
}
...
And if there is a port in configuration:
Device (S10)
{
Name (_ADR, 0x00020000) // _ADR: Address
+ Name (BSEL, Zero)
+ Device (S00)
+ {
+ Name (_SUN, Zero) // _SUN: Slot User Number
+ Name (_ADR, Zero) // _ADR: Address
+ Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device, x=0-9
+ {
+ PCEJ (BSEL, _SUN)
+ }
+
+ Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ {
+ Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+ }
+ }
+
...
+ Method (DVNT, 2, NotSerialized)
+ {
+ If ((Arg0 & One))
+ {
+ Notify (S00, Arg1)
+ }
...
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 11 -----------
tests/data/acpi/q35/DSDT | Bin 7859 -> 8289 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9184 -> 9614 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7877 -> 11003 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8323 -> 8753 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9513 -> 9943 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7934 -> 8364 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9218 -> 9648 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8990 -> 9419 bytes
tests/data/acpi/q35/DSDT.nohpet | Bin 7717 -> 8147 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 8295 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8465 -> 8894 bytes
12 files changed, 11 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index c5167f48af..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,12 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT",
-"tests/data/acpi/q35/DSDT.tis",
-"tests/data/acpi/q35/DSDT.bridge",
-"tests/data/acpi/q35/DSDT.mmio64",
-"tests/data/acpi/q35/DSDT.ipmibt",
-"tests/data/acpi/q35/DSDT.cphp",
-"tests/data/acpi/q35/DSDT.memhp",
-"tests/data/acpi/q35/DSDT.acpihmat",
-"tests/data/acpi/q35/DSDT.numamem",
-"tests/data/acpi/q35/DSDT.dimmpxm",
-"tests/data/acpi/q35/DSDT.nohpet",
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index cccf92f0466fa4eaf2e9e06675b3b102c7a8eb86..842533f53e6db40935c3cdecd1d182edba6c17d4 100644
GIT binary patch
delta 466
zcmdmN`_O^QCD<h-QGtPh@##jc-%Rxaeu8@O!A|i3&YlL*4FS%<A&lHdc(^#C8wEfd
zXP|hf0+4V~h;9gW^@?ZYKEuPs65YrR;y61QxOxF4ocw%)7`d-Nw1GIzPJW@j3IU!j
z5em_aPCm}w3?ixl&aPfe(aIp+|NjLUAQc6b^^BPeAVZ?nLE=joM6?}&nlO|BRe=mE
z01*jLwFx3FYymF8zI+ROSSM!)3UdiuFhBV;*tE)bqWCLc$-~aR1t7C>auXIPPtIak
zAUQdci)n%Iq}s^|43iUh{sRHS5=B8~#>OQ;f?=Wf0@2A?T<irxtV|5N42cB^9f=GK
zOA<G;GT)YD@@JX+P)68R#4_I1z>pzYIYyru7HSX=AqfOH`-McSBME?12>6O_{w~AM
zs2<(q#S-8V5X2Gh;pxH~;1^)vXkf;`5g!WTIeRL2<d+oWl%`L9C@af$gy#qkqr_wu
axlArUmKczQ0&Xmm%j9g?Mc6>LFaQ7^vWU|F
delta 57
zcmaFpu-TT&CD<iovm65h<GYPqznM0xu-ukp6q(E@C(JD1D7ZOLj-PSz8u@fiH<p<A
NV5fKiyUDT&wgCMJ5G4Qr
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index b3c1dd6bc40210425ac37dba88a650b0ea60ce1c..8d00f2ea0dd78f962e136273d68cb0c568e43c27 100644
GIT binary patch
delta 465
zcmaFh-sjEb66_Mvr^>*<xM(BSZ>D+yKS90tV5j&1XHSFZh5+Z_5Jv7JJX{>njRGK!
zGf+HK0Z6zgL^lMxdc`wxpW)$RiEiWuah#nDT)lu2PJX^YjNDfs+CUs<C%;f%g#b^N
z2!&`yCm&~T1`*W&XIC$#Xk`%Z|NnvvkcxuJdd5r!kRj3PAn_#(BHE5XO&H36sz8Pn
zfQSUB+5{07wg8u4U%mxCtdlbYg}DSSn4f$aY+B_zQT&y!<Y8yu0+3lbxd{uDCucD%
zker;!#k4?pQtjjfhRF#$|AByEiJ~AgW8)Ga!LU$#f#~EcF7^T;Rwf2shQxw|jzk8A
zC5f9^nK|Tx{8{4N16(=cJv?1_9i0O_4Gav-7(^`NT@4HwqLpLxnPFiF@g9;ufU{pn
zv^tUiNQHo}=w=lqenyVyCNGu%kAR@bfwCgpz+hJJ$S*0#DNUcur!32Mgy#qkqr_xG
a<xDO=mKcx?0&Xmm7c1Mci?D%=WdHyz0)`?0
delta 71
zcmeD4e&EjK66_N4K$(GoarH*7-%OiTSUBXxMdIB9Tsh)BJY9GlodY}#3=GT|1RMo7
b*D3QePQIy<&gsSy6CdmpFJL#>OVt(t-l7wM
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index eb5d27d95b2cdeda5f7e1f6b151cfea02e6bd907..55ad4bd7ab4759d68031b2f1d0307355a7332fe7 100644
GIT binary patch
delta 3198
zcmb7{PjBNy7{<qr)4Gn+I*q$&cAGY+LX|*TvUJ;~5-Xb6L2YP^RW}I9E1aH4$|7<A
zA*j%#J*>Ebzd(?3;ItCjk3cyfap1s>(;m2R;mDzf?)%1mvr$RXI0q}wJmcRyGxq%P
zuaB<kis_igS8p&T{QI+VC34M5-S?cbXB!6vw__X5v5>e_71=%4P#92N&%?#cyB()e
z^@POlsv^1P2^2=TP^p4ZwrWR0;?LkV6h_&q*Yfs(`BB~#%7@1LOn==rDpk>4M|FMO
zZeor0*IZ~Zgt!}6-edZkCAjH-)u0x_+IVS%bsM@lW1CKGzIzz=o2j&-{ju}g-_F)I
zweK_k)Uv-AwKlSzeAei$`=_j%^;?SAP5<z%-(Y@2y@ns_y_Sjz=RLTvA2Z!0|5TZ6
zFUCcdWGC&$cPEVXPM*hOe`UoFB>%-q`jNitRSFN7yZ-pC7>!!cp`ilXu#Vl0p#rR-
z)s~+Bx3VB?xEEE)-nWlrZ~wqd+Lm1?6}Fk|)lnG-`TftoXg~S#6aPgnH*=|8s=~4_
zt)9+3dyMUAd(Zr@R^R2;PtJpaCwnFHNM%f9a@naIGWX&gr+~#y-2x(*OkeaW)q;Ry
zk5$moGaihZ+QYGHwoX5#a`&PnIfY_yyFhGV6oQXul%*gUG^B_G5l2BLGr}B7j2N*Y
z2A`rc5ajJ}EEI_#(hy`y1_g-|BhGXlWmF-^CIy)x$V}uQn-pYL_IU8zXCp%%QV^Mf
z$k7=^kN00OWsV?oks(_YL?MO}fx#{J0)};qh9p6dL<C5{*rp&VG1S``e0bJCkZlT*
zBuMgh$PLC01(_$t{8$DbIej6>4h2~t$ii62I7X3zXvENNGWg_m1cDSPNQxk-n~+->
zyA))R7>gl;Pr0B$kX;IrCP+GRkP-#aLk4GhbO;4mq999=8RaktSMCtxPIL$b$q*wG
zmBAf@hGdx_%TXbYK|!*_$WF=N{f(@@2ACA&E<x^22^nHgkbA_qH<`h2?ws(VRVc^`
zK~^S1MlmQzju^R-3_f-Xham4!kX3@Lj)Y8PP>?lZtPL|bof*dM;kuPGKeUd;ZsM#d
zu%~z`X?4}JrXr#!hC%`X$C$v8QIMm7fZaQA@+ipJ1F^};>Yzd^dDcuYYzcO_P+?OK
z2OhQsn@;FqW3YRK3R{DHAXL~KEa;)a_F!oa6}AU!EN^ue&YE!Y5*QS~f=OEJYrHw|
zCDgEvk(T@Vm9L?O<%RrwU%&GAP{XVzzth)6J}Bt-873oDfV1FJoLB>m{f0R*(AaZx
z$j83Jt2^-XsE7PL(C|hL{0?dyR(%c6xvb6bAde3lzZ8V<kEz6T_<jKY<NNrC@$XB%
CMohN=
delta 73
zcmewzdeoN7CD<k8s2l?WWBW#~-%OiTSZ+%)icDsd6J{206x<vr$Im!<jeG_tYfOBw
dQ@nr!>ttC4dw%|4LxXsZct@8Y9tH*`1^^Ot5>o&G
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index e55d12990c98d8eed760b858ce18a036b612da1c..ccde2add9f87db0c0eaf0cc155717d5744e4ef40 100644
GIT binary patch
delta 466
zcmZp6+~~sP66_LUsKmg)=(&;WH&eZUpP*iRuv2`1v!_9HLx6K|2qX6q9xjgPMgb7V
z87LmA03=)#q8ox;z2X_U&+u@uL^pDSIL=N6u3kV1CqLgHM(!&RZ6J=blV7N>LV%}B
zghDi<laI4EgNSN?v#S?Vv@(eI|9?RSNJT+qJ!2*V$dG7tkoXb?5p74HCJbdjRUpF(
zKtuvmZGwmkTYyWjFW&+m*2x)y!dwCu%uhZIHm&lVDE`V<^02dS0m!VJ+=K<nld~8W
zNKVe=Vp<?PsdjP#!{h{>|3JX7L{X5Lv2h8IU|1-=Ky-2z7khyaD-#1RLt;TfM<N5m
zlElrd%&VoD{8=V%loPfUv5a>$Fl2~Uj?rg^g&M>|NCE-Qej(B7NCF@g0=}Y~kIV5h
zsz*0@u>^Po1aZWBc)IWg_yrg^8kjL~#D{`-&YlV$`6UH8rRkG5%FD7H;W@&?C^7kp
Zd?uG4OAN?D0XLS(ZVI;SB5WXA7y#g;hnxTa
delta 57
zcmdn!((K6P66_MvtiZs)7_^b=H`8VnmetaXB9kx53o{Ei3T{?Z;Afm%qnOU=#u5`B
N>=Z9xH~FoiEdbPr5E}ph
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index 95901f94c0d445919cb84dd1f6d98c646ae8176e..b062e30117f955c7a2ac9629a0512324bbd28bf2 100644
GIT binary patch
delta 500
zcmZ4Kb={ZCCD<k8x*7umqwz+r-%Rxaeu8@O!A|i3&YlL*4FS%<A&lHdc(^#C8wEfd
zXP|hf0+4V~h;9gW^@?ZYKEuPs65YrR;y61QxOxF4ocw%)7`d-Nw1GIzPJW@j3IU!j
z5em_aPCm}w3?ixl&aPfe(aIp+|NjLUAQc6b^^BPeAVZ?nLE=joM6?}&nlO|BRe=mE
z01*jLwFx3FYymF8zI+ROSSM!)3UdiuFhBV;*tE)bqWCLc$-~aR1t7C>auXIPPtIak
zAUQdci)n%Iq}s^|43iUh{sRHS5=B8~#>OQ;f?=Wf0@2A?T<irxtV|5N42cB^9f=GK
zOA<G;GTSPoIkUyP2e@*?dw9C=Iywh<8W<RuF^E{kyBZiWL@USWGsD6V;yomR0B66D
zXmun3kO~oDpiUEpwis~O`-Qm#aWx3|if+zT=4VuoZt`LY@CXQk*cRXyVBiR}j3Yi2
zXg4q>6g=`v3UW%*Cx@%ZvK`?$!ow&rxk@FI%a0`n<Ol&bmdS5aY}rNFKsGY~09jj*
AH2?qr
delta 75
zcmccayV8rxCD<iIQ<Z^%QFbHOZ>G&EEVc?F8u9J{t{m|mo-VwO&H<hV1_ovflQ*b{
fGC2xvKCZ&gI9W+8ozsmaCO+6HUchehWHnm=`KA-;
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index ce07e9f152def6a22ab29b3bde98b7d1f15a0522..1c5737692f56fc678e685a4ad0bb1df38d487a14 100644
GIT binary patch
delta 480
zcmZWlF;Buk7`=lEq!cYuqYRQ7Vi+`92Q>~3^jZa^<w7AbIg*x1AQ(I7povkOT=o7y
zO&DE_zre_1;^5%s?Bb6Q?}8>I-tdz5?tSm&`@XmD^P=GxwR;r+n0^p<!WXigTyY)U
z#i*!}ON^Wvj1MF+MtU+Qi0O5efuWL1$1J%p{wj$A>BTugx@MNxhi+R{7=Is)ae{Qa
zZmGCv?5hOoWwZyBMU2c+h-5h%4*eE)_@@`J4R{F2alH?er92z@XJalN=5z3Vy`Ex3
zu;EyfL3x6<k||Ms+kE+S*3Yb)*)J>oj4YpLbq~EDOFJM))3z82Qn%;S1Jsj-?1BD5
zG7Nit2H<Wsn4ujp>Q7IEKms&<@45*<zxjy6AL(jEaCfm8a|=Zykw>_wY1#%*wp>$N
z10>s64gcM6wBDG2kzFIXF@ZZ|Yvxg(pMp7ZRT6LkS4YX%##){Q$J{#WVOLiRN5_8m
i<bW<`<|6}XAi+6W$+SgVXz)ULN1#uc?WhvrE#L<Umx=`d
delta 71
zcmZ4E_|KNhCD<k8pBw`NqtZq$H|EXxEVm`aMdIB9Tsh)BJY9GlodY}#3=GT|1RMo7
b&ynM2ocvurozsmaCO+6HUchd0hJq~s<(w1f
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 7acf6243f08cd906aa8a02d3acf1d720b36385ea..7b6f6487b229cac3424a5215c8f1755c0c85310c 100644
GIT binary patch
delta 480
zcmZqj*x=3O66_MPL6w1l@%l!t-%Rxaeu8@O!A|i3&YlL*4FS%<A&lHdc(^#C8wEfd
zXP|hf0+4V~h;9gW^@?ZYKEuPs65YrR;y61QxOxF4ocw%)7`d-Nw1GIzPJW@j3IU!j
z5em_aPCm}w3?ixl&aPfe(aIp+|NjLUAQc6b^^BPeAVZ?nLE=joM6?}&nlO|BRe=mE
z01*jLwFx3FYymF8zI+ROSSM!)3UdiuFhBV;*tE)bqWCLc$-~aR1t7C>auXIPPtIak
zAUQdci)n%Iq}s^|43iUh{sRHS5=B8~#>OQ;f?=Wf0@2A?T<irxtV|5N42cB^9f=GK
zOA<G;GHb~P`Lo2k2e@*?dw9C=Iywh<8W<RuF^E{kyBZiWL@USWGsD6V;yomR0B66D
zXmun3kO~1`(amm3{EX_+O<pVk9sxlRlLPz$3><+5bHs-N?E!|mf=7NyK~8D<WCLYc
jwj(@8co-!n2P<cC`LV=+EEI5KnS4~)mR*DmWD5fTV<d`8
delta 71
zcmdns-Q>aL66_Mfq{6_!cz+|;Z>G&EEL!s7BJu74t{m|mo-VwO&H<hV1_ovf0*->4
b7bx>HPX4Wu&gsSy6CdmpFJL!0N7WVpu^|&B
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 77d46369e48efca9a9e5024542c77cd26144beff..2e0a772a85275c9c3b4c9317d98cc7c3d27417f3 100644
GIT binary patch
delta 479
zcmZWl%}T>S7~Ji@Y1C9x)PoXRr3XPH_=Db>O|+>dU78kztt36!iee6WP!L;BUUlyh
zp)Vjlg1v}>9()BazJiCkOBDq7Ffg<8&G3D<8_%YoJ9_1L1^`I!g|E<sWT)m`M{_YU
zRno%9sla4c6cVH@ae|m$RT$_BX*ovGg~=CDh>&)Y6Qrp|k$q^kRffs;{un1nv#XYZ
zO?^ipP}@dZK%T|OD27OuvtG|{aEE_-0h@q_kQ~=LK%UF9p?@~!;$c1q@5k%OdJOB1
zH56<|X(Kf%NN=l8AI|cbbv^cJO*|sY=UE4bUXZ2xAef*{Ary?>oKp`_PwcS=I@75z
zY<C!dyRjfm_l3AW5)FYtQ1`v7Isl#eLo~cJRW3!`O>89Gf~gPY5jIs-T><i%Yp9EW
zWNQoI-y8PV`y*gvS4i%Uz#Wp6e!$biFh?$n5nRCKeloVPnrFcYx5|3h)s@20v43!~
hN7plxv98z^;W*vRGzUu28ZVSK>1(DGmt(vHd;|L@ije>S
delta 71
zcmX@@InRyDCD<iIPMLv$amq%n-%OiTSWM)^MdIB9Tsh)BJY9GlodY}#3=GT|1RMo7
buTtV?oXo0{&gsD#6CdmpFW@k_RK*_vr*sly
diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet
index 0b10128e42af0c7b65e010963085bbf690a64065..ceb61f4115c2ccf4bcbb0d529551236933ecee15 100644
GIT binary patch
delta 504
zcmZ2#bJ?EDCD<k8vOEI=<NA$UxlHu}eu8@O!A|i3&YlL*4FS%<A&lHdc(^#C8wEfd
zXP|hf0+4V~h;9gW^@?ZYKEuPs65YrR;y61QxOxF4ocw%)7`d-Nw1GIzPJW@j3IU!j
z5em_aPCm}w3?ixl&aPfe(aIp+|NjLUAQc6b^^BPeAVZ?nLE=joM6?}&nlO|BRe=mE
z01*jLwFx3FYymF8zI+ROSSM!)3UdiuFhBV;*tE)bqWCLc$-~aR1t7C>auXIPPtIak
zAUQdci)n%Iq}s^|43iUh{sRHS5=B8~#>OQ;f?=Wf0@2A?T<irxtV|5N42cB^9f=GK
zOA<GiGu@U9@@I*64{+s(_waP#b#xB!G%zqQV-T^7cQr6%h*pl#XNH9##Cu2r0nUCQ
z(dtM7AQb|>qMI{i_!+&To4i;8JOY9sCI|Qh7&rnA=7<jk+5-%A1&{oaf}GNHg`(8r
z{L-T2)MB7qaEK!pH%Gj4P_Q5`7neKZ5uPJFj1rT}WHY(^SYkj95pZLf{6f~2U4#u}
GI|BgciIJ56
delta 71
zcmca?zto1yCD<iIRgQsyar;KDT&B&_m~Tspi^RJJxN^jMc)IX9ItO?f7#Nr_2sjFE
bJ}k@6I9X0UozsmaCO+6HUchd0zq~B~$tDvn
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index e4c4582e7f76b072ab1123c748b89ea33ea1db87..a3f846df541a70ce0730d0351954b78818bbcdd0 100644
GIT binary patch
delta 480
zcmdmK``m%cCD<h-U4emtv411iZ>D+yKS90tV5j&1XHSFZh5+Z_5Jv7JJX{>njRGK!
zGf+HK0Z6zgL^lMxdc`wxpW)$RiEiWuah#nDT)lu2PJX^YjNDfs+CUs<C%;f%g#b^N
z2!&`yCm&~T1`*W&XIC$#Xk`%Z|NnvvkcxuJdd5r!kRj3PAn_#(BHE5XO&H36sz8Pn
zfQSUB+5{07wg8u4U%mxCtdlbYg}DSSn4f$aY+B_zQT&y!<Y8yu0+3lbxd{uDCucD%
zker;!#k4?pQtjjfhRF#$|AByEiJ~AgW8)Ga!LU$#f#~EcF7^T;Rwf2shQxw|jzk8A
zC5f9^nIB39`Lo2k2e@*?dw9C=Iywh<8W<RuF^E{kyBZiWL@USWGsD6V;yomR0B66D
zXmun3kO~1`(anEl_!-rso4i;8JOY9sCI|Qh7&rnA=7<jk+5-%A1&{oaf}GOy$<JkF
j*^clW;bD}R%q5q}<;M~OvQWT{Wpa(2ExQOC$QA|wu$qg6
delta 71
zcmaFvu+x^yCD<ioryK(V<BW}5znM0xusoC$7m0TdaOH^i@O0sIbPn(|FfcG<5O5US
b94^PtIC+zNI;R^;Onk6Yynx+gWd&OR`Gym&
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 15a26a14e4be5280c0f1cc09f66428311100b7ab..d1433e3c14570bbd17b029a9aec6bc53134c3b7d 100644
GIT binary patch
delta 469
zcmbQ}w9l2xCD<iopArKD<JpZ|znSU<`~>ylgPr07oIMSq8v>kzLm0V_@NjWNHwu6_
z&Oq@{1t8&~5Zw^$>J`t(eTIjNCAyIt#Bp{qaP<O8IQjVoF>+slXajMao%}+56#_h6
zA{3$-oqU|V8AMbAoL#+`qLo3s|NjdzKq?9<>lrf{K!!xCgT$9Gh-f<kHDM?Nssb5S
z03s5gY7<0U*aBRFefbvnuujep6y_4RV1DvxuxXX=MDbU?l82pr3qWS&<R&aoo}9(7
zKyq>>7t;daNwt#`7$zt1{09PtC5nQ~jEzfx1j9n{1)`I)xY!GXSeY1j84?Q;IuaQe
zmLzUwWxg%R<j*qsp^UJth-JL1fgwY*a*RGREYu(#LJ|mY_6vztM-l+35bzY+{9Q(x
zQ9ZiJizUD#Ac!O0!_$Q~z%RhS(ZGy>BR&+wbM{p5$S*0#DNUdJR#ukn2+t87Mv2J+
da+#d|nlT^?1>7e$$=R`quz@Vuyk9Pe2>{`!ixL0;
delta 62
zcmdnzI?;*CCD<iIP?3RwasNiH-%OiTSZ+%)icDsd6J{3h72F&sC(Sr{hkQDxhh|KC
Suv5H%!(<HwyUn=@flL4**AWN+
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (6 preceding siblings ...)
2021-05-13 6:26 ` [RFC PATCH v4 7/7] bios-tables-test: Update golden binaries Julia Suvorova
@ 2021-05-20 14:37 ` Igor Mammedov
2021-05-23 8:25 ` Michael S. Tsirkin
2021-05-26 5:39 ` Michael S. Tsirkin
9 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-05-20 14:37 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Michael S. Tsirkin, qemu-devel, Eduardo Habkost
On Thu, 13 May 2021 08:26:35 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> The patch set consists of two parts:
> patches 1-4: introduce new feature
> 'acpi-pci-hotplug-with-bridge-support' on Q35
> patches 5-7: make the feature default along with changes in ACPI tables
>
> This way maintainers can decide which way to choose without breaking
> the patch set.
>
> With the feature disabled Q35 falls back to the native hot-plug.
>
> Pros
> * no racy behavior during boot (see 110c477c2ed)
> * eject is possible - according to PCIe spec, attention button
> press should lead to power off, and then the adapter should be
> removed manually. As there is no power down state exists in QEMU,
> we cannot distinguish between an eject and a power down
> request.
> * no delay during deleting - after the actual power off software
> must wait at least 1 second before indicating about it. This case
> is quite important for users, it even has its own bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> * no timer-based behavior - in addition to the previous example,
> the attention button has a 5-second waiting period, during which
> the operation can be canceled with a second press. While this
> looks fine for manual button control, automation will result in
> the need to queue or drop events, and the software receiving
> events in all sort of unspecified combinations of attention/power
> indicator states, which is racy and uppredictable.
> * fixes or reduces the likelihood of the bugs:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1833187
> * https://bugzilla.redhat.com/show_bug.cgi?id=1657077
> * https://bugzilla.redhat.com/show_bug.cgi?id=1669931
> * https://bugzilla.redhat.com/show_bug.cgi?id=1678290
in addition,
while it's not trivial to test ACPI hotplug at 'make check' time (as it needs AML execution on guest side),
it should be possible to test native PCI-E and SHPC hotplug by using existing qpci_foo()
to read/write respective PCI device registers, so we could test that we didn't regress
current native/SHPC with this series.
It's would be better if test would come as part of this series,
but I'm fine with test being a separate patch on top if
you will take care of it within the same release cycle.
> Cons:
> * no access to possible features presented in slot capabilities
> (this is only surprise removal AFAIK)
>
> v4:
> * regain per-port control over hot-plug
> * rebased over acpi-index changes
> * set property on machine type to
> make pci code more generic [Igor, Michael]
>
> v3:
> * drop change of _OSC to allow SHPC on hotplugged bridges
> * use 'acpi-root-pci-hotplug'
> * add migration states [Igor]
> * minor style changes
>
> v2:
> * new ioport range for acpiphp [Gerd]
> * drop find_pci_host() [Igor]
> * explain magic numbers in _OSC [Igor]
> * drop build_q35_pci_hotplug() wrapper [Igor]
>
> Julia Suvorova (7):
> hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35
> hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
> hw/acpi/ich9: Enable ACPI PCI hot-plug
> hw/pci/pcie: Do not set HPC flag if acpihp is used
> bios-tables-test: Allow changes in DSDT ACPI tables
> hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
> bios-tables-test: Update golden binaries
>
> hw/i386/acpi-build.h | 5 +++
> include/hw/acpi/ich9.h | 5 +++
> include/hw/acpi/pcihp.h | 3 +-
> include/hw/boards.h | 1 +
> hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++
> hw/acpi/pcihp.c | 22 +++++++---
> hw/acpi/piix4.c | 4 +-
> hw/core/machine.c | 19 +++++++++
> hw/i386/acpi-build.c | 32 ++++++++------
> hw/i386/pc.c | 4 +-
> hw/i386/pc_q35.c | 8 ++++
> hw/pci/pcie.c | 11 ++++-
> tests/data/acpi/q35/DSDT | Bin 7859 -> 8289 bytes
> tests/data/acpi/q35/DSDT.acpihmat | Bin 9184 -> 9614 bytes
> tests/data/acpi/q35/DSDT.bridge | Bin 7877 -> 11003 bytes
> tests/data/acpi/q35/DSDT.cphp | Bin 8323 -> 8753 bytes
> tests/data/acpi/q35/DSDT.dimmpxm | Bin 9513 -> 9943 bytes
> tests/data/acpi/q35/DSDT.ipmibt | Bin 7934 -> 8364 bytes
> tests/data/acpi/q35/DSDT.memhp | Bin 9218 -> 9648 bytes
> tests/data/acpi/q35/DSDT.mmio64 | Bin 8990 -> 9419 bytes
> tests/data/acpi/q35/DSDT.nohpet | Bin 7717 -> 8147 bytes
> tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 8295 bytes
> tests/data/acpi/q35/DSDT.tis | Bin 8465 -> 8894 bytes
> 23 files changed, 161 insertions(+), 21 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (7 preceding siblings ...)
2021-05-20 14:37 ` [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Igor Mammedov
@ 2021-05-23 8:25 ` Michael S. Tsirkin
2021-06-16 17:26 ` Julia Suvorova
2021-05-26 5:39 ` Michael S. Tsirkin
9 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-05-23 8:25 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Igor Mammedov, qemu-devel, Eduardo Habkost
On Thu, May 13, 2021 at 08:26:35AM +0200, Julia Suvorova wrote:
> The patch set consists of two parts:
> patches 1-4: introduce new feature
> 'acpi-pci-hotplug-with-bridge-support' on Q35
> patches 5-7: make the feature default along with changes in ACPI tables
>
> This way maintainers can decide which way to choose without breaking
> the patch set.
>
> With the feature disabled Q35 falls back to the native hot-plug.
Overall I think this version has addressed my concerns, good job!
I see Igor has some suggestions on how to structure the code,
they seem easy to address.
Question: what makes this an RFC? Any known bugs/missing functionality?
I don't see anything obvious - what did I miss?
> Pros
> * no racy behavior during boot (see 110c477c2ed)
> * eject is possible - according to PCIe spec, attention button
> press should lead to power off, and then the adapter should be
> removed manually. As there is no power down state exists in QEMU,
> we cannot distinguish between an eject and a power down
> request.
> * no delay during deleting - after the actual power off software
> must wait at least 1 second before indicating about it. This case
> is quite important for users, it even has its own bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> * no timer-based behavior - in addition to the previous example,
> the attention button has a 5-second waiting period, during which
> the operation can be canceled with a second press. While this
> looks fine for manual button control, automation will result in
> the need to queue or drop events, and the software receiving
> events in all sort of unspecified combinations of attention/power
> indicator states, which is racy and uppredictable.
> * fixes or reduces the likelihood of the bugs:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1833187
> * https://bugzilla.redhat.com/show_bug.cgi?id=1657077
> * https://bugzilla.redhat.com/show_bug.cgi?id=1669931
> * https://bugzilla.redhat.com/show_bug.cgi?id=1678290
>
> Cons:
> * no access to possible features presented in slot capabilities
> (this is only surprise removal AFAIK)
>
> v4:
> * regain per-port control over hot-plug
> * rebased over acpi-index changes
> * set property on machine type to
> make pci code more generic [Igor, Michael]
>
> v3:
> * drop change of _OSC to allow SHPC on hotplugged bridges
> * use 'acpi-root-pci-hotplug'
> * add migration states [Igor]
> * minor style changes
>
> v2:
> * new ioport range for acpiphp [Gerd]
> * drop find_pci_host() [Igor]
> * explain magic numbers in _OSC [Igor]
> * drop build_q35_pci_hotplug() wrapper [Igor]
>
> Julia Suvorova (7):
> hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35
> hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
> hw/acpi/ich9: Enable ACPI PCI hot-plug
> hw/pci/pcie: Do not set HPC flag if acpihp is used
> bios-tables-test: Allow changes in DSDT ACPI tables
> hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
> bios-tables-test: Update golden binaries
>
> hw/i386/acpi-build.h | 5 +++
> include/hw/acpi/ich9.h | 5 +++
> include/hw/acpi/pcihp.h | 3 +-
> include/hw/boards.h | 1 +
> hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++
> hw/acpi/pcihp.c | 22 +++++++---
> hw/acpi/piix4.c | 4 +-
> hw/core/machine.c | 19 +++++++++
> hw/i386/acpi-build.c | 32 ++++++++------
> hw/i386/pc.c | 4 +-
> hw/i386/pc_q35.c | 8 ++++
> hw/pci/pcie.c | 11 ++++-
> tests/data/acpi/q35/DSDT | Bin 7859 -> 8289 bytes
> tests/data/acpi/q35/DSDT.acpihmat | Bin 9184 -> 9614 bytes
> tests/data/acpi/q35/DSDT.bridge | Bin 7877 -> 11003 bytes
> tests/data/acpi/q35/DSDT.cphp | Bin 8323 -> 8753 bytes
> tests/data/acpi/q35/DSDT.dimmpxm | Bin 9513 -> 9943 bytes
> tests/data/acpi/q35/DSDT.ipmibt | Bin 7934 -> 8364 bytes
> tests/data/acpi/q35/DSDT.memhp | Bin 9218 -> 9648 bytes
> tests/data/acpi/q35/DSDT.mmio64 | Bin 8990 -> 9419 bytes
> tests/data/acpi/q35/DSDT.nohpet | Bin 7717 -> 8147 bytes
> tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 8295 bytes
> tests/data/acpi/q35/DSDT.tis | Bin 8465 -> 8894 bytes
> 23 files changed, 161 insertions(+), 21 deletions(-)
>
> --
> 2.30.2
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35
2021-05-23 8:25 ` Michael S. Tsirkin
@ 2021-06-16 17:26 ` Julia Suvorova
0 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-06-16 17:26 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Igor Mammedov, QEMU Developers, Eduardo Habkost
On Sun, May 23, 2021 at 10:26 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, May 13, 2021 at 08:26:35AM +0200, Julia Suvorova wrote:
> > The patch set consists of two parts:
> > patches 1-4: introduce new feature
> > 'acpi-pci-hotplug-with-bridge-support' on Q35
> > patches 5-7: make the feature default along with changes in ACPI tables
> >
> > This way maintainers can decide which way to choose without breaking
> > the patch set.
> >
> > With the feature disabled Q35 falls back to the native hot-plug.
>
> Overall I think this version has addressed my concerns, good job!
> I see Igor has some suggestions on how to structure the code,
> they seem easy to address.
> Question: what makes this an RFC? Any known bugs/missing functionality?
> I don't see anything obvious - what did I miss?
No known issues, just indecisiveness about the default. The next
version will be non-RFC.
Best regards, Julia Suvorova.
> > Pros
> > * no racy behavior during boot (see 110c477c2ed)
> > * eject is possible - according to PCIe spec, attention button
> > press should lead to power off, and then the adapter should be
> > removed manually. As there is no power down state exists in QEMU,
> > we cannot distinguish between an eject and a power down
> > request.
> > * no delay during deleting - after the actual power off software
> > must wait at least 1 second before indicating about it. This case
> > is quite important for users, it even has its own bug:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> > * no timer-based behavior - in addition to the previous example,
> > the attention button has a 5-second waiting period, during which
> > the operation can be canceled with a second press. While this
> > looks fine for manual button control, automation will result in
> > the need to queue or drop events, and the software receiving
> > events in all sort of unspecified combinations of attention/power
> > indicator states, which is racy and uppredictable.
> > * fixes or reduces the likelihood of the bugs:
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1833187
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1657077
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1669931
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1678290
> >
> > Cons:
> > * no access to possible features presented in slot capabilities
> > (this is only surprise removal AFAIK)
> >
> > v4:
> > * regain per-port control over hot-plug
> > * rebased over acpi-index changes
> > * set property on machine type to
> > make pci code more generic [Igor, Michael]
> >
> > v3:
> > * drop change of _OSC to allow SHPC on hotplugged bridges
> > * use 'acpi-root-pci-hotplug'
> > * add migration states [Igor]
> > * minor style changes
> >
> > v2:
> > * new ioport range for acpiphp [Gerd]
> > * drop find_pci_host() [Igor]
> > * explain magic numbers in _OSC [Igor]
> > * drop build_q35_pci_hotplug() wrapper [Igor]
> >
> > Julia Suvorova (7):
> > hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35
> > hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
> > hw/acpi/ich9: Enable ACPI PCI hot-plug
> > hw/pci/pcie: Do not set HPC flag if acpihp is used
> > bios-tables-test: Allow changes in DSDT ACPI tables
> > hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
> > bios-tables-test: Update golden binaries
> >
> > hw/i386/acpi-build.h | 5 +++
> > include/hw/acpi/ich9.h | 5 +++
> > include/hw/acpi/pcihp.h | 3 +-
> > include/hw/boards.h | 1 +
> > hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++
> > hw/acpi/pcihp.c | 22 +++++++---
> > hw/acpi/piix4.c | 4 +-
> > hw/core/machine.c | 19 +++++++++
> > hw/i386/acpi-build.c | 32 ++++++++------
> > hw/i386/pc.c | 4 +-
> > hw/i386/pc_q35.c | 8 ++++
> > hw/pci/pcie.c | 11 ++++-
> > tests/data/acpi/q35/DSDT | Bin 7859 -> 8289 bytes
> > tests/data/acpi/q35/DSDT.acpihmat | Bin 9184 -> 9614 bytes
> > tests/data/acpi/q35/DSDT.bridge | Bin 7877 -> 11003 bytes
> > tests/data/acpi/q35/DSDT.cphp | Bin 8323 -> 8753 bytes
> > tests/data/acpi/q35/DSDT.dimmpxm | Bin 9513 -> 9943 bytes
> > tests/data/acpi/q35/DSDT.ipmibt | Bin 7934 -> 8364 bytes
> > tests/data/acpi/q35/DSDT.memhp | Bin 9218 -> 9648 bytes
> > tests/data/acpi/q35/DSDT.mmio64 | Bin 8990 -> 9419 bytes
> > tests/data/acpi/q35/DSDT.nohpet | Bin 7717 -> 8147 bytes
> > tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 8295 bytes
> > tests/data/acpi/q35/DSDT.tis | Bin 8465 -> 8894 bytes
> > 23 files changed, 161 insertions(+), 21 deletions(-)
> >
> > --
> > 2.30.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35
2021-05-13 6:26 [RFC PATCH v4 0/7] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (8 preceding siblings ...)
2021-05-23 8:25 ` Michael S. Tsirkin
@ 2021-05-26 5:39 ` Michael S. Tsirkin
9 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-05-26 5:39 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Igor Mammedov, qemu-devel, Eduardo Habkost
On Thu, May 13, 2021 at 08:26:35AM +0200, Julia Suvorova wrote:
> The patch set consists of two parts:
> patches 1-4: introduce new feature
> 'acpi-pci-hotplug-with-bridge-support' on Q35
> patches 5-7: make the feature default along with changes in ACPI tables
>
> This way maintainers can decide which way to choose without breaking
> the patch set.
This is in good shape, I think the next version can be a non-RFC series.
> With the feature disabled Q35 falls back to the native hot-plug.
>
> Pros
> * no racy behavior during boot (see 110c477c2ed)
> * eject is possible - according to PCIe spec, attention button
> press should lead to power off, and then the adapter should be
> removed manually. As there is no power down state exists in QEMU,
> we cannot distinguish between an eject and a power down
> request.
> * no delay during deleting - after the actual power off software
> must wait at least 1 second before indicating about it. This case
> is quite important for users, it even has its own bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> * no timer-based behavior - in addition to the previous example,
> the attention button has a 5-second waiting period, during which
> the operation can be canceled with a second press. While this
> looks fine for manual button control, automation will result in
> the need to queue or drop events, and the software receiving
> events in all sort of unspecified combinations of attention/power
> indicator states, which is racy and uppredictable.
> * fixes or reduces the likelihood of the bugs:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1833187
> * https://bugzilla.redhat.com/show_bug.cgi?id=1657077
> * https://bugzilla.redhat.com/show_bug.cgi?id=1669931
> * https://bugzilla.redhat.com/show_bug.cgi?id=1678290
>
> Cons:
> * no access to possible features presented in slot capabilities
> (this is only surprise removal AFAIK)
>
> v4:
> * regain per-port control over hot-plug
> * rebased over acpi-index changes
> * set property on machine type to
> make pci code more generic [Igor, Michael]
>
> v3:
> * drop change of _OSC to allow SHPC on hotplugged bridges
> * use 'acpi-root-pci-hotplug'
> * add migration states [Igor]
> * minor style changes
>
> v2:
> * new ioport range for acpiphp [Gerd]
> * drop find_pci_host() [Igor]
> * explain magic numbers in _OSC [Igor]
> * drop build_q35_pci_hotplug() wrapper [Igor]
>
> Julia Suvorova (7):
> hw/acpi/pcihp: Enhance acpi_pcihp_disable_root_bus() to support Q35
> hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
> hw/acpi/ich9: Enable ACPI PCI hot-plug
> hw/pci/pcie: Do not set HPC flag if acpihp is used
> bios-tables-test: Allow changes in DSDT ACPI tables
> hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
> bios-tables-test: Update golden binaries
>
> hw/i386/acpi-build.h | 5 +++
> include/hw/acpi/ich9.h | 5 +++
> include/hw/acpi/pcihp.h | 3 +-
> include/hw/boards.h | 1 +
> hw/acpi/ich9.c | 68 ++++++++++++++++++++++++++++++
> hw/acpi/pcihp.c | 22 +++++++---
> hw/acpi/piix4.c | 4 +-
> hw/core/machine.c | 19 +++++++++
> hw/i386/acpi-build.c | 32 ++++++++------
> hw/i386/pc.c | 4 +-
> hw/i386/pc_q35.c | 8 ++++
> hw/pci/pcie.c | 11 ++++-
> tests/data/acpi/q35/DSDT | Bin 7859 -> 8289 bytes
> tests/data/acpi/q35/DSDT.acpihmat | Bin 9184 -> 9614 bytes
> tests/data/acpi/q35/DSDT.bridge | Bin 7877 -> 11003 bytes
> tests/data/acpi/q35/DSDT.cphp | Bin 8323 -> 8753 bytes
> tests/data/acpi/q35/DSDT.dimmpxm | Bin 9513 -> 9943 bytes
> tests/data/acpi/q35/DSDT.ipmibt | Bin 7934 -> 8364 bytes
> tests/data/acpi/q35/DSDT.memhp | Bin 9218 -> 9648 bytes
> tests/data/acpi/q35/DSDT.mmio64 | Bin 8990 -> 9419 bytes
> tests/data/acpi/q35/DSDT.nohpet | Bin 7717 -> 8147 bytes
> tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 8295 bytes
> tests/data/acpi/q35/DSDT.tis | Bin 8465 -> 8894 bytes
> 23 files changed, 161 insertions(+), 21 deletions(-)
>
> --
> 2.30.2
^ permalink raw reply [flat|nested] 16+ messages in thread