[SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg

Michael S. Tsirkin mst at redhat.com
Mon Apr 22 19:23:39 CEST 2013


On Mon, Apr 22, 2013 at 06:37:32PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote:
> > On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote:
> > > On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote:
> > > > Okay I'm pretty close to posting some patches
> > > > that advance this project further, but wanted to
> > > > check something beforehand: there are several tables
> > > > that point to other tables (for example: FADT points
> > > > to DSDT). What I did is provide a list of fixups
> > > > such that bios can patch in pointers without
> > > > any need to understand what's what.
> > > > Thoughts?
> > > 
> > > Great!
> > > 
> > > For the RSDP, RSDT, and FADT I think SeaBIOS should just update those
> > > tables to set the pointers within them and then recalculate the
> > > checksum.  I don't think anything complex is needed - it's easy for
> > > SeaBIOS to recognize those special tables and modify them.
> > > 
> > > -Kevin
> > 
> > True, that's simple enough.  My worry is we can add more such tables.
> > For example, we can decide to switch to XSDT in the future.
> > If you look at the latest ACPI spec, there are tons of
> > pointer tables not implemented in seabios.
> > If we need to add support in seabios as well as qemu
> > for each of them, this will kind of defeat the purpose.
> 
> And just to show how simple it can be, here's that patch in the
> series: basically ACPI tables will be in-memory, so all we need to do to
> link them together is keep a pointer from file to the table.
> Basically all linker needs is ability to add or subtract.
> What I like with this approach is host is in complete control of table
> layout. All BIOS does is allocate memory for them, it never changes the
> tables, including the checksum.
> Note that it's coded very defensively, which makes it
> a bit bigger than absolutely necessary, but hopefully
> more future proof.

Addressing some comments by Laszlo (sent off list)/.

> --->
> linker: utility to patch in-memory ROM files
> 
> Add ability for a ROM file to point to
> it's image in memory. When file is in memory,
> add utility that can patch it, storing
> pointers to one file within another file.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> 
> ---
> 
> Makefile     |    2 -
> src/linker.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/linker.h |   28 +++++++++++++++++++++
> src/util.h   |    1 
> 4 files changed, 108 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/Makefile b/Makefile
> index 759bbbb..020fb2f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -18,7 +18,7 @@ SRC16=$(SRCBOTH) system.c disk.c font.c
>  SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c pmm.c coreboot.c boot.c \
>      acpi.c smm.c mptable.c pirtable.c smbios.c pciinit.c optionroms.c mtrr.c \
>      lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c \
> -    biostables.c xen.c bmp.c romfile.c csm.c
> +    biostables.c xen.c bmp.c romfile.c csm.c linker.c
>  SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
>  
>  # Default compiler flags
> diff --git a/src/linker.c b/src/linker.c
> new file mode 100644
> index 0000000..5009f88
> --- /dev/null
> +++ b/src/linker.c
> @@ -0,0 +1,78 @@
> +#include "linker.h"
> +#include "byteorder.h" // le64_to_cpu
> +
> +void linker_link(const char *name)
> +{
> +    struct linker_entry_s *entry;
> +    int offset = 0;
> +    int size, lsrc;
> +    void *data = romfile_loadfile(name, &size);
> +    struct romfile_s *src, *dst;
> +    u32 src_offset, dst_offset;
> +    u64 val, buf;
> +    if (!data)
> +        return;
> +    
> +    for (offset = 0; offset < size; offset += entry->size) {
> +        entry = data + offset;
> +        /* Entry must have some data. If not - skip it. */
> +        if (entry->size <= sizeof *entry)
> +            continue;
> +        /* Check that entry fits in buffer. */
> +        if (offset + entry->size > size) {
> +            warn_internalerror();
> +            break;
> +        }
> +        /* Skip any command that we don't recognize. */
> +        if (entry->reserved)
> +            continue;
> +        if (entry->bytes != 1 &&
> +            entry->bytes != 2 &&
> +            entry->bytes != 4 &&
> +            entry->bytes != 8)
> +            continue;
> +        if (entry->type != LINKER_ENTRY_TYPE_ADD &&
> +            entry->type != LINKER_ENTRY_TYPE_SUB)
> +            continue;
> +        if (entry->format != LINKER_ENTRY_FORMAT_LE)
> +            continue;
> +        /* Last byte must be 0 */
> +        if (entry->src_dst[entry->size - 1]) {
> +            warn_internalerror();
> +            continue;
> +        }
> +        lsrc = strlen(entry->src_dst);
> +        if (!lsrc || lsrc + 1 +  sizeof *entry >= entry->size) {
> +            warn_internalerror();
> +            continue;
> +        }
> +        src = romfile_find(entry->src_dst);
> +        dst = romfile_find(entry->src_dst + lsrc + 1);
> +        if (!src || !dst) {
> +            warn_internalerror();
> +            continue;
> +        }
> +        if (!src->data || !dst->data) {
> +            warn_internalerror();
> +            continue;
> +        }
> +        src_offset = le32_to_cpu(entry->src_offset);
> +        dst_offset = le32_to_cpu(entry->dst_offset);
> +        if (dst->size < dst_offset + entry->bytes) {
> +            warn_internalerror();
> +            continue;
> +        }
> +	buf = 0;
> +	memcpy(&buf, dst->data + dst_offset, entry->bytes);
> +        val = ((unsigned long)src->data) + src_offset;
> +	buf = le64_to_cpu(buf);
> +        if (entry->type == LINKER_ENTRY_TYPE_ADD)
> +            buf += val;
> +        else
> +            buf -= val;
> +	buf = cpu_to_le64(val);
> +	memcpy(dst->data + dst_offset, &buf, entry->bytes);
> +    }
> +
> +    free(data);
> +}
> diff --git a/src/linker.h b/src/linker.h
> new file mode 100644
> index 0000000..e1d55ea
> --- /dev/null
> +++ b/src/linker.h
> @@ -0,0 +1,28 @@
> +#include "types.h" // u8
> +#include "util.h" // romfile_s
> +
> +/* ROM file linker interface. Linker uses little endian format */
> +struct linker_entry_s {
> +    u8 size; /* size in bytes including the header */
> +    u8 bytes; /* How many bytes to change. Can be 1,2,4 or 8. */
> +    u8 type;
> +    u8 format;
> +    u32 reserved;
> +    u64 src_offset; /* Offset in source. Little endian format. */

src_offset is not needed: host can pre-fill destination
table with the offset value.
Will probably drop it.

> +    u64 dst_offset; /* Offset in destination. Little endian format. */
> +    /* Followed by source and destination

Add "file names"

>, optionally padded by
> +     * 0, up to the total of entry_len - 4 bytes.
> +     * Strings are 0-terminated. */
> +    char src_dst[];
> +} PACKED;
> +
> +enum linker_entry_type {

Documentation:

ADD: increment value in DST by the address of SRC
     useful e.g. to fill in pointer values in ACPI tables
SUB: decrement value in DST by the address of SRC
     useful e.g. to fix up checksum values in ACPI tables


> +    LINKER_ENTRY_TYPE_ADD = 0x0,
> +    LINKER_ENTRY_TYPE_SUB = 0x1,
> +};
> +
> +enum linker_entry_format {
> +    LINKER_ENTRY_FORMAT_LE = 0x0,
> +};
> +

Documentation:

/*
 * Load linker script and execute it.
 * All linked files referenced by the script
 * must already be in memory.
 */

> +void linker_link(const char *name);
> diff --git a/src/util.h b/src/util.h
> index 996c29a..7b50c38 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -436,6 +436,7 @@ struct romfile_s {
>      char name[128];
>      u32 size;
>      int (*copy)(struct romfile_s *file, void *dest, u32 maxlen);
> +    void *data;
>  };
>  void romfile_add(struct romfile_s *file);
>  struct romfile_s *romfile_findprefix(const char *prefix, struct romfile_s *prev);



More information about the SeaBIOS mailing list