* [edk2-platforms][PATCH 0/3] Platform/RPi: Fix compatibility with recent start.elf @ 2020-05-04 11:15 Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code Pete Batard ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Pete Batard @ 2020-05-04 11:15 UTC (permalink / raw) To: devel; +Cc: ard.biesheuvel, leif, awarkentin As per https://github.com/raspberrypi/firmware/issues/1376 we found two issues with recent versions of the Raspberry Pi Foundation's start.elf on the Pi 3: 1. Our timeout delay for mbox reply was too short, leading to loss of USB due to USB mbox power state request not being applied. 2. The MiniUART baudrate shifted from the expected value due to the Raspberry Pi Foundation deviating from their official documentation and increasing the default core clock (which the MiniUART bases its baudrate divisor on) to 400 MHz, instead using a fixed 250 MHz, when 'enable_uart=1' is in effect. Fixing the first issue is fairly straightforward, and is done in patch 1/3, where we also take this opportunity to improve the overall mbox code. Fixing the second issue requires a little more involvement as we want to ensure that we no longer depend on a fixed PCD clock rate to derive the MiniUART baudrate divisor, but instead use the actual core clock frequency retreived from mbox. However, because there basically exists two instances of DualSerialLib in the firmware (one in PEI phase and one in DXE) and the serial initialization is done very early, we have to use the following process: 1. ArmPlatformPeiBootAction set the core clock of the PEI instance of DualSerialLib, which enables the PEI serial instance to be set with a proper baudrate. 2. A newly introduced PlatformPeiLib provides a PlatformPeim () call that reads the global value from the PEI serial instance and stores it into a GUID Hob. 3. In DXE phase, when the DXE instance of DualSerialLib attempts to read the core clock value for the first time, it detects that it has not been set and sets its value from the GUID Hob. It needs to be pointed out however that we can't use HobLib helper functions such as GetFirstGuidHob () or even GetHobList () directly because DualSerialLib gets instantiated before the DXE version of HobLib. However we work around that by using PrePeiGetHobList (). This serial baudrate fixup is accomplished in patch 2/3. We also take this opportunity to improve the Pi 4 code, which currently doesn't use the core clock frequency at all, andt instead applies a variable 12.12 fixed divisor (which we can always access without mbox transactions) onto a 1 GHz fixed clock. It should be noted that, with 2/3 applied, users of the Pi firmware have the new capability of being able to override the core frequency to whichever value they like (by adding 'core_freq=###' in their 'config.txt') without losing MiniUART baudrate, as was the case without this patch. Finally, considering that the knowing the actual core frequency the system booted with can be useful during early init, we add patch 3/3 to display this value for DEBUG builds. Andrei Warkentin (2): Platform/RPi: Fortify mailbox code Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation Pete Batard (1): Platform/RPi: Report core clock frequency during early init Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +- Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h | 22 ++++ Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h | 11 +- Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 106 +++++++++++++++++--- Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf | 6 ++ Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 83 ++++++++------- Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 8 +- Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c | 33 ++++++ Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf | 47 +++++++++ Platform/RaspberryPi/RPi3/RPi3.dsc | 8 +- Platform/RaspberryPi/RPi4/RPi4.dsc | 9 +- Platform/RaspberryPi/RaspberryPi.dec | 1 + 12 files changed, 278 insertions(+), 67 deletions(-) create mode 100644 Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h create mode 100644 Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c create mode 100644 Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf -- 2.21.0.windows.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code 2020-05-04 11:15 [edk2-platforms][PATCH 0/3] Platform/RPi: Fix compatibility with recent start.elf Pete Batard @ 2020-05-04 11:15 ` Pete Batard 2020-05-06 12:39 ` Ard Biesheuvel 2020-05-04 11:15 ` [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init Pete Batard 2 siblings, 1 reply; 11+ messages in thread From: Pete Batard @ 2020-05-04 11:15 UTC (permalink / raw) To: devel; +Cc: ard.biesheuvel, leif, awarkentin, Andrei Warkentin From: Andrei Warkentin <andrey.warkentin@gmail.com> As part of the analysis done in: https://github.com/raspberrypi/firmware/issues/1376: * Bump up max tries, to avoid command time-outs. * Macro-ify RaspberryPiHelper.S some more to make code more maintainable. * Add ".align 4" before every command buffer. Co-authored-by: Pete Batard <pete@akeo.ie> Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com> Signed-off-by: Pete Batard <pete@akeo.ie> --- Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +--- Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h | 11 +++- Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 60 ++++++++------------ 3 files changed, 37 insertions(+), 45 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c index 40c78b5d57cf..6c45cf47f082 100644 --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c @@ -1,5 +1,6 @@ /** @file * + * Copyright (c) 2020, Pete Batard <pete@akeo.ie> * Copyright (c) 2019, ARM Limited. All rights reserved. * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com> * Copyright (c) 2016, Linaro, Ltd. All rights reserved. @@ -30,12 +31,6 @@ // #define NUM_PAGES 1 -// -// The number of iterations to perform when waiting for the mailbox -// status to change -// -#define MAX_TRIES 0x100000 - STATIC VOID *mDmaBuffer; STATIC VOID *mDmaBufferMapping; STATIC UINTN mDmaBufferBusAddress; @@ -62,7 +57,7 @@ DrainMailbox ( } ArmDataSynchronizationBarrier (); MmioRead32 (BCM2836_MBOX_BASE_ADDRESS + BCM2836_MBOX_READ_OFFSET); - } while (++Tries < MAX_TRIES); + } while (++Tries < RPI_MBOX_MAX_TRIES); return FALSE; } @@ -86,7 +81,7 @@ MailboxWaitForStatusCleared ( return TRUE; } ArmDataSynchronizationBarrier (); - } while (++Tries < MAX_TRIES); + } while (++Tries < RPI_MBOX_MAX_TRIES); return FALSE; } diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h index 584786a61dfd..3328be582df1 100644 --- a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h @@ -1,6 +1,6 @@ /** @file * - * Copyright (c) 2019, Pete Batard <pete@akeo.ie> + * Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie> * Copyright (c) 2016, Linaro Limited. All rights reserved. * * SPDX-License-Identifier: BSD-2-Clause-Patent @@ -10,6 +10,15 @@ #ifndef __RASPBERRY_PI_MAILBOX_H__ #define __RASPBERRY_PI_MAILBOX_H__ +/* + * Number of iterations to perform when waiting for the mailbox. + * + * This number was arrived at empirically, following discussion + * at https://github.com/raspberrypi/firmware/issues/1376, to + * avoid mailbox time-outs on some commands. + */ +#define RPI_MBOX_MAX_TRIES 0x8000000 + /* Mailbox channels */ #define RPI_MBOX_PM_CHANNEL 0 #define RPI_MBOX_FB_CHANNEL 1 diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S index cc58406e1bfc..91dfe1bb981e 100644 --- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S @@ -1,6 +1,7 @@ /** @file * - * Copyright (c) 2019, Pete Batard <pete@akeo.ie> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com> + * Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie> * Copyright (c) 2016, Linaro Limited. All rights reserved. * Copyright (c) 2011-2013, ARM Limited. All rights reserved. * @@ -13,10 +14,8 @@ #include <IndustryStandard/Bcm2836.h> #include <IndustryStandard/RpiMbox.h> -#define MAX_TRIES 0x100000 - .macro drain - mov x5, #MAX_TRIES + mov x5, #RPI_MBOX_MAX_TRIES 0: ldr w6, [x4, #BCM2836_MBOX_STATUS_OFFSET] tbnz w6, #BCM2836_MBOX_STATUS_EMPTY, 1f dmb ld @@ -27,7 +26,7 @@ .endm .macro poll, status - mov x5, #MAX_TRIES + mov x5, #RPI_MBOX_MAX_TRIES 0: ldr w6, [x4, #BCM2836_MBOX_STATUS_OFFSET] tbz w6, #\status, 1f dmb ld @@ -36,23 +35,30 @@ 1: .endm + .macro run, command_buffer + adr x0, \command_buffer + orr x0, x0, #RPI_MBOX_VC_CHANNEL + add x0, x0, x1 + + poll BCM2836_MBOX_STATUS_FULL + str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] + dmb sy + poll BCM2836_MBOX_STATUS_EMPTY + dmb sy + ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] + dmb ld + .endm + ASM_FUNC (ArmPlatformPeiBootAction) - adr x0, .Lmeminfo_buffer mov x1, #FixedPcdGet64 (PcdDmaDeviceOffset) orr x0, x0, #RPI_MBOX_VC_CHANNEL // x1 holds the value of PcdDmaDeviceOffset throughout this function - add x0, x0, x1 MOV32 (x4, BCM2836_MBOX_BASE_ADDRESS) drain - poll BCM2836_MBOX_STATUS_FULL - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] - dmb sy - poll BCM2836_MBOX_STATUS_EMPTY - dmb sy - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] - dmb ld + + run .Lmeminfo_buffer ldr w0, .Lmembase adr x2, mSystemMemoryBase @@ -63,17 +69,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) adr x2, mSystemMemoryEnd str x0, [x2] - adr x0, .Lvcinfo_buffer - orr x0, x0, #RPI_MBOX_VC_CHANNEL - add x0, x0, x1 - - poll BCM2836_MBOX_STATUS_FULL - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] - dmb sy - poll BCM2836_MBOX_STATUS_EMPTY - dmb sy - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] - dmb ld + run .Lvcinfo_buffer ldr w0, .Lvcbase adr x2, mVideoCoreBase @@ -83,17 +79,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) adr x2, mVideoCoreSize str x0, [x2] - adr x0, .Lrevinfo_buffer - orr x0, x0, #RPI_MBOX_VC_CHANNEL - add x0, x0, x1 - - poll BCM2836_MBOX_STATUS_FULL - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] - dmb sy - poll BCM2836_MBOX_STATUS_EMPTY - dmb sy - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] - dmb ld + run .Lrevinfo_buffer ldr w0, .Lrevision adr x2, mBoardRevision @@ -115,6 +101,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) .long 0 // end tag .set .Lmeminfo_size, . - .Lmeminfo_buffer + .align 4 .Lvcinfo_buffer: .long .Lvcinfo_size .long 0x0 @@ -128,6 +115,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) .long 0 // end tag .set .Lvcinfo_size, . - .Lvcinfo_buffer + .align 4 .Lrevinfo_buffer: .long .Lrevinfo_size .long 0x0 -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code 2020-05-04 11:15 ` [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code Pete Batard @ 2020-05-06 12:39 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2020-05-06 12:39 UTC (permalink / raw) To: Pete Batard, devel; +Cc: leif, awarkentin, Andrei Warkentin On 5/4/20 1:15 PM, Pete Batard wrote: > From: Andrei Warkentin <andrey.warkentin@gmail.com> > > As part of the analysis done in: > https://github.com/raspberrypi/firmware/issues/1376: > * Bump up max tries, to avoid command time-outs. > * Macro-ify RaspberryPiHelper.S some more to make code more maintainable. > * Add ".align 4" before every command buffer. > > Co-authored-by: Pete Batard <pete@akeo.ie> > Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com> > Signed-off-by: Pete Batard <pete@akeo.ie> This patch Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Pushed as 725d1198e262..35a5402a718f > --- > Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +--- > Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h | 11 +++- > Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 60 ++++++++------------ > 3 files changed, 37 insertions(+), 45 deletions(-) > > diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c > index 40c78b5d57cf..6c45cf47f082 100644 > --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c > +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c > @@ -1,5 +1,6 @@ > /** @file > * > + * Copyright (c) 2020, Pete Batard <pete@akeo.ie> > * Copyright (c) 2019, ARM Limited. All rights reserved. > * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com> > * Copyright (c) 2016, Linaro, Ltd. All rights reserved. > @@ -30,12 +31,6 @@ > // > #define NUM_PAGES 1 > > -// > -// The number of iterations to perform when waiting for the mailbox > -// status to change > -// > -#define MAX_TRIES 0x100000 > - > STATIC VOID *mDmaBuffer; > STATIC VOID *mDmaBufferMapping; > STATIC UINTN mDmaBufferBusAddress; > @@ -62,7 +57,7 @@ DrainMailbox ( > } > ArmDataSynchronizationBarrier (); > MmioRead32 (BCM2836_MBOX_BASE_ADDRESS + BCM2836_MBOX_READ_OFFSET); > - } while (++Tries < MAX_TRIES); > + } while (++Tries < RPI_MBOX_MAX_TRIES); > > return FALSE; > } > @@ -86,7 +81,7 @@ MailboxWaitForStatusCleared ( > return TRUE; > } > ArmDataSynchronizationBarrier (); > - } while (++Tries < MAX_TRIES); > + } while (++Tries < RPI_MBOX_MAX_TRIES); > > return FALSE; > } > diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h > index 584786a61dfd..3328be582df1 100644 > --- a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h > +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h > @@ -1,6 +1,6 @@ > /** @file > * > - * Copyright (c) 2019, Pete Batard <pete@akeo.ie> > + * Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie> > * Copyright (c) 2016, Linaro Limited. All rights reserved. > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -10,6 +10,15 @@ > #ifndef __RASPBERRY_PI_MAILBOX_H__ > #define __RASPBERRY_PI_MAILBOX_H__ > > +/* > + * Number of iterations to perform when waiting for the mailbox. > + * > + * This number was arrived at empirically, following discussion > + * at https://github.com/raspberrypi/firmware/issues/1376, to > + * avoid mailbox time-outs on some commands. > + */ > +#define RPI_MBOX_MAX_TRIES 0x8000000 > + > /* Mailbox channels */ > #define RPI_MBOX_PM_CHANNEL 0 > #define RPI_MBOX_FB_CHANNEL 1 > diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S > index cc58406e1bfc..91dfe1bb981e 100644 > --- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S > +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S > @@ -1,6 +1,7 @@ > /** @file > * > - * Copyright (c) 2019, Pete Batard <pete@akeo.ie> > + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com> > + * Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie> > * Copyright (c) 2016, Linaro Limited. All rights reserved. > * Copyright (c) 2011-2013, ARM Limited. All rights reserved. > * > @@ -13,10 +14,8 @@ > #include <IndustryStandard/Bcm2836.h> > #include <IndustryStandard/RpiMbox.h> > > -#define MAX_TRIES 0x100000 > - > .macro drain > - mov x5, #MAX_TRIES > + mov x5, #RPI_MBOX_MAX_TRIES > 0: ldr w6, [x4, #BCM2836_MBOX_STATUS_OFFSET] > tbnz w6, #BCM2836_MBOX_STATUS_EMPTY, 1f > dmb ld > @@ -27,7 +26,7 @@ > .endm > > .macro poll, status > - mov x5, #MAX_TRIES > + mov x5, #RPI_MBOX_MAX_TRIES > 0: ldr w6, [x4, #BCM2836_MBOX_STATUS_OFFSET] > tbz w6, #\status, 1f > dmb ld > @@ -36,23 +35,30 @@ > 1: > .endm > > + .macro run, command_buffer > + adr x0, \command_buffer > + orr x0, x0, #RPI_MBOX_VC_CHANNEL > + add x0, x0, x1 > + > + poll BCM2836_MBOX_STATUS_FULL > + str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] > + dmb sy > + poll BCM2836_MBOX_STATUS_EMPTY > + dmb sy > + ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] > + dmb ld > + .endm > + > ASM_FUNC (ArmPlatformPeiBootAction) > - adr x0, .Lmeminfo_buffer > mov x1, #FixedPcdGet64 (PcdDmaDeviceOffset) > orr x0, x0, #RPI_MBOX_VC_CHANNEL > // x1 holds the value of PcdDmaDeviceOffset throughout this function > - add x0, x0, x1 > > MOV32 (x4, BCM2836_MBOX_BASE_ADDRESS) > > drain > - poll BCM2836_MBOX_STATUS_FULL > - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] > - dmb sy > - poll BCM2836_MBOX_STATUS_EMPTY > - dmb sy > - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] > - dmb ld > + > + run .Lmeminfo_buffer > > ldr w0, .Lmembase > adr x2, mSystemMemoryBase > @@ -63,17 +69,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) > adr x2, mSystemMemoryEnd > str x0, [x2] > > - adr x0, .Lvcinfo_buffer > - orr x0, x0, #RPI_MBOX_VC_CHANNEL > - add x0, x0, x1 > - > - poll BCM2836_MBOX_STATUS_FULL > - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] > - dmb sy > - poll BCM2836_MBOX_STATUS_EMPTY > - dmb sy > - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] > - dmb ld > + run .Lvcinfo_buffer > > ldr w0, .Lvcbase > adr x2, mVideoCoreBase > @@ -83,17 +79,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) > adr x2, mVideoCoreSize > str x0, [x2] > > - adr x0, .Lrevinfo_buffer > - orr x0, x0, #RPI_MBOX_VC_CHANNEL > - add x0, x0, x1 > - > - poll BCM2836_MBOX_STATUS_FULL > - str w0, [x4, #BCM2836_MBOX_WRITE_OFFSET] > - dmb sy > - poll BCM2836_MBOX_STATUS_EMPTY > - dmb sy > - ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] > - dmb ld > + run .Lrevinfo_buffer > > ldr w0, .Lrevision > adr x2, mBoardRevision > @@ -115,6 +101,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) > .long 0 // end tag > .set .Lmeminfo_size, . - .Lmeminfo_buffer > > + .align 4 > .Lvcinfo_buffer: > .long .Lvcinfo_size > .long 0x0 > @@ -128,6 +115,7 @@ ASM_FUNC (ArmPlatformPeiBootAction) > .long 0 // end tag > .set .Lvcinfo_size, . - .Lvcinfo_buffer > > + .align 4 > .Lrevinfo_buffer: > .long .Lrevinfo_size > .long 0x0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation 2020-05-04 11:15 [edk2-platforms][PATCH 0/3] Platform/RPi: Fix compatibility with recent start.elf Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code Pete Batard @ 2020-05-04 11:15 ` Pete Batard 2020-05-05 10:05 ` Ard Biesheuvel 2020-05-04 11:15 ` [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init Pete Batard 2 siblings, 1 reply; 11+ messages in thread From: Pete Batard @ 2020-05-04 11:15 UTC (permalink / raw) To: devel; +Cc: ard.biesheuvel, leif, awarkentin, Andrei Warkentin From: Andrei Warkentin <andrey.warkentin@gmail.com> Fix for https://github.com/raspberrypi/firmware/issues/1376. For the Pi 3, to properly configure miniUART, we need the core clock, which can be vary between VideoCore firmare release or config.txt options. Unfortunately, it's painful to get - it's only available via the mailbox interface. Additionally, SerialLib is a very limited environment, even when linked in a DXE-mode component, because as part of a DebugLib implementation it ends being the base prerequisite of everything. That means a "normal" mailbox implementation like the one from RpiFirmwareDxe (with dependencies on DmaLib) is out of the question. Using a basic implementation such as the one in PlatformLib doesn't work either because it operates in both environments with MMU on (DXE phase) and MMU off (SEC/PrePi). Ideally, we read the value via mbox exactly once at boot (PlatformLib), and then somehow stash it. A GUID Hob sounds appropriate, yet when SerialPortLib operates in DXE components, we can't use the HobLib to *locate* the Hob list itself (remember, SerialPortLib initializes before any of HobLib's dependencies, like UeflLib...). FORTUNATELY, there is a way out - we can use PrePeiGetHobList to cut through to the HOB list pointer stashed by PrePi without dealing with any of the libraries meant for DXE. Once we have the Hob list pointer, we can use the regular DXE HobLib routines safely (so long as they are the ones taking a HobList pointer as a parameter). So we can link DualSerialPortLib with whatever HobLib that is appropriate for the environment - just need to also link in PrePiHobListPointerLib. gSerialLibCoreClockFreq is always externally set when DualSerialPortLib run as part of PrePi. So - if gSerialLibCoreClockFreq is not set, then we must be in the DXE phase and can fetch the Hob list via PrePeiGetHobList. The GUID Hob is registered as part of a Pi-customized PlatformPeiLib. For the Pi 4, the situation is a bit different, as the system clock used to compute the baud rate is not derived directly from the core clock, but instead from a fixed clock (current 1 GHz) to which a 12.12 fixed point divisor is applied. But even though we don't use it, we still initialize gSerialLibCoreClockFreq a.k.a. the VPU core frequency, as this can be useful to have. As a result of the above, the PcdSerialClockRate values in the .dsc are also updated, so that it can be used as a fallback to the default core frequency in case of the Pi 3, and as the fixed frequency to which the fixed point divisor is applied in case of the Pi 4. Co-authored-by: Pete Batard <pete@akeo.ie> Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com> Signed-off-by: Pete Batard <pete@akeo.ie> --- Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h | 22 ++++ Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 106 +++++++++++++++++--- Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf | 6 ++ Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 23 +++++ Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c | 33 ++++++ Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf | 47 +++++++++ Platform/RaspberryPi/RPi3/RPi3.dsc | 8 +- Platform/RaspberryPi/RPi4/RPi4.dsc | 9 +- Platform/RaspberryPi/RaspberryPi.dec | 1 + 9 files changed, 234 insertions(+), 21 deletions(-) diff --git a/Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h b/Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h new file mode 100644 index 000000000000..82864eb33268 --- /dev/null +++ b/Platform/RaspberryPi/Include/Guid/DualSerialPortLibHobGuid.h @@ -0,0 +1,22 @@ +/** @file +* +* Copyright (c) 2020, Pete Batard <pete@akeo.ie> +* Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com> +* +* SPDX-License-Identifier: BSD-2-Clause-Patent +* +**/ + +#ifndef _DUAL_SERIAL_PORT_LIB_HOB_GUID_H_ +#define _DUAL_SERIAL_PORT_LIB_HOB_GUID_H_ + +#define DUAL_SERIAL_PORT_LIB_HOB_GUID \ + { 0x1CB0EB5B, 0x4F73, 0x4308, { 0xA3, 0x33, 0x1D, 0x95, 0xBE, 0x5F, 0x30, 0xB5 } } + +extern GUID gDualSerialPortLibHobGuid; + +typedef struct { + UINT32 CoreClockFreq; +} DUAL_SERIAL_PORT_LIB_VARS; + +#endif /* _DUAL_SERIAL_PORT_LIB_HOB_GUID_H_ */ diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c index 05e12f383785..77b5f586b6c5 100644 --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c @@ -1,6 +1,7 @@ /** @file 16550 and PL011 Serial Port library functions for Raspberry Pi + Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com> Copyright (c) 2020, Pete Batard <pete@akeo.ie> Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR> Copyright (c) 2014, Hewlett-Packard Development Company, L.P.<BR> @@ -12,7 +13,8 @@ **/ -#include <Base.h> +#include <Uefi.h> +#include <Guid/DualSerialPortLibHobGuid.h> #include <IndustryStandard/Bcm2836.h> #include <IndustryStandard/Bcm2836Gpio.h> #include <Library/BaseLib.h> @@ -21,9 +23,21 @@ #include <Library/PL011UartClockLib.h> #include <Library/PL011UartLib.h> #include <Library/SerialPortLib.h> +// +// The order of the following includes should not be altered +// +#include <Library/PrePiHobListPointerLib.h> +#include <Pi/PiMultiPhase.h> +#include <Library/HobLib.h> -BOOLEAN UsePl011Uart = FALSE; -BOOLEAN UsePl011UartSet = FALSE; +STATIC BOOLEAN UsePl011Uart = FALSE; +STATIC BOOLEAN UsePl011UartSet = FALSE; +UINT32 gSerialLibCoreClockFreq = 0; +// +// Set by ArmPlatformPeiBootAction to skip PrePeiGetHobList, +// as it won't be initialized and could return garbage. +// +UINT32 gSerialLibCoreSkipHob = 0; #define PL011_UART_REGISTER_BASE BCM2836_PL011_UART_BASE_ADDRESS #define MINI_UART_REGISTER_BASE (BCM2836_MINI_UART_BASE_ADDRESS + 0x40) @@ -152,26 +166,86 @@ SerialPortGetDivisor ( UINT32 SerialBaudRate ) { - UINT64 BaseClockRate; - UINT32 Divisor; + UINT64 BaseSerialClockFreq; + UINT32 Divisor; + +#if (RPI_MODEL == 3) + VOID *HobList; + EFI_HOB_GUID_TYPE *GuidHob; + DUAL_SERIAL_PORT_LIB_VARS *Vars; + + // + // For the Pi 3, the base serial clock is the VPU core clock. + // + // Unfortunately, it's painful to get - it's only available via + // the mailbox interface. Additionally, SerialLib is a very + // limited environment, even when linked in a DXE-mode component, because + // as part of a DebugLib implementation it ends being the base prerequisite + // of /everything/. That means a "normal" mailbox implementation like + // the one im RpiFirmwareDxe (with dependencies on DmaLib) is out + // of the question. Using a basic implementation such as the one + // in PlatformLib doesn't work either because operate in both + // environments with MMU on (DXE phase) and MMU off (SEC/PrePi). + // + // Ideally, we read the value via mbox exactly once (PlatformLib), + // then somehow stash it. A GUID Hob sounds pretty nice, but + // we can't use the DXE HobLib to *locate* the Hob list itself + // (remember, SerialPortLib initializes before any of HobLib's + // dependencies, like UeflLib). + // + // FORTUNATELY, there is a way out - we can use PrePeiGetHobList + // to cut through to the HOB list pointer stashed by PrePi without. + // Once we have the Hob list pointer, we can use the DXE HobLib routines + // safely (so long as they are the ones taking a HobList pointer + // as a parameter). + // + // gSerialLibCoreClockFreq is always externally set when we run + // as part of PrePi. So - if gSerialLibCoreClockFreq is not set, + // then we must be in the DXE phase and can fetch the Hob list + // via our little hack. + // + if (gSerialLibCoreClockFreq == 0 && !gSerialLibCoreSkipHob) { + HobList = PrePeiGetHobList (); + if (HobList != NULL) { + GuidHob = (EFI_HOB_GUID_TYPE *) GetNextGuidHob (&gDualSerialPortLibHobGuid, + HobList); + if (GuidHob != NULL) { + Vars = (DUAL_SERIAL_PORT_LIB_VARS *) GET_GUID_HOB_DATA (GuidHob); + gSerialLibCoreClockFreq = Vars->CoreClockFreq; + gSerialLibCoreSkipHob = 1; + } + } + } // - // On the Raspberry Pi, the clock to use for the 16650-compatible UART - // is the base clock divided by the 12.12 fixed point VPU clock divisor. + // Fall back to PCD if we have a nonsensical value. + // + if (gSerialLibCoreClockFreq < 10000000) { + gSerialLibCoreClockFreq = PcdGet32 (PcdSerialClockRate); + } + BaseSerialClockFreq = (UINT64) gSerialLibCoreClockFreq; +#else // - BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate) * 4; - Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF; - if (Divisor != 0) - BaseClockRate = (BaseClockRate << 12) / Divisor; + // For the Pi 4, the base serial clock is not derived from the VPU + // core clock, but from a fixed clock (currently 1 GHz) to which a + // variable 12.12 fixed point clock divisor is applied. + // + BaseSerialClockFreq = (UINT64) PcdGet32 (PcdSerialClockRate); + Divisor = MmioRead32 (BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF; + if (Divisor != 0) { + BaseSerialClockFreq = (BaseSerialClockFreq << 12) / Divisor; + } +#endif // - // Now calculate divisor for baud generator - // Ref_Clk_Rate / Baud_Rate / 16 + // As per the BCM2xxx datasheets: + // baudrate = system_clock_freq / (8 * (divisor + 1)). // - Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16); - if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= SerialBaudRate * 8) { - Divisor++; + Divisor = (UINT32)BaseSerialClockFreq / (SerialBaudRate * 8); + if (Divisor != 0) { + Divisor--; } + return Divisor; } diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf index af1e6b026fe6..7430eede18db 100644 --- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf +++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf @@ -22,17 +22,23 @@ [Packages] EmbeddedPkg/EmbeddedPkg.dec MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec + Platform/RaspberryPi/RaspberryPi.dec Silicon/Broadcom/Bcm283x/Bcm283x.dec [LibraryClasses] IoLib + HobLib PcdLib PL011UartClockLib PL011UartLib + PrePiHobListPointerLib [Sources] DualSerialPortLib.c +[Guids] + gDualSerialPortLibHobGuid + [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterAccessWidth ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S index 91dfe1bb981e..83a3511d3507 100644 --- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S +++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S @@ -85,6 +85,16 @@ ASM_FUNC (ArmPlatformPeiBootAction) adr x2, mBoardRevision str w0, [x2] + run .Lclkinfo_buffer + + ldr w0, .Lfrequency + adr x2, gSerialLibCoreClockFreq + str w0, [x2] + adr x2, gSerialLibCoreSkipHob + // + // Any non-zero value will do. + // + str w0, [x2] ret .align 4 @@ -127,6 +137,19 @@ ASM_FUNC (ArmPlatformPeiBootAction) .long 0 // end tag .set .Lrevinfo_size, . - .Lrevinfo_buffer + .align 4 +.Lclkinfo_buffer: + .long .Lclkinfo_size + .long 0x0 + .long RPI_MBOX_GET_CLOCK_RATE + .long 8 // buf size + .long 4 // input len + .long 4 // clock id: 0x04 = Core/VPU +.Lfrequency: + .long 0 // frequency + .long 0 // end tag + .set .Lclkinfo_size, . - .Lclkinfo_buffer + //UINTN //ArmPlatformGetPrimaryCoreMpId ( // VOID diff --git a/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c b/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c new file mode 100644 index 000000000000..367a32bd4ca7 --- /dev/null +++ b/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.c @@ -0,0 +1,33 @@ +/** @file +* +* Copyright (c) 2011-2014, ARM Limited. All rights reserved. +* +* SPDX-License-Identifier: BSD-2-Clause-Patent +* +**/ + +#include <PiPei.h> + +#include <Library/ArmPlatformLib.h> +#include <Library/HobLib.h> +#include <Library/PcdLib.h> +#include <Library/DebugLib.h> +#include <Guid/DualSerialPortLibHobGuid.h> + +extern UINT32 gSerialLibCoreClockFreq, gSerialLibCoreSkipHob; + +static DUAL_SERIAL_PORT_LIB_VARS mUartLibVars; + +EFI_STATUS +EFIAPI +PlatformPeim ( + VOID + ) +{ + BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize)); + + mUartLibVars.CoreClockFreq = gSerialLibCoreClockFreq; + BuildGuidDataHob (&gDualSerialPortLibHobGuid, &mUartLibVars, sizeof(mUartLibVars)); + + return EFI_SUCCESS; +} diff --git a/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf b/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf new file mode 100644 index 000000000000..8e2bab14a89a --- /dev/null +++ b/Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf @@ -0,0 +1,47 @@ +#/** @file +# +# Copyright (c) 2011-2012, ARM Limited. All rights reserved. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +#**/ + +[Defines] + INF_VERSION = 0x0001001A + BASE_NAME = ArmPlatformPeiLib + FILE_GUID = 49d37060-70b5-11e0-aa2d-0002a5d5c51b + MODULE_TYPE = SEC + VERSION_STRING = 1.0 + LIBRARY_CLASS = PlatformPeiLib + +[Sources] + PlatformPeiLib.c + +[Packages] + ArmPkg/ArmPkg.dec + ArmPlatformPkg/ArmPlatformPkg.dec + MdePkg/MdePkg.dec + EmbeddedPkg/EmbeddedPkg.dec + MdeModulePkg/MdeModulePkg.dec + Platform/RaspberryPi/RaspberryPi.dec + +[LibraryClasses] + ArmPlatformLib + DebugLib + HobLib + +[Guids] + gDualSerialPortLibHobGuid + +[Ppis] + gEfiPeiMasterBootModePpiGuid # PPI ALWAYS_PRODUCED + gEfiPeiBootInRecoveryModePpiGuid # PPI SOMETIMES_PRODUCED + +[FixedPcd] + gArmTokenSpaceGuid.PcdFdBaseAddress + gArmTokenSpaceGuid.PcdFdSize + gArmTokenSpaceGuid.PcdFvBaseAddress + gArmTokenSpaceGuid.PcdFvSize + +[Depex] + TRUE diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc index 563fb891b841..efebf1f823b3 100644 --- a/Platform/RaspberryPi/RPi3/RPi3.dsc +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc @@ -77,6 +77,10 @@ [LibraryClasses.common] UefiLib|MdePkg/Library/UefiLib/UefiLib.inf HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf + # + # PrePiHobListPointerLib is necessary for DualSerialPortLib. + # + PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf @@ -174,7 +178,7 @@ [LibraryClasses.common.SEC] PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf MemoryInitPeiLib|Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf - PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf + PlatformPeiLib|Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf @@ -409,7 +413,7 @@ [PcdsFixedAtBuild.common] gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4 - gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000 + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|250000000 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc index 4deccd9d3ecc..4dc9b493cf79 100644 --- a/Platform/RaspberryPi/RPi4/RPi4.dsc +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc @@ -75,6 +75,10 @@ [LibraryClasses.common] UefiLib|MdePkg/Library/UefiLib/UefiLib.inf HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf + # + # PrePiHobListPointerLib is necessary for DualSerialPortLib. + # + PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf @@ -182,12 +186,11 @@ [LibraryClasses.common.SEC] PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf MemoryInitPeiLib|Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf - PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf + PlatformPeiLib|Platform/RaspberryPi/Library/PlatformPeiLib/PlatformPeiLib.inf ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf - PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf [LibraryClasses.common.DXE_CORE] @@ -420,7 +423,7 @@ [PcdsFixedAtBuild.common] gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4 - gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000 + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27 gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8 gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200 diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec index 66ef6186644b..a915387fc822 100644 --- a/Platform/RaspberryPi/RaspberryPi.dec +++ b/Platform/RaspberryPi/RaspberryPi.dec @@ -26,6 +26,7 @@ [Guids] gRaspberryPiTokenSpaceGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3, 0x63, 0xB0, 0xB8, 0xEE, 0xD6, 0xB5}} gRaspberryPiEventResetGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3, 0x63, 0xB4, 0xB4, 0xE4, 0xD4, 0xB4}} gConfigDxeFormSetGuid = {0xCD7CC258, 0x31DB, 0x22E6, {0x9F, 0x22, 0x63, 0xB0, 0xB8, 0xEE, 0xD6, 0xB5}} + gDualSerialPortLibHobGuid = { 0x1CB0EB5B, 0x4F73, 0x4308, { 0xA3, 0x33, 0x1D, 0x95, 0xBE, 0x5F, 0x30, 0xB5 } } [PcdsFixedAtBuild.common] # -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation 2020-05-04 11:15 ` [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation Pete Batard @ 2020-05-05 10:05 ` Ard Biesheuvel 2020-05-05 11:54 ` Pete Batard 0 siblings, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2020-05-05 10:05 UTC (permalink / raw) To: Pete Batard, devel; +Cc: leif, awarkentin, Andrei Warkentin On 5/4/20 1:15 PM, Pete Batard wrote: > From: Andrei Warkentin <andrey.warkentin@gmail.com> > > Fix for https://github.com/raspberrypi/firmware/issues/1376. > > For the Pi 3, to properly configure miniUART, we need the core clock, > which can be vary between VideoCore firmare release or config.txt options. > > Unfortunately, it's painful to get - it's only available via the mailbox > interface. Additionally, SerialLib is a very limited environment, even > when linked in a DXE-mode component, because as part of a DebugLib > implementation it ends being the base prerequisite of everything. > That means a "normal" mailbox implementation like the one from > RpiFirmwareDxe (with dependencies on DmaLib) is out of the question. > Using a basic implementation such as the one in PlatformLib doesn't work > either because it operates in both environments with MMU on (DXE phase) > and MMU off (SEC/PrePi). > > Ideally, we read the value via mbox exactly once at boot (PlatformLib), > and then somehow stash it. A GUID Hob sounds appropriate, yet when > SerialPortLib operates in DXE components, we can't use the HobLib to > *locate* the Hob list itself (remember, SerialPortLib initializes > before any of HobLib's dependencies, like UeflLib...). > Yeah, the gift that keeps on giving :-) NXP are struggling with a similar issue. So the problem is that SerialPortInitialize() is called before we know what value to program, and we cannot rely on other libraries to discover this value because they may not work before their constructor has been called. So can we break the contents of SerialPortInitialize() into things that need to happen once (program the divisors etc) and things that need to happen each and every time (figure out which UART we are using in the first place)? If the second set does not suffer from the same issue, can we just move the entire logic that programs the UART block into PrePi, so that all subsequent users don't have to touch those registers at all? This means we may need two versions of DualSerialPortLib, where the one that PrePi uses may need to be attached to SerialDxe as well, so that it can reprogram the baud rate as needed. But for all the remaining DebugLib related uses, we don't really need to reprogram the UART at all afaics. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation 2020-05-05 10:05 ` Ard Biesheuvel @ 2020-05-05 11:54 ` Pete Batard 2020-05-05 12:05 ` Ard Biesheuvel 0 siblings, 1 reply; 11+ messages in thread From: Pete Batard @ 2020-05-05 11:54 UTC (permalink / raw) To: Ard Biesheuvel, devel; +Cc: leif, awarkentin, Andrei Warkentin Hi Ard, On 2020.05.05 11:05, Ard Biesheuvel wrote: > On 5/4/20 1:15 PM, Pete Batard wrote: >> From: Andrei Warkentin <andrey.warkentin@gmail.com> >> >> Fix for https://github.com/raspberrypi/firmware/issues/1376. >> >> For the Pi 3, to properly configure miniUART, we need the core clock, >> which can be vary between VideoCore firmare release or config.txt >> options. >> >> Unfortunately, it's painful to get - it's only available via the mailbox >> interface. Additionally, SerialLib is a very limited environment, even >> when linked in a DXE-mode component, because as part of a DebugLib >> implementation it ends being the base prerequisite of everything. >> That means a "normal" mailbox implementation like the one from >> RpiFirmwareDxe (with dependencies on DmaLib) is out of the question. >> Using a basic implementation such as the one in PlatformLib doesn't work >> either because it operates in both environments with MMU on (DXE phase) >> and MMU off (SEC/PrePi). >> >> Ideally, we read the value via mbox exactly once at boot (PlatformLib), >> and then somehow stash it. A GUID Hob sounds appropriate, yet when >> SerialPortLib operates in DXE components, we can't use the HobLib to >> *locate* the Hob list itself (remember, SerialPortLib initializes >> before any of HobLib's dependencies, like UeflLib...). >> > > Yeah, the gift that keeps on giving :-) NXP are struggling with a > similar issue. > > So the problem is that SerialPortInitialize() is called before we know > what value to program, and we cannot rely on other libraries to discover > this value because they may not work before their constructor has been > called. I wouldn't qualify the problem that way. The problem is that we have everything we need to program the UART by the time we get into SerialPortInitialize(), because ArmPlatformPeiBootAction() has long retrieved that one variable/value we need, which everything downstream relies on, but sharing that one variable properly, so that it available in both PEI and DXE by the time it is needed, is a headache. For all intent and purposes, we always have the value we need, because that's pretty much the very first thing we query. On the other hand, directing that value to the places that consume it is problematic because most of the mechanisms we have to do that have some kind of reliance that serial output would already have been initialized... So we have been treating it as mere problem of sharing a global variable between PEI and DXE and nothing else, hence the proposal. As a matter of fact, we started with an alternate solution where we just added an extra page on top of the NvStorage reserved region, and stored the variable there at an address that DualSerialPortLib could easily retrieve in either PEI or DXE mode (with no need for the extra PlatformPeiLib then). Hob is just the natural progression of that, to achieve it in a more EDK2-like way. > So can we break the contents of SerialPortInitialize() into things that > need to happen once (program the divisors etc) and things that need to > happen each and every time (figure out which UART we are using in the > first place)? I'm trying to understand what you're getting at here. By once and multiple time, I am assuming that you're referring to the various instantiations of the library, because I only expect SerialPortInitialize() to be called once per instance for DebugLib related uses, so technically, everything from SerialPortInitialize() could be considered a one-time operation. So I guess what you're suggesting is that we somehow drop setting up the baudrate in DXE phase and just assume that it has already been set from PEI. > If the second set does not suffer from the same issue, can > we just move the entire logic that programs the UART block into PrePi, > so that all subsequent users don't have to touch those registers at all? Well, someone can and will want to switch baudrate from Shell, which means we need to compute a dynamic divisor from the base serial clock frequency. So the only way I can see your idea work is if we re-query the mbox to obtain the base serial clock frequency in SerialPortSetAttributes(), but that means that, for the compiler to be happy, it will need to set up a DXE dependency with RpiFirmwareDxe, which we can't have in PEI phase... > This means we may need two versions of DualSerialPortLib, Unless there exists a compile time macros that indicate whether code is compiling in PEI or DXE phase (and even then I suspect that .inf sections will not cooperate that easily), then I don't see how we can avoid having two separate versions of DualSerialPortLib indeed, and I see that as becoming more of a headache than what we're proposing here... > where the one > that PrePi uses may need to be attached to SerialDxe as well, so that it > can reprogram the baud rate as needed. But for all the remaining > DebugLib related uses, we don't really need to reprogram the UART at all > afaics. From a design standpoint, this may look fine, but from an implementation standpoint, when, again, the one problem we are really trying to solve is the sharing of a global variable, I fear that we are going to shoot ourselves in the foot if we try to go in that direction, because duplication of code, when it can be avoided, doesn't strike me like a good idea. If we are going to push for something like that, I'd much rather work on another EDK2 library that solves the problem of early global variable sharing between DXE and PEI, than something that's custom to RPi and that's not going to help anybody else in the process. However, considering the time that has already spent trying to solve this issue, I'd rather not have to do either of these, really, because I don't think there is much to gain from adding bells and whistle to a problem that really boils down to "We need to share a global variable, that we set in early PEI, between PEI and DXE" and which we have solved using what I believe to be the mandated EDK2 way of doing it (HOBs). Now, if you are *really* that opposed to the current solution, I can see what's achievable. But I'd rather only have to do that on a major objection ("This proposal may create problems in the long run because X, Y...") rather than preference of how things may look be better organized if we did it in this other fashion... Besides the need to add an extra PlatformPeiLib and the small hack we had to use to get to the Hob in early DXE phase, do you have specific concerns about the current proposal that you identify as reasonable ground to want to push it back? Regards, /Pete ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation 2020-05-05 11:54 ` Pete Batard @ 2020-05-05 12:05 ` Ard Biesheuvel 2020-05-05 12:09 ` Pete Batard 0 siblings, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2020-05-05 12:05 UTC (permalink / raw) To: Pete Batard, devel; +Cc: leif, awarkentin, Andrei Warkentin On 5/5/20 1:54 PM, Pete Batard wrote: > Hi Ard, > > On 2020.05.05 11:05, Ard Biesheuvel wrote: >> On 5/4/20 1:15 PM, Pete Batard wrote: >>> From: Andrei Warkentin <andrey.warkentin@gmail.com> >>> >>> Fix for https://github.com/raspberrypi/firmware/issues/1376. >>> >>> For the Pi 3, to properly configure miniUART, we need the core clock, >>> which can be vary between VideoCore firmare release or config.txt >>> options. >>> >>> Unfortunately, it's painful to get - it's only available via the mailbox >>> interface. Additionally, SerialLib is a very limited environment, even >>> when linked in a DXE-mode component, because as part of a DebugLib >>> implementation it ends being the base prerequisite of everything. >>> That means a "normal" mailbox implementation like the one from >>> RpiFirmwareDxe (with dependencies on DmaLib) is out of the question. >>> Using a basic implementation such as the one in PlatformLib doesn't work >>> either because it operates in both environments with MMU on (DXE phase) >>> and MMU off (SEC/PrePi). >>> >>> Ideally, we read the value via mbox exactly once at boot (PlatformLib), >>> and then somehow stash it. A GUID Hob sounds appropriate, yet when >>> SerialPortLib operates in DXE components, we can't use the HobLib to >>> *locate* the Hob list itself (remember, SerialPortLib initializes >>> before any of HobLib's dependencies, like UeflLib...). >>> >> >> Yeah, the gift that keeps on giving :-) NXP are struggling with a >> similar issue. >> >> So the problem is that SerialPortInitialize() is called before we know >> what value to program, and we cannot rely on other libraries to >> discover this value because they may not work before their constructor >> has been called. > > I wouldn't qualify the problem that way. > > The problem is that we have everything we need to program the UART by > the time we get into SerialPortInitialize(), because > ArmPlatformPeiBootAction() has long retrieved that one variable/value we > need, which everything downstream relies on, but sharing that one > variable properly, so that it available in both PEI and DXE by the time > it is needed, is a headache. > > For all intent and purposes, we always have the value we need, because > that's pretty much the very first thing we query. On the other hand, > directing that value to the places that consume it is problematic > because most of the mechanisms we have to do that have some kind of > reliance that serial output would already have been initialized... > > So we have been treating it as mere problem of sharing a global variable > between PEI and DXE and nothing else, hence the proposal. As a matter of > fact, we started with an alternate solution where we just added an extra > page on top of the NvStorage reserved region, and stored the variable > there at an address that DualSerialPortLib could easily retrieve in > either PEI or DXE mode (with no need for the extra PlatformPeiLib then). > Hob is just the natural progression of that, to achieve it in a more > EDK2-like way. > >> So can we break the contents of SerialPortInitialize() into things >> that need to happen once (program the divisors etc) and things that >> need to > happen each and every time (figure out which UART we are >> using in the >> first place)? > > I'm trying to understand what you're getting at here. > > By once and multiple time, I am assuming that you're referring to the > various instantiations of the library, because I only expect > SerialPortInitialize() to be called once per instance for DebugLib > related uses, so technically, everything from SerialPortInitialize() > could be considered a one-time operation. > > So I guess what you're suggesting is that we somehow drop setting up the > baudrate in DXE phase and just assume that it has already been set from > PEI. > >> If the second set does not suffer from the same issue, can we just >> move the entire logic that programs the UART block into PrePi, so that >> all subsequent users don't have to touch those registers at all? > > Well, someone can and will want to switch baudrate from Shell, which > means we need to compute a dynamic divisor from the base serial clock > frequency. So the only way I can see your idea work is if we re-query > the mbox to obtain the base serial clock frequency in > SerialPortSetAttributes(), but that means that, for the compiler to be > happy, it will need to set up a DXE dependency with RpiFirmwareDxe, > which we can't have in PEI phase... > >> This means we may need two versions of DualSerialPortLib, > > Unless there exists a compile time macros that indicate whether code is > compiling in PEI or DXE phase (and even then I suspect that .inf > sections will not cooperate that easily), then I don't see how we can > avoid having two separate versions of DualSerialPortLib indeed, and I > see that as becoming more of a headache than what we're proposing here... > >> where the one that PrePi uses may need to be attached to SerialDxe as >> well, so that it can reprogram the baud rate as needed. But for all >> the remaining DebugLib related uses, we don't really need to reprogram >> the UART at all afaics. > > From a design standpoint, this may look fine, but from an > implementation standpoint, when, again, the one problem we are really > trying to solve is the sharing of a global variable, I fear that we are > going to shoot ourselves in the foot if we try to go in that direction, > because duplication of code, when it can be avoided, doesn't strike me > like a good idea. > > If we are going to push for something like that, I'd much rather work on > another EDK2 library that solves the problem of early global variable > sharing between DXE and PEI, than something that's custom to RPi and > that's not going to help anybody else in the process. > > However, considering the time that has already spent trying to solve > this issue, I'd rather not have to do either of these, really, because I > don't think there is much to gain from adding bells and whistle to a > problem that really boils down to "We need to share a global variable, > that we set in early PEI, between PEI and DXE" and which we have solved > using what I believe to be the mandated EDK2 way of doing it (HOBs). > > Now, if you are *really* that opposed to the current solution, I can see > what's achievable. But I'd rather only have to do that on a major > objection ("This proposal may create problems in the long run because X, > Y...") rather than preference of how things may look be better organized > if we did it in this other fashion... > > Besides the need to add an extra PlatformPeiLib and the small hack we > had to use to get to the Hob in early DXE phase, do you have specific > concerns about the current proposal that you identify as reasonable > ground to want to push it back? > I'll have a stab at coding it up myself. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation 2020-05-05 12:05 ` Ard Biesheuvel @ 2020-05-05 12:09 ` Pete Batard 0 siblings, 0 replies; 11+ messages in thread From: Pete Batard @ 2020-05-05 12:09 UTC (permalink / raw) To: Ard Biesheuvel, devel; +Cc: leif, awarkentin, Andrei Warkentin On 2020.05.05 13:05, Ard Biesheuvel wrote: > On 5/5/20 1:54 PM, Pete Batard wrote: >> Hi Ard, >> >> On 2020.05.05 11:05, Ard Biesheuvel wrote: >>> On 5/4/20 1:15 PM, Pete Batard wrote: >>>> From: Andrei Warkentin <andrey.warkentin@gmail.com> >>>> >>>> Fix for https://github.com/raspberrypi/firmware/issues/1376. >>>> >>>> For the Pi 3, to properly configure miniUART, we need the core clock, >>>> which can be vary between VideoCore firmare release or config.txt >>>> options. >>>> >>>> Unfortunately, it's painful to get - it's only available via the >>>> mailbox >>>> interface. Additionally, SerialLib is a very limited environment, even >>>> when linked in a DXE-mode component, because as part of a DebugLib >>>> implementation it ends being the base prerequisite of everything. >>>> That means a "normal" mailbox implementation like the one from >>>> RpiFirmwareDxe (with dependencies on DmaLib) is out of the question. >>>> Using a basic implementation such as the one in PlatformLib doesn't >>>> work >>>> either because it operates in both environments with MMU on (DXE phase) >>>> and MMU off (SEC/PrePi). >>>> >>>> Ideally, we read the value via mbox exactly once at boot (PlatformLib), >>>> and then somehow stash it. A GUID Hob sounds appropriate, yet when >>>> SerialPortLib operates in DXE components, we can't use the HobLib to >>>> *locate* the Hob list itself (remember, SerialPortLib initializes >>>> before any of HobLib's dependencies, like UeflLib...). >>>> >>> >>> Yeah, the gift that keeps on giving :-) NXP are struggling with a >>> similar issue. >>> >>> So the problem is that SerialPortInitialize() is called before we >>> know what value to program, and we cannot rely on other libraries to >>> discover this value because they may not work before their >>> constructor has been called. >> >> I wouldn't qualify the problem that way. >> >> The problem is that we have everything we need to program the UART by >> the time we get into SerialPortInitialize(), because >> ArmPlatformPeiBootAction() has long retrieved that one variable/value >> we need, which everything downstream relies on, but sharing that one >> variable properly, so that it available in both PEI and DXE by the >> time it is needed, is a headache. >> >> For all intent and purposes, we always have the value we need, because >> that's pretty much the very first thing we query. On the other hand, >> directing that value to the places that consume it is problematic >> because most of the mechanisms we have to do that have some kind of >> reliance that serial output would already have been initialized... >> >> So we have been treating it as mere problem of sharing a global >> variable between PEI and DXE and nothing else, hence the proposal. As >> a matter of fact, we started with an alternate solution where we just >> added an extra page on top of the NvStorage reserved region, and >> stored the variable there at an address that DualSerialPortLib could >> easily retrieve in either PEI or DXE mode (with no need for the extra >> PlatformPeiLib then). Hob is just the natural progression of that, to >> achieve it in a more EDK2-like way. >> >>> So can we break the contents of SerialPortInitialize() into things >>> that need to happen once (program the divisors etc) and things that >>> need to > happen each and every time (figure out which UART we are >>> using in the >>> first place)? >> >> I'm trying to understand what you're getting at here. >> >> By once and multiple time, I am assuming that you're referring to the >> various instantiations of the library, because I only expect >> SerialPortInitialize() to be called once per instance for DebugLib >> related uses, so technically, everything from SerialPortInitialize() >> could be considered a one-time operation. >> >> So I guess what you're suggesting is that we somehow drop setting up >> the baudrate in DXE phase and just assume that it has already been set >> from PEI. >> >>> If the second set does not suffer from the same issue, can we just >>> move the entire logic that programs the UART block into PrePi, so >>> that all subsequent users don't have to touch those registers at all? >> >> Well, someone can and will want to switch baudrate from Shell, which >> means we need to compute a dynamic divisor from the base serial clock >> frequency. So the only way I can see your idea work is if we re-query >> the mbox to obtain the base serial clock frequency in >> SerialPortSetAttributes(), but that means that, for the compiler to be >> happy, it will need to set up a DXE dependency with RpiFirmwareDxe, >> which we can't have in PEI phase... >> >>> This means we may need two versions of DualSerialPortLib, >> >> Unless there exists a compile time macros that indicate whether code >> is compiling in PEI or DXE phase (and even then I suspect that .inf >> sections will not cooperate that easily), then I don't see how we can >> avoid having two separate versions of DualSerialPortLib indeed, and I >> see that as becoming more of a headache than what we're proposing here... >> >>> where the one that PrePi uses may need to be attached to SerialDxe as >>> well, so that it can reprogram the baud rate as needed. But for all >>> the remaining DebugLib related uses, we don't really need to >>> reprogram the UART at all afaics. >> >> From a design standpoint, this may look fine, but from an >> implementation standpoint, when, again, the one problem we are really >> trying to solve is the sharing of a global variable, I fear that we >> are going to shoot ourselves in the foot if we try to go in that >> direction, because duplication of code, when it can be avoided, >> doesn't strike me like a good idea. >> >> If we are going to push for something like that, I'd much rather work >> on another EDK2 library that solves the problem of early global >> variable sharing between DXE and PEI, than something that's custom to >> RPi and that's not going to help anybody else in the process. >> >> However, considering the time that has already spent trying to solve >> this issue, I'd rather not have to do either of these, really, because >> I don't think there is much to gain from adding bells and whistle to a >> problem that really boils down to "We need to share a global variable, >> that we set in early PEI, between PEI and DXE" and which we have >> solved using what I believe to be the mandated EDK2 way of doing it >> (HOBs). >> >> Now, if you are *really* that opposed to the current solution, I can >> see what's achievable. But I'd rather only have to do that on a major >> objection ("This proposal may create problems in the long run because >> X, Y...") rather than preference of how things may look be better >> organized if we did it in this other fashion... >> >> Besides the need to add an extra PlatformPeiLib and the small hack we >> had to use to get to the Hob in early DXE phase, do you have specific >> concerns about the current proposal that you identify as reasonable >> ground to want to push it back? >> > > I'll have a stab at coding it up myself. > Obviously, that works for me. :) Thanks! /Pete ^ permalink raw reply [flat|nested] 11+ messages in thread
* [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init 2020-05-04 11:15 [edk2-platforms][PATCH 0/3] Platform/RPi: Fix compatibility with recent start.elf Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation Pete Batard @ 2020-05-04 11:15 ` Pete Batard 2020-05-04 11:37 ` [edk2-devel] " Philippe Mathieu-Daudé 2020-05-05 6:04 ` Andrei Warkentin 2 siblings, 2 replies; 11+ messages in thread From: Pete Batard @ 2020-05-04 11:15 UTC (permalink / raw) To: devel; +Cc: ard.biesheuvel, leif, awarkentin The previous commit ensures that gSerialLibCoreClockFreq contains the VPU core frequency by the time we execute ArmPlatformGetVirtualMemoryMap (). This value can prove useful for troubleshooting, so report it. Signed-off-by: Pete Batard <pete@akeo.ie> --- Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c index aae189ec8136..015d3dccc27c 100644 --- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c +++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c @@ -22,7 +22,12 @@ extern UINT64 mSystemMemoryEnd; UINT64 mVideoCoreBase; UINT64 mVideoCoreSize; UINT32 mBoardRevision; - +// +// gSerialLibCoreClockFreq, which resides in DualSerialLib is set +// to the VPU Core Clock frequency by ArmPlatformPeiBootAction (). +// We use it to report the core frequency during early boot. +// +extern UINT32 gSerialLibCoreClockFreq; // The total number of descriptors, including the final "end-of-table" descriptor. #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 11 @@ -66,6 +71,7 @@ ArmPlatformGetVirtualMemoryMap ( DEBUG ((DEBUG_INFO, "Board Rev: 0x%lX\n", mBoardRevision)); DEBUG ((DEBUG_INFO, "Base RAM : 0x%ll08X (Size 0x%ll08X)\n", mSystemMemoryBase, mSystemMemoryEnd + 1)); DEBUG ((DEBUG_INFO, "VideoCore: 0x%ll08X (Size 0x%ll08X)\n", mVideoCoreBase, mVideoCoreSize)); + DEBUG ((DEBUG_INFO, "Core Freq: %d MHz\n", gSerialLibCoreClockFreq / 1000000)); ASSERT (mSystemMemoryBase == 0); ASSERT (VirtualMemoryMap != NULL); -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init 2020-05-04 11:15 ` [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init Pete Batard @ 2020-05-04 11:37 ` Philippe Mathieu-Daudé 2020-05-05 6:04 ` Andrei Warkentin 1 sibling, 0 replies; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2020-05-04 11:37 UTC (permalink / raw) To: devel, pete; +Cc: ard.biesheuvel, leif, awarkentin On 5/4/20 1:15 PM, Pete Batard wrote: > The previous commit ensures that gSerialLibCoreClockFreq contains the VPU > core frequency by the time we execute ArmPlatformGetVirtualMemoryMap (). > > This value can prove useful for troubleshooting, so report it. > > Signed-off-by: Pete Batard <pete@akeo.ie> > --- > Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c > index aae189ec8136..015d3dccc27c 100644 > --- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c > +++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c > @@ -22,7 +22,12 @@ extern UINT64 mSystemMemoryEnd; > UINT64 mVideoCoreBase; > UINT64 mVideoCoreSize; > UINT32 mBoardRevision; > - > +// > +// gSerialLibCoreClockFreq, which resides in DualSerialLib is set > +// to the VPU Core Clock frequency by ArmPlatformPeiBootAction (). > +// We use it to report the core frequency during early boot. > +// > +extern UINT32 gSerialLibCoreClockFreq; > > // The total number of descriptors, including the final "end-of-table" descriptor. > #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 11 > @@ -66,6 +71,7 @@ ArmPlatformGetVirtualMemoryMap ( > DEBUG ((DEBUG_INFO, "Board Rev: 0x%lX\n", mBoardRevision)); > DEBUG ((DEBUG_INFO, "Base RAM : 0x%ll08X (Size 0x%ll08X)\n", mSystemMemoryBase, mSystemMemoryEnd + 1)); > DEBUG ((DEBUG_INFO, "VideoCore: 0x%ll08X (Size 0x%ll08X)\n", mVideoCoreBase, mVideoCoreSize)); > + DEBUG ((DEBUG_INFO, "Core Freq: %d MHz\n", gSerialLibCoreClockFreq / 1000000)); > > ASSERT (mSystemMemoryBase == 0); > ASSERT (VirtualMemoryMap != NULL); > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init 2020-05-04 11:15 ` [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init Pete Batard 2020-05-04 11:37 ` [edk2-devel] " Philippe Mathieu-Daudé @ 2020-05-05 6:04 ` Andrei Warkentin 1 sibling, 0 replies; 11+ messages in thread From: Andrei Warkentin @ 2020-05-05 6:04 UTC (permalink / raw) To: Pete Batard, devel@edk2.groups.io Cc: ard.biesheuvel@arm.com, leif@nuviainc.com [-- Attachment #1: Type: text/plain, Size: 2170 bytes --] Reviewed-by: Andrei Warkentin <andrey.warkentin@gmail.com> A ________________________________ From: Pete Batard <pete@akeo.ie> Sent: Monday, May 4, 2020 6:15 AM To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; Andrei Warkentin <awarkentin@vmware.com> Subject: [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init The previous commit ensures that gSerialLibCoreClockFreq contains the VPU core frequency by the time we execute ArmPlatformGetVirtualMemoryMap (). This value can prove useful for troubleshooting, so report it. Signed-off-by: Pete Batard <pete@akeo.ie> --- Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c index aae189ec8136..015d3dccc27c 100644 --- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c +++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c @@ -22,7 +22,12 @@ extern UINT64 mSystemMemoryEnd; UINT64 mVideoCoreBase; UINT64 mVideoCoreSize; UINT32 mBoardRevision; - +// +// gSerialLibCoreClockFreq, which resides in DualSerialLib is set +// to the VPU Core Clock frequency by ArmPlatformPeiBootAction (). +// We use it to report the core frequency during early boot. +// +extern UINT32 gSerialLibCoreClockFreq; // The total number of descriptors, including the final "end-of-table" descriptor. #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 11 @@ -66,6 +71,7 @@ ArmPlatformGetVirtualMemoryMap ( DEBUG ((DEBUG_INFO, "Board Rev: 0x%lX\n", mBoardRevision)); DEBUG ((DEBUG_INFO, "Base RAM : 0x%ll08X (Size 0x%ll08X)\n", mSystemMemoryBase, mSystemMemoryEnd + 1)); DEBUG ((DEBUG_INFO, "VideoCore: 0x%ll08X (Size 0x%ll08X)\n", mVideoCoreBase, mVideoCoreSize)); + DEBUG ((DEBUG_INFO, "Core Freq: %d MHz\n", gSerialLibCoreClockFreq / 1000000)); ASSERT (mSystemMemoryBase == 0); ASSERT (VirtualMemoryMap != NULL); -- 2.21.0.windows.1 [-- Attachment #2: Type: text/html, Size: 4464 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-06 12:39 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-04 11:15 [edk2-platforms][PATCH 0/3] Platform/RPi: Fix compatibility with recent start.elf Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code Pete Batard 2020-05-06 12:39 ` Ard Biesheuvel 2020-05-04 11:15 ` [edk2-platforms][PATCH 2/3] Platform/RPi/DualSerialPortLib: Fix miniUART serial divisor computation Pete Batard 2020-05-05 10:05 ` Ard Biesheuvel 2020-05-05 11:54 ` Pete Batard 2020-05-05 12:05 ` Ard Biesheuvel 2020-05-05 12:09 ` Pete Batard 2020-05-04 11:15 ` [edk2-platforms][PATCH 3/3] Platform/RPi: Report core clock frequency during early init Pete Batard 2020-05-04 11:37 ` [edk2-devel] " Philippe Mathieu-Daudé 2020-05-05 6:04 ` Andrei Warkentin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox