From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.5266.1588768798260555560 for ; Wed, 06 May 2020 05:39:58 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DE2621FB; Wed, 6 May 2020 05:39:57 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9EB8F3F71F; Wed, 6 May 2020 05:39:56 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code To: Pete Batard , devel@edk2.groups.io Cc: leif@nuviainc.com, awarkentin@vmware.com, Andrei Warkentin References: <20200504111548.11112-1-pete@akeo.ie> <20200504111548.11112-2-pete@akeo.ie> From: "Ard Biesheuvel" Message-ID: Date: Wed, 6 May 2020 14:39:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200504111548.11112-2-pete@akeo.ie> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/4/20 1:15 PM, Pete Batard wrote: > From: Andrei Warkentin > > 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 This patch Reviewed-by: Ard Biesheuvel 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 > * 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 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 > + * 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 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 > + * 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,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 >