From b32bd763a1ca929677e22ae1c51cb3920921bdce Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 15 Mar 2021 14:00:58 -0400 Subject: [PATCH] pci: introduce acpi-index property for PCI device In x86/ACPI world, 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 itm on has to plug NIC in exactly 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 to reshape PCI configuration of VM without need to reconfigure used guest image. systemd also offers "onboard" naming scheme which is preferred 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 require any advance configuration on guest side to com in effect at 'onboard' scheme takes priority over path based naming. Hope is that 'acpi-index' it will be easier to consume by management layer, compared to forcing specific 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 NIC. This patch adds, 'acpi-index'* property and wires up a 32bit register on top of pci hotplug register block 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 Message-Id: <20210315180102.3008391-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/pci.c | 1 - hw/acpi/pcihp.c | 58 +++++++++++++++++++++++++++++++++++++++-- hw/acpi/piix4.c | 3 ++- hw/acpi/trace-events | 2 ++ hw/i386/acpi-build.c | 13 ++++++++- hw/pci/pci.c | 1 + include/hw/acpi/pcihp.h | 9 +++++-- include/hw/pci/pci.h | 1 + 8 files changed, 81 insertions(+), 7 deletions(-) diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c index ec455c3b25..75b1103ec4 100644 --- a/hw/acpi/pci.c +++ b/hw/acpi/pci.c @@ -59,4 +59,3 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info, build_header(linker, table_data, (void *)(table_data->data + mcfg_start), "MCFG", table_data->len - mcfg_start, 1, oem_id, oem_table_id); } - diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index 9dc4d3e2db..ceab287bd3 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -39,12 +39,13 @@ #include "trace.h" #define ACPI_PCIHP_ADDR 0xae00 -#define ACPI_PCIHP_SIZE 0x0014 +#define ACPI_PCIHP_SIZE 0x0018 #define PCI_UP_BASE 0x0000 #define PCI_DOWN_BASE 0x0004 #define PCI_EJ_BASE 0x0008 #define PCI_RMV_BASE 0x000c #define PCI_SEL_BASE 0x0010 +#define PCI_AIDX_BASE 0x0014 typedef struct AcpiPciHpFind { int bsel; @@ -251,9 +252,13 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off) acpi_pcihp_update(s); } +#define ONBOARD_INDEX_MAX (16 * 1024 - 1) + void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { + PCIDevice *pdev = PCI_DEVICE(dev); + /* Only hotplugged devices need the hotplug capability. */ if (dev->hotplugged && acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))) < 0) { @@ -261,6 +266,17 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev, ACPI_PCIHP_PROP_BSEL "' set"); return; } + + /* + * capped by systemd (see: udev-builtin-net_id.c) + * as it's the only known user honor it to avoid users + * misconfigure QEMU and then wonder why acpi-index doesn't work + */ + if (pdev->acpi_index > ONBOARD_INDEX_MAX) { + error_setg(errp, "acpi-index should be less or equal to %u", + ONBOARD_INDEX_MAX); + return; + } } void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, @@ -347,7 +363,6 @@ 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 */ trace_acpi_pci_features_read(val); break; case PCI_RMV_BASE: @@ -357,6 +372,12 @@ static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size) case PCI_SEL_BASE: val = s->hotplug_select; trace_acpi_pci_sel_read(val); + break; + case PCI_AIDX_BASE: + val = s->acpi_index; + s->acpi_index = 0; + trace_acpi_pci_acpi_index_read(val); + break; default: break; } @@ -367,8 +388,35 @@ 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_AIDX_BASE: + /* + * fetch acpi-index for specified slot so that follow up read from + * PCI_AIDX_BASE can return it to guest + */ + 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; + } + } + trace_acpi_pci_acpi_index_write(s->hotplug_select, slot, s->acpi_index); + break; case PCI_EJ_BASE: if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) { break; @@ -413,6 +461,12 @@ void acpi_pcihp_init(Object *owner, AcpiPciHpState *s, PCIBus *root_bus, OBJ_PROP_FLAG_READ); } +bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id) +{ + AcpiPciHpState *s = opaque; + return s->acpi_index; +} + const VMStateDescription vmstate_acpi_pcihp_pci_status = { .name = "acpi_pcihp_pci_status", .version_id = 1, diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 1efc0ded9f..6056d51667 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -297,7 +297,8 @@ static const VMStateDescription vmstate_acpi = { 2, vmstate_pci_status, struct AcpiPciHpPciStatus), VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState, - vmstate_test_use_acpi_hotplug_bridge), + vmstate_test_use_acpi_hotplug_bridge, + vmstate_acpi_pcihp_use_acpi_index), VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription*[]) { diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events index f91ced477d..dcc1438f3a 100644 --- a/hw/acpi/trace-events +++ b/hw/acpi/trace-events @@ -41,6 +41,8 @@ acpi_pci_unplug_request(int bsel, int slot) "bsel: %d slot: %d" acpi_pci_up_read(uint32_t val) "%" PRIu32 acpi_pci_down_read(uint32_t val) "%" PRIu32 acpi_pci_features_read(uint32_t val) "%" PRIu32 +acpi_pci_acpi_index_read(uint32_t val) "%" PRIu32 +acpi_pci_acpi_index_write(unsigned bsel, unsigned slot, uint32_t aidx) "bsel: %u slot: %u aidx: %" PRIu32 acpi_pci_rmv_read(uint32_t val) "%" PRIu32 acpi_pci_sel_read(uint32_t val) "%" PRIu32 acpi_pci_ej_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64 diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 442b4629a9..e49fae2bfd 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1168,9 +1168,10 @@ static void build_piix4_pci_hotplug(Aml *table) aml_append(scope, field); aml_append(scope, - aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04)); + aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x08)); field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS); aml_append(field, aml_named_field("BNUM", 32)); + aml_append(field, aml_named_field("PIDX", 32)); aml_append(scope, field); aml_append(scope, aml_mutex("BLCK", 0)); @@ -1184,6 +1185,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("PIDX"))); + aml_append(method, aml_store(aml_name("PIDX"), 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 0eadcdbc9e..ac9a24889c 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -79,6 +79,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, 0), DEFINE_PROP_END_OF_LIST() }; diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h index dfd375820f..2dd90aea30 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,13 +72,17 @@ void acpi_pcihp_reset(AcpiPciHpState *s, bool acpihp_root_off); extern const VMStateDescription vmstate_acpi_pcihp_pci_status; -#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp) \ +bool vmstate_acpi_pcihp_use_acpi_index(void *opaque, int version_id); + +#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \ VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \ test_pcihp), \ VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \ ACPI_PCIHP_MAX_HOTPLUG_BUS, \ test_pcihp, 1, \ vmstate_acpi_pcihp_pci_status, \ - AcpiPciHpPciStatus) + AcpiPciHpPciStatus), \ + VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \ + test_acpi_index) #endif diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 1bc231480f..6be4e0c460 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -359,6 +359,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,