[Qemu-devel] Re: [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