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.web11.33581.1591006252059763870 for ; Mon, 01 Jun 2020 03:10:52 -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 CD8671FB; Mon, 1 Jun 2020 03:10:50 -0700 (PDT) Received: from [192.168.1.69] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D38673F305; Mon, 1 Jun 2020 03:10:49 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH 1/1] Pi4: notify VPU to load xHCI firmware before XhciDxe binds To: Andrei Warkentin , devel@edk2.groups.io Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com References: <20200601094258.116519-1-andrey.warkentin@gmail.com> From: "Ard Biesheuvel" Message-ID: <92c3092f-41b6-a903-4c63-15cf359c7f67@arm.com> Date: Mon, 1 Jun 2020 12:10:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 MIME-Version: 1.0 In-Reply-To: <20200601094258.116519-1-andrey.warkentin@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 6/1/20 11:42 AM, Andrei Warkentin wrote: > v2 with Ard's feedback. > Please put revision changelogs below the --- so I don't have to remove them by hand when applying the patch. > 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 Reviewed-by: Ard Biesheuvel Pushed as 01005f9aa73d..dae68d51c5df > --- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 8 +- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h | 19 ++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 5 +- > Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c | 99 ++++++++++++++++++++ > Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 59 +++++++++++- > Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h | 3 + > Platform/RaspberryPi/Include/Protocol/RpiFirmware.h | 10 ++ > 7 files changed, 200 insertions(+), 3 deletions(-) > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index 1257ac8f..abfe440c 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 > + * Copyright (c) 2018 - 2020, Andrei Warkentin > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > @@ -26,6 +26,7 @@ > #include > #include > #include "ConfigDxeFormSetGuid.h" > +#include "ConfigDxe.h" > > #define FREQ_1_MHZ 1000000 > > @@ -598,5 +599,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..6f0466f8 > --- /dev/null > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.h > @@ -0,0 +1,19 @@ > +/** @file > + * > + * Copyright (c) 2020, Andrei Warkentin > + * > + * SPDX-License-Identifier: BSD-2-Clause-Patent > + * > + **/ > + > +#ifndef _CONFIG_DXE_H_ > +#define _CONFIG_DXE_H_ > + > +#include > +#include > + > +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..5a9819b5 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 > +# Copyright (c) 2018 - 2020, Andrei Warkentin > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -28,6 +28,7 @@ > ConfigDxeFormSetGuid.h > ConfigDxeHii.vfr > ConfigDxeHii.uni > + XhciQuirk.c > > [Packages] > ArmPkg/ArmPkg.dec > @@ -52,6 +53,7 @@ > PcdLib > UefiBootServicesTableLib > UefiDriverEntryPoint > + UefiLib > UefiRuntimeServicesTableLib > > [Guids] > @@ -60,6 +62,7 @@ > > [Protocols] > gBcmGenetPlatformDeviceProtocolGuid ## PRODUCES > + gEfiPciIoProtocolGuid ## CONSUMES > gRaspberryPiFirmwareProtocolGuid ## CONSUMES > gRaspberryPiConfigAppliedProtocolGuid ## PRODUCES > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c b/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c > new file mode 100644 > index 00000000..5753a1b9 > --- /dev/null > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/XhciQuirk.c > @@ -0,0 +1,99 @@ > +/** @file > + * > + * Copyright (c) 2020, Andrei Warkentin > + * > + * SPDX-License-Identifier: BSD-2-Clause-Patent > + * > + **/ > + > +#include "ConfigDxe.h" > +#include > +#include > +#include > +#include > +#include > + > +#pragma pack(1) > +typedef struct { > + UINT8 ProgInterface; > + UINT8 SubClassCode; > + UINT8 BaseCode; > +} USB_CLASSC; > +#pragma pack() > + > +STATIC VOID *mPciIoNotificationRegistration = NULL; > + > +STATIC > +VOID > +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); > + if (EFI_ERROR (Status)) { > + return; > + } > + > + Status = PciIo->Pci.Read (PciIo, > + EfiPciIoWidthUint8, > + PCI_CLASSCODE_OFFSET, > + sizeof (USB_CLASSC) / sizeof (UINT8), > + &UsbClassCReg > + ); > + > + 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)); > + return; > + } > + > + 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..8cd8db7b 100644 > --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c > +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c > @@ -2,7 +2,7 @@ > * > * Copyright (c) 2020, Pete Batard > * Copyright (c) 2019, ARM Limited. All rights reserved. > - * Copyright (c) 2017-2018, Andrei Warkentin > + * Copyright (c) 2017-2020, Andrei Warkentin > * Copyright (c) 2016, Linaro, Ltd. All rights reserved. > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -1237,6 +1237,62 @@ 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); > + > + 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 +1315,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 > + * Copyright (c) 2017-2020, Andrei Warkentin > * 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 > * 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; >