From b32bd763a1ca929677e22ae1c51cb3920921bdce Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 15 Mar 2021 14:00:58 -0400 Subject: 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 --- include/hw/acpi/pcihp.h | 9 +++++++-- include/hw/pci/pci.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h index dfd3758..2dd90ae 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 1bc2314..6be4e0c 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, -- cgit v1.1 From 910e4069710d854757c8fe8921dcff5b62dcd960 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 15 Mar 2021 14:01:00 -0400 Subject: acpi: add aml_to_decimalstring() and aml_call6() helpers it will be used by follow up patches Signed-off-by: Igor Mammedov Message-Id: <20210315180102.3008391-5-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/aml-build.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include') diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 380d3e3..e652106 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, -- cgit v1.1 From b7f23f62e40bb7bc87fe170471a31ab1fb8a0784 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 15 Mar 2021 14:01:01 -0400 Subject: pci: acpi: add _DSM method to PCI devices 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 -device e1000,acpi-index=100 => 'eno100' - hot-plugged (monitor) device_add e1000,acpi-index=200,id=remove_me => 'eno200' - re-plugged (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 Message-Id: <20210315180102.3008391-6-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/pci.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/hw/acpi/pci.h b/include/hw/acpi/pci.h index e514f17..b5deee0 100644 --- a/include/hw/acpi/pci.h +++ b/include/hw/acpi/pci.h @@ -35,4 +35,5 @@ typedef struct AcpiMcfgInfo { void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info, const char *oem_id, const char *oem_table_id); +Aml *aml_pci_device_dsm(void); #endif -- cgit v1.1 From 6c2b24d1d2b19cd330d971cdbc8e6b115dc97ca4 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Thu, 4 Mar 2021 11:55:51 +0100 Subject: acpi: Set proper maximum size for "etc/table-loader" blob The resizeable memory region / RAMBlock that is created for the cmd blob has a maximum size of whole host pages (e.g., 4k), because RAMBlocks work on full host pages. In addition, in i386 ACPI code: acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE); makes sure to align to multiples of 4k, padding with 0. For example, if our cmd_blob is created with a size of 2k, the maximum size is 4k - we cannot grow beyond that. Growing might be required due to guest action when rebuilding the tables, but also on incoming migration. This automatic generation of the maximum size used to be sufficient, however, there are cases where we cross host pages now when growing at runtime: we exceed the maximum size of the RAMBlock and can crash QEMU when trying to resize the resizeable memory region / RAMBlock: $ build/qemu-system-x86_64 --enable-kvm \ -machine q35,nvdimm=on \ -smp 1 \ -cpu host \ -m size=2G,slots=8,maxmem=4G \ -object memory-backend-file,id=mem0,mem-path=/tmp/nvdimm,size=256M \ -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \ -nodefaults \ -device vmgenid \ -device intel-iommu Results in: Unexpected error in qemu_ram_resize() at ../softmmu/physmem.c:1850: qemu-system-x86_64: Size too large: /rom@etc/table-loader: 0x2000 > 0x1000: Invalid argument In this configuration, we consume exactly 4k (32 entries, 128 bytes each) when creating the VM. However, once the guest boots up and maps the MCFG, we also create the MCFG table and end up consuming 2 additional entries (pointer + checksum) -- which is where we try resizing the memory region / RAMBlock, however, the maximum size does not allow for it. Currently, we get the following maximum sizes for our different mutable tables based on behavior of resizeable RAMBlock: hw table max_size ------- --------------------------------------------------------- virt "etc/acpi/tables" ACPI_BUILD_TABLE_MAX_SIZE (0x200000) virt "etc/table-loader" HOST_PAGE_ALIGN(initial_size) virt "etc/acpi/rsdp" HOST_PAGE_ALIGN(initial_size) i386 "etc/acpi/tables" ACPI_BUILD_TABLE_MAX_SIZE (0x200000) i386 "etc/table-loader" HOST_PAGE_ALIGN(initial_size) i386 "etc/acpi/rsdp" HOST_PAGE_ALIGN(initial_size) microvm "etc/acpi/tables" ACPI_BUILD_TABLE_MAX_SIZE (0x200000) microvm "etc/table-loader" HOST_PAGE_ALIGN(initial_size) microvm "etc/acpi/rsdp" HOST_PAGE_ALIGN(initial_size) Let's set the maximum table size for "etc/table-loader" to 64k, so we can properly grow at runtime, which should be good enough for the future. Migration is not concerned with the maximum size of a RAMBlock, only with the used size - so existing setups are not affected. Of course, we cannot migrate a VM that would have crash when started on older QEMU from new QEMU to older QEMU without failing early on the destination when synchronizing the RAM state: qemu-system-x86_64: Size too large: /rom@etc/table-loader: 0x2000 > 0x1000: Invalid argument qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram' qemu-system-x86_64: load of migration failed: Invalid argument We'll refactor the code next, to make sure we get rid of this implicit behavior for "etc/acpi/rsdp" as well and to make the code easier to grasp. Reviewed-by: Igor Mammedov Cc: Alistair Francis Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Peter Maydell Cc: Shannon Zhao Cc: Marcel Apfelbaum Cc: Paolo Bonzini Cc: Richard Henderson Cc: Laszlo Ersek Signed-off-by: David Hildenbrand Message-Id: <20210304105554.121674-2-david@redhat.com> Reviewed-by: Laszlo Ersek Reviewed-by: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/aml-build.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index e652106..ca781f3 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -6,6 +6,7 @@ /* Reserve RAM space for tables: add another order of magnitude. */ #define ACPI_BUILD_TABLE_MAX_SIZE 0x200000 +#define ACPI_BUILD_LOADER_MAX_SIZE 0x10000 #define ACPI_BUILD_APPNAME6 "BOCHS " #define ACPI_BUILD_APPNAME8 "BXPC " -- cgit v1.1 From 6930ba0d44b2f4420948aec5209f665385411f7f Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Thu, 4 Mar 2021 11:55:53 +0100 Subject: acpi: Move maximum size logic into acpi_add_rom_blob() We want to have safety margins for all tables based on the table type. Let's move the maximum size logic into acpi_add_rom_blob() and make it dependent on the table name, so we don't have to replicate for each and every instance that creates such tables. Suggested-by: Laszlo Ersek Cc: Alistair Francis Cc: Paolo Bonzini Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: Peter Maydell Cc: Shannon Zhao Cc: Marcel Apfelbaum Cc: Paolo Bonzini Cc: Richard Henderson Cc: Laszlo Ersek Signed-off-by: David Hildenbrand Message-Id: <20210304105554.121674-4-david@redhat.com> Reviewed-by: Laszlo Ersek Reviewed-by: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/aml-build.h | 4 ---- include/hw/acpi/utils.h | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) (limited to 'include') diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index ca781f3..471266d 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -4,10 +4,6 @@ #include "hw/acpi/acpi-defs.h" #include "hw/acpi/bios-linker-loader.h" -/* Reserve RAM space for tables: add another order of magnitude. */ -#define ACPI_BUILD_TABLE_MAX_SIZE 0x200000 -#define ACPI_BUILD_LOADER_MAX_SIZE 0x10000 - #define ACPI_BUILD_APPNAME6 "BOCHS " #define ACPI_BUILD_APPNAME8 "BXPC " diff --git a/include/hw/acpi/utils.h b/include/hw/acpi/utils.h index 140b4de..0022df0 100644 --- a/include/hw/acpi/utils.h +++ b/include/hw/acpi/utils.h @@ -4,6 +4,5 @@ #include "hw/nvram/fw_cfg.h" MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque, - GArray *blob, const char *name, - uint64_t max_size); + GArray *blob, const char *name); #endif -- cgit v1.1 From d07b22863b8e0981bdc9384a787a703f1fd4ba42 Mon Sep 17 00:00:00 2001 From: Marian Postevca Date: Sun, 21 Feb 2021 02:17:36 +0200 Subject: acpi: Move setters/getters of oem fields to X86MachineState The code that sets/gets oem fields is duplicated in both PC and MICROVM variants. This commit moves it to X86MachineState so that all x86 variants can use it and duplication is removed. Signed-off-by: Marian Postevca Message-Id: <20210221001737.24499-2-posteuca@mutex.one> Reviewed-by: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/i386/microvm.h | 4 ---- include/hw/i386/pc.h | 4 ---- include/hw/i386/x86.h | 4 ++++ 3 files changed, 4 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h index 372b057..f25f837 100644 --- a/include/hw/i386/microvm.h +++ b/include/hw/i386/microvm.h @@ -76,8 +76,6 @@ #define MICROVM_MACHINE_ISA_SERIAL "isa-serial" #define MICROVM_MACHINE_OPTION_ROMS "x-option-roms" #define MICROVM_MACHINE_AUTO_KERNEL_CMDLINE "auto-kernel-cmdline" -#define MICROVM_MACHINE_OEM_ID "oem-id" -#define MICROVM_MACHINE_OEM_TABLE_ID "oem-table-id" struct MicrovmMachineClass { X86MachineClass parent; @@ -106,8 +104,6 @@ struct MicrovmMachineState { Notifier machine_done; Notifier powerdown_req; struct GPEXConfig gpex; - char *oem_id; - char *oem_table_id; }; #define TYPE_MICROVM_MACHINE MACHINE_TYPE_NAME("microvm") diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index d4c3d73..dcf060b 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -46,8 +46,6 @@ typedef struct PCMachineState { bool pit_enabled; bool hpet_enabled; uint64_t max_fw_size; - char *oem_id; - char *oem_table_id; /* NUMA information: */ uint64_t numa_nodes; @@ -65,8 +63,6 @@ typedef struct PCMachineState { #define PC_MACHINE_SATA "sata" #define PC_MACHINE_PIT "pit" #define PC_MACHINE_MAX_FW_SIZE "max-fw-size" -#define PC_MACHINE_OEM_ID "oem-id" -#define PC_MACHINE_OEM_TABLE_ID "oem-table-id" /** * PCMachineClass: * diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index 56080bd..26c9cc4 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -67,6 +67,8 @@ struct X86MachineState { OnOffAuto smm; OnOffAuto acpi; + char *oem_id; + char *oem_table_id; /* * Address space used by IOAPIC device. All IOAPIC interrupts * will be translated to MSI messages in the address space. @@ -76,6 +78,8 @@ struct X86MachineState { #define X86_MACHINE_SMM "smm" #define X86_MACHINE_ACPI "acpi" +#define X86_MACHINE_OEM_ID "oem-id" +#define X86_MACHINE_OEM_TABLE_ID "oem-table-id" #define TYPE_X86_MACHINE MACHINE_TYPE_NAME("x86") OBJECT_DECLARE_TYPE(X86MachineState, X86MachineClass, X86_MACHINE) -- cgit v1.1