From 35a5402a718f6547749cc2b6764bfcd8ed46a032 Mon Sep 17 00:00:00 2001 From: Andrei Warkentin Date: Mon, 4 May 2020 12:15:46 +0100 Subject: [PATCH] Platform/RaspberryPi: Fortify mailbox code 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 Co-authored-by: Andrei Warkentin Signed-off-by: Pete Batard Reviewed-by: Ard Biesheuvel --- .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +--- .../Include/IndustryStandard/RpiMbox.h | 11 +++- .../PlatformLib/AArch64/RaspberryPiHelper.S | 56 ++++++++----------- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c index 40c78b5d..6c45cf47 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 * Copyright (c) 2019, ARM Limited. All rights reserved. * Copyright (c) 2017-2018, Andrei Warkentin * 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 584786a6..3328be58 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 + * Copyright (c) 2019-2020, Pete Batard * 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 cc58406e..91dfe1bb 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 + * Copyright (c) 2020, Andrei Warkentin + * Copyright (c) 2019-2020, Pete Batard * Copyright (c) 2016, Linaro Limited. All rights reserved. * Copyright (c) 2011-2013, ARM Limited. All rights reserved. * @@ -13,10 +14,8 @@ #include #include -#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,16 +35,11 @@ 1: .endm -ASM_FUNC (ArmPlatformPeiBootAction) - adr x0, .Lmeminfo_buffer - mov x1, #FixedPcdGet64 (PcdDmaDeviceOffset) + .macro run, command_buffer + adr x0, \command_buffer 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 @@ -53,6 +47,18 @@ ASM_FUNC (ArmPlatformPeiBootAction) dmb sy ldr wzr, [x4, #BCM2836_MBOX_READ_OFFSET] dmb ld + .endm + +ASM_FUNC (ArmPlatformPeiBootAction) + mov x1, #FixedPcdGet64 (PcdDmaDeviceOffset) + orr x0, x0, #RPI_MBOX_VC_CHANNEL + // x1 holds the value of PcdDmaDeviceOffset throughout this function + + MOV32 (x4, BCM2836_MBOX_BASE_ADDRESS) + + drain + + 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