[PATCH 1/2] Clean up MADT Table Creation (v2)

[PATCH 1/2] Clean up MADT Table Creation (v2)

[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.

There has been much ado over the acpi_bios_init function recently. I had actually 
done a rewrite very similar to Gleb's, but Avi argued that the rewrite has to be 
more incremental. This patch contains minimal changes without any rewrite because 
the changes are kvm-only. The rewrite would better be a separate step, submitted to 
qemu and then merged into kvm.  

I am submitting the RSDT fix to kvm because the kvm and qemu RSDT implementation differs.
Again, as a separate rewrite effort, the kvm and qemu RSDT manipulation could be merged 
into one base as a later, separate step.

This patch will get MADT into reasonable enough shape for me to resubmit hpet patches
on top of it. After that, I'd be willing to submit incremental rewrite patches for 
acpi_bios_init to qemu, starting with MADT and RSDT.  

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

diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
index 369cbef..cdae363 100755
--- a/kvm/bios/rombios32.c
+++ b/kvm/bios/rombios32.c
@@ -86,6 +86,8 @@ typedef unsigned long long uint64_t;
 #define MTRRphysBase_MSR(reg) (0x200 + 2 * (reg))
 #define MTRRphysMask_MSR(reg) (0x200 + 2 * (reg) + 1)
 
+#define MAX_INT_OVERRIDES 16
+
 static inline void outl(int addr, int val)
 {
     asm volatile ("outl %1, %w0" : : "d" (addr), "a" (val));
@@ -1600,7 +1602,7 @@ void acpi_bios_init(void)
     uint32_t hpet_addr;
 #endif
     uint32_t base_addr, rsdt_addr, fadt_addr, addr, facs_addr, dsdt_addr, ssdt_addr;
-    uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size;
+    uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size, madt_end;
     uint32_t srat_addr,srat_size;
     uint16_t i, external_tables;
     int nb_numa_nodes;
@@ -1668,7 +1670,7 @@ void acpi_bios_init(void)
     madt_size = sizeof(*madt) +
         sizeof(struct madt_processor_apic) * MAX_CPUS +
 #ifdef BX_QEMU
-        sizeof(struct madt_io_apic) /* + sizeof(struct madt_int_override) */;
+        sizeof(struct madt_io_apic)  + sizeof(struct madt_int_override) * MAX_INT_OVERRIDES;
 #else
         sizeof(struct madt_io_apic);
 #endif
@@ -1786,8 +1788,9 @@ void acpi_bios_init(void)
                 continue;
             }
             int_override++;
-            madt_size += sizeof(struct madt_int_override);
         }
+        madt_end = (uint32_t)int_override;
+        madt_size = madt_end - madt_addr;
         acpi_build_table_header((struct acpi_table_header *)madt,
                                 "APIC", madt_size, 1);
     }

[PATCH 2/2] Clean up RSDT Table Creation (v2)

[Beth Kon]

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.

I moved the for() loop to the end of the code that adds table_offset_entry 
entries so I could add the check for overflow - || (nb_rsdt_entries > MAX_RSDT_ENTRIES)
This is not ideal. An ideal fix would require a rewrite of the rsdt build code, which 
I can do later and submit to qemu.

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


diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
index cdae363..7db91d8 100755
--- a/kvm/bios/rombios32.c
+++ b/kvm/bios/rombios32.c
@@ -1602,7 +1602,7 @@ void acpi_bios_init(void)
     uint32_t hpet_addr;
 #endif
     uint32_t base_addr, rsdt_addr, fadt_addr, addr, facs_addr, dsdt_addr, ssdt_addr;
-    uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size, madt_end;
+    uint32_t acpi_tables_size, madt_addr, madt_size, rsdt_size, madt_end, rsdt_end;
     uint32_t srat_addr,srat_size;
     uint16_t i, external_tables;
     int nb_numa_nodes;
@@ -1628,7 +1628,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;
@@ -1872,16 +1872,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 */
@@ -1894,9 +1884,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 */
+    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) || (nb_rsdt_entries > MAX_RSDT_ENTRIES))
+            BX_PANIC("ACPI table overflow\n");
+    }
 #endif
-    rsdt_size -= MAX_RSDT_ENTRIES * 4;
-    rsdt_size += nb_rsdt_entries * 4;
+    rsdt_end = (uint32_t)(&rsdt->table_offset_entry[nb_rsdt_entries]);
+    rsdt_size = rsdt_end - rsdt_addr;
     acpi_build_table_header((struct acpi_table_header *)rsdt, "RSDT",
                             rsdt_size, 1);
 

Re: [PATCH 1/2] Clean up MADT Table Creation (v2)

[Avi Kivity]

Beth Kon wrote:
> 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.
>
> There has been much ado over the acpi_bios_init function recently. I had actually 
> done a rewrite very similar to Gleb's, but Avi argued that the rewrite has to be 
> more incremental. This patch contains minimal changes without any rewrite because 
> the changes are kvm-only. The rewrite would better be a separate step, submitted to 
> qemu and then merged into kvm.  
>
> I am submitting the RSDT fix to kvm because the kvm and qemu RSDT implementation differs.
> Again, as a separate rewrite effort, the kvm and qemu RSDT manipulation could be merged 
> into one base as a later, separate step.
>
> This patch will get MADT into reasonable enough shape for me to resubmit hpet patches
> on top of it. After that, I'd be willing to submit incremental rewrite patches for 
> acpi_bios_init to qemu, starting with MADT and RSDT.  
>
>   

Applied both, thanks.

-- 
error compiling committee.c: too many arguments to function