Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 02:24:54PM +0100, Gerd Hoffmann wrote:
> On Mi, 2014-03-12 at 09:05 -0400, Gabriel L. Somlo wrote:
> > On Wed, Mar 12, 2014 at 09:27:18AM +0100, Gerd Hoffmann wrote:
> > > I think we should just use e820_table (see pc.c) here.  Loop over it and
> > > add a type 19 table for each ram region in there.
> > 
> > I'm assuming this should be another post-Seabios-compatibility patch,
> > at the end of the series, and I should still do the (start,size)
> > arithmetic cut'n'pasted from SeaBIOS first, right ?
> 
> You should get identical results with both methods.  It's just that the
> e820 method is more future proof, i.e. if the numa people add support
> for non-contignous memory some day we don't have to adapt the smbios
> code to handle it.

So, for now, I have the patch at the end of this email. 

The problem is, e820_get_entry matches on indices 1 and 2, so my two
type-19 tables get handles (0x1301, 0x1302), instead of (0x1300, 0x1301).

Then, the logic setting up type 20 tables has to figure out which
indices matched for below-4g and above-4g, so that we can tie them
back to the type 19 tables.

The numbering conventions won't match SeaBIOS anymore, and I need to
clean up the cut'n'paste type 20 arithmetic from SeaBIOS to match the
e820 change for type 19.

Is this something we can decouple from the other smbios patches to buy
me some extra time ? :)

Thanks,
--Gabriel


diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9c4623e..7c58e88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -611,6 +611,21 @@ int e820_add_entry(uint64_t address, uint64_t length, 
uint32_t type)
     return e820_entries;
 }
 
+int e820_num_entries(void)
+{
+    return e820_entries;
+}
+
+bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t 
*length)
+{
+    if (idx >= e820_entries || e820_table[idx].type != cpu_to_le32(type)) {
+      return false;
+    }
+    *address = le64_to_cpu(e820_table[idx].address);
+    *length = le64_to_cpu(e820_table[idx].length);
+    return true;
+}
+
 /* Calculates the limit to CPU APIC ID values
  *
  * This function returns the limit for the APIC ID value, so that all
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 06e3f61..15c022e 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -18,6 +18,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "hw/i386/pc.h"
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"
 
@@ -666,11 +667,15 @@ uint8_t *smbios_get_table(size_t *length)
                                ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
             smbios_build_type_17_table(i, size_mb);
         }
-        smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        for (i = 0; i < e820_num_entries(); i++) {
+            uint64_t address, length;
+            if (e820_get_entry(i, E820_RAM, &address, &length)) {
+                smbios_build_type_19_table(i, address >> 10, length >> 10);
+            }
+        }
         smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram >> 10);
         if (smbios_above_4g_ram) {
             uint32_t start_mb = 4096, size_mb, j;
-            smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
             for (j = 1, i = 0; i < memdev_count; i++, j++) {
                 size_mb = (i == memdev_count - 1) ?
                           ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9010246..44932b8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -239,6 +239,8 @@ uint16_t pvpanic_port(void);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+int e820_num_entries(void);
+bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_Q35_COMPAT_1_7 \
         PC_COMPAT_1_7, \