[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