[edk2] [Patch] BaseTools: Fix the bug to display the single SKUID info
when defined SKUID_IDENTIFIER = DEFAULT|TEST in DSC [Defines] section, per spec it means current SKUID is single, the bug is build report print both DEFAULT and TEST info, it should only print TEST. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- BaseTools/Source/Python/Common/Misc.py | 4 1 file changed, 4 insertions(+) diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py index b13e35c..1461d00 100644 --- a/BaseTools/Source/Python/Common/Misc.py +++ b/BaseTools/Source/Python/Common/Misc.py @@ -2237,10 +2237,14 @@ class SkuClass(): self.AvailableSkuIds.update({'DEFAULT':0, 'COMMON':0}) if self.SkuIdSet: GlobalData.gSkuids = (self.SkuIdSet) if 'COMMON' in GlobalData.gSkuids: GlobalData.gSkuids.remove('COMMON') +if self.SkuUsageType == self.SINGLE: +if len(GlobalData.gSkuids) != 1: +if 'DEFAULT' in GlobalData.gSkuids: +GlobalData.gSkuids.remove('DEFAULT') if GlobalData.gSkuids: GlobalData.gSkuids.sort() def GetNextSkuId(self, skuname): if not self.__SkuInherit: -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
Ard, Both FixedAtBuild and PatchableInModule are safe at runtime. Should not limit to only FixedAtBuild. It is also possible to use Dynamic and DynamicEx from a runtime driver as long as the PCD values are retrieved before ExitBootServices(). Mike > -Original Message- > From: edk2-devel [mailto:edk2-devel- > boun...@lists.01.org] On Behalf Of Ard Biesheuvel > Sent: Tuesday, February 20, 2018 3:05 AM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Ard Biesheuvel > ; af...@apple.com; > leif.lindh...@linaro.org; Gao, Liming > ; Kinney, Michael D > ; ler...@redhat.com; Zeng, > Star > Subject: [edk2] [PATCH 1/3] MdePkg: introduce > DxeRuntimeDebugLibSerialPort > > Introduce a variant of BaseDebugLibSerialPort that > behaves correctly wrt > to use of the serial port after ExitBootServices(). > Also, it uses fixed > PCDs for all the parameterized values so that no calls > into PcdLib are > made at runtime. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > > --- > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > | 342 > > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe > bugLibSerialPort.inf | 46 +++ > > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe > bugLibSerialPort.uni | 21 ++ > 3 files changed, 409 insertions(+) > > diff --git > a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > new file mode 100644 > index ..d18267d91322 > --- /dev/null > +++ > b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > @@ -0,0 +1,342 @@ > +/** @file > + DXE runtime Debug library instance based on Serial > Port library. > + It uses PrintLib to send debug messages to serial > port device. > + > + NOTE: If the Serial Port library enables hardware > flow control, then a call > + to DebugPrint() or DebugAssert() may hang if writes > to the serial port are > + being blocked. This may occur if a key(s) are > pressed in a terminal emulator > + used to monitor the DEBUG() and ASSERT() messages. > + > + Copyright (c) 2006 - 2011, Intel Corporation. All > rights reserved. > + Copyright (c) 2018, Linaro, Ltd. All rights > reserved. > + > + This program and the accompanying materials > + are licensed and made available under the terms and > conditions of the BSD License > + which accompanies this distribution. The full text > of the license may be found at > + http://opensource.org/licenses/bsd-license.php. > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON > AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER EXPRESS OR IMPLIED. > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +STATIC EFI_EVENT mEfiExitBootServicesEvent; > +STATIC BOOLEANmEfiAtRuntime; > + > +// > +// Define the maximum debug and assert message length > that this library supports > +// > +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 > + > +/** > + Set AtRuntime flag as TRUE after ExitBootServices. > + > + @param[in] Event The Event that is being > processed. > + @param[in] Context The Event Context. > + > +**/ > +STATIC > +VOID > +EFIAPI > +RuntimeLibExitBootServicesEvent ( > + IN EFI_EVENTEvent, > + IN VOID *Context > + ) > +{ > + mEfiAtRuntime = TRUE; > +} > + > +/** > + The constructor function initialize the Serial Port > Library > + > + @retval EFI_SUCCESS The constructor always returns > RETURN_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +DxeRuntimeDebugLibSerialPortConstructor ( > + IN EFI_HANDLEImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUSStatus; > + > + Status = SerialPortInitialize (); > + if (EFI_ERROR (Status)) { > +return Status; > + } > + > + return SystemTable->BootServices->CreateEventEx ( > + > EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + > RuntimeLibExitBootServicesEvent, > + NULL, > + > &gEfiEventExitBootServicesGuid, > + > &mEfiExitBootServicesEvent); > +} > + > +/** > + Prints a debug message to the debug output device if > the specified error level > + is enabled. > + > + If any bit in ErrorLevel is also set in > DebugPrintErrorLevelLib function > + GetDebugPrintErrorLevel (), then print the message > specified by Format and the > + associated variable argument list to the debug output > device. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevel The error level of the debug > message. > + @param Format Format string for the debug > message to print. > + @param ... Variable argument list whose > contents are accessed > + based on the format string > specified by Format. > + > +**/ > +VOID > +EFIAPI > +DebugPrint ( > + IN UINTNErrorLevel, > + IN CONST CHAR8 *Format, > + ... > + ) > +{ >
Re: [edk2] [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers
I do not agree with this change. If the PCDs that describe the UART are for a UART that is owned by the FW and hidden from the OS, then this lib can work well at runtime. Mike > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, February 20, 2018 9:00 AM > To: Leif Lindholm ; Ard > Biesheuvel > Cc: edk2-devel@lists.01.org; Ni, Ruiyu > ; Kinney, Michael D > ; Gao, Liming > ; Zeng, Star > ; af...@apple.com > Subject: Re: [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: > blacklist for use by DXE runtime drivers > > On 02/20/18 15:22, Leif Lindholm wrote: > > On Tue, Feb 20, 2018 at 11:05:24AM +, Ard > Biesheuvel wrote: > >> BaseDebugLibSerialPort is not suitable for use by > DXE_RUNTIME_DRIVER > >> modules, so blacklist it for use by such modules. > >> > >> Contributed-under: TianoCore Contribution Agreement > 1.1 > >> Signed-off-by: Ard Biesheuvel > > >> --- > >> > MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerial > Port.inf | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git > a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri > alPort.inf > b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri > alPort.inf > >> index 823511b22f6b..25da1fb9363a 100644 > >> --- > a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri > alPort.inf > >> +++ > b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeri > alPort.inf > >> @@ -21,7 +21,7 @@ [Defines] > >>FILE_GUID = BB83F95F-EDBC- > 4884-A520-CD42AF388FAE > >>MODULE_TYPE= BASE > >>VERSION_STRING = 1.0 > >> - LIBRARY_CLASS = DebugLib > >> + LIBRARY_CLASS = DebugLib|SEC > PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_SMM_DRIVER > DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION SMM_CORE > MM_STANDALONE MM_CORE_STANDALONE > > > > Not a comment on this patch as such (which looks > sensible to me), but > > what you're doing here isn't blacklisting > DXE_RUNTIME_DRIVER, but > > rather whitelisting every other module type. > > > > Could we use a ! operator added to the syntax? > > No, I don't think so, based on the INF file spec. > > Reviewed-by: Laszlo Ersek > > ( > > A future customization would be to give a similar > treatment to SMM (or > "MM") drivers. While MM has its own set of (identity > mapped) page > tables, and therefore I'd expect an MMIO serial port to > "just work", we > still shouldn't mess with the serial port once the OS > owns it, > regardless of the fact that we're in MM. So, for that, > the lib instance > meant for MM drivers would have to catch EBS too. > > Of course, that doesn't work the same way -- the > SMM_CORE catches the > normal EBS notification, and "forwards" it into the MM > protocol database > (see SmmExitBootServicesHandler() in > "MdeModulePkg/Core/PiSmmCore/PiSmmCore.c"). So the MM > lib instance would > have to register an MM protocol notify for > "gEdkiiSmmExitBootServicesProtocolGuid". > > But, that's for the future at best. > > *This* lib instance should remain correct for the > SMM_CORE itself, however. > > ) > > Thanks, > Laszlo > > > > > / > > Leif > > > >>CONSTRUCTOR= > BaseDebugLibSerialPortConstructor > >> > >> # > >> -- > >> 2.11.0 > >> ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH edk2-platforms v2 3/7] Platform/NinetySixBoards: introduce I2C driver
Implement a I2C DXE driver that wires up the I2C devices exposed by a 96boards mezzanine into the EDK2 I2C stack. Note that this requires the platform to identify its I2C master implementations using special GUIDs-as-protocols. It also assumes [for now] that I2C buses are not shared between the 96boards connector and other platform peripherals. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c | 202 Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.inf | 51 + 2 files changed, 253 insertions(+) diff --git a/Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c b/Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c new file mode 100644 index ..9a779626b19f --- /dev/null +++ b/Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c @@ -0,0 +1,202 @@ +/** @file + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD License + which accompanies this distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +STATIC MEZZANINE_PROTOCOL *mMezzanine; + +typedef struct { + EFI_I2C_ENUMERATE_PROTOCOLI2cEnumerate; + EFI_I2C_BUS_CONFIGURATION_MANAGEMENT_PROTOCOL I2cConfigManagement; + EFI_HANDLEI2cMasterHandle; + UINT32BusFrequency; + UINTN NumDevices; + CONST EFI_I2C_DEVICE *Devices; +} I2C_BUS; + +STATIC +EFI_STATUS +EFIAPI +I2cEnumerate ( + IN CONST EFI_I2C_ENUMERATE_PROTOCOL *This, + IN OUT CONST EFI_I2C_DEVICE **Device + ) +{ + I2C_BUS *Bus; + + if (Device == NULL) { +return EFI_INVALID_PARAMETER; + } + + Bus = BASE_CR (This, I2C_BUS, I2cEnumerate); + + if (*Device == NULL) { +*Device = &Bus->Devices[0]; + } else if (*Device >= &Bus->Devices[0] && + *Device < &Bus->Devices[Bus->NumDevices - 2]) { +++*Device; + } else { +return EFI_NO_MAPPING; + } + return EFI_SUCCESS; +} + +STATIC +EFI_STATUS +EFIAPI +I2cGetBusFrequency ( + IN CONST EFI_I2C_ENUMERATE_PROTOCOL *This, + IN UINTNI2cBusConfiguration, + OUT UINTN *BusClockHertz + ) +{ + I2C_BUS *Bus; + + if (BusClockHertz == NULL) { +return EFI_INVALID_PARAMETER; + } + + if (I2cBusConfiguration > 0) { +return EFI_NO_MAPPING; + } + + Bus = BASE_CR (This, I2C_BUS, I2cEnumerate); + + *BusClockHertz = Bus->BusFrequency; + + return EFI_SUCCESS; +} + +STATIC +EFI_STATUS +EFIAPI +EnableI2cBusConfiguration ( + IN CONST EFI_I2C_BUS_CONFIGURATION_MANAGEMENT_PROTOCOL *This, + IN UINTNI2cBusConfiguration, + IN EFI_EVENTEvent OPTIONAL, + IN EFI_STATUS *I2cStatus OPTIONAL + ) +{ + EFI_I2C_MASTER_PROTOCOL *I2cMaster; + EFI_STATUS Status; + UINTN BusClockHertz; + I2C_BUS *Bus; + + if (I2cBusConfiguration > 0) { +return EFI_NO_MAPPING; + } + + Bus = BASE_CR (This, I2C_BUS, I2cConfigManagement); + + Status = gBS->HandleProtocol (Bus->I2cMasterHandle, + &gEfiI2cMasterProtocolGuid, (VOID **)&I2cMaster); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "%a: gBS->HandleProtocol() failed - %r\n", + __FUNCTION__, Status)); +return Status; + } + + BusClockHertz = Bus->BusFrequency; + Status = I2cMaster->SetBusFrequency (I2cMaster, &BusClockHertz); + if (EFI_ERROR (Status)) { +DEBUG ((DEBUG_ERROR, "%a: I2cMaster->SetBusFrequency() failed - %r\n", + __FUNCTION__, Status)); +return Status; + } + + if (Event != NULL) { +*I2cStatus = EFI_SUCCESS; +gBS->SignalEvent (Event); + } + return EFI_SUCCESS; +} + +STATIC I2C_BUS mI2cBus0 = { + { I2cEnumerate, I2cGetBusFrequency }, + { EnableI2cBusConfiguration }, + NULL, + FixedPcdGet32 (PcdI2c0BusFrequencyHz), + 0, + NULL, +}; + +STATIC I2C_BUS mI2cBus1 = { + { I2cEnumerate, I2cGetBusFrequency }, + { EnableI2cBusConfiguration }, + NULL, + FixedPcdGet32 (PcdI2c1BusFrequencyHz), + 0, + NULL, +}; + +STATIC +VOID +RegisterI2cBus ( + IN EFI_GUID*Guid, + IN I2C_BUS *I2cBus, + IN UINTN NumDevices, + IN CONST EFI_I2C_DEVICE*Devices + ) +{ + EFI_STATUSStatus; + UINTN
[edk2] [PATCH edk2-platforms v2 1/7] Silicon/Atmel: add support for AtSha204a RNG
This adds support for using the random number generator in the Atmel AtSha204a over I2C. Other functionality of the chip is currently unsupported. Note that the the I2C support in this device essentially violates the protocol layering, by requiring that the device is woken up by driving SDA low for a certain amount of time, which is something that cannot be expressed in terms of an I2C packet sent to the device's slave address. Instead, the slave address 0x0 is added to the device's address array, and the wake is sent by sending a dummy write to address 0x0, and ignoring the subsequent error. This requires the I2C bus to be clocked at 100 kHz. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Silicon/Atmel/AtSha204a/AtSha204a.dec | 22 ++ Silicon/Atmel/AtSha204a/AtSha204aDriver.c | 309 Silicon/Atmel/AtSha204a/AtSha204aDriver.h | 81 + Silicon/Atmel/AtSha204a/AtSha204aDxe.inf | 52 Silicon/Atmel/AtSha204a/ComponentName.c | 186 Silicon/Atmel/AtSha204a/DriverBinding.c | 242 +++ 6 files changed, 892 insertions(+) diff --git a/Silicon/Atmel/AtSha204a/AtSha204a.dec b/Silicon/Atmel/AtSha204a/AtSha204a.dec new file mode 100644 index ..f1fdea59843d --- /dev/null +++ b/Silicon/Atmel/AtSha204a/AtSha204a.dec @@ -0,0 +1,22 @@ +## @file +# +# Copyright (c) 2018, Linaro Ltd. All rights reserved. +# +# This program and the accompanying materials +# are licensed and made available under the terms and conditions of the BSD License +# which accompanies this distribution. The full text of the license may be found at +# http://opensource.org/licenses/bsd-license.php +# +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +# +## + +[Defines] + DEC_SPECIFICATION = 0x0001001A + PACKAGE_NAME = AtSha204a + PACKAGE_GUID = 86085a5b-355b-4e72-92ab-fc3e1d71c9ad + PACKAGE_VERSION= 0.1 + +[Guids] + gAtSha204aI2cDeviceGuid = { 0x52e9b64b, 0x4ec1, 0x4bd6, { 0x9e, 0x1c, 0x6d, 0xac, 0xef, 0x35, 0x18, 0x21 } } diff --git a/Silicon/Atmel/AtSha204a/AtSha204aDriver.c b/Silicon/Atmel/AtSha204a/AtSha204aDriver.c new file mode 100644 index ..5db2de21a731 --- /dev/null +++ b/Silicon/Atmel/AtSha204a/AtSha204aDriver.c @@ -0,0 +1,309 @@ +/** @file + Device driver for the Atmel ATSHA204A random number generator. + + Copyright (c) 2018, Linaro Ltd. All rights reserved. + + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD + License which accompanies this distribution. The full text of the license may + be found at http://opensource.org/licenses/bsd-license.php. + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include "AtSha204aDriver.h" + +#include +#include +#include + +#define MAX_RETRIES 5 + +// Don't bother calculating the CRC for the immutable RANDOM opcode packet +#define OPCODE_COMMAND_PACKET_CRC 0xcd24 + +/** + Returns information about the random number generation implementation. + + @param[in] This A pointer to the EFI_RNG_PROTOCOL instance. + @param[in,out] AlgorithmListSize On input, the size in bytes of AlgorithmList +On output with a return code of EFI_SUCCESS, +the size in bytes of the data returned in +AlgorithmList. On output with a return +code of EFI_BUFFER_TOO_SMALL, the size of +AlgorithmList required to obtain the list. + @param[out] AlgorithmList A caller-allocated memory buffer filled by +the driver with one EFI_RNG_ALGORITHM +element for each supported RNG algorithm. +The list must not change across multiple +calls to the same driver. The first +algorithm in the list is the default +algorithm for the driver. + + @retval EFI_SUCCESS The RNG algorithm list was returned +successfully. + @retval EFI_UNSUPPORTED The services is not supported by this driver + @retval EFI_DEVICE_ERROR The list of algorithms could not be +retrieved due to a hardware or firmware +error. + @retval EFI_INVALID_PARAMETER One or more of the parameters are incorrect. + @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to +
[edk2] [PATCH edk2-platforms v2 2/7] Platform/NinetySixBoards: introduce package and mezzanine protocol
Introduce the mezzanine protocol and the 96boards package defining the PCDs and GUIDs that may be used by implementations of the protocol. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/NinetySixBoards/Include/Protocol/Mezzanine.h | 71 Platform/NinetySixBoards/NinetySixBoards.dec | 67 ++ 2 files changed, 138 insertions(+) diff --git a/Platform/NinetySixBoards/Include/Protocol/Mezzanine.h b/Platform/NinetySixBoards/Include/Protocol/Mezzanine.h new file mode 100644 index ..7869ea979b48 --- /dev/null +++ b/Platform/NinetySixBoards/Include/Protocol/Mezzanine.h @@ -0,0 +1,71 @@ +/** @file + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef _MEZZANINE_H_ +#define _MEZZANINE_H_ + +#include +#include + +#define MEZZANINE_PROTOCOL_GUID \ + { 0xf0467a37, 0x3436, 0x40ef, { 0x94, 0x09, 0x4d, 0x1d, 0x7f, 0x51, 0x06, 0xd3 } } + +typedef struct _MEZZANINE_PROTOCOL MEZZANINE_PROTOCOL; + +/** + Apply the mezzanine's DT overlay + + @param[in] This Pointer to the MEZZANINE_PROTOCOL instance. + @param[in,out] Dtb Pointer to the device tree blob + + @return EFI_SUCCESS Operation succeeded. + @return other An error has occurred. +**/ +typedef +EFI_STATUS +(EFIAPI *APPLY_DEVICE_TREE_OVERLAY) ( + IN MEZZANINE_PROTOCOL*This, + IN OUT VOID *Dtb + ); + +struct _MEZZANINE_PROTOCOL { + // + // Get the device tree overlay for this mezzanine board + // + APPLY_DEVICE_TREE_OVERLAY ApplyDeviceTreeOverlay; + // + // The number of devices on LS connector I2C bus #0 + // + UINT32 I2c0NumDevices; + // + // The number of devices on LS connector I2C bus #1 + // + UINT32 I2c1NumDevices; + // + // Linear array of I2C devices on LS connector bus #0 + // + CONST EFI_I2C_DEVICE*I2c0DeviceArray; + // + // Linear array of I2C devices on LS connector bus #0 + // + CONST EFI_I2C_DEVICE*I2c1DeviceArray; + // + // NULL terminated linked list of SPI devices attached to the LS connector + // + CONST EFI_SPI_PERIPHERAL*SpiDeviceLinkedList; +}; + +extern EFI_GUID gNinetySixBoardsMezzanineProtocolGuid; + +#endif // _MEZZANINE_H_ diff --git a/Platform/NinetySixBoards/NinetySixBoards.dec b/Platform/NinetySixBoards/NinetySixBoards.dec new file mode 100644 index ..f7e2b01459d7 --- /dev/null +++ b/Platform/NinetySixBoards/NinetySixBoards.dec @@ -0,0 +1,67 @@ +## @file +# +# Copyright (c) 2018, Linaro Ltd. All rights reserved. +# +# This program and the accompanying materials +# are licensed and made available under the terms and conditions of the BSD License +# which accompanies this distribution. The full text of the license may be found at +# http://opensource.org/licenses/bsd-license.php +# +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +# +## + +[Defines] + DEC_SPECIFICATION = 0x0001001A + PACKAGE_NAME = NinetySixBoards + PACKAGE_GUID = ce4a4683-6e2d-4ec3-bc11-974289a09ab0 + PACKAGE_VERSION= 0.1 + +[Includes] + Include + +[Protocols] + ## Include/Protocol/Mezzanine.h + gNinetySixBoardsMezzanineProtocolGuid = { 0xf0467a37, 0x3436, 0x40ef, { 0x94, 0x09, 0x4d, 0x1d, 0x7f, 0x51, 0x06, 0xd3 } } + +[Guids] + # PCD scope GUID + gNinetySixBoardsTokenSpaceGuid = { 0xe0d2f33a, 0xb7dd, 0x4a69, { 0xb6, 0x76, 0xda, 0xe8, 0xa4, 0x17, 0xa7, 0xb5 } } + + # GUIDs to be installed as protocols to identify which controller connects to which bus + gNinetySixBoardsI2c0MasterGuid = { 0xba10e402, 0xcfdd, 0x4b87, { 0xbd, 0x02, 0x6e, 0x26, 0x9f, 0x01, 0x94, 0x11 } } + gNinetySixBoardsI2c1MasterGuid = { 0xcf64ac46, 0xd0be, 0x4a69, { 0x90, 0xa2, 0xf2, 0x82, 0x5b, 0x92, 0x25, 0x61 } } + gNinetySixBoardsSpiMasterGuid = { 0x9703fd99, 0xe638, 0x42b8, { 0xab, 0x81, 0x52, 0x61, 0x1b, 0xf7, 0xf7, 0x5d } } + +[PcdsFixedAtBuild] + # ASCII DT paths to the I2C parent nodes of the 96boards LS connector + gNinetySixBoardsTokenSpaceGuid.PcdI2c0Parent|""|VOID*|0x0001 + gNinetySixBoardsTokenSpaceGuid.PcdI2c1Parent|""|VOID*|0x0002 + + # I2C bus frequency in Hertz + gNinetySixBoardsTokenSpaceGuid.PcdI2c0BusFrequencyHz|0|UINT32|0x0003 + gNinetySixBoardsTokenSpaceGuid.PcdI2c1BusFrequencyHz|0|UINT32|0x0004 + + # ASCII DT path to the SPI parent node of the 96boa
[edk2] [PATCH edk2-platforms v2 7/7] Platform/Socionext/DeveloperBox: add 96boards mezzanine support
Wire up the various drivers for the 96boards LS connector and the optional Secure96 mezzanine board. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/Socionext/DeveloperBox/DeveloperBox.dsc| 34 Platform/Socionext/DeveloperBox/DeveloperBox.fdf| 10 ++ Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c | 9 ++ Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 ++ 4 files changed, 55 insertions(+) diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc index 9c95618a20fe..39f40534bb39 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc @@ -33,6 +33,9 @@ [Defines] [BuildOptions] RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 + # add ample padding to the DTC so we can apply 96boards mezzanine overlays + *_*_*_DTC_FLAGS = -p 1024 + [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION] GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 @@ -398,6 +401,28 @@ [PcdsFixedAtBuild.common] !endif gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER) + # + # 96boards mezzanine support + # + gNinetySixBoardsTokenSpaceGuid.PcdI2c0Parent|"/i2c@5121" + gNinetySixBoardsTokenSpaceGuid.PcdI2c0BusFrequencyHz|10 + gNinetySixBoardsTokenSpaceGuid.PcdSpiParent|"/spi@5481" + gNinetySixBoardsTokenSpaceGuid.PcdGpioParent|"/gpio@5100" + gNinetySixBoardsTokenSpaceGuid.PcdGpioPolarity|0 + + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinA|10 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinB|11 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinC|12 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinD|13 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinE|18 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinF|19 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinG|20 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinH|21 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinI|22 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinJ|23 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinK|24 + gNinetySixBoardsTokenSpaceGuid.PcdGpioPinL|25 + [PcdsPatchableInModule] gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0 gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0 @@ -644,8 +669,17 @@ [Components.common] SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf # + # 96board mezzanine support + # + Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf + Silicon/Atmel/AtSha204a/AtSha204aDxe.inf + Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.inf + Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf + + # # I2C # Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.inf + MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf Platform/Socionext/DeveloperBox/OsInstallerMenuDxe/OsInstallerMenuDxe.inf diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf index c2bc5aa85739..636bfaa3dd29 100644 --- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf @@ -237,9 +237,18 @@ [FV.FvMain] } # + # 96board mezzanine support + # + INF Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf + INF Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.inf + INF Silicon/Atmel/AtSha204a/AtSha204aDxe.inf + INF Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf + + # # I2C # INF Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.inf + INF MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf INF Platform/Socionext/DeveloperBox/OsInstallerMenuDxe/OsInstallerMenuDxe.inf @@ -430,6 +439,7 @@ [Rule.Common.DXE_DRIVER] DXE_DEPEXDXE_DEPEX Optional $(INF_OUTPUT)/$(MODULE_NAME).depex PE32 PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi UI STRING="$(MODULE_NAME)" Optional +RAW BINOptional|.dtb } [Rule.Common.DXE_RUNTIME_DRIVER] diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c index aab830dc3a5a..11c0648c2ca0 100644 --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c @@ -313,6 +313,15 @@ PlatformDxeEntryPoint ( &Handle); ASSERT_EFI_ERROR (Status); + // + // Install the gNinetySixBoardsI2c0MasterGuid GUID onto the same handle, + // identifying I2C #1 on our SoC as I2C #0 on the 96boards low speed connector + // + Status = gBS->InstallProtocolInterface (&Handle, + &gNinetySixBoardsI2c0MasterGuid, +
[edk2] [PATCH edk2-platforms v2 0/7] Add Secure96 mezzanine support
Almost a complete rewrite of the v1. I omitted the patches that add SPI and I2C DT nodes to the SynQuacer DTS. I did include the v2 of the Atmel AtSha204a driver, since the latter patches depend on it. This version implements a complete split between the generic 96boards LS connector support and its associated plumbing (defining which I2C, SPI and GPIO controllers are connected) on the one hand, and support for the Secure96 mezzanine board in particular on the other. More specifically, all Secure96 PCDs were dropped, and the only platform specific configuration that remains is including the Secure96 driver and the driver for its peripherals to the build. Patch #1 is v2 of the AtSha204a driver, with Leif's review comments addressed. Patch #2 introduces the mezzanine protocol, which abstracts away from any particular mezzanine implementation. Patch #3 introduces the generic I2C plumbing for any mezzanine that exposes I2C peripherals. Patch #4 implements the protocol that asserts the presence of a 96boards LS connector and the type of mezzanine connected to it. Patch #5 adds the Secure96 driver, which incorporates the DT overlay, and a description of the I2C peripheral. Patch #6 adds a generic driver for configuring the type of mezzanine, and to interface with it at end of DXE time to install the appropriate DT overlay. Patch #7 wires everything up for the DeveloperBox platform. Ard Biesheuvel (7): Silicon/Atmel: add support for AtSha204a RNG Platform/NinetySixBoards: introduce package and mezzanine protocol Platform/NinetySixBoards: introduce I2C driver Platform/NinetySixBoards: introduce LsConnector protocol Platform/NinetySixBoards: add a driver for the Secure96 mezzanine board Platform/NinetySixBoards: add core driver for LS connector and config Platform/Socionext/DeveloperBox: add 96boards mezzanine support Platform/NinetySixBoards/Include/Guid/FormSet.h | 23 ++ Platform/NinetySixBoards/Include/Protocol/LsConnector.h | 35 +++ Platform/NinetySixBoards/Include/Protocol/Mezzanine.h| 71 + Platform/NinetySixBoards/NinetySixBoards.dec | 73 + Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c | 221 ++ Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h | 32 ++ Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf | 57 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni | 27 ++ Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr | 51 Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c | 202 + Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.inf | 51 Platform/NinetySixBoards/Secure96Dxe/Secure96.dts| 76 + Platform/NinetySixBoards/Secure96Dxe/Secure96.h | 26 ++ Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c | 211 + Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf | 67 + Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 34 +++ Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 10 + Silicon/Atmel/AtSha204a/AtSha204a.dec| 22 ++ Silicon/Atmel/AtSha204a/AtSha204aDriver.c| 309 Silicon/Atmel/AtSha204a/AtSha204aDriver.h| 81 + Silicon/Atmel/AtSha204a/AtSha204aDxe.inf | 52 Silicon/Atmel/AtSha204a/ComponentName.c | 186 Silicon/Atmel/AtSha204a/DriverBinding.c | 242 +++ Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c| 9 + Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 + 25 files changed, 2170 insertions(+) create mode 100644 Platform/NinetySixBoards/Include/Guid/FormSet.h create mode 100644 Platform/NinetySixBoards/Include/Protocol/LsConnector.h create mode 100644 Platform/NinetySixBoards/Include/Protocol/Mezzanine.h create mode 100644 Platform/NinetySixBoards/NinetySixBoards.dec create mode 100644 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c create mode 100644 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h create mode 100644 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf create mode 100644 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni create mode 100644 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr create mode 100644 Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI2cDxe.c create mode 100644 Platform/NinetySixBoards/NinetySixBoardsI2cDxe/NinetySixBoardsI
[edk2] [PATCH edk2-platforms v2 5/7] Platform/NinetySixBoards: add a driver for the Secure96 mezzanine board
Add a driver that describes the Secure96 mezzanine board, and exposes both the information required to describe it to the OS using a DT overlay, and to describe it to UEFI itself. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/NinetySixBoards/Secure96Dxe/Secure96.dts| 76 +++ Platform/NinetySixBoards/Secure96Dxe/Secure96.h | 26 +++ Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c | 211 Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf | 67 +++ 4 files changed, 380 insertions(+) diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts b/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts new file mode 100644 index ..08f59978c06f --- /dev/null +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts @@ -0,0 +1,76 @@ +/** @file + * Copyright (c) 2018, Linaro Limited. All rights reserved. + * + * This program and the accompanying materials are licensed and made + * available under the terms and conditions of the BSD License which + * accompanies this distribution. The full text of the license may be + * found at http://opensource.org/licenses/bsd-license.php + * + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR + * IMPLIED. + */ + +#include "Secure96.h" + +#define GPIO_PARENT_PLACEHOLDER_PHANDLE 0x0 + +/dts-v1/; +/plugin/; + +/ { +fragment@0 { +target-path = "I2C_PARENT_PLACEHOLDER_STRING"; +__overlay__ { +ATSHA204A_DT_NODENAME { +compatible = "atmel,atsha204a"; +reg = ; +}; +ATECC508A_DT_NODENAME { +compatible = "atmel,atecc508a"; +reg = ; +}; +}; +}; + +fragment@1 { +target-path = "SPI_PARENT_PLACEHOLDER_STRING"; +__overlay__ { +INFINEON_SLB9670_DT_NODENAME { +compatible = "infineon,slb9670"; +reg = ; +spi-max-frequency = <2250>; +}; +}; +}; + +fragment@2 { +target-path = "/"; +__overlay__ { +gpio-leds { +compatible = "gpio-leds"; + +secure96-u1 { +gpios = ; +}; +secure96-u2 { +gpios = ; +}; +secure96-u3 { +gpios = ; +}; +secure96-u4 { +gpios = ; +}; +}; +}; +}; +}; diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96.h b/Platform/NinetySixBoards/Secure96Dxe/Secure96.h new file mode 100644 index ..84b8aed13d1e --- /dev/null +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96.h @@ -0,0 +1,26 @@ +/** @file + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +**/ + +#ifndef _SECURE96_H_ +#define _SECURE96_H_ + +#define ATSHA204A_SLAVE_ADDRESS 0x60 +#define ATSHA204A_DT_NODENAME atsha204a@60 + +#define ATECC508A_SLAVE_ADDRESS 0x51 +#define ATECC508A_DT_NODENAME atecc508a@51 + +#define INFINEON_SLB9670_SPI_CS 0x0 +#define INFINEON_SLB9670_DT_NODENAMEtpm@0 + +#endif // _SECURE96_H_ diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c new file mode 100644 index ..000f4b4abba4 --- /dev/null +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c @@ -0,0 +1,211 @@ +/** @file + 96boards Secure96 mezzanine board DXE driver. + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +**/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "Secure96.h" + +STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = { + ATSHA204A_SLAVE_ADDRESS, + + // + // The Atmel AtSha204a has an annoying 'wake' mode where it will only wake + // up if SDA is held low for a certain amount of time. Attempting to access + // a device at address 0x0 at 10
[edk2] [PATCH edk2-platforms v2 4/7] Platform/NinetySixBoards: introduce LsConnector protocol
Introduce a protocol describing the presence of a 96boards LS connector, and identifying the type of mezzanine that has been installed. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/NinetySixBoards/Include/Protocol/LsConnector.h | 35 Platform/NinetySixBoards/NinetySixBoards.dec| 3 ++ 2 files changed, 38 insertions(+) diff --git a/Platform/NinetySixBoards/Include/Protocol/LsConnector.h b/Platform/NinetySixBoards/Include/Protocol/LsConnector.h new file mode 100644 index ..a1f0132c85ae --- /dev/null +++ b/Platform/NinetySixBoards/Include/Protocol/LsConnector.h @@ -0,0 +1,35 @@ +/** @file + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef _LS_CONNECTOR_H_ +#define _LS_CONNECTOR_H_ + +#define LS_CONNECTOR_PROTOCOL_GUID \ + { 0xae548d4c, 0x9062, 0x4eed, { 0x83, 0x5f, 0xf5, 0x10, 0xf8, 0xfc, 0x48, 0xaf } } + +typedef struct _LS_CONNECTOR_PROTOCOL LS_CONNECTOR_PROTOCOL; + +typedef enum { + MezzanineUnknown, + MezzanineSecure96, + MezzanineMax +} MEZZANINE_TYPE; + +struct _LS_CONNECTOR_PROTOCOL { + MEZZANINE_TYPE MezzanineType; +}; + +extern EFI_GUID gNinetySixBoardsLsConnectorProtocolGuid; + +#endif // _LS_CONNECTOR_H_ diff --git a/Platform/NinetySixBoards/NinetySixBoards.dec b/Platform/NinetySixBoards/NinetySixBoards.dec index f7e2b01459d7..5c3fe43dbb24 100644 --- a/Platform/NinetySixBoards/NinetySixBoards.dec +++ b/Platform/NinetySixBoards/NinetySixBoards.dec @@ -25,6 +25,9 @@ [Protocols] ## Include/Protocol/Mezzanine.h gNinetySixBoardsMezzanineProtocolGuid = { 0xf0467a37, 0x3436, 0x40ef, { 0x94, 0x09, 0x4d, 0x1d, 0x7f, 0x51, 0x06, 0xd3 } } + ## Include/Protocol/LsConnector.h + gNinetySixBoardsLsConnectorProtocolGuid = { 0xae548d4c, 0x9062, 0x4eed, { 0x83, 0x5f, 0xf5, 0x10, 0xf8, 0xfc, 0x48, 0xaf } } + [Guids] # PCD scope GUID gNinetySixBoardsTokenSpaceGuid = { 0xe0d2f33a, 0xb7dd, 0x4a69, { 0xb6, 0x76, 0xda, 0xe8, 0xa4, 0x17, 0xa7, 0xb5 } } -- 2.11.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH edk2-platforms v2 6/7] Platform/NinetySixBoards: add core driver for LS connector and config
This adds a driver that manages the 96boards LS connector, i.e, it installs a HII page to configure the type of mezzanine that is installed in the slot, and it exposes this information via the LS connector protocol. It is also in charge of applying the overlay to the platform device tree at end of DXE. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- Platform/NinetySixBoards/Include/Guid/FormSet.h| 23 ++ Platform/NinetySixBoards/NinetySixBoards.dec | 3 + Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c | 221 Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h | 32 +++ Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf | 57 + Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni | 27 +++ Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr | 51 + 7 files changed, 414 insertions(+) diff --git a/Platform/NinetySixBoards/Include/Guid/FormSet.h b/Platform/NinetySixBoards/Include/Guid/FormSet.h new file mode 100644 index ..db16657f0848 --- /dev/null +++ b/Platform/NinetySixBoards/Include/Guid/FormSet.h @@ -0,0 +1,23 @@ +/** @file + + Copyright (c) 2018, Linaro Limited. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#ifndef __NINETY_SIX_BOARDS_FORMSET_H__ +#define __NINETY_SIX_BOARDS_FORMSET_H__ + +#define NINETY_SIX_BOARDS_FORMSET_GUID \ + { 0x7500c9d2, 0x9203, 0x4a37, { 0x84, 0xbb, 0x92, 0xa9, 0xce, 0x34, 0x38, 0xbd } } + +extern EFI_GUID gNinetySixBoardsFormsetGuid; + +#endif // __NINETY_SIX_BOARDS_FORMSET_H__ diff --git a/Platform/NinetySixBoards/NinetySixBoards.dec b/Platform/NinetySixBoards/NinetySixBoards.dec index 5c3fe43dbb24..e7d1705d47ff 100644 --- a/Platform/NinetySixBoards/NinetySixBoards.dec +++ b/Platform/NinetySixBoards/NinetySixBoards.dec @@ -37,6 +37,9 @@ [Guids] gNinetySixBoardsI2c1MasterGuid = { 0xcf64ac46, 0xd0be, 0x4a69, { 0x90, 0xa2, 0xf2, 0x82, 0x5b, 0x92, 0x25, 0x61 } } gNinetySixBoardsSpiMasterGuid = { 0x9703fd99, 0xe638, 0x42b8, { 0xab, 0x81, 0x52, 0x61, 0x1b, 0xf7, 0xf7, 0x5d } } + # GUID for the HII configuration form + gNinetySixBoardsFormsetGuid = { 0x7500c9d2, 0x9203, 0x4a37, { 0x84, 0xbb, 0x92, 0xa9, 0xce, 0x34, 0x38, 0xbd } } + [PcdsFixedAtBuild] # ASCII DT paths to the I2C parent nodes of the 96boards LS connector gNinetySixBoardsTokenSpaceGuid.PcdI2c0Parent|""|VOID*|0x0001 diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c new file mode 100644 index ..6dc5f549e560 --- /dev/null +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c @@ -0,0 +1,221 @@ +/** @file + + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "NinetySixBoardsDxe.h" + +extern UINT8 NinetySixBoardsHiiBin[]; +extern UINT8 NinetySixBoardsDxeStrings[]; + +typedef struct { + VENDOR_DEVICE_PATH VendorDevicePath; + EFI_DEVICE_PATH_PROTOCOLEnd; +} HII_VENDOR_DEVICE_PATH; + +STATIC HII_VENDOR_DEVICE_PATH mNinetySixBoardsDxeVendorDevicePath = { + { +{ + HARDWARE_DEVICE_PATH, + HW_VENDOR_DP, + { +(UINT8) (sizeof (VENDOR_DEVICE_PATH)), +(UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) + } +}, +NINETY_SIX_BOARDS_FORMSET_GUID + }, + { +END_DEVICE_PATH_TYPE, +END_ENTIRE_DEVICE_PATH_SUBTYPE, +{ + (UINT8) (END_DEVICE_PATH_LENGTH), + (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8) +} + } +}; + +STATIC LS_CONNECTOR_PROTOCOL mLsConnector; +STATIC EFI_EVENT EndOfDxeEvent; + +STATIC +EFI_STATUS +InstallHiiPages ( + VOID + ) +{ + EFI_STATUS Status; + EFI_HII_HANDLE HiiHandle; + EFI_HANDLE DriverHandle; + + DriverHandle = NULL; + Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
Re: [edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
On Tue, Feb 20, 2018 at 08:20:48AM -0800, Andrew Fish wrote: > > On Feb 20, 2018, at 6:16 AM, Leif Lindholm wrote: > > > > On Tue, Feb 20, 2018 at 11:05:22AM +, Ard Biesheuvel wrote: > >> +/** > >> + Prints an assert message containing a filename, line number, and > >> description. > >> + This may be followed by a breakpoint or a dead loop. > >> + > >> + Print a message of the form "ASSERT (): > >> \n" > >> + to the debug output device. If > >> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit > >> + of PcdDebugProperyMask is set then CpuBreakpoint() is called. > >> Otherwise, if > >> + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is > >> set then > >> + CpuDeadLoop() is called. If neither of these bits are set, then this > >> function > >> + returns immediately after the message is printed to the debug output > >> device. > >> + DebugAssert() must actively prevent recursion. If DebugAssert() is > >> called > >> + while processing another DebugAssert(), then DebugAssert() must return > >> + immediately. > >> + > >> + If FileName is NULL, then a string of "(NULL) Filename" is > >> printed. > >> + If Description is NULL, then a string of "(NULL) > >> Description" is > >> + printed. > >> + > >> + @param FileName The pointer to the name of the source file that > >> generated > >> + the assert condition. > >> + @param LineNumber The line number in the source file that generated > >> the > >> + assert condition > >> + @param Description The pointer to the description of the assert > >> condition. > >> + > >> +**/ > >> +VOID > >> +EFIAPI > >> +DebugAssert ( > >> + IN CONST CHAR8 *FileName, > >> + IN UINTNLineNumber, > >> + IN CONST CHAR8 *Description > >> + ) > >> +{ > >> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; > >> + > >> + if (!mEfiAtRuntime) { > >> +// > >> +// Generate the ASSERT() message in Ascii format > >> +// > >> +AsciiSPrint (Buffer, sizeof (Buffer), "ASSERT [%a] %a(%d): %a\n", > >> + gEfiCallerBaseName, FileName, LineNumber, Description); > >> + > >> +// > >> +// Send the print string to the Console Output device > >> +// > >> +SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer)); > >> + } > >> + > >> + // > >> + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings > >> + // > >> + if ((FixedPcdGet8 (PcdDebugPropertyMask) & > >> + DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > >> +CpuBreakpoint (); > >> + } else if ((FixedPcdGet8 (PcdDebugPropertyMask) & > >> + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > >> +CpuDeadLoop (); > >> + } > > > > Hmm ... I know this does not fundamentally change the behaviour of the > > existing implementation, but if we're looking to improve runtime > > behaviour, we've just gone from generating a runtime fault to silently > > freezing (if BREAKPOINT_ENABLED or DEADLOOP_ENABLED). > > > > What do breakpoint/deadloop mean in a runtime context anyway - do we > > not need to halt _all_ running cores? > > > > I don't see an obvious "right way" solution here, and this only > > affects DEBUG builds. > > Leif, > > It is not related to DEBUG builds, it is related to PCD configuration. Sorry, I was oversimplifying based on most RELEASE builds tending to have DebugAssertEnabled disabled through PcdDebugPropertyMask. Looking through edk2 platforms, that shows to be incorrect even among most open platform ports, so thanks for pointing this out. > > Possible ways of handling this that I can think > > of include: > > - Don't respect BREAKPOINT/DEADLOOP if at runtime. > > - Respect BREAKPOINT/DEADLOOP and disable all cores. > > - Take ownership back of the system and re-enable 1:1 mapping so > > messages can be printed. > > There is not much risk of losing user data if you "panic" EFI, that > is not true if you are going to "panic" the OS. I've seen more bugs > at runtime from confusion about what is legal to do at runtime, > vs. actual bugs. You can always just return device error on failure, > and if the RT Services hangs you can core dump the OS. Given the OS > provided the virtual mapping it is likely the stuck EFI could would > be in the stack trace of the panic. Oh, indeed. My concern is regarding the fact that this library (with either of those options set) would halt the executing processor (and no others) without any output whatsoever. / Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers
On 02/20/18 15:22, Leif Lindholm wrote: > On Tue, Feb 20, 2018 at 11:05:24AM +, Ard Biesheuvel wrote: >> BaseDebugLibSerialPort is not suitable for use by DXE_RUNTIME_DRIVER >> modules, so blacklist it for use by such modules. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git >> a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf >> b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf >> index 823511b22f6b..25da1fb9363a 100644 >> --- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf >> +++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf >> @@ -21,7 +21,7 @@ [Defines] >>FILE_GUID = BB83F95F-EDBC-4884-A520-CD42AF388FAE >>MODULE_TYPE= BASE >>VERSION_STRING = 1.0 >> - LIBRARY_CLASS = DebugLib >> + LIBRARY_CLASS = DebugLib|SEC PEI_CORE PEIM DXE_CORE >> DXE_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION >> SMM_CORE MM_STANDALONE MM_CORE_STANDALONE > > Not a comment on this patch as such (which looks sensible to me), but > what you're doing here isn't blacklisting DXE_RUNTIME_DRIVER, but > rather whitelisting every other module type. > > Could we use a ! operator added to the syntax? No, I don't think so, based on the INF file spec. Reviewed-by: Laszlo Ersek ( A future customization would be to give a similar treatment to SMM (or "MM") drivers. While MM has its own set of (identity mapped) page tables, and therefore I'd expect an MMIO serial port to "just work", we still shouldn't mess with the serial port once the OS owns it, regardless of the fact that we're in MM. So, for that, the lib instance meant for MM drivers would have to catch EBS too. Of course, that doesn't work the same way -- the SMM_CORE catches the normal EBS notification, and "forwards" it into the MM protocol database (see SmmExitBootServicesHandler() in "MdeModulePkg/Core/PiSmmCore/PiSmmCore.c"). So the MM lib instance would have to register an MM protocol notify for "gEdkiiSmmExitBootServicesProtocolGuid". But, that's for the future at best. *This* lib instance should remain correct for the SMM_CORE itself, however. ) Thanks, Laszlo > / > Leif > >>CONSTRUCTOR= BaseDebugLibSerialPortConstructor >> >> # >> -- >> 2.11.0 >> ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 2/3] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate
On 02/20/18 12:05, Ard Biesheuvel wrote: > Switch all users of ArmVirt.dsc.inc to the new DebugLib implementation > that was created especially for DXE_RUNTIME_DRIVER modules, ensuring > that DEBUG() calls do not touch the UART at runtime. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmVirtPkg/ArmVirt.dsc.inc | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc > index 0cb48f08e9bf..cde514958da2 100644 > --- a/ArmVirtPkg/ArmVirt.dsc.inc > +++ b/ArmVirtPkg/ArmVirt.dsc.inc > @@ -231,6 +231,9 @@ [LibraryClasses.common.UEFI_DRIVER] > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf >CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf > +!if $(TARGET) != RELEASE > + > DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf > +!endif > > !if $(SECURE_BOOT_ENABLE) == TRUE >BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf > I think all occurrences of the DebugLib --> "MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf" resolution should be audited and possibly patched / extended in the edk2 tree. Otherwise those platforms will be broken by patch #3. But, that can wait for the next iteration. Reviewed-by: Laszlo Ersek Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
On 02/20/18 12:05, Ard Biesheuvel wrote: > Introduce a variant of BaseDebugLibSerialPort that behaves correctly wrt > to use of the serial port after ExitBootServices(). Also, it uses fixed > PCDs for all the parameterized values so that no calls into PcdLib are > made at runtime. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > | 342 > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf > | 46 +++ > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni > | 21 ++ > 3 files changed, 409 insertions(+) > > diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > new file mode 100644 > index ..d18267d91322 > --- /dev/null > +++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > @@ -0,0 +1,342 @@ > +/** @file > + DXE runtime Debug library instance based on Serial Port library. > + It uses PrintLib to send debug messages to serial port device. > + > + NOTE: If the Serial Port library enables hardware flow control, then a call > + to DebugPrint() or DebugAssert() may hang if writes to the serial port are > + being blocked. This may occur if a key(s) are pressed in a terminal > emulator > + used to monitor the DEBUG() and ASSERT() messages. > + > + Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved. > + Copyright (c) 2018, Linaro, Ltd. All rights reserved. > + > + This program and the accompanying materials > + are licensed and made available under the terms and conditions of the BSD > License > + which accompanies this distribution. The full text of the license may be > found at > + http://opensource.org/licenses/bsd-license.php. > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +STATIC EFI_EVENT mEfiExitBootServicesEvent; > +STATIC BOOLEANmEfiAtRuntime; > + > +// > +// Define the maximum debug and assert message length that this library > supports > +// > +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 > + > +/** > + Set AtRuntime flag as TRUE after ExitBootServices. > + > + @param[in] Event The Event that is being processed. > + @param[in] Context The Event Context. > + > +**/ > +STATIC > +VOID > +EFIAPI > +RuntimeLibExitBootServicesEvent ( > + IN EFI_EVENTEvent, > + IN VOID *Context > + ) > +{ > + mEfiAtRuntime = TRUE; > +} > + > +/** > + The constructor function initialize the Serial Port Library > + > + @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS. > + > +**/ > +EFI_STATUS > +EFIAPI > +DxeRuntimeDebugLibSerialPortConstructor ( > + IN EFI_HANDLEImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUSStatus; > + > + Status = SerialPortInitialize (); > + if (EFI_ERROR (Status)) { > +return Status; > + } > + > + return SystemTable->BootServices->CreateEventEx ( > + EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + RuntimeLibExitBootServicesEvent, > + NULL, > + &gEfiEventExitBootServicesGuid, > + &mEfiExitBootServicesEvent); > +} > + > +/** > + Prints a debug message to the debug output device if the specified error > level > + is enabled. > + > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function > + GetDebugPrintErrorLevel (), then print the message specified by Format and > the > + associated variable argument list to the debug output device. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevel The error level of the debug message. > + @param Format Format string for the debug message to print. > + @param ... Variable argument list whose contents are accessed > + based on the format string specified by Format. > + > +**/ > +VOID > +EFIAPI > +DebugPrint ( > + IN UINTNErrorLevel, > + IN CONST CHAR8 *Format, > + ... > + ) > +{ > + CHAR8Buffer[MAX_DEBUG_MESSAGE_LENGTH]; > + VA_LIST Marker; > + > + if (mEfiAtRuntime) { > +return; > + } > + > + // > + // Check driver debug mask value and global mask > + // > + if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) { > +return; > + } > + > + // > + // Convert the DEBUG() message to an ASCII String > + // > + VA_START (Marker, Format); > + AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker); > + VA_END (Marker); > + > + // > + // Send the print string to a Serial
Re: [edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
> On Feb 20, 2018, at 6:16 AM, Leif Lindholm wrote: > > On Tue, Feb 20, 2018 at 11:05:22AM +, Ard Biesheuvel wrote: >> +/** >> + Prints an assert message containing a filename, line number, and >> description. >> + This may be followed by a breakpoint or a dead loop. >> + >> + Print a message of the form "ASSERT (): >> \n" >> + to the debug output device. If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED >> bit >> + of PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, >> if >> + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is set >> then >> + CpuDeadLoop() is called. If neither of these bits are set, then this >> function >> + returns immediately after the message is printed to the debug output >> device. >> + DebugAssert() must actively prevent recursion. If DebugAssert() is called >> + while processing another DebugAssert(), then DebugAssert() must return >> + immediately. >> + >> + If FileName is NULL, then a string of "(NULL) Filename" is >> printed. >> + If Description is NULL, then a string of "(NULL) >> Description" is >> + printed. >> + >> + @param FileName The pointer to the name of the source file that >> generated >> + the assert condition. >> + @param LineNumber The line number in the source file that generated the >> + assert condition >> + @param Description The pointer to the description of the assert >> condition. >> + >> +**/ >> +VOID >> +EFIAPI >> +DebugAssert ( >> + IN CONST CHAR8 *FileName, >> + IN UINTNLineNumber, >> + IN CONST CHAR8 *Description >> + ) >> +{ >> + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; >> + >> + if (!mEfiAtRuntime) { >> +// >> +// Generate the ASSERT() message in Ascii format >> +// >> +AsciiSPrint (Buffer, sizeof (Buffer), "ASSERT [%a] %a(%d): %a\n", >> + gEfiCallerBaseName, FileName, LineNumber, Description); >> + >> +// >> +// Send the print string to the Console Output device >> +// >> +SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer)); >> + } >> + >> + // >> + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings >> + // >> + if ((FixedPcdGet8 (PcdDebugPropertyMask) & >> + DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { >> +CpuBreakpoint (); >> + } else if ((FixedPcdGet8 (PcdDebugPropertyMask) & >> + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { >> +CpuDeadLoop (); >> + } > > Hmm ... I know this does not fundamentally change the behaviour of the > existing implementation, but if we're looking to improve runtime > behaviour, we've just gone from generating a runtime fault to silently > freezing (if BREAKPOINT_ENABLED or DEADLOOP_ENABLED). > > What do breakpoint/deadloop mean in a runtime context anyway - do we > not need to halt _all_ running cores? > > I don't see an obvious "right way" solution here, and this only > affects DEBUG builds. Leif, It is not related to DEBUG builds, it is related to PCD configuration. > Possible ways of handling this that I can think > of include: > - Don't respect BREAKPOINT/DEADLOOP if at runtime. > - Respect BREAKPOINT/DEADLOOP and disable all cores. > - Take ownership back of the system and re-enable 1:1 mapping so > messages can be printed. > There is not much risk of losing user data if you "panic" EFI, that is not true if you are going to "panic" the OS. I've seen more bugs at runtime from confusion about what is legal to do at runtime, vs. actual bugs. You can always just return device error on failure, and if the RT Services hangs you can core dump the OS. Given the OS provided the virtual mapping it is likely the stuck EFI could would be in the stack trace of the panic. Thanks, Andrew Fish > / >Leif > ___ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
Hi, On 02/15/18 16:14, Haojian Zhuang wrote: > Add four boot options in emu variable region. They are > "Boot on SD", "Grub", "Android Boot" and "Android Fastboot". > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Haojian Zhuang > --- > Platform/Hisilicon/HiKey960/HiKey960.dsc | 3 + > Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 > ++- > 2 files changed, 242 insertions(+), 2 deletions(-) > > diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc > b/Platform/Hisilicon/HiKey960/HiKey960.dsc > index 98289c0..a6864c1 100644 > --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc > +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc > @@ -78,6 +78,9 @@ >gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE > > [PcdsFixedAtBuild.common] > + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x1AD88048 > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8 > + >gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4 > >gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha" > diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf > b/Platform/Hisilicon/HiKey960/HiKey960.fdf > index 655032a..af10430 100644 > --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf > +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf > @@ -26,12 +26,12 @@ > > [FD.BL33_AP_UEFI] > BaseAddress = 0x1AC98000|gArmTokenSpaceGuid.PcdFdBaseAddress # The base > address of the Firmware in NOR Flash. > -Size = 0x000F|gArmTokenSpaceGuid.PcdFdSize # The size > in bytes of the FLASH Device > +Size = 0x0010|gArmTokenSpaceGuid.PcdFdSize # The size > in bytes of the FLASH Device > ErasePolarity = 1 > > # This one is tricky, it must be: BlockSize * NumBlocks = Size > BlockSize = 0x1000 > -NumBlocks = 0xF0 > +NumBlocks = 0x100 > > > > # > @@ -53,6 +53,243 @@ NumBlocks = 0xF0 > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize > FV = FVMAIN_COMPACT > > +0x000F|0x8000 > +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > +DATA = { > + ## This is the EFI_FIRMWARE_VOLUME_HEADER > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + # FileSystemGuid: gEfiSystemNvDataFvGuid = > + 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C, > + 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50, > + # FvLength: 0x8000 > + 0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00, > + #Signature "_FVH" #Attributes > + 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00, > + #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision > + 0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02, > + #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block > + 0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, > + #Blockmap[1]: End > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + ## end of EFI_FIRMWARE_VOLUME_HEADER. > + > + ### Offset (0x48) ### > + ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid > + 0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41, > + 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d, > + #Size: 0x8000 > (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size > of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8 > + 0xB8, 0x7F, 0x00, 0x00, > + #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32 > + 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + > + ### Offset (0x64) ### > + ## This is the VARIABLE_HEADER > + #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved > + 0xAA, 0x55, 0x3F, 0x00, > + #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS > + 0x03, 0x00, 0x00, 0x00, > + #NameSize: > + 0x14, 0x00, 0x00, 0x00, > + #DataSize: > + 0x18, 0x00, 0x00, 0x00, > + #VariableGuid: gEfiGlobalVariableGuid > + 0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11, > + 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C, > + ## End of VARIABLE_HEADER. Offset (0x84) > + #VariableName: BootOrder > + 0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00, > + 0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00, > + 0x72, 0x00, 0x00, 0x00, > + #Data > + 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00, > + 0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00, > + 0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00, > + ### End of BootOrder. > + > + ### Offset (0xB0) ### > + ## This is the VARIABLE_HEADER > + #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved > + 0xAA, 0x55, 0x3F, 0x00, > + #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS > + 0x03, 0x00, 0x00, 0x00, > + #NameSize: > + 0x12, 0x00, 0x00, 0x00, > + #DataSize: > + 0x42, 0x00, 0x00, 0x00, > + #VariableGuid: gEfiGlobalVariableGuid > + 0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11, > + 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C, > + #Va
Re: [edk2] [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers
On Tue, Feb 20, 2018 at 11:05:24AM +, Ard Biesheuvel wrote: > BaseDebugLibSerialPort is not suitable for use by DXE_RUNTIME_DRIVER > modules, so blacklist it for use by such modules. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > index 823511b22f6b..25da1fb9363a 100644 > --- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > +++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > @@ -21,7 +21,7 @@ [Defines] >FILE_GUID = BB83F95F-EDBC-4884-A520-CD42AF388FAE >MODULE_TYPE= BASE >VERSION_STRING = 1.0 > - LIBRARY_CLASS = DebugLib > + LIBRARY_CLASS = DebugLib|SEC PEI_CORE PEIM DXE_CORE > DXE_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION > SMM_CORE MM_STANDALONE MM_CORE_STANDALONE Not a comment on this patch as such (which looks sensible to me), but what you're doing here isn't blacklisting DXE_RUNTIME_DRIVER, but rather whitelisting every other module type. Could we use a ! operator added to the syntax? / Leif >CONSTRUCTOR= BaseDebugLibSerialPortConstructor > > # > -- > 2.11.0 > ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
On Tue, Feb 20, 2018 at 11:05:22AM +, Ard Biesheuvel wrote: > +/** > + Prints an assert message containing a filename, line number, and > description. > + This may be followed by a breakpoint or a dead loop. > + > + Print a message of the form "ASSERT (): > \n" > + to the debug output device. If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED > bit > + of PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, if > + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is set > then > + CpuDeadLoop() is called. If neither of these bits are set, then this > function > + returns immediately after the message is printed to the debug output > device. > + DebugAssert() must actively prevent recursion. If DebugAssert() is called > + while processing another DebugAssert(), then DebugAssert() must return > + immediately. > + > + If FileName is NULL, then a string of "(NULL) Filename" is > printed. > + If Description is NULL, then a string of "(NULL) > Description" is > + printed. > + > + @param FileName The pointer to the name of the source file that > generated > + the assert condition. > + @param LineNumber The line number in the source file that generated the > + assert condition > + @param Description The pointer to the description of the assert > condition. > + > +**/ > +VOID > +EFIAPI > +DebugAssert ( > + IN CONST CHAR8 *FileName, > + IN UINTNLineNumber, > + IN CONST CHAR8 *Description > + ) > +{ > + CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; > + > + if (!mEfiAtRuntime) { > +// > +// Generate the ASSERT() message in Ascii format > +// > +AsciiSPrint (Buffer, sizeof (Buffer), "ASSERT [%a] %a(%d): %a\n", > + gEfiCallerBaseName, FileName, LineNumber, Description); > + > +// > +// Send the print string to the Console Output device > +// > +SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer)); > + } > + > + // > + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings > + // > + if ((FixedPcdGet8 (PcdDebugPropertyMask) & > + DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > +CpuBreakpoint (); > + } else if ((FixedPcdGet8 (PcdDebugPropertyMask) & > + DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > +CpuDeadLoop (); > + } Hmm ... I know this does not fundamentally change the behaviour of the existing implementation, but if we're looking to improve runtime behaviour, we've just gone from generating a runtime fault to silently freezing (if BREAKPOINT_ENABLED or DEADLOOP_ENABLED). What do breakpoint/deadloop mean in a runtime context anyway - do we not need to halt _all_ running cores? I don't see an obvious "right way" solution here, and this only affects DEBUG builds. Possible ways of handling this that I can think of include: - Don't respect BREAKPOINT/DEADLOOP if at runtime. - Respect BREAKPOINT/DEADLOOP and disable all cores. - Take ownership back of the system and re-enable 1:1 mapping so messages can be printed. / Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers
BaseDebugLibSerialPort is not suitable for use by DXE_RUNTIME_DRIVER modules, so blacklist it for use by such modules. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf index 823511b22f6b..25da1fb9363a 100644 --- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf +++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf @@ -21,7 +21,7 @@ [Defines] FILE_GUID = BB83F95F-EDBC-4884-A520-CD42AF388FAE MODULE_TYPE= BASE VERSION_STRING = 1.0 - LIBRARY_CLASS = DebugLib + LIBRARY_CLASS = DebugLib|SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION SMM_CORE MM_STANDALONE MM_CORE_STANDALONE CONSTRUCTOR= BaseDebugLibSerialPortConstructor # -- 2.11.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/3] Create UART DebugLib implementation for runtime drivers
Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message") broke the DEBUG build for systems using a MMIO mapped UART for DEBUG output. Instead of patching it up locally, let's fix this issue once and for all, by creating a UART DebugLib implementation that does the right thing by default, and blacklisting the BASE version for use by DXE_RUNTIME_DRIVER modules. Ard Biesheuvel (3): MdePkg: introduce DxeRuntimeDebugLibSerialPort ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers ArmVirtPkg/ArmVirt.dsc.inc | 3 + MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf | 2 +- MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c | 342 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf | 46 +++ MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni | 21 ++ 5 files changed, 413 insertions(+), 1 deletion(-) create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni -- 2.11.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
Introduce a variant of BaseDebugLibSerialPort that behaves correctly wrt to use of the serial port after ExitBootServices(). Also, it uses fixed PCDs for all the parameterized values so that no calls into PcdLib are made at runtime. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c | 342 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf | 46 +++ MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni | 21 ++ 3 files changed, 409 insertions(+) diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c new file mode 100644 index ..d18267d91322 --- /dev/null +++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c @@ -0,0 +1,342 @@ +/** @file + DXE runtime Debug library instance based on Serial Port library. + It uses PrintLib to send debug messages to serial port device. + + NOTE: If the Serial Port library enables hardware flow control, then a call + to DebugPrint() or DebugAssert() may hang if writes to the serial port are + being blocked. This may occur if a key(s) are pressed in a terminal emulator + used to monitor the DEBUG() and ASSERT() messages. + + Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved. + Copyright (c) 2018, Linaro, Ltd. All rights reserved. + + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD License + which accompanies this distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php. + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include + +STATIC EFI_EVENT mEfiExitBootServicesEvent; +STATIC BOOLEANmEfiAtRuntime; + +// +// Define the maximum debug and assert message length that this library supports +// +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 + +/** + Set AtRuntime flag as TRUE after ExitBootServices. + + @param[in] Event The Event that is being processed. + @param[in] Context The Event Context. + +**/ +STATIC +VOID +EFIAPI +RuntimeLibExitBootServicesEvent ( + IN EFI_EVENTEvent, + IN VOID *Context + ) +{ + mEfiAtRuntime = TRUE; +} + +/** + The constructor function initialize the Serial Port Library + + @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS. + +**/ +EFI_STATUS +EFIAPI +DxeRuntimeDebugLibSerialPortConstructor ( + IN EFI_HANDLEImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + EFI_STATUSStatus; + + Status = SerialPortInitialize (); + if (EFI_ERROR (Status)) { +return Status; + } + + return SystemTable->BootServices->CreateEventEx ( + EVT_NOTIFY_SIGNAL, + TPL_NOTIFY, + RuntimeLibExitBootServicesEvent, + NULL, + &gEfiEventExitBootServicesGuid, + &mEfiExitBootServicesEvent); +} + +/** + Prints a debug message to the debug output device if the specified error level + is enabled. + + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function + GetDebugPrintErrorLevel (), then print the message specified by Format and the + associated variable argument list to the debug output device. + + If Format is NULL, then ASSERT(). + + @param ErrorLevel The error level of the debug message. + @param Format Format string for the debug message to print. + @param ... Variable argument list whose contents are accessed + based on the format string specified by Format. + +**/ +VOID +EFIAPI +DebugPrint ( + IN UINTNErrorLevel, + IN CONST CHAR8 *Format, + ... + ) +{ + CHAR8Buffer[MAX_DEBUG_MESSAGE_LENGTH]; + VA_LIST Marker; + + if (mEfiAtRuntime) { +return; + } + + // + // Check driver debug mask value and global mask + // + if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) { +return; + } + + // + // Convert the DEBUG() message to an ASCII String + // + VA_START (Marker, Format); + AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker); + VA_END (Marker); + + // + // Send the print string to a Serial Port + // + SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer)); +} + + +/** + Prints an assert message containing a filename, line number, and description. + This may be followed by a breakpoint or a dead loop. + + Print a message of the form "ASSERT (): \n" + to the debug output device. If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit + of PcdDebugProp
[edk2] [PATCH 2/3] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate
Switch all users of ArmVirt.dsc.inc to the new DebugLib implementation that was created especially for DXE_RUNTIME_DRIVER modules, ensuring that DEBUG() calls do not touch the UART at runtime. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- ArmVirtPkg/ArmVirt.dsc.inc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 0cb48f08e9bf..cde514958da2 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -231,6 +231,9 @@ [LibraryClasses.common.UEFI_DRIVER] [LibraryClasses.common.DXE_RUNTIME_DRIVER] MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf +!if $(TARGET) != RELEASE + DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf +!endif !if $(SECURE_BOOT_ENABLE) == TRUE BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf -- 2.11.0 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
On 02/20/18 00:30, Andrew Fish wrote: > > >> On Feb 19, 2018, at 11:23 AM, Laszlo Ersek wrote: >> >> On 02/19/18 19:59, Ard Biesheuvel wrote: >>> On 9 February 2018 at 04:16, Ruiyu Ni wrote: + DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth)); + >>> >>> This DEBUG() print is breaking system reset from the Linux OS at >>> runtime in DEBUG builds. >>> >>> [4.223704] reboot: Restarting system >>> [4.224733] Unable to handle kernel paging request at virtual >>> address 0918 >>> >>> This is the boottime MMIO address of the UART on the QEMU mach-virt >>> model, and no runtime mapping exists for it, resulting in the crash. >>> >>> Please ensure that DEBUG () is used with care in DXE_RUNTIME_DRIVER >>> modules. >> >> Not disagreeing, just asking: should we perhaps take care of this in >> a new DebugLib instance, specifically for DXE runtime drivers? >> >> "MdePkg/Library/UefiRuntimeLib" provides functions like >> EfiAtRuntime() and EfiGoneVirtual(). We couldn't use UefiRuntimeLib >> in DebugLib, because UefiRuntimeLib already depends on DebugLib (we >> can't introduce a circular dependency). But, we could reimplement >> EfiAtRuntime() manually, in order to silence all debug messages after >> ExitBootServices(). >> >> This would make sense also because after ExitBootServices(), the >> serial port is considered "owned" by the boot loader or the OS, and >> the firmware should likely not mess up whatever IO occurs there. >> >> I guess the two possible places to implement such runtime logic would >> be: >> >> - in a RuntimeDxe clone of BaseDebugLibSerialPort (i.e., commonly for >> all edk2 platforms), >> >> - in a RuntimeDxe clone of >> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf" >> (i.e., move the checking to the serial port lib level). >> >> (This is different from OVMF / x86, because (a) there the debug data >> are written to IO port 0x402, and the IO address space does not >> depend on paging, (b) largely, no boot loader or OS ever are aware of >> the QEMU debug port, it can be considered as owned by the firmware, >> always.) >> >> Just thinking out loud. >> > > Laszlo, > > From a Pedantic point of view an EFI Runtime Service can only use > hardware not exposed to OS as there is no clean way to share. There > are some scary wiggle words about the RTC that date all the way back > to EFI 1.1, and that is the only conformant exception. So that is > probably why we don't have a generic solution as it is kind of > dangerous. I think a DebugLib instance located at MdePkg/Library/DxeRuntimeDebugLibSerialPort could be general enough, since it would not share hardware with the OS -- it would stop runtime DXE drivers from making SerialPortLib calls. > For example what happens if the OS has a kernel debugger running on > that serial port and EFI Spews DEBUG prints, that would probably not > come out well. > > For things I've written I usually end up writing a macro that does > something like: > > if (!EfiAtRuntime ()) { > DEBUG ((DEBUG_ERROR, "Hello World!")); > } Right, so this supports Ard's original idea, namely that we should disable the DEBUGs in the client code, one way or another... > and > > if (!EfiAtRuntime ()) { > ASSERT (FALSE); > } ... On the other hand, I think only the debug message should be suppressed for ASSERTs; the exception or deadloop (whatever the assert disposition) should not be suppressed at runtime. If the firmware encounters a fatal unexpected error, it's better to hang the system (with the deadloop) or crash it (raise an exception and make the kernel panic) than silently corrupt more state and pretend everything's fine. So wrapping "ASSERT (Predicate)" with "if (!EfiAtRuntime ())" does not seem like the best solution to me. > Maybe it would possible to add a RUNTIME_DEBUG(), RUNTIME_ASSERT(), > etc. macros to the UefiRuntimeLib? > Makes me remember the story from back in the 1990's about and update > to the Windows Plug-N-Play subsystem to auto magically detect modems. > It worked great, and made it easier to get folks online (even if it > was very slow), but seems a software update managed to destroy a very > very expensive custom milling machine. It seems this milling machine > was connected to the serial port of the PC, and it was a very dumb > device as it interpreted data across the serial port as coordinates > and commands, and all the error checking was done on the PC. So these > random data on the serial line told the milling machine to attack its > self. A "winmodem" on steroids :) Thanks! Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel