* [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35
@ 2021-07-13 0:41 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:41 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, Michael S. Tsirkin
The patch set consists of two parts:
patches 1-3: introduce new feature
'acpi-pci-hotplug-with-bridge-support' on Q35
patches 4-6: make the feature default along with changes in ACPI tables
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)
v6:
* move acpi_pcihp_disable_root_bus() changes into "Enable ACPI
PCI hot-plug" patch
* fix mips compilation [Michael, Marcel]
* additional check in pm_reset() [David]
* rename property to "native-hotplug" [Igor]
v5:
* make sugar property on TYPE_PCIE_SLOT
instead of old TYPE_MACHINE property [Igor]
* minor style changes
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 (6):
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/pci/pcie_port.h | 5 ++-
hw/acpi/acpi-x86-stub.c | 6 +++
hw/acpi/ich9.c | 70 ++++++++++++++++++++++++++++++
hw/acpi/pcihp.c | 26 ++++++++---
hw/acpi/piix4.c | 4 +-
hw/core/machine.c | 1 -
hw/i386/acpi-build.c | 44 ++++++++++++-------
hw/i386/pc.c | 1 +
hw/i386/pc_q35.c | 11 +++++
hw/pci/pcie.c | 8 +++-
hw/pci/pcie_port.c | 1 +
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
25 files changed, 165 insertions(+), 25 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
2021-07-13 4:02 ` David Gibson
2021-07-13 0:42 ` [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, 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>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.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 4999277d57..d98a284b7a 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
@@ -488,10 +487,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 357437ff1d..e1c246d6e8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -219,10 +219,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,
@@ -238,6 +234,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). */
@@ -392,6 +392,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)));
@@ -521,7 +524,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;
}
@@ -1251,7 +1254,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;
@@ -1260,20 +1263,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));
@@ -1407,7 +1412,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 {
@@ -1455,6 +1460,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);
@@ -1489,7 +1497,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] 14+ messages in thread
* [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
2021-07-13 4:09 ` David Gibson
2021-07-13 0:42 ` [PATCH v6 3/6] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, 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>
Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/acpi-build.h | 1 +
include/hw/acpi/ich9.h | 3 ++
hw/acpi/acpi-x86-stub.c | 6 ++++
hw/acpi/ich9.c | 70 +++++++++++++++++++++++++++++++++++++++++
hw/acpi/pcihp.c | 12 +++++--
hw/i386/acpi-build.c | 14 ++++++---
6 files changed, 100 insertions(+), 6 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/acpi-x86-stub.c b/hw/acpi/acpi-x86-stub.c
index f88d6a090b..e9e46c5c5f 100644
--- a/hw/acpi/acpi-x86-stub.c
+++ b/hw/acpi/acpi-x86-stub.c
@@ -1,7 +1,13 @@
#include "qemu/osdep.h"
#include "hw/i386/pc.h"
+#include "hw/i386/acpi-build.h"
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry)
{
}
+
+Object *acpi_get_i386_pci_host(void)
+{
+ return NULL;
+}
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 4daa79ec8d..2f4eb453ac 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,10 @@ static void pm_reset(void *opaque)
}
pm->smi_en_wmask = ~0;
+ if (pm->use_acpi_hotplug_bridge) {
+ acpi_pcihp_reset(&pm->acpi_pci_hotplug, true);
+ }
+
acpi_update_sci(&pm->acpi_regs, pm->irq);
}
@@ -297,6 +322,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 +405,20 @@ 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 +427,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 +451,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 +461,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 +501,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 +536,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 +557,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 d98a284b7a..9fdc6342b0 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,11 @@ static void acpi_set_pci_info(void)
}
bsel_is_set = true;
- bus = find_i440fx(); /* TODO: Q35 support */
+ if (!host) {
+ return;
+ }
+
+ 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);
@@ -121,13 +128,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);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e1c246d6e8..bc966a4110 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -299,7 +299,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;
@@ -320,7 +320,10 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
Object *pci_host;
pci_host = acpi_get_i386_pci_host();
- g_assert(pci_host);
+
+ if (!pci_host) {
+ return;
+ }
range_set_bounds1(hole,
object_property_get_uint(pci_host,
@@ -1765,6 +1768,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
PCIBus *bus = NULL;
pci_host = acpi_get_i386_pci_host();
+
if (pci_host) {
bus = PCI_HOST_BRIDGE(pci_host)->bus;
}
@@ -2321,7 +2325,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
QObject *o;
pci_host = acpi_get_i386_pci_host();
- g_assert(pci_host);
+ if (!pci_host) {
+ return false;
+ }
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_BASE, NULL);
if (!o) {
@@ -2351,7 +2357,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
AcpiPmInfo pm;
AcpiMiscInfo misc;
AcpiMcfgInfo mcfg;
- Range pci_hole, pci_hole64;
+ Range pci_hole = {}, pci_hole64 = {};
uint8_t *u;
size_t aml_len = 0;
GArray *tables_blob = tables->table_data;
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v6 3/6] hw/pci/pcie: Do not set HPC flag if acpihp is used
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 4/6] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Julia Suvorova, David Gibson, Igor Mammedov,
David Gibson
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>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
include/hw/pci/pcie_port.h | 5 ++++-
hw/acpi/pcihp.c | 8 ++++++++
hw/core/machine.c | 1 -
hw/i386/pc_q35.c | 11 +++++++++++
hw/pci/pcie.c | 8 +++++++-
hw/pci/pcie_port.c | 1 +
6 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index bea8ecad0f..e25b289ce8 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -57,8 +57,11 @@ struct PCIESlot {
/* Disable ACS (really for a pcie_root_port) */
bool disable_acs;
- /* Indicates whether hot-plug is enabled on the slot */
+ /* Indicates whether any type of hot-plug is allowed on the slot */
bool hotplug;
+
+ bool native_hotplug;
+
QLIST_ENTRY(PCIESlot) next;
};
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 9fdc6342b0..f4d706e47d 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"
@@ -336,6 +337,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 ca69f0343a..339031219d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -583,7 +583,6 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
ms->ram_memdev_id = g_strdup(value);
}
-
static void machine_init_notify(Notifier *notifier, void *data)
{
MachineState *machine = MACHINE(qdev_get_machine());
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 46a0f196f4..04b4a4788d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -37,6 +37,7 @@
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
#include "hw/pci-host/q35.h"
+#include "hw/pci/pcie_port.h"
#include "hw/qdev-properties.h"
#include "hw/i386/x86.h"
#include "hw/i386/pc.h"
@@ -136,6 +137,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 +238,15 @@ 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);
+
+ if (acpi_pcihp) {
+ object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
+ "false", true);
+ }
+
/* 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..6e95d82903 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -529,7 +529,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 &&
+ (s->native_hotplug || 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 eb563ad435..da850e8dde 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -148,6 +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_END_OF_LIST()
};
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v6 4/6] bios-tables-test: Allow changes in DSDT ACPI tables
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (2 preceding siblings ...)
2021-07-13 0:42 ` [PATCH v6 3/6] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 6/6] bios-tables-test: Update golden binaries Julia Suvorova
5 siblings, 0 replies; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, 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>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.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] 14+ messages in thread
* [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (3 preceding siblings ...)
2021-07-13 0:42 ` [PATCH v6 4/6] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
2021-07-13 4:11 ` David Gibson
2021-07-13 7:59 ` Igor Mammedov
2021-07-13 0:42 ` [PATCH v6 6/6] bios-tables-test: Update golden binaries Julia Suvorova
5 siblings, 2 replies; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, 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:
* https://bugzilla.redhat.com/show_bug.cgi?id=1752465
* https://bugzilla.redhat.com/show_bug.cgi?id=1690256
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 | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 2f4eb453ac..778e27b659 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -427,7 +427,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 8e1220db72..7e03848792 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
{ "qemu64" "-" TYPE_X86_CPU, "family", "6" },
{ "qemu64" "-" TYPE_X86_CPU, "model", "6" },
{ "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
+ { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
};
const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v6 6/6] bios-tables-test: Update golden binaries
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
` (4 preceding siblings ...)
2021-07-13 0:42 ` [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
@ 2021-07-13 0:42 ` Julia Suvorova
5 siblings, 0 replies; 14+ messages in thread
From: Julia Suvorova @ 2021-07-13 0:42 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, David Gibson, Julia Suvorova, 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] 14+ messages in thread
* Re: [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35
2021-07-13 0:42 ` [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
@ 2021-07-13 4:02 ` David Gibson
0 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2021-07-13 4:02 UTC (permalink / raw)
To: Julia Suvorova
Cc: Igor Mammedov, David Gibson, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 9621 bytes --]
On Tue, Jul 13, 2021 at 02:42:00AM +0200, Julia Suvorova wrote:
> 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>
> Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
I'm not especially familiar with either x86 or ACPI code, so my
review's depth is according.
> ---
> 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 4999277d57..d98a284b7a 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
> @@ -488,10 +487,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 357437ff1d..e1c246d6e8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -219,10 +219,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,
> @@ -238,6 +234,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). */
> @@ -392,6 +392,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)));
> @@ -521,7 +524,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;
> }
>
> @@ -1251,7 +1254,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;
> @@ -1260,20 +1263,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));
> @@ -1407,7 +1412,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 {
> @@ -1455,6 +1460,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);
> @@ -1489,7 +1497,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));
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug
2021-07-13 0:42 ` [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
@ 2021-07-13 4:09 ` David Gibson
2021-07-13 10:02 ` Marcel Apfelbaum
0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2021-07-13 4:09 UTC (permalink / raw)
To: Julia Suvorova
Cc: Igor Mammedov, David Gibson, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]
On Tue, Jul 13, 2021 at 02:42:01AM +0200, Julia Suvorova 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>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Since it looks safe, however I think there are a couple of unnecessary
changes here:
[snip]
> @@ -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,11 @@ static void acpi_set_pci_info(void)
> }
> bsel_is_set = true;
>
> - bus = find_i440fx(); /* TODO: Q35 support */
> + if (!host) {
AFAICT acpi_get_i386_pci_host() still can't return NULL, so I'm not
sure this test is necessary.
[snip]
> -static Object *acpi_get_i386_pci_host(void)
> +Object *acpi_get_i386_pci_host(void)
> {
> PCIHostState *host;
>
> @@ -320,7 +320,10 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
> Object *pci_host;
>
> pci_host = acpi_get_i386_pci_host();
> - g_assert(pci_host);
> +
> + if (!pci_host) {
> + return;
> + }
Likewise this change.
>
> range_set_bounds1(hole,
> object_property_get_uint(pci_host,
> @@ -1765,6 +1768,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> PCIBus *bus = NULL;
>
> pci_host = acpi_get_i386_pci_host();
> +
> if (pci_host) {
> bus = PCI_HOST_BRIDGE(pci_host)->bus;
> }
> @@ -2321,7 +2325,9 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> QObject *o;
>
> pci_host = acpi_get_i386_pci_host();
> - g_assert(pci_host);
> + if (!pci_host) {
> + return false;
> + }
And this one.
>
> o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_BASE, NULL);
> if (!o) {
> @@ -2351,7 +2357,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> AcpiPmInfo pm;
> AcpiMiscInfo misc;
> AcpiMcfgInfo mcfg;
> - Range pci_hole, pci_hole64;
> + Range pci_hole = {}, pci_hole64 = {};
> uint8_t *u;
> size_t aml_len = 0;
> GArray *tables_blob = tables->table_data;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-07-13 0:42 ` [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
@ 2021-07-13 4:11 ` David Gibson
2021-07-13 7:59 ` Igor Mammedov
1 sibling, 0 replies; 14+ messages in thread
From: David Gibson @ 2021-07-13 4:11 UTC (permalink / raw)
To: Julia Suvorova
Cc: Igor Mammedov, David Gibson, qemu-devel, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]
On Tue, Jul 13, 2021 at 02:42:04AM +0200, Julia Suvorova 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:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
> * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
>
> 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>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/acpi/ich9.c | 2 +-
> hw/i386/pc.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 2f4eb453ac..778e27b659 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -427,7 +427,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 8e1220db72..7e03848792 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
> { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> };
> const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-07-13 0:42 ` [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
2021-07-13 4:11 ` David Gibson
@ 2021-07-13 7:59 ` Igor Mammedov
2021-07-13 10:23 ` Marcel Apfelbaum
2021-07-13 15:17 ` Michael S. Tsirkin
1 sibling, 2 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-07-13 7:59 UTC (permalink / raw)
To: Julia Suvorova; +Cc: David Gibson, qemu-devel, Michael S. Tsirkin
On Tue, 13 Jul 2021 02:42:04 +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.
Before we flip the switch,
has the issue about not hotplug ports not getting IO (Michael)
been addressed, if not are there any plans to fix it?
> 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:
> * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
> * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
>
> 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 | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 2f4eb453ac..778e27b659 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -427,7 +427,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 8e1220db72..7e03848792 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
> { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> };
> const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug
2021-07-13 4:09 ` David Gibson
@ 2021-07-13 10:02 ` Marcel Apfelbaum
0 siblings, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2021-07-13 10:02 UTC (permalink / raw)
To: David Gibson
Cc: Igor Mammedov, Julia Suvorova, qemu devel list, David Gibson,
Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]
Hi David,
On Tue, Jul 13, 2021 at 7:12 AM David Gibson <david@gibson.dropbear.id.au>
wrote:
> On Tue, Jul 13, 2021 at 02:42:01AM +0200, Julia Suvorova 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>
> > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> Since it looks safe, however I think there are a couple of unnecessary
> changes here:
>
>
> [snip]
> > @@ -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,11 @@ static void acpi_set_pci_info(void)
> > }
> > bsel_is_set = true;
> >
> > - bus = find_i440fx(); /* TODO: Q35 support */
> > + if (!host) {
>
> AFAICT acpi_get_i386_pci_host() still can't return NULL, so I'm not
> sure this test is necessary.
>
It can, please see the new stub in hw/acpi/acpi-x86-stub.c
Object *acpi_get_i386_pci_host(void)
{
return NULL;
}
Thanks,
Marcel
[...]
[-- Attachment #2: Type: text/html, Size: 2236 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-07-13 7:59 ` Igor Mammedov
@ 2021-07-13 10:23 ` Marcel Apfelbaum
2021-07-13 15:17 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Marcel Apfelbaum @ 2021-07-13 10:23 UTC (permalink / raw)
To: Igor Mammedov
Cc: David Gibson, Julia Suvorova, qemu devel list, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 3577 bytes --]
Hi Igor,
On Tue, Jul 13, 2021 at 10:59 AM Igor Mammedov <imammedo@redhat.com> wrote:
> On Tue, 13 Jul 2021 02:42:04 +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.
>
> Before we flip the switch,
> has the issue about not hotplug ports not getting IO (Michael)
> been addressed, if not are there any plans to fix it?
>
>
Following an offline discussion with Michael the understanding is
PCIe devices should work correctly without IO, so it only affects PCI
devices hot-plugged into PCIe ports, but those devices should be plugged
into pcie-pci bridges.
Another alternative is to drop this patch and let ACPI hotplug be an option
and not the default until we find a solution, meaning delaying the flip
until 6.2 since today is the soft-freeze.
I will let Michael decide, I am fine one way or the other.
Thanks,
Marcel
>
> > 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:
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
> >
> > 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 | 1 +
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 2f4eb453ac..778e27b659 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -427,7 +427,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 8e1220db72..7e03848792 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
> > { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> > { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> > + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> > };
> > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
> >
>
>
[-- Attachment #2: Type: text/html, Size: 5346 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35
2021-07-13 7:59 ` Igor Mammedov
2021-07-13 10:23 ` Marcel Apfelbaum
@ 2021-07-13 15:17 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-07-13 15:17 UTC (permalink / raw)
To: Igor Mammedov; +Cc: David Gibson, Julia Suvorova, qemu-devel
On Tue, Jul 13, 2021 at 09:59:31AM +0200, Igor Mammedov wrote:
> On Tue, 13 Jul 2021 02:42:04 +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.
>
> Before we flip the switch,
> has the issue about not hotplug ports not getting IO (Michael)
> been addressed, if not are there any plans to fix it?
>
I think it's a guest bug frankly. We'll workaround it
by setting io-reserve to 4k for hotplugged bridges,
I think this is minor enough that it's better to just
merge now and fix on top.
I've added this note to the commit log though.
> > 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:
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
> > * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
> >
> > 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 | 1 +
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 2f4eb453ac..778e27b659 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -427,7 +427,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 8e1220db72..7e03848792 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
> > { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> > { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> > + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> > };
> > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-07-13 15:18 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 0:41 [PATCH v6 0/6] Use ACPI PCI hot-plug for Q35 Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 1/6] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35 Julia Suvorova
2021-07-13 4:02 ` David Gibson
2021-07-13 0:42 ` [PATCH v6 2/6] hw/acpi/ich9: Enable ACPI PCI hot-plug Julia Suvorova
2021-07-13 4:09 ` David Gibson
2021-07-13 10:02 ` Marcel Apfelbaum
2021-07-13 0:42 ` [PATCH v6 3/6] hw/pci/pcie: Do not set HPC flag if acpihp is used Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 4/6] bios-tables-test: Allow changes in DSDT ACPI tables Julia Suvorova
2021-07-13 0:42 ` [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 Julia Suvorova
2021-07-13 4:11 ` David Gibson
2021-07-13 7:59 ` Igor Mammedov
2021-07-13 10:23 ` Marcel Apfelbaum
2021-07-13 15:17 ` Michael S. Tsirkin
2021-07-13 0:42 ` [PATCH v6 6/6] 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.