On 05/30/13 09:30, Gerd Hoffmann wrote:
On 05/30/13 03:34, Kevin O'Connor wrote:
On Wed, May 29, 2013 at 04:25:59PM +0200, Gerd Hoffmann wrote:
Allow selecting DEBUG_IO for non-qemu configurations, which is useful when running coreboot+seabios on qemu.
Unfortunately, if one does run seabios on real hardware and has DEBUG_IO enabled, it will write to IO port 0x402 before confirming that it is actually running on QEMU. This could cause mysterious failures on real hardware if something is listening to that port. It's why I left the option dependent on QEMU instead of QEMU_HARDWARE. Ideally the code would verify it is on QEMU before using the IO port,
(*)
while still providing the very early debugging when it is known to be safe.
The debgconsole port returns 0xe9 on reads, so we could use that for probing and do something like the attached patch. Which doesn't build for some reason. Is the F segment read-only in 16bit mode?
See "GCC 16 bit limitations" in the README, especially
103 Global variables defined in the C code can be read in 16bit mode if 104 the variable declaration is marked with VAR16, VAR16VISIBLE, 105 VAR16EXPORT, or VAR16FIXED. The GET_GLOBAL macro will then allow read 106 access to the variable. Global variables are stored in the 0xf000 107 segment. Because the f-segment is marked read-only during run-time, 108 the 16bit code is not permitted to change the value of 16bit variables 109 (use of the SET_GLOBAL macro from 16bit mode will cause a link error).
Should I use something else instead? Or #ifdef the SET_GLOBAL for 32bit mode, which should work fine given that POST runs in 32bit mode?
I think I had attempted to do something like this before: http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/5453/focus=5503
In patch 2 I used SET_FARVAR(), but am still not sure if it was completely brain-dead or not.
(*) However Kevin mentioned an idea (originally from David it seems) in the rest of that thread that he could have possible accepted as early detection of QEMU: parse the SMBIOS table for some information identifying QEMU.
Maybe reading from the debug port is a good alternative. In that case I think your suggestion could be viable; in the 16-bit phase and the 32-bit segmented phase putc_debug() could read the debug port before each write, but in the 32-bit flat phase it could flip the global flag and skip the reads.
diff --git a/src/output.c b/src/output.c index 102f177..e082283 100644 --- a/src/output.c +++ b/src/output.c @@ -24,6 +24,8 @@ struct putcinfo { #define DEBUG_TIMEOUT 100000
u16 DebugOutputPort VARFSEG = 0x402; + +// 0xff -- undecided, 0x01 -- running on qemu, 0x00 -- not running on qemu u8 DebugOutputEnabled VARFSEG = 0xff;
void @@ -83,9 +85,10 @@ putc_debug(struct putcinfo *action, char c) u8 enabled = GET_GLOBAL(DebugOutputEnabled); if (enabled == 0xff) { enabled = (inb(GET_GLOBAL(DebugOutputPort)) == 0xe9); - SET_GLOBAL(DebugOutputEnabled, enabled); + if (MODE16 == 0 && MODESEGMENT == 0) + SET_GLOBAL(DebugOutputEnabled, enabled); } - if (enabled ) { + if (enabled) { outb(c, GET_GLOBAL(DebugOutputPort)); } }
Laszlo