* [RFC 0/5] pc: support user provided NIC naming/indexing
@ 2020-12-22 23:39 Igor Mammedov
2020-12-22 23:39 ` [RFC 1/5] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jusual, laine, mst
Series implements support for 'onboard' naming scheme for network
interfaces (1), which is based on PCI firmware spec and lets user
to explicitly specify index that will be used by guest to name
network interface, ex:
-device e1000,acpi-index=33
should make guest rename NIC name to 'eno33' where 'eno' is default
prefix for this scheme.
Hope is that it will allow to simplify launching VMs from
template disk images with different set VM configurations
without need to reconfigure guest network intrfaces or
risk of loosing network connectivity.
For more detailed description/examples see patches [3-4/5]
1)
https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
Git repo for testing:
https://github.com/imammedo/qemu/branches acpi-index-rfc
Igor Mammedov (5):
acpi: add aml_to_decimalstring() and aml_call6() helpers
tests: acpi: temporary whitelist DSDT changes
pci: introduce apci-index property for PCI device
pci: acpi: add _DSM method to PCI devices
tests: acpi: update expected data files
include/hw/acpi/aml-build.h | 3 +
include/hw/acpi/pci.h | 1 +
include/hw/acpi/pcihp.h | 7 +-
include/hw/pci/pci.h | 1 +
tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++
hw/acpi/aml-build.c | 28 +++++++
hw/acpi/pci.c | 84 ++++++++++++++++++++
hw/acpi/pcihp.c | 25 +++++-
hw/i386/acpi-build.c | 31 +++++++-
hw/pci/pci.c | 1 +
tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
31 files changed, 197 insertions(+), 5 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC 1/5] acpi: add aml_to_decimalstring() and aml_call6() helpers
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
@ 2020-12-22 23:39 ` Igor Mammedov
2020-12-22 23:39 ` [RFC 2/5] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jusual, laine, mst
it will be used by follow up patches
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/aml-build.h | 3 +++
hw/acpi/aml-build.c | 28 ++++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index e727bea1bc..695b7e6323 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -301,6 +301,7 @@ Aml *aml_arg(int pos);
Aml *aml_to_integer(Aml *arg);
Aml *aml_to_hexstring(Aml *src, Aml *dst);
Aml *aml_to_buffer(Aml *src, Aml *dst);
+Aml *aml_to_decimalstring(Aml *src, Aml *dst);
Aml *aml_store(Aml *val, Aml *target);
Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst);
Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst);
@@ -323,6 +324,8 @@ Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
Aml *arg5);
+Aml *aml_call6(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+ Aml *arg5, Aml *arg6);
Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
AmlLevelAndEdge edge_level,
AmlActiveHighAndLow active_level, AmlShared shared,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f976aa667b..89702be788 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -633,6 +633,19 @@ Aml *aml_to_buffer(Aml *src, Aml *dst)
return var;
}
+/* ACPI 2.0a: 17.2.4.4 Type 2 Opcodes Encoding: DefToDecimalString */
+Aml *aml_to_decimalstring(Aml *src, Aml *dst)
+{
+ Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);
+ aml_append(var, src);
+ if (dst) {
+ aml_append(var, dst);
+ } else {
+ build_append_byte(var->buf, 0x00 /* NullNameOp */);
+ }
+ return var;
+}
+
/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefStore */
Aml *aml_store(Aml *val, Aml *target)
{
@@ -834,6 +847,21 @@ Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
return var;
}
+/* helper to call method with 5 arguments */
+Aml *aml_call6(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+ Aml *arg5, Aml *arg6)
+{
+ Aml *var = aml_alloc();
+ build_append_namestring(var->buf, "%s", method);
+ aml_append(var, arg1);
+ aml_append(var, arg2);
+ aml_append(var, arg3);
+ aml_append(var, arg4);
+ aml_append(var, arg5);
+ aml_append(var, arg6);
+ return var;
+}
+
/*
* ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor
* Type 1, Large Item Name 0xC
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 2/5] tests: acpi: temporary whitelist DSDT changes
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
2020-12-22 23:39 ` [RFC 1/5] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
@ 2020-12-22 23:39 ` Igor Mammedov
2020-12-22 23:39 ` [RFC 3/5] pci: introduce apci-index property for PCI device Igor Mammedov
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jusual, laine, mst
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..5c695cdf37 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,22 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT",
+"tests/data/acpi/pc/DSDT.acpihmat",
+"tests/data/acpi/pc/DSDT.bridge",
+"tests/data/acpi/pc/DSDT.cphp",
+"tests/data/acpi/pc/DSDT.dimmpxm",
+"tests/data/acpi/pc/DSDT.hpbridge",
+"tests/data/acpi/pc/DSDT.hpbrroot",
+"tests/data/acpi/pc/DSDT.ipmikcs",
+"tests/data/acpi/pc/DSDT.memhp",
+"tests/data/acpi/pc/DSDT.numamem",
+"tests/data/acpi/pc/DSDT.roothp",
+"tests/data/acpi/q35/DSDT",
+"tests/data/acpi/q35/DSDT.acpihmat",
+"tests/data/acpi/q35/DSDT.bridge",
+"tests/data/acpi/q35/DSDT.cphp",
+"tests/data/acpi/q35/DSDT.dimmpxm",
+"tests/data/acpi/q35/DSDT.ipmibt",
+"tests/data/acpi/q35/DSDT.memhp",
+"tests/data/acpi/q35/DSDT.mmio64",
+"tests/data/acpi/q35/DSDT.numamem",
+"tests/data/acpi/q35/DSDT.tis",
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 3/5] pci: introduce apci-index property for PCI device
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
2020-12-22 23:39 ` [RFC 1/5] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
2020-12-22 23:39 ` [RFC 2/5] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
@ 2020-12-22 23:39 ` Igor Mammedov
2020-12-22 23:39 ` [RFC 4/5] pci: acpi: add _DSM method to PCI devices Igor Mammedov
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, jusual, laine, mst
In x86/ACPI world, since systemd v197, linux distros are
using predictable network interface naming since systemd
v197. Which on QEMU based VMs results into path based
naming scheme, that names network interfaces based on PCI
topology.
With this one has to plug NIC in exacly the same bus/slot,
which was used when disk image was first provisioned/configured
or one risks to loose network configuration due to NIC being
renamed to actually used topology.
That also restricts freedom reshape PCI configuration of
VM without need to reconfigure used guest image.
systemd also offers "onboard" naming scheme which is
preffered over PCI slot/topology one, provided that
firmware implements:
"
PCI Firmware Specification 3.1
4.6.7. DSM for Naming a PCI or PCI Express Device Under
Operating Systems
"
that allows to assign user defined index to PCI device,
which systemd will use to name NIC. For example, using
-device e1000,acpi-index=100
guest will rename NIC to 'eno100', where 'eno' is default
prefix for "onboard" naming scheme. This doesn't reqiure
any advance configuration on guest side.
Hope is that 'acpi-index' will be easier to consume by
mangment layer, compared to forcing specic PCI topology
and/or having several disk image templates for different
topologies and will help to simplify process of spawning
VM from the same template without need to reconfigure
guest network configuration.
this patch adds, 'acpi-index'* property and wires up
(abuses) unused pci hotplug registers to pass index
value to AML code at runtime.
Following patch will add corresponding _DSM code and
wire it up to PCI devices described in ACPI.
*) name comes from linux kernel terminology
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: libvir-list@redhat.com
include/hw/acpi/pcihp.h | 7 ++++++-
include/hw/pci/pci.h | 1 +
hw/acpi/pci.c | 6 ++++++
hw/acpi/pcihp.c | 25 ++++++++++++++++++++++++-
hw/i386/acpi-build.c | 10 ++++++++++
hw/pci/pci.c | 1 +
6 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index dfd375820f..72d1773ca1 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -46,6 +46,7 @@ typedef struct AcpiPciHpPciStatus {
typedef struct AcpiPciHpState {
AcpiPciHpPciStatus acpi_pcihp_pci_status[ACPI_PCIHP_MAX_HOTPLUG_BUS];
uint32_t hotplug_select;
+ uint32_t acpi_index;
PCIBus *root;
MemoryRegion io;
bool legacy_piix;
@@ -71,6 +72,8 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off);
extern const VMStateDescription vmstate_acpi_pcihp_pci_status;
+bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id);
+
#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp) \
VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
test_pcihp), \
@@ -78,6 +81,8 @@ extern const VMStateDescription vmstate_acpi_pcihp_pci_status;
ACPI_PCIHP_MAX_HOTPLUG_BUS, \
test_pcihp, 1, \
vmstate_acpi_pcihp_pci_status, \
- AcpiPciHpPciStatus)
+ AcpiPciHpPciStatus), \
+ VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
+ vmstate_acpi_pcihp_use_acpi_index)
#endif
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 259f9c992d..e592532558 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -357,6 +357,7 @@ struct PCIDevice {
/* ID of standby device in net_failover pair */
char *failover_pair_id;
+ uint32_t acpi_index;
};
void pci_register_bar(PCIDevice *pci_dev, int region_num,
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index 9510597a19..07d5101d83 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -27,6 +27,7 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/pci.h"
#include "hw/pci/pcie_host.h"
+#include "hw/acpi/pcihp.h"
void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
{
@@ -59,3 +60,8 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
"MCFG", table_data->len - mcfg_start, 1, NULL, NULL);
}
+bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
+{
+ AcpiPciHpState *s = opaque;
+ return s->acpi_index;
+}
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 9dc4d3e2db..9634567e3a 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -347,7 +347,8 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
trace_acpi_pci_down_read(val);
break;
case PCI_EJ_BASE:
- /* No feature defined yet */
+ val = s->acpi_index;
+ s->acpi_index = 0;
trace_acpi_pci_features_read(val);
break;
case PCI_RMV_BASE:
@@ -367,8 +368,30 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
static void pci_write(void *opaque, hwaddr addr, uint64_t data,
unsigned int size)
{
+ int slot;
+ PCIBus *bus;
+ BusChild *kid, *next;
AcpiPciHpState *s = opaque;
+
+ s->acpi_index = 0;
switch (addr) {
+ case PCI_UP_BASE:
+ slot = ctz32(data);
+
+ if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
+ break;
+ }
+
+ bus = acpi_pcihp_find_hotplug_bus(s, s->hotplug_select);
+ QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
+ Object *o = OBJECT(kid->child);
+ PCIDevice *dev = PCI_DEVICE(o);
+ if (PCI_SLOT(dev->devfn) == slot) {
+ s->acpi_index = object_property_get_uint(o, "acpi-index", NULL);
+ break;
+ }
+ }
+ break;
case PCI_EJ_BASE:
if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
break;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f18b71dea9..27d2958e25 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1132,6 +1132,16 @@ static void build_piix4_pci_hotplug(Aml *table)
aml_append(method, aml_return(aml_int(0)));
aml_append(scope, method);
+ method = aml_method("AIDX", 2, AML_NOTSERIALIZED);
+ aml_append(method, aml_acquire(aml_name("BLCK"), 0xFFFF));
+ aml_append(method, aml_store(aml_arg(0), aml_name("BNUM")));
+ aml_append(method,
+ aml_store(aml_shiftleft(aml_int(1), aml_arg(1)), aml_name("PCIU")));
+ aml_append(method, aml_store(aml_name("B0EJ"), aml_local(0)));
+ aml_append(method, aml_release(aml_name("BLCK")));
+ aml_append(method, aml_return(aml_local(0)));
+ aml_append(scope, method);
+
aml_append(table, scope);
}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d4349ea577..617f48ff3b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -76,6 +76,7 @@ static Property pci_props[] = {
QEMU_PCIE_EXTCAP_INIT_BITNR, true),
DEFINE_PROP_STRING("failover_pair_id", PCIDevice,
failover_pair_id),
+ DEFINE_PROP_UINT32("acpi-index", PCIDevice, acpi_index, -1),
DEFINE_PROP_END_OF_LIST()
};
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 4/5] pci: acpi: add _DSM method to PCI devices
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
` (2 preceding siblings ...)
2020-12-22 23:39 ` [RFC 3/5] pci: introduce apci-index property for PCI device Igor Mammedov
@ 2020-12-22 23:39 ` Igor Mammedov
2021-01-13 12:13 ` Michael S. Tsirkin
2021-01-26 11:16 ` Michael S. Tsirkin
2020-12-22 23:39 ` [RFC 5/5] tests: acpi: update expected data files Igor Mammedov
2021-01-13 12:09 ` [RFC 0/5] pc: support user provided NIC naming/indexing Michael S. Tsirkin
5 siblings, 2 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jusual, laine, mst
Implement _DSM according to:
PCI Firmware Specification 3.1
4.6.7. DSM for Naming a PCI or PCI Express Device Under
Operating Systems
and wire it up to cold and hot-plugged PCI devices.
Feature depends on ACPI hotplug being enabled (as that provides
PCI devices descriptions in ACPI and MMIO registers that are
reused to fetch acpi-index).
acpi-index should work for
- cold plugged NICs:
$QEMU -evice e1000,acpi-index=100
=> 'eno100'
- hot-plugged
(monitor) device_add e1000,acpi-index=200,id=remove_me
=> 'eno200'
- replugged
(monitor) device_del remove_me
(monitor) device_add e1000,acpi-index=1
=> 'eno1'
Windows also sees index under "PCI Label Id" field in properties
dialog but otherwise it doesn't seem to have any effect.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/acpi/pci.h | 1 +
hw/acpi/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++
hw/i386/acpi-build.c | 21 ++++++++++--
3 files changed, 97 insertions(+), 3 deletions(-)
diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
index bf2a3ed0ba..5e1eb2a96a 100644
--- a/include/hw/acpi/pci.h
+++ b/include/hw/acpi/pci.h
@@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
} AcpiMcfgInfo;
void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
+Aml *aml_pci_device_dsm(void);
#endif
diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
index 07d5101d83..6d49d515d3 100644
--- a/hw/acpi/pci.c
+++ b/hw/acpi/pci.c
@@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
AcpiPciHpState *s = opaque;
return s->acpi_index;
}
+
+Aml *aml_pci_device_dsm(void)
+{
+ Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
+ Aml *acpi_index = aml_local(0);
+ Aml *zero = aml_int(0);
+ Aml *bnum = aml_arg(4);
+ Aml *sun = aml_arg(5);
+
+ method = aml_method("PDSM", 6, AML_SERIALIZED);
+
+ /*
+ * PCI Firmware Specification 3.1
+ * 4.6. _DSM Definitions for PCI
+ */
+ UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
+ ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+ {
+ aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
+ ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
+ {
+ uint8_t byte_list[1];
+
+ ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
+ {
+ /*
+ * advertise function 7 if device has acpi-index
+ */
+ ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
+ {
+ byte_list[0] =
+ 1 /* have supported functions */ |
+ 1 << 7 /* support for function 7 */
+ ;
+ aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
+ }
+ aml_append(ifctx2, ifctx3);
+ }
+ aml_append(ifctx1, ifctx2);
+
+ byte_list[0] = 0; /* nothing supported */
+ aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
+ }
+ aml_append(ifctx, ifctx1);
+ elsectx = aml_else();
+ /*
+ * PCI Firmware Specification 3.1
+ * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
+ * Operating Systems
+ */
+ ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
+ {
+ Aml *pkg = aml_package(2);
+ Aml *label = aml_local(2);
+ Aml *ret = aml_local(1);
+
+ aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
+ aml_to_decimalstring(acpi_index, NULL), label));
+
+ aml_append(pkg, zero);
+ aml_append(pkg, aml_string("placeholder"));
+ aml_append(ifctx1, aml_store(pkg, ret));
+ /*
+ * update apci-index to actual value
+ */
+ aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
+ /*
+ * update device label to actual value
+ */
+ aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
+ aml_append(ifctx1, aml_return(ret));
+ }
+ aml_append(elsectx, ifctx1);
+ aml_append(ifctx, elsectx);
+ }
+ aml_append(method, ifctx);
+ return method;
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 27d2958e25..447ad39c35 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
);
aml_append(dev, method);
+ method = aml_method("_DSM", 4, AML_SERIALIZED);
+ aml_append(method,
+ aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
+ aml_arg(2), aml_arg(3),
+ aml_name("BSEL"), aml_name("_SUN")))
+ );
+ aml_append(dev, method);
aml_append(parent_scope, dev);
build_append_pcihp_notify_entry(notify_method, slot);
@@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+ aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
+ method = aml_method("_DSM", 4, AML_SERIALIZED);
+ aml_append(method,
+ aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
+ aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
+ );
+ aml_append(dev, method);
+
if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
/* add VGA specific AML methods */
int s3d;
@@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
aml_append(method, aml_return(aml_int(s3d)));
aml_append(dev, method);
} else if (hotplug_enabled_dev) {
- /* add _SUN/_EJ0 to make slot hotpluggable */
- aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
-
+ /* add _EJ0 to make slot hotpluggable */
method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
aml_append(method,
aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
@@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
aml_append(method, aml_return(aml_local(0)));
aml_append(scope, method);
+ aml_append(scope, aml_pci_device_dsm());
+
aml_append(table, scope);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC 5/5] tests: acpi: update expected data files
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
` (3 preceding siblings ...)
2020-12-22 23:39 ` [RFC 4/5] pci: acpi: add _DSM method to PCI devices Igor Mammedov
@ 2020-12-22 23:39 ` Igor Mammedov
2021-01-13 12:09 ` [RFC 0/5] pc: support user provided NIC naming/indexing Michael S. Tsirkin
5 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2020-12-22 23:39 UTC (permalink / raw)
To: qemu-devel; +Cc: jusual, laine, mst
Expected change is addition of
AIDX and PDSM methods at PCI0 scope and
_DSM methods (+ _SUN field where it was missing) to each PCI device slot
@@ -277,6 +277,54 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
Release (BLCK)
Return (Zero)
}
+
+ Method (AIDX, 2, NotSerialized)
+ {
+ Acquire (BLCK, 0xFFFF)
+ BNUM = Arg0
+ PCIU = (One << Arg1)
+ Local0 = B0EJ /* \_SB_.PCI0.B0EJ */
+ 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))
+ {
+ Return (Buffer (One)
+ {
+ 0x81 // .
+ })
+ }
+ }
+
+ Return (Buffer (One)
+ {
+ 0x00 // .
+ })
+ }
+ ElseIf ((Arg2 == 0x07))
+ {
+ Concatenate ("PCI Device ", ToDecimalString (Local0), Local2)
+ Local1 = Package (0x02)
+ {
+ Zero,
+ "placeholder"
+ }
+ Local1 [Zero] = Local0
+ Local1 [One] = Local2
+ Return (Local1)
+ }
+ }
+ }
}
Scope (_SB)
@@ -883,11 +931,22 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
Device (S00)
{
Name (_ADR, Zero) // _ADR: Address
+ Name (_SUN, Zero) // _SUN: Slot User Number
+ Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ {
+ Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+ }
}
[...]
@@ -912,6 +971,11 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
{
PCEJ (BSEL, _SUN)
}
+
+ Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
+ {
+ Return (PDSM (Arg0, Arg1, Arg2, Arg3, BSEL, _SUN))
+ }
}
[...]
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
21 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/tests/data/acpi/pc/DSDT b/tests/data/acpi/pc/DSDT
index f6173df1d598767a79aa34ad7585ad7d45c5d4f3..b9afc0a458d77b7346fca9cd2d8797a4d4ed8302 100644
GIT binary patch
delta 1298
zcmX@9-mcH(66_MvF3!NfxMw5R9wsJd;mIeM+UiBL9X(wln4*=Pe4M@i|1ZdJ@(cAX
zsASA!2ypfcEpRe$^-73V2T3kT5b<LVa0&M1Tj0h%IYUsGOW=a}$)~}lRlXC&U-?QN
zcJ?g*nU#~9us~{Z7Q+I8$(dYC3%DjtPGDFf$jsOXA{Z80EwGrJ#l>FC3p8KBMJKf^
zGdWd(VR`~XQh|^F69aESPGWLuMt)98Y7s+XK|)6&14BVlM<OG`lElq^%w?SQ9t&gQ
zgPr0992NpiFyL?scJ*P1Zqf`kFo@@fcXSD2;D`?n^<xkbj|aMyX9>u~8JStxAax*#
z=q4|gU_+={9tH*`h<Yw2TpC1x<{7#$EMWjKj9dhuEMt&RbdwXyWD#Bs8C;sd3hW6f
z$Rea*Dj@|22q}0?NP!q10be*0QjkMP!E{0j4iQrDmXHDoegeL5CZr&bkb;?n6dWO>
z;5{J)QUU~g;YvtB0U-sm2`M;6NWn)!3S<Nc_`;o#f+9i+<`PnHLQs<*tsvO^LhvCY
E0E_>Hx&QzG
delta 321
zcmZqIKdH{;66_LkQka2(aluBeJxoj<9FtElwQc^%T*hhSS`ZT->=ZBHn;+opX~5wW
z?CQf1-NYSiU=Ys{@8}W)<Y)yOLODDP3``7@@AJwsYEJ>HB)Iq&eA%4??#==~Gae
zj~^l)1f{1z>03}*P5>gG1f`cj=~qx%PY@zs1f{n@>0eOVP6#631f`EbX)a-ixF3|B
h1f{P*X(<tid>oWs1f`!rX)RHRe4eQ0=JldC7y$$2T}A)^
diff --git a/tests/data/acpi/pc/DSDT.acpihmat b/tests/data/acpi/pc/DSDT.acpihmat
index 67f3f7249eaaa9404ebf0f2d0a324b8c8e3bd445..806bcbee30dc38117fbb37f433bfbcec171ce34b 100644
GIT binary patch
delta 1298
zcmexnxW$soCD<ioiwpw;W93G!Jxol_!jn%hwbhGgJ9@fAFhwgn`8a$3|6h>d<QM8&
zP|29d5a8??THs{h>Xi_!4w77wAmYa!;1cZ1x4?~ka)zKVm%s({lTU+9t9&Plzw(tl
z?Ce_rGAk!HVS&`-EQSRFlQX%P7I00PoWQU|keRU&L@+G0T3|6bi;KOO7ihkMi%x1;
zW^$?m!}J7(qyixUCI;SuoW$hRjQpIG)FOt&f`pDl28M#9jzmU=C5fB;m>2Wadn}BJ
z4|a+da99X5!GOal*wu$2x=AzGz#yI@-q9t9fg?UR)Q>?#JRay)o+Tg`XJlq&gVcc}
zqMN)}f(@Z+c^DX&AnLi8aA^<$nrG<3u!I4`Fme%qvW!7O(M?V)lO2ULWN>K)E65?F
zU^*cMhX^TnOGtr)Fabw86H<^zNWn}(3XTv`@SczYDG>s`a3!RmfRKXOgcKYjq~IeV
z1u~)peBn+=K@lMZa|tOpK}f-8LJH)>2>8O2kb)9I3g#11aEg$EuVR|~Xa&J$QSpb2
E0B6*PDgXcg
delta 321
zcmdmD`OT2aCD<k8n*;*`W9&w*Jxoj<9FtElwQc^%yqM3(wIC)w*ePDXH$TAH(}2S%
z*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>C)^9ai_YEIS`)?kiq(wv+Rr8h(A?@-!W
z1R`G#r4K`Cc2S79H<a#&(wCvMxEMq}8cNTH(vP9Ex;R8W8%nQ-(x0KUxdcSM8cOen
g(#(<&ad#-)4W-XRX<;ddd^nVzEv32nj?@iC0Ji^J>Hq)$
diff --git a/tests/data/acpi/pc/DSDT.bridge b/tests/data/acpi/pc/DSDT.bridge
index 643390f4c4138b37fc481656d3f555d0eeedcb02..fd0c98e8adacada8252ed74f887773feb275bfc8 100644
GIT binary patch
delta 2372
zcmeA%`{>N&66_N4QIUaxF>fQ+9wsJd;mIeM+UiBL9X(wln4*=Pe4M@i|1ZdJ@(cAX
zsASA!2ypfcEpRe$^-73V2T3kT5b<LVa0&M1Tj0h%IYUsGOW=a}$)~}lRlXC&U-?QN
zcJ?g*nU#~9us~{Z7Q+I8$(dYC3%DjtPGDFf$jsOXA{Z80EwGrJ#l>FC3p8KBMJKf^
zGdWd(VR`~XQh|^F69aESPGWLuMt)98Y7s+XK|)6&14BVlM<OG`lElq^%w?SQO&*Lf
z@xe~<0-f%RKr;+DoPu3_7^0grgAEMgIpQ5%f*3gBgG2ooM8xBPuH{(*a&tyzRyIf-
zNFut)izV0)s+Na=feE6XiwTzo5uka7E(}W;KnxEf7|R&Mh;H&-6>Mk$u^4C=Gt@9<
zT!w+2%NX5cGg*gMSq7J?=q5*&$w9p8coaCXOs?V8z^`CAAqA}>1WedKNWpDF3fM#m
zSYb#=K@1@U?SvHk=Of^TyM#>O5F=nh13v);afD3hB&2{*fPfYE37NnpPQV0HLJATH
zDd;AoU>hL?4+$yYks#m;b3zK%3lcD)myiiN2q`cSB47odq$WRF3AMReh?9v4n6XE*
bC9sqRhQnyKgoMLrwuFSk0A))($$N|d2}8v<
delta 595
zcmez9++)V&66_MfBhA3T_+lg19wsIaj>#vO+BW}WF5@)vcoGvI>=ZBH@Fc+5(}2S%
z*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>C)Kjf8_^e+oGw1B7p3NkZ*H8MswsZ2KG
zQ)Z5C(wJPzrw*hwC-30X0Mmb<v;#kc-vXsiKxrNUh<E^$o&u$BKxr94h<pN+UIL|G
zKxrKzh<pK*-U6k6KxrFch<pQ-J_4mVL?GfmP<jHCz5=BsL?QAqP<jEBegdU6#31rH
WVw#KBiZL;2P9DZS+$^JTl@S19fvV&H
diff --git a/tests/data/acpi/pc/DSDT.cphp b/tests/data/acpi/pc/DSDT.cphp
index 1ddcf7d8812f5d8d4d38fe7e7b35fd5885806046..8f453a59f297be7a2afaa8b8d9dbd9a45dc20fdd 100644
GIT binary patch
delta 1298
zcmbQKecg!5CD<h-T#|u-QDGz39wsJd;mIeM+UiBL9X(wln4*=Pe4M@i|1ZdJ@(cAX
zsASA!2ypfcEpRe$^-73V2T3kT5b<LVa0&M1Tj0h%IYUsGOW=a}$)~}lRlXC&U-?QN
zcJ?g*nU#~9us~{Z7Q+I8$(dYC3%DjtPGDFf$jsOXA{Z80EwGrJ#l>FC3p8KBMJKf^
zGdWd(VR`~XQh|^F69aESPGWLuMt)98Y7s+XK|)6&14BVlM<OG`lElq^%x>KE9t&gQ
zgPr0992NpiFyL?scJ*P1Zqf`kFo@@fcXSD2;D`?n^<xkbj|aMyX9>u~8JStxAax*#
z=q4|gU_+={9tH*`h<Yw2TpC1x<{7#$EMWjKj9dhuEMt&RbdwXy<R^R@GPpE@74Y*D
zP+&z!K`J2y69_5TO-R8rLJ9;02>8N=kb-nV3MLU!u$Pd67lafD3li{!9U%pogcM96
zq+mZG1+NGx5EUZe3kO09vI!}eMo7UyLJHmxQXno&z!y%06yy?8Fhf|AAFUwRyhHdQ
FBLHotgn$45
delta 321
zcmca^G*g?)CD<iorYHjgBg;mvJxoj<9FtElwQc^%?8a^6S`ZT->=ZBHn;+opX~5wW
z?CQf1-NYSiU=Ys{@8}W)<Y)yOLODDP3``7@*YnFVYEC}KufZJMq&Zng078dB=~+<v
z9+XxRgvh5s=~Ynr9h5c_g2<Oa>0MCzACz_yhRC-;=~GaePXr<!1f{1z>03}*P81@a
g1f`cj=~qx%PYfbo1f{n@>0eOVPF!<yz4#4A0QJ*c-2eap
diff --git a/tests/data/acpi/pc/DSDT.dimmpxm b/tests/data/acpi/pc/DSDT.dimmpxm
index c44385cc01879324738ffb7f997b8cdd762cbf97..0dc9455c519a0a9116ba1f34026a9bdbabf6575e 100644
GIT binary patch
delta 1288
zcmdmQ^4FTnCD<k8uPg%tBiBZ*Jxol_!jn%hwbhGgJ9@fAFhwgn`8a$3|6h>d<QM8&
zP|29d5a8??THs{h>Xi_!4w77wAmYa!;1cZ1x4?~ka)zKVm%s({lTU+9t9&Plzw(tl
z?Ce_rGAk!HVS&`-EQSRFlQX%P7I00PoWQU|keRU&L@+G0T3|6bi;KOO7ihkMi%x1;
zW^$?m!}J7(qyixUCI;SuoW$hRjQpIG)FOt&f`pDl28M#9jzmU=C5fB;nBVZ%dn}BJ
z4|a+da99X5!GOal*wu$2x=AzGz#yI@-q9t9fg?UR)Q>?#JRay)o+Tg`XJlq&gVcc}
zqMN)}f(@Z+c^DX&AnLi8aA^<$nrG<3u!I4`Fmjn(EzHN{#4@={SVIPvmgpv+f~|xU
zJRqciTZDiWW`q<Z5>n7ZNWpeO3LX(sz$;3?7Z!vRBok85M@Yd=LJFP`Qot`pz!z48
z6r>VTFoBSQ-Gmf8Bcwo3oPaNE2q{P>q+k*u1$zl8ctJ>kumk~L*hy&eqh<fiaS{(1
E0q3lP(*OVf
delta 345
zcmexsz2Ah(CD<jzUW$Q%@$E*gJxoj<9FtElwQc^%{DwckwIC)w*ePDXH$TAH(}2S%
z*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>DDIO2m1T^N>t8AdRMF^Cb}q&c}nM1vVb
z?|{;OptOT1M7#w`pMcUlVi55FC_M#A-+<CG;t=@+D7^$qzkt#@5)k<UD7^(r|A5jq
qk`Va@D18J<b4WqNeW3IND18M=OGrcHW1#c`DE&lQbF-?<4MqTmFJrv`
diff --git a/tests/data/acpi/pc/DSDT.hpbridge b/tests/data/acpi/pc/DSDT.hpbridge
index 4ecf1eb13bf49499f729b53a6d0114672a76e28d..78f97cbda1c17f3e09cc4332e87d473fe99646b2 100644
GIT binary patch
delta 1312
zcmZ3a{!EX{CD<h-O`L&&v3w)f9wsJd;mIeM+UiBL9X(wln4*=Pe4M@i|1ZdJ@(cAX
zsASA!2ypfcEpRe$^-73V2T3kT5b<LVa0&M1Tj0h%IYUsGOW=a}$)~}lRlXC&U-?QN
zcJ?g*nU#~9us~{Z7Q+I8$(dYC3%DjtPGDFf$jsOXA{Z80EwGrJ#l>FC3p8KBMJKf^
zGdWd(VR`~XQh|^F69aESPGWLuMt)98Y7s+XK|)6&14BVlM<OG`lElq^%w?SQ?(<{f
zgPr09{O18pFyL?scJ*P1Zqf`kFo@@fcXSD2;D`?n^<xkbj|aMyX9>u~8JStxAax*#
z=q4|gU_+={9tH*`h<Yw2TpC1x<{7#$EMWjKj9eU`EMt&Rbdz4Np#{WVpjpgNvzT$2
z72V{-GFg>ZLk6#c0743?2`N}cNWn!y3jPpMpw36Y7eRy+)Dlv#f{=pCgcSTEq(GCO
zfG<J_DX1r;U=<++R|zPXBtXE8+JqE@5mL}dNWmIH3a%4Uz$8e(7rKNLL=aNYOh~~x
PK}~+NVqx<+!7Gdaeocpj
delta 330
zcmaE+w@97KCD<iokuU=T<IRm+dzhF!I3}N9YTNvixs21uFE=JW*ePDXEjPf~(}2S%
z*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>C)-{+MT;tw{ofG7ZpF#{E8PS)eoU<T1e
zP<k7b{spD&_#yI5Q2H2@<`RI2`$6eRQ2H8_mJ)=>$3f{uQ2H5^))Ioq=RxUBQ2HB`
mwi1TO*Fot+P?}8yBJKsH`=InCC@m%mk&hD9+&o+KG$R1zbY0H?
diff --git a/tests/data/acpi/pc/DSDT.hpbrroot b/tests/data/acpi/pc/DSDT.hpbrroot
index a3046226ec1dcb234b726029b3790dfedb3b9221..4f930ed9644a4018da96ef3e5dbedcb89ef19ee0 100644
GIT binary patch
delta 177
zcmeB?cqzf<66_L^$-}_FsJfAB6?c{^cT9Y+Q@nsLSAespL3ER5uz^85N4%p;5Ccbi
zaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&?2{weP1)9eMQP0JMOT**`+_HLl!G;zP
O<AGY3p<0-6X#oHg0xpgK
delta 84
zcmaDU(Idg-66_Mf!^6P9*tn5v6}OHDYfOBwQ@nr!Yk;$-L39&$uz^85N4%p;5JPm6
cR<I$I1JuC8Fj<aAmY+Y^&;p_WD8|eH0A{ojZ~y=R
diff --git a/tests/data/acpi/pc/DSDT.ipmikcs b/tests/data/acpi/pc/DSDT.ipmikcs
index f1638c5d079a9442c09390426a913010df6efd8d..3231292e1ca1be6c6a5ac0297a886ab23a52ebb3 100644
GIT binary patch
delta 1288
zcmajeJxjx25C`x}QfLfqO<$lW4h07l1yiL|oit6wswu@-5F#dN8%S%#pdjc{?4T79
zsR3P6$QSSh5mzTC{Q@E)E`AA3&`FS69v-;g{ka>iJ@_=r2$Hb;6bB#*myC}hGf|7B
z4jQrHnVhgf%KiLeX5s70H28c`G<S(A6f(J@$#dCxRUYt)jvCAR3xXtu>{-8Mgrf}m
zoPPX}QhQ=M{wel6WJJ@;s&A-vuT=w^wW<thGiR0xPM9M0y$3o|_Jmbq{M$h{KOv0h
zJJqtDfD08$W`rdnXx6o|Zfw>o`W9%W+R^~bQcEMi(XP*_ovy}QXH}7Sg`MuW2^`H!
z*+r0>LlVa+v@#<s0j)^IJjCLPJ8Qu4CN`?Ib<cSPxjFBXlD{?pK>m1!{I4PA<|PGi
z!265|mKY3?yw6$<4)*=mA~#)dh=D5%ykLM1Av26)U=0ID7`VZ}D+ZWuWQ!CAR16$r
z;1&aK7znV)7HJHWFmQr_I}E&IAQVQnn8ZK@1E(0c$G`^$*vL@#pYy-wB5%Yu=FxzD
delta 321
zcmX@FKT(6rCD<iIP=tYj(Rd>l3p0}k$7BKKw#|+#Wt>K?1u^l#PVoZ1`2o(J1{_Yo
zu09OWP29l-2Jsy6jxIq!j#jWCl*7Zoz{D_FolllgbFv$s26J?i=HxCYeGW<s@k7MJ
zp!6&#eGf`22|(o2p!6yz{SHbS2}0z{p!6;%{SQhz2|?uBp!6vy%_j^I4}#Lup!6*$
gEhhqzPlD3Rp!6#!ttSeRFM`tBL^U^m7rns<094vp&j0`b
diff --git a/tests/data/acpi/pc/DSDT.memhp b/tests/data/acpi/pc/DSDT.memhp
index 4c19e45e66918c61674785c99e4474e58866f125..df83eaf0cf87e29ae7a6bf6f6a2dac26015e8689 100644
GIT binary patch
delta 1298
zcmbPXbj^~>CD<k8nhXO2<GPJpdzhG<g(sh2YO5E~cJy?KV2W0D@^SY5|Gyx^$uHEm
zppr3@A;8%)w7|*0)hi)d9VEFVLBx+ez$Mt1Z-E>8<P1S!E`bZ?C!YqJR{2g8f8{HA
z*x9!LWL8dY!UCzuSquvVCTDUnE#R6oIe}q`ATwhlh+tS~wZLL>78iRlFVK7i7oF6y
z%;Z!BhUp0mNd-azObomQIf==s8TmOWsYMKl1qmIA3=9QH9f^z#OA<HxF(2it_gEMc
zAM6w_;II&Af&qt9u&WP4bdzSVfk8Y+yrWAH14n#ts2_ufcs$UpJWD_>&dAKl2B`x{
zL^pY{1RFxt@-Q$kLDX|G;nE-iG|$k5VF?3>VdNqJWf_BnqMMvpCMOGN$l%fpR?tUC
z!A?R7o)A*NFHFD<R)iF!5>haMkb>QW6g(rOKv0B$FKh@YNGGIV5+Mb92`P9%NP(~@
z0bke=QjkeV!4yIY_7hU@ijV?PF#^7DAfzChkb-H16dWX^;0+-K;^Lb8Xa&J$bMc3a
E0H>0MX#fBK
delta 321
zcmca+Im3v{CD<iILXv@japFd<Jxoj<9FtElwQc^%e3Z|~wIC)w*ePDXH$TAH(}2S%
z*wu$2x`{j3z#yI@-q9rp$k7TmgmQQo7?>C)>j}#;YEJeQ)?kiq(wsaIN?(Q2k|GfC
zSSY;^N<W3tnxYW-TqwN}N`HmYmSPb3S}1)GO0$YX#66*OFO<Frr9~wm@{v$_E|h)<
grBx*%@|jS2EtLKUrA?(E@|93}uaxHIe^NIX0Tp>&NdN!<
diff --git a/tests/data/acpi/pc/DSDT.numamem b/tests/data/acpi/pc/DSDT.numamem
index 40cfd933259af05ac2aee07fca32f22122255211..dbb6b182fbd3b45a6ca38d8492892867f9000a4e 100644
GIT binary patch
delta 1288
zcmX@F-mA~$66_MvE6%{cn6{B?4-=EK@Z=LrZS^ABj-D<NOwr0tKF;3%{}*I9`Gxux
zR5E5V1UP$!7C0HWdL=}wgCv(Ei1@JwxCHz1EpTI>oFOR8C2+y~<kMi&D&L9XuY4sB
zJNp)Z%*x44SRgeyi(!Gl<V-H61zeLRCon7#WM*sx5ey5h7FbNq;$ko61)8tmqLW&d
znVhP?Fg<}GsX$18iGjBuCowrSBR?l4wTL0HAfY3XfuSI&Bax9|N#bTd<{Hj=kA*Su
z!A|i44hw-M7;rcRyZSIhH)#eN7{qhLJGulhaKs0P`Z0)z#{=EUvjpVgjLfWTkUEe=
zbdwiLupv|}4+8@eL_HT1E)60;^9)@WmN0-AMlO@NdHI-}SSCyHYRKTy65Rw;;7UkA
z0U-sm2`M;6NWn)!3S{^Q_`;o#f+9i+<`PnHf{=pGgcQi}6Yzy6Aq6Fb6wD{2;1nSR
zUkNEt5Fp?SZ$b*n2q{=dNWmFG3ceFkpd?7Z7rulPR1i|In2>^Vf|~qj*?;pp!H0|h
Dh4h1Q
delta 344
zcmeCxKd;W^66_LkUYLP_(Q6~u9wsIaj>#vO+BW}WuHg)DEr^K^c8VA9%@1()G~jRw
zcJ*P1ZsHC$Fo@@fcXSB?a<qaCp&T9t1||j(j`(0h7ltKZh7pWm3}Qq#X-;P0(_jYC
z9#FamN?(A|BK#2f2q--VN<V<oDgqGs3@E(@N`HXTCV~+83Mjn?LQfKc2)jV(4k&#F
qN(%@><U^qJ3@CjEN-Ky!<Wr#Z3Ml;sN*jnm<V!>~H*Xca!3Y41)nL;A
diff --git a/tests/data/acpi/pc/DSDT.roothp b/tests/data/acpi/pc/DSDT.roothp
index 078fc8031b479cc77b6527a2b7b4bd576b6e6028..fccb574c38ef112aa5e415ddf9a049d12b1c142c 100644
GIT binary patch
delta 1420
zcmeCx++xV(66_MPMS_8Wap6X;Jxol_!jn%hwbhGgJ9@fAFhwgn`8a$3|6h>d<QM8&
zP|29d5a8??THs{h>Xi_!4w77wAmYa!;1cZ1x4?~ka)zKVm%s({lTU+9t9&Plzw(tl
z?Ce_rGAk!HVS&`-EQSRFlQX%P7I00PoWQU|keRU&L@+G0T3|6bi;KOO7ihkMi%x1;
zW^$?m!}J7(qyixUCI;SuoW$hRjQpIG)FOt&f`pDl28M#9jzmU=C5fB;n9De`T(`u;
z2Rp?J_-+Q8U=ZD;8Ejw>&k^tF62!m}9~|n(AR-<QbS2LckXth{v$CCnU41|j(M?_~
z!G=(^JPZs>5cOP4xHL@uz$0tnyDHew0%AT;4>ME`GcG+GAln$Cn`|bl@hZ#UQWf3g
z$THc7R~?T6Czi=&yc+lwEF`4h3?T*I2`NzGBj5&KLJBGfDOgNM!8t+-eiBlk!cV{#
z{)7}%5mK;}kb(<@6#OQnKuv&vF9HcEs3D|aIUxm?2r2kWNP&hR0bc|YQcy=o!Ae33
ct`Ji2pO6AAAp*V#71HEKD~L9i3P~^l09X{PMgRZ+
delta 362
zcmdmD*sICq66_MvE5g9Qcx)rr9wsIaj>#vO+BW}WF5}ektci&ac8V8ptO;=TG>C5E
z4mL1|=ZJT731WzD(h4?&a(EaRm>4GC<dv25FAFxbfT#cpGBa>E1-tqH)v8R^<5Omi
zZqk^X%cl;cH79T2(*V<7ptJ=)gkJ-t4?t-a0f@K<l<tAj7ofC=AVfX_O3#7P51_P)
z5JWx$O0R*^AE2~}FhsrrO7DTtlSClGE>OAyN}qw!0-_N45GXwZO5cIf3StoX6fw=s
IOU3w^0M)Q&D*ylh
diff --git a/tests/data/acpi/q35/DSDT b/tests/data/acpi/q35/DSDT
index d25cd7072932886d6967f4023faac1e1fa6e836c..9506b60117147fd8ebe30fc3bf9f124773e6a3c5 100644
GIT binary patch
delta 129
zcmexqv)z`<CD<ioyBq@p<Nl3YnsS*g95M01PVoXh>;cZ62GLEL!3GBL9Py4WK@1%6
y!J&Q(BI5Bb!M;380)S*jW>&USu&WP9BD%?wCD_0MqLv3}9z;DOF7=b+<fH)|#Up(H
delta 67
zcmdmP`_qQYCD<jTQjURv@!Li&O*w5hmYDcpr+5Lo0B28w=qB!91A};uct@8YhUg}(
RU;_&XhX<&hVe%X~X#j{75q$sv
diff --git a/tests/data/acpi/q35/DSDT.acpihmat b/tests/data/acpi/q35/DSDT.acpihmat
index 722e06af83abcde203a2b96a8ec81fd3bab9fc98..ae51a89dc056a13f50bc515e05207e875c369a51 100644
GIT binary patch
delta 129
zcmZ4H{=}WjCD<k8i82ENW6egcFy%}aj+pphr+5J$_5f#3gXkvBU;~4Aj(A6xAO?>3
y;7~sX5%GAJU|*gk0YEY%Gb`IE*wqIl5#8j;5^P`rQOg4~52BtCm-@-w%F+NNZzI0|
delta 67
zcmaFjzRaD=CD<ionKA<d<K~TAVanQWEHUxHPVoYE0nVNV(M{aJ1_tpQ@s2J*4AD(m
R!3Gu(4i8X0!{n37(g2Iy5x)Qc
diff --git a/tests/data/acpi/q35/DSDT.bridge b/tests/data/acpi/q35/DSDT.bridge
index 06bac139d668ddfc7914e258b471a303c9dbd192..0cf9f067d25ae5d596f236cda3b1a495a527249b 100644
GIT binary patch
delta 176
zcmeCSeQwL;66_N4T#kW((Rm}6rd*~AcT9Y+Q@nr=SAespL3ER5uz^85N4%p;5Ccbi
zaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&^2{y2RsO15g2T{+6Oa0_{IaxiuU_+?s
OKpjjF9b8Pfv;Y7-elC;%
delta 84
zcmaEE+ilC`66_MvEyuvX*uRlWQ%=W&H6}jTDPF*VHNe@^Ai9Y=*uWs3Bi_*^h#|U3
cE7-sS!r=jGV3<5dPL`iP*bu4!D8|G90GZPhbpQYW
diff --git a/tests/data/acpi/q35/DSDT.cphp b/tests/data/acpi/q35/DSDT.cphp
index 2b933ac482e6883efccbd7d6c96089602f2c0b4d..3d17ac851fe4a988006f49deccdffd426581b6a7 100644
GIT binary patch
delta 129
zcmX@<(C*0P66_MvuE4;+sJxNur+lUhM@)RMQ@nr=dw{d2L3ER5uz^85N4%p;5Ccbi
yaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&^2{y2RsO15g2T{+6OZ{X$1!(~A>mvRD
delta 67
zcmZp7Jn6vY66_M<sldR%7`2h>r@XcsOH6#QQ@ns(fU~DTbQ5>5fk8Y+yrWAHLv)i?
Ruz>}H!vj>$FgZs-8USwS5dHuF
diff --git a/tests/data/acpi/q35/DSDT.dimmpxm b/tests/data/acpi/q35/DSDT.dimmpxm
index bd8f8305b028ef20f9b6d1a0c69ac428d027e3d1..acde6cb0a22f031c0f8743e5cceb3b707e3aab61 100644
GIT binary patch
delta 153
zcmaFwxz>xzCD<iISCxT*@!dwQ`6~4;95M01PVoXh>;cZ62GLEL!3GBL9Py4WK@1%6
z!J&Q(BI5Bb!M;380)S*jW>&USu&WP9BD%?wCD_0MqLv3}9z;DOF7+Zn;|yIGmN0-A
LMlc4eF^B~K#WN^W
delta 90
zcmZ4M_1=@qCD<k8y$S;ZquNHU`6_O1EHUxHPVoYE0nVNV(M{aJ1_tpQ@s2J*4AD(m
c!3Gu(4i8X0g9t}_u%QdX5-`IE#xMpk09m~j761SM
diff --git a/tests/data/acpi/q35/DSDT.ipmibt b/tests/data/acpi/q35/DSDT.ipmibt
index a8f868e23c25688ab1c0371016c071f23e9d732f..77dac3feb29b6769b0ccb6b29a16538a4ac31879 100644
GIT binary patch
delta 151
zcmX?N+hoV(66_MfB+tOW_-P|oiCn!4M@)RMQ@nr=dw{d2L3ER5uz^85N4%p;5Ccbi
zaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&^2{y2RsO15g2T{+6OT7rtI71hPB@7^j
L5sWc;zZ@$71t2Hg
delta 90
zcmZp&J7UY_66_LkM2>-hQEMYtiJY4oOH6#QQ@ns(fU~DTbQ5>5fk8Y+yrWAHLv)i?
cuz>}H!vj>$Ai@zJZ0N$U1k5mkF^oYB02f&mN&o-=
diff --git a/tests/data/acpi/q35/DSDT.memhp b/tests/data/acpi/q35/DSDT.memhp
index 9a802e4c67022386442976d5cb997ea3fc57b58f..5e3c1a4b1a34a63edcf9394455dbbaa86a24b044 100644
GIT binary patch
delta 129
zcmX@%-sZvO66_MfrozC$xM3q#g>t3~M@)RMQ@nr=dw{d2L3ER5uz^85N4%p;5Ccbi
yaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&^2{y2RsO15g2T{+6Oa0`v%F+PFXCof~
delta 67
zcmZqkIN{Fa66_LkLYaYq@x?~23T16KmYDcpr+5Lo0B28w=qB!91A};uct@8YhUg}(
RU;_&XhX<&hVe)HbX#kbp5+48n
diff --git a/tests/data/acpi/q35/DSDT.mmio64 b/tests/data/acpi/q35/DSDT.mmio64
index 948c2dc7264c31932b490ca00691a7c4d9aefdb0..09faed2cd326d591b6c33203dd9c0504808a0952 100644
GIT binary patch
delta 198
zcmaFjdccj#CD<jzL79PpF<>KClTy74cT9Y+Q@nr=SAespL3ER5uz^85N4%p;5Ccbi
zaHt=Hh<Ln9urJS&03ex>nU(Dn?CJxOh;H&^2{y2RsO15g2T{+6OT7rtI71hPB@7^j
b5sWeUoD#d9Ua%q5PM|qV5OcVga2W&uZFw`V
delta 108
zcmX@$_QaLTCD<k8i4p??<J^s0O-d0StTFMyPVoW`tO3rR2GLF2!3GBL9Py4WK@8DN
oTEPYu5DpJe1A_=he6XPl!xAvV2*xl5F`}FJgAJh?foho;03~7>$N&HU
diff --git a/tests/data/acpi/q35/DSDT.numamem b/tests/data/acpi/q35/DSDT.numamem
index 44ec1b0af400da6d298284aa959aa38add7e6dd5..642a7b39cc8bb174daa098c4a377df8cd5938289 100644
GIT binary patch
delta 153
zcmexwv)7i(CD<iouN(sdW9CLKeYtuUj+pphr+5J$_5f#3gXkvBU;~4Aj(A6xAO?>3
z;7~sX5%GAJU|*gk0YEY%Gb`IE*wqIl5#8j;5^P`rQOg4~52BtCmwFMPafU7oOBg^5
LBN&6#7{meqj}s^8
delta 90
zcmdmM``?DkCD<jTUXFo*alu9|eK|KbmYDcpr+5Lo0B28w=qB!91A};uct@8YhUg}(
cU;_&XhX<&hL4+ed*wBSx37BC7V;F-N068@kssI20
diff --git a/tests/data/acpi/q35/DSDT.tis b/tests/data/acpi/q35/DSDT.tis
index 30da3ec27958881801dacc954a343321ba26a2ae..bef6c030b8fa08abf538bfb4ac244d1567c83c2e 100644
GIT binary patch
delta 130
zcmccaIK_#}CD<iIM3I4kar#CsQ@Ko6-I(}br+5Khod9P~gXkvBU;~4Aj(A6xAO?>3
z;7~sX5%GAJU|*gk0YEY%Gb`IE*wqIl5#8j;5^P`rQOg4~52BtCm-@}wayd)@>z*T0
delta 69
zcmbQ@bls85CD<k8x&i|O<B5%2rgAzSnlbUgPVoW`ngPz92GLF2!3GBL9Py4WK@8DN
TTEPYu5DpJe1H<MOayd)@u>KNK
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/5] pc: support user provided NIC naming/indexing
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
` (4 preceding siblings ...)
2020-12-22 23:39 ` [RFC 5/5] tests: acpi: update expected data files Igor Mammedov
@ 2021-01-13 12:09 ` Michael S. Tsirkin
2021-01-15 1:59 ` Igor Mammedov
5 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-01-13 12:09 UTC (permalink / raw)
To: Igor Mammedov; +Cc: laine, jusual, qemu-devel
On Tue, Dec 22, 2020 at 06:39:29PM -0500, Igor Mammedov wrote:
>
> Series implements support for 'onboard' naming scheme for network
> interfaces (1), which is based on PCI firmware spec and lets user
> to explicitly specify index that will be used by guest to name
> network interface, ex:
> -device e1000,acpi-index=33
> should make guest rename NIC name to 'eno33' where 'eno' is default
> prefix for this scheme.
>
> Hope is that it will allow to simplify launching VMs from
> template disk images with different set VM configurations
> without need to reconfigure guest network intrfaces or
> risk of loosing network connectivity.
Questions:
the spec says:
Assignment of specific device names to multi-function devices installed in expansion
slots, and/or PCI or PCI Express devices that are hot-added to expansion slots in operating system-
environment would be handled in operating system-specific manner, and is not specified via this
specification.
Accordingly, link below says:
" Names incorporating Firmware/BIOS provided index numbers for on-board devices (example: eno1)"
to what extend does guest assume the index is for on-board devices?
it seems things work for fedora but how confident are we that this
will keep working.
Further, code seems to only look at the slot level.
According to this, and according to the spec, this does not work with
multifunction devices, does it?
The link you supplied lists another option:
"Names incorporating Firmware/BIOS provided PCI Express hotplug slot index numbers (example: ens1)"
these are under management control already ...
Also if we ask users to supply the property on the slot then it seems
that the property can be baked into ACPI when it's created instead of
being loaded from host - we can avoid adding new registers, this seems
preferable. Could someone from management side chime in on whether that
is sufficient?
More questions:
does all this affect windows guests at all?
where does the "acpi index" terminology come from?
the pci firmware spec talks about "instance number", right?
> For more detailed description/examples see patches [3-4/5]
>
> 1)
> https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
>
> Git repo for testing:
> https://github.com/imammedo/qemu/branches acpi-index-rfc
>
> Igor Mammedov (5):
> acpi: add aml_to_decimalstring() and aml_call6() helpers
> tests: acpi: temporary whitelist DSDT changes
> pci: introduce apci-index property for PCI device
> pci: acpi: add _DSM method to PCI devices
> tests: acpi: update expected data files
>
> include/hw/acpi/aml-build.h | 3 +
> include/hw/acpi/pci.h | 1 +
> include/hw/acpi/pcihp.h | 7 +-
> include/hw/pci/pci.h | 1 +
> tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++
> hw/acpi/aml-build.c | 28 +++++++
> hw/acpi/pci.c | 84 ++++++++++++++++++++
> hw/acpi/pcihp.c | 25 +++++-
> hw/i386/acpi-build.c | 31 +++++++-
> hw/pci/pci.c | 1 +
> tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
> tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
> tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
> tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
> tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
> tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
> tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
> tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
> tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
> tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
> tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
> tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
> tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
> tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
> tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
> tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
> tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
> tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
> tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
> tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
> tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
> 31 files changed, 197 insertions(+), 5 deletions(-)
>
> --
> 2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 4/5] pci: acpi: add _DSM method to PCI devices
2020-12-22 23:39 ` [RFC 4/5] pci: acpi: add _DSM method to PCI devices Igor Mammedov
@ 2021-01-13 12:13 ` Michael S. Tsirkin
2021-01-15 0:23 ` Igor Mammedov
2021-01-26 11:16 ` Michael S. Tsirkin
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-01-13 12:13 UTC (permalink / raw)
To: Igor Mammedov; +Cc: laine, jusual, qemu-devel
On Tue, Dec 22, 2020 at 06:39:33PM -0500, Igor Mammedov wrote:
> Implement _DSM according to:
> PCI Firmware Specification 3.1
> 4.6.7. DSM for Naming a PCI or PCI Express Device Under
> Operating Systems
> and wire it up to cold and hot-plugged PCI devices.
> Feature depends on ACPI hotplug being enabled (as that provides
> PCI devices descriptions in ACPI and MMIO registers that are
> reused to fetch acpi-index).
>
> acpi-index should work for
> - cold plugged NICs:
> $QEMU -evice e1000,acpi-index=100
> => 'eno100'
> - hot-plugged
> (monitor) device_add e1000,acpi-index=200,id=remove_me
> => 'eno200'
> - replugged
> (monitor) device_del remove_me
> (monitor) device_add e1000,acpi-index=1
> => 'eno1'
>
> Windows also sees index under "PCI Label Id" field in properties
> dialog but otherwise it doesn't seem to have any effect.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/pci.h | 1 +
> hw/acpi/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 21 ++++++++++--
> 3 files changed, 97 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> index bf2a3ed0ba..5e1eb2a96a 100644
> --- a/include/hw/acpi/pci.h
> +++ b/include/hw/acpi/pci.h
> @@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
> } AcpiMcfgInfo;
>
> void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
> +Aml *aml_pci_device_dsm(void);
> #endif
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index 07d5101d83..6d49d515d3 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> AcpiPciHpState *s = opaque;
> return s->acpi_index;
> }
> +
> +Aml *aml_pci_device_dsm(void)
> +{
> + Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
> + Aml *acpi_index = aml_local(0);
> + Aml *zero = aml_int(0);
> + Aml *bnum = aml_arg(4);
> + Aml *sun = aml_arg(5);
> +
> + method = aml_method("PDSM", 6, AML_SERIALIZED);
> +
> + /*
> + * PCI Firmware Specification 3.1
> + * 4.6. _DSM Definitions for PCI
> + */
> + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> + {
> + aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
> + ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
> + {
> + uint8_t byte_list[1];
> +
> + ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
> + {
> + /*
> + * advertise function 7 if device has acpi-index
> + */
> + ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
> + {
> + byte_list[0] =
> + 1 /* have supported functions */ |
> + 1 << 7 /* support for function 7 */
> + ;
> + aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx2, ifctx3);
> + }
> + aml_append(ifctx1, ifctx2);
> +
> + byte_list[0] = 0; /* nothing supported */
> + aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx, ifctx1);
> + elsectx = aml_else();
> + /*
> + * PCI Firmware Specification 3.1
> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> + * Operating Systems
> + */
> + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
> + {
> + Aml *pkg = aml_package(2);
> + Aml *label = aml_local(2);
> + Aml *ret = aml_local(1);
> +
> + aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
> + aml_to_decimalstring(acpi_index, NULL), label));
> +
> + aml_append(pkg, zero);
> + aml_append(pkg, aml_string("placeholder"));
> + aml_append(ifctx1, aml_store(pkg, ret));
> + /*
> + * update apci-index to actual value
> + */
> + aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> + /*
> + * update device label to actual value
> + */
> + aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
> + aml_append(ifctx1, aml_return(ret));
This code needs more comments to explain what it's doing.
E.g. what is device label?
Spec seems to say the string is optional. Is it relevant somehow?
spec also says string must be unique. Do we need to enforce that?
> + }
> + aml_append(elsectx, ifctx1);
> + aml_append(ifctx, elsectx);
> + }
> + aml_append(method, ifctx);
> + return method;
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 27d2958e25..447ad39c35 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> );
> aml_append(dev, method);
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + aml_append(method,
> + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
> + aml_arg(2), aml_arg(3),
> + aml_name("BSEL"), aml_name("_SUN")))
> + );
> + aml_append(dev, method);
> aml_append(parent_scope, dev);
>
> build_append_pcihp_notify_entry(notify_method, slot);
> @@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
>
> + aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + aml_append(method,
> + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
> + aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
> + );
> + aml_append(dev, method);
> +
> if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
> /* add VGA specific AML methods */
> int s3d;
> @@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> aml_append(method, aml_return(aml_int(s3d)));
> aml_append(dev, method);
> } else if (hotplug_enabled_dev) {
> - /* add _SUN/_EJ0 to make slot hotpluggable */
> - aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> -
> + /* add _EJ0 to make slot hotpluggable */
> method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> aml_append(method,
> aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> @@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
> aml_append(method, aml_return(aml_local(0)));
> aml_append(scope, method);
>
> + aml_append(scope, aml_pci_device_dsm());
> +
> aml_append(table, scope);
> }
>
> --
> 2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 4/5] pci: acpi: add _DSM method to PCI devices
2021-01-13 12:13 ` Michael S. Tsirkin
@ 2021-01-15 0:23 ` Igor Mammedov
0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-01-15 0:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: laine, jusual, qemu-devel
On Wed, 13 Jan 2021 07:13:04 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 22, 2020 at 06:39:33PM -0500, Igor Mammedov wrote:
> > Implement _DSM according to:
> > PCI Firmware Specification 3.1
> > 4.6.7. DSM for Naming a PCI or PCI Express Device Under
> > Operating Systems
> > and wire it up to cold and hot-plugged PCI devices.
> > Feature depends on ACPI hotplug being enabled (as that provides
> > PCI devices descriptions in ACPI and MMIO registers that are
> > reused to fetch acpi-index).
> >
> > acpi-index should work for
> > - cold plugged NICs:
> > $QEMU -evice e1000,acpi-index=100
> > => 'eno100'
> > - hot-plugged
> > (monitor) device_add e1000,acpi-index=200,id=remove_me
> > => 'eno200'
> > - replugged
> > (monitor) device_del remove_me
> > (monitor) device_add e1000,acpi-index=1
> > => 'eno1'
> >
> > Windows also sees index under "PCI Label Id" field in properties
> > dialog but otherwise it doesn't seem to have any effect.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/acpi/pci.h | 1 +
> > hw/acpi/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++
> > hw/i386/acpi-build.c | 21 ++++++++++--
> > 3 files changed, 97 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> > index bf2a3ed0ba..5e1eb2a96a 100644
> > --- a/include/hw/acpi/pci.h
> > +++ b/include/hw/acpi/pci.h
> > @@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
> > } AcpiMcfgInfo;
> >
> > void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
> > +Aml *aml_pci_device_dsm(void);
> > #endif
> > diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> > index 07d5101d83..6d49d515d3 100644
> > --- a/hw/acpi/pci.c
> > +++ b/hw/acpi/pci.c
> > @@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> > AcpiPciHpState *s = opaque;
> > return s->acpi_index;
> > }
> > +
> > +Aml *aml_pci_device_dsm(void)
> > +{
> > + Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
> > + Aml *acpi_index = aml_local(0);
> > + Aml *zero = aml_int(0);
> > + Aml *bnum = aml_arg(4);
> > + Aml *sun = aml_arg(5);
> > +
> > + method = aml_method("PDSM", 6, AML_SERIALIZED);
> > +
> > + /*
> > + * PCI Firmware Specification 3.1
> > + * 4.6. _DSM Definitions for PCI
> > + */
> > + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> > + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > + {
> > + aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
> > + ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
> > + {
> > + uint8_t byte_list[1];
> > +
> > + ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
> > + {
> > + /*
> > + * advertise function 7 if device has acpi-index
> > + */
> > + ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
> > + {
> > + byte_list[0] =
> > + 1 /* have supported functions */ |
> > + 1 << 7 /* support for function 7 */
> > + ;
> > + aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
> > + }
> > + aml_append(ifctx2, ifctx3);
> > + }
> > + aml_append(ifctx1, ifctx2);
> > +
> > + byte_list[0] = 0; /* nothing supported */
> > + aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
> > + }
> > + aml_append(ifctx, ifctx1);
> > + elsectx = aml_else();
> > + /*
> > + * PCI Firmware Specification 3.1
> > + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> > + * Operating Systems
> > + */
> > + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
> > + {
> > + Aml *pkg = aml_package(2);
> > + Aml *label = aml_local(2);
> > + Aml *ret = aml_local(1);
> > +
> > + aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
> > + aml_to_decimalstring(acpi_index, NULL), label));
> > +
> > + aml_append(pkg, zero);
> > + aml_append(pkg, aml_string("placeholder"));
> > + aml_append(ifctx1, aml_store(pkg, ret));
> > + /*
> > + * update apci-index to actual value
> > + */
> > + aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> > + /*
> > + * update device label to actual value
> > + */
> > + aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
> > + aml_append(ifctx1, aml_return(ret));
>
> This code needs more comments to explain what it's doing.
> E.g. what is device label?
> Spec seems to say the string is optional. Is it relevant somehow?
potentially label may be used (with custom rules) to for interface naming
or as human readable PCI device description.
We probably can drop it, or provide and additional PCIDevice option to let user
specify it on CLI along with acpi-index.
>
> spec also says string must be unique. Do we need to enforce that?
I think we should enforce it (Though I intentionally dropped it from RFC)
> > + }
> > + aml_append(elsectx, ifctx1);
> > + aml_append(ifctx, elsectx);
> > + }
> > + aml_append(method, ifctx);
> > + return method;
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 27d2958e25..447ad39c35 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> > );
> > aml_append(dev, method);
> > + method = aml_method("_DSM", 4, AML_SERIALIZED);
> > + aml_append(method,
> > + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
> > + aml_arg(2), aml_arg(3),
> > + aml_name("BSEL"), aml_name("_SUN")))
> > + );
> > + aml_append(dev, method);
> > aml_append(parent_scope, dev);
> >
> > build_append_pcihp_notify_entry(notify_method, slot);
> > @@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> > aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
> >
> > + aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > + method = aml_method("_DSM", 4, AML_SERIALIZED);
> > + aml_append(method,
> > + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
> > + aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
> > + );
> > + aml_append(dev, method);
> > +
> > if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
> > /* add VGA specific AML methods */
> > int s3d;
> > @@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > aml_append(method, aml_return(aml_int(s3d)));
> > aml_append(dev, method);
> > } else if (hotplug_enabled_dev) {
> > - /* add _SUN/_EJ0 to make slot hotpluggable */
> > - aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > -
> > + /* add _EJ0 to make slot hotpluggable */
> > method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> > aml_append(method,
> > aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> > @@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
> > aml_append(method, aml_return(aml_local(0)));
> > aml_append(scope, method);
> >
> > + aml_append(scope, aml_pci_device_dsm());
> > +
> > aml_append(table, scope);
> > }
> >
> > --
> > 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/5] pc: support user provided NIC naming/indexing
2021-01-13 12:09 ` [RFC 0/5] pc: support user provided NIC naming/indexing Michael S. Tsirkin
@ 2021-01-15 1:59 ` Igor Mammedov
2021-01-17 10:59 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-01-15 1:59 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jusual, qemu-devel, laine
On Wed, 13 Jan 2021 07:09:56 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 22, 2020 at 06:39:29PM -0500, Igor Mammedov wrote:
> >
> > Series implements support for 'onboard' naming scheme for network
> > interfaces (1), which is based on PCI firmware spec and lets user
> > to explicitly specify index that will be used by guest to name
> > network interface, ex:
> > -device e1000,acpi-index=33
> > should make guest rename NIC name to 'eno33' where 'eno' is default
> > prefix for this scheme.
> >
> > Hope is that it will allow to simplify launching VMs from
> > template disk images with different set VM configurations
> > without need to reconfigure guest network intrfaces or
> > risk of loosing network connectivity.
>
> Questions:
> the spec says:
> Assignment of specific device names to multi-function devices installed in expansion
> slots, and/or PCI or PCI Express devices that are hot-added to expansion slots in operating system-
> environment would be handled in operating system-specific manner, and is not specified via this
> specification.
>
> Accordingly, link below says:
> " Names incorporating Firmware/BIOS provided index numbers for on-board devices (example: eno1)"
>
> to what extend does guest assume the index is for on-board devices?
> it seems things work for fedora but how confident are we that this
> will keep working.
code itself is not limiting it to onboard devices in any way.
(I can only speculate here, reason for calling it onboard is that
on real hardware ACPI is mostly static tables and firmware provides
an option to set index for only built-in NICs).
Technically there is no reason to call in onboad though.
I'd believe it should work with any distribution that uses
recent enough systemd/udev (released starting from 2003).
> Further, code seems to only look at the slot level.
> According to this, and according to the spec, this does not work with
> multifunction devices, does it?
we probably should disable it for multifunction devices,
any suggestions how to detect those in QEMU?
> The link you supplied lists another option:
> "Names incorporating Firmware/BIOS provided PCI Express hotplug slot index numbers (example: ens1)"
> these are under management control already ...
with it interface name continues to depend on PCI topology (and theoretically
limited to PCI expess). That's becomes harder to consume as complexity grows
(i.e. mgmt needs to keep NIC in the same place for which guest image was configured for).
acpi-index doesn't impose such limitation.
In case of 1 NIC, it could be moved anywhere within PCI hierarchy and guest
doesn't have to be reconfigured to account for new interface name
(i.e without loosing network connectivity - that's the actual issue coming from
upper layers that made me look into acpi-index approach).
In another words acpi index is easier to consume for users above libvirt
and frees mgmt hands in a way it could distribute PCI devices.
Even better would be if guest image could carry index as metadata
> Also if we ask users to supply the property on the slot then it seems
> that the property can be baked into ACPI when it's created instead of
> being loaded from host - we can avoid adding new registers, this seems
> preferable. Could someone from management side chime in on whether that
> is sufficient?
I did consider it (it would be simpler, but not much), however unless we disable
PCI hotplug for affected slots it won't work. (unplug device from such slot and
plug another in that place will still return boot time index.
That's why I ended up with hotplug variant.
I chose abusing existing PCI hotplug registers for it,
but we can use a new range if that's preferred.
> More questions:
>
> does all this affect windows guests at all?
I don't know (spec shows examples that reminded
me about NIC naming which Windows use(s|d)).
But I won't bet on it.
If I recall correctly, for e1000 NIC, it didn't made any
difference in naming (pre-existing guest image).
> where does the "acpi index" terminology come from?
> the pci firmware spec talks about "instance number", right?
it comes from linux kernel (that's how it's named in sysfs)
and systemd/udev uses it. So I tried to avoid making up another
one.
> > For more detailed description/examples see patches [3-4/5]
> >
> > 1)
> > https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> >
> > Git repo for testing:
> > https://github.com/imammedo/qemu/branches acpi-index-rfc
> >
> > Igor Mammedov (5):
> > acpi: add aml_to_decimalstring() and aml_call6() helpers
> > tests: acpi: temporary whitelist DSDT changes
> > pci: introduce apci-index property for PCI device
> > pci: acpi: add _DSM method to PCI devices
> > tests: acpi: update expected data files
> >
> > include/hw/acpi/aml-build.h | 3 +
> > include/hw/acpi/pci.h | 1 +
> > include/hw/acpi/pcihp.h | 7 +-
> > include/hw/pci/pci.h | 1 +
> > tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++
> > hw/acpi/aml-build.c | 28 +++++++
> > hw/acpi/pci.c | 84 ++++++++++++++++++++
> > hw/acpi/pcihp.c | 25 +++++-
> > hw/i386/acpi-build.c | 31 +++++++-
> > hw/pci/pci.c | 1 +
> > tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
> > tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
> > tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
> > tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
> > tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
> > tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
> > tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
> > tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
> > tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
> > tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
> > tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
> > tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
> > tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
> > tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
> > tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
> > tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
> > tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
> > tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
> > tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
> > tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
> > tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
> > 31 files changed, 197 insertions(+), 5 deletions(-)
> >
> > --
> > 2.27.0
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/5] pc: support user provided NIC naming/indexing
2021-01-15 1:59 ` Igor Mammedov
@ 2021-01-17 10:59 ` Michael S. Tsirkin
2021-01-20 12:07 ` Igor Mammedov
0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-01-17 10:59 UTC (permalink / raw)
To: Igor Mammedov; +Cc: jusual, qemu-devel, laine
On Fri, Jan 15, 2021 at 02:59:02AM +0100, Igor Mammedov wrote:
> On Wed, 13 Jan 2021 07:09:56 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Tue, Dec 22, 2020 at 06:39:29PM -0500, Igor Mammedov wrote:
> > >
> > > Series implements support for 'onboard' naming scheme for network
> > > interfaces (1), which is based on PCI firmware spec and lets user
> > > to explicitly specify index that will be used by guest to name
> > > network interface, ex:
> > > -device e1000,acpi-index=33
> > > should make guest rename NIC name to 'eno33' where 'eno' is default
> > > prefix for this scheme.
> > >
> > > Hope is that it will allow to simplify launching VMs from
> > > template disk images with different set VM configurations
> > > without need to reconfigure guest network intrfaces or
> > > risk of loosing network connectivity.
> >
> > Questions:
> > the spec says:
> > Assignment of specific device names to multi-function devices installed in expansion
> > slots, and/or PCI or PCI Express devices that are hot-added to expansion slots in operating system-
> > environment would be handled in operating system-specific manner, and is not specified via this
> > specification.
> >
> > Accordingly, link below says:
> > " Names incorporating Firmware/BIOS provided index numbers for on-board devices (example: eno1)"
> >
> > to what extend does guest assume the index is for on-board devices?
> > it seems things work for fedora but how confident are we that this
> > will keep working.
>
> code itself is not limiting it to onboard devices in any way.
> (I can only speculate here, reason for calling it onboard is that
> on real hardware ACPI is mostly static tables and firmware provides
> an option to set index for only built-in NICs).
> Technically there is no reason to call in onboad though.
Well there's an smbios thing that I believe says onboard
in the spec. Come to think of it that one actually
applies to multifunction devices too.
>
> I'd believe it should work with any distribution that uses
> recent enough systemd/udev (released starting from 2003).
>
> > Further, code seems to only look at the slot level.
> > According to this, and according to the spec, this does not work with
> > multifunction devices, does it?
>
> we probably should disable it for multifunction devices,
> any suggestions how to detect those in QEMU?
QEMU_PCI_CAP_MULTIFUNCTION_BITNR ?
Enforcing this will help but can we make this self-documenting somehow?
>
> > The link you supplied lists another option:
> > "Names incorporating Firmware/BIOS provided PCI Express hotplug slot index numbers (example: ens1)"
> > these are under management control already ...
>
> with it interface name continues to depend on PCI topology (and theoretically
> limited to PCI expess). That's becomes harder to consume as complexity grows
> (i.e. mgmt needs to keep NIC in the same place for which guest image was configured for).
> acpi-index doesn't impose such limitation.
>
> In case of 1 NIC, it could be moved anywhere within PCI hierarchy and guest
> doesn't have to be reconfigured to account for new interface name
> (i.e without loosing network connectivity - that's the actual issue coming from
> upper layers that made me look into acpi-index approach).
Could you describe the issue in a bit more detail in the commit
log pls?
> In another words acpi index is easier to consume for users above libvirt
> and frees mgmt hands in a way it could distribute PCI devices.
> Even better would be if guest image could carry index as metadata
>
> > Also if we ask users to supply the property on the slot then it seems
> > that the property can be baked into ACPI when it's created instead of
> > being loaded from host - we can avoid adding new registers, this seems
> > preferable. Could someone from management side chime in on whether that
> > is sufficient?
>
> I did consider it (it would be simpler, but not much), however unless we disable
> PCI hotplug for affected slots it won't work. (unplug device from such slot and
> plug another in that place will still return boot time index.
Well it's interesting that you mention it. Let's say you remove
a device then plug in a different index immediately ...
Guest gets a wrong name temporarily, right?
Should we worry about that?
> That's why I ended up with hotplug variant.
>
> I chose abusing existing PCI hotplug registers for it,
> but we can use a new range if that's preferred.
Would it work for multi-function devices though?
If yes that's a big advantage ...
> > More questions:
> >
> > does all this affect windows guests at all?
> I don't know (spec shows examples that reminded
> me about NIC naming which Windows use(s|d)).
> But I won't bet on it.
> If I recall correctly, for e1000 NIC, it didn't made any
> difference in naming (pre-existing guest image).
>
> > where does the "acpi index" terminology come from?
> > the pci firmware spec talks about "instance number", right?
> it comes from linux kernel (that's how it's named in sysfs)
> and systemd/udev uses it. So I tried to avoid making up another
> one.
OTOH that's guest specific ... and does not seem to imply
the limitations unless you look at the code ...
>
> > > For more detailed description/examples see patches [3-4/5]
> > >
> > > 1)
> > > https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> > >
> > > Git repo for testing:
> > > https://github.com/imammedo/qemu/branches acpi-index-rfc
> > >
> > > Igor Mammedov (5):
> > > acpi: add aml_to_decimalstring() and aml_call6() helpers
> > > tests: acpi: temporary whitelist DSDT changes
> > > pci: introduce apci-index property for PCI device
> > > pci: acpi: add _DSM method to PCI devices
> > > tests: acpi: update expected data files
> > >
> > > include/hw/acpi/aml-build.h | 3 +
> > > include/hw/acpi/pci.h | 1 +
> > > include/hw/acpi/pcihp.h | 7 +-
> > > include/hw/pci/pci.h | 1 +
> > > tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++
> > > hw/acpi/aml-build.c | 28 +++++++
> > > hw/acpi/pci.c | 84 ++++++++++++++++++++
> > > hw/acpi/pcihp.c | 25 +++++-
> > > hw/i386/acpi-build.c | 31 +++++++-
> > > hw/pci/pci.c | 1 +
> > > tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
> > > tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
> > > tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
> > > tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
> > > tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
> > > tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
> > > tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
> > > tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
> > > tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
> > > tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
> > > tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
> > > tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
> > > tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
> > > tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
> > > tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
> > > tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
> > > tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
> > > tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
> > > tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
> > > tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
> > > tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
> > > 31 files changed, 197 insertions(+), 5 deletions(-)
> > >
> > > --
> > > 2.27.0
> >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 0/5] pc: support user provided NIC naming/indexing
2021-01-17 10:59 ` Michael S. Tsirkin
@ 2021-01-20 12:07 ` Igor Mammedov
0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-01-20 12:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jusual, qemu-devel, laine
On Sun, 17 Jan 2021 05:59:18 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Jan 15, 2021 at 02:59:02AM +0100, Igor Mammedov wrote:
> > On Wed, 13 Jan 2021 07:09:56 -0500
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Tue, Dec 22, 2020 at 06:39:29PM -0500, Igor Mammedov wrote:
> > > >
> > > > Series implements support for 'onboard' naming scheme for network
> > > > interfaces (1), which is based on PCI firmware spec and lets user
> > > > to explicitly specify index that will be used by guest to name
> > > > network interface, ex:
> > > > -device e1000,acpi-index=33
> > > > should make guest rename NIC name to 'eno33' where 'eno' is default
> > > > prefix for this scheme.
> > > >
> > > > Hope is that it will allow to simplify launching VMs from
> > > > template disk images with different set VM configurations
> > > > without need to reconfigure guest network intrfaces or
> > > > risk of loosing network connectivity.
> > >
> > > Questions:
> > > the spec says:
> > > Assignment of specific device names to multi-function devices installed in expansion
> > > slots, and/or PCI or PCI Express devices that are hot-added to expansion slots in operating system-
> > > environment would be handled in operating system-specific manner, and is not specified via this
> > > specification.
> > >
> > > Accordingly, link below says:
> > > " Names incorporating Firmware/BIOS provided index numbers for on-board devices (example: eno1)"
> > >
> > > to what extend does guest assume the index is for on-board devices?
> > > it seems things work for fedora but how confident are we that this
> > > will keep working.
> >
> > code itself is not limiting it to onboard devices in any way.
> > (I can only speculate here, reason for calling it onboard is that
> > on real hardware ACPI is mostly static tables and firmware provides
> > an option to set index for only built-in NICs).
> > Technically there is no reason to call in onboad though.
>
> Well there's an smbios thing that I believe says onboard
> in the spec. Come to think of it that one actually
> applies to multifunction devices too.
Can you point to the specific place in the spec, pls?
>
>
> >
> > I'd believe it should work with any distribution that uses
> > recent enough systemd/udev (released starting from 2003).
> >
> > > Further, code seems to only look at the slot level.
> > > According to this, and according to the spec, this does not work with
> > > multifunction devices, does it?
> >
> > we probably should disable it for multifunction devices,
> > any suggestions how to detect those in QEMU?
>
> QEMU_PCI_CAP_MULTIFUNCTION_BITNR ?
>
> Enforcing this will help but can we make this self-documenting somehow?
what do you mean saying "to make it self-documenting somehow"?
> > > The link you supplied lists another option:
> > > "Names incorporating Firmware/BIOS provided PCI Express hotplug slot index numbers (example: ens1)"
> > > these are under management control already ...
> >
> > with it interface name continues to depend on PCI topology (and theoretically
> > limited to PCI expess). That's becomes harder to consume as complexity grows
> > (i.e. mgmt needs to keep NIC in the same place for which guest image was configured for).
> > acpi-index doesn't impose such limitation.
> >
> > In case of 1 NIC, it could be moved anywhere within PCI hierarchy and guest
> > doesn't have to be reconfigured to account for new interface name
> > (i.e without loosing network connectivity - that's the actual issue coming from
> > upper layers that made me look into acpi-index approach).
>
> Could you describe the issue in a bit more detail in the commit
> log pls?
sure
> > In another words acpi index is easier to consume for users above libvirt
> > and frees mgmt hands in a way it could distribute PCI devices.
> > Even better would be if guest image could carry index as metadata
> >
> > > Also if we ask users to supply the property on the slot then it seems
> > > that the property can be baked into ACPI when it's created instead of
> > > being loaded from host - we can avoid adding new registers, this seems
> > > preferable. Could someone from management side chime in on whether that
> > > is sufficient?
> >
> > I did consider it (it would be simpler, but not much), however unless we disable
> > PCI hotplug for affected slots it won't work. (unplug device from such slot and
> > plug another in that place will still return boot time index.
>
> Well it's interesting that you mention it. Let's say you remove
> a device then plug in a different index immediately ...
> Guest gets a wrong name temporarily, right?
> Should we worry about that?
guest will get not a wrong name but rather name that user asked for.
(i.e. NIC with old index could be moved to other slot and still keep its name)
> > That's why I ended up with hotplug variant.
> >
> > I chose abusing existing PCI hotplug registers for it,
> > but we can use a new range if that's preferred.
>
> Would it work for multi-function devices though?
> If yes that's a big advantage ...
Does ACPI hotplug work with multi-function devices (I thought not)?
If not, I'd prefer explicitly disable new feature
for multi-function devices (the code is heavily based on
ACPI hotplug infrastructure we already have).
(
in other words, I don't see immediate benefit in grabbing new registers
)
> > > More questions:
> > >
> > > does all this affect windows guests at all?
> > I don't know (spec shows examples that reminded
> > me about NIC naming which Windows use(s|d)).
> > But I won't bet on it.
> > If I recall correctly, for e1000 NIC, it didn't made any
> > difference in naming (pre-existing guest image).
> >
> > > where does the "acpi index" terminology come from?
> > > the pci firmware spec talks about "instance number", right?
> > it comes from linux kernel (that's how it's named in sysfs)
> > and systemd/udev uses it. So I tried to avoid making up another
> > one.
>
> OTOH that's guest specific ... and does not seem to imply
> the limitations unless you look at the code ...
"instance number" looks to me too generic (it's fine in context of
specific chapter in spec). While linux kernel's variant looks to me as
as better option that fits the bill.
If you don't object, I'd prefer 'acpi-index'.
> >
> > > > For more detailed description/examples see patches [3-4/5]
> > > >
> > > > 1)
> > > > https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/
> > > >
> > > > Git repo for testing:
> > > > https://github.com/imammedo/qemu/branches acpi-index-rfc
> > > >
> > > > Igor Mammedov (5):
> > > > acpi: add aml_to_decimalstring() and aml_call6() helpers
> > > > tests: acpi: temporary whitelist DSDT changes
> > > > pci: introduce apci-index property for PCI device
> > > > pci: acpi: add _DSM method to PCI devices
> > > > tests: acpi: update expected data files
> > > >
> > > > include/hw/acpi/aml-build.h | 3 +
> > > > include/hw/acpi/pci.h | 1 +
> > > > include/hw/acpi/pcihp.h | 7 +-
> > > > include/hw/pci/pci.h | 1 +
> > > > tests/qtest/bios-tables-test-allowed-diff.h | 21 +++++
> > > > hw/acpi/aml-build.c | 28 +++++++
> > > > hw/acpi/pci.c | 84 ++++++++++++++++++++
> > > > hw/acpi/pcihp.c | 25 +++++-
> > > > hw/i386/acpi-build.c | 31 +++++++-
> > > > hw/pci/pci.c | 1 +
> > > > tests/data/acpi/pc/DSDT | Bin 5065 -> 6023 bytes
> > > > tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7348 bytes
> > > > tests/data/acpi/pc/DSDT.bridge | Bin 6924 -> 8689 bytes
> > > > tests/data/acpi/pc/DSDT.cphp | Bin 5529 -> 6487 bytes
> > > > tests/data/acpi/pc/DSDT.dimmpxm | Bin 6719 -> 7677 bytes
> > > > tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5990 bytes
> > > > tests/data/acpi/pc/DSDT.hpbrroot | Bin 3084 -> 3177 bytes
> > > > tests/data/acpi/pc/DSDT.ipmikcs | Bin 5137 -> 6095 bytes
> > > > tests/data/acpi/pc/DSDT.memhp | Bin 6424 -> 7382 bytes
> > > > tests/data/acpi/pc/DSDT.numamem | Bin 5071 -> 6029 bytes
> > > > tests/data/acpi/pc/DSDT.roothp | Bin 5261 -> 6324 bytes
> > > > tests/data/acpi/q35/DSDT | Bin 7801 -> 7863 bytes
> > > > tests/data/acpi/q35/DSDT.acpihmat | Bin 9126 -> 9188 bytes
> > > > tests/data/acpi/q35/DSDT.bridge | Bin 7819 -> 7911 bytes
> > > > tests/data/acpi/q35/DSDT.cphp | Bin 8265 -> 8327 bytes
> > > > tests/data/acpi/q35/DSDT.dimmpxm | Bin 9455 -> 9517 bytes
> > > > tests/data/acpi/q35/DSDT.ipmibt | Bin 7876 -> 7938 bytes
> > > > tests/data/acpi/q35/DSDT.memhp | Bin 9160 -> 9222 bytes
> > > > tests/data/acpi/q35/DSDT.mmio64 | Bin 8932 -> 9024 bytes
> > > > tests/data/acpi/q35/DSDT.numamem | Bin 7807 -> 7869 bytes
> > > > tests/data/acpi/q35/DSDT.tis | Bin 8407 -> 8468 bytes
> > > > 31 files changed, 197 insertions(+), 5 deletions(-)
> > > >
> > > > --
> > > > 2.27.0
> > >
> > >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 4/5] pci: acpi: add _DSM method to PCI devices
2020-12-22 23:39 ` [RFC 4/5] pci: acpi: add _DSM method to PCI devices Igor Mammedov
2021-01-13 12:13 ` Michael S. Tsirkin
@ 2021-01-26 11:16 ` Michael S. Tsirkin
2021-01-26 14:29 ` Igor Mammedov
1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2021-01-26 11:16 UTC (permalink / raw)
To: Igor Mammedov; +Cc: laine, jusual, qemu-devel
On Tue, Dec 22, 2020 at 06:39:33PM -0500, Igor Mammedov wrote:
> Implement _DSM according to:
> PCI Firmware Specification 3.1
> 4.6.7. DSM for Naming a PCI or PCI Express Device Under
> Operating Systems
> and wire it up to cold and hot-plugged PCI devices.
> Feature depends on ACPI hotplug being enabled (as that provides
> PCI devices descriptions in ACPI and MMIO registers that are
> reused to fetch acpi-index).
>
> acpi-index should work for
> - cold plugged NICs:
> $QEMU -evice e1000,acpi-index=100
> => 'eno100'
> - hot-plugged
> (monitor) device_add e1000,acpi-index=200,id=remove_me
> => 'eno200'
> - replugged
> (monitor) device_del remove_me
> (monitor) device_add e1000,acpi-index=1
> => 'eno1'
>
> Windows also sees index under "PCI Label Id" field in properties
> dialog but otherwise it doesn't seem to have any effect.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/pci.h | 1 +
> hw/acpi/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++
> hw/i386/acpi-build.c | 21 ++++++++++--
> 3 files changed, 97 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> index bf2a3ed0ba..5e1eb2a96a 100644
> --- a/include/hw/acpi/pci.h
> +++ b/include/hw/acpi/pci.h
> @@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
> } AcpiMcfgInfo;
>
> void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
> +Aml *aml_pci_device_dsm(void);
> #endif
> diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> index 07d5101d83..6d49d515d3 100644
> --- a/hw/acpi/pci.c
> +++ b/hw/acpi/pci.c
> @@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> AcpiPciHpState *s = opaque;
> return s->acpi_index;
> }
> +
> +Aml *aml_pci_device_dsm(void)
> +{
> + Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
s/UUID/uuid/ I think ...
And can we move ifctx things to the correct scope?
> + Aml *acpi_index = aml_local(0);
> + Aml *zero = aml_int(0);
> + Aml *bnum = aml_arg(4);
> + Aml *sun = aml_arg(5);
> +
> + method = aml_method("PDSM", 6, AML_SERIALIZED);
> +
> + /*
> + * PCI Firmware Specification 3.1
> + * 4.6. _DSM Definitions for PCI
> + */
> + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> + {
> + aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
> + ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
> + {
> + uint8_t byte_list[1];
> +
> + ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
> + {
> + /*
> + * advertise function 7 if device has acpi-index
> + */
> + ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
> + {
> + byte_list[0] =
> + 1 /* have supported functions */ |
> + 1 << 7 /* support for function 7 */
> + ;
> + aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx2, ifctx3);
> + }
> + aml_append(ifctx1, ifctx2);
> +
> + byte_list[0] = 0; /* nothing supported */
> + aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx, ifctx1);
> + elsectx = aml_else();
> + /*
> + * PCI Firmware Specification 3.1
> + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> + * Operating Systems
> + */
> + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
> + {
> + Aml *pkg = aml_package(2);
> + Aml *label = aml_local(2);
> + Aml *ret = aml_local(1);
> +
> + aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
> + aml_to_decimalstring(acpi_index, NULL), label));
> +
> + aml_append(pkg, zero);
> + aml_append(pkg, aml_string("placeholder"));
> + aml_append(ifctx1, aml_store(pkg, ret));
> + /*
> + * update apci-index to actual value
> + */
> + aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> + /*
> + * update device label to actual value
> + */
> + aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
> + aml_append(ifctx1, aml_return(ret));
> + }
> + aml_append(elsectx, ifctx1);
> + aml_append(ifctx, elsectx);
> + }
> + aml_append(method, ifctx);
> + return method;
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 27d2958e25..447ad39c35 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> );
> aml_append(dev, method);
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + aml_append(method,
> + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
> + aml_arg(2), aml_arg(3),
> + aml_name("BSEL"), aml_name("_SUN")))
> + );
> + aml_append(dev, method);
> aml_append(parent_scope, dev);
>
> build_append_pcihp_notify_entry(notify_method, slot);
> @@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
>
> + aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> + method = aml_method("_DSM", 4, AML_SERIALIZED);
> + aml_append(method,
> + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
> + aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
> + );
> + aml_append(dev, method);
> +
> if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
> /* add VGA specific AML methods */
> int s3d;
> @@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> aml_append(method, aml_return(aml_int(s3d)));
> aml_append(dev, method);
> } else if (hotplug_enabled_dev) {
> - /* add _SUN/_EJ0 to make slot hotpluggable */
> - aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> -
> + /* add _EJ0 to make slot hotpluggable */
> method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> aml_append(method,
> aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> @@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
> aml_append(method, aml_return(aml_local(0)));
> aml_append(scope, method);
>
> + aml_append(scope, aml_pci_device_dsm());
> +
> aml_append(table, scope);
> }
>
> --
> 2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC 4/5] pci: acpi: add _DSM method to PCI devices
2021-01-26 11:16 ` Michael S. Tsirkin
@ 2021-01-26 14:29 ` Igor Mammedov
0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-01-26 14:29 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: laine, jusual, qemu-devel
On Tue, 26 Jan 2021 06:16:50 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 22, 2020 at 06:39:33PM -0500, Igor Mammedov wrote:
> > Implement _DSM according to:
> > PCI Firmware Specification 3.1
> > 4.6.7. DSM for Naming a PCI or PCI Express Device Under
> > Operating Systems
> > and wire it up to cold and hot-plugged PCI devices.
> > Feature depends on ACPI hotplug being enabled (as that provides
> > PCI devices descriptions in ACPI and MMIO registers that are
> > reused to fetch acpi-index).
> >
> > acpi-index should work for
> > - cold plugged NICs:
> > $QEMU -evice e1000,acpi-index=100
> > => 'eno100'
> > - hot-plugged
> > (monitor) device_add e1000,acpi-index=200,id=remove_me
> > => 'eno200'
> > - replugged
> > (monitor) device_del remove_me
> > (monitor) device_add e1000,acpi-index=1
> > => 'eno1'
> >
> > Windows also sees index under "PCI Label Id" field in properties
> > dialog but otherwise it doesn't seem to have any effect.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > include/hw/acpi/pci.h | 1 +
> > hw/acpi/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++
> > hw/i386/acpi-build.c | 21 ++++++++++--
> > 3 files changed, 97 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h
> > index bf2a3ed0ba..5e1eb2a96a 100644
> > --- a/include/hw/acpi/pci.h
> > +++ b/include/hw/acpi/pci.h
> > @@ -34,4 +34,5 @@ typedef struct AcpiMcfgInfo {
> > } AcpiMcfgInfo;
> >
> > void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info);
> > +Aml *aml_pci_device_dsm(void);
> > #endif
> > diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c
> > index 07d5101d83..6d49d515d3 100644
> > --- a/hw/acpi/pci.c
> > +++ b/hw/acpi/pci.c
> > @@ -65,3 +65,81 @@ bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id)
> > AcpiPciHpState *s = opaque;
> > return s->acpi_index;
> > }
> > +
> > +Aml *aml_pci_device_dsm(void)
> > +{
> > + Aml *method, *UUID, *ifctx, *ifctx1, *ifctx2, *ifctx3, *elsectx;
>
> s/UUID/uuid/ I think ...
>
> And can we move ifctx things to the correct scope?
I'll try to do it.
>
> > + Aml *acpi_index = aml_local(0);
> > + Aml *zero = aml_int(0);
> > + Aml *bnum = aml_arg(4);
> > + Aml *sun = aml_arg(5);
> > +
> > + method = aml_method("PDSM", 6, AML_SERIALIZED);
> > +
> > + /*
> > + * PCI Firmware Specification 3.1
> > + * 4.6. _DSM Definitions for PCI
> > + */
> > + UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> > + ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > + {
> > + aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sun), acpi_index));
> > + ifctx1 = aml_if(aml_equal(aml_arg(2), zero));
> > + {
> > + uint8_t byte_list[1];
> > +
> > + ifctx2 = aml_if(aml_equal(aml_arg(1), aml_int(2)));
> > + {
> > + /*
> > + * advertise function 7 if device has acpi-index
> > + */
> > + ifctx3 = aml_if(aml_lnot(aml_equal(acpi_index, zero)));
> > + {
> > + byte_list[0] =
> > + 1 /* have supported functions */ |
> > + 1 << 7 /* support for function 7 */
> > + ;
> > + aml_append(ifctx3, aml_return(aml_buffer(1, byte_list)));
> > + }
> > + aml_append(ifctx2, ifctx3);
> > + }
> > + aml_append(ifctx1, ifctx2);
> > +
> > + byte_list[0] = 0; /* nothing supported */
> > + aml_append(ifctx1, aml_return(aml_buffer(1, byte_list)));
> > + }
> > + aml_append(ifctx, ifctx1);
> > + elsectx = aml_else();
> > + /*
> > + * PCI Firmware Specification 3.1
> > + * 4.6.7. _DSM for Naming a PCI or PCI Express Device Under
> > + * Operating Systems
> > + */
> > + ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(7)));
> > + {
> > + Aml *pkg = aml_package(2);
> > + Aml *label = aml_local(2);
> > + Aml *ret = aml_local(1);
> > +
> > + aml_append(ifctx1, aml_concatenate(aml_string("PCI Device "),
> > + aml_to_decimalstring(acpi_index, NULL), label));
> > +
> > + aml_append(pkg, zero);
> > + aml_append(pkg, aml_string("placeholder"));
> > + aml_append(ifctx1, aml_store(pkg, ret));
> > + /*
> > + * update apci-index to actual value
> > + */
> > + aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> > + /*
> > + * update device label to actual value
> > + */
> > + aml_append(ifctx1, aml_store(label, aml_index(ret, aml_int(1))));
> > + aml_append(ifctx1, aml_return(ret));
> > + }
> > + aml_append(elsectx, ifctx1);
> > + aml_append(ifctx, elsectx);
> > + }
> > + aml_append(method, ifctx);
> > + return method;
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 27d2958e25..447ad39c35 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -385,6 +385,13 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> > );
> > aml_append(dev, method);
> > + method = aml_method("_DSM", 4, AML_SERIALIZED);
> > + aml_append(method,
> > + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1),
> > + aml_arg(2), aml_arg(3),
> > + aml_name("BSEL"), aml_name("_SUN")))
> > + );
> > + aml_append(dev, method);
> > aml_append(parent_scope, dev);
> >
> > build_append_pcihp_notify_entry(notify_method, slot);
> > @@ -412,6 +419,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> > aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
> >
> > + aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > + method = aml_method("_DSM", 4, AML_SERIALIZED);
> > + aml_append(method,
> > + aml_return(aml_call6("PDSM", aml_arg(0), aml_arg(1), aml_arg(2),
> > + aml_arg(3), aml_name("BSEL"), aml_name("_SUN")))
> > + );
> > + aml_append(dev, method);
> > +
> > if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
> > /* add VGA specific AML methods */
> > int s3d;
> > @@ -434,9 +449,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> > aml_append(method, aml_return(aml_int(s3d)));
> > aml_append(dev, method);
> > } else if (hotplug_enabled_dev) {
> > - /* add _SUN/_EJ0 to make slot hotpluggable */
> > - aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> > -
> > + /* add _EJ0 to make slot hotpluggable */
> > method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> > aml_append(method,
> > aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> > @@ -1142,6 +1155,8 @@ static void build_piix4_pci_hotplug(Aml *table)
> > aml_append(method, aml_return(aml_local(0)));
> > aml_append(scope, method);
> >
> > + aml_append(scope, aml_pci_device_dsm());
> > +
> > aml_append(table, scope);
> > }
> >
> > --
> > 2.27.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-01-26 15:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-22 23:39 [RFC 0/5] pc: support user provided NIC naming/indexing Igor Mammedov
2020-12-22 23:39 ` [RFC 1/5] acpi: add aml_to_decimalstring() and aml_call6() helpers Igor Mammedov
2020-12-22 23:39 ` [RFC 2/5] tests: acpi: temporary whitelist DSDT changes Igor Mammedov
2020-12-22 23:39 ` [RFC 3/5] pci: introduce apci-index property for PCI device Igor Mammedov
2020-12-22 23:39 ` [RFC 4/5] pci: acpi: add _DSM method to PCI devices Igor Mammedov
2021-01-13 12:13 ` Michael S. Tsirkin
2021-01-15 0:23 ` Igor Mammedov
2021-01-26 11:16 ` Michael S. Tsirkin
2021-01-26 14:29 ` Igor Mammedov
2020-12-22 23:39 ` [RFC 5/5] tests: acpi: update expected data files Igor Mammedov
2021-01-13 12:09 ` [RFC 0/5] pc: support user provided NIC naming/indexing Michael S. Tsirkin
2021-01-15 1:59 ` Igor Mammedov
2021-01-17 10:59 ` Michael S. Tsirkin
2021-01-20 12:07 ` Igor Mammedov
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.