diff mbox

Re: [PATCH] Seabios - read e820 table from qemu_cfg

Message ID 4B6FE80C.2060408@redhat.com
State New
Headers show

Commit Message

Jes Sorensen Feb. 8, 2010, 10:31 a.m. UTC 
On 01/28/10 05:39, Kevin O'Connor wrote:
> As a side note, it should probably do the e820 map check even for qemu
> users (ie, not just kvm).

Hi Kevin,

Here is an updated version of the patch which does the e820 read
unconditionally of  the return from kvm_para_available() so it should
work for coreboot too.

I haven't touched the file descriptor issue as I find it's a different
discussion.

Let me know what you think.

Cheers,
Jes

PS: I tried to subscribe to the seabios list but I never got an ack for
it :(
Read optional table of e820 entries from qemu_cfg

Read optional table of e820 entries through qemu_cfg, allowing QEMU to
provide the location of KVM's switch area etc. rather than rely on
hard coded values.

For now, fall back to the old hard coded values for the TSS and EPT
switch page for compatibility reasons. Compatibility code could
possibly be removed in the future.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

---
 src/paravirt.c |   17 +++++++++++++++++
 src/paravirt.h |    9 +++++++++
 src/post.c     |   13 ++++++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)

Comments

Kevin O'Connor Feb. 14, 2010, 3:16 a.m. UTC | #1 
On Mon, Feb 08, 2010 at 11:31:40AM +0100, Jes Sorensen wrote:
> On 01/28/10 05:39, Kevin O'Connor wrote:
> >As a side note, it should probably do the e820 map check even for qemu
> >users (ie, not just kvm).
> 
> Hi Kevin,
> 
> Here is an updated version of the patch which does the e820 read
> unconditionally of  the return from kvm_para_available() so it should
> work for coreboot too.
> 
> I haven't touched the file descriptor issue as I find it's a different
> discussion.

I'd prefer to use the "file" method - but I wont hold up your patch
for it.  If the host part of your patch is committed to qemu, I'll
commit the SeaBIOS part.

[...]
> +struct e820_entry {
> +    u64 address;
> +    u64 length;
> +    u32 type;
> +};

I find this struct to be easily confused with 'struct e820entry' in
memmap.h.  Both code should use the same struct, or the new struct
should clearly indicate it's for qemu (eg, "qemu_e820_entry").

Thanks,
-Kevin
diff mbox

Patch

Index: seabios/src/paravirt.c
===================================================================
--- seabios.orig/src/paravirt.c
+++ seabios/src/paravirt.c
@@ -132,6 +132,23 @@  u16 qemu_cfg_smbios_entries(void)
     return cnt;
 }
 
+u32 qemu_cfg_e820_entries(void)
+{
+    u32 cnt;
+
+    if (!qemu_cfg_present)
+        return 0;
+
+    qemu_cfg_read_entry(&cnt, QEMU_CFG_E820_TABLE, sizeof(cnt));
+    return cnt;
+}
+
+void* qemu_cfg_e820_load_next(void *addr)
+{
+    qemu_cfg_read(addr, sizeof(struct e820_entry));
+    return addr;
+}
+
 struct smbios_header {
     u16 length;
     u8 type;
Index: seabios/src/paravirt.h
===================================================================
--- seabios.orig/src/paravirt.h
+++ seabios/src/paravirt.h
@@ -36,6 +36,7 @@  static inline int kvm_para_available(voi
 #define QEMU_CFG_ACPI_TABLES		(QEMU_CFG_ARCH_LOCAL + 0)
 #define QEMU_CFG_SMBIOS_ENTRIES		(QEMU_CFG_ARCH_LOCAL + 1)
 #define QEMU_CFG_IRQ0_OVERRIDE		(QEMU_CFG_ARCH_LOCAL + 2)
+#define QEMU_CFG_E820_TABLE		(QEMU_CFG_ARCH_LOCAL + 3)
 
 extern int qemu_cfg_present;
 
@@ -61,8 +62,16 @@  typedef struct QemuCfgFile {
     char name[56];
 } QemuCfgFile;
 
+struct e820_entry {
+    u64 address;
+    u64 length;
+    u32 type;
+};
+
 u16 qemu_cfg_first_file(QemuCfgFile *entry);
 u16 qemu_cfg_next_file(QemuCfgFile *entry);
 u32 qemu_cfg_read_file(QemuCfgFile *entry, void *dst, u32 maxlen);
+u32 qemu_cfg_e820_entries(void);
+void* qemu_cfg_e820_load_next(void *addr);
 
 #endif
Index: seabios/src/post.c
===================================================================
--- seabios.orig/src/post.c
+++ seabios/src/post.c
@@ -135,10 +135,21 @@  ram_probe(void)
              , E820_RESERVED);
     add_e820(BUILD_BIOS_ADDR, BUILD_BIOS_SIZE, E820_RESERVED);
 
-    if (kvm_para_available())
+    u32 count = qemu_cfg_e820_entries();
+    if (count) {
+        struct e820_entry entry;
+        int i;
+
+        for (i = 0; i < count; i++) {
+            qemu_cfg_e820_load_next(&entry);
+            add_e820(entry.address, entry.length, entry.type);
+        }
+    } else if (kvm_para_available()) {
+        // Backwards compatibility - provide hard coded range.
         // 4 pages before the bios, 3 pages for vmx tss pages, the
         // other page for EPT real mode pagetable
         add_e820(0xfffbc000, 4*4096, E820_RESERVED);
+    }
 
     dprintf(1, "Ram Size=0x%08x (0x%08x%08x high)\n"
             , RamSize, (u32)(RamSizeOver4G >> 32), (u32)RamSizeOver4G);