From eee822e3595bbdd69e71198edd65dd29db27a6e5 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 12 Mar 2014 16:13:58 +0200 Subject: acpi-build: fix misaligned access clang build reported a misaligned access: runtime error: store to misaligned address 0x2b5aa47dfb19 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment 0x2b5aa47dfb19: note: pointer points here 45 53 54 0b ff ff 5b 80 50 45 4f 52 01 50 45 53 54 01 5b 81 0b 50 45 4f 52 01 50 45 50 54 08 14 fix this up Reported-by: Peter Maydell Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/i386') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 7ecfd7004b..4d781a1687 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1019,8 +1019,8 @@ build_ssdt(GArray *table_data, GArray *linker, patch_pci_windows(pci, ssdt_ptr, sizeof(ssdp_misc_aml)); - *(uint16_t *)(ssdt_ptr + *ssdt_isa_pest) = - cpu_to_le16(misc->pvpanic_port); + ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml), + ssdt_isa_pest[0], 16, misc->pvpanic_port); { GArray *sb_scope = build_alloc_array(); -- cgit v1.2.3 From 5ff020b7b02dce36a66c106df986ff68f8452542 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:51 -0300 Subject: pc: Refuse CPU hotplug if the resulting APIC ID is too large The ACPI CPU hotplug code requires APIC IDs to be smaller than ACPI_CPU_HOTPLUG_ID_LIMIT, so enforce the limit before trying to hotplug a new vCPU, returning an error instead of crashing. Signed-off-by: Eduardo Habkost Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'hw/i386') diff --git a/hw/i386/pc.c b/hw/i386/pc.c index e715a3312d..74cb4f962c 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -53,6 +53,7 @@ #include "qemu/bitmap.h" #include "qemu/config-file.h" #include "hw/acpi/acpi.h" +#include "hw/acpi/cpu_hotplug.h" #include "hw/cpu/icc_bus.h" #include "hw/boards.h" #include "hw/pci/pci_host.h" @@ -974,6 +975,13 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) return; } + if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) { + error_setg(errp, "Unable to add CPU: %" PRIi64 + ", resulting APIC ID (%" PRIi64 ") is too large", + id, apic_id); + return; + } + icc_bridge = DEVICE(object_resolve_path_type("icc-bridge", TYPE_ICC_BRIDGE, NULL)); pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp); -- cgit v1.2.3 From 798325ed3856bc1e2f2b640b7e0db60044fdddf9 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:53 -0300 Subject: acpi: Don't use MAX_CPUMASK_BITS for APIC ID bitmap MAX_CPUMASK_BITS is a limit for max_cpus and CPU indexes, not for APIC IDs. ACPI_CPU_HOTPLUG_ID_LIMIT is the right macro for the limit on APIC IDs on the ACPI and CPU hotplug code. There are no functional changes introduced by this patch, as MAX_CPUMASK_BITS + 1 == 255 + 1 == 256 == ACPI_CPU_HOTPLUG_ID_LIMIT. Signed-off-by: Eduardo Habkost Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/i386') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 4d781a1687..41f3d8a426 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -52,7 +52,7 @@ #include "qom/qom-qobject.h" typedef struct AcpiCpuInfo { - DECLARE_BITMAP(found_cpus, MAX_CPUMASK_BITS + 1); + DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT); } AcpiCpuInfo; typedef struct AcpiMcfgInfo { @@ -117,7 +117,7 @@ int acpi_add_cpu_info(Object *o, void *opaque) if (object_dynamic_cast(o, TYPE_CPU)) { apic_id = object_property_get_int(o, "apic-id", NULL); - assert(apic_id <= MAX_CPUMASK_BITS); + assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT); set_bit(apic_id, cpu->found_cpus); } -- cgit v1.2.3 From f03bd716a2935532379cff1c71c6f0f399921b70 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:54 -0300 Subject: pc: Refuse max_cpus if it results in too large APIC ID This changes the PC initialization code to reject max_cpus if it results in an APIC ID that's too large, instead of aborting or erroring out when it is already too late. Signed-off-by: Eduardo Habkost Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'hw/i386') diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 74cb4f962c..14f0d91f76 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -992,6 +992,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) int i; X86CPU *cpu = NULL; Error *error = NULL; + unsigned long apic_id_limit; /* init CPUs */ if (cpu_model == NULL) { @@ -1003,6 +1004,13 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) } current_cpu_model = cpu_model; + apic_id_limit = pc_apic_id_limit(max_cpus); + if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { + error_report("max_cpus is too large. APIC ID of last CPU is %lu", + apic_id_limit - 1); + exit(1); + } + for (i = 0; i < smp_cpus; i++) { cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), icc_bridge, &error); -- cgit v1.2.3 From 9bcc80cd71892df42605e0c097d85c0237ff45d1 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Mon, 17 Mar 2014 17:05:16 +0100 Subject: i386/acpi-build: allow more than 255 elements in CPON The build_ssdt() function builds a number of AML objects that are related to CPU hotplug, and whose IDs form a contiguous sequence of APIC IDs. (APIC IDs are in fact discontiguous, but this is the traditional interface: build a contiguous sequence from zero up that covers all possible APIC IDs.) These objects are: - a Processor() object for each VCPU, - a NTFY method, with one branch for each VCPU, - a CPON package with one element (hotplug status byte) for each VCPU. The build_ssdt() function currently limits the *count* of processor objects, and NTFY branches, and CPON elements, in 0xFF (see the assignment to "acpi_cpus"). This allows for an inclusive APIC ID range of [0..254]. This is incorrect, because the highest APIC ID that we otherwise allow a VCPU to take is 255. In order to extend the maximum count to 256, and the traversed APIC ID range correspondingly to [0..255]: - the Processor() objects need no change, - the NTFY method also needs no change, - the CPON package must be updated, because it is defined with a DefPackage, and the number of elements in such a package can be at most 255. We pick a DefVarPackage instead. We replace the Op byte, and the encoding of the number of elements. Compare: DefPackage := PackageOp PkgLength NumElements PackageElementList DefVarPackage := VarPackageOp PkgLength VarNumElements PackageElementList PackageOp := 0x12 VarPackageOp := 0x13 NumElements := ByteData VarNumElements := TermArg => Integer The build_append_int() function implements precisely the following TermArg encodings (a subset of what the ACPI spec describes): TermArg := DataObject DataObject := ComputationalData ComputationalData := ConstObj | ByteConst | WordConst | DWordConst directly encoded in the function, with build_append_byte(): ConstObj := ZeroOp | OneOp ZeroOp := 0x00 OneOp := 0x01 call to build_append_value(..., 1): ByteConst := BytePrefix ByteData BytePrefix := 0x0A ByteData := 0x00 - 0xFF call to build_append_value(..., 2): WordConst := WordPrefix WordData WordPrefix := 0x0B WordData := ByteData[0:7] ByteData[8:15] call to build_append_value(..., 4): DWordConst := DWordPrefix DWordData DWordPrefix := 0x0C DWordData := WordData[0:15] WordData[16:31] Signed-off-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/i386') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 41f3d8a426..a5039d4e98 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1050,9 +1050,9 @@ build_ssdt(GArray *table_data, GArray *linker, { GArray *package = build_alloc_array(); - uint8_t op = 0x12; /* PackageOp */ + uint8_t op = 0x13; /* VarPackageOp */ - build_append_byte(package, acpi_cpus); /* NumElements */ + build_append_int(package, acpi_cpus); /* VarNumElements */ for (i = 0; i < acpi_cpus; i++) { uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00; build_append_byte(package, b); -- cgit v1.2.3 From 2fd71f1be2a2554b790fa63fc14b6b4c7683f1f3 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Mon, 17 Mar 2014 17:05:17 +0100 Subject: i386/acpi-build: support hotplug of VCPU with APIC ID 0xFF Building on the previous patch, raise the maximal count of processor objects / NTFY branches / CPON elements from 255 to 256. This allows the VCPU with APIC ID 0xFF to be hotplugged. Signed-off-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'hw/i386') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a5039d4e98..1dcfb25c28 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -999,11 +999,16 @@ build_ssdt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc, PcPciInfo *pci, PcGuestInfo *guest_info) { - int acpi_cpus = MIN(0xff, guest_info->apic_id_limit); + unsigned acpi_cpus = guest_info->apic_id_limit; int ssdt_start = table_data->len; uint8_t *ssdt_ptr; int i; + /* The current AML generator can cover the APIC ID range [0..255], + * inclusive, for VCPU hotplug. */ + QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256); + g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT); + /* Copy header and patch values in the S3_ / S4_ / S5_ packages */ ssdt_ptr = acpi_data_push(table_data, sizeof(ssdp_misc_aml)); memcpy(ssdt_ptr, ssdp_misc_aml, sizeof(ssdp_misc_aml)); -- cgit v1.2.3 From 821e3227863ea8db057190e578efa0f1f57ed9de Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 18 Mar 2014 15:49:41 +0200 Subject: acpi: fix endian-ness for table ids when using signature for table ID, we forgot to byte-swap it. signatures are really ASCII strings, let's treat them as such. While at it, get rid of most of _SIGNATURE macros. Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 31 ++++++++++++++++--------------- hw/i386/acpi-defs.h | 14 -------------- 2 files changed, 16 insertions(+), 29 deletions(-) (limited to 'hw/i386') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 1dcfb25c28..f1054dd831 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -226,14 +226,14 @@ static void acpi_get_pci_info(PcPciInfo *info) static void build_header(GArray *linker, GArray *table_data, - AcpiTableHeader *h, uint32_t sig, int len, uint8_t rev) + AcpiTableHeader *h, const char *sig, int len, uint8_t rev) { - h->signature = cpu_to_le32(sig); + memcpy(&h->signature, sig, 4); h->length = cpu_to_le32(len); h->revision = rev; memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6); memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4); - memcpy(h->oem_table_id + 4, (void *)&sig, 4); + memcpy(h->oem_table_id + 4, sig, 4); h->oem_revision = cpu_to_le32(1); memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4); h->asl_compiler_revision = cpu_to_le32(1); @@ -495,7 +495,7 @@ static void build_facs(GArray *table_data, GArray *linker, PcGuestInfo *guest_info) { AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs); - facs->signature = cpu_to_le32(ACPI_FACS_SIGNATURE); + memcpy(&facs->signature, "FACS", 4); facs->length = cpu_to_le32(sizeof(*facs)); } @@ -552,7 +552,7 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo *pm, fadt_setup(fadt, pm); build_header(linker, table_data, - (void *)fadt, ACPI_FACP_SIGNATURE, sizeof(*fadt), 1); + (void *)fadt, "FACP", sizeof(*fadt), 1); } static void @@ -621,7 +621,7 @@ build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu, local_nmi->lint = 1; /* ACPI_LINT1 */ build_header(linker, table_data, - (void *)(table_data->data + madt_start), ACPI_APIC_SIGNATURE, + (void *)(table_data->data + madt_start), "APIC", table_data->len - madt_start, 1); } @@ -1098,7 +1098,7 @@ build_ssdt(GArray *table_data, GArray *linker, build_header(linker, table_data, (void *)(table_data->data + ssdt_start), - ACPI_SSDT_SIGNATURE, table_data->len - ssdt_start, 1); + "SSDT", table_data->len - ssdt_start, 1); } static void @@ -1113,7 +1113,7 @@ build_hpet(GArray *table_data, GArray *linker) hpet->timer_block_id = cpu_to_le32(0x8086a201); hpet->addr.address = cpu_to_le64(HPET_BASE); build_header(linker, table_data, - (void *)hpet, ACPI_HPET_SIGNATURE, sizeof(*hpet), 1); + (void *)hpet, "HPET", sizeof(*hpet), 1); } static void @@ -1205,7 +1205,7 @@ build_srat(GArray *table_data, GArray *linker, build_header(linker, table_data, (void *)(table_data->data + srat_start), - ACPI_SRAT_SIGNATURE, + "SRAT", table_data->len - srat_start, 1); } @@ -1213,7 +1213,7 @@ static void build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info) { AcpiTableMcfg *mcfg; - uint32_t sig; + const char *sig; int len = sizeof(*mcfg) + 1 * sizeof(mcfg->allocation[0]); mcfg = acpi_data_push(table_data, len); @@ -1230,9 +1230,10 @@ build_mcfg_q35(GArray *table_data, GArray *linker, AcpiMcfgInfo *info) * ACPI spec requires OSPMs to ignore such tables. */ if (info->mcfg_base == PCIE_BASE_ADDR_UNMAPPED) { - sig = ACPI_RSRV_SIGNATURE; + /* Reserved signature: ignored by OSPM */ + sig = "QEMU"; } else { - sig = ACPI_MCFG_SIGNATURE; + sig = "MCFG"; } build_header(linker, table_data, (void *)mcfg, sig, len, 1); } @@ -1248,7 +1249,7 @@ build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc) memcpy(dsdt, misc->dsdt_code, misc->dsdt_size); memset(dsdt, 0, sizeof *dsdt); - build_header(linker, table_data, dsdt, ACPI_DSDT_SIGNATURE, + build_header(linker, table_data, dsdt, "DSDT", misc->dsdt_size, 1); } @@ -1273,7 +1274,7 @@ build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets) sizeof(uint32_t)); } build_header(linker, table_data, - (void *)rsdt, ACPI_RSDT_SIGNATURE, rsdt_len, 1); + (void *)rsdt, "RSDT", rsdt_len, 1); } static GArray * @@ -1284,7 +1285,7 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 1, true /* fseg memory */); - rsdp->signature = cpu_to_le64(ACPI_RSDP_SIGNATURE); + memcpy(&rsdp->signature, "RSD PTR ", 8); memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, 6); rsdp->rsdt_physical_address = cpu_to_le32(rsdt); /* Address to be filled by Guest linker */ diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h index 78ca20489f..e93babb026 100644 --- a/hw/i386/acpi-defs.h +++ b/hw/i386/acpi-defs.h @@ -52,8 +52,6 @@ struct Acpi20GenericAddress { } QEMU_PACKED; typedef struct Acpi20GenericAddress Acpi20GenericAddress; -#define ACPI_RSDP_SIGNATURE 0x2052545020445352LL // "RSD PTR " - struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */ uint64_t signature; /* ACPI signature, contains "RSD PTR " */ uint8_t checksum; /* To make sum of struct == 0 */ @@ -92,7 +90,6 @@ typedef struct AcpiTableHeader AcpiTableHeader; /* * ACPI 1.0 Fixed ACPI Description Table (FADT) */ -#define ACPI_FACP_SIGNATURE 0x50434146 // FACP struct AcpiFadtDescriptorRev1 { ACPI_TABLE_HEADER_DEF /* ACPI common table header */ @@ -141,7 +138,6 @@ typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1; /* * ACPI 1.0 Root System Description Table (RSDT) */ -#define ACPI_RSDT_SIGNATURE 0x54445352 // RSDT struct AcpiRsdtDescriptorRev1 { ACPI_TABLE_HEADER_DEF /* ACPI common table header */ @@ -153,7 +149,6 @@ typedef struct AcpiRsdtDescriptorRev1 AcpiRsdtDescriptorRev1; /* * ACPI 1.0 Firmware ACPI Control Structure (FACS) */ -#define ACPI_FACS_SIGNATURE 0x53434146 // FACS struct AcpiFacsDescriptorRev1 { uint32_t signature; /* ACPI Signature */ @@ -169,7 +164,6 @@ typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; /* * Differentiated System Description Table (DSDT) */ -#define ACPI_DSDT_SIGNATURE 0x54445344 // DSDT /* * MADT values and structures @@ -182,7 +176,6 @@ typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; /* Master MADT */ -#define ACPI_APIC_SIGNATURE 0x43495041 // APIC struct AcpiMultipleApicTable { ACPI_TABLE_HEADER_DEF /* ACPI common table header */ @@ -253,7 +246,6 @@ typedef struct AcpiMadtLocalNmi AcpiMadtLocalNmi; /* * HPET Description Table */ -#define ACPI_HPET_SIGNATURE 0x54455048 // HPET struct Acpi20Hpet { ACPI_TABLE_HEADER_DEF /* ACPI common table header */ uint32_t timer_block_id; @@ -268,7 +260,6 @@ typedef struct Acpi20Hpet Acpi20Hpet; * SRAT (NUMA topology description) table */ -#define ACPI_SRAT_SIGNATURE 0x54415253 // SRAT struct AcpiSystemResourceAffinityTable { ACPI_TABLE_HEADER_DEF @@ -316,11 +307,6 @@ struct AcpiMcfgAllocation { } QEMU_PACKED; typedef struct AcpiMcfgAllocation AcpiMcfgAllocation; -#define ACPI_MCFG_SIGNATURE 0x4746434d // MCFG - -/* Reserved signature: ignored by OSPM */ -#define ACPI_RSRV_SIGNATURE 0x554d4551 // QEMU - struct AcpiTableMcfg { ACPI_TABLE_HEADER_DEF; uint8_t reserved[8]; -- cgit v1.2.3