On 1 March 2016 at 15:59, Laszlo Ersek <[email protected]> wrote: > 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 >
OK, you both seem convinced about it, I'll happily leave you to it! >> >> / >> 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
