[SeaBIOS] [PATCH] make seabios 386/486 friendly

[Kevin O'Connor]

On Sun, Jan 22, 2012 at 11:12:00PM +0100, Rudolf Marek wrote:
> >Can't the existing timer_counter in the BDA be used instead?
> 
> I got some earlier iteration which used the *_timer value but it did
> not work for some reason (most likely I forgot all the magic for
> real mode)
> 
> I think using extra variables will remove the complexity with day
> rollover and custom changes to the timer variable either from option
> ROM or from BIOS API.

I'm not sure there would be an issue with third party code (optionroms
/ old code changing the pit).  The seabios timer users are fairly
straight forward - get the current value, and then wait for the
expected value in a loop with yield().  So, there'd only be a problem
if some third-party irq handler messed with the pit - if they did that
from a hardware irq handler, I'd guess it would break lots of things.
(Things are less clear with CONFIG_THREAD_OPTIONROMS, but I'd say it
would be safe to leave that off for your use case.)

> >BTW, what's the impact of writing to the PORT_PIT_MODE?  Will it
> >corrupt the old value?  (Not an issue for the patch, as obviously
> >something is better than nothing for your board.)
> 
> I think none, it is a readback command of the 8254 chip which allows
> to read back the latch and configuration. The mode of timer or timer
> itself should not be affected. I think this may need to be protected
> if some interrupt jump in. I had in previous patch the
> IRQ_save/restore stuff but you told me that irqs are enabled only
> during yields. Maybe it would be safer to have the irq_save /
> restore here too?

All SeaBIOS C code runs with irqs disabled.  There's no reason to have
irq_save/restore in the C code.

However, are there any seabios timer users reachable from irq
handlers?  Should userspace start programming the pit without irq
prevention, could it get interrupted mid-way through and then
corrupted by a seabios timer user in an irq handler?

> >>+    // Emulate the CPUID if 386/486
> >>+    detect_cpuid();
> >
> >This is too early (can't assign global variables here on QEmu).  I
> >suggest you put in maininit() after the malloc_fixup() call.
> 
> It is too late there. There is some CPUID instruction executed from xen_probe().
> Yes I xen is enabled for coreboot for some reason... The whole point
> of the cpuid emulation is to have it globally available and not
> making it dependent on particular configuration. Not sure how to
> solve it here.

Hrmm - the xen thing is a special case - I think we could just make
CONFIG_XEN dependent on !CONFIG_COREBOOT in Kconfig and ignore it's
early cpuid usage.

-Kevin