[SeaBIOS] [PATCH v3] config: allow DEBUG_IO for !QEMU

[Laszlo Ersek]

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