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

Michael S. Tsirkin mst at redhat.com
Wed Apr 24 10:09:16 CEST 2013


On Tue, Apr 23, 2013 at 08:23:44PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 22, 2013 at 08:38:58PM -0400, Kevin O'Connor 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?
> > > > 
> > > > 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.
> > > 
> > > 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.
> > 
> > I know of the following quirks that would have to be handled:
> > 
> > 1 - the RSDP must be in the f-segment (where as all other tables can
> > go into "high" memory).
> > 
> > 2 - the RSDP has a checksum in a different location from the other
> > tables and (with an XSDT) it can have two checksums.
> > 
> > 3 - the RSDP has a pointer to the RSDT (and to the XSDT if present).
> > 
> > 4 - the RSDT (and XSDT if present) has pointers to all the other
> > tables (except RSDP, RSDT, DSDT, and FACS).  The FADT pointer must be
> > first in the list.
> > 
> > 5 - the FADT table has pointers to DSDT and FACS.
> > 
> > 6 - the FACS table must be 64 byte aligned.
> > 
> > So, will a generic scheme really be able to handle all of the above
> > quirks, or will we just be mixing some hardcoded quirks with some
> > generic quirks?  And, will the code to handle the above quirks in a
> > generic fashion be of a higher complexity than simply hard-coding it?
> > 
> > -Kevin
> 
> -->
> 
> So here's an implementation for align and FSEG.
> Not a big deal as you see.
> 
> I really have doubts about it however: BIOS still must be able to parse
> get the resume vector in FACS in order to support wakeup, right? So this
> means that we need to be able to parse RSDP and FACT.  These happen to
> be the only things that need anything not addressed by ADD and SUB so
> ... maybe a couple of hardcoded quirks just to allocate these correctly
> is cleaner.

Heh, it's actually pretty easy: let's just ask qemu
to give us the address of the resume vector
in a file with a pre-defined name.
Linker can patch table offset there in the
regular way.



> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> 
> ---
> 
> diff --git a/src/linker.c b/src/linker.c
> index a1f473d..22a0dff 100644
> --- a/src/linker.c
> +++ b/src/linker.c
> @@ -34,7 +34,9 @@ void linker_link(const char *name)
>  	if (entry->shift > 63)
>              continue;
>          if (entry->type != LINKER_ENTRY_TYPE_ADD &&
> -            entry->type != LINKER_ENTRY_TYPE_SUB)
> +            entry->type != LINKER_ENTRY_TYPE_SUB &&
> +            entry->type != LINKER_ENTRY_TYPE_ALIGN &&
> +            entry->type != LINKER_ENTRY_TYPE_FSEG)
>              continue;
>          if (entry->format != LINKER_ENTRY_FORMAT_LE)
>              continue;
> @@ -44,17 +46,55 @@ void linker_link(const char *name)
>              continue;
>          }
>          lsrc = strlen(entry->src_dst);
> -        if (!lsrc || lsrc + 1 +  sizeof *entry >= entry->size) {
> +        if (!lsrc) {
>              warn_internalerror();
>              continue;
>          }
>          src = romfile_find(entry->src_dst);
> +        if (!src) {
> +            warn_internalerror();
> +            continue;
> +        }
> +        if (!src->data) {
> +            warn_internalerror();
> +            continue;
> +        }
> +	if (entry->type == LINKER_ENTRY_TYPE_ALIGN ||
> +	    entry->type == LINKER_ENTRY_TYPE_FSEG) {
> +		void *data;
> +		u32 align;
> +		struct zone_s *zone;
> +
> +		if (entry->shift > 31) {
> +			warn_internalerror();
> +			continue;
> +		}
> +		align = 0x1 << entry->shift;
> +		if (align < MALLOC_MIN_ALIGN)
> +			align = MALLOC_MIN_ALIGN;
> +		zone = entry->type == LINKER_ENTRY_TYPE_FSEG ?
> +			&ZoneFSeg : &ZoneHigh;
> +		data = pmm_malloc(zone, PMM_DEFAULT_HANDLE, src->size, align);
> +		if (!data) {
> +			warn_internalerror();
> +			continue;
> +		}
> +		memcpy(data, src->data, src->size);
> +		free(src->data);
> +		src->data = data;
> +		continue;
> +	}
> +
> +        if (lsrc + 1 +  sizeof *entry >= entry->size) {
> +            warn_internalerror();
> +            continue;
> +        }
>          dst = romfile_find(entry->src_dst + lsrc + 1);
> -        if (!src || !dst) {
> +        if (!dst) {
>              warn_internalerror();
>              continue;
>          }
> -        if (!src->data || !dst->data) {
> +        if (!dst->data) {
>              warn_internalerror();
>              continue;
>          }
> diff --git a/src/linker.h b/src/linker.h
> index 2bb376d..b0cc536 100644
> --- a/src/linker.h
> +++ b/src/linker.h
> @@ -17,9 +17,12 @@ struct linker_entry_s {
>      char src_dst[];
>  } PACKED;
>  
> +/* Note: align types must appear before all other types. */
>  enum linker_entry_type {
>      LINKER_ENTRY_TYPE_ADD = 0x0,
>      LINKER_ENTRY_TYPE_SUB = 0x1,
> +    LINKER_ENTRY_TYPE_ALIGN = 0x2, /* Align source to shift bits. Must come first. */
> +    LINKER_ENTRY_TYPE_FSEG  = 0x3, /* Align source to shift bits in FSEG memory. */
>  };
>  
>  enum linker_entry_format {



More information about the SeaBIOS mailing list