[edk2] [Patch] BaseTools: Fix the bug to display the single SKUID info

2018-02-20 Thread Yonghong Zhu
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

2018-02-20 Thread Kinney, Michael D
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

2018-02-20 Thread Kinney, Michael D
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Leif Lindholm
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

2018-02-20 Thread Laszlo Ersek
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

2018-02-20 Thread Laszlo Ersek
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

2018-02-20 Thread Laszlo Ersek
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

2018-02-20 Thread Andrew Fish


> 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

2018-02-20 Thread Laszlo Ersek
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

2018-02-20 Thread Leif Lindholm
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

2018-02-20 Thread Leif Lindholm
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Ard Biesheuvel
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

2018-02-20 Thread Laszlo Ersek
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