+Sami On Tue, May 19, 2020 at 12:00:02 +0200, Laszlo Ersek wrote: > On 05/18/20 19:11, Ard Biesheuvel wrote: > > In the ArmPkg version of PlatformBootManagerLib, we construct a > > serial device path based on the default settings for baud rate, > > parity and the number of stop bits, to ensure that a serial console > > is available even on the very first boot. > > > > This assumes that PcdUartDefaultParity or PcdUartDefaultStopBits are > > not set to '0', meaning 'the default', as there is no default for > > these when constructing a device path. > > > > So add a couple of ASSERT()s to make sure that we catch this condition, > > since it otherwise ignores the bogus device path silently, which is > > rather tedious to debug,. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheu...@arm.com> > > --- > > ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > index e6e788e0f107..a030d510aa62 100644 > > --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c > > @@ -583,6 +583,8 @@ PlatformBootManagerBeforeConsole ( > > // > > // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut. > > // > > + ASSERT (FixedPcdGet8 (PcdUartDefaultParity) > 0); > > + ASSERT (FixedPcdGet8 (PcdUartDefaultStopBits) > 0); > > ASSERT (FixedPcdGet8 (PcdDefaultTerminalType) == 4); > > CopyGuid (&mSerialConsole.TermType.Guid, &gEfiTtyTermGuid); > > > > > > Given that these are fixed PCDs, I'd recommend STATIC_ASSERT(). We've > recently put STATIC_ASSERT() to use (in "OvmfPkg/MptScsiDxe/MptScsi.c") > in combination with a fixed PCD: > > STATIC_ASSERT ( > FixedPcdGet8 (PcdMptScsiMaxTargetLimit) < 255, > "Req supports 255 targets only (max target is 254)" > ); > > Just an idea of course; if that's out of scope for now, I have nothing > against this patch.
That sounds useful, but should then probably get applied to the TerminalType as well, so could come in as a separate patch on top. Sami: could this be an alternative resolution for https://edk2.groups.io/g/devel/message/59740 ? Regards, Leif > Thanks, > Laszlo > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59805): https://edk2.groups.io/g/devel/message/59805 Mute This Topic: https://groups.io/mt/74297223/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
