On 07/25/12 22:40, Jordan Justen wrote:
> On Wed, Jul 25, 2012 at 12:02 PM, Laszlo Ersek <[email protected]> wrote:
>> From: Paolo Bonzini
>>
>> This patch adds support for a debug console on the same port that is used
>> by SeaBIOS.  This makes it easier to debug OVMF, because it does not mix
>> debug and serial output on the same device.  It also makes it easier to
>> leave some of the debug messages on even in release builds.
>>
>> To enable it, pass "-debugcon stdio -global isa-debugcon.iobase=0x402" to
>> QEMU.
>>
>> The new mechanism is enabled by default, but a regular serial console can
>> be chosen by adding -D DEBUG_ON_SERIAL_PORT to the build options.
>>
>> v2:
>> - replace IOPort with IoPort
>> - replace BASE_DEBUG_SERIAL_PORT with DEBUG_ON_SERIAL_PORT
>> - increase MAX_DEBUG_MESSAGE_LENGTH to 2KB
> 
> May I ask why this was increased?
> 
> I don't think 2k would trigger it, but some compilers start trying to
> insert stack checking calls when large sized local variables are used.
> 
> Also, it would seem good to match BaseDebugLibSerialPort.

I added a DEBUG call to PlatformBdsPolicyBehavior()
[OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] that would dump the
enumerated boot options in a loop; something like

    DEBUG ((EFI_D_INFO, "Signature=%c%c%c%c DevPath=%s DevPathShort=%s"
      " OptionName=\"%s\" OptionNumber=%02ld BootCurrent=%02d"
      " Attribute=0x%08x Description=\"%s\" StatusString=\"%s\"\n",
      ((CONST UINT8 *)&BootOption->Signature)[0],
      ((CONST UINT8 *)&BootOption->Signature)[1],
      ((CONST UINT8 *)&BootOption->Signature)[2],
      ((CONST UINT8 *)&BootOption->Signature)[3],
      DevPath,
      DevPathShort,
      BootOption->OptionName,
      (long)BootOption->OptionNumber,
      BootOption->BootCurrent,
      BootOption->Attribute,
      BootOption->Description,
      BootOption->StatusString
    ));

The longest line produced was 433 characters long, but
MAX_DEBUG_MESSAGE_LENGTH (0x100) in
"MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c" truncated it
originally. I temporarily raised it, but now that this patch flips the
default log target to the qemu debug console, I thought raising the
limit here would suffice, as the BaseDebugLibSerialPort limit has caused
no problems for others apparently.

The value 2048 is inspired by _POSIX2_LINE_MAX, but of course I don't
insist on it.


> I guess I missed my chance to complain about the 'p' variable name. I
> would recommend Ptr.
> http://sourceforge.net/projects/edk2/files/General%20Documentation/EDK%20II%20C%20Coding%20Standards%20Specification.pdf/download

Sorry about that. I do have the spec and I know "IOPort" violates 9.2
"File, Function, and Data Names" / "Acronyms are not capitalized". I
guess "p" violates the "CamelCasing" rules in this same section. No good
excuse here, just a bad one: the specs I have to keep open are numerous
and very long. (The Coding Standards weigh in at 67 pages.)


> I can make that change though...

That would be greatly appreciated.

Laszlo

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to