From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Pete Batard <pete@akeo.ie>, devel@edk2.groups.io
Cc: leif@nuviainc.com, awarkentin@vmware.com,
Andrei Warkentin <andrey.warkentin@gmail.com>
Subject: Re: [edk2-platforms][PATCH 1/3] Platform/RPi: Fortify mailbox code
Date: Wed, 6 May 2020 14:39:53 +0200 [thread overview]
Message-ID: <bf8ce89b-31b7-0517-5915-f4f84f91c153@arm.com> (raw)
In-Reply-To: <20200504111548.11112-2-pete@akeo.ie>
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
>
next prev parent reply other threads:[~2020-05-06 12:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf8ce89b-31b7-0517-5915-f4f84f91c153@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox