From eee822e3595bbdd69e71198edd65dd29db27a6e5 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 12 Mar 2014 16:13:58 +0200 Subject: [PATCH 01/15] 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(-) 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(); From 3dd46eb4969baaf32f490eaf57ed23672f1daad2 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 10 Mar 2014 21:13:59 +0200 Subject: [PATCH 02/15] acpi-test: update expected SSDT files commit 13f65b2e1073cf7e2c8fb3880c77d8a53fa2f95e acpi-test: update expected SSDT files set an incorrect SSDT. rebuild it. Signed-off-by: Michael S. Tsirkin --- tests/acpi-test-data/pc/SSDT | Bin 2363 -> 2261 bytes tests/acpi-test-data/q35/SSDT | Bin 652 -> 550 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT index ae5a9a57d63c5bdc72b1a16c3ce36a9516143fbb..6444f60ce786da7000d23b11d796cca579747f0c 100644 GIT binary patch delta 40 wcmdljbXAZmIM^lRDhC4tBwDsI`$Rl99Wahao=LDL%T{iFI-wW58q;rVIeGNC|NO delta 142 zcmZ3+(! Date: Fri, 14 Mar 2014 16:33:50 -0300 Subject: [PATCH 03/15] acpi: Add ACPI_CPU_HOTPLUG_ID_LIMIT macro The new macro will be helpful to allow us to detect too large SMP limits before it is too late. Signed-off-by: Eduardo Habkost Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/cpu_hotplug_defs.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/hw/acpi/cpu_hotplug_defs.h b/include/hw/acpi/cpu_hotplug_defs.h index 2725b50aac..9f33663511 100644 --- a/include/hw/acpi/cpu_hotplug_defs.h +++ b/include/hw/acpi/cpu_hotplug_defs.h @@ -17,7 +17,15 @@ * between C and ASL code. */ #define ACPI_CPU_HOTPLUG_STATUS 4 + +/* Limit for CPU arch IDs for CPU hotplug. All hotpluggable CPUs should + * have CPUClass.get_arch_id() < ACPI_CPU_HOTPLUG_ID_LIMIT. + */ +#define ACPI_CPU_HOTPLUG_ID_LIMIT 256 + +/* 256 CPU IDs, 8 bits per entry: */ #define ACPI_GPE_PROC_LEN 32 + #define ICH9_CPU_HOTPLUG_IO_BASE 0x0CD8 #define PIIX4_CPU_HOTPLUG_IO_BASE 0xaf00 From 5ff020b7b02dce36a66c106df986ff68f8452542 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:51 -0300 Subject: [PATCH 04/15] 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(+) 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); From 39ee3af3a85fedb55b9eeb1a0bc81a2460eeaa01 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:52 -0300 Subject: [PATCH 05/15] acpi: Assert sts array limit on AcpiCpuHotplug_add() AcpiCpuHotplug_add() can't handle vCPU arch IDs larger than ACPI_CPU_HOTPLUG_ID_LIMIT. Instead of corrupting memory in case the vCPU ID is too large, use g_assert() to ensure we are not over the limit. Signed-off-by: Eduardo Habkost Reviewed-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/cpu_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 48928dc0ea..2ad83a0ede 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -43,6 +43,7 @@ void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu) *gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS; cpu_id = k->get_arch_id(CPU(cpu)); + g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN); g->sts[cpu_id / 8] |= (1 << (cpu_id % 8)); } From 798325ed3856bc1e2f2b640b7e0db60044fdddf9 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:53 -0300 Subject: [PATCH 06/15] 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(-) 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); } From f03bd716a2935532379cff1c71c6f0f399921b70 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 14 Mar 2014 16:33:54 -0300 Subject: [PATCH 07/15] 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(+) 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); From 9bcc80cd71892df42605e0c097d85c0237ff45d1 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Mon, 17 Mar 2014 17:05:16 +0100 Subject: [PATCH 08/15] 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(-) 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); From d07e0e9cddf02dd2abedbbf7ab0e069c8f5dabfd Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 18 Mar 2014 16:14:59 +0200 Subject: [PATCH 09/15] acpi-test: rebuild SSDT commit 9bcc80cd71892df42605e0c097d85c0237ff45d1 i386/acpi-build: allow more than 255 elements in CPON Replaces 0x1 with a smaller One constant. rebuild expected SSDT. Signed-off-by: Michael S. Tsirkin --- tests/acpi-test-data/pc/SSDT | Bin 2261 -> 2261 bytes tests/acpi-test-data/q35/SSDT | Bin 550 -> 550 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT index 6444f60ce786da7000d23b11d796cca579747f0c..727853eece80ef2d163730506f987de181f69d5e 100644 GIT binary patch delta 26 icmcaAcvX-qIM^lRDhC4t Date: Mon, 17 Mar 2014 17:05:17 +0100 Subject: [PATCH 10/15] 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(-) 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)); From c225aa3c6de64f61fe68de2e5f4e3897544daf0d Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 18 Mar 2014 16:42:05 +0200 Subject: [PATCH 11/15] acpi-test: signature endian-ness fixes acpi table signature is really an ASCII string. Treat it as such in tests. Signed-off-by: Michael S. Tsirkin --- tests/acpi-test.c | 48 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/tests/acpi-test.c b/tests/acpi-test.c index 185309a241..249fe03fd7 100644 --- a/tests/acpi-test.c +++ b/tests/acpi-test.c @@ -23,7 +23,6 @@ #define MACHINE_Q35 "q35" #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML" -#define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */ /* DSDT and SSDTs format */ typedef struct { @@ -101,6 +100,20 @@ typedef struct { ACPI_READ_FIELD((table)->asl_compiler_revision, addr); \ } while (0); +#define ACPI_ASSERT_CMP(actual, expected) do { \ + uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \ + char ACPI_ASSERT_CMP_str[5] = {}; \ + memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 4); \ + g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \ +} while (0) + +#define ACPI_ASSERT_CMP64(actual, expected) do { \ + uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \ + char ACPI_ASSERT_CMP_str[9] = {}; \ + memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 8); \ + g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \ +} while (0) + /* Boot sector code: write SIGNATURE into memory, * then halt. * Q35 machine requires a minimum 0x7e000 bytes disk. @@ -213,7 +226,7 @@ static void test_acpi_rsdp_table(test_data *data) uint32_t addr = data->rsdp_addr; ACPI_READ_FIELD(rsdp_table->signature, addr); - g_assert_cmphex(rsdp_table->signature, ==, ACPI_RSDP_SIGNATURE); + ACPI_ASSERT_CMP64(rsdp_table->signature, "RSD PTR "); ACPI_READ_FIELD(rsdp_table->checksum, addr); ACPI_READ_ARRAY(rsdp_table->oem_id, addr); @@ -235,7 +248,7 @@ static void test_acpi_rsdt_table(test_data *data) /* read the header */ ACPI_READ_TABLE_HEADER(rsdt_table, addr); - g_assert_cmphex(rsdt_table->signature, ==, ACPI_RSDT_SIGNATURE); + ACPI_ASSERT_CMP(rsdt_table->signature, "RSDT"); /* compute the table entries in rsdt */ tables_nr = (rsdt_table->length - sizeof(AcpiRsdtDescriptorRev1)) / @@ -304,7 +317,7 @@ static void test_acpi_fadt_table(test_data *data) ACPI_READ_FIELD(fadt_table->reserved4b, addr); ACPI_READ_FIELD(fadt_table->flags, addr); - g_assert_cmphex(fadt_table->signature, ==, ACPI_FACP_SIGNATURE); + ACPI_ASSERT_CMP(fadt_table->signature, "FACP"); g_assert(!acpi_checksum((uint8_t *)fadt_table, fadt_table->length)); } @@ -321,7 +334,7 @@ static void test_acpi_facs_table(test_data *data) ACPI_READ_FIELD(facs_table->flags, addr); ACPI_READ_ARRAY(facs_table->resverved3, addr); - g_assert_cmphex(facs_table->signature, ==, ACPI_FACS_SIGNATURE); + ACPI_ASSERT_CMP(facs_table->signature, "FACS"); } static void test_dst_table(AcpiSdtTable *sdt_table, uint32_t addr) @@ -348,7 +361,7 @@ static void test_acpi_dsdt_table(test_data *data) data->tables = g_array_new(false, true, sizeof(AcpiSdtTable)); test_dst_table(&dsdt_table, addr); - g_assert_cmphex(dsdt_table.header.signature, ==, ACPI_DSDT_SIGNATURE); + ACPI_ASSERT_CMP(dsdt_table.header.signature, "DSDT"); /* Place DSDT first */ g_array_append_val(data->tables, dsdt_table); @@ -383,8 +396,9 @@ static void dump_aml_files(test_data *data, bool rebuild) g_assert(sdt->aml); if (rebuild) { + uint32_t signature = cpu_to_le32(sdt->header.signature); aml_file = g_strdup_printf("%s/%s/%.4s", data_dir, data->machine, - (gchar *)&sdt->header.signature); + (gchar *)&signature); fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH); } else { @@ -406,9 +420,9 @@ static void dump_aml_files(test_data *data, bool rebuild) } } -static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature) +static bool compare_signature(AcpiSdtTable *sdt, const char *signature) { - return sdt->header.signature == signature; + return !memcmp(&sdt->header.signature, signature, 4); } static bool load_asl(GArray *sdts, AcpiSdtTable *sdt) @@ -427,12 +441,12 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt) /* build command line */ g_string_append_printf(command_line, " -p %s ", sdt->asl_file); - if (compare_signature(sdt, ACPI_DSDT_SIGNATURE) || - compare_signature(sdt, ACPI_SSDT_SIGNATURE)) { + if (compare_signature(sdt, "DSDT") || + compare_signature(sdt, "SSDT")) { for (i = 0; i < sdts->len; ++i) { temp = &g_array_index(sdts, AcpiSdtTable, i); - if (compare_signature(temp, ACPI_DSDT_SIGNATURE) || - compare_signature(temp, ACPI_SSDT_SIGNATURE)) { + if (compare_signature(temp, "DSDT") || + compare_signature(temp, "SSDT")) { g_string_append_printf(command_line, "-e %s ", temp->aml_file); } } @@ -495,13 +509,16 @@ static GArray *load_expected_aml(test_data *data) GArray *exp_tables = g_array_new(false, true, sizeof(AcpiSdtTable)); for (i = 0; i < data->tables->len; ++i) { AcpiSdtTable exp_sdt; + uint32_t signature; + sdt = &g_array_index(data->tables, AcpiSdtTable, i); memset(&exp_sdt, 0, sizeof(exp_sdt)); exp_sdt.header.signature = sdt->header.signature; + signature = cpu_to_le32(sdt->header.signature); aml_file = g_strdup_printf("%s/%s/%.4s", data_dir, data->machine, - (gchar *)&exp_sdt.header.signature); + (gchar *)&signature); exp_sdt.aml_file = aml_file; g_assert(g_file_test(aml_file, G_FILE_TEST_EXISTS)); ret = g_file_get_contents(aml_file, &exp_sdt.aml, @@ -543,12 +560,13 @@ static void test_acpi_asl(test_data *data) g_assert(!err || exp_err); if (g_strcmp0(asl->str, exp_asl->str)) { + uint32_t signature = cpu_to_le32(exp_sdt->header.signature); sdt->tmp_files_retain = true; exp_sdt->tmp_files_retain = true; fprintf(stderr, "acpi-test: Warning! %.4s mismatch. " "Actual [asl:%s, aml:%s], Expected [asl:%s, aml:%s].\n", - (gchar *)&exp_sdt->header.signature, + (gchar *)&signature, sdt->asl_file, sdt->aml_file, exp_sdt->asl_file, exp_sdt->aml_file); } From 821e3227863ea8db057190e578efa0f1f57ed9de Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 18 Mar 2014 15:49:41 +0200 Subject: [PATCH 12/15] 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(-) 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]; From d2995916ea262bca40788f275c7f53f1c0a0b606 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 18 Mar 2014 16:29:23 -0300 Subject: [PATCH 13/15] sysemu.h: Document what MAX_CPUMASK_BITS really limits Signed-off-by: Eduardo Habkost Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/sysemu/sysemu.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index b90df9ada1..90f192a074 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -133,7 +133,14 @@ extern uint8_t qemu_extra_params_fw[2]; extern QEMUClockType rtc_clock; #define MAX_NODES 64 + +/* The following shall be true for all CPUs: + * cpu->cpu_index < max_cpus <= MAX_CPUMASK_BITS + * + * Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS. + */ #define MAX_CPUMASK_BITS 255 + extern int nb_numa_nodes; extern uint64_t node_mem[MAX_NODES]; extern unsigned long *node_cpumask[MAX_NODES]; From af67ee9264c135f4b213b4bc1a3871c4e9ec7da3 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 18 Mar 2014 16:29:24 -0300 Subject: [PATCH 14/15] vl.c: Use MAX_CPUMASK_BITS macro instead of hardcoded constant Signed-off-by: Eduardo Habkost Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index c8a5bfa959..23e1dbd245 100644 --- a/vl.c +++ b/vl.c @@ -1413,7 +1413,7 @@ static void smp_parse(QemuOpts *opts) max_cpus = smp_cpus; } - if (max_cpus > 255) { + if (max_cpus > MAX_CPUMASK_BITS) { fprintf(stderr, "Unsupported number of maxcpus\n"); exit(1); } From dac23a6c05e543590508b48b8ed31d89b0c99c61 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Mon, 24 Mar 2014 12:02:33 +0200 Subject: [PATCH 15/15] tests/acpi-test: do not fail if iasl is broken There is an issue with iasl on big endian machines: It cannot disassemble acpi tables taken from little endian machines, so we cannot check the expected tables. The acpi test will check if the expected aml files can be disassembled, and will issue an warning not failing the test on those machines until this problem is solved by the acpica community. Signed-off-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/acpi-test.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tests/acpi-test.c b/tests/acpi-test.c index 249fe03fd7..76fbccfa4b 100644 --- a/tests/acpi-test.c +++ b/tests/acpi-test.c @@ -456,13 +456,12 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt) /* pass 'out' and 'out_err' in order to be redirected */ ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error); g_assert_no_error(error); - if (ret) { ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl, &sdt->asl_len, &error); g_assert(ret); g_assert_no_error(error); - g_assert(sdt->asl_len); + ret = (sdt->asl_len > 0); } g_free(out); @@ -560,15 +559,20 @@ static void test_acpi_asl(test_data *data) g_assert(!err || exp_err); if (g_strcmp0(asl->str, exp_asl->str)) { - uint32_t signature = cpu_to_le32(exp_sdt->header.signature); - sdt->tmp_files_retain = true; - exp_sdt->tmp_files_retain = true; - fprintf(stderr, - "acpi-test: Warning! %.4s mismatch. " - "Actual [asl:%s, aml:%s], Expected [asl:%s, aml:%s].\n", - (gchar *)&signature, - sdt->asl_file, sdt->aml_file, - exp_sdt->asl_file, exp_sdt->aml_file); + if (exp_err) { + fprintf(stderr, + "Warning! iasl couldn't parse the expected aml\n"); + } else { + uint32_t signature = cpu_to_le32(exp_sdt->header.signature); + sdt->tmp_files_retain = true; + exp_sdt->tmp_files_retain = true; + fprintf(stderr, + "acpi-test: Warning! %.4s mismatch. " + "Actual [asl:%s, aml:%s], Expected [asl:%s, aml:%s].\n", + (gchar *)&signature, + sdt->asl_file, sdt->aml_file, + exp_sdt->asl_file, exp_sdt->aml_file); + } } g_string_free(asl, true); g_string_free(exp_asl, true);