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