Laszlo:
Thanks for your comments.
1) BIT28 should be BIT31.
2) New PCD PcdFixedDebugPrintErrorLevel value should be mask value of all BITs
so that it doesn't bring impact for current platform. If platform wants to use
this feature, platform can configure its value to new one in their DSC file.
I make new patch for this update. Please help review it.
---
MdePkg/MdePkg.dec | 6 +++---
MdePkg/MdePkg.uni | Bin 66564 -> 66564 bytes
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3f3485d..1f9b54d 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1628,10 +1628,10 @@
# BIT20 - Global Coherency Database changes message.<BR>
# BIT21 - Memory range cachability changes message.<BR>
# BIT22 - Detailed debug message.<BR>
- # BIT28 - Error message.<BR>
+ # BIT31 - Error message.<BR>
# @Prompt Fixed Debug Message Print Level.
# @Expression 0x80000002 |
(gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel & 0x7F84AA00) == 0
-
gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x80000000|UINT32|0x30001016
+
gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x807B55FF|UINT32|0x30001016
[PcdsFixedAtBuild,PcdsPatchableInModule]
## Indicates the maximum length of unicode string used in the following
@@ -1697,7 +1697,7 @@
# BIT20 - Global Coherency Database changes message.<BR>
# BIT21 - Memory range cachability changes message.<BR>
# BIT22 - Detailed debug message.<BR>
- # BIT28 - Error message.<BR>
+ # BIT31 - Error message.<BR>
# @Prompt Debug Message Print Level.
# @Expression 0x80000002 |
(gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel & 0x7F84AA00) == 0
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000|UINT32|0x00000006
diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index
c357d61dd60477f7ad5d36e1151e98f52ecd7bfb..893e62c2663a6385e62920d51f24aa11b1758a8e
100644
GIT binary patch
delta 31
kcmZqaU}@=K*$~#lY|LP|IkM-t2#7H`vZr<PiiI&r0I&@Us{jB1
delta 31
kcmZqaU}@=K*$~#lY{X!(IkM-t2#7H`vZr<PiiI&r0I;_Uw*UYD
--
1.9.0.msysgit.0
Thanks
Liming
-----Original Message-----
From: Laszlo Ersek [mailto:[email protected]]
Sent: Monday, February 02, 2015 8:58 PM
To: [email protected]
Subject: Re: [edk2] [PATCH 10/10] OvmfPkg: Update PlatformBaseDebugLibIoPort
library
On 02/02/15 07:35, Liming Gao wrote:
> Implement new API DebugPrintLevelEnabled() to base on PCD
> PcdFixedDebugPrintErrorLevel.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao <[email protected]>
> ---
> OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 19
> ++++++++++++++++++-
> .../PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf | 3 ++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> index 84299ca..585f9fb 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> @@ -2,7 +2,7 @@
> Base Debug library instance for QEMU debug port.
> It uses PrintLib to send debug messages to a fixed I/O port.
>
> - Copyright (c) 2006 - 2012, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2006 - 2015, Intel Corporation. All rights
> + reserved.<BR>
> Copyright (c) 2012, Red Hat, Inc.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License @@ -267,3 +267,20 @@ DebugClearMemoryEnabled (
> return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) &
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0); }
>
> +/**
> + Returns TRUE if any one of the bit is set both in ErrorLevel and
> PcdFixedDebugPrintErrorLevel.
> +
> + This function compares the bit mask of ErrorLevel and
> PcdFixedDebugPrintErrorLevel.
> +
> + @retval TRUE Current ErrorLevel is supported.
> + @retval FALSE Current ErrorLevel is not supported.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugPrintLevelEnabled (
> + IN CONST UINTN ErrorLevel
> + )
> +{
> + return (BOOLEAN) ((ErrorLevel &
> +PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0); }
> diff --git
> a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> index a70c7b5..0e74fe9 100644
> ---
> a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.in
> +++ f
> @@ -2,7 +2,7 @@
> # Instance of Debug Library for the QEMU debug console port.
> # It uses Print Library to produce formatted output strings.
> #
> -# Copyright (c) 2006 - 2011, Intel Corporation. All rights
> reserved.<BR>
> +# Copyright (c) 2006 - 2015, Intel Corporation. All rights
> +reserved.<BR>
> # Copyright (c) 2012, Red Hat, Inc.<BR> # # This program and the
> accompanying materials @@ -47,4 +47,5 @@
> gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort ## CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask ## CONSUMES
> + gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
>
>
The code in this patch looks okay to me, but I'd like to ask about the context.
I can see:
- http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11979
- http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11961
First, in patch 1/6, I think the following is wrong:
+ # BIT28 - Error message.<BR>
That should be BIT31 (for 0x80000000).
Second, the patchset doesn't seem to introduce any DSC-specific values for the
new PcdFixedDebugPrintErrorLevel. And, the default value is
0x80000000 (ie. EFI_D_ERROR only).
If I understand correctly, after this patchset, in order for a debug message to
reach the debug output, its error level (or error mask, more
precisely) must intersect with *both* of PcdDebugPrintErrorLevel and the new
PcdFixedDebugPrintErrorLevel.
If that's the case, then the default value 0x80000000 in the .dec file (without
any DSC-specific assignments) will regress debug logging in all platforms that
log on any different level. For example, OVMF will lose all EFI_D_INFO messages
(0x00000040).
I think this patchset should not only add the new function to all DebugLib
instances, and it should not only add the new PCD to those INF files.
I think it should also update all DSC (and DSC include) files that assign a
value different from 0x80000000 to PcdDebugPrintErrorLevel, and it should
assign the same value to PcdFixedDebugPrintErrorLevel too.
Otherwise this patchset will globally restrict debug messages to EFI_D_ERROR.
I'm attaching the search results for non-0x80000000 PcdDebugPrintErrorLevel
assignments.
Thanks,
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
