* [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues
@ 2021-11-10 5:30 Julia Suvorova
2021-11-10 5:30 ` [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit' Julia Suvorova
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
Attempt [1] to fix I/O allocation with the 'reserve-io' hint on each
pcie-root-port resulted in regression [2-3]. This patchset aims to fix
it by addressing the root cause of the problem - the disabled PCIe
Slot HPC bit.
[1] 'hw/pcie-root-port: Fix hotplug for PCI devices requiring IO'
[2] https://gitlab.com/qemu-project/qemu/-/issues/641
[3] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Julia Suvorova (5):
hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
hw/acpi/ich9: Add compatibility option for 'native-hpc-bit'
bios-tables-test: Allow changes in DSDT ACPI tables
hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
bios-tables-test: Update golden binaries
include/hw/acpi/ich9.h | 1 +
include/hw/pci/pcie_port.h | 2 +-
hw/acpi/ich9.c | 18 ++++++++++++++++++
hw/i386/acpi-build.c | 12 ++++++++----
hw/i386/pc.c | 2 ++
hw/i386/pc_q35.c | 9 +++++++--
hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
hw/pci/pcie.c | 2 +-
hw/pci/pcie_port.c | 2 +-
tests/data/acpi/q35/DSDT | Bin 8289 -> 8289 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9614 -> 9614 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 11003 -> 11003 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8753 -> 8753 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9943 -> 9943 bytes
tests/data/acpi/q35/DSDT.dmar | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 8364 -> 8364 bytes
tests/data/acpi/q35/DSDT.ivrs | Bin 8306 -> 8306 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9648 -> 9648 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 9419 -> 9419 bytes
tests/data/acpi/q35/DSDT.multi-bridge | Bin 8583 -> 8583 bytes
tests/data/acpi/q35/DSDT.nohpet | Bin 8147 -> 8147 bytes
tests/data/acpi/q35/DSDT.nosmm | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 8295 -> 8295 bytes
tests/data/acpi/q35/DSDT.smm-compat | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.smm-compat-nosmm | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.tis.tpm12 | Bin 8894 -> 8894 bytes
tests/data/acpi/q35/DSDT.tis.tpm2 | Bin 8894 -> 8894 bytes
tests/data/acpi/q35/DSDT.xapic | Bin 35652 -> 35652 bytes
28 files changed, 44 insertions(+), 10 deletions(-)
create mode 100644 tests/data/acpi/q35/DSDT.dmar
create mode 100644 tests/data/acpi/q35/DSDT.nosmm
create mode 100644 tests/data/acpi/q35/DSDT.smm-compat
create mode 100644 tests/data/acpi/q35/DSDT.smm-compat-nosmm
--
2.31.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
@ 2021-11-10 5:30 ` Julia Suvorova
2021-11-10 6:04 ` Michael S. Tsirkin
2021-11-10 5:30 ` [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit' Julia Suvorova
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
Rename the option to better represent its function - toggle Hot-Plug
Capable bit in the PCIe Slot Capability.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
include/hw/pci/pcie_port.h | 2 +-
hw/i386/pc_q35.c | 2 +-
hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
hw/pci/pcie.c | 2 +-
hw/pci/pcie_port.c | 2 +-
5 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index e25b289ce8..0da6d66c95 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -60,7 +60,7 @@ struct PCIESlot {
/* Indicates whether any type of hot-plug is allowed on the slot */
bool hotplug;
- bool native_hotplug;
+ bool native_hpc_bit;
QLIST_ENTRY(PCIESlot) next;
};
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 797e09500b..ded61f8ef7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
NULL);
if (acpi_pcihp) {
- object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
+ object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
"false", true);
}
diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index 20099a8ae3..ed079d72b3 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -87,7 +87,11 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
return;
}
- if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
+ /*
+ * Make sure that IO is assigned in case the slot is hot-pluggable
+ * but it is not visible through the PCIe Slot Capabilities
+ */
+ if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hpc_bit) {
grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
}
int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 914a9bf3d1..ebe002831e 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -535,7 +535,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
* hot-plug is disabled on the slot.
*/
if (s->hotplug &&
- (s->native_hotplug || DEVICE(dev)->hotplugged)) {
+ (s->native_hpc_bit || DEVICE(dev)->hotplugged)) {
pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
PCI_EXP_SLTCAP_HPS |
PCI_EXP_SLTCAP_HPC);
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index da850e8dde..eae06d10e2 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
- DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
+ DEFINE_PROP_BOOL("native-hpc-bit", PCIESlot, native_hpc_bit, true),
DEFINE_PROP_END_OF_LIST()
};
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit'
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
2021-11-10 5:30 ` [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit' Julia Suvorova
@ 2021-11-10 5:30 ` Julia Suvorova
2021-11-10 13:33 ` Igor Mammedov
2021-11-10 5:30 ` [PATCH 3/5] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be
turned on, while the switch to ACPI Hot-plug will be done in the
DSDT table.
Introducing 'x-keep-native-hpc' option disables the HPC bit only
in 6.1 and as a result keeps the forced 'reserve-io' on
pcie-root-ports in 6.1 too.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
include/hw/acpi/ich9.h | 1 +
hw/acpi/ich9.c | 18 ++++++++++++++++++
hw/i386/pc.c | 2 ++
hw/i386/pc_q35.c | 7 ++++++-
4 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index f04f1791bd..7ca92843c6 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -56,6 +56,7 @@ typedef struct ICH9LPCPMRegs {
AcpiCpuHotplug gpe_cpu;
CPUHotplugState cpuhp_state;
+ bool keep_pci_slot_hpc;
bool use_acpi_hotplug_bridge;
AcpiPciHpState acpi_pci_hotplug;
MemHotplugState acpi_memory_hotplug;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 1ee2ba2c50..ebe08ed831 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -419,6 +419,20 @@ static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp)
s->pm.use_acpi_hotplug_bridge = value;
}
+static bool ich9_pm_get_keep_pci_slot_hpc(Object *obj, Error **errp)
+{
+ ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
+
+ return s->pm.keep_pci_slot_hpc;
+}
+
+static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp)
+{
+ ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
+
+ s->pm.keep_pci_slot_hpc = value;
+}
+
void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
{
static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
@@ -428,6 +442,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
pm->disable_s4 = 0;
pm->s4_val = 2;
pm->use_acpi_hotplug_bridge = true;
+ pm->keep_pci_slot_hpc = true;
object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
&pm->pm_io_base, OBJ_PROP_FLAG_READ);
@@ -454,6 +469,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
object_property_add_bool(obj, ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
ich9_pm_get_acpi_pci_hotplug,
ich9_pm_set_acpi_pci_hotplug);
+ object_property_add_bool(obj, "x-keep-pci-slot-hpc",
+ ich9_pm_get_keep_pci_slot_hpc,
+ ich9_pm_set_keep_pci_slot_hpc);
}
void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2592a82148..a2ef40ecbc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_1[] = {
{ TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
{ TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
{ TYPE_X86_CPU, "hv-version-id-minor", "0x0001" },
+ { "ICH9-LPC", "x-keep-pci-slot-hpc", "false" },
};
const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
@@ -107,6 +108,7 @@ GlobalProperty pc_compat_6_0[] = {
{ "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
{ TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
{ "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
+ { "ICH9-LPC", "x-keep-pci-slot-hpc", "true" },
};
const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ded61f8ef7..06f09dfcc6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -137,6 +137,7 @@ static void pc_q35_init(MachineState *machine)
DriveInfo *hd[MAX_SATA_PORTS];
MachineClass *mc = MACHINE_GET_CLASS(machine);
bool acpi_pcihp;
+ bool keep_pci_slot_hpc;
/* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
* and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -242,7 +243,11 @@ static void pc_q35_init(MachineState *machine)
ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
NULL);
- if (acpi_pcihp) {
+ keep_pci_slot_hpc = object_property_get_bool(OBJECT(lpc),
+ "x-keep-pci-slot-hpc",
+ NULL);
+
+ if (!keep_pci_slot_hpc && acpi_pcihp) {
object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
"false", true);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/5] bios-tables-test: Allow changes in DSDT ACPI tables
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
2021-11-10 5:30 ` [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit' Julia Suvorova
2021-11-10 5:30 ` [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit' Julia Suvorova
@ 2021-11-10 5:30 ` Julia Suvorova
2021-11-10 5:30 ` [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC Julia Suvorova
2021-11-10 5:30 ` [PATCH 5/5] bios-tables-test: Update golden binaries Julia Suvorova
4 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
Prepare for changing the _OSC method in q35 DSDT.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..48e5634d4b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,17 @@
/* 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",
+"tests/data/acpi/q35/DSDT.tis.tpm2",
+"tests/data/acpi/q35/DSDT.tis.tpm12",
+"tests/data/acpi/q35/DSDT.multi-bridge",
+"tests/data/acpi/q35/DSDT.ivrs",
+"tests/data/acpi/q35/DSDT.xapic",
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
` (2 preceding siblings ...)
2021-11-10 5:30 ` [PATCH 3/5] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
@ 2021-11-10 5:30 ` Julia Suvorova
2021-11-10 7:21 ` Michael S. Tsirkin
2021-11-10 5:30 ` [PATCH 5/5] bios-tables-test: Update golden binaries Julia Suvorova
4 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
There are two ways to enable ACPI PCI Hot-plug:
* Disable the Hot-plug Capable bit on PCIe slots.
This was the first approach which led to regression [1-2], as
I/O space for a port is allocated only when it is hot-pluggable,
which is determined by HPC bit.
* Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
method.
This removes the (future) ability of hot-plugging switches with PCIe
Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
bridges. If the user wants to explicitely use this feature, they can
disable ACPI PCI Hot-plug with:
--global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
instead of PCIe Native.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
hw/i386/acpi-build.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a3ad6abd33..a2f92ab32b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1337,7 +1337,7 @@ static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
aml_append(table, scope);
}
-static Aml *build_q35_osc_method(void)
+static Aml *build_q35_osc_method(bool acpi_pcihp)
{
Aml *if_ctx;
Aml *if_ctx2;
@@ -1345,6 +1345,7 @@ static Aml *build_q35_osc_method(void)
Aml *method;
Aml *a_cwd1 = aml_name("CDW1");
Aml *a_ctrl = aml_local(0);
+ const uint64_t hotplug = acpi_pcihp ? 0x1E : 0x1F;
method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
@@ -1359,8 +1360,9 @@ static Aml *build_q35_osc_method(void)
/*
* Always allow native PME, AER (no dependencies)
* Allow SHPC (PCI bridges can have SHPC controller)
+ * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
*/
- aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
+ aml_append(if_ctx, aml_and(a_ctrl, aml_int(hotplug), a_ctrl));
if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
/* Unknown revision */
@@ -1449,7 +1451,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
- aml_append(dev, build_q35_osc_method());
+ aml_append(dev, build_q35_osc_method(pm->pcihp_bridge_en));
aml_append(sb_scope, dev);
if (mcfg_valid) {
aml_append(sb_scope, build_q35_dram_controller(&mcfg));
@@ -1565,7 +1567,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
if (pci_bus_is_express(bus)) {
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
- aml_append(dev, build_q35_osc_method());
+
+ /* Expander bridges do not have ACPI PCI Hot-plug enabled */
+ aml_append(dev, build_q35_osc_method(false));
} else {
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
}
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] bios-tables-test: Update golden binaries
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
` (3 preceding siblings ...)
2021-11-10 5:30 ` [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC Julia Suvorova
@ 2021-11-10 5:30 ` Julia Suvorova
4 siblings, 0 replies; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 5:30 UTC (permalink / raw)
To: qemu-devel
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova,
Gerd Hoffmann, Ani Sinha, Igor Mammedov
The changes are the result of
'hw/i386/acpi-build: Deny control on PCIe Native Hot-Plug in _OSC'
and listed here:
Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities
{
CreateDWordField (Arg3, Zero, CDW1)
If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
{
CreateDWordField (Arg3, 0x04, CDW2)
CreateDWordField (Arg3, 0x08, CDW3)
Local0 = CDW3 /* \_SB_.PCI0._OSC.CDW3 */
- Local0 &= 0x1F
+ Local0 &= 0x1E
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 16 ----------------
tests/data/acpi/q35/DSDT | Bin 8289 -> 8289 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9614 -> 9614 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 11003 -> 11003 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8753 -> 8753 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9943 -> 9943 bytes
tests/data/acpi/q35/DSDT.dmar | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 8364 -> 8364 bytes
tests/data/acpi/q35/DSDT.ivrs | Bin 8306 -> 8306 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9648 -> 9648 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 9419 -> 9419 bytes
tests/data/acpi/q35/DSDT.multi-bridge | Bin 8583 -> 8583 bytes
tests/data/acpi/q35/DSDT.nohpet | Bin 8147 -> 8147 bytes
tests/data/acpi/q35/DSDT.nosmm | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 8295 -> 8295 bytes
tests/data/acpi/q35/DSDT.smm-compat | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.smm-compat-nosmm | Bin 0 -> 8289 bytes
tests/data/acpi/q35/DSDT.tis.tpm12 | Bin 8894 -> 8894 bytes
tests/data/acpi/q35/DSDT.tis.tpm2 | Bin 8894 -> 8894 bytes
tests/data/acpi/q35/DSDT.xapic | Bin 35652 -> 35652 bytes
20 files changed, 16 deletions(-)
create mode 100644 tests/data/acpi/q35/DSDT.dmar
create mode 100644 tests/data/acpi/q35/DSDT.nosmm
create mode 100644 tests/data/acpi/q35/DSDT.smm-compat
create mode 100644 tests/data/acpi/q35/DSDT.smm-compat-nosmm
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 48e5634d4b..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,17 +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",
-"tests/data/acpi/q35/DSDT.tis.tpm2",
-"tests/data/acpi/q35/DSDT.tis.tpm12",
-"tests/data/acpi/q35/DSDT.multi-bridge",
-"tests/data/acpi/q35/DSDT.ivrs",
-"tests/data/acpi/q35/DSDT.xapic",
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index 281fc82c03b2562d2e6b7caec0d817b034a47138..c1965f6051ef2af81dd8412abe169d87845bb033 100644
GIT binary patch
delta 24
gcmaFp@X&$FCD<h-QGtPh@z+GID~xg*?>ET<0BnZ{w*UYD
delta 24
gcmaFp@X&$FCD<h-QGtPh@#jRYD~$3R?>ET<0BnK?w*UYD
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 8c1e05a11a328ec1cc6f86e36e52c28f41f9744e..f24d4874bff8d327a165ed7c36de507aea114edd 100644
GIT binary patch
delta 24
fcmeD4?(^ny33dtTQ)OUa+&+=(3ZvY{`|DKzU@Hhn
delta 24
fcmeD4?(^ny33dtTQ)OUa+%}Qx3ZwkS`|DKzU?vDi
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 6f1464b6c712d7f33cb4b891b7ce76fe228f44c9..424d51bd1cb39ea73501ef7d0044ee52cec5bdac 100644
GIT binary patch
delta 24
gcmewz`a6`%CD<k8w-y5fBg;gtD~xg*@5^Wb0CWThXaE2J
delta 24
gcmewz`a6`%CD<k8w-y5fBlASAD~$3R@5^Wb0CWEcXaE2J
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index f8337ff5191a37a47dcf7c09a6c39c4e704a15bf..f1275606f68eeba54bfb11e63d818420385a62b9 100644
GIT binary patch
delta 24
fcmdn!veAXhCD<jzP>F$oF>WH)6-K#@_k$DxTWtqt
delta 24
fcmdn!veAXhCD<jzP>F$oF?J%?6-N1u_k$DxTWAMo
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index fe5820d93d057ef09a001662369b15afbc5b87e2..76e451e829ec4c245315f7eed8731aa1be45a747 100644
GIT binary patch
delta 24
gcmccad)=4ICD<k8x*7umqsK(9D~xg*@BdH*0B$o00{{R3
delta 24
gcmccad)=4ICD<k8x*7umqx(dzD~$3R@BdH*0B$Y`0{{R3
diff --git a/tests/data/acpi/q35/DSDT.dmar b/tests/data/acpi/q35/DSDT.dmar
new file mode 100644
index 0000000000000000000000000000000000000000..c1965f6051ef2af81dd8412abe169d87845bb033
GIT binary patch
literal 8289
zcmb7JOKcm*8J^`!tL0K!Qk3OaY{E(UN|8{0Bx#EVC3pF<L~6yQ;si9nrRB7;og#}k
z2J%1vSpgEqhoT9aphr4TfZp0uuQkw1d-JUUdg`H<T#F(`4`IK5h9l3A6cF>U+JE-@
z{(rvNo&8obUf^~51;&j3l{egaP$<9Ry9N9V#u%N`U#F3{%(}Z?xz;n%v4qjRo#y8_
zl+FB)z4BJg`29}!c^JO+QE2odcI6A_vn&1RgFE3BMxa|)BFmx^r?Sus%DwKMYx!=Y
zX!hz2&n`W%Ota^Tv$)wSd2MF)wi+JGug~>10ylB@26MejtHV}uu#;V~oNn`<=e{|8
z@)w_9daHE*=l}TS-5XW{fV22(;_tbL4&i&Dbt)V>pAN1W?-QM0TOBTaI^dT@n?qNk
zkXr89qKQ(i?%(L{z38<|F7IQ;Z}~;HxQc$c?I3&FI{DEGV>S&A{r)r>js7A|nez^F
znx;@$G3$P%BUKvEMyk5Ib-&YOHe(OIO|wMzwnOpK%axW_@Y`Omkbk}xbV@9umN25G
z{=1>&ghTt2LC6MSdNkT+!-Y=<efGP)&HnY@<UZTC7DeB(*20JL)Rza}vIJx3&G(+A
z-q3h=_l`G$QjZTY%AZD8{6>2-W`&mRtSvqlmq0>w_s*}CJ%GhMilg`Fl`M1ATCN!(
z3z=cmjH~P_{+@}UT6gcM=El|mYjH_qnO5=)jZk;*?7H7smAMn*ej6Iru`)H|?fBbq
z(>$}wGoN)gng^UD&=}1boQLs;ab})zIOpkh$35sxdVK$Q>WD?mFwZapL)6MX8lzp`
z4LU~pzHOPfH59lQyR8BaUZJ}iR4!r`{LQv-<I)tk<znR`E|go%wvm1yXcV~R)>gCN
zH@sI0U8B72uCe^nzFWB%>pn@*XcX-bB6`QT#wbVk`_#P8^3RmOnT4WU5go!YHq3h|
z-wT=x!}amhc<{pC{~cU-xB1D+*Ue*pb(<08di~eo@X5h-Has@CX2mcA-sjJejmLJx
z_Oo&0HXYc#mErQ>x-}Q&k};NH{V2TCXKcT3`VF_r@^%k@jcA7==Qpn!^sqSY*L))T
z?T(*YySbZ7C-8K#_10R2iE{(zRhF}<-HIWo$smrP&T^Rq`hl4waN~$r04|7YoCl~3
ziHZCi8{<r1#}P3lG{!g+P|ie1O!S6h<D4mb5NE`c(70d%%9#j>DXU;!2`tZ(;7kck
z2&U{jLRDu{)0yN<2~7&7>^wqMXG+tV;!Fum={garI_ET<bDGXMT_-|Sr>W^QHJzre
z6QQaTZy;r)=QW-4x=w_uPD|5iX*w-kCqh+cTGN@<bf$Hk2vwbU7b^GD)^yssPK2t?
zjHWZA>CEUl5vn>DG@T2Y&IMg3LRIIYrgKr#xv1+zsOoeyosOo{(RCtJb!Ii4SxskF
z*NITonbUOUG@UtJCqh-{lBRP>)48PUM5yY_YdZ6q&b+P@p{jFP)48naT-J3WRCONF
zbRN-k9?^9oRCVG@0#BcO53OiAS9F~SRh>sQokumDM|GVDRh`E;vowAa9^=gN_+@uY
zFx^KXF}*SNxW+uLF^}s^gevod#yp`hPv}g9D)XeqJgG5H>P&<x^9halgvNYAXChRY
zr#Mr(%qh;4@Ay-KDPP<Oi7D^kX|3jIt>$UHCPGougn^b61FZ!Ol;&n?;2ed=28sY>
zMG!|Ii^4z!j$4w!h$Z`Hps0Wz7$`z14F)Q((m)j`8K{6B7$`#YMjEKVjw7n68mNFC
z7$`#Yh+|=(0y~bz8ab2<R6u2@>V$!cw44b8RiI>`2&F5Mfg+STVW0xbnJ`cVN(L&R
zye1haLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9!+N1}d<e2?JH2
zWS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;B
zKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!
z%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-
z7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZul
zLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPO
zNd}5g>V$y`EN8+%6(|{~fN~}oC_<?d28u{AP(-SMB2*0&p<<v469%d<$v_n*8K}a9
zfhtTGsKO)zRhVR;3KIsZFkzqylMGa0l7T8r7^uR8fhtTgP=!ebsxV=oh~(Xcfg-{k
z<AX^CiV(L!hQyR(3j;+Y#})>PNRBNTC_*{5WS|JKRCK%t3uQxl!2em@r+-NArRYzo
zd;6!)#p%CPTGgQS4#RJ6mf*OXWjeTY@Jxoam(2>DSLslrL*2|TH8!ytFr0tS+TFn-
z%ly8T&Yy2=y6t%Y+QsaGmHs%z=J`s{JM1pCSxFol(R(}ABBL{OqK&1O^*MYal;o0!
zjpZ6z3^LK}AbZ4G`gnoO$Kh@a?{9`TF;*|~i+mkupSpnFl=9i0*9h4AbZ+hPmPJ~R
zVnDcU9<TI*c3|Ay8TWz8Q~S7jd7?TNPrh^YvQoXwtC!I~wR$-}yL!31*Cg#?DGkeg
zW9?S#G0MA2d6$=WC(65rly}GFy$SIa7PP*zdQU0u@$%k8dGC<&-ne{avV7t(%2$-~
z6<)qFQND6W`O3I_b+UZ&G0In!@>O2GI#IrQNcrlxd~LFP>M_dKl=3xRzBW<5c1Zc!
zxO{!G{M=)duPf#2ynKD4eEpE}b(Bxgt8B8oBg)H{2R<t>-NpK?-j+}CDUUxiugB>o
zr?HjQ)8SiVDxGMXnQm%c=Ja&<0GUcB+GeJknrB!7rg<iX;j3gSooJhxZff4-^>p}D
znMx<xW~Q5(2g`aoe7{Vk6KyloO+D8~^mO>BnMx<xW~Q5ZPFM7F_|ln5C))IM&WT6s
z*?WsYvy814Y^&IDtob_|FNxiWx^?-{iof5k1oA1hFgf8(8Q0$`)Zch3di~a~3U9u4
z^Nm|?ZohVubq&{FUmHKI8Oy2}U)f*ThK*HycoHq0e`Q;C_x2AcD6xX;S6YVKXa$Db
zZoX__0lY|us=?x}+w`!=o+D)34Jzvez`atZfVJjEt6@fr8u8ShyUg{VL!y(JlIu5$
z#nmEpLu?!+^I5l3eKj_;20QFSw^%G*%hyv_y)VsopS=8BLT+u4P!d4P?~~0a7O`-6
zd@y-|Bt+ICmffeWtgr2d`DT)2JzgPWG8Hn+%%86waRawy9wNhZTa`<>*eGJ#k#Y44
zFE_YqMC@v0gc0k1v2Oo%5E`FtqbZEGCrfg{la073!<Ek_TX9>qzZ}jF`ql_#zKmvF
zA3ilTa&+IL?K{-RKD*UNukO7tyf6jhPX=z^wiS=21_l?wyR}Uli<N}fS)_zl*f!mA
zXxr|t&K5V73;nruW%e*P=iQB<V;?fiv!9l!{209G+KjP@tk`5LL9b6XVwh+4uKU4@
z^h{&^<A>>i=i1z-ryDb%S?XldmdUW%K;NPH1#Fsj(j<oMT)Un>fyq<zE4@nbEE~^w
znn?6aIU5PO#3%lEP8FYL`JVv0%4i@Cb_B(amD1q5E#6ZN;WU@UOy!$Ge6RFk+WPaI
z42xlj0KX9(uw(eperOve+6}yu@cRarAKC|T275@1{fBR|^6T_jJQ~qwG5ump+kQ2%
zjrB)yh5bRhav%G$--;{uU%LAYwrt1QL>rA3?SWz7v>(N^N$DmqIAWKY?*NxDIO5ev
z?aDh*Y+R;=gY$P{Ayr5je5dnb!ms;nF~qcu{3r<qX~z3?I@f*szHJ=EHsud5qkM3#
zmrErK^d0BtdpR><#Vjo1A-X_UNQv{aN4KH7_iXi&D||1rS!r(JF*-|VZ04#xc9Kq#
z-r`OxahY%JRTFd<yL;)NSQLB47?K|a#TP|@J7zGLaj@z!Y+B;vtUNi#Pg3-Vu>S)s
Cro3tZ
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index 631741065860fd5036aa303904dabd1d2839f9c6..6ad2411d0ec95f204cfc64b430c537bce09c35bd 100644
GIT binary patch
delta 24
gcmZ4ExW<voCD<iojRFG$quxZWD~xg*@1K<i0AMf(82|tP
delta 24
gcmZ4ExW<voCD<iojRFG$qwYkmD~$3R@1K<i0AMQ!82|tP
diff --git a/tests/data/acpi/q35/DSDT.ivrs b/tests/data/acpi/q35/DSDT.ivrs
index b0eafe90e5832935557ec5e6802c0147c88f379c..cad26e3f0c27a40a33101155a5282ed9bcb1d441 100644
GIT binary patch
delta 24
gcmez5@X3M8CD<jTNP&TYan?kxD~xg*?@yKo0BrUMn*aa+
delta 24
gcmez5@X3M8CD<jTNP&TYappv>D~$3R?@yKo0BrFHn*aa+
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 9bc11518fc57687ca789dc70793b48b29a0d74ed..4e9cb3dc6896bb79ccac0fe342a404549f6610e8 100644
GIT binary patch
delta 24
gcmdnsy}_HyCD<iogDL|9<C}?GR~Y3s-oK~<0BV~F1poj5
delta 24
gcmdnsy}_HyCD<iogDL|9<LilBR~Y3t-oK~<0BV*A1poj5
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 713288a12ef2b70a20b4e8836c036ba4db3a57a8..eb5a1c7171c02d153487bfcecfb7019b7c1bf406 100644
GIT binary patch
delta 24
gcmX@@dD@f9CD<k8v<d?Q<BEw~R~Y3s-hZPE0B$h}CjbBd
delta 24
gcmX@@dD@f9CD<k8v<d?Q<MN4IR~Y3t-hZPE0B$S^CjbBd
diff --git a/tests/data/acpi/q35/DSDT.multi-bridge b/tests/data/acpi/q35/DSDT.multi-bridge
index a24c713d22102a1a1583b5c902edffe1694e5cfe..45808eb03b78d07ebbe853f674abfed589d35e26 100644
GIT binary patch
delta 24
fcmZp7Zg=K#33dr-S7cyd?3>7Sg;8$f{S^uTTRaEr
delta 24
fcmZp7Zg=K#33dr-S7cyd?48JUg;9Rv{S^uTTQ>*m
diff --git a/tests/data/acpi/q35/DSDT.nohpet b/tests/data/acpi/q35/DSDT.nohpet
index e8202e6ddfbe96071f32f1ec05758f650569943e..83d1aa00ac5686df479673fb0d7830f946e25dea 100644
GIT binary patch
delta 24
gcmca?f7zbPCD<k8vOEI=<B^G6R~Y3s-v1&80B=4CMF0Q*
delta 24
gcmca?f7zbPCD<k8vOEI=<Kc;1R~Y3t-v1&80B<=7MF0Q*
diff --git a/tests/data/acpi/q35/DSDT.nosmm b/tests/data/acpi/q35/DSDT.nosmm
new file mode 100644
index 0000000000000000000000000000000000000000..c1965f6051ef2af81dd8412abe169d87845bb033
GIT binary patch
literal 8289
zcmb7JOKcm*8J^`!tL0K!Qk3OaY{E(UN|8{0Bx#EVC3pF<L~6yQ;si9nrRB7;og#}k
z2J%1vSpgEqhoT9aphr4TfZp0uuQkw1d-JUUdg`H<T#F(`4`IK5h9l3A6cF>U+JE-@
z{(rvNo&8obUf^~51;&j3l{egaP$<9Ry9N9V#u%N`U#F3{%(}Z?xz;n%v4qjRo#y8_
zl+FB)z4BJg`29}!c^JO+QE2odcI6A_vn&1RgFE3BMxa|)BFmx^r?Sus%DwKMYx!=Y
zX!hz2&n`W%Ota^Tv$)wSd2MF)wi+JGug~>10ylB@26MejtHV}uu#;V~oNn`<=e{|8
z@)w_9daHE*=l}TS-5XW{fV22(;_tbL4&i&Dbt)V>pAN1W?-QM0TOBTaI^dT@n?qNk
zkXr89qKQ(i?%(L{z38<|F7IQ;Z}~;HxQc$c?I3&FI{DEGV>S&A{r)r>js7A|nez^F
znx;@$G3$P%BUKvEMyk5Ib-&YOHe(OIO|wMzwnOpK%axW_@Y`Omkbk}xbV@9umN25G
z{=1>&ghTt2LC6MSdNkT+!-Y=<efGP)&HnY@<UZTC7DeB(*20JL)Rza}vIJx3&G(+A
z-q3h=_l`G$QjZTY%AZD8{6>2-W`&mRtSvqlmq0>w_s*}CJ%GhMilg`Fl`M1ATCN!(
z3z=cmjH~P_{+@}UT6gcM=El|mYjH_qnO5=)jZk;*?7H7smAMn*ej6Iru`)H|?fBbq
z(>$}wGoN)gng^UD&=}1boQLs;ab})zIOpkh$35sxdVK$Q>WD?mFwZapL)6MX8lzp`
z4LU~pzHOPfH59lQyR8BaUZJ}iR4!r`{LQv-<I)tk<znR`E|go%wvm1yXcV~R)>gCN
zH@sI0U8B72uCe^nzFWB%>pn@*XcX-bB6`QT#wbVk`_#P8^3RmOnT4WU5go!YHq3h|
z-wT=x!}amhc<{pC{~cU-xB1D+*Ue*pb(<08di~eo@X5h-Has@CX2mcA-sjJejmLJx
z_Oo&0HXYc#mErQ>x-}Q&k};NH{V2TCXKcT3`VF_r@^%k@jcA7==Qpn!^sqSY*L))T
z?T(*YySbZ7C-8K#_10R2iE{(zRhF}<-HIWo$smrP&T^Rq`hl4waN~$r04|7YoCl~3
ziHZCi8{<r1#}P3lG{!g+P|ie1O!S6h<D4mb5NE`c(70d%%9#j>DXU;!2`tZ(;7kck
z2&U{jLRDu{)0yN<2~7&7>^wqMXG+tV;!Fum={garI_ET<bDGXMT_-|Sr>W^QHJzre
z6QQaTZy;r)=QW-4x=w_uPD|5iX*w-kCqh+cTGN@<bf$Hk2vwbU7b^GD)^yssPK2t?
zjHWZA>CEUl5vn>DG@T2Y&IMg3LRIIYrgKr#xv1+zsOoeyosOo{(RCtJb!Ii4SxskF
z*NITonbUOUG@UtJCqh-{lBRP>)48PUM5yY_YdZ6q&b+P@p{jFP)48naT-J3WRCONF
zbRN-k9?^9oRCVG@0#BcO53OiAS9F~SRh>sQokumDM|GVDRh`E;vowAa9^=gN_+@uY
zFx^KXF}*SNxW+uLF^}s^gevod#yp`hPv}g9D)XeqJgG5H>P&<x^9halgvNYAXChRY
zr#Mr(%qh;4@Ay-KDPP<Oi7D^kX|3jIt>$UHCPGougn^b61FZ!Ol;&n?;2ed=28sY>
zMG!|Ii^4z!j$4w!h$Z`Hps0Wz7$`z14F)Q((m)j`8K{6B7$`#YMjEKVjw7n68mNFC
z7$`#Yh+|=(0y~bz8ab2<R6u2@>V$!cw44b8RiI>`2&F5Mfg+STVW0xbnJ`cVN(L&R
zye1haLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9!+N1}d<e2?JH2
zWS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;B
zKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!
z%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-
z7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZul
zLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPO
zNd}5g>V$y`EN8+%6(|{~fN~}oC_<?d28u{AP(-SMB2*0&p<<v469%d<$v_n*8K}a9
zfhtTGsKO)zRhVR;3KIsZFkzqylMGa0l7T8r7^uR8fhtTgP=!ebsxV=oh~(Xcfg-{k
z<AX^CiV(L!hQyR(3j;+Y#})>PNRBNTC_*{5WS|JKRCK%t3uQxl!2em@r+-NArRYzo
zd;6!)#p%CPTGgQS4#RJ6mf*OXWjeTY@Jxoam(2>DSLslrL*2|TH8!ytFr0tS+TFn-
z%ly8T&Yy2=y6t%Y+QsaGmHs%z=J`s{JM1pCSxFol(R(}ABBL{OqK&1O^*MYal;o0!
zjpZ6z3^LK}AbZ4G`gnoO$Kh@a?{9`TF;*|~i+mkupSpnFl=9i0*9h4AbZ+hPmPJ~R
zVnDcU9<TI*c3|Ay8TWz8Q~S7jd7?TNPrh^YvQoXwtC!I~wR$-}yL!31*Cg#?DGkeg
zW9?S#G0MA2d6$=WC(65rly}GFy$SIa7PP*zdQU0u@$%k8dGC<&-ne{avV7t(%2$-~
z6<)qFQND6W`O3I_b+UZ&G0In!@>O2GI#IrQNcrlxd~LFP>M_dKl=3xRzBW<5c1Zc!
zxO{!G{M=)duPf#2ynKD4eEpE}b(Bxgt8B8oBg)H{2R<t>-NpK?-j+}CDUUxiugB>o
zr?HjQ)8SiVDxGMXnQm%c=Ja&<0GUcB+GeJknrB!7rg<iX;j3gSooJhxZff4-^>p}D
znMx<xW~Q5(2g`aoe7{Vk6KyloO+D8~^mO>BnMx<xW~Q5ZPFM7F_|ln5C))IM&WT6s
z*?WsYvy814Y^&IDtob_|FNxiWx^?-{iof5k1oA1hFgf8(8Q0$`)Zch3di~a~3U9u4
z^Nm|?ZohVubq&{FUmHKI8Oy2}U)f*ThK*HycoHq0e`Q;C_x2AcD6xX;S6YVKXa$Db
zZoX__0lY|us=?x}+w`!=o+D)34Jzvez`atZfVJjEt6@fr8u8ShyUg{VL!y(JlIu5$
z#nmEpLu?!+^I5l3eKj_;20QFSw^%G*%hyv_y)VsopS=8BLT+u4P!d4P?~~0a7O`-6
zd@y-|Bt+ICmffeWtgr2d`DT)2JzgPWG8Hn+%%86waRawy9wNhZTa`<>*eGJ#k#Y44
zFE_YqMC@v0gc0k1v2Oo%5E`FtqbZEGCrfg{la073!<Ek_TX9>qzZ}jF`ql_#zKmvF
zA3ilTa&+IL?K{-RKD*UNukO7tyf6jhPX=z^wiS=21_l?wyR}Uli<N}fS)_zl*f!mA
zXxr|t&K5V73;nruW%e*P=iQB<V;?fiv!9l!{209G+KjP@tk`5LL9b6XVwh+4uKU4@
z^h{&^<A>>i=i1z-ryDb%S?XldmdUW%K;NPH1#Fsj(j<oMT)Un>fyq<zE4@nbEE~^w
znn?6aIU5PO#3%lEP8FYL`JVv0%4i@Cb_B(amD1q5E#6ZN;WU@UOy!$Ge6RFk+WPaI
z42xlj0KX9(uw(eperOve+6}yu@cRarAKC|T275@1{fBR|^6T_jJQ~qwG5ump+kQ2%
zjrB)yh5bRhav%G$--;{uU%LAYwrt1QL>rA3?SWz7v>(N^N$DmqIAWKY?*NxDIO5ev
z?aDh*Y+R;=gY$P{Ayr5je5dnb!ms;nF~qcu{3r<qX~z3?I@f*szHJ=EHsud5qkM3#
zmrErK^d0BtdpR><#Vjo1A-X_UNQv{aN4KH7_iXi&D||1rS!r(JF*-|VZ04#xc9Kq#
z-r`OxahY%JRTFd<yL;)NSQLB47?K|a#TP|@J7zGLaj@z!Y+B;vtUNi#Pg3-Vu>S)s
Cro3tZ
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 151e7cf42953f3f5fe61ff0140ab7b976fe9e5b8..050aaa237b466b0bda8cca5cfaa06f84661d622e 100644
GIT binary patch
delta 24
gcmaFv@Z5pRCD<h-U4emtamhrkD~xg*?{~-p0BWTOc>n+a
delta 24
gcmaFv@Z5pRCD<h-U4emtaq&d1D~$3R?{~-p0BWEJc>n+a
diff --git a/tests/data/acpi/q35/DSDT.smm-compat b/tests/data/acpi/q35/DSDT.smm-compat
new file mode 100644
index 0000000000000000000000000000000000000000..c1965f6051ef2af81dd8412abe169d87845bb033
GIT binary patch
literal 8289
zcmb7JOKcm*8J^`!tL0K!Qk3OaY{E(UN|8{0Bx#EVC3pF<L~6yQ;si9nrRB7;og#}k
z2J%1vSpgEqhoT9aphr4TfZp0uuQkw1d-JUUdg`H<T#F(`4`IK5h9l3A6cF>U+JE-@
z{(rvNo&8obUf^~51;&j3l{egaP$<9Ry9N9V#u%N`U#F3{%(}Z?xz;n%v4qjRo#y8_
zl+FB)z4BJg`29}!c^JO+QE2odcI6A_vn&1RgFE3BMxa|)BFmx^r?Sus%DwKMYx!=Y
zX!hz2&n`W%Ota^Tv$)wSd2MF)wi+JGug~>10ylB@26MejtHV}uu#;V~oNn`<=e{|8
z@)w_9daHE*=l}TS-5XW{fV22(;_tbL4&i&Dbt)V>pAN1W?-QM0TOBTaI^dT@n?qNk
zkXr89qKQ(i?%(L{z38<|F7IQ;Z}~;HxQc$c?I3&FI{DEGV>S&A{r)r>js7A|nez^F
znx;@$G3$P%BUKvEMyk5Ib-&YOHe(OIO|wMzwnOpK%axW_@Y`Omkbk}xbV@9umN25G
z{=1>&ghTt2LC6MSdNkT+!-Y=<efGP)&HnY@<UZTC7DeB(*20JL)Rza}vIJx3&G(+A
z-q3h=_l`G$QjZTY%AZD8{6>2-W`&mRtSvqlmq0>w_s*}CJ%GhMilg`Fl`M1ATCN!(
z3z=cmjH~P_{+@}UT6gcM=El|mYjH_qnO5=)jZk;*?7H7smAMn*ej6Iru`)H|?fBbq
z(>$}wGoN)gng^UD&=}1boQLs;ab})zIOpkh$35sxdVK$Q>WD?mFwZapL)6MX8lzp`
z4LU~pzHOPfH59lQyR8BaUZJ}iR4!r`{LQv-<I)tk<znR`E|go%wvm1yXcV~R)>gCN
zH@sI0U8B72uCe^nzFWB%>pn@*XcX-bB6`QT#wbVk`_#P8^3RmOnT4WU5go!YHq3h|
z-wT=x!}amhc<{pC{~cU-xB1D+*Ue*pb(<08di~eo@X5h-Has@CX2mcA-sjJejmLJx
z_Oo&0HXYc#mErQ>x-}Q&k};NH{V2TCXKcT3`VF_r@^%k@jcA7==Qpn!^sqSY*L))T
z?T(*YySbZ7C-8K#_10R2iE{(zRhF}<-HIWo$smrP&T^Rq`hl4waN~$r04|7YoCl~3
ziHZCi8{<r1#}P3lG{!g+P|ie1O!S6h<D4mb5NE`c(70d%%9#j>DXU;!2`tZ(;7kck
z2&U{jLRDu{)0yN<2~7&7>^wqMXG+tV;!Fum={garI_ET<bDGXMT_-|Sr>W^QHJzre
z6QQaTZy;r)=QW-4x=w_uPD|5iX*w-kCqh+cTGN@<bf$Hk2vwbU7b^GD)^yssPK2t?
zjHWZA>CEUl5vn>DG@T2Y&IMg3LRIIYrgKr#xv1+zsOoeyosOo{(RCtJb!Ii4SxskF
z*NITonbUOUG@UtJCqh-{lBRP>)48PUM5yY_YdZ6q&b+P@p{jFP)48naT-J3WRCONF
zbRN-k9?^9oRCVG@0#BcO53OiAS9F~SRh>sQokumDM|GVDRh`E;vowAa9^=gN_+@uY
zFx^KXF}*SNxW+uLF^}s^gevod#yp`hPv}g9D)XeqJgG5H>P&<x^9halgvNYAXChRY
zr#Mr(%qh;4@Ay-KDPP<Oi7D^kX|3jIt>$UHCPGougn^b61FZ!Ol;&n?;2ed=28sY>
zMG!|Ii^4z!j$4w!h$Z`Hps0Wz7$`z14F)Q((m)j`8K{6B7$`#YMjEKVjw7n68mNFC
z7$`#Yh+|=(0y~bz8ab2<R6u2@>V$!cw44b8RiI>`2&F5Mfg+STVW0xbnJ`cVN(L&R
zye1haLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9!+N1}d<e2?JH2
zWS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;B
zKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!
z%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-
z7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZul
zLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPO
zNd}5g>V$y`EN8+%6(|{~fN~}oC_<?d28u{AP(-SMB2*0&p<<v469%d<$v_n*8K}a9
zfhtTGsKO)zRhVR;3KIsZFkzqylMGa0l7T8r7^uR8fhtTgP=!ebsxV=oh~(Xcfg-{k
z<AX^CiV(L!hQyR(3j;+Y#})>PNRBNTC_*{5WS|JKRCK%t3uQxl!2em@r+-NArRYzo
zd;6!)#p%CPTGgQS4#RJ6mf*OXWjeTY@Jxoam(2>DSLslrL*2|TH8!ytFr0tS+TFn-
z%ly8T&Yy2=y6t%Y+QsaGmHs%z=J`s{JM1pCSxFol(R(}ABBL{OqK&1O^*MYal;o0!
zjpZ6z3^LK}AbZ4G`gnoO$Kh@a?{9`TF;*|~i+mkupSpnFl=9i0*9h4AbZ+hPmPJ~R
zVnDcU9<TI*c3|Ay8TWz8Q~S7jd7?TNPrh^YvQoXwtC!I~wR$-}yL!31*Cg#?DGkeg
zW9?S#G0MA2d6$=WC(65rly}GFy$SIa7PP*zdQU0u@$%k8dGC<&-ne{avV7t(%2$-~
z6<)qFQND6W`O3I_b+UZ&G0In!@>O2GI#IrQNcrlxd~LFP>M_dKl=3xRzBW<5c1Zc!
zxO{!G{M=)duPf#2ynKD4eEpE}b(Bxgt8B8oBg)H{2R<t>-NpK?-j+}CDUUxiugB>o
zr?HjQ)8SiVDxGMXnQm%c=Ja&<0GUcB+GeJknrB!7rg<iX;j3gSooJhxZff4-^>p}D
znMx<xW~Q5(2g`aoe7{Vk6KyloO+D8~^mO>BnMx<xW~Q5ZPFM7F_|ln5C))IM&WT6s
z*?WsYvy814Y^&IDtob_|FNxiWx^?-{iof5k1oA1hFgf8(8Q0$`)Zch3di~a~3U9u4
z^Nm|?ZohVubq&{FUmHKI8Oy2}U)f*ThK*HycoHq0e`Q;C_x2AcD6xX;S6YVKXa$Db
zZoX__0lY|us=?x}+w`!=o+D)34Jzvez`atZfVJjEt6@fr8u8ShyUg{VL!y(JlIu5$
z#nmEpLu?!+^I5l3eKj_;20QFSw^%G*%hyv_y)VsopS=8BLT+u4P!d4P?~~0a7O`-6
zd@y-|Bt+ICmffeWtgr2d`DT)2JzgPWG8Hn+%%86waRawy9wNhZTa`<>*eGJ#k#Y44
zFE_YqMC@v0gc0k1v2Oo%5E`FtqbZEGCrfg{la073!<Ek_TX9>qzZ}jF`ql_#zKmvF
zA3ilTa&+IL?K{-RKD*UNukO7tyf6jhPX=z^wiS=21_l?wyR}Uli<N}fS)_zl*f!mA
zXxr|t&K5V73;nruW%e*P=iQB<V;?fiv!9l!{209G+KjP@tk`5LL9b6XVwh+4uKU4@
z^h{&^<A>>i=i1z-ryDb%S?XldmdUW%K;NPH1#Fsj(j<oMT)Un>fyq<zE4@nbEE~^w
znn?6aIU5PO#3%lEP8FYL`JVv0%4i@Cb_B(amD1q5E#6ZN;WU@UOy!$Ge6RFk+WPaI
z42xlj0KX9(uw(eperOve+6}yu@cRarAKC|T275@1{fBR|^6T_jJQ~qwG5ump+kQ2%
zjrB)yh5bRhav%G$--;{uU%LAYwrt1QL>rA3?SWz7v>(N^N$DmqIAWKY?*NxDIO5ev
z?aDh*Y+R;=gY$P{Ayr5je5dnb!ms;nF~qcu{3r<qX~z3?I@f*szHJ=EHsud5qkM3#
zmrErK^d0BtdpR><#Vjo1A-X_UNQv{aN4KH7_iXi&D||1rS!r(JF*-|VZ04#xc9Kq#
z-r`OxahY%JRTFd<yL;)NSQLB47?K|a#TP|@J7zGLaj@z!Y+B;vtUNi#Pg3-Vu>S)s
Cro3tZ
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.smm-compat-nosmm b/tests/data/acpi/q35/DSDT.smm-compat-nosmm
new file mode 100644
index 0000000000000000000000000000000000000000..c1965f6051ef2af81dd8412abe169d87845bb033
GIT binary patch
literal 8289
zcmb7JOKcm*8J^`!tL0K!Qk3OaY{E(UN|8{0Bx#EVC3pF<L~6yQ;si9nrRB7;og#}k
z2J%1vSpgEqhoT9aphr4TfZp0uuQkw1d-JUUdg`H<T#F(`4`IK5h9l3A6cF>U+JE-@
z{(rvNo&8obUf^~51;&j3l{egaP$<9Ry9N9V#u%N`U#F3{%(}Z?xz;n%v4qjRo#y8_
zl+FB)z4BJg`29}!c^JO+QE2odcI6A_vn&1RgFE3BMxa|)BFmx^r?Sus%DwKMYx!=Y
zX!hz2&n`W%Ota^Tv$)wSd2MF)wi+JGug~>10ylB@26MejtHV}uu#;V~oNn`<=e{|8
z@)w_9daHE*=l}TS-5XW{fV22(;_tbL4&i&Dbt)V>pAN1W?-QM0TOBTaI^dT@n?qNk
zkXr89qKQ(i?%(L{z38<|F7IQ;Z}~;HxQc$c?I3&FI{DEGV>S&A{r)r>js7A|nez^F
znx;@$G3$P%BUKvEMyk5Ib-&YOHe(OIO|wMzwnOpK%axW_@Y`Omkbk}xbV@9umN25G
z{=1>&ghTt2LC6MSdNkT+!-Y=<efGP)&HnY@<UZTC7DeB(*20JL)Rza}vIJx3&G(+A
z-q3h=_l`G$QjZTY%AZD8{6>2-W`&mRtSvqlmq0>w_s*}CJ%GhMilg`Fl`M1ATCN!(
z3z=cmjH~P_{+@}UT6gcM=El|mYjH_qnO5=)jZk;*?7H7smAMn*ej6Iru`)H|?fBbq
z(>$}wGoN)gng^UD&=}1boQLs;ab})zIOpkh$35sxdVK$Q>WD?mFwZapL)6MX8lzp`
z4LU~pzHOPfH59lQyR8BaUZJ}iR4!r`{LQv-<I)tk<znR`E|go%wvm1yXcV~R)>gCN
zH@sI0U8B72uCe^nzFWB%>pn@*XcX-bB6`QT#wbVk`_#P8^3RmOnT4WU5go!YHq3h|
z-wT=x!}amhc<{pC{~cU-xB1D+*Ue*pb(<08di~eo@X5h-Has@CX2mcA-sjJejmLJx
z_Oo&0HXYc#mErQ>x-}Q&k};NH{V2TCXKcT3`VF_r@^%k@jcA7==Qpn!^sqSY*L))T
z?T(*YySbZ7C-8K#_10R2iE{(zRhF}<-HIWo$smrP&T^Rq`hl4waN~$r04|7YoCl~3
ziHZCi8{<r1#}P3lG{!g+P|ie1O!S6h<D4mb5NE`c(70d%%9#j>DXU;!2`tZ(;7kck
z2&U{jLRDu{)0yN<2~7&7>^wqMXG+tV;!Fum={garI_ET<bDGXMT_-|Sr>W^QHJzre
z6QQaTZy;r)=QW-4x=w_uPD|5iX*w-kCqh+cTGN@<bf$Hk2vwbU7b^GD)^yssPK2t?
zjHWZA>CEUl5vn>DG@T2Y&IMg3LRIIYrgKr#xv1+zsOoeyosOo{(RCtJb!Ii4SxskF
z*NITonbUOUG@UtJCqh-{lBRP>)48PUM5yY_YdZ6q&b+P@p{jFP)48naT-J3WRCONF
zbRN-k9?^9oRCVG@0#BcO53OiAS9F~SRh>sQokumDM|GVDRh`E;vowAa9^=gN_+@uY
zFx^KXF}*SNxW+uLF^}s^gevod#yp`hPv}g9D)XeqJgG5H>P&<x^9halgvNYAXChRY
zr#Mr(%qh;4@Ay-KDPP<Oi7D^kX|3jIt>$UHCPGougn^b61FZ!Ol;&n?;2ed=28sY>
zMG!|Ii^4z!j$4w!h$Z`Hps0Wz7$`z14F)Q((m)j`8K{6B7$`#YMjEKVjw7n68mNFC
z7$`#Yh+|=(0y~bz8ab2<R6u2@>V$!cw44b8RiI>`2&F5Mfg+STVW0xbnJ`cVN(L&R
zye1haLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9!+N1}d<e2?JH2
zWS|1dnPi{{rA`>Az;Y%GRDqI#3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;B
zKouw%sDN@N87M-j69y`<oCyO}pk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!
z%b74x1xf}gpqxnticso=feI{V!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-
z7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZul
zLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPO
zNd}5g>V$y`EN8+%6(|{~fN~}oC_<?d28u{AP(-SMB2*0&p<<v469%d<$v_n*8K}a9
zfhtTGsKO)zRhVR;3KIsZFkzqylMGa0l7T8r7^uR8fhtTgP=!ebsxV=oh~(Xcfg-{k
z<AX^CiV(L!hQyR(3j;+Y#})>PNRBNTC_*{5WS|JKRCK%t3uQxl!2em@r+-NArRYzo
zd;6!)#p%CPTGgQS4#RJ6mf*OXWjeTY@Jxoam(2>DSLslrL*2|TH8!ytFr0tS+TFn-
z%ly8T&Yy2=y6t%Y+QsaGmHs%z=J`s{JM1pCSxFol(R(}ABBL{OqK&1O^*MYal;o0!
zjpZ6z3^LK}AbZ4G`gnoO$Kh@a?{9`TF;*|~i+mkupSpnFl=9i0*9h4AbZ+hPmPJ~R
zVnDcU9<TI*c3|Ay8TWz8Q~S7jd7?TNPrh^YvQoXwtC!I~wR$-}yL!31*Cg#?DGkeg
zW9?S#G0MA2d6$=WC(65rly}GFy$SIa7PP*zdQU0u@$%k8dGC<&-ne{avV7t(%2$-~
z6<)qFQND6W`O3I_b+UZ&G0In!@>O2GI#IrQNcrlxd~LFP>M_dKl=3xRzBW<5c1Zc!
zxO{!G{M=)duPf#2ynKD4eEpE}b(Bxgt8B8oBg)H{2R<t>-NpK?-j+}CDUUxiugB>o
zr?HjQ)8SiVDxGMXnQm%c=Ja&<0GUcB+GeJknrB!7rg<iX;j3gSooJhxZff4-^>p}D
znMx<xW~Q5(2g`aoe7{Vk6KyloO+D8~^mO>BnMx<xW~Q5ZPFM7F_|ln5C))IM&WT6s
z*?WsYvy814Y^&IDtob_|FNxiWx^?-{iof5k1oA1hFgf8(8Q0$`)Zch3di~a~3U9u4
z^Nm|?ZohVubq&{FUmHKI8Oy2}U)f*ThK*HycoHq0e`Q;C_x2AcD6xX;S6YVKXa$Db
zZoX__0lY|us=?x}+w`!=o+D)34Jzvez`atZfVJjEt6@fr8u8ShyUg{VL!y(JlIu5$
z#nmEpLu?!+^I5l3eKj_;20QFSw^%G*%hyv_y)VsopS=8BLT+u4P!d4P?~~0a7O`-6
zd@y-|Bt+ICmffeWtgr2d`DT)2JzgPWG8Hn+%%86waRawy9wNhZTa`<>*eGJ#k#Y44
zFE_YqMC@v0gc0k1v2Oo%5E`FtqbZEGCrfg{la073!<Ek_TX9>qzZ}jF`ql_#zKmvF
zA3ilTa&+IL?K{-RKD*UNukO7tyf6jhPX=z^wiS=21_l?wyR}Uli<N}fS)_zl*f!mA
zXxr|t&K5V73;nruW%e*P=iQB<V;?fiv!9l!{209G+KjP@tk`5LL9b6XVwh+4uKU4@
z^h{&^<A>>i=i1z-ryDb%S?XldmdUW%K;NPH1#Fsj(j<oMT)Un>fyq<zE4@nbEE~^w
znn?6aIU5PO#3%lEP8FYL`JVv0%4i@Cb_B(amD1q5E#6ZN;WU@UOy!$Ge6RFk+WPaI
z42xlj0KX9(uw(eperOve+6}yu@cRarAKC|T275@1{fBR|^6T_jJQ~qwG5ump+kQ2%
zjrB)yh5bRhav%G$--;{uU%LAYwrt1QL>rA3?SWz7v>(N^N$DmqIAWKY?*NxDIO5ev
z?aDh*Y+R;=gY$P{Ayr5je5dnb!ms;nF~qcu{3r<qX~z3?I@f*szHJ=EHsud5qkM3#
zmrErK^d0BtdpR><#Vjo1A-X_UNQv{aN4KH7_iXi&D||1rS!r(JF*-|VZ04#xc9Kq#
z-r`OxahY%JRTFd<yL;)NSQLB47?K|a#TP|@J7zGLaj@z!Y+B;vtUNi#Pg3-Vu>S)s
Cro3tZ
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.tis.tpm12 b/tests/data/acpi/q35/DSDT.tis.tpm12
index c96b5277a14ae98174408d690d6e0246bd932623..0ebdf6fbd77967f1ab5d5337b7b1fed314cfaca8 100644
GIT binary patch
delta 24
gcmdnzy3du%CD<iopArKDqxwXyD~xg*@84Gh0A(f!W&i*H
delta 24
gcmdnzy3du%CD<iopArKDquNBSD~$3R@84Gh0A(QvW&i*H
diff --git a/tests/data/acpi/q35/DSDT.tis.tpm2 b/tests/data/acpi/q35/DSDT.tis.tpm2
index c92d4d29c79352a60974ea9f665d0b9a410a4bac..dcbb7f0af377425db53130e8ba1c62c09c22e006 100644
GIT binary patch
delta 24
gcmdnzy3du%CD<iopArKD<D-dOR~Y3s-oLL10Bm##ApigX
delta 24
gcmdnzy3du%CD<iopArKD<HLzuR~Y3t-oLL10BmmwApigX
diff --git a/tests/data/acpi/q35/DSDT.xapic b/tests/data/acpi/q35/DSDT.xapic
index 119fc90f1f8a7b6934df6fd95609446e627ce15d..17552ce363ae81985f69f9ae85837a1540b79ae0 100644
GIT binary patch
delta 26
icmX>yjp@iVCN7s?mk^h31_s6r6S=N1%5A)#+64f6_X&Rh
delta 26
icmX>yjp@iVCN7s?mk^h31_s9U6S=N1%5S`%+64f6@(F(c
--
2.31.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
2021-11-10 5:30 ` [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit' Julia Suvorova
@ 2021-11-10 6:04 ` Michael S. Tsirkin
2021-11-10 9:08 ` Daniel P. Berrangé
0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-11-10 6:04 UTC (permalink / raw)
To: Julia Suvorova
Cc: Ani Sinha, Igor Mammedov, Marcel Apfelbaum, qemu-devel, Gerd Hoffmann
On Wed, Nov 10, 2021 at 06:30:10AM +0100, Julia Suvorova wrote:
> Rename the option to better represent its function - toggle Hot-Plug
> Capable bit in the PCIe Slot Capability.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> include/hw/pci/pcie_port.h | 2 +-
> hw/i386/pc_q35.c | 2 +-
> hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
> hw/pci/pcie.c | 2 +-
> hw/pci/pcie_port.c | 2 +-
> 5 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index e25b289ce8..0da6d66c95 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -60,7 +60,7 @@ struct PCIESlot {
> /* Indicates whether any type of hot-plug is allowed on the slot */
> bool hotplug;
>
> - bool native_hotplug;
> + bool native_hpc_bit;
>
> QLIST_ENTRY(PCIESlot) next;
> };
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
This is ok.
> index 797e09500b..ded61f8ef7 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
> NULL);
>
> if (acpi_pcihp) {
> - object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
> + object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> "false", true);
> }
>
This part is problematic since we made the feature user-settable in 6.1.
We can have two features if we really want to ...
I don't think we have a way to mark properties deprecated, do we?
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index 20099a8ae3..ed079d72b3 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -87,7 +87,11 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> + /*
> + * Make sure that IO is assigned in case the slot is hot-pluggable
> + * but it is not visible through the PCIe Slot Capabilities
> + */
> + if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hpc_bit) {
> grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> }
> int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 914a9bf3d1..ebe002831e 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -535,7 +535,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> * hot-plug is disabled on the slot.
> */
> if (s->hotplug &&
> - (s->native_hotplug || DEVICE(dev)->hotplugged)) {
> + (s->native_hpc_bit || DEVICE(dev)->hotplugged)) {
> pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
> PCI_EXP_SLTCAP_HPS |
> PCI_EXP_SLTCAP_HPC);
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index da850e8dde..eae06d10e2 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
> DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
> - DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
> + DEFINE_PROP_BOOL("native-hpc-bit", PCIESlot, native_hpc_bit, true),
> DEFINE_PROP_END_OF_LIST()
> };
>
> --
> 2.31.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
2021-11-10 5:30 ` [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC Julia Suvorova
@ 2021-11-10 7:21 ` Michael S. Tsirkin
2021-11-10 13:57 ` Igor Mammedov
0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-11-10 7:21 UTC (permalink / raw)
To: Julia Suvorova
Cc: Ani Sinha, Igor Mammedov, Marcel Apfelbaum, qemu-devel, Gerd Hoffmann
On Wed, Nov 10, 2021 at 06:30:13AM +0100, Julia Suvorova wrote:
> There are two ways to enable ACPI PCI Hot-plug:
>
> * Disable the Hot-plug Capable bit on PCIe slots.
>
> This was the first approach which led to regression [1-2], as
> I/O space for a port is allocated only when it is hot-pluggable,
> which is determined by HPC bit.
>
> * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
> method.
>
> This removes the (future) ability of hot-plugging switches with PCIe
> Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
> bridges. If the user wants to explicitely use this feature, they can
> disable ACPI PCI Hot-plug with:
> --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
>
> Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
> instead of PCIe Native.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> hw/i386/acpi-build.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a3ad6abd33..a2f92ab32b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1337,7 +1337,7 @@ static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
> aml_append(table, scope);
> }
>
> -static Aml *build_q35_osc_method(void)
> +static Aml *build_q35_osc_method(bool acpi_pcihp)
> {
> Aml *if_ctx;
> Aml *if_ctx2;
> @@ -1345,6 +1345,7 @@ static Aml *build_q35_osc_method(void)
> Aml *method;
> Aml *a_cwd1 = aml_name("CDW1");
> Aml *a_ctrl = aml_local(0);
> + const uint64_t hotplug = acpi_pcihp ? 0x1E : 0x1F;
drop this variable and open-code at use point below.
As it is the comment is misplaced.
Also a slightly better way to write this:
0x1E | (acpi_pcihp ? 0x0 : 0x1)
>
> method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
So what acpi_pcihp does is enable/disable native pcie hotplug.
How about we call the option exactly that?
> @@ -1359,8 +1360,9 @@ static Aml *build_q35_osc_method(void)
> /*
> * Always allow native PME, AER (no dependencies)
> * Allow SHPC (PCI bridges can have SHPC controller)
> + * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
> */
> - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
> + aml_append(if_ctx, aml_and(a_ctrl, aml_int(hotplug), a_ctrl));
>
using the variable hotplug just made things confusing,
open-coding will be clearer.
> if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
> /* Unknown revision */
> @@ -1449,7 +1451,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> - aml_append(dev, build_q35_osc_method());
> + aml_append(dev, build_q35_osc_method(pm->pcihp_bridge_en));
> aml_append(sb_scope, dev);
> if (mcfg_valid) {
> aml_append(sb_scope, build_q35_dram_controller(&mcfg));
One of the complaints of libvirt people was that the
name is confusing. It seems to say whether to describe bridges
in ACPI but it also disables native hotplug, but only for PCIe.
Maybe we should address this with flags saying whether to enable each of
acpi,pcie,shpc hotplug separately...
> @@ -1565,7 +1567,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> if (pci_bus_is_express(bus)) {
> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> - aml_append(dev, build_q35_osc_method());
> +
> + /* Expander bridges do not have ACPI PCI Hot-plug enabled */
> + aml_append(dev, build_q35_osc_method(false));
> } else {
> aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> }
> --
> 2.31.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
2021-11-10 6:04 ` Michael S. Tsirkin
@ 2021-11-10 9:08 ` Daniel P. Berrangé
2021-11-10 13:30 ` Igor Mammedov
0 siblings, 1 reply; 16+ messages in thread
From: Daniel P. Berrangé @ 2021-11-10 9:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Marcel Apfelbaum, Julia Suvorova, qemu-devel, Gerd Hoffmann,
Ani Sinha, Igor Mammedov
On Wed, Nov 10, 2021 at 01:04:54AM -0500, Michael S. Tsirkin wrote:
> On Wed, Nov 10, 2021 at 06:30:10AM +0100, Julia Suvorova wrote:
> > Rename the option to better represent its function - toggle Hot-Plug
> > Capable bit in the PCIe Slot Capability.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > include/hw/pci/pcie_port.h | 2 +-
> > hw/i386/pc_q35.c | 2 +-
> > hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
> > hw/pci/pcie.c | 2 +-
> > hw/pci/pcie_port.c | 2 +-
> > 5 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > index e25b289ce8..0da6d66c95 100644
> > --- a/include/hw/pci/pcie_port.h
> > +++ b/include/hw/pci/pcie_port.h
> > @@ -60,7 +60,7 @@ struct PCIESlot {
> > /* Indicates whether any type of hot-plug is allowed on the slot */
> > bool hotplug;
> >
> > - bool native_hotplug;
> > + bool native_hpc_bit;
> >
> > QLIST_ENTRY(PCIESlot) next;
> > };
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>
>
> This is ok.
>
>
> > index 797e09500b..ded61f8ef7 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
> > NULL);
> >
> > if (acpi_pcihp) {
> > - object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
> > + object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> > "false", true);
> > }
> >
>
>
> This part is problematic since we made the feature user-settable in 6.1.
> We can have two features if we really want to ...
> I don't think we have a way to mark properties deprecated, do we?
IMHO just leave the feature with its current name. It won't be the
first thing without a "perfect" name and the name doesn't have a
negative impact on mgmt apps. Changing the name will cause more
pain than it solves.
>
> > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> > index 20099a8ae3..ed079d72b3 100644
> > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > @@ -87,7 +87,11 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> >
> > - if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> > + /*
> > + * Make sure that IO is assigned in case the slot is hot-pluggable
> > + * but it is not visible through the PCIe Slot Capabilities
> > + */
> > + if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hpc_bit) {
> > grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> > }
> > int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 914a9bf3d1..ebe002831e 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -535,7 +535,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> > * hot-plug is disabled on the slot.
> > */
> > if (s->hotplug &&
> > - (s->native_hotplug || DEVICE(dev)->hotplugged)) {
> > + (s->native_hpc_bit || DEVICE(dev)->hotplugged)) {
> > pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
> > PCI_EXP_SLTCAP_HPS |
> > PCI_EXP_SLTCAP_HPC);
> > diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> > index da850e8dde..eae06d10e2 100644
> > --- a/hw/pci/pcie_port.c
> > +++ b/hw/pci/pcie_port.c
> > @@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
> > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> > DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
> > - DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
> > + DEFINE_PROP_BOOL("native-hpc-bit", PCIESlot, native_hpc_bit, true),
> > DEFINE_PROP_END_OF_LIST()
> > };
> >
> > --
> > 2.31.1
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
2021-11-10 9:08 ` Daniel P. Berrangé
@ 2021-11-10 13:30 ` Igor Mammedov
2021-11-10 15:52 ` Michael S. Tsirkin
0 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2021-11-10 13:30 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Marcel Apfelbaum, Michael S. Tsirkin, Julia Suvorova, qemu-devel,
Gerd Hoffmann, Ani Sinha
On Wed, 10 Nov 2021 09:08:34 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Nov 10, 2021 at 01:04:54AM -0500, Michael S. Tsirkin wrote:
> > On Wed, Nov 10, 2021 at 06:30:10AM +0100, Julia Suvorova wrote:
> > > Rename the option to better represent its function - toggle Hot-Plug
> > > Capable bit in the PCIe Slot Capability.
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > > include/hw/pci/pcie_port.h | 2 +-
> > > hw/i386/pc_q35.c | 2 +-
> > > hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
> > > hw/pci/pcie.c | 2 +-
> > > hw/pci/pcie_port.c | 2 +-
> > > 5 files changed, 9 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > > index e25b289ce8..0da6d66c95 100644
> > > --- a/include/hw/pci/pcie_port.h
> > > +++ b/include/hw/pci/pcie_port.h
> > > @@ -60,7 +60,7 @@ struct PCIESlot {
> > > /* Indicates whether any type of hot-plug is allowed on the slot */
> > > bool hotplug;
> > >
> > > - bool native_hotplug;
> > > + bool native_hpc_bit;
> > >
> > > QLIST_ENTRY(PCIESlot) next;
> > > };
> > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >
> >
> > This is ok.
> >
> >
> > > index 797e09500b..ded61f8ef7 100644
> > > --- a/hw/i386/pc_q35.c
> > > +++ b/hw/i386/pc_q35.c
> > > @@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
> > > NULL);
> > >
> > > if (acpi_pcihp) {
> > > - object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
> > > + object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> > > "false", true);
> > > }
> > >
> >
> >
> > This part is problematic since we made the feature user-settable in 6.1.
> > We can have two features if we really want to ...
> > I don't think we have a way to mark properties deprecated, do we?
describe them in deprecated.rst
>
> IMHO just leave the feature with its current name. It won't be the
> first thing without a "perfect" name and the name doesn't have a
> negative impact on mgmt apps. Changing the name will cause more
> pain than it solves.
looking at the code, it was introduced in 6.1 for internal needs mostly
3f3cbbb23 hw/pci/pcie: Do not set HPC flag if acpihp is used
to hide slot from guest's native-hotplug module.
Even if user tried explicitly to set native-hoplug=on on a slot
with ACPI hotplug enabled, native hotplug will not work properly
as slot's hotplug is still handled by acpi_pcihp_device_plug_cb().
Given above and that it shouldn't been used by users,
I'd rather rename it to x-native-hotplug now than go through deprecation
process and expose it for 2 more releases.
> >
> > > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> > > index 20099a8ae3..ed079d72b3 100644
> > > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > > @@ -87,7 +87,11 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> > > return;
> > > }
> > >
> > > - if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> > > + /*
> > > + * Make sure that IO is assigned in case the slot is hot-pluggable
> > > + * but it is not visible through the PCIe Slot Capabilities
> > > + */
> > > + if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hpc_bit) {
> > > grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> > > }
> > > int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index 914a9bf3d1..ebe002831e 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -535,7 +535,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> > > * hot-plug is disabled on the slot.
> > > */
> > > if (s->hotplug &&
> > > - (s->native_hotplug || DEVICE(dev)->hotplugged)) {
> > > + (s->native_hpc_bit || DEVICE(dev)->hotplugged)) {
> > > pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
> > > PCI_EXP_SLTCAP_HPS |
> > > PCI_EXP_SLTCAP_HPC);
> > > diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> > > index da850e8dde..eae06d10e2 100644
> > > --- a/hw/pci/pcie_port.c
> > > +++ b/hw/pci/pcie_port.c
> > > @@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
> > > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> > > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> > > DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
> > > - DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
> > > + DEFINE_PROP_BOOL("native-hpc-bit", PCIESlot, native_hpc_bit, true),
> > > DEFINE_PROP_END_OF_LIST()
> > > };
> > >
> > > --
> > > 2.31.1
> >
> >
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit'
2021-11-10 5:30 ` [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit' Julia Suvorova
@ 2021-11-10 13:33 ` Igor Mammedov
2021-11-10 13:45 ` Igor Mammedov
0 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2021-11-10 13:33 UTC (permalink / raw)
To: Julia Suvorova
Cc: Ani Sinha, Marcel Apfelbaum, Gerd Hoffmann, qemu-devel,
Michael S. Tsirkin
On Wed, 10 Nov 2021 06:30:11 +0100
Julia Suvorova <jusual@redhat.com> wrote:
> To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be
> turned on, while the switch to ACPI Hot-plug will be done in the
> DSDT table.
>
> Introducing 'x-keep-native-hpc' option disables the HPC bit only
> in 6.1 and as a result keeps the forced 'reserve-io' on
> pcie-root-ports in 6.1 too.
using property from the 1st patch (native-hotplug),
should be enough to keep 6.1 ABI, so this patch is likely
not needed.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> include/hw/acpi/ich9.h | 1 +
> hw/acpi/ich9.c | 18 ++++++++++++++++++
> hw/i386/pc.c | 2 ++
> hw/i386/pc_q35.c | 7 ++++++-
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index f04f1791bd..7ca92843c6 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -56,6 +56,7 @@ typedef struct ICH9LPCPMRegs {
> AcpiCpuHotplug gpe_cpu;
> CPUHotplugState cpuhp_state;
>
> + bool keep_pci_slot_hpc;
> bool use_acpi_hotplug_bridge;
> AcpiPciHpState acpi_pci_hotplug;
> MemHotplugState acpi_memory_hotplug;
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 1ee2ba2c50..ebe08ed831 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -419,6 +419,20 @@ static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp)
> s->pm.use_acpi_hotplug_bridge = value;
> }
>
> +static bool ich9_pm_get_keep_pci_slot_hpc(Object *obj, Error **errp)
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + return s->pm.keep_pci_slot_hpc;
> +}
> +
> +static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp)
> +{
> + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> +
> + s->pm.keep_pci_slot_hpc = value;
> +}
> +
> void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> {
> static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
> @@ -428,6 +442,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> pm->disable_s4 = 0;
> pm->s4_val = 2;
> pm->use_acpi_hotplug_bridge = true;
> + pm->keep_pci_slot_hpc = true;
>
> object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> @@ -454,6 +469,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> object_property_add_bool(obj, ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> ich9_pm_get_acpi_pci_hotplug,
> ich9_pm_set_acpi_pci_hotplug);
> + object_property_add_bool(obj, "x-keep-pci-slot-hpc",
> + ich9_pm_get_keep_pci_slot_hpc,
> + ich9_pm_set_keep_pci_slot_hpc);
> }
>
> void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2592a82148..a2ef40ecbc 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_1[] = {
> { TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
> { TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
> { TYPE_X86_CPU, "hv-version-id-minor", "0x0001" },
> + { "ICH9-LPC", "x-keep-pci-slot-hpc", "false" },
> };
> const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
>
> @@ -107,6 +108,7 @@ GlobalProperty pc_compat_6_0[] = {
> { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
> { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
> + { "ICH9-LPC", "x-keep-pci-slot-hpc", "true" },
> };
> const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index ded61f8ef7..06f09dfcc6 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -137,6 +137,7 @@ static void pc_q35_init(MachineState *machine)
> DriveInfo *hd[MAX_SATA_PORTS];
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> bool acpi_pcihp;
> + bool keep_pci_slot_hpc;
>
> /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> @@ -242,7 +243,11 @@ static void pc_q35_init(MachineState *machine)
> ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> NULL);
>
> - if (acpi_pcihp) {
> + keep_pci_slot_hpc = object_property_get_bool(OBJECT(lpc),
> + "x-keep-pci-slot-hpc",
> + NULL);
> +
> + if (!keep_pci_slot_hpc && acpi_pcihp) {
> object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> "false", true);
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit'
2021-11-10 13:33 ` Igor Mammedov
@ 2021-11-10 13:45 ` Igor Mammedov
0 siblings, 0 replies; 16+ messages in thread
From: Igor Mammedov @ 2021-11-10 13:45 UTC (permalink / raw)
To: Julia Suvorova
Cc: Ani Sinha, Marcel Apfelbaum, Gerd Hoffmann, qemu-devel,
Michael S. Tsirkin
On Wed, 10 Nov 2021 14:33:54 +0100
Igor Mammedov <imammedo@redhat.com> wrote:
> On Wed, 10 Nov 2021 06:30:11 +0100
> Julia Suvorova <jusual@redhat.com> wrote:
>
> > To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be
> > turned on, while the switch to ACPI Hot-plug will be done in the
> > DSDT table.
> >
> > Introducing 'x-keep-native-hpc' option disables the HPC bit only
> > in 6.1 and as a result keeps the forced 'reserve-io' on
> > pcie-root-ports in 6.1 too.
>
> using property from the 1st patch (native-hotplug),
> should be enough to keep 6.1 ABI, so this patch is likely
> not needed.
scratch this off, it's conditional on acpihp which
compat machinery can't handle.
>
> >
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > include/hw/acpi/ich9.h | 1 +
> > hw/acpi/ich9.c | 18 ++++++++++++++++++
> > hw/i386/pc.c | 2 ++
> > hw/i386/pc_q35.c | 7 ++++++-
> > 4 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> > index f04f1791bd..7ca92843c6 100644
> > --- a/include/hw/acpi/ich9.h
> > +++ b/include/hw/acpi/ich9.h
> > @@ -56,6 +56,7 @@ typedef struct ICH9LPCPMRegs {
> > AcpiCpuHotplug gpe_cpu;
> > CPUHotplugState cpuhp_state;
> >
> > + bool keep_pci_slot_hpc;
> > bool use_acpi_hotplug_bridge;
> > AcpiPciHpState acpi_pci_hotplug;
> > MemHotplugState acpi_memory_hotplug;
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 1ee2ba2c50..ebe08ed831 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -419,6 +419,20 @@ static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp)
> > s->pm.use_acpi_hotplug_bridge = value;
> > }
> >
> > +static bool ich9_pm_get_keep_pci_slot_hpc(Object *obj, Error **errp)
> > +{
> > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> > +
> > + return s->pm.keep_pci_slot_hpc;
> > +}
> > +
> > +static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp)
> > +{
> > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> > +
> > + s->pm.keep_pci_slot_hpc = value;
> > +}
> > +
> > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> > {
> > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
> > @@ -428,6 +442,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> > pm->disable_s4 = 0;
> > pm->s4_val = 2;
> > pm->use_acpi_hotplug_bridge = true;
> > + pm->keep_pci_slot_hpc = true;
> >
> > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> > &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> > @@ -454,6 +469,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> > object_property_add_bool(obj, ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> > ich9_pm_get_acpi_pci_hotplug,
> > ich9_pm_set_acpi_pci_hotplug);
> > + object_property_add_bool(obj, "x-keep-pci-slot-hpc",
> > + ich9_pm_get_keep_pci_slot_hpc,
> > + ich9_pm_set_keep_pci_slot_hpc);
> > }
> >
> > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 2592a82148..a2ef40ecbc 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_1[] = {
> > { TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
> > { TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
> > { TYPE_X86_CPU, "hv-version-id-minor", "0x0001" },
> > + { "ICH9-LPC", "x-keep-pci-slot-hpc", "false" },
> > };
> > const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
> >
> > @@ -107,6 +108,7 @@ GlobalProperty pc_compat_6_0[] = {
> > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> > { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
> > { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
> > + { "ICH9-LPC", "x-keep-pci-slot-hpc", "true" },
> > };
> > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
> >
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index ded61f8ef7..06f09dfcc6 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -137,6 +137,7 @@ static void pc_q35_init(MachineState *machine)
> > DriveInfo *hd[MAX_SATA_PORTS];
> > MachineClass *mc = MACHINE_GET_CLASS(machine);
> > bool acpi_pcihp;
> > + bool keep_pci_slot_hpc;
> >
> > /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> > * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> > @@ -242,7 +243,11 @@ static void pc_q35_init(MachineState *machine)
> > ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> > NULL);
> >
> > - if (acpi_pcihp) {
> > + keep_pci_slot_hpc = object_property_get_bool(OBJECT(lpc),
> > + "x-keep-pci-slot-hpc",
> > + NULL);
> > +
> > + if (!keep_pci_slot_hpc && acpi_pcihp) {
> > object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> > "false", true);
> > }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
2021-11-10 7:21 ` Michael S. Tsirkin
@ 2021-11-10 13:57 ` Igor Mammedov
2021-11-10 15:25 ` Julia Suvorova
0 siblings, 1 reply; 16+ messages in thread
From: Igor Mammedov @ 2021-11-10 13:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Ani Sinha, Marcel Apfelbaum, Julia Suvorova, qemu-devel, Gerd Hoffmann
On Wed, 10 Nov 2021 02:21:34 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Nov 10, 2021 at 06:30:13AM +0100, Julia Suvorova wrote:
> > There are two ways to enable ACPI PCI Hot-plug:
> >
> > * Disable the Hot-plug Capable bit on PCIe slots.
> >
> > This was the first approach which led to regression [1-2], as
> > I/O space for a port is allocated only when it is hot-pluggable,
> > which is determined by HPC bit.
> >
> > * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
> > method.
> >
> > This removes the (future) ability of hot-plugging switches with PCIe
> > Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
> > bridges. If the user wants to explicitely use this feature, they can
> > disable ACPI PCI Hot-plug with:
> > --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
> >
> > Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
> > instead of PCIe Native.
> >
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > hw/i386/acpi-build.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index a3ad6abd33..a2f92ab32b 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1337,7 +1337,7 @@ static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
> > aml_append(table, scope);
> > }
> >
> > -static Aml *build_q35_osc_method(void)
> > +static Aml *build_q35_osc_method(bool acpi_pcihp)
> > {
> > Aml *if_ctx;
> > Aml *if_ctx2;
> > @@ -1345,6 +1345,7 @@ static Aml *build_q35_osc_method(void)
> > Aml *method;
> > Aml *a_cwd1 = aml_name("CDW1");
> > Aml *a_ctrl = aml_local(0);
> > + const uint64_t hotplug = acpi_pcihp ? 0x1E : 0x1F;
>
> drop this variable and open-code at use point below.
> As it is the comment is misplaced.
> Also a slightly better way to write this:
> 0x1E | (acpi_pcihp ? 0x0 : 0x1)
>
>
>
> >
> > method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
>
> So what acpi_pcihp does is enable/disable native pcie hotplug.
> How about we call the option exactly that?
>
>
> > @@ -1359,8 +1360,9 @@ static Aml *build_q35_osc_method(void)
> > /*
> > * Always allow native PME, AER (no dependencies)
> > * Allow SHPC (PCI bridges can have SHPC controller)
> > + * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
> > */
> > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
> > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(hotplug), a_ctrl));
> >
>
> using the variable hotplug just made things confusing,
> open-coding will be clearer.
>
>
> > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
> > /* Unknown revision */
> > @@ -1449,7 +1451,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> > - aml_append(dev, build_q35_osc_method());
> > + aml_append(dev, build_q35_osc_method(pm->pcihp_bridge_en));
> > aml_append(sb_scope, dev);
> > if (mcfg_valid) {
> > aml_append(sb_scope, build_q35_dram_controller(&mcfg));
>
> One of the complaints of libvirt people was that the
> name is confusing. It seems to say whether to describe bridges
> in ACPI but it also disables native hotplug, but only for PCIe.
>
> Maybe we should address this with flags saying whether to enable each of
> acpi,pcie,shpc hotplug separately...
yep, mask with field defines would be better,
but I'd hold off such change to post 6.2 time.
>
> > @@ -1565,7 +1567,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > if (pci_bus_is_express(bus)) {
> > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > - aml_append(dev, build_q35_osc_method());
> > + /* Expander bridges do not have ACPI PCI Hot-plug enabled */
> > + aml_append(dev, build_q35_osc_method(false));
this's it not obvious, why PXBs aren't capable of ACPI PCI Hot-plug?
> > } else {
> > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> > }
> > --
> > 2.31.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
2021-11-10 13:57 ` Igor Mammedov
@ 2021-11-10 15:25 ` Julia Suvorova
2021-11-10 15:37 ` Michael S. Tsirkin
0 siblings, 1 reply; 16+ messages in thread
From: Julia Suvorova @ 2021-11-10 15:25 UTC (permalink / raw)
To: Igor Mammedov
Cc: Ani Sinha, Marcel Apfelbaum, Gerd Hoffmann, QEMU Developers,
Michael S. Tsirkin
On Wed, Nov 10, 2021 at 2:58 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 10 Nov 2021 02:21:34 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Nov 10, 2021 at 06:30:13AM +0100, Julia Suvorova wrote:
> > > There are two ways to enable ACPI PCI Hot-plug:
> > >
> > > * Disable the Hot-plug Capable bit on PCIe slots.
> > >
> > > This was the first approach which led to regression [1-2], as
> > > I/O space for a port is allocated only when it is hot-pluggable,
> > > which is determined by HPC bit.
> > >
> > > * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
> > > method.
> > >
> > > This removes the (future) ability of hot-plugging switches with PCIe
> > > Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
> > > bridges. If the user wants to explicitely use this feature, they can
> > > disable ACPI PCI Hot-plug with:
> > > --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
> > >
> > > Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
> > > instead of PCIe Native.
> > >
> > > [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> > > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > > hw/i386/acpi-build.c | 12 ++++++++----
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index a3ad6abd33..a2f92ab32b 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1337,7 +1337,7 @@ static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
> > > aml_append(table, scope);
> > > }
> > >
> > > -static Aml *build_q35_osc_method(void)
> > > +static Aml *build_q35_osc_method(bool acpi_pcihp)
> > > {
> > > Aml *if_ctx;
> > > Aml *if_ctx2;
> > > @@ -1345,6 +1345,7 @@ static Aml *build_q35_osc_method(void)
> > > Aml *method;
> > > Aml *a_cwd1 = aml_name("CDW1");
> > > Aml *a_ctrl = aml_local(0);
> > > + const uint64_t hotplug = acpi_pcihp ? 0x1E : 0x1F;
> >
> > drop this variable and open-code at use point below.
> > As it is the comment is misplaced.
> > Also a slightly better way to write this:
> > 0x1E | (acpi_pcihp ? 0x0 : 0x1)
> >
> >
> >
> > >
> > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> >
> > So what acpi_pcihp does is enable/disable native pcie hotplug.
> > How about we call the option exactly that?
> >
> >
> > > @@ -1359,8 +1360,9 @@ static Aml *build_q35_osc_method(void)
> > > /*
> > > * Always allow native PME, AER (no dependencies)
> > > * Allow SHPC (PCI bridges can have SHPC controller)
> > > + * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
> > > */
> > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
> > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(hotplug), a_ctrl));
> > >
> >
> > using the variable hotplug just made things confusing,
> > open-coding will be clearer.
> >
> >
> > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
> > > /* Unknown revision */
> > > @@ -1449,7 +1451,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > > aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > > aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> > > - aml_append(dev, build_q35_osc_method());
> > > + aml_append(dev, build_q35_osc_method(pm->pcihp_bridge_en));
> > > aml_append(sb_scope, dev);
> > > if (mcfg_valid) {
> > > aml_append(sb_scope, build_q35_dram_controller(&mcfg));
> >
> > One of the complaints of libvirt people was that the
> > name is confusing. It seems to say whether to describe bridges
> > in ACPI but it also disables native hotplug, but only for PCIe.
> >
> > Maybe we should address this with flags saying whether to enable each of
> > acpi,pcie,shpc hotplug separately...
>
> yep, mask with field defines would be better,
> but I'd hold off such change to post 6.2 time.
>
> >
> > > @@ -1565,7 +1567,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > if (pci_bus_is_express(bus)) {
> > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > > - aml_append(dev, build_q35_osc_method());
>
>
> > > + /* Expander bridges do not have ACPI PCI Hot-plug enabled */
> > > + aml_append(dev, build_q35_osc_method(false));
>
> this's it not obvious, why PXBs aren't capable of ACPI PCI Hot-plug?
Lack of support from the original patchset.
> > > } else {
> > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> > > }
> > > --
> > > 2.31.1
> >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
2021-11-10 15:25 ` Julia Suvorova
@ 2021-11-10 15:37 ` Michael S. Tsirkin
0 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-11-10 15:37 UTC (permalink / raw)
To: Julia Suvorova
Cc: Ani Sinha, Igor Mammedov, Marcel Apfelbaum, QEMU Developers,
Gerd Hoffmann
On Wed, Nov 10, 2021 at 04:25:40PM +0100, Julia Suvorova wrote:
> On Wed, Nov 10, 2021 at 2:58 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed, 10 Nov 2021 02:21:34 -0500
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Wed, Nov 10, 2021 at 06:30:13AM +0100, Julia Suvorova wrote:
> > > > There are two ways to enable ACPI PCI Hot-plug:
> > > >
> > > > * Disable the Hot-plug Capable bit on PCIe slots.
> > > >
> > > > This was the first approach which led to regression [1-2], as
> > > > I/O space for a port is allocated only when it is hot-pluggable,
> > > > which is determined by HPC bit.
> > > >
> > > > * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC
> > > > method.
> > > >
> > > > This removes the (future) ability of hot-plugging switches with PCIe
> > > > Native hotplug since ACPI PCI Hot-plug only works with cold-plugged
> > > > bridges. If the user wants to explicitely use this feature, they can
> > > > disable ACPI PCI Hot-plug with:
> > > > --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
> > > >
> > > > Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug
> > > > instead of PCIe Native.
> > > >
> > > > [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> > > > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
> > > >
> > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > ---
> > > > hw/i386/acpi-build.c | 12 ++++++++----
> > > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index a3ad6abd33..a2f92ab32b 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -1337,7 +1337,7 @@ static void build_x86_acpi_pci_hotplug(Aml *table, uint64_t pcihp_addr)
> > > > aml_append(table, scope);
> > > > }
> > > >
> > > > -static Aml *build_q35_osc_method(void)
> > > > +static Aml *build_q35_osc_method(bool acpi_pcihp)
> > > > {
> > > > Aml *if_ctx;
> > > > Aml *if_ctx2;
> > > > @@ -1345,6 +1345,7 @@ static Aml *build_q35_osc_method(void)
> > > > Aml *method;
> > > > Aml *a_cwd1 = aml_name("CDW1");
> > > > Aml *a_ctrl = aml_local(0);
> > > > + const uint64_t hotplug = acpi_pcihp ? 0x1E : 0x1F;
> > >
> > > drop this variable and open-code at use point below.
> > > As it is the comment is misplaced.
> > > Also a slightly better way to write this:
> > > 0x1E | (acpi_pcihp ? 0x0 : 0x1)
> > >
> > >
> > >
> > > >
> > > > method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> > > > aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> > >
> > > So what acpi_pcihp does is enable/disable native pcie hotplug.
> > > How about we call the option exactly that?
> > >
> > >
> > > > @@ -1359,8 +1360,9 @@ static Aml *build_q35_osc_method(void)
> > > > /*
> > > > * Always allow native PME, AER (no dependencies)
> > > > * Allow SHPC (PCI bridges can have SHPC controller)
> > > > + * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
> > > > */
> > > > - aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
> > > > + aml_append(if_ctx, aml_and(a_ctrl, aml_int(hotplug), a_ctrl));
> > > >
> > >
> > > using the variable hotplug just made things confusing,
> > > open-coding will be clearer.
> > >
> > >
> > > > if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
> > > > /* Unknown revision */
> > > > @@ -1449,7 +1451,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > > > aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > > > aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> > > > - aml_append(dev, build_q35_osc_method());
> > > > + aml_append(dev, build_q35_osc_method(pm->pcihp_bridge_en));
> > > > aml_append(sb_scope, dev);
> > > > if (mcfg_valid) {
> > > > aml_append(sb_scope, build_q35_dram_controller(&mcfg));
> > >
> > > One of the complaints of libvirt people was that the
> > > name is confusing. It seems to say whether to describe bridges
> > > in ACPI but it also disables native hotplug, but only for PCIe.
> > >
> > > Maybe we should address this with flags saying whether to enable each of
> > > acpi,pcie,shpc hotplug separately...
> >
> > yep, mask with field defines would be better,
> > but I'd hold off such change to post 6.2 time.
> >
> > >
> > > > @@ -1565,7 +1567,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > > if (pci_bus_is_express(bus)) {
> > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> > > > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > > > - aml_append(dev, build_q35_osc_method());
> >
> >
> > > > + /* Expander bridges do not have ACPI PCI Hot-plug enabled */
> > > > + aml_append(dev, build_q35_osc_method(false));
> >
> > this's it not obvious, why PXBs aren't capable of ACPI PCI Hot-plug?
>
> Lack of support from the original patchset.
Hmm. So PXB will keep using native hotplug. Looks like a good reason
to pick up Gerd's patches to fix it up ...
> > > > } else {
> > > > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> > > > }
> > > > --
> > > > 2.31.1
> > >
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit'
2021-11-10 13:30 ` Igor Mammedov
@ 2021-11-10 15:52 ` Michael S. Tsirkin
0 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2021-11-10 15:52 UTC (permalink / raw)
To: Igor Mammedov
Cc: Marcel Apfelbaum, Daniel P. Berrangé,
Julia Suvorova, qemu-devel, Gerd Hoffmann, Ani Sinha
On Wed, Nov 10, 2021 at 02:30:27PM +0100, Igor Mammedov wrote:
> On Wed, 10 Nov 2021 09:08:34 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> > On Wed, Nov 10, 2021 at 01:04:54AM -0500, Michael S. Tsirkin wrote:
> > > On Wed, Nov 10, 2021 at 06:30:10AM +0100, Julia Suvorova wrote:
> > > > Rename the option to better represent its function - toggle Hot-Plug
> > > > Capable bit in the PCIe Slot Capability.
> > > >
> > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > ---
> > > > include/hw/pci/pcie_port.h | 2 +-
> > > > hw/i386/pc_q35.c | 2 +-
> > > > hw/pci-bridge/gen_pcie_root_port.c | 6 +++++-
> > > > hw/pci/pcie.c | 2 +-
> > > > hw/pci/pcie_port.c | 2 +-
> > > > 5 files changed, 9 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> > > > index e25b289ce8..0da6d66c95 100644
> > > > --- a/include/hw/pci/pcie_port.h
> > > > +++ b/include/hw/pci/pcie_port.h
> > > > @@ -60,7 +60,7 @@ struct PCIESlot {
> > > > /* Indicates whether any type of hot-plug is allowed on the slot */
> > > > bool hotplug;
> > > >
> > > > - bool native_hotplug;
> > > > + bool native_hpc_bit;
> > > >
> > > > QLIST_ENTRY(PCIESlot) next;
> > > > };
> > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > >
> > >
> > > This is ok.
> > >
> > >
> > > > index 797e09500b..ded61f8ef7 100644
> > > > --- a/hw/i386/pc_q35.c
> > > > +++ b/hw/i386/pc_q35.c
> > > > @@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
> > > > NULL);
> > > >
> > > > if (acpi_pcihp) {
> > > > - object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
> > > > + object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hpc-bit",
> > > > "false", true);
> > > > }
> > > >
> > >
> > >
> > > This part is problematic since we made the feature user-settable in 6.1.
> > > We can have two features if we really want to ...
> > > I don't think we have a way to mark properties deprecated, do we?
>
> describe them in deprecated.rst
>
> >
> > IMHO just leave the feature with its current name. It won't be the
> > first thing without a "perfect" name and the name doesn't have a
> > negative impact on mgmt apps. Changing the name will cause more
> > pain than it solves.
>
> looking at the code, it was introduced in 6.1 for internal needs mostly
>
> 3f3cbbb23 hw/pci/pcie: Do not set HPC flag if acpihp is used
>
> to hide slot from guest's native-hotplug module.
> Even if user tried explicitly to set native-hoplug=on on a slot
> with ACPI hotplug enabled, native hotplug will not work properly
> as slot's hotplug is still handled by acpi_pcihp_device_plug_cb().
>
> Given above and that it shouldn't been used by users,
> I'd rather rename it to x-native-hotplug now than go through deprecation
> process and expose it for 2 more releases.
>
Well what works is disabling native hotplug. Has same effect as
disabling hotplug really. Yes, I see little point
in keeping it. Technically an ABI breakage but hey.
And I would also try to backport a patch renaming it to stable
(if another stable release happens). So users don't start
using this.
> > >
> > > > diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> > > > index 20099a8ae3..ed079d72b3 100644
> > > > --- a/hw/pci-bridge/gen_pcie_root_port.c
> > > > +++ b/hw/pci-bridge/gen_pcie_root_port.c
> > > > @@ -87,7 +87,11 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
> > > > return;
> > > > }
> > > >
> > > > - if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hotplug) {
> > > > + /*
> > > > + * Make sure that IO is assigned in case the slot is hot-pluggable
> > > > + * but it is not visible through the PCIe Slot Capabilities
> > > > + */
> > > > + if (grp->res_reserve.io == -1 && s->hotplug && !s->native_hpc_bit) {
> > > > grp->res_reserve.io = GEN_PCIE_ROOT_DEFAULT_IO_RANGE;
> > > > }
> > > > int rc = pci_bridge_qemu_reserve_cap_init(d, 0,
> > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > > index 914a9bf3d1..ebe002831e 100644
> > > > --- a/hw/pci/pcie.c
> > > > +++ b/hw/pci/pcie.c
> > > > @@ -535,7 +535,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
> > > > * hot-plug is disabled on the slot.
> > > > */
> > > > if (s->hotplug &&
> > > > - (s->native_hotplug || DEVICE(dev)->hotplugged)) {
> > > > + (s->native_hpc_bit || DEVICE(dev)->hotplugged)) {
> > > > pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
> > > > PCI_EXP_SLTCAP_HPS |
> > > > PCI_EXP_SLTCAP_HPC);
> > > > diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> > > > index da850e8dde..eae06d10e2 100644
> > > > --- a/hw/pci/pcie_port.c
> > > > +++ b/hw/pci/pcie_port.c
> > > > @@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
> > > > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> > > > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> > > > DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
> > > > - DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
> > > > + DEFINE_PROP_BOOL("native-hpc-bit", PCIESlot, native_hpc_bit, true),
> > > > DEFINE_PROP_END_OF_LIST()
> > > > };
> > > >
> > > > --
> > > > 2.31.1
> > >
> > >
> >
> > Regards,
> > Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-11-10 15:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10 5:30 [PATCH 0/5] Fix Q35 ACPI PCI Hot-plug I/O issues Julia Suvorova
2021-11-10 5:30 ` [PATCH 1/5] hw/pci/pcie_port: Rename 'native-hotplug' to 'native-hpc-bit' Julia Suvorova
2021-11-10 6:04 ` Michael S. Tsirkin
2021-11-10 9:08 ` Daniel P. Berrangé
2021-11-10 13:30 ` Igor Mammedov
2021-11-10 15:52 ` Michael S. Tsirkin
2021-11-10 5:30 ` [PATCH 2/5] hw/acpi/ich9: Add compatibility option for 'native-hpc-bit' Julia Suvorova
2021-11-10 13:33 ` Igor Mammedov
2021-11-10 13:45 ` Igor Mammedov
2021-11-10 5:30 ` [PATCH 3/5] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
2021-11-10 5:30 ` [PATCH 4/5] hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC Julia Suvorova
2021-11-10 7:21 ` Michael S. Tsirkin
2021-11-10 13:57 ` Igor Mammedov
2021-11-10 15:25 ` Julia Suvorova
2021-11-10 15:37 ` Michael S. Tsirkin
2021-11-10 5:30 ` [PATCH 5/5] bios-tables-test: Update golden binaries Julia Suvorova
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.