Clean up MADT Table Creation

This patch is also based on the patch by Vincent Minet. It corrects the size
calculation of the RSDT, and checks for overflow of MAX_RSDT_ENTRIES, assuming that the external table entry count is contained within MAX_RSDT_ENTRIES.

Signed-off-by Beth Kon <[email protected]>

kvm/bios/rombios32.c +7 -11

index 7f62e4f..ac8f9c5 100755
@@ -1626,7 +1626,7 @@ void acpi_bios_init(void)
     addr = base_addr = ram_size - ACPI_DATA_SIZE;
     rsdt_addr = addr;
     rsdt = (void *)(addr);
-    rsdt_size = sizeof(*rsdt) + external_tables * 4;
+    rsdt_size = sizeof(*rsdt);
     addr += rsdt_size;
 
     fadt_addr = addr;

@@ -1873,16 +1873,6 @@ void acpi_bios_init(void)
                              "HPET", sizeof(*hpet), 1);
 #endif
 
-    acpi_additional_tables(); /* resets cfg to required entry */
-    for(i = 0; i < external_tables; i++) {
-        uint16_t len;
-        if(acpi_load_table(i, addr, &len) < 0)
-            BX_PANIC("Failed to load ACPI table from QEMU\n");
-        rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(addr);
-        addr += len;
-        if(addr >= ram_size)
-            BX_PANIC("ACPI table overflow\n");
-    }
 #endif
 
     /* RSDT */

@@ -1895,6 +1885,19 @@ void acpi_bios_init(void)
 //  rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(hpet_addr);
     if (nb_numa_nodes > 0)
         rsdt->table_offset_entry[nb_rsdt_entries++] = cpu_to_le32(srat_addr);
+    acpi_additional_tables(); /* resets cfg to required entry */
+    /* external_tables load must occur last to 
+     * properly check for MAX_RSDT_ENTRIES overflow.
+     */
+    for(i = 0; i < external_tables; i++) {
+        uint16_t len;

This patch is based on the recent patch from Vincent Minet. I split Vincent's
changes into 2 patches (to separate MADT and RSDT table cleanup, as suggested by
Marcelo) and added a bit to them. And to give credit where it is due, this
cleanup is also related to the patch Marcelo provided when the HPET addition tripped over the same problem. (Thanks again Marcelo :-)

This patch moves all the table layout calculations to the same area of
acpi_bios_init. This prevents corruption problems when, in the middle of
filling in the tables, the MADT table size grows. The idea is to do all the layout in one section, then fill things in afterwards. It also corrects a problem where the madt table was memset to 0 before the final size of the table had been determined.

Signed-off-by Beth Kon <[email protected]>

kvm/bios/rombios32.c +1 -0

index cbd5f15..7f62e4f 100755
@@ -1665,6 +1665,7 @@ void acpi_bios_init(void)
 
     addr = (addr + 7) & ~7;
     madt_addr = addr;
+    madt = (void *)(addr);
     madt_size = sizeof(*madt) +
         sizeof(struct madt_processor_apic) * MAX_CPUS +
 #ifdef BX_QEMU

@@ -1672,7 +1673,11 @@ void acpi_bios_init(void)
 #else
         sizeof(struct madt_io_apic);

Quoted 29 lines · Beth Kon

This patch is based on the recent patch from Vincent Minet. I split Vincent's
changes into 2 patches (to separate MADT and RSDT table cleanup, as suggested by
Marcelo) and added a bit to them. And to give credit where it is due, this
cleanup is also related to the patch Marcelo provided when the HPET addition tripped over the same problem. (Thanks again Marcelo :-)
This patch moves all the table layout calculations to the same area of
acpi_bios_init. This prevents corruption problems when, in the middle of
filling in the tables, the MADT table size grows. The idea is to do all the layout in one section, then fill things in afterwards. It also corrects a problem where the madt table was memset to 0 before the final size of the table had been determined.

Signed-off-by Beth Kon <[email protected]>

kvm/bios/rombios32.c +3 -1

index cbd5f15..7f62e4f 100755
@@ -1665,6 +1665,7 @@ void acpi_bios_init(void)
 
     addr = (addr + 7) & ~7;
     madt_addr = addr;
+    madt = (void *)(addr);
     madt_size = sizeof(*madt) +
         sizeof(struct madt_processor_apic) * MAX_CPUS +
 #ifdef BX_QEMU

@@ -1672,7 +1673,11 @@ void acpi_bios_init(void)
 #else
         sizeof(struct madt_io_apic);
 #endif
-    madt = (void *)(addr);
+    for ( i = 0; i < 16; i++ ) {
+        if ( PCI_ISA_IRQ_MASK & (1U << i) ) {

kvm/bios/rombios32.c void acpi_bios_init(void)

Quoted 6 lines

kvm/bios/rombios32.c +3 -0

+            madt_size += sizeof(struct madt_int_override);
+        }
+    }
     addr += madt_size;
 
 

You're just duplicating the override creation loop (with its internal if); if we update it, we'll have to update this too.

Why not set madt_end = int_override and calculate madt_size = madt_end
- madt?