On 03/01/16 15:18, Leif Lindholm wrote:
> On Tue, Mar 01, 2016 at 02:10:37PM +0000, Ryan Harkin wrote:
>> On 1 March 2016 at 10:11, Leif Lindholm <[email protected]> wrote:
>>> On Tue, Mar 01, 2016 at 11:00:45AM +0100, Ard Biesheuvel wrote:
>>>> On 1 March 2016 at 10:58, Laszlo Ersek <[email protected]> wrote:
>>>>> Public branch:
>>>>> <https://github.com/lersek/edk2/commits/debugh_comment_fix>.
>>>>>
>>>>> Cc: Ard Biesheuvel <[email protected]>
>>>>> Cc: Jordan Justen <[email protected]>
>>>>> Cc: Leif Lindholm <[email protected]>
>>>>> Cc: Liming Gao <[email protected]>
>>>>> Cc: Michael Kinney <[email protected]>
>>>>>
>>>>
>>>> Looks ok to me
>>>>
>>>> For where you need it:
>>>> Reviewed-by: Ard Biesheuvel <[email protected]>
>>>
>>> Apart from 2/6 (which can be dropped), ditto:
>>> Reviewed-by: Leif Lindholm <[email protected]>
>>>
>>
>> To go against the grain, what's the reason for having this info
>> duplicated across multiple files as comments?
>>
>> Without knowing the reason for it's being, I'd find the delete key
>> more useful in this scenario.
>
> Mainly that it is very handy to have this information nearby rather
> than having to cross-reference against a random when you're looking to
> set individual bits in a diagnostics facilities variable...
>
> Certainly this particular snippet would be an excellent candidate to
> reduce down to being in a single .dsc with your configuration file
> consolidation plans. But until then, I think the handiness motivates
> the duplication.
Sorry, another point that I forgot to add in my last email.
Fixed PCDs can be controlled on the driver level in the DSC file. I use
that feature quite liberally in my everyday development tree. I set the
log mask to verbose generally, but tone it down for a handful of
drivers. This would not be covered by a single DSC include file I think
-- being able to look at the bit meanings in the individual DSCs (where
the drivers are listed) remains a plus.
Example:
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf {
<PcdsFixedAtBuild>
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
}
NvmExpressDxe logs all BlockIo read & write calls on the EFI_D_VERBOSE
level. Which is fully valid if someone is developing or debugging
NvmExpressDxe, but for my everyday work, it's quite exceptional.
However, in general, I like to enable DEBUG_VERBOSE, so at the top
level, I set PcdDebugPrintErrorLevel to 0x8040004F.
Same for the DXE core: it logs debug info related to the Properties
Table and the Memory Attributes Table on the DEBUG_VERBOSE log level.
That log level selection is fully appropriate (and the debug messages
are appreciated), it's just that most of the time I would't like to look
at that specific output:
MdeModulePkg/Core/Dxe/DxeMain.inf {
<LibraryClasses>
NULL|IntelFrameworkModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
<PcdsFixedAtBuild>
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
}
I obsess about accurate log level (log mask) arguments for DEBUG() calls
because said accuracy helps with whole-sale log filtering. But, beyond
that, I find the driver-level settability really flexible -- which is
why I liberally produce verbose debug output in my patches, without
remorse. :)
BTW the report file generated with the build utility's "--report-file"
option is invaluable for seeing where a particular PCD value comes from,
for a particular module.
Thanks!
Laszlo
>
> /
> Leif
>
>>>>> Laszlo Ersek (6):
>>>>> MdePkg: DebugLib: more cleanup for log level comments in lib class
>>>>> header
>>>>> ArmPlatformPkg/ArmVExpressPkg: sync log level comments to DebugLib.h
>>>>> ArmVirtPkg: sync log level comments to DebugLib.h
>>>>> BeagleBoardPkg: sync log level comments to DebugLib.h
>>>>> Omap35xxPkg: sync log level comments to DebugLib.h
>>>>> OvmfPkg: copy log level comments from DebugLib.h
>>>>>
>>>>> ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc | 14 +++++++------
>>>>> ArmVirtPkg/ArmVirt.dsc.inc | 8 +++----
>>>>> BeagleBoardPkg/BeagleBoardPkg.dsc | 16 ++++++++------
>>>>> Omap35xxPkg/Omap35xxPkg.dsc | 16 ++++++++------
>>>>> OvmfPkg/OvmfPkgIa32.dsc | 22
>>>>> ++++++++++++++++++++
>>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 22
>>>>> ++++++++++++++++++++
>>>>> OvmfPkg/OvmfPkgX64.dsc | 22
>>>>> ++++++++++++++++++++
>>>>> MdePkg/Include/Library/DebugLib.h | 9 ++++----
>>>>> 8 files changed, 103 insertions(+), 26 deletions(-)
>>>>>
>>>>> --
>>>>> 1.8.3.1
>>>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> [email protected]
>>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
