From: "Pete Batard" <pete@akeo.ie>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
"Leif Lindholm" <leif.lindholm@linaro.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
samer.el-haj-mahmoud@arm.com,
"Andrei E. Warkentin" <andrey.warkentin@gmail.com>
Subject: Re: [edk2-platforms][PATCH 2/5] Silicon/Bcm283x: Add FIFO mode for RNG
Date: Wed, 27 Nov 2019 12:47:12 +0000 [thread overview]
Message-ID: <23ffe668-e6ca-ff63-c4ea-d3be306ce8d9@akeo.ie> (raw)
In-Reply-To: <CAKv+Gu8jH+nv98gb0hiU=9f0cYYwVpJOAiSA8ev70NHDF+oKxA@mail.gmail.com>
On 2019.11.27 12:44, Ard Biesheuvel wrote:
> On Wed, 27 Nov 2019 at 13:37, Pete Batard <pete@akeo.ie> wrote:
>>
>> The Bcm283x Random Number Generator does not work in regular mode
>> on the Bcm2711 powered Raspberry Pi 4. It does however work when
>> using FIFO mode. So we add this new mode, which is governed by the
>> use of the new PcdBcm283xRngUseFifo boolean PCD.
>>
>> Note that just as a Pi 4 doesn't seem to support regular mode, a
>> Pi 3 doesn't seem to support FIFO mode, which is why we can't
>> switch to simply using FIFO mode for both platforms.
>>
>> We also fix the register to which RNG_WARMUP_COUNT is written,
>> which was incorrect.
>>
>
> At the risk of triggering another endless debate, could we /please/
> not mix in trivial bugfixes with more complicated refactoring like
> this?
Well, the "trivial" is the precise reason it is mixed in, but sure, I
can split that in v2.
Regards,
/Pete
>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>> Silicon/Broadcom/Bcm283x/Bcm283x.dec | 1 +
>> Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.c | 96 +++++++++++++++-----
>> Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf | 2 +
>> 3 files changed, 74 insertions(+), 25 deletions(-)
>>
>> diff --git a/Silicon/Broadcom/Bcm283x/Bcm283x.dec b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
>> index 5b839b00d286..4a9be7b18c97 100644
>> --- a/Silicon/Broadcom/Bcm283x/Bcm283x.dec
>> +++ b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
>> @@ -21,3 +21,4 @@ [Guids]
>>
>> [PcdsFixedAtBuild.common]
>> gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x0|UINT32|0x00000001
>> + gBcm283xTokenSpaceGuid.PcdBcm283xRngUseFifo|0x0|BOOLEAN|0x00000002
>> diff --git a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.c b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.c
>> index 722815d32f06..462a21a6f3c3 100644
>> --- a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.c
>> +++ b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.c
>> @@ -2,6 +2,7 @@
>>
>> This driver produces an EFI_RNG_PROTOCOL instance for the Broadcom 2836 RNG
>>
>> + Copyright (C) 2019, Pete Batard <pete@akeo.ie>
>> Copyright (C) 2019, Linaro Ltd. All rights reserved.<BR>
>>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>> @@ -12,6 +13,8 @@
>> #include <Library/BaseMemoryLib.h>
>> #include <Library/DebugLib.h>
>> #include <Library/IoLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/TimerLib.h>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> #include <IndustryStandard/Bcm2836.h>
>> @@ -20,6 +23,8 @@
>>
>> #define RNG_WARMUP_COUNT 0x40000
>> #define RNG_MAX_RETRIES 0x100 // arbitrary upper bound
>> +#define RNG_FIFO_NUMVAL_MASK 0xff
>> +#define RNG_STATUS_NUMVAL_SHIFT 24
>>
>> /**
>> Returns information about the random number generation implementation.
>> @@ -84,6 +89,54 @@ Bcm2836RngGetInfo (
>> return EFI_SUCCESS;
>> }
>>
>> +/**
>> + Read a single random value, in either FIFO or regular mode.
>> +
>> + @param[in] Val A pointer to the 32-bit word that is to
>> + be filled with a random value.
>> +
>> + @retval EFI_SUCCESS A random value was successfully read.
>> + @retval EFI_NOT_READY The number of retries elapsed before a
>> + random value was generated.
>> +
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +Bcm2836RngReadValue (
>> + IN OUT UINT32 *Val
>> +)
>> +{
>> + UINT32 Avail;
>> + UINT32 i;
>> + BOOLEAN UseFifo = FixedPcdGetBool (PcdBcm283xRngUseFifo);
>> +
>> + ASSERT (Val != NULL);
>> +
>> + Avail = UseFifo ?
>> + (MmioRead32 (RNG_FIFO_COUNT) & RNG_FIFO_NUMVAL_MASK) :
>> + (MmioRead32 (RNG_STATUS) >> RNG_STATUS_NUMVAL_SHIFT);
>> +
>> + //
>> + // If we don't have a value ready, wait 1 us and retry.
>> + //
>> + for (i = 0; Avail < 1 && i < RNG_MAX_RETRIES; i++) {
>> + MicroSecondDelay (1);
>> + Avail = UseFifo ?
>> + (MmioRead32 (RNG_FIFO_COUNT) & RNG_FIFO_NUMVAL_MASK) :
>> + (MmioRead32 (RNG_STATUS) >> RNG_STATUS_NUMVAL_SHIFT);
>> + }
>> + if (Avail < 1) {
>> + return EFI_NOT_READY;
>> + }
>> +
>> + *Val = UseFifo ?
>> + MmioRead32 (RNG_FIFO_DATA):
>> + MmioRead32 (RNG_DATA);
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> /**
>> Produces and returns an RNG value using either the default or specified RNG
>> algorithm.
>> @@ -123,9 +176,8 @@ Bcm2836RngGetRNG (
>> OUT UINT8 *RNGValue
>> )
>> {
>> - UINT32 Val;
>> - UINT32 Num;
>> - UINT32 Retries;
>> + EFI_STATUS Status;
>> + UINT32 Val;
>>
>> if (This == NULL || RNGValueLength == 0 || RNGValue == NULL) {
>> return EFI_INVALID_PARAMETER;
>> @@ -139,30 +191,24 @@ Bcm2836RngGetRNG (
>> return EFI_UNSUPPORTED;
>> }
>>
>> - while (RNGValueLength > 0) {
>> - Retries = RNG_MAX_RETRIES;
>> - do {
>> - Num = MmioRead32 (RNG_STATUS) >> 24;
>> - MemoryFence ();
>> - } while (!Num && Retries-- > 0);
>> -
>> - if (!Num) {
>> - return EFI_DEVICE_ERROR;
>> + while (RNGValueLength >= sizeof (UINT32)) {
>> + Status = Bcm2836RngReadValue (&Val);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> }
>> + WriteUnaligned32 ((VOID *)RNGValue, Val);
>> + RNGValue += sizeof (UINT32);
>> + RNGValueLength -= sizeof (UINT32);
>> + }
>>
>> - while (RNGValueLength >= sizeof (UINT32) && Num > 0) {
>> - WriteUnaligned32 ((VOID *)RNGValue, MmioRead32 (RNG_DATA));
>> - RNGValue += sizeof (UINT32);
>> - RNGValueLength -= sizeof (UINT32);
>> - Num--;
>> + if (RNGValueLength > 0) {
>> + Status = Bcm2836RngReadValue (&Val);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> }
>> -
>> - if (RNGValueLength > 0 && Num > 0) {
>> - Val = MmioRead32 (RNG_DATA);
>> - while (RNGValueLength--) {
>> - *RNGValue++ = (UINT8)Val;
>> - Val >>= 8;
>> - }
>> + while (RNGValueLength--) {
>> + *RNGValue++ = (UINT8)Val;
>> + Val >>= 8;
>> }
>> }
>> return EFI_SUCCESS;
>> @@ -190,7 +236,7 @@ Bcm2836RngEntryPoint (
>> NULL);
>> ASSERT_EFI_ERROR (Status);
>>
>> - MmioWrite32 (RNG_STATUS, RNG_WARMUP_COUNT);
>> + MmioWrite32 (RNG_BIT_COUNT_THRESHOLD, RNG_WARMUP_COUNT);
>> MmioWrite32 (RNG_CTRL, RNG_CTRL_ENABLE);
>>
>> return EFI_SUCCESS;
>> diff --git a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
>> index 8eb90de85cfd..31415231ae0b 100644
>> --- a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
>> +++ b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
>> @@ -28,6 +28,7 @@ [LibraryClasses]
>> DebugLib
>> IoLib
>> PcdLib
>> + TimerLib
>> UefiBootServicesTableLib
>> UefiDriverEntryPoint
>>
>> @@ -39,6 +40,7 @@ [Guids]
>>
>> [FixedPcd]
>> gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
>> + gBcm283xTokenSpaceGuid.PcdBcm283xRngUseFifo
>>
>> [Depex]
>> TRUE
>> --
>> 2.21.0.windows.1
>>
next prev parent reply other threads:[~2019-11-27 12:47 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-27 12:37 [edk2-platforms][PATCH 0/5] Further RPi4 support groundwork Pete Batard
2019-11-27 12:37 ` [edk2-platforms][PATCH 1/5] Silicon/Bcm283x: Clean up Bcm2836.h header Pete Batard
2019-11-27 12:48 ` Ard Biesheuvel
2019-11-27 12:56 ` Pete Batard
2019-11-27 13:00 ` Ard Biesheuvel
2019-11-27 13:09 ` Pete Batard
2019-11-27 13:17 ` Ard Biesheuvel
2019-11-27 13:21 ` Pete Batard
2019-11-27 14:47 ` Philippe Mathieu-Daudé
2019-11-27 14:59 ` [edk2-devel] " Pete Batard
2019-11-27 15:18 ` Philippe Mathieu-Daudé
2019-11-27 12:37 ` [edk2-platforms][PATCH 2/5] Silicon/Bcm283x: Add FIFO mode for RNG Pete Batard
2019-11-27 12:44 ` Ard Biesheuvel
2019-11-27 12:47 ` Pete Batard [this message]
2019-11-27 12:37 ` [edk2-platforms][PATCH 3/5] Platform/RPi/MmcDxe: Factorize SCR call and clean up MMC init Pete Batard
2019-11-27 12:37 ` [edk2-platforms][PATCH 4/5] Platform/RPi/MmcDxe: Improve MMC driver stability Pete Batard
2019-11-27 12:37 ` [edk2-platforms][PATCH 5/5] Platform/RPi: Set SD routing according to model Pete Batard
2019-11-27 15:24 ` Philippe Mathieu-Daudé
2019-11-27 16:33 ` Pete Batard
2019-11-27 17:04 ` Leif Lindholm
2019-11-27 17:17 ` Philippe Mathieu-Daudé
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=23ffe668-e6ca-ff63-c4ea-d3be306ce8d9@akeo.ie \
--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