[Qemu-devel] [PATCH] Fix ACPI Table Collision

[Qemu-devel] [PATCH] Fix ACPI Table Collision

[Beth Kon]

Signed-off-by: Beth Kon <eak@us.ibm.com>

A potential collision exists in the current rombios32.c between 2 ACPI 
table entries:

rsdt->table_offset_entry[i+4] = cpu_to_le32(addr); (when i == 0)  

and

rsdt->table_offset_entry[4] = cpu_to_le32(srat_addr);

This patch corrects the problem. I have built this and very primitively tested 
that -numa doesn't make anything explode, but have not tested additional 
acpi tables. I'm hope others who have been playing with this stuff would 
be more readily able to confirm that nothing breaks.

One thing I don't understand is why the line

 acpi_additional_tables(); /* resets cfg to required entry */

is needed. So maybe I'm missing some subtlety here. If someone could
explain I'd appreciate it. 



diff --git a/bios/rombios32.c b/bios/rombios32.c
index d8f6d4e..72f5e4c 100644
--- a/bios/rombios32.c
+++ b/bios/rombios32.c
@@ -47,6 +47,12 @@ typedef unsigned long long uint64_t;

 #define wbinvd() asm volatile("wbinvd")

+#ifdef BX_QEMU
+#define BASE_ACPI_TABLES   4
+#else
+#define BASE_ACPI_TABLES   3
+#endif
+
 #define CPUID_MSR (1 << 5)
 #define CPUID_APIC (1 << 9)
 #define CPUID_MTRR (1 << 12)
@@ -1294,12 +1300,9 @@ struct rsdp_descriptor         /* Root System Descriptor Pointer */
 struct rsdt_descriptor_rev1
 {
 	ACPI_TABLE_HEADER_DEF                           /* ACPI common table header */
-#ifdef BX_QEMU
-	uint32_t                             table_offset_entry [5]; /* Array of pointers to other */
-#else
-	uint32_t                             table_offset_entry [3]; /* Array of pointers to other */
-#endif
-			 /* ACPI tables */
+	uint32_t       table_offset_entry [BASE_ACPI_TABLES]; /* Array of pointers
+                                                                 * to other ACPI
+                                                                 * tables*/
 } __attribute__((__packed__));

 /*
@@ -1666,8 +1669,14 @@ void acpi_bios_init(void)

 #ifdef BX_QEMU
     external_tables = acpi_additional_tables();
+    qemu_cfg_select(QEMU_CFG_NUMA);
+    nb_numa_nodes = qemu_cfg_get64();
+    if (nb_numa_nodes > 0) {
+        external_tables++;
+    }
 #else
     external_tables = 0;
+    nb_numa_nodes = 0;
 #endif

     addr = base_addr = ram_size - ACPI_DATA_SIZE;
@@ -1693,12 +1702,6 @@ void acpi_bios_init(void)
     ssdt_addr = addr;
     ssdt = (void *)(addr);
     addr += acpi_build_processor_ssdt(ssdt);
-#ifdef BX_QEMU
-    qemu_cfg_select(QEMU_CFG_NUMA);
-    nb_numa_nodes = qemu_cfg_get64();
-#else
-    nb_numa_nodes = 0;
-#endif
     if (nb_numa_nodes > 0) {
         addr = (addr + 7) & ~7;
         srat_addr = addr;
@@ -1897,9 +1900,13 @@ void acpi_bios_init(void)
     acpi_additional_tables(); /* resets cfg to required entry */
     for(i = 0; i < external_tables; i++) {
         uint16_t len;
+        if (i == 0 && nb_numa_nodes > 0) {
+            rsdt->table_offset_entry[BASE_ACPI_TABLES] = cpu_to_le32(srat_addr);
+            continue;
+        }
         if(acpi_load_table(i, addr, &len) < 0)
             BX_PANIC("Failed to load ACPI table from QEMU\n");
-        rsdt->table_offset_entry[i+4] = cpu_to_le32(addr);
+        rsdt->table_offset_entry[i + BASE_ACPI_TABLES] = cpu_to_le32(addr);
         addr += len;
         if(addr >= ram_size)
             BX_PANIC("ACPI table overflow\n");
@@ -1912,11 +1919,9 @@ void acpi_bios_init(void)
     rsdt->table_offset_entry[2] = cpu_to_le32(ssdt_addr);
 #ifdef BX_QEMU
     rsdt->table_offset_entry[3] = cpu_to_le32(hpet_addr);
-    if (nb_numa_nodes > 0)
-        rsdt->table_offset_entry[4] = cpu_to_le32(srat_addr);
 #endif
     acpi_build_table_header((struct acpi_table_header *)rsdt, "RSDT",
-        rsdt_size - (nb_numa_nodes > 0? 0: sizeof(uint32_t)), 1);
+        rsdt_size, 1);

     acpi_tables_size = addr - base_addr;