On Thu, 2013-01-17 at 19:58 -0500, Kevin O'Connor wrote:
> Looks like you forgot to include csm.h in the patch.

Oops, sorry. Added in the git tree at
   http://git.infradead.org/users/dwmw2/seabios.git
    git://git.infradead.org/users/dwmw2/seabios.git

> > +#include "util.h" // checksum
> > +#include "bregs.h"
> > +
> > +
> > +#if CONFIG_CSM
> > +extern void entry_csm16(void);
> > +EFI_COMPATIBILITY16_TABLE csm_compat_table VAR16EXPORT __aligned(16) = {
> > +	.Signature = 0x24454649,
> > +	.TableChecksum = 0 /* Filled in by checkrom.py */,
> > +	.TableLength = sizeof(csm_compat_table),
> > +	.Compatibility16CallSegment = SEG_BIOS,
> > +	.Compatibility16CallOffset = 0 /* Filled in by checkrom.py */,
> > +	.OemIdStringPointer = (u32)"SeaBIOS",
> > +};
> > +#endif
> 
> Does this table have to be in the f-segment, or can it be anywhere in
> the binary?  BTW, how does OVMF know where to place the seabios blob
> in memory?

It can be anywhere in memory. OVMF loads the blob so that it ends at
1MiB.

> Are these values that SeaBIOS is expected to fill, or values from EFI
> that SeaBIOS is expected to utilize?  In either case it shouldn't be
> difficult to wire them into SeaBIOS' existing structures.  

There is a mixture of both. The ACPI one we *need* to point at a buffer
which is big enough to hold the table. Leave it at zero, and OVMF will
scribble an ACPI table over the interrupt vectors and then promptly go
off into the weeds the next time a timer interrupt comes in (hence the
problem I was seeing when I posted the patches).

Others it'll call the 'GetTableAddress' call (which is actually malloc)
and then fill in the address it's given.

More details at
http://www.intel.com/content/dam/doc/reference-guide/efi-compatibility-support-module-specification-v097.pdf

> I'd need to
> see csm.h to confirm, but it mostly looks like it requires some
> function calls and variable assignments.

Yeah, I figured it shouldn't be *so* hard. We'll get handed a full E820
table instead of our own for example. In memory which OVMF allocated by
calling the GetTableAddress function.

> [...]
> > +/* Legacy16GetTableAddress */
> > +void handle_csm_0006(struct bregs *regs)
> > +{
> > +	u16 size = regs->cx;
> > +	u16 align = regs->dx;
> > +	u16 region = regs->bx; // (1 for F000 seg, 2 for E000 seg, 0 for either)
> > +	void *chunk;
> > +
> > +	/* FIXME: I don't know if we can allocate in the E000 segment at all. */
> > +	dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
> > +		size, align, region);
> > +	chunk = pmm_malloc(&ZoneFSeg, PMM_DEFAULT_HANDLE, size, align);
> 
> This is the same as malloc_fseg(). 

Apart from the alignment. Which is a power of two, yes. I'll fix it to
use ZoneLow for the e-segment too; thanks. If no preference is expressed
by the caller, should I try ZoneLow first? That'll be larger, right?

> BTW, SeaBIOS uses 4 space indentation and no tabs.

I must teach emacs that rather than attempting to fix it up manually
when I notice that it's not conforming... :)

-- 
dwmw2