| Message ID | 20180222195700.7279-1-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | Create UART DebugLib implementation for runtime drivers | expand |
On 22 February 2018 at 19:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug > message") broke the DEBUG build for systems using a MMIO mapped UART for > DEBUG output. In other words, it broke the build for all ARM and AARCH64 > systems, given that port I/O does not exist on those architectures. > > Instead of patching it up locally, let's fix this issue once and for all, > by creating a UART DebugLib implementation for DXE_RUNTIME_DRIVER modules > that does the right thing by default. > > v4: > - add Laszlo's R-b > - keep ASSERT() message in local buffer even it is not printed to the serial > port, to allow it to be accessed via the debugger > Mike, Given that all ARM and AARCH64 DEBUG builds are still broken, may we please have your R-b on this patch so we can proceed to start fixing things? Thanks, Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard, Just a few comments: * Please pre-initialize global mEfiAtRuntime = FALSE; * The UNI file in the patch is identical to BaseDebugLibSerialPort. Please update UNI file header and strings to describe this as the runtime version of the DebugLib as described in the .inf and .C files. * gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue should be SOMETIMES_CONSUMES in the INF file. With those changes: Reviewed-by: Michael D Kinney <Michael.d.kinney@intel.com> Mike > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, February 23, 2018 6:10 AM > To: edk2-devel@lists.01.org > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Laszlo > Ersek <lersek@redhat.com>; Gao, Liming > <liming.gao@intel.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; afish@apple.com; Zeng, > Star <star.zeng@intel.com>; Ni, Ruiyu > <ruiyu.ni@intel.com>; Ard Biesheuvel > <ard.biesheuvel@linaro.org> > Subject: Re: [PATCH v4 0/2] Create UART DebugLib > implementation for runtime drivers > > On 22 February 2018 at 19:56, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: > > Commit 4bf95a9f361e > ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug > > message") broke the DEBUG build for systems using a > MMIO mapped UART for > > DEBUG output. In other words, it broke the build for > all ARM and AARCH64 > > systems, given that port I/O does not exist on those > architectures. > > > > Instead of patching it up locally, let's fix this > issue once and for all, > > by creating a UART DebugLib implementation for > DXE_RUNTIME_DRIVER modules > > that does the right thing by default. > > > > v4: > > - add Laszlo's R-b > > - keep ASSERT() message in local buffer even it is > not printed to the serial > > port, to allow it to be accessed via the debugger > > > > Mike, > > Given that all ARM and AARCH64 DEBUG builds are still > broken, may we > please have your R-b on this patch so we can proceed to > start fixing > things? > > Thanks, > Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 23 February 2018 at 22:36, Kinney, Michael D <michael.d.kinney@intel.com> wrote: > Ard, > > Just a few comments: > * Please pre-initialize global mEfiAtRuntime = FALSE; > * The UNI file in the patch is identical to BaseDebugLibSerialPort. > Please update UNI file header and strings to describe this as the > runtime version of the DebugLib as described in the .inf and .C files. > * gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue should be > SOMETIMES_CONSUMES in the INF file. > > With those changes: > > Reviewed-by: Michael D Kinney <Michael.d.kinney@intel.com> > Thanks all Pushed as 8bdb0221152c..ebfca258f5d7 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message") broke the DEBUG build for systems using a MMIO mapped UART for DEBUG output. In other words, it broke the build for all ARM and AARCH64 systems, given that port I/O does not exist on those architectures. Instead of patching it up locally, let's fix this issue once and for all, by creating a UART DebugLib implementation for DXE_RUNTIME_DRIVER modules that does the right thing by default. v4: - add Laszlo's R-b - keep ASSERT() message in local buffer even it is not printed to the serial port, to allow it to be accessed via the debugger v3: - revert PCD changes, as they are unnecessary for the PCDs in question (since they are fixed or patchable only) - drop redundant mEfiAtRuntime checks around ASSERT()s - revert whitespace cleanup, i.e., move back to BaseDebugLibSerialPort origin for most of the file v2: - incorporate Laszlo's feedback into patch #1 - add Laszlo's R-b to patch #2 - dropped patch #3 that blacklists the original BASE implementation for DXE_RUNTIME_DRIVER modules Note that I retained the deadloop/breakpoint code for ASSERT()s at runtime. I think it is perfectly reasonable behavior for a DEBUG build at runtime, even if other cores may be up and running as well: the purpose of these facilities is to allow a debugger to attach to the CPU to figure out what has happened, and both deadloops and breakpoints can achieve that just fine even at runtime. Ard Biesheuvel (2): MdePkg: introduce DxeRuntimeDebugLibSerialPort ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate ArmVirtPkg/ArmVirt.dsc.inc | 3 + MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c | 346 ++++++++++++++++++++ MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf | 55 ++++ MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni | 21 ++ 4 files changed, 425 insertions(+) create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel