From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Andrei Warkentin <andrey.warkentin@gmail.com>, devel@edk2.groups.io
Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com
Subject: Re: [edk2-platforms][PATCH 1/1] Pi4: notify VPU to load xHCI firmware before XhciDxe binds
Date: Mon, 1 Jun 2020 08:59:53 +0200 [thread overview]
Message-ID: <3121e7a6-afb7-038e-4fcd-5a317d3194e9@arm.com> (raw)
In-Reply-To: <20200601032130.95634-1-andrey.warkentin@gmail.com>
Hi Andrei,
On 6/1/20 5:21 AM, Andrei Warkentin wrote:
> Newer Pi 4 boards (such as 8GiB variant) no longer carry a SPI
> EEPROM with the VLI805 (USB) controller firmware. So, ask the
> VPU firmare to load the image it has into the controller
> specified by BDF.
>
> This is benign on non-8GiB boards (reloading that firmware if
> VPU fw is new enough or doing nothing on old VPU firmware).
>
> Tested on 4GB. Also have a positive test report for an 8GB board
> from a forum member.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
> ---
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 12 ++-
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h | 22 +++++
> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 5 +-
> Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c | 97 ++++++++++++++++++++
> Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 58 +++++++++++-
> Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h | 3 +
> Platform/RaspberryPi/Include/Protocol/RpiFirmware.h | 10 ++
> 7 files changed, 200 insertions(+), 7 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index ac1004fe..ad14eb3d 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -1,7 +1,7 @@
> /** @file
> *
> * Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> - * Copyright (c) 2018 - 2019, Andrei Warkentin <andrey.warkentin@gmail.com>
> + * Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> *
> @@ -11,21 +11,18 @@
> #include <IndustryStandard/Bcm2711.h>
> #include <IndustryStandard/Bcm2836.h>
> #include <IndustryStandard/Bcm2836Gpio.h>
> -#include <IndustryStandard/RpiMbox.h>
> #include <Library/AcpiLib.h>
> -#include <Library/DebugLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/DxeServicesTableLib.h>
> #include <Library/GpioLib.h>
> #include <Library/HiiLib.h>
> #include <Library/IoLib.h>
> #include <Library/NetLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiRuntimeServicesTableLib.h>
> #include <Protocol/BcmGenetPlatformDevice.h>
> -#include <Protocol/RpiFirmware.h>
> #include <ConfigVars.h>
> #include "ConfigDxeFormSetGuid.h"
> +#include "ConfigDxe.h"
>
> #define FREQ_1_MHZ 1000000
>
> @@ -584,5 +581,10 @@ ConfigInitialize (
> NULL, &gEfiEndOfDxeEventGroupGuid, &EndOfDxeEvent);
> ASSERT_EFI_ERROR (Status);
>
> +
> + if (mModelFamily == 4) {
> + RegisterXhciQuirkHandler (mFwProtocol);
> + }
> +
> return EFI_SUCCESS;
> }
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h
> new file mode 100644
> index 00000000..6e35fd95
> --- /dev/null
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h
> @@ -0,0 +1,22 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#ifndef _CONFIG_DXE_H_
> +#define _CONFIG_DXE_H_
> +
> +#include <Uefi.h>
> +#include <Protocol/RpiFirmware.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <IndustryStandard/RpiMbox.h>
> +
Please include only the headers you need to declare the prototypes below.
> +VOID RegisterXhciQuirkHandler (
> + IN RASPBERRY_PI_FIRMWARE_PROTOCOL *FwProtocol
> + );
> +
> +#endif /* _CONFIG_DXE_H_ */
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index f20f3bcc..b3ef2624 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -3,7 +3,7 @@
> # Component description file for the RasbperryPi DXE platform config driver.
> #
> # Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> -# Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> +# Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -25,6 +25,7 @@
> #
> [Sources]
> ConfigDxe.c
> + XhciQuirk.c
Keep this ordered
> ConfigDxeFormSetGuid.h
> ConfigDxeHii.vfr
> ConfigDxeHii.uni
> @@ -50,6 +51,7 @@
> HiiLib
> NetLib
> PcdLib
> + UefiLib
Keep ordered
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> UefiRuntimeServicesTableLib
> @@ -62,6 +64,7 @@
> gBcmGenetPlatformDeviceProtocolGuid ## PRODUCES
> gRaspberryPiFirmwareProtocolGuid ## CONSUMES
> gRaspberryPiConfigAppliedProtocolGuid ## PRODUCES
> + gEfiPciIoProtocolGuid ## CONSUMES
>
Keep ordered
> [FixedPcd]
> gRaspberryPiTokenSpaceGuid.PcdCpuLowSpeedMHz
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c b/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c
> new file mode 100644
> index 00000000..93ab5a35
> --- /dev/null
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c
> @@ -0,0 +1,97 @@
> +/** @file
> + *
> + * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include "ConfigDxe.h"
> +#include <Protocol/PciIo.h>
> +#include <Library/UefiLib.h>
> +#include <IndustryStandard/Pci.h>
> +
> +#pragma pack(1)
> +typedef struct {
> + UINT8 ProgInterface;
> + UINT8 SubClassCode;
> + UINT8 BaseCode;
> +} USB_CLASSC;
> +#pragma pack()
> +
> +STATIC VOID *mPciIoNotificationRegistration = NULL;
> +
> +STATIC
> +VOID
This should have EFIAPI
> +PciIoNotificationEvent (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PCI_IO_PROTOCOL *PciIo;
> + USB_CLASSC UsbClassCReg;
> + UINTN SegmentNumber;
> + UINTN BusNumber;
> + UINTN DeviceNumber;
> + UINTN FunctionNumber;
> + RASPBERRY_PI_FIRMWARE_PROTOCOL *FwProtocol = Context;
> +
> + Status = gBS->LocateProtocol (&gEfiPciIoProtocolGuid,
> + mPciIoNotificationRegistration, (VOID **)&PciIo);
Indentation
> + if (EFI_ERROR (Status)) {
> + return;
> + }
> +
> + Status = PciIo->Pci.Read (
> + PciIo,
> + EfiPciIoWidthUint8,
> + PCI_CLASSCODE_OFFSET,
> + sizeof (USB_CLASSC) / sizeof (UINT8),
> + &UsbClassCReg
> + );
> +
Indentation
> + if (EFI_ERROR (Status)) {
> + return;
> + }
> +
> + //
> + // Test whether the controller belongs to Xhci type
> + //
> + if ((UsbClassCReg.BaseCode != PCI_CLASS_SERIAL) ||
> + (UsbClassCReg.SubClassCode != PCI_CLASS_SERIAL_USB) ||
> + (UsbClassCReg.ProgInterface != PCI_IF_XHCI)) {
> + return;
> + }
> +
> + Status = PciIo->GetLocation (PciIo, &SegmentNumber, &BusNumber,
> + &DeviceNumber, &FunctionNumber);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "%a: failed to get SBDF for xHCI controller: %r\n",
> + __FUNCTION__, Status));
Need to return here.
> + }
> +
> + DEBUG ((DEBUG_INFO, "xHCI found at %u:%u:%u:%u\n",
> + SegmentNumber, BusNumber, DeviceNumber, FunctionNumber));
> +
> + ASSERT (SegmentNumber == 0);
> + Status = FwProtocol->NotifyXhciReset(BusNumber, DeviceNumber, FunctionNumber);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "%a: couldn't signal xHCI firmware load: %r\n",
> + __FUNCTION__, Status));
> + }
> +}
> +
> +VOID
> +RegisterXhciQuirkHandler (
> + IN RASPBERRY_PI_FIRMWARE_PROTOCOL *FwProtocol
> + )
> +{
> + EfiCreateProtocolNotifyEvent (
> + &gEfiPciIoProtocolGuid,
> + TPL_NOTIFY,
> + PciIoNotificationEvent,
> + FwProtocol,
> + &mPciIoNotificationRegistration
> + );
> +}
> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index 6c45cf47..22f75a52 100644
> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -2,7 +2,7 @@
> *
> * 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) 2017-2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> * Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -1237,6 +1237,61 @@ RpiFirmwareSetLed (
> }
> }
>
> +#pragma pack()
> +typedef struct {
> + UINT32 DeviceAddress;
> +} RPI_FW_NOTIFY_XHCI_RESET_TAG;
> +
> +typedef struct {
> + RPI_FW_BUFFER_HEAD BufferHead;
> + RPI_FW_TAG_HEAD TagHead;
> + RPI_FW_NOTIFY_XHCI_RESET_TAG TagBody;
> + UINT32 EndTag;
> +} RPI_FW_NOTIFY_XHCI_RESET_CMD;
> +#pragma pack()
> +
> +STATIC
> +EFI_STATUS
EFIAPI
> +RpiFirmwareNotifyXhciReset (
> + IN UINTN BusNumber,
> + IN UINTN DeviceNumber,
> + IN UINTN FunctionNumber
> + )
> +{
> + RPI_FW_NOTIFY_XHCI_RESET_CMD *Cmd;
> + EFI_STATUS Status;
> + UINT32 Result;
> +
> + if (!AcquireSpinLockOrFail (&mMailboxLock)) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
> + return EFI_DEVICE_ERROR;
> + }
> +
> + Cmd = mDmaBuffer;
> + ZeroMem (Cmd, sizeof (*Cmd));
> +
> + Cmd->BufferHead.BufferSize = sizeof (*Cmd);
> + Cmd->BufferHead.Response = 0;
> + Cmd->TagHead.TagId = RPI_MBOX_NOTIFY_XHCI_RESET;
> + Cmd->TagHead.TagSize = sizeof (Cmd->TagBody);
> + Cmd->TagHead.TagValueSize = 0;
> + Cmd->TagBody.DeviceAddress = BusNumber << 20 | DeviceNumber << 15 | FunctionNumber << 12;
> + Cmd->EndTag = 0;
> +
> + Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);
> +
So this pokes the XHCI controller via its config space? Is it safe to do
that without raising the TPL? What happens if another config space
access occurs concurrently?
> + ReleaseSpinLock (&mMailboxLock);
> +
> + if (EFI_ERROR (Status) ||
> + Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
> + DEBUG ((DEBUG_ERROR,
> + "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
> + __FUNCTION__, Status, Cmd->BufferHead.Response));
> + }
> +
> + return Status;
> +}
> +
> STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
> RpiFirmwareSetPowerState,
> RpiFirmwareGetMacAddress,
> @@ -1259,6 +1314,7 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
> RpiFirmwareGetCpuName,
> RpiFirmwareGetArmMemory,
> RPiFirmwareGetModelInstalledMB,
> + RpiFirmwareNotifyXhciReset
> };
>
> /**
> diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
> index 3328be58..71040689 100644
> --- a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
> +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
> @@ -1,6 +1,7 @@
> /** @file
> *
> * Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> * Copyright (c) 2016, Linaro Limited. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -61,6 +62,8 @@
> #define RPI_MBOX_GET_MAX_CLOCK_RATE 0x00030004
> #define RPI_MBOX_GET_MIN_CLOCK_RATE 0x00030007
>
> +#define RPI_MBOX_NOTIFY_XHCI_RESET 0x00030058
> +
> #define RPI_MBOX_SET_CLOCK_RATE 0x00038002
> #define RPI_MBOX_SET_GPIO 0x00038041
>
> diff --git a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
> index 108becbd..56a8d15a 100644
> --- a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
> +++ b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
> @@ -1,6 +1,7 @@
> /** @file
> *
> * Copyright (c) 2019, ARM Limited. All rights reserved.
> + * Copyright (c) 2017 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> * Copyright (c) 2016, Linaro Limited. All rights reserved.
> *
> * SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -140,6 +141,14 @@ EFI_STATUS
> UINT32 *Size
> );
>
> +typedef
> +EFI_STATUS
> +(EFIAPI *NOTIFY_XHCI_RESET) (
> + UINTN BusNumber,
> + UINTN DeviceNumber,
> + UINTN FunctionNumber
> + );
> +
> typedef struct {
> SET_POWER_STATE SetPowerState;
> GET_MAC_ADDRESS GetMacAddress;
> @@ -162,6 +171,7 @@ typedef struct {
> GET_CPU_NAME GetCpuName;
> GET_ARM_MEM GetArmMem;
> GET_MODEL_INSTALLED_MB GetModelInstalledMB;
> + NOTIFY_XHCI_RESET NotifyXhciReset;
> } RASPBERRY_PI_FIRMWARE_PROTOCOL;
>
> extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
>
next prev parent reply other threads:[~2020-06-01 6:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-01 3:21 [edk2-platforms][PATCH 1/1] Pi4: notify VPU to load xHCI firmware before XhciDxe binds Andrei Warkentin
2020-06-01 6:59 ` Ard Biesheuvel [this message]
2020-06-01 9:37 ` [edk2-devel] " Andrei Warkentin
2020-06-01 9:44 ` Ard Biesheuvel
2020-06-01 9:48 ` Andrei Warkentin
-- strict thread matches above, loose matches on Subject: below --
2020-06-01 9:42 Andrei Warkentin
2020-06-01 10:10 ` Ard Biesheuvel
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=3121e7a6-afb7-038e-4fcd-5a317d3194e9@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