Bug 1714446

To refine the suggested syntax: we should likely write:

      gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|($(DEBUG_PRINT_ERROR_LEVEL)) & 0xFFBFFFFF

Because: the edk2 DSC spec at <https://edk2-docs.gitbooks.io/edk-ii-dsc-specification/content/2_dsc_overview/22_build_description_file_format.html#table-3-using-system-environment-variable> provides the following example:

[LibraryClasses.X64, LibraryClasses.IA32]
  DEFINE PERF = PerformancePkg/Library
  TimerLib|$(PERF)/DxeTscTimerLib/DxeTscTimerLib.inf

This implies that macro expansion doesn't (mustn't) automatically wrap the replacement text in parentheses. And therefore, in preparation for "tricky" values in DEBUG_PRINT_ERROR_LEVEL, we should tightly enclose $(DEBUG_PRINT_ERROR_LEVEL) in parens.

Furthermore, an outermost set of parens doesn't look necessary (the PCD setting syntax doesn't appear to require it).

Here's a summary of the rhvirt-patches review discussion (linked in
comment 3):

(1) The patch, as posted, uses the mask ~0x00400000 for clearing the
DEBUG_VERBOSE bit. Using the bitwise-complement operator for setting
PCDs in the DSC is not recommended. The edk2 DSC spec refers (somewhat
vaguely) to the C standard when it specifies such PCD assignments. In C,
on all platforms that we care about, (~0x00400000) leads to flipping the
sign bit in a signed integer. We should avoid that. If we could write
(~0x00400000u), that would be fine, but my understanding is that the DSC
spec does not permit that. I suggest using 0xFFBFFFFF as mask,
explicitly. (This C language constant has type "unsigned int" on all
platforms that we care about.)

(2) There are two new log messages that the patch does not suppress:

- "Error: Image at 00047505000 start failed: Not Found"
- "remove-symbol-file ..."

Suppressing each of these will require a dedicated patch; see below:

(2.1) The first message comes from the DXE Core. It can only be
suppressed with downstream-only changes, as printing the message at
DEBUG_ERROR level is the right thing for the DXE Core otherwise, and it
is also right for QemuRamfbDxe to exit with an error under the given
circumstances. Options:

(2.1.1) Downgrade the log mask to DEBUG_WARN in the DXE Core. Bad idea,
we'd lose valuable reports of driver startup failures.

(2.1.2) Change QemuRamfbDxe to return EFI_SUCCESS even when it fails
(e.g. ramfb is not configured). Wastes some memory (driver stays
resident without any good use), but it is mostly harmless, as the memory
is released by the OS after ExitBootServices().

(2.1.3) Replace "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000" in the spec file
with "-D DEBUG_PRINT_ERROR_LEVEL=0x0". This will suppress *all* debug
messages (even errors). Not ideal because (a) it allows errors to
"snowball" (we could notice an issue only much later, from behavioral
problems), (b) it makes the "silent" firmware binary entirely unsuitable
for bug reporting and triaging, (c) it prevents us from cleaning up new
debug messages up-stream that are needlessly logged at DEBUG_ERROR
level.

(2.2) The second message should be downgraded to DEBUG_INFO level, with
an upstream patch, similarly to upstream commit 1fce963d89f3 ("ArmPkg:
DebugPeCoffExtraActionLib: debugger commands are not errors",
2015-03-02). This upstream patch should be backported then.


My recommendation is a series of 3 patches:
- patch #1: what was posted already (downstream-only patch), but replace
            ~0x00400000 with 0xFFBFFFFF
- patch #2: downstream-only patch implementing (2.1.2)
- patch #3: backport of new upstream patch implementing (2.2)

Thanks!

(In reply to Laszlo Ersek from comment #4)
> My recommendation is a series of 3 patches:
> - patch #1: what was posted already (downstream-only patch), but replace
>             ~0x00400000 with 0xFFBFFFFF
> - patch #2: downstream-only patch implementing (2.1.2)

Yes, I prefer 2.1.2 as well.

> - patch #3: backport of new upstream patch implementing (2.2)
> 
> Thanks!

Thanks,
drew

For (2.2), Phil posted:

[edk2-devel] [PATCH 0/2] ArmPkg: DebugPeCoffExtraActionLib: debugger
                         commands are not errors
http://mid.mail-archive.com/20190729180321.1715-1-philmd@redhat.com
https://edk2.groups.io/g/devel/message/44569

Pushed by Leif as commit range 8fed4e47d9a6..3d34b5f32692:

1 a6cd7fbac494 ArmPkg: DebugPeCoffExtraActionLib: debugger commands are
               not errors
2 3d34b5f32692 ArmPkg: DebugPeCoffExtraActionLib: fix trivial comment
               typos

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:3338