[PATCH v3] config: allow DEBUG_IO for !QEMU
Allow selecting DEBUG_IO for non-qemu configurations, which is useful when running coreboot+seabios on qemu. [ v2: QEMU_HARDWARE is even better as DEBUG_IO default value ] [ v3: make QEMU_HARDWARE usage consistent with other config options, fix spellings in commit message ] Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Kconfig b/src/Kconfig index 3c80132..5882d11 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -425,7 +425,7 @@ menu "Debugging" Base port for serial - generally 0x3f8, 0x2f8, 0x3e8, or 0x2e8. config DEBUG_IO - depends on QEMU && DEBUG_LEVEL != 0 + depends on QEMU_HARDWARE && DEBUG_LEVEL != 0 bool "Special IO port debugging" default y help -- 1.7.9.7
On 05/29/13 16:25, Gerd Hoffmann wrote:
Allow selecting DEBUG_IO for non-qemu configurations, which is useful when running coreboot+seabios on qemu.
[ v2: QEMU_HARDWARE is even better as DEBUG_IO default value ] [ v3: make QEMU_HARDWARE usage consistent with other config options, fix spellings in commit message ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Kconfig b/src/Kconfig index 3c80132..5882d11 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -425,7 +425,7 @@ menu "Debugging" Base port for serial - generally 0x3f8, 0x2f8, 0x3e8, or 0x2e8.
config DEBUG_IO - depends on QEMU && DEBUG_LEVEL != 0 + depends on QEMU_HARDWARE && DEBUG_LEVEL != 0 bool "Special IO port debugging" default y help
QEMU selects QEMU_HARDWARE, so OK in that direction. CSM and COREBOOT (!QEMU in general) make the QEMU_HARDWARE option available, which when set to y opens up DEBUG_IO. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo
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. -Kevin
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? 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? cheers, Gerd
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
On Thu, May 30, 2013 at 09:30:33AM +0200, 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? 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?
Same problem - one can't reliably do an inb(0xe9) on real hardware without risking mysterious failures. -Kevin
On 05/31/13 03:09, Kevin O'Connor wrote:
On Thu, May 30, 2013 at 09:30:33AM +0200, 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? 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?
Same problem - one can't reliably do an inb(0xe9) on real hardware without risking mysterious failures.
This entire problem seems to exist only if someone runs a SeaBIOS binary on real hardware that was configured like: - COREBOOT or CSM (ie. not QEMU), - and QEMU_HARDWARE. Why would someone set QEMU_HARDWARE ("Support hardware found on emulators") for a binary intended to run on real hardware? In the rest of src/Kconfig, the following options depend on QEMU_HARDWARE: - VIRTIO_BLK - VIRTIO_SCSI - ESP_SCSI - LSI_SCSI The first two clearly don't make sense on bare metal. The last two might (no idea), but if that's the case, then their dependency on emulated hardware is wrong. We need a way to say in Kconfig, "this is a CSM build specifically meant for emulators", and CONFIG_CSM + QEMU_HARDWARE looks like an ideal candidate. People building for bare metal shouldn't (need to) set QEMU_HARDWARE. Alternatively, we need code that can run in 16-bit mode (consequently, without writing any globals) and that can identify *any* of the supported hypervisors. "PlatformRunningOn" is too late. KVM and Xen could be covered by cpuid() (*), but this approach is a pain because a single patch would have to add detection for all supported hypervisors at once. (*) I checked the SDM and also there's code like this out there, eg. <http://lkml.indiana.edu/hypermail/linux/kernel/0705.0/0019.html>. What if we replace the inb() in the proposed patch with a cpuid call that detects only KVM, as first step? On KVM it would do the right thing, on bare metal, ditto. On other hypervisors it would err towards safety and could be extended later. What do you think? Thanks, Laszlo
On Fri, May 31, 2013 at 03:30:48PM +0200, Laszlo Ersek wrote:
On 05/31/13 03:09, Kevin O'Connor wrote:
Same problem - one can't reliably do an inb(0xe9) on real hardware without risking mysterious failures.
This entire problem seems to exist only if someone runs a SeaBIOS binary on real hardware that was configured like: - COREBOOT or CSM (ie. not QEMU), - and QEMU_HARDWARE.
Why would someone set QEMU_HARDWARE ("Support hardware found on emulators") for a binary intended to run on real hardware?
It's common to build seabios with all options enabled and let seabios auto-detect what it can use. That's the intent of QEMU_HARDWARE - it allows one to compile in more drivers and seabios will use them if it detects the corresponding hardware. This does not obviate the need for proper hardware detection though. If this goes wrong on real hardware, it could require a lengthy session with a soldering iron to fix it. People don't like that. [...]
What if we replace the inb() in the proposed patch with a cpuid call that detects only KVM, as first step? On KVM it would do the right thing, on bare metal, ditto. On other hypervisors it would err towards safety and could be extended later. What do you think?
The plan was to detect qemu on coreboot via the mainboard vendor/part info that coreboot generates. The plan on uefi was to use the smbios tables to detect qemu. Once detection is in place, the DEBUG_IO support could be made dependent on QEMU_HARDWARE and only activated after detection succeeds. -Kevin
On 06/01/13 05:35, Kevin O'Connor wrote:
The plan on uefi was to use the smbios tables to detect qemu. Once detection is in place, the DEBUG_IO support could be made dependent on QEMU_HARDWARE and only activated after detection succeeds.
Understood. I wasn't aware this was pending on me (skimmed src/csm.c). I'll see what I can do in OvmfPkg/SmbiosPlatformDxe. Doesn't seem particularly easy, given that qemu may optionally pass smbios structures over fw_cfg (see the -smbios option, in which case OVMF should not build, just install them), plus there's a big number of mandatory smbios structures for which almost a full system configuration must be collected, seemingly. I hope edk2 will at least provide the data types. Thanks Laszlo
Hi Kevin, David, On 06/02/13 21:24, Laszlo Ersek wrote:
On 06/01/13 05:35, Kevin O'Connor wrote:
The plan on uefi was to use the smbios tables to detect qemu. Once detection is in place, the DEBUG_IO support could be made dependent on QEMU_HARDWARE and only activated after detection succeeds.
Understood.
I wasn't aware this was pending on me (skimmed src/csm.c). I'll see what I can do in OvmfPkg/SmbiosPlatformDxe.
I posted <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/3037>, which provides Type 0 and Type 1 tables under OVMF. (Might code up more tables as time allows, but I didn't want to stall this thread here.) "dmidecode" reports the following in an OVMF Fedora 19 guest, running on RHEL-6.4 qemu-kvm (with no special "-smbios" options passed to qemu-kvm): # dmidecode 2.12 # SMBIOS entry point at 0x3f8f7000 SMBIOS 2.7 present. 3 structures occupying 163 bytes. Table at 0x3F8F6000. Handle 0x0000, DMI type 0, 24 bytes BIOS Information Vendor: EFI Development Kit II / OVMF Version: 0.1 Release Date: 06/03/2013 Address: 0xE8000 Runtime Size: 96 kB ROM Size: 64 kB Characteristics: BIOS characteristics not supported UEFI is supported System is a virtual machine BIOS Revision: 0.1 Handle 0x0001, DMI type 1, 27 bytes System Information Manufacturer: Red Hat Product Name: KVM Version: RHEL 6.4.0 PC Serial Number: n/a UUID: 7B20A9F5-274C-0F58-8183-A89538BCA62A Wake-up Type: Power Switch SKU Number: n/a Family: Red Hat Enterprise Linux Handle 0xFEFF, DMI type 127, 4 bytes End Of Table Questions: (1) What field should we key off of in src/csm.c whether the platform is a VM? I would suggest the "System is a virtual machine" bit, but it could be too obvious to be feasible. This bit is Bit 4 in BIOS Characteristics Extension Byte 2, and available from SMBIOS version 2.3 on. (2) How should the information be parsed? (Since "src/csm.c" is SRC32FLAT only, I might even have a chance implementing the parsing.) I've added SMBIOS parsing code before (see display_uuid() in src/smbios.c). Can I simply copy and customize it, or should it be refactored / reused somehow? (3) *When* should we parse the SMBIOS tables? I would say "as early as possible" (debug port access depends on it). handle_csm_0000 (Compatibility16InitializeYourself()) gets an EFI_TO_COMPATIBILITY16_INIT_TABLE, but that table doesn't provide SMBIOS info. handle_csm_0001 (Compatibility16UpdateBbs()) gets an EFI_TO_COMPATIBILITY16_BOOT_TABLE. It provides SmbiosTable and SmbiosTableLength. Unfortunately handle_csm_0001() is not called until quite late in the boot process. I think we should receive SeaBIOS debug output before that point. I'm attaching a debug port log for illustration: - It was produced by booting Windows 2008 R2 SP1. (UEFI boot, not legacy boot, but win2k8r2sp1 is buggy and needs CSM even for UEFI boot...), - SeaBIOS messages were only produced because I had hacked the SeaBIOS config to enable DEBUG_IO in the CSM build, - the SeaBIOS code in this CSM image is somewhat dated, but that shouldn't detract from main point. Which is: if we enable debug port access no earlier than handle_csm_0001() -- because SMBIOS is unavailable before that --, then we lose all SeaBIOS debug messages between handle_csm_0000() and handle_csm_0001(): 412 handle_csm regs 0x0004ffd4 AX=0000 413 Legacy16InitializeYourself table 4000:8080 to 1515 handle_csm regs 0x0004ffd4 AX=0001 1516 Legacy16UpdateBbs table 480a:0008 Thanks, Laszlo
Gerd Hoffmann wrote:
Allow selecting DEBUG_IO for non-qemu configurations, which is useful when running coreboot+seabios on qemu.
[ v2: QEMU_HARDWARE is even better as DEBUG_IO default value ] [ v3: make QEMU_HARDWARE usage consistent with other config options, fix spellings in commit message ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Peter Stuge <peter@stuge.se>
participants (4)
-
Gerd Hoffmann -
Kevin O'Connor -
Laszlo Ersek -
Peter Stuge