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

[Kevin O'Connor]

On Tue, Jan 26, 2010 at 10:52:12PM +0100, Jes Sorensen wrote:
> Read optional table of e820 entries from qemu_cfg

[...]
> --- 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;
>
> +}

I think defining accessor functions for every piece of data passed
through qemu-cfg interface is going to get tiring.  I'd prefer to
extend the existing qemu-cfg "file" interface for new content.

For example, add a helper with something like:

int qemu_cfg_get_file(const char *name, void *dest, int maxsize);
> if (kvm_para_available())
>    // 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);
> if (kvm_para_available()) {
>    u32 count;
>    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);
>        }

and then this becomes:

struct e820entry map[128];
int len = qemu_cfg_get_file("e820map", &map, sizeof(map));
if (len >= 0)
    for (i=0; i<len / sizeof(map[0]); i++)
        add_e820(map[i].start, map[i].size, map[i].type);

The advantage being that it should be possible to write one set of
helper functions in both qemu and seabios that can then be used to
pass arbitrary content.

As a side note, it should probably do the e820 map check even for qemu
users (ie, not just kvm).

-Kevin