[edk2] [platforms: PATCH 02/12] Marvell/Library: ArmadaBoardDescLib: Add GPIO information

2018-10-19 Thread Marcin Wojtas
This patch extends library with GPIO devices per-board
description. Both embedded SoC controllers and
I2C IO expanders are supported. Add a helper routine
for obtaining information about the latter.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h | 23 
 1 file changed, 23 insertions(+)

diff --git a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
index ee8e06e..109164c 100644
--- a/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h
@@ -25,6 +25,29 @@ typedef struct {
 } MV_BOARD_COMPHY_DESC;
 
 //
+// GPIO devices per-board description
+//
+typedef struct {
+  UINTN ChipId;
+  UINTN I2cAddress;
+  UINTN I2cBus;
+} MV_I2C_IO_EXPANDER_DESC;
+
+typedef struct {
+  MV_SOC_GPIO_DESC*SoC;
+  UINTNGpioDevCount;
+  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
+  UINTNI2cIoExpanderCount;
+} MV_BOARD_GPIO_DESC;
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescGpioGet (
+  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
+  IN OUT UINTN*I2cIoExpanderCount
+  );
+
+//
 // I2C devices per-board description
 //
 typedef struct {
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 09/12] Marvell/Drivers: I2c: Use common header for macros

2018-10-19 Thread Marcin Wojtas
Hitherto I2c solution used same macros, defined in multiple
places. Move them to a new common header.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h | 10 ---
 Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h   | 17 ++-
 Silicon/Marvell/Include/Protocol/MvI2c.h  | 31 
 Silicon/Marvell/Applications/EepromCmd/EepromCmd.c|  5 +---
 Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.c |  3 +-
 Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   |  4 +--
 6 files changed, 37 insertions(+), 33 deletions(-)
 create mode 100644 Silicon/Marvell/Include/Protocol/MvI2c.h

diff --git a/Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h 
b/Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h
index b1af645..c32ee48 100644
--- a/Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h
+++ b/Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h
@@ -41,16 +41,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #define MAX_BUFFER_LENGTH 64
 
-/*
- * I2C_FLAG_NORESTART is not part of PI spec, it allows to continue
- * transmission without repeated start operation.
- * FIXME: This flag is also defined in Drivers/I2c/MvI2cDxe/MvI2cDxe.h
- * and it's important to have both version synced. This solution is
- * temporary and shared flag should be used by both files.
- * Situation is analogous with I2C_GUID, which also should be common, but is
- * for now defined same way in two header files.
- */
-#define I2C_FLAG_NORESTART 0x0002
 #define I2C_GUID \
   { \
   0xadc1901b, 0xb83c, 0x4831, { 0x8f, 0x59, 0x70, 0x89, 0x8f, 0x26, 0x57, 0x1e 
} \
diff --git a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h 
b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
index 3c9beaf..6850c34 100644
--- a/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
+++ b/Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
@@ -32,8 +32,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
***/
 
-#ifndef __MV_I2C_H__
-#define __MV_I2C_H__
+#ifndef __MV_I2C_DXE_H__
+#define __MV_I2C_DXE_H__
 
 #include 
 
@@ -75,17 +75,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define I2C_FAST   0x2
 #define I2C_FASTEST0x3
 
-/*
- * I2C_FLAG_NORESTART is not part of PI spec, it allows to continue
- * transmission without repeated start operation.
- * FIXME: This flag is also defined in
- * Platforms/Marvell/Include/Protocol/Eeprom.h and it's important to have both
- * version synced. This solution is temporary and shared flag should be used by
- * both files.
- * Situation is analogous with I2C_GUID, which also should be common, but is
- * for now defined same way in two header files.
- */
-#define I2C_FLAG_NORESTART 0x0002
 #define I2C_GUID \
   { \
   0xadc1901b, 0xb83c, 0x4831, { 0x8f, 0x59, 0x70, 0x89, 0x8f, 0x26, 0x57, 0x1e 
} \
@@ -266,4 +255,4 @@ MvI2cEnableConf (
   IN EFI_STATUS  *I2cStatus OPTIONAL
   );
 
-#endif // __MV_I2C_H__
+#endif // __MV_I2C_DXE_H__
diff --git a/Silicon/Marvell/Include/Protocol/MvI2c.h 
b/Silicon/Marvell/Include/Protocol/MvI2c.h
new file mode 100644
index 000..d8e644e
--- /dev/null
+++ b/Silicon/Marvell/Include/Protocol/MvI2c.h
@@ -0,0 +1,31 @@
+/**
+*
+*  Copyright (c) 2018, Marvell International 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 __MV_I2C_H__
+#define __MV_I2C_H__
+
+/*
+ * I2C_FLAG_NORESTART is not part of PI spec, it allows to continue
+ * transmission without repeated start operation.
+ */
+#define I2C_FLAG_NORESTART 0x0002
+
+/*
+ * Helper macros for maintaining multiple I2C buses
+ * and devices defined via EFI_I2C_DEVICE.
+ */
+#define I2C_DEVICE_ADDRESS(Index)  ((Index) & MAX_UINT16)
+#define I2C_DEVICE_BUS(Index)  ((Index) >> 16)
+#define I2C_DEVICE_INDEX(Bus, Address) (((Address) & MAX_UINT16) | (Bus) << 16)
+
+#endif
diff --git a/Silicon/Marvell/Applications/EepromCmd/EepromCmd.c 
b/Silicon/Marvell/Applications/EepromCmd/EepromCmd.c
index f43e411..a390f23 100644
--- a/Silicon/Marvell/Applications/EepromCmd/EepromCmd.c
+++ b/Silicon/Marvell/Applications/EepromCmd/EepromCmd.c
@@ -52,10 +52,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include 
 
 #include 
-
-#define I2C_DEVICE_INDEX(bus, address) (((address) & 0x) | (bus) << 16)
-#define I2C_DEVICE_ADDRESS(index) ((index) & 0x)
-#define 

[edk2] [platforms: PATCH 11/12] Marvell/Armada7k8k: Enable GPIO drivers compilation

2018-10-19 Thread Marcin Wojtas
Enable building new GPIO drivers before adding VBUS
pins handling. Update relevant boards .dsc files with
IO expander information.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 ++
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc| 4 ++--
 Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf.inc| 2 ++
 Platform/Marvell/Armada80x0Db/Armada80x0Db.fdf.inc| 2 ++
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.fdf.inc | 2 ++
 5 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index d4c67a2..62a46a6 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -456,6 +456,8 @@
   Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
 
   # Platform drivers
+  Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf
+  Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
   Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
   MdeModulePkg/Bus/I2c/I2cDxe/I2cDxe.inf
   Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.inf
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index a935f36..31815e4 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -89,8 +89,8 @@
   gMarvellTokenSpaceGuid.PcdChip1MppSel6|{ 0xE, 0xE, 0xE, 0x0, 0x0, 0x0, 0x0, 
0x0, 0x0, 0x0 }
 
   # I2C
-  gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60 }
-  gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0 }
+  gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60, 0x21 }
+  gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x1 }
   gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57 }
   gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0 }
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf.inc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf.inc
index b7e7a65..7129606 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf.inc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf.inc
@@ -12,6 +12,8 @@
 
 # Per-board additional content of the DXE phase firmware volume
 
+  INF Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
+
   # DTB
   INF RuleOverride = DTB Silicon/Marvell/Armada7k8k/DeviceTree/Armada70x0Db.inf
 
diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.fdf.inc 
b/Platform/Marvell/Armada80x0Db/Armada80x0Db.fdf.inc
index 81a81d0..f2fcc55 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.fdf.inc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.fdf.inc
@@ -12,6 +12,8 @@
 
 # Per-board additional content of the DXE phase firmware volume
 
+  INF Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
+
   # DTB
   INF RuleOverride = DTB Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0Db.inf
 
diff --git a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.fdf.inc 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.fdf.inc
index 326da2e..254fcee 100644
--- a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.fdf.inc
+++ b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.fdf.inc
@@ -12,6 +12,8 @@
 
 # Per-board additional content of the DXE phase firmware volume
 
+  INF Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf
+
   # DTB
   INF RuleOverride = DTB 
Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0McBin.inf
 
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 03/12] SolidRun/Armada80x0McBin: Introduce board description library

2018-10-19 Thread Marcin Wojtas
This patch implements ArmadaBoarDescLib library for
Armada80x0McBin comunity board and introduces ArmadaBoardDescGpioGet
routine with per-board I2C IO expander description.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc  
   |  3 ++
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 | 34 ++
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
   | 36 
 3 files changed, 73 insertions(+)
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c

diff --git a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
index 52e2b9b..077224d 100644
--- a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
+++ b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
@@ -55,6 +55,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
new file mode 100644
index 000..63a4f66
--- /dev/null
+++ 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  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]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = ArmadaMcBinBoardDescLib
+  FILE_GUID  = 8208558f-5f33-46e2-b5c5-43354384389e
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada80x0McBinBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
new file mode 100644
index 000..979db11
--- /dev/null
+++ 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c
@@ -0,0 +1,36 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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 
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescGpioGet (
+  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
+  IN OUT UINTN*I2cIoExpanderCount
+  )
+{
+  /* No I2C IO expanders on board */
+  *I2cIoExpanderDesc = NULL;
+  *I2cIoExpanderCount = 0;
+
+  return EFI_SUCCESS;
+}
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 06/12] Marvell/Drivers: MvBoardDesc: Extend protocol with GPIO support

2018-10-19 Thread Marcin Wojtas
Introduce new callback that can provide information
about GPIO SoC controllers, as well as on-board
I2C IO expanders. According ArmadaSoCDescLib
ArmadaBoardDescLib routines are used for
obtaining required data.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf |  1 +
 Silicon/Marvell/Include/Protocol/BoardDesc.h |  8 
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c   | 48 
 3 files changed, 57 insertions(+)

diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf 
b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
index 41f72d6..0b93948 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
@@ -47,6 +47,7 @@
   Silicon/Marvell/Marvell.dec
 
 [LibraryClasses]
+  ArmadaBoardDescLib
   ArmadaSoCDescLib
   DebugLib
   MemoryAllocationLib
diff --git a/Silicon/Marvell/Include/Protocol/BoardDesc.h 
b/Silicon/Marvell/Include/Protocol/BoardDesc.h
index 1d57a16..e06d689 100644
--- a/Silicon/Marvell/Include/Protocol/BoardDesc.h
+++ b/Silicon/Marvell/Include/Protocol/BoardDesc.h
@@ -50,6 +50,13 @@ EFI_STATUS
 
 typedef
 EFI_STATUS
+(EFIAPI *MV_BOARD_DESC_GPIO_GET) (
+  IN MARVELL_BOARD_DESC_PROTOCOL  *This,
+  IN OUT MV_BOARD_GPIO_DESC  **GpioDesc
+  );
+
+typedef
+EFI_STATUS
 (EFIAPI *MV_BOARD_DESC_I2C_GET) (
   IN MARVELL_BOARD_DESC_PROTOCOL  *This,
   IN OUT MV_BOARD_I2C_DESC   **I2cDesc
@@ -106,6 +113,7 @@ VOID
 struct _MARVELL_BOARD_DESC_PROTOCOL {
   MV_BOARD_DESC_AHCI_GET BoardDescAhciGet;
   MV_BOARD_DESC_COMPHY_GET   BoardDescComPhyGet;
+  MV_BOARD_DESC_GPIO_GET BoardDescGpioGet;
   MV_BOARD_DESC_I2C_GET  BoardDescI2cGet;
   MV_BOARD_DESC_MDIO_GET BoardDescMdioGet;
   MV_BOARD_DESC_PP2_GET  BoardDescPp2Get;
diff --git a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c 
b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
index 39dc06c..948a6a0 100644
--- a/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
+++ b/Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c
@@ -100,6 +100,53 @@ MvBoardDescComPhyGet (
 
 STATIC
 EFI_STATUS
+MvBoardDescGpioGet (
+  IN MARVELL_BOARD_DESC_PROTOCOL  *This,
+  IN OUT MV_BOARD_GPIO_DESC  **GpioDesc
+  )
+{
+  MV_I2C_IO_EXPANDER_DESC *I2cIoExpanderDesc;
+  UINTN SoCGpioCount, I2cIoExpanderCount;
+  STATIC MV_BOARD_GPIO_DESC *BoardDesc;
+  MV_SOC_GPIO_DESC *SoCDesc;
+  EFI_STATUS Status;
+
+  if (BoardDesc != NULL) {
+*GpioDesc = BoardDesc;
+return EFI_SUCCESS;
+  }
+
+  /* Get SoC data about all available GPIO controllers */
+  Status = ArmadaSoCDescGpioGet (, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  /* Get per-board information about all available I2C IO expanders */
+  Status = ArmadaBoardDescGpioGet (, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  /* Allocate and fill board description */
+  BoardDesc = AllocateZeroPool (sizeof (MV_BOARD_GPIO_DESC));
+  if (BoardDesc == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  BoardDesc->SoC = SoCDesc;
+  BoardDesc->GpioDevCount = SoCGpioCount;
+  BoardDesc->I2cIoExpanderDesc = I2cIoExpanderDesc;
+  BoardDesc->I2cIoExpanderCount = I2cIoExpanderCount;
+
+  *GpioDesc = BoardDesc;
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
 MvBoardDescI2cGet (
   IN MARVELL_BOARD_DESC_PROTOCOL  *This,
   IN OUT MV_BOARD_I2C_DESC   **I2cDesc
@@ -556,6 +603,7 @@ MvBoardDescInitProtocol (
 {
   BoardDescProtocol->BoardDescAhciGet = MvBoardDescAhciGet;
   BoardDescProtocol->BoardDescComPhyGet = MvBoardDescComPhyGet;
+  BoardDescProtocol->BoardDescGpioGet = MvBoardDescGpioGet;
   BoardDescProtocol->BoardDescI2cGet = MvBoardDescI2cGet;
   BoardDescProtocol->BoardDescMdioGet = MvBoardDescMdioGet;
   BoardDescProtocol->BoardDescPp2Get = MvBoardDescPp2Get;
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 04/12] Marvell/Armada70x0Db: Introduce board description library

2018-10-19 Thread Marcin Wojtas
This patch implements ArmadaBoarDescLib library for
Armada70x0Db and introduces ArmadaBoardDescGpioGet
routine with per-board I2C IO expander description.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
 |  3 ++
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 | 34 
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
   | 43 
 3 files changed, 80 insertions(+)
 create mode 100644 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 create mode 100644 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index e0bf447..a935f36 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -54,6 +54,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
new file mode 100644
index 000..b26f55b
--- /dev/null
+++ 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  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]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Armada70x0DbBoardDescLib
+  FILE_GUID  = 3164c8d9-19d4-4ad6-8196-cea094b1ddf1
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada70x0DbBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
new file mode 100644
index 000..51e6687
--- /dev/null
+++ 
b/Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
@@ -0,0 +1,43 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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 MV_I2C_IO_EXPANDER_DESC mI2cIoExpanderDescTemplate = {
+  PCA9555_ID,
+  0x21,
+  0x0,
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescGpioGet (
+  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
+  IN OUT UINTN*I2cIoExpanderCount
+  )
+{
+  *I2cIoExpanderCount = 1;
+  *I2cIoExpanderDesc = 
+
+  return EFI_SUCCESS;
+}
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 05/12] Marvell/Armada80x0Db: Introduce board description library

2018-10-19 Thread Marcin Wojtas
This patch implements ArmadaBoarDescLib library for
Armada80x0Db and introduces ArmadaBoardDescGpioGet
routine with per-board I2C IO expander description.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
 |  3 ++
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 | 34 +
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
   | 50 
 3 files changed, 87 insertions(+)
 create mode 100644 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 create mode 100644 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c

diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
index 92e2dc8..42f7bd3 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
@@ -54,6 +54,9 @@
 [Components.AARCH64]
   Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db.inf
 
+[LibraryClasses.common]
+  
ArmadaBoardDescLib|Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
diff --git 
a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
new file mode 100644
index 000..2d39d96
--- /dev/null
+++ 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
@@ -0,0 +1,34 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  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]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = Armada80x0DbBoardDescLib
+  FILE_GUID  = fee9e874-1481-4b4f-9882-966bd0d1310f
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaBoardDescLib
+
+[Sources]
+  Armada80x0DbBoardDescLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  IoLib
diff --git 
a/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
new file mode 100644
index 000..16561bc
--- /dev/null
+++ 
b/Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
@@ -0,0 +1,50 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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 MV_I2C_IO_EXPANDER_DESC mI2cIoExpanderDescTemplate[] = {
+  {
+PCA9555_ID,
+0x21,
+0x0,
+  },
+  {
+PCA9555_ID,
+0x25,
+0x0,
+  },
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaBoardDescGpioGet (
+  IN OUT MV_I2C_IO_EXPANDER_DESC **I2cIoExpanderDesc,
+  IN OUT UINTN*I2cIoExpanderCount
+  )
+{
+  *I2cIoExpanderCount = ARRAY_SIZE (mI2cIoExpanderDescTemplate);
+  *I2cIoExpanderDesc = mI2cIoExpanderDescTemplate;
+
+  return EFI_SUCCESS;
+}
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 12/12] Marvell/Armada7k8k: Introduce NonDiscoverable device init routines

2018-10-19 Thread Marcin Wojtas
To abstract the initialization required for non-discoverable devices, which
is often platform specific (i.e., enabling VBUS gpios for USB), introduce
a NonDiscoverableInitLib for use by the NonDiscoverable code, for which
each platform can supply its own version.

Add VBUS enabling routines for supported platforms (Armada70x0Db,
Armada80x0Db, Armada80x0McBin).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Marvell.dec
 |   1 +
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
 |   3 +
 Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
 |   3 +
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc  
 |   3 +
 
Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf 
|  47 
 
Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf 
|  48 +
 
Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
 |  48 +
 Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf  
 |   1 +
 Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h   
 |  28 +
 Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c  
 |  99 +
 Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c  
 | 113 
 
Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c
   |  73 +
 Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c
 |   7 +-
 13 files changed, 471 insertions(+), 3 deletions(-)
 create mode 100644 
Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
 create mode 100644 
Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
 create mode 100644 Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h
 create mode 100644 
Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
 create mode 100644 
Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
 create mode 100644 
Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c

diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
index 7e1c37a..20a32ef 100644
--- a/Silicon/Marvell/Marvell.dec
+++ b/Silicon/Marvell/Marvell.dec
@@ -63,6 +63,7 @@
   ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h
   ArmadaIcuLib|Include/Library/ArmadaIcuLib.h
   ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h
+  NonDiscoverableInitLib|Include/Library/NonDiscoverableInitLib.h
   SampleAtResetLib|Include/Library/SampleAtResetLib.h
 
 [Protocols]
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index 31815e4..e8cd177 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -48,6 +48,9 @@
 
 !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
 
+[LibraryClasses.common]
+  
NonDiscoverableInitLib|Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
+
 [Components.common]
   Silicon/Marvell/Armada7k8k/DeviceTree/Armada70x0Db.inf
 
diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
index 42f7bd3..8e8c2ba 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
@@ -48,6 +48,9 @@
 
 !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
 
+[LibraryClasses.common]
+  
NonDiscoverableInitLib|Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
+
 [Components.common]
   Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0Db.inf
 
diff --git a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc 
b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
index 077224d..d080136 100644
--- a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
+++ b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
@@ -49,6 +49,9 @@
 
 !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
 
+[LibraryClasses.common]
+  
NonDiscoverableInitLib|Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
+
 [Components.common]
   Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0McBin.inf
 
diff --git 
a/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
 
b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
new file mode 100644
index 000..bbe7ff8
--- /dev/null
+++ 
b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
@@ -0,0 +1,47 @@
+## @file
+#
+#  Copyright 

[edk2] [platforms: PATCH 10/12] Marvell/Drivers: MvPca95xxDxe: Introduce I2C GPIO driver

2018-10-19 Thread Marcin Wojtas
Marvell Armada 7k/8k-based platforms may use Pca95xx to extend
amount of the GPIO pins.

This patch introduces support for PCA95xxx I2C IO expander family,
which is a producer of MARVELL_GPIO_PROTOCOL, by implementing
necessary routines.

Driver is based on initial work done by Allen Yan .

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf |  44 ++
 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h   |  74 +++
 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.c   | 592 

 3 files changed, 710 insertions(+)
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.c

diff --git a/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf 
b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
new file mode 100644
index 000..3066732
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf
@@ -0,0 +1,44 @@
+## @file
+#
+#  Copyright (c) 2017, Marvell International 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]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = MvPca95xxDxe
+  FILE_GUID  = f0e405eb-8407-43b9-88e6-2f7d70715c72
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= MvPca95xxEntryPoint
+
+[Sources]
+  MvPca95xxDxe.c
+  MvPca95xxDxe.h
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  DebugLib
+  MemoryAllocationLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiDriverBindingProtocolGuid
+  gEfiI2cIoProtocolGuid
+  gMarvellBoardDescProtocolGuid
+  gMarvellGpioProtocolGuid
+
+[Depex]
+  TRUE
diff --git a/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h 
b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h
new file mode 100644
index 000..43daee0
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h
@@ -0,0 +1,74 @@
+/**
+*
+*  Copyright (c) 2018, Marvell International 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 __MV_PCA953X_H__
+#define __MV_PCA953X_H__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+
+#define PCA95XX_GPIO_SIGNATURE   SIGNATURE_32 ('I', 'O', 'E', 'X')
+
+#ifndef BIT
+#define BIT(nr)  (1 << (nr))
+#endif
+
+#define PCA95XX_INPUT_REG0x0
+#define PCA95XX_OUTPUT_REG   0x2
+#define PCA95XX_DIRECTION_REG0x6
+
+#define PCA95XX_BANK_SIZE8
+#define PCA95XX_OPERATION_COUNT  2
+#define PCA95XX_OPERATION_LENGTH 1
+
+typedef enum {
+  PCA9505_PIN_COUNT = 40,
+  PCA9534_PIN_COUNT = 8,
+  PCA9535_PIN_COUNT = 16,
+  PCA9536_PIN_COUNT = 4,
+  PCA9537_PIN_COUNT = 4,
+  PCA9538_PIN_COUNT = 8,
+  PCA9539_PIN_COUNT = 16,
+  PCA9554_PIN_COUNT = 8,
+  PCA9555_PIN_COUNT = 16,
+  PCA9556_PIN_COUNT = 16,
+  PCA9557_PIN_COUNT = 16,
+} PCA95XX_PIN_COUNT;
+
+typedef enum {
+  PCA95XX_READ,
+  PCA95XX_WRITE,
+} PCA95XX_OPERATION;
+
+typedef struct {
+  MARVELL_GPIO_PROTOCOL   GpioProtocol;
+  MV_I2C_IO_EXPANDER_DESC *ControllerDesc;
+  UINTN   ControllerCount;
+  UINTN   Signature;
+  EFI_HANDLE  ControllerHandle;
+} PCA95XX;
+
+#endif
diff --git a/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.c 
b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.c
new file mode 100644
index 000..f4a474e
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.c
@@ -0,0 +1,592 @@
+/**
+*
+*  Copyright (c) 2018, Marvell International 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
+*  

[edk2] [platforms: PATCH 07/12] Marvell/Protocol: Introduce MARVELL_GPIO_PROTOCOL

2018-10-19 Thread Marcin Wojtas
From: jinghua 

This patch introduces protocol that can be used by multiple
producers (e.g. platform driver or I2C I/O expander).
It exposes generic api for controlling GPIO pins state.
Drives are differentiated by MARVELL_GPIO_DRIVER_TYPE
field of driver's MV_GPIO_DEVICE_PATH. In order to ease
selection of the desired GPIO controller a helper
function was added - MarvellGpioGetHandle().

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Marvell.dec   |   1 +
 Silicon/Marvell/Include/Protocol/MvGpio.h | 199 
 2 files changed, 200 insertions(+)
 create mode 100644 Silicon/Marvell/Include/Protocol/MvGpio.h

diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
index 616624e..7e1c37a 100644
--- a/Silicon/Marvell/Marvell.dec
+++ b/Silicon/Marvell/Marvell.dec
@@ -215,6 +215,7 @@
 [Protocols]
   gMarvellBoardDescProtocolGuid= { 0xebed8738, 0xd4a6, 0x4001, { 
0xa9, 0xc9, 0x52, 0xb0, 0xcb, 0x7d, 0xdb, 0xf9 }}
   gMarvellEepromProtocolGuid   = { 0x71954bda, 0x60d3, 0x4ef8, { 
0x8e, 0x3c, 0x0e, 0x33, 0x9f, 0x3b, 0xc2, 0x2b }}
+  gMarvellGpioProtocolGuid = { 0x8b01a5b7, 0xc570, 0x4e97, { 
0x80, 0x95, 0x4f, 0x74, 0x2a, 0x8d, 0x7d, 0x43 }}
   gMarvellMdioProtocolGuid = { 0x40010b03, 0x5f08, 0x496a, { 
0xa2, 0x64, 0x10, 0x5e, 0x72, 0xd3, 0x71, 0xaa }}
   gMarvellPhyProtocolGuid  = { 0x32f48a43, 0x37e3, 0x4acf, { 
0x93, 0xc4, 0x3e, 0x57, 0xa7, 0xb0, 0xfb, 0xdc }}
   gMarvellSpiMasterProtocolGuid= { 0x23de66a3, 0xf666, 0x4b3e, { 
0xaa, 0xa2, 0x68, 0x9b, 0x18, 0xae, 0x2e, 0x19 }}
diff --git a/Silicon/Marvell/Include/Protocol/MvGpio.h 
b/Silicon/Marvell/Include/Protocol/MvGpio.h
new file mode 100644
index 000..a335cab
--- /dev/null
+++ b/Silicon/Marvell/Include/Protocol/MvGpio.h
@@ -0,0 +1,199 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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 __MARVELL_GPIO_PROTOCOL_H__
+#define __MARVELL_GPIO_PROTOCOL_H__
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+extern EFI_GUID gMarvellGpioProtocolGuid;
+
+typedef struct _MARVELL_GPIO_PROTOCOL MARVELL_GPIO_PROTOCOL;
+
+typedef enum {
+  GPIO_MODE_INPUT = 0x0,
+  GPIO_MODE_OUTPUT= 0x1
+} MARVELL_GPIO_MODE;
+
+typedef enum {
+  GPIO_DRIVER_TYPE_SOC_CONTROLLER = 0x0,
+  GPIO_DRIVER_TYPE_PCA95XX= 0x1
+} MARVELL_GPIO_DRIVER_TYPE;
+
+typedef enum {
+  PCA9505_ID,
+  PCA9534_ID,
+  PCA9535_ID,
+  PCA9536_ID,
+  PCA9537_ID,
+  PCA9538_ID,
+  PCA9539_ID,
+  PCA9554_ID,
+  PCA9555_ID,
+  PCA9556_ID,
+  PCA9557_ID,
+  PCA95XX_MAX_ID,
+} MARVELL_IO_EXPANDER_TYPE_PCA95XX;
+
+typedef
+EFI_STATUS
+(EFIAPI *MV_GPIO_DIRECTION_OUTPUT) (
+  IN  MARVELL_GPIO_PROTOCOL *This,
+  IN  UINTN ControllerIndex,
+  IN  UINTN GpioPin,
+  IN  BOOLEAN Value
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *MV_GPIO_DIRECTION_INPUT) (
+  IN  MARVELL_GPIO_PROTOCOL *This,
+  IN  UINTN ControllerIndex,
+  IN  UINTN GpioPin
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *MV_GPIO_GET_FUNCTION) (
+  IN  MARVELL_GPIO_PROTOCOL *This,
+  IN  UINTN ControllerIndex,
+  IN  UINTN GpioPin,
+  OUT MARVELL_GPIO_MODE *Mode
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *MV_GPIO_GET_VALUE) (
+  IN  MARVELL_GPIO_PROTOCOL *This,
+  IN  UINTN ControllerIndex,
+  IN  UINTN GpioPin,
+  OUT BOOLEAN *Value
+  );
+
+typedef
+EFI_STATUS
+(EFIAPI *MV_GPIO_SET_VALUE) (
+  IN  MARVELL_GPIO_PROTOCOL *This,
+  IN  UINTN ControllerIndex,
+  IN  UINTN GpioPin,
+  IN  BOOLEAN Value
+  );
+
+struct _MARVELL_GPIO_PROTOCOL {
+  MV_GPIO_DIRECTION_INPUT   DirectionInput;
+  MV_GPIO_DIRECTION_OUTPUT  DirectionOutput;
+  MV_GPIO_GET_FUNCTION  GetFunction;
+  MV_GPIO_GET_VALUE GetValue;
+  MV_GPIO_SET_VALUE SetValue;
+};
+
+typedef struct {
+  VENDOR_DEVICE_PATHHeader;
+  MARVELL_GPIO_DRIVER_TYPE  GpioDriverType;
+  EFI_DEVICE_PATH_PROTOCOL  End;
+} MV_GPIO_DEVICE_PATH;
+
+typedef struct {
+  UINTN ControllerId;
+  UINTN PinNumber;
+  BOOLEAN   ActiveHigh;
+} GPIO_PIN_DESC;
+
+/*
+ * Select desired protocol producer upon MARVELL_GPIO_DRIVER_TYPE
+ * field of driver's MV_GPIO_DEVICE_PATH.
+ */
+STATIC
+inline
+EFI_STATUS
+EFIAPI
+MvGpioGetProtocol (
+  IN MARVELL_GPIO_DRIVER_TYPE GpioDriverType,
+  IN OUT MARVELL_GPIO_PROTOCOL  **GpioProtocol
+  )
+{
+  MV_GPIO_DEVICE_PATH *GpioDevicePath;
+  EFI_DEVICE_PATH *DevicePath;
+  

[edk2] [platforms: PATCH 08/12] Marvell/Drivers: MvGpioDxe: Introduce platform GPIO driver

2018-10-19 Thread Marcin Wojtas
From: jinghua 

Marvell Armada 7k/8k SoCs comprise integrated GPIO controllers,
one in AP and two in each possible CP hardware blocks.

This patch introduces support for them, which is a producer of
MARVELL_GPIO_PROTOCOL, which add necessary routines.
Hardware description of the controllers is placed in MvHwDescLib.c,
same as other devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf |  43 +++
 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h   |  52 
 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.c   | 298 
 3 files changed, 393 insertions(+)
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h
 create mode 100644 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.c

diff --git a/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf 
b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf
new file mode 100644
index 000..2d56433
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf
@@ -0,0 +1,43 @@
+## @file
+#
+#  Copyright (c) 2017, Marvell International 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]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = MvGpioDxe
+  FILE_GUID  = 706eb761-b3b5-4f41-8558-5fd9217c0079
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= MvGpioEntryPoint
+
+[Sources]
+  MvGpioDxe.c
+  MvGpioDxe.h
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  ArmadaSoCDescLib
+  DebugLib
+  MemoryAllocationLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gMarvellBoardDescProtocolGuid
+  gMarvellGpioProtocolGuid
+
+[Depex]
+  TRUE
diff --git a/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h 
b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h
new file mode 100644
index 000..48744dc
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h
@@ -0,0 +1,52 @@
+/**
+*
+*  Copyright (c) 2018, Marvell International 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 __MV_GPIO_H__
+#define __MV_GPIO_H__
+
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+
+#define GPIO_SIGNATURE   SIGNATURE_32 ('G', 'P', 'I', 'O')
+
+#ifndef BIT
+#define BIT(nr)  (1 << (nr))
+#endif
+
+// Marvell GPIO Controller Registers
+#define GPIO_DATA_OUT_REG(0x0)
+#define GPIO_OUT_EN_REG  (0x4)
+#define GPIO_BLINK_EN_REG(0x8)
+#define GPIO_DATA_IN_POL_REG (0xc)
+#define GPIO_DATA_IN_REG (0x10)
+
+typedef struct {
+  MARVELL_GPIO_PROTOCOL   GpioProtocol;
+  MV_BOARD_GPIO_DESC  *Desc;
+  UINTN   Signature;
+  EFI_HANDLE  Handle;
+} MV_GPIO;
+
+#endif // __MV_GPIO_H__
diff --git a/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.c 
b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.c
new file mode 100644
index 000..fc2d416
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.c
@@ -0,0 +1,298 @@
+/**
+*
+*  Copyright (c) 2018, Marvell International 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 "MvGpioDxe.h"
+
+STATIC MV_GPIO *mGpioInstance;
+
+STATIC MV_GPIO_DEVICE_PATH mGpioDevicePathTemplate = {
+  {
+{
+  HARDWARE_DEVICE_PATH,
+  HW_VENDOR_DP,
+  {
+(UINT8) (sizeof (VENDOR_DEVICE_PATH) +
+ sizeof (MARVELL_GPIO_DRIVER_TYPE)),
+

[edk2] [platforms: PATCH 01/12] Marvell/Library: ArmadaSoCDescLib: Add GPIO information

2018-10-19 Thread Marcin Wojtas
This patch introduces new library callback (ArmadaSoCDescGpioGet ()),
which dynamically allocates and fills MV_SOC_GPIO_DESC structure with
the SoC description of GPIO controllers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h 
| 10 +
 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
| 15 
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c 
| 39 
 3 files changed, 64 insertions(+)

diff --git 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
index c14b985..85dd67c 100644
--- 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
+++ 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
@@ -22,6 +22,7 @@
 // Common macros
 //
 #define MV_SOC_CP_BASE(Cp)   (0xF200 + ((Cp) * 0x200))
+#define MV_SOC_AP_COUNT  1
 
 //
 // Platform description of AHCI controllers
@@ -38,6 +39,15 @@
 #define MV_SOC_COMPHY_MUX_BITS   4
 
 //
+// Platform description of GPIO controllers
+//
+#define MV_SOC_AP_GPIO_BASE  0xF06F5040
+#define MV_SOC_AP_GPIO_PIN_COUNT 20
+#define MV_SOC_GPIO_PER_CP_COUNT 2
+#define MV_SOC_CP_GPIO_BASE(Gpio)(0x440100 + ((Gpio) * 0x40))
+#define MV_SOC_CP_GPIO_PIN_COUNT(Gpio)   ((Gpio) == 0 ? 32 : 31)
+
+//
 // Platform description of I2C controllers
 //
 #define MV_SOC_I2C_PER_CP_COUNT  2
diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
index cdfb51b..f3d4f80 100644
--- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
@@ -46,6 +46,21 @@ ArmadaSoCDescCpBaseGet (
   );
 
 //
+// GPIO devices description template definition
+//
+typedef struct {
+  UINTN GpioBaseAddress;
+  UINTN GpioPinCount;
+} MV_SOC_GPIO_DESC;
+
+EFI_STATUS
+EFIAPI
+ArmadaSoCDescGpioGet (
+  IN OUT MV_SOC_GPIO_DESC  **GpioDesc,
+  IN OUT UINTN  *DescCount
+  );
+
+//
 // I2C
 //
 typedef struct {
diff --git 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
index 6902fda..7db4ec7 100644
--- 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
+++ 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
@@ -74,6 +74,45 @@ ArmadaSoCDescCpBaseGet (
 
 EFI_STATUS
 EFIAPI
+ArmadaSoCDescGpioGet (
+  IN OUT MV_SOC_GPIO_DESC  **GpioDesc,
+  IN OUT UINTN *DescCount
+  )
+{
+  MV_SOC_GPIO_DESC *Desc;
+  UINTN CpCount, CpIndex, Index;
+
+  CpCount = FixedPcdGet8 (PcdMaxCpCount);
+
+  *DescCount = CpCount * MV_SOC_GPIO_PER_CP_COUNT + MV_SOC_AP_COUNT;
+  Desc = AllocateZeroPool (*DescCount * sizeof (MV_SOC_GPIO_DESC));
+  if (Desc == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  *GpioDesc = Desc;
+
+  /* AP GPIO controller */
+  Desc->GpioBaseAddress = MV_SOC_AP_GPIO_BASE;
+  Desc->GpioPinCount = MV_SOC_AP_GPIO_PIN_COUNT;
+  Desc++;
+
+  /* CP GPIO controllers */
+  for (CpIndex = 0; CpIndex < CpCount; CpIndex++) {
+for (Index = 0; Index < MV_SOC_GPIO_PER_CP_COUNT; Index++) {
+  Desc->GpioBaseAddress = MV_SOC_CP_BASE (CpIndex) +
+  MV_SOC_CP_GPIO_BASE (Index);
+  Desc->GpioPinCount = MV_SOC_CP_GPIO_PIN_COUNT (Index);
+  Desc++;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
 ArmadaSoCDescI2cGet (
   IN OUT MV_SOC_I2C_DESC  **I2cDesc,
   IN OUT UINTN *DescCount
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [platforms: PATCH 00/12] Armada7k8k GPIO support

2018-10-19 Thread Marcin Wojtas
Hi,

This patchset introduces entire infrastructure for using
GPIO on Marvell devices. Finally the USB ports are properly
power-supplied on all currently supported boards.

Main changes are as follows:
- New GPIO protocol for handling basic pins operations. It allows
  very easy usage from consumer perspective.
- An example of it is enabling USB ports' VBUS via newly introduced
  NonDiscoverableInitLib, that abstracts the initialization required
  for non-discoverable devices (see last commit).
- Also Board/SoC description is added for both embedded SoC controllers,
  as well as I2C IO expanders.
- This description is utilized by two new drivers - Armada7k8k GPIO
  controllers and PCA95xx I2C IO expander family.
More detailed explanation can be found in the commit logs.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/gpio-upstream-r20181020

The diffstat may be overwhelming at the first sight, but I made
a really big effort to minimize a risk it turns out to be painful for
reviewers :)
I'm looking forward to the comments and remarks.

Best regards,
Marcin

Marcin Wojtas (10):
  Marvell/Library: ArmadaSoCDescLib: Add GPIO information
  Marvell/Library: ArmadaBoardDescLib: Add GPIO information
  SolidRun/Armada80x0McBin: Introduce board description library
  Marvell/Armada70x0Db: Introduce board description library
  Marvell/Armada80x0Db: Introduce board description library
  Marvell/Drivers: MvBoardDesc: Extend protocol with GPIO support
  Marvell/Drivers: I2c: Use common header for macros
  Marvell/Drivers: MvPca95xxDxe: Introduce I2C GPIO driver
  Marvell/Armada7k8k: Enable GPIO drivers compilation
  Marvell/Armada7k8k: Introduce NonDiscoverable device init routines

jinghua (2):
  Marvell/Protocol: Introduce MARVELL_GPIO_PROTOCOL
  Marvell/Drivers: MvGpioDxe: Introduce platform GPIO driver

 Silicon/Marvell/Marvell.dec
   |   2 +
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  
   |   2 +
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
   |  10 +-
 Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc 
   |   6 +
 Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc  
   |   6 +
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.inf
   |  34 ++
 
Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf 
  |  47 ++
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.inf
   |  34 ++
 
Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf 
  |  48 ++
 
Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.inf
 |  34 ++
 
Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
   |  48 ++
 Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf   
   |   1 +
 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.inf   
   |  43 ++
 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.inf 
   |  44 ++
 Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf  
   |   1 +
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h 
   |  10 +
 Silicon/Marvell/Drivers/Gpio/MvGpioDxe/MvGpioDxe.h 
   |  52 ++
 Silicon/Marvell/Drivers/Gpio/MvPca95xxDxe/MvPca95xxDxe.h   
   |  74 +++
 Silicon/Marvell/Drivers/I2c/MvEepromDxe/MvEepromDxe.h  
   |  10 -
 Silicon/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.h
   |  17 +-
 Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h   
   |  23 +
 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
   |  15 +
 Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h   
   |  28 +
 Silicon/Marvell/Include/Protocol/BoardDesc.h   
   |   8 +
 Silicon/Marvell/Include/Protocol/MvGpio.h  
   | 199 +++
 Silicon/Marvell/Include/Protocol/MvI2c.h   
   |  31 +
 
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c
 |  43 ++
 Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c  
   |  99 
 
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c
 |  50 ++
 

Re: [edk2] Community Discussion: General Code and Commit message standards

2018-10-19 Thread Jeremiah Cox
In GitHub, adding a link from a PR to an Issue is extremely easy:
https://help.github.com/articles/closing-issues-using-keywords/ 

Use any of the following strings, followed by a number, in a PR description...

close
closes
closed
fix
fixes
fixed
resolve
resolves
resolved

For example:
"Closes #123"

-Original Message-
From: edk2-devel  On Behalf Of Kinney, Michael 
D
Sent: Thursday, October 18, 2018 4:01 PM
To: stephano ; Andrew Fish ; 
Kinney, Michael D 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org
Subject: Re: [edk2] Community Discussion: General Code and Commit message 
standards

I would also hope that most (if not all) patches do have an associated BZ.  For 
either a feature request or a bug fix.

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org]
> On Behalf Of stephano
> Sent: Thursday, October 18, 2018 10:21 AM
> To: Andrew Fish 
> Cc: Carsey, Jaben ; edk2- de...@lists.01.org
> Subject: Re: [edk2] Community Discussion: General Code and Commit 
> message standards
> 
> On 10/18/2018 6:11 PM, Andrew Fish wrote:> What I've done in the past 
> on a branch based github PR flow is have a naming convention for the 
> branch. For example eng/PR--.
> Then we have a
> git hook that looks at the branch name and if it sees a Bugzilla 
> number it inserts the Bugzilla reference in the bottom of every commit 
> message for that branch. The CI also played tricks with the branch 
> names and could update the bug tracker with CI results, and the 
> process status of the bug.
> Interesting. This will make a good point for coming discussions. I'll 
> make a note of it.
> 
> I would hope that most modern workflows have a way to accomplish this 
> in some automated way. Seems like a pretty standard ask.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists
> .01.org%2Fmailman%2Flistinfo%2Fedk2-develdata=02%7C01%7Cjerecox%4
> 0microsoft.com%7C9a1bd80da4cd497ff85308d6354d9e4a%7C72f988bf86f141af91
> ab2d7cd011db47%7C1%7C0%7C636755004889706325sdata=ub18%2BhCBJNCpmL
> x3gh11Aqo59UAZFn0yOczb%2BW2UzvU%3Dreserved=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fmailman%2Flistinfo%2Fedk2-develdata=02%7C01%7Cjerecox%40microsoft.com%7C9a1bd80da4cd497ff85308d6354d9e4a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636755004889706325sdata=ub18%2BhCBJNCpmLx3gh11Aqo59UAZFn0yOczb%2BW2UzvU%3Dreserved=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] TianoCore Community Meeting Minutes

2018-10-19 Thread Jeremiah Cox
Finally finding time to respond to an older thread...
>>> The was a concern raised over potential lock-in to Github's, specifically in
>>> regards to history retention.
>>> Several Github users brought up that this shouldn't be an issue.
>>
>> Hopefully they said more than that.
>> What does "shouldn't be an issue" mean.
>> Were these users from multiple organisations?
>
>I will ask for more details in the next meeting, but the comments were
>that the API is quite robust and lends itself to readily accessible data.

I'd like to better understand assertions of GitHub "lock in".  GitHub provides 
a comprehensive REST API that you can easily drive via Python ("pip install 
PyGithub").
I recall one assertion that GitHub holds PR comments hostage, but it is trivial 
to dump out all Comments on all PRs in a repo.


   g = Github(  )
   repo = g.get_repo("chipsec/chipsec")
   pulls = repo.get_pulls(sort='created', base='master')
   for pr in pulls:
  for comment in pr.get_review_comments():
 print(pr.number)
 print(comment.body)
 ...

Can folks help me better understand examples of GitHub "lock in"?  I think it 
would be trivial to author a daemon that listens for PRs and comments, and 
forward those to a mailing list, if that is preferred for archival.


>>
>>> Shawn mentioned some benefits to stock Github such as
>>> it is always up to date, it includes APIs to extract data, pull
>>> requests
>>
>> Since we are discussing multiple different development systems, can we
>> try to be a bit more explicit? This is referring to github's web-based
>> branch-based ticketing system, yes?
>
>Yes that's correct.

With respect to web-based, we believe it facilitates convenient, multi-platform 
access, and GitHub's REST APIs can be leveraged to provide a command-line 
driven experience.  I believe Sean's point was that GitHub provides a 1-stop 
shop for code reviews and issue tracking such that there is no need to spend 
TianoCore resources set up, maintain, and update separate Bugzilla and Gerrit 
services, VMs, OSs, & machines.  The REST APIs could be used to maintain a 
mailing list or other mirror.  I am not asserting that GitHub provides the best 
code review and issue tracker experience, our team finds Azure Dev Ops is 
superior, but GitHub is sufficient for most use cases while handling the 
infrastructure so that we can focus on getting things done.

I think it would be helpful to construct a PROs/CONs table for each of the 
proposed end-to-end solution (source control, code review, gates, CI/CD, issue 
tracking, testing, ...).  Perhaps this table is on a wiki to enable faster 
iteration than emails on the DL.


>> I know language drift and all, but
>> 
>> is what pull request means to at least 3 participants in this conversation.
>
>I think this is an important distinction to make and I'm sure it will
>come up in future discussions. I'll be sure to bring this up.

We think of "Pull Request" as a contribution & review process akin to GitHub, 
GitLab, or Azure DevOps.  Feedback, responses, automated check-in gates, and 
signoff can be handled within each PR ticket.  Policies can be assigned, 
automated tests that must pass, enforcement of community wide rules (# of 
reviewers, reviewer list, all comments "resolved").  Tests could start as 
simple as successful build and over time could grow to a more comprehensive set 
of boot, functional, static analysis, ...
By leveraging a popular hosting provider's built-in workflow we enjoy 
significant efficiencies.  Its fully documented and known to most.  New 
capabilities are being added daily.  No TianoCore resources would be required 
to manage systems, OS patches, network issues, etc and can instead be focused 
on TianoCore code, building tests, and driving process improvements.


>>
>> It seems somewhat less than ideal to me that all of the github
>> proponents were on the opposite call to me and Laszlo (and Ard). Were
>> any of them Asia-based or could we try to get them on the call with
>> Europe next time? I'm sure me and Laszlo could be somewhat more
>> accommodating than your 7AM, but we're not going to stay up for a 3/4AM
>> meeting about source control.
>
>That's understandable. I'm not sure where Shawn and Nate are located,
>but I believe it is in the US. We could certainly move the AM call a bit
>later if it makes it easier to get more folks from the US on that call.

Agreed, pushing the US/EU call out a couple hours (8 or 9am Pacific) would 
likely increase Pacific participation.


Thanks,
Jeremiah
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Remove Arch specific build options for PcdValueInit tool.

2018-10-19 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of BobCF
> Sent: Thursday, October 18, 2018 1:43 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch] BaseTools: Remove Arch specific build options for 
> PcdValueInit tool.
> 
> PcdValueInit tool is Arch independent, the Arch specific
> build options should be removed from PcdValueInit makefile.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob C Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/Workspace/DscBuildData.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index 19ac71ac09..47f5033953 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -2344,11 +2344,11 @@ class DscBuildData(PlatformBuildClassObject):
>  if Tool != 'CC':
>  continue
> 
>  if Target == "*" or Target == self._Target:
>  if Tag == "*" or Tag == self._Toolchain:
> -if Arch == "*" or Arch == self.Arch:
> +if Arch == "*":
>  if Tool not in BuildOptions:
>  BuildOptions[Tool] = OrderedDict()
>  if Attr != "FLAGS" or Attr not in BuildOptions[Tool] 
> or self.BuildOptions[Options].startswith('='):
>  BuildOptions[Tool][Attr] = 
> self.BuildOptions[Options]
>  else:
> --
> 2.18.0.windows.1
> 
> ___
> 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 0/3] Add more checker for Tianocompress and Ueficompress

2018-10-19 Thread Gao, Liming
Laszlo:

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, October 19, 2018 7:25 PM
> To: Gao, Liming ; Zeng, Star ; 
> edk2-devel@lists.01.org; Ard Biesheuvel
> ; Holtsclaw, Brent 
> Subject: Re: [edk2] [Patch 0/3] Add more checker for Tianocompress and 
> Ueficompress
> 
> On 10/19/18 08:40, Gao, Liming wrote:
> > Laszlo:
> >   I try to answer your question. I also include the BZ submitter
> >   brent.holtsc...@intel.com. Holtsclaw, please add your comments if my
> >   info is not enough.
> >
> > Thanks
> > Liming
> >> -Original Message-
> >> From: Laszlo Ersek [mailto:ler...@redhat.com]
> >> Sent: Friday, October 19, 2018 12:01 AM
> >> To: Gao, Liming ; Zeng, Star
> >> ; edk2-devel@lists.01.org; Ard Biesheuvel
> >> 
> >> Subject: Re: [edk2] [Patch 0/3] Add more checker for Tianocompress
> >> and Ueficompress
> >>
> >> On 10/18/18 15:36, Gao, Liming wrote:
> >>> Laszlo and Star:
> >>>   Thank your notes. I will add CVE number in patch subject although
> >>>   it will make subject long than 80 characters.
> >>
> >> I agree the subject will be overlong, but I also think that including
> >> the CVE numbers is important enough for that.
> >>
> >>> Here is my proposed patch subject: CVE-2017-5731..5735 MdePkg: Add
> >>> more checker in UefiDecompressLib to access the valid buffer only.
> >>
> >> I suggest (based on tradition) that we keep the normal subject at the
> >> front, and then we append the CVE numbers at the end. Also, we should
> >> spell out all those CVE identifiers individually, if the same patch
> >> solves them all. It should be possible to search the subject line for
> >> any one of these CVE numbers in separation, using the official CVE
> >> number format.
> >>
> >
> > So, your proposal is like:  MdePkg: Add more checker in
> > UefiDecompressLib to access the valid buffer only CVE-2017-5731
> > CVE-2017-5732 CVE-2017-5733 CVE-2017-5734 CVE-2017-5735
> 
> Yes:
> 
>   MdePkg: Add more checker in UefiDecompressLib to access the valid buffer 
> only (CVE-2017-5731 CVE-2017-5732 CVE-2017-5733
> CVE-2017-5734 CVE-2017-5735)
> 
> It looks terrible, but the real subject is still readable to the left,
> and subjects with searchable CVE numbers take priority (in my opinion
> anyway).
> 
> Actually: I wonder why we needed five different CVEs, if they can all be
> fixed with a small, single patch.
> 
> More precisely: looking at the patch in more detail, I see that the
> patch fixes multiple functions / separate buffer overflows. Is it
> possible to associate each CVE with a specific, small code change in the
> patch? Because if it is possible, then I think we should split the patch
> *per CVE*. The subjects would go:
> 
> - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5731)
> - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5732)
> - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5733)
> - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5734)
> - MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5735)
> 
> (71 characters, in each subject)
> 
> If such separation is technically possible, then I think it would be an
> improvement; minimally for documentation purposes.
> 

I don't find the detail information for each CVE. BZ 686 attaches one doc to 
list all issues. So, I fix them together. I think one patch is allowed to 
include more than one CVEs. Even if with single CVE, patch subject may be 
longer than 80 characters. If we need strictly follow subject length rule, I 
suggest to mention CVE FIX in subject, and list CVE number info in the commit 
message. User can use git command to get full commit log and know which commit 
is CVE fix. For example:
MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE FIX)

> >>>   In PEI phase, the recovery image is from the external device. If
> >>>   the recovery image has the corrupt EFI compression section, they
> >>>   will be handled by EFI Decompression PPI.
> >>
> >> In the PEI phase, if the recovery image is crafted, it could cause a
> >> buffer overflow during decompression. However, if the recovery image
> >> is crafted, it might as well decompress cleanly, and once it is
> >> dispatched, do "bad things". Do the decompression and the dispatch
> >> occur at different privilege levels?
> >>
> >
> > This patch focuses on the wrong decompression data that cause the
> > decompression failure or hang.  The data content can be signed and
> > verified.
> >
> >>> In DXE phase, UEFI option ROM is the third party code. If it is EFI
> >>> compression option ROM, EFI decompression protocol will be used to
> >>> decode its data. I don't think SMM uses EFI decompression protocol.
> >>> UefiDecompressionLib is used as EFI compression PPI/Protocol. It
> >>> matches PI EFI compression section instead of GUID section. So, it
> >>> has no GUID extraction PPI/Protocol.
> >>
> >> In the DXE phase, if the option ROM is crafted, 

Re: [edk2] [Patch] BaseTool: Filter out unused structure pcds

2018-10-19 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> BobCF
> Sent: Friday, October 19, 2018 5:00 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch] BaseTool: Filter out unused structure pcds
> 
> The current code handle all the structure pcds
> even if there is no module or library use them.
> This patch is going to filter out the unused structure pcds.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  .../Source/Python/Workspace/DscBuildData.py   | 19
> +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index e481ea4f64..31bce16d15 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -274,10 +274,11 @@ class DscBuildData(PlatformBuildClassObject):
>  self._RFCLanguages  = None
>  self._ISOLanguages  = None
>  self._VpdToolGuid   = None
>  self._MacroDict = None
>  self.DefaultStores  = None
> +self.UsedStructurePcd   = None
> 
>  ## handle Override Path of Module
>  def _HandleOverridePath(self):
>  RecordList = self._RawData[MODEL_META_DATA_COMPONENT,
> self._Arch]
>  for Record in RecordList:
> @@ -1476,10 +1477,11 @@ class DscBuildData(PlatformBuildClassObject):
>  if str_pcd_obj.Type in
> [self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII],
> self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]]:
>  str_pcd_obj_str.DefaultFromDSC = 
> {skuname:{defaultstore:
> str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore,
> str_pcd_obj.SkuInfoList[skuname].HiiDefaultValue) for defaultstore in
> DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
>  else:
>  str_pcd_obj_str.DefaultFromDSC = 
> {skuname:{defaultstore:
> str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore,
> str_pcd_obj.SkuInfoList[skuname].DefaultValue) for defaultstore in
> DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
>  S_pcd_set[Pcd] = str_pcd_obj_str
> +self.FilterStrcturePcd(S_pcd_set)
>  if S_pcd_set:
>  GlobalData.gStructurePcd[self.Arch] = S_pcd_set
>  for stru_pcd in S_pcd_set.values():
>  for skuid in SkuIds:
>  if skuid in stru_pcd.SkuOverrideValues:
> @@ -1571,10 +1573,27 @@ class DscBuildData(PlatformBuildClassObject):
>  elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in
> pcd.SkuInfoList:
>  del pcd.SkuInfoList[TAB_COMMON]
> 
>  map(self.FilterSkuSettings, [Pcds[pcdkey] for pcdkey in Pcds if
> Pcds[pcdkey].Type in DynamicPcdType])
>  return Pcds
> +#Filter the StrucutrePcd that is not used by any module in dsc file and 
> fdf
> file.
> +def FilterStrcturePcd(self, S_pcd_set):
> +if not self.UsedStructurePcd:
> +FdfInfList = []
> +if GlobalData.gFdfParser:
> +FdfInfList = GlobalData.gFdfParser.Profile.InfList
> +FdfModuleList = [PathClass(NormPath(Inf), GlobalData.gWorkspace,
> Arch=self._Arch) for Inf in FdfInfList]
> +AllModulePcds = set()
> +ModuleSet = set(self._Modules.keys() + self.LibraryInstances +
> FdfModuleList)
> +for ModuleFile in ModuleSet:
> +ModuleData = self._Bdb[ModuleFile, self._Arch, self._Target,
> self._Toolchain]
> +AllModulePcds = AllModulePcds | set(ModuleData.Pcds.keys())
> +
> +self.UsedStructurePcd = AllModulePcds
> +UnusedStruPcds = set(S_pcd_set.keys()) - self.UsedStructurePcd
> +for (Token, TokenSpaceGuid) in UnusedStruPcds:
> +del S_pcd_set[(Token, TokenSpaceGuid)]
> 
>  ## Retrieve non-dynamic PCD settings
>  #
>  #   @param  TypePCD type
>  #
> --
> 2.18.0.windows.1
> 
> ___
> 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] BaseTools: Fix the bug that PcdValueFromComm is not set

2018-10-19 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Friday, October 19, 2018 12:27 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Fix the bug that PcdValueFromComm is
> not set
> 
> the bug is PcdValueFromComm is not set, but the Pcd have been override
> by the command line option.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Workspace/DscBuildData.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index e481ea4..17e6f62 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -1065,10 +1065,11 @@ class DscBuildData(PlatformBuildClassObject):
>  continue
>  for key in BuildData.Pcds:
>  PcdItem = BuildData.Pcds[key]
>  if (TokenSpaceGuidCName, TokenCName) ==
> (PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName
> =="":
>  PcdItem.DefaultValue = pcdvalue
> +PcdItem.PcdValueFromComm = pcdvalue
>  #In command line, the latter full assign value in commandLine should
> override the former field assign value.
>  #For example, --pcd Token.pcd.field="" --pcd Token.pcd=H"{}"
>  delete_assign = []
>  field_assign = {}
>  if GlobalData.BuildOptionPcd:
> --
> 2.6.1.windows.1
> 
> ___
> 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] BaseTools: Fix the crash issue when Dynamic structure Pcd use in FDF

2018-10-19 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Friday, October 19, 2018 12:21 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Fix the crash issue when Dynamic
> structure Pcd use in FDF
> 
> The case is use Dynamic structure Pcd in the FDF file.
> Current code already save the  Guid, Name and Filed info for those Pcd,
> but it directly use the dict key as [Name, Guid] and cause this crash
> issue.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1264
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index f2146a7..804f579 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -567,12 +567,12 @@ class WorkspaceAutoGen(AutoGen):
>  if (Name, Guid) not in DecPcds:
>  EdkLogger.error(
>  'build',
>  PARSER_ERROR,
>  "PCD (%s.%s) used in FDF is not declared in DEC 
> files." % (Guid,
> Name),
> -File = self.FdfProfile.PcdFileLineDict[Name, 
> Guid][0],
> -Line = self.FdfProfile.PcdFileLineDict[Name, Guid][1]
> +File = self.FdfProfile.PcdFileLineDict[Name, Guid, 
> Fileds][0],
> +Line = self.FdfProfile.PcdFileLineDict[Name, Guid, 
> Fileds][1]
>  )
>  else:
>  # Check whether Dynamic or DynamicEx PCD used in FDF 
> file. If
> used, build break and give a error message.
>  if (Name, Guid, TAB_PCDS_FIXED_AT_BUILD) in DecPcdsKey \
>  or (Name, Guid, TAB_PCDS_PATCHABLE_IN_MODULE) in
> DecPcdsKey \
> @@ -581,12 +581,12 @@ class WorkspaceAutoGen(AutoGen):
>  elif (Name, Guid, TAB_PCDS_DYNAMIC) in DecPcdsKey or 
> (Name,
> Guid, TAB_PCDS_DYNAMIC_EX) in DecPcdsKey:
>  EdkLogger.error(
>  'build',
>  PARSER_ERROR,
>  "Using Dynamic or DynamicEx type of PCD 
> [%s.%s] in FDF file
> is not allowed." % (Guid, Name),
> -File = self.FdfProfile.PcdFileLineDict[Name, 
> Guid][0],
> -Line = self.FdfProfile.PcdFileLineDict[Name, 
> Guid][1]
> +File = self.FdfProfile.PcdFileLineDict[Name, 
> Guid, Fileds][0],
> +Line = self.FdfProfile.PcdFileLineDict[Name, 
> Guid, Fileds][1]
>  )
> 
>  Pa = PlatformAutoGen(self, self.MetaFile, Target, Toolchain, 
> Arch)
>  #
>  # Explicitly collect platform's dynamic PCDs
> --
> 2.6.1.windows.1
> 
> ___
> 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 v4 5/6] UefiCpuPkg/CpuS3DataDxe: Keep old data if value already existed.

2018-10-19 Thread Laszlo Ersek
On 10/19/18 04:06, Eric Dong wrote:
> AcpiCpuData add new fields, keep these fields if old data already existed.
> 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> Reviewed-by: Ruiyu Ni 
> ---
>  UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c 
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> index ef98239844..1b847e453a 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -259,6 +259,8 @@ CpuS3DataInitialize (
>if (OldAcpiCpuData != NULL) {
>  AcpiCpuData->RegisterTable   = OldAcpiCpuData->RegisterTable;
>  AcpiCpuData->PreSmmInitRegisterTable = 
> OldAcpiCpuData->PreSmmInitRegisterTable;
> +AcpiCpuData->ApLocation  = OldAcpiCpuData->ApLocation;
> +CopyMem (>CpuStatus, >CpuStatus, sizeof 
> (CPU_STATUS_INFORMATION));
>} else {
>  //
>  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable 
> for all CPUs
> 

Reviewed-by: Laszlo Ersek 
Regression-tested-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD

2018-10-19 Thread Ard Biesheuvel
On 14 October 2018 at 05:35, Laszlo Ersek  wrote:
> On 10/12/18 16:40, Sami Mujawar wrote:
>> Some virtual machine managers provide the base address of the DT
>> in memory in the X0 register. Save the DT Base address in the
>> PcdDeviceTreeInitialBaseAddress so that the firmware can use the
>> PCD to parse the DT and obtain the platform information subsequently.
>>
>> This change also requires that the PcdDeviceTreeInitialBaseAddress
>> be a Dynamic PCD.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Sami Mujawar 
>> ---
>> The changes can be seen at 
>> https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
>>
>> Notes:
>> v1:
>> - Enable loading of DT from base address passed in X0.[SAMI]
>>
>>  ArmVirtPkg/ArmVirtPkg.dec   | 12 
>>  ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S |  9 +++--
>>  2 files changed, 15 insertions(+), 6 deletions(-)
>
> This patch seems to overlap with a feature that (I think) Ard, Drew and
> Eric have discussed earlier, for QEMU (floating RAM base). I suggest
> coordinating as early as possible.
>

This is the PrePi code, which we only use currently for
ArmVirtQemuKernel and ArmVirtXen.

PrePi is a hack which I am only willing to tolerate for legacy
platforms, and for platforms that require relocation, i.e., where the
runtime address of the SEC module is not known at link time.

I have asked Sami [in person] to look into whether we can avoid using
this PCD entirely, or at least avoid switching to a dynamic PCD.
Instead, I would prefer an approach where we store the value of X0 in
a global variable (which is permitted given that a self relocating
PrePi can only execute from DRAM anyway), and create the FDT GUID HOB
directly once we are running C code.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch v4 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Add logic to support semaphore type.

2018-10-19 Thread Laszlo Ersek
On 10/19/18 04:06, Eric Dong wrote:
> V4 changes:
> 1. Serial console log for different threads when program register table.
> 2. Check the AcpiCpuData before use it to avoid potential ASSERT.
> 
> V3 changes:
> 1. Use global variable instead of internal function to return string for 
> register type
>and dependence type.
> 2. Add comments for some complicated logic.
> 
> V1 changes:
> Because this driver needs to set MSRs saved in normal boot phase, sync
> semaphore logic from RegisterCpuFeaturesLib code which used for normal boot 
> phase.
> 
> Detail see below change for RegisterCpuFeaturesLib:
>   UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
> 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  | 413 
> +++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |   3 -
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |   3 +-
>  3 files changed, 276 insertions(+), 143 deletions(-)

Acked-by: Laszlo Ersek 
Regression-tested-by: Laszlo Ersek 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch v4 1/6] UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information.

2018-10-19 Thread Laszlo Ersek
On 10/19/18 04:06, Eric Dong wrote:
> v3 changes:
> 1. Move CPU_FEATURE_DEPENDENCE_TYPE definition here from 
> RegisterCpuFeaturesLib.h file.
> 2. Add Invalid type for REGISTER_TYPE which will be used in code.
> 
> v2 changes:
> 1. Add more description about why we do this change.
> 2. Change structure field type from pointer to EFI_PHYSICAL_ADDRESS because 
> it will
>be share between PEI and DXE.
> 
> v1 Changes:
> In order to support semaphore related logic, add new definition for it.
> 
> In a system which has multiple cores, current set register value task costs 
> huge times.
> After investigation, current set MSR task costs most of the times. Current 
> logic uses
> SpinLock to let set MSR task as an single thread task for all cores. Because 
> MSR has
> scope attribute which may cause GP fault if multiple APs set MSR at the same 
> time,
> current logic use an easiest solution (use SpinLock) to avoid this issue, but 
> it will
> cost huge times.
> 
> In order to fix this performance issue, new solution will set MSRs base on 
> their scope
> attribute. After this, the SpinLock will not needed. Without SpinLock, new 
> issue raised
> which is caused by MSR dependence. For example, MSR A depends on MSR B which 
> means MSR A
> must been set after MSR B has been set. Also MSR B is package scope level and 
> MSR A is
> thread scope level. If system has multiple threads, Thread 1 needs to set the 
> thread level
> MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs task 
> for thread 1
> and thread 2 like below:
> 
> Thread 1 Thread 2
> MSR B  NY
> MSR A  YY
> 
> If driver don't control execute MSR order, for thread 1, it will execute MSR 
> A first, but
> at this time, MSR B not been executed yet by thread 2. system may trig 
> exception at this
> time.
> 
> In order to fix the above issue, driver introduces semaphore logic to control 
> the MSR
> execute sequence. For the above case, a semaphore will be add between MSR A 
> and B for
> all threads. Semaphore has scope info for it. The possible scope value is 
> core or package.
> For each thread, when it meets a semaphore during it set registers, it will 
> 1) release
> semaphore (+1) for each threads in this core or package(based on the scope 
> info for this
> semaphore) 2) acquire semaphore (-1) for all the threads in this core or 
> package(based
> on the scope info for this semaphore). With these two steps, driver can 
> control MSR
> sequence. Sample code logic like below:
> 
>   //
>   // First increase semaphore count by 1 for processors in this package.
>   //
>   for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ; 
> ProcessorIndex ++) {
> LibReleaseSemaphore ((UINT32 *) [PackageOffset + 
> ProcessorIndex]);
>   }
>   //
>   // Second, check whether the count has reach the check number.
>   //
>   for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
> LibWaitForSemaphore ([ApOffset]);
>   }
> 
> Platform Requirement:
> 1. This change requires register MSR setting base on MSR scope info. If still 
> register MSR
>for all threads, exception may raised.
> 
> Known limitation:
> 1. Current CpuFeatures driver supports DXE instance and PEI instance. But 
> semaphore logic
>requires Aps execute in async mode which is not supported by PEI driver. 
> So CpuFeature
>PEI instance not works after this change. We plan to support async mode 
> for PEI in phase
>2 for this task.
> 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> Reviewed-by: Ruiyu Ni 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/Include/AcpiCpuData.h | 67 
> +++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Include/AcpiCpuData.h 
> b/UefiCpuPkg/Include/AcpiCpuData.h
> index 9e51145c08..005d48d7ca 100644
> --- a/UefiCpuPkg/Include/AcpiCpuData.h
> +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> @@ -22,9 +22,60 @@ typedef enum {
>Msr,
>ControlRegister,
>MemoryMapped,
> -  CacheControl
> +  CacheControl,
> +
> +  //
> +  // Semaphore type used to control the execute sequence of the Msr.
> +  // It will be insert between two Msr which has execute dependence.
> +  //
> +  Semaphore,
> +  InvalidReg
>  } REGISTER_TYPE;
>  
> +//
> +// Describe the dependency type for different features.
> +// The value set to CPU_REGISTER_TABLE_ENTRY.Value when the REGISTER_TYPE is 
> Semaphore.
> +//
> +typedef enum {
> +  NoneDepType,
> +  ThreadDepType,
> +  CoreDepType,
> +  PackageDepType,
> +  InvalidDepType
> +} CPU_FEATURE_DEPENDENCE_TYPE;
> +
> +//
> +// CPU information.
> +//
> +typedef struct {
> +  //
> +  // Record the package count in this CPU.
> +  //
> +  UINT32  PackageCount;
> +  //
> +  // Record the max core count in this CPU.
> +  // Different 

Re: [edk2] [Patch v4 1/6] UefiCpuPkg/Include/AcpiCpuData.h: Add Semaphore related Information.

2018-10-19 Thread Laszlo Ersek
On 10/19/18 04:06, Eric Dong wrote:
> v3 changes:
> 1. Move CPU_FEATURE_DEPENDENCE_TYPE definition here from 
> RegisterCpuFeaturesLib.h file.
> 2. Add Invalid type for REGISTER_TYPE which will be used in code.
> 
> v2 changes:
> 1. Add more description about why we do this change.
> 2. Change structure field type from pointer to EFI_PHYSICAL_ADDRESS because 
> it will
>be share between PEI and DXE.
> 
> v1 Changes:
> In order to support semaphore related logic, add new definition for it.
> 
> In a system which has multiple cores, current set register value task costs 
> huge times.
> After investigation, current set MSR task costs most of the times. Current 
> logic uses
> SpinLock to let set MSR task as an single thread task for all cores. Because 
> MSR has
> scope attribute which may cause GP fault if multiple APs set MSR at the same 
> time,
> current logic use an easiest solution (use SpinLock) to avoid this issue, but 
> it will
> cost huge times.
> 
> In order to fix this performance issue, new solution will set MSRs base on 
> their scope
> attribute. After this, the SpinLock will not needed. Without SpinLock, new 
> issue raised
> which is caused by MSR dependence. For example, MSR A depends on MSR B which 
> means MSR A
> must been set after MSR B has been set. Also MSR B is package scope level and 
> MSR A is
> thread scope level. If system has multiple threads, Thread 1 needs to set the 
> thread level
> MSRs and thread 2 needs to set thread and package level MSRs. Set MSRs task 
> for thread 1
> and thread 2 like below:
> 
> Thread 1 Thread 2
> MSR B  NY
> MSR A  YY
> 
> If driver don't control execute MSR order, for thread 1, it will execute MSR 
> A first, but
> at this time, MSR B not been executed yet by thread 2. system may trig 
> exception at this
> time.
> 
> In order to fix the above issue, driver introduces semaphore logic to control 
> the MSR
> execute sequence. For the above case, a semaphore will be add between MSR A 
> and B for
> all threads. Semaphore has scope info for it. The possible scope value is 
> core or package.
> For each thread, when it meets a semaphore during it set registers, it will 
> 1) release
> semaphore (+1) for each threads in this core or package(based on the scope 
> info for this
> semaphore) 2) acquire semaphore (-1) for all the threads in this core or 
> package(based
> on the scope info for this semaphore). With these two steps, driver can 
> control MSR
> sequence. Sample code logic like below:
> 
>   //
>   // First increase semaphore count by 1 for processors in this package.
>   //
>   for (ProcessorIndex = 0; ProcessorIndex < PackageThreadsCount ; 
> ProcessorIndex ++) {
> LibReleaseSemaphore ((UINT32 *) [PackageOffset + 
> ProcessorIndex]);
>   }
>   //
>   // Second, check whether the count has reach the check number.
>   //
>   for (ProcessorIndex = 0; ProcessorIndex < ValidApCount; ProcessorIndex ++) {
> LibWaitForSemaphore ([ApOffset]);
>   }
> 
> Platform Requirement:
> 1. This change requires register MSR setting base on MSR scope info. If still 
> register MSR
>for all threads, exception may raised.
> 
> Known limitation:
> 1. Current CpuFeatures driver supports DXE instance and PEI instance. But 
> semaphore logic
>requires Aps execute in async mode which is not supported by PEI driver. 
> So CpuFeature
>PEI instance not works after this change. We plan to support async mode 
> for PEI in phase
>2 for this task.
> 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> Reviewed-by: Ruiyu Ni 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/Include/AcpiCpuData.h | 67 
> +++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Include/AcpiCpuData.h 
> b/UefiCpuPkg/Include/AcpiCpuData.h
> index 9e51145c08..005d48d7ca 100644
> --- a/UefiCpuPkg/Include/AcpiCpuData.h
> +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> @@ -22,9 +22,60 @@ typedef enum {
>Msr,
>ControlRegister,
>MemoryMapped,
> -  CacheControl
> +  CacheControl,
> +
> +  //
> +  // Semaphore type used to control the execute sequence of the Msr.
> +  // It will be insert between two Msr which has execute dependence.
> +  //
> +  Semaphore,
> +  InvalidReg
>  } REGISTER_TYPE;
>  
> +//
> +// Describe the dependency type for different features.
> +// The value set to CPU_REGISTER_TABLE_ENTRY.Value when the REGISTER_TYPE is 
> Semaphore.
> +//
> +typedef enum {
> +  NoneDepType,
> +  ThreadDepType,
> +  CoreDepType,
> +  PackageDepType,
> +  InvalidDepType
> +} CPU_FEATURE_DEPENDENCE_TYPE;
> +
> +//
> +// CPU information.
> +//
> +typedef struct {
> +  //
> +  // Record the package count in this CPU.
> +  //
> +  UINT32  PackageCount;
> +  //
> +  // Record the max core count in this CPU.
> +  // Different 

Re: [edk2] CVE-2018-3613 [was: MdeModulePkg Variable: Fix Timestamp zeroing issue on APPEND_WRITE]

2018-10-19 Thread Laszlo Ersek
On 10/19/18 09:09, Zeng, Star wrote:
> Hi Laszlo,
> 
> Cc Qin also. Qin and Chao are secure boot experts, I also had some talk
> with them.
> 
> On 2018/10/19 5:45, Laszlo Ersek wrote:
>> Hi All,
>>
>> On 10/16/18 04:41, Star Zeng wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=415
>>>
>>> When SetVariable() to a time based auth variable with APPEND_WRITE
>>> attribute, and if the EFI_VARIABLE_AUTHENTICATION_2.TimeStamp in
>>> the input Data is earlier than current value, it will cause timestamp
>>> zeroing.
>>>
>>> This issue may bring time based auth variable downgrade problem.
>>> For example:
>>> A vendor released three certs at 2014, 2015, and 2016, and system
>>> integrated the 2016 cert. User can SetVariable() with 2015 cert and
>>> APPEND_WRITE attribute to cause timestamp zeroing first, then
>>> SetVariable() with 2014 cert to downgrade the cert.
>>>
>>> This patch fixes this issue.
>>>
>>> Cc: Jiewen Yao 
>>> Cc: Chao Zhang 
>>> Cc: Jian J Wang 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Star Zeng 
>>> ---
>>>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> index a2d61c8cd618..8e8db71bd201 100644
>>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> @@ -2462,6 +2462,8 @@ UpdateVariable (
>>>   if (Variable->CurrPtr != NULL) {
>>>     if (VariableCompareTimeStampInternal
>>> (&(((AUTHENTICATED_VARIABLE_HEADER *)
>>> CacheVariable->CurrPtr)->TimeStamp), TimeStamp)) {
>>>   CopyMem (>TimeStamp, TimeStamp, sizeof
>>> (EFI_TIME));
>>> +  } else {
>>> +    CopyMem (>TimeStamp,
>>> &(((AUTHENTICATED_VARIABLE_HEADER *)
>>> CacheVariable->CurrPtr)->TimeStamp), sizeof (EFI_TIME));
>>>     }
>>>   }
>>>     }
>>>
>>
>> I believe I found a significant mitigating factor for this
>> vulnerability.
> 
> Very good analysis, I totally agree. :)
> 
> Yes, if the dbx signature(includes the "attribute" information) was
> generated with "APPEND" attribute (that is the case you are seeing),
> it's infeasible to apply the downgrade write since the signature
> includes the "attribute" information, the PKCS7 verification will block
> the direct write without "APPEND" attribute.
> 
> But there may be some initial dbx signature was generated without
> "APPEND" attribute. E.g. OEM may have some this kind of dbx. It should
> be rarely case, but I am not sure about that.
> 
> Another, similar situation is also for other authenticated variables
> (not PK/KEK/DB/DBX/DBT).

Makes sense, thanks.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 3/3] MdeModulePkg/Core: add use-after-free memory detection

2018-10-19 Thread Laszlo Ersek
On 10/19/18 03:50, Jian J Wang wrote:
> UAF (Use-After-Free) memory detection is new feature introduced to
> detect illegal access to memory which has been freed. The principle
> behind is similar to heap guard feature, that is the core turn all pool
> memory allocation to page allocation and mark them to be not-present
> once they are freed.
> 
> This also implies that, once a page is allocated and freed, it cannot
> be re-allocated. This will bring another issue, which is that there's
> risk that memory space will be used out. To address it, the memory
> service add logic to put part (at most 64 pages a time) of freed pages
> back into page pool, so that the memory service can still have memory
> to allocate, when all memory space have been allocated once. This is
> called memory promotion. The promoted pages are always from the eldest
> pages which haven been freed.
> 
> To use this feature, one can simply set following PCD to 1
>   gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask
> 
> Please note this feature cannot be used with heap guard feature controlled
> by PCD gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask.
> 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h  |   1 +
>  MdeModulePkg/Core/Dxe/DxeMain.inf|   1 +
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c  |   8 +
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c| 393 
> ++-
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h|  66 +
>  MdeModulePkg/Core/Dxe/Mem/Page.c |  39 ++-
>  MdeModulePkg/Core/Dxe/Mem/Pool.c |  21 +-
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c |  16 +-
>  8 files changed, 519 insertions(+), 26 deletions(-)

I don't think I can review this patch well (I'm not an MdeModulePkg
reviewer). However I do think this patch is way too large. I suggest
splitting out everything that counts as "preparation" to separate patches.

For example:

> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 2dec9da5e3..ae75cc5b25 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -92,6 +92,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  
>  //
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 10375443c0..d91258c087 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -198,6 +198,7 @@
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType   
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   
> ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask   
> ## CONSUMES
>  
>  # [Hob]
>  # RESOURCE_DESCRIPTOR   ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index d9c65a8045..e43ec74010 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -160,6 +160,10 @@ CoreDumpGcdMemorySpaceMap (
>  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemorySpaceMap;
>  UINTNIndex;
>  
> +if ((GetDebugPrintErrorLevel () & DEBUG_GCD) == 0) {
> +  return;
> +}
> +
>  Status = CoreGetMemorySpaceMap (, );
>  ASSERT (Status == EFI_SUCCESS && MemorySpaceMap != NULL);
>  

this change doesn't belong to the UAF feature introduction. This change
streamlines some debug code, which may or may not be useful, but either
way, it should be presented separately, and discussed separately.

> @@ -202,6 +206,10 @@ CoreDumpGcdIoSpaceMap (
>  EFI_GCD_IO_SPACE_DESCRIPTOR  *IoSpaceMap;
>  UINTNIndex;
>  
> +if ((GetDebugPrintErrorLevel () & DEBUG_GCD) == 0) {
> +  return;
> +}
> +
>  Status = CoreGetIoSpaceMap (, );
>  ASSERT (Status == EFI_SUCCESS && IoSpaceMap != NULL);
>  
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> index 663f969c0d..b120c04f8f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> @@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN 
> mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
>  GLOBAL_REMOVE_IF_UNREFERENCED UINTN mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
>  = GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
>  
> +//
> +// Used for Use-After-Free memory detection to promote freed but not used 
> pages.
> +//
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS mLastPromotedPage = 
> BASE_4GB;
> +
>  /**
>Set corresponding bits in bitmap table to 1 

[edk2] [Patch] BaseTool: Filter out unused structure pcds

2018-10-19 Thread BobCF
The current code handle all the structure pcds
even if there is no module or library use them.
This patch is going to filter out the unused structure pcds.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 .../Source/Python/Workspace/DscBuildData.py   | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index e481ea4f64..31bce16d15 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -274,10 +274,11 @@ class DscBuildData(PlatformBuildClassObject):
 self._RFCLanguages  = None
 self._ISOLanguages  = None
 self._VpdToolGuid   = None
 self._MacroDict = None
 self.DefaultStores  = None
+self.UsedStructurePcd   = None
 
 ## handle Override Path of Module
 def _HandleOverridePath(self):
 RecordList = self._RawData[MODEL_META_DATA_COMPONENT, self._Arch]
 for Record in RecordList:
@@ -1476,10 +1477,11 @@ class DscBuildData(PlatformBuildClassObject):
 if str_pcd_obj.Type in 
[self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII], 
self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]]:
 str_pcd_obj_str.DefaultFromDSC = 
{skuname:{defaultstore: 
str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore, 
str_pcd_obj.SkuInfoList[skuname].HiiDefaultValue) for defaultstore in 
DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
 else:
 str_pcd_obj_str.DefaultFromDSC = 
{skuname:{defaultstore: 
str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore, 
str_pcd_obj.SkuInfoList[skuname].DefaultValue) for defaultstore in 
DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
 S_pcd_set[Pcd] = str_pcd_obj_str
+self.FilterStrcturePcd(S_pcd_set)
 if S_pcd_set:
 GlobalData.gStructurePcd[self.Arch] = S_pcd_set
 for stru_pcd in S_pcd_set.values():
 for skuid in SkuIds:
 if skuid in stru_pcd.SkuOverrideValues:
@@ -1571,10 +1573,27 @@ class DscBuildData(PlatformBuildClassObject):
 elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in 
pcd.SkuInfoList:
 del pcd.SkuInfoList[TAB_COMMON]
 
 map(self.FilterSkuSettings, [Pcds[pcdkey] for pcdkey in Pcds if 
Pcds[pcdkey].Type in DynamicPcdType])
 return Pcds
+#Filter the StrucutrePcd that is not used by any module in dsc file and 
fdf file.
+def FilterStrcturePcd(self, S_pcd_set):
+if not self.UsedStructurePcd:
+FdfInfList = []
+if GlobalData.gFdfParser:
+FdfInfList = GlobalData.gFdfParser.Profile.InfList
+FdfModuleList = [PathClass(NormPath(Inf), GlobalData.gWorkspace, 
Arch=self._Arch) for Inf in FdfInfList]
+AllModulePcds = set()
+ModuleSet = set(self._Modules.keys() + self.LibraryInstances + 
FdfModuleList)
+for ModuleFile in ModuleSet:
+ModuleData = self._Bdb[ModuleFile, self._Arch, self._Target, 
self._Toolchain]
+AllModulePcds = AllModulePcds | set(ModuleData.Pcds.keys())
+
+self.UsedStructurePcd = AllModulePcds
+UnusedStruPcds = set(S_pcd_set.keys()) - self.UsedStructurePcd
+for (Token, TokenSpaceGuid) in UnusedStruPcds:
+del S_pcd_set[(Token, TokenSpaceGuid)]
 
 ## Retrieve non-dynamic PCD settings
 #
 #   @param  TypePCD type
 #
-- 
2.18.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/3] UefiCpuPkg/CpuDxe: fix an infinite loop issue

2018-10-19 Thread Laszlo Ersek
On 10/19/18 03:50, Jian J Wang wrote:
> The UAF (Use-After-Free) memory detection feature will cause an
> infinite calling of InitializePageTablePool(). This is due to a
> fact that AllocateAlignedPages() is used to allocate page table
> pool memory. This function will most likely call gBS->FreePages
> to free unaligned pages and then cause another round of page
> attributes change, like below
> 
>FreePages() <===|
> => SetMemoryAttributes()   |

This should likely be "SetMemorySpaceAttributes" (the DXE service), or else 
"CpuSetMemoryAttributes" (the underlying CpuDxe function name).

> =>  |
> => InitializePageTablePool()   |
> => AllocateAlignedPages()  |
> => FreePages() |
> 
> The solution is add a lock in page table pool allocation function
> and fail any other requests if it has not been done.

OK, but what is the end result? InitializePageTablePool() will return FALSE. 
How far back up is that error propagated? To what components will the error be 
visible?

BTW, I've found the following comment in CpuSetMemoryAttributes():

  //
  // During memory attributes updating, new pages may be allocated to setup
  // smaller granularity of page table. Page allocation action might then cause
  // another calling of CpuSetMemoryAttributes() recursively, due to memory
  // protection policy configured (such as PcdDxeNxMemoryProtectionPolicy).
  // Since this driver will always protect memory used as page table by itself,
  // there's no need to apply protection policy requested from memory service.
  // So it's safe to just return EFI_SUCCESS if this time of calling is caused
  // by page table memory allocation.
  //

Is the current argument similar? I think it should be documented somehow.

> 
> Cc: Laszlo Ersek 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c 
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 33e8ee2d2c..2145e623fa 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
>  };
>  
>  PAGE_TABLE_POOL   *mPageTablePool = NULL;
> +EFI_LOCK  mPageTablePoolLock = 
> EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTIFY);

Why does this have to be an "EFI_LOCK"? Can't we just use a global variable? (I 
don't understand why messing with the TPL is necessary.)

In fact, I totally don't understand the point of EfiAcquireLock(). If we have 
two independent resources, each protected with its own separate lock, then why 
do both locks share the system-wide TPL between each other?


>  PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext;
>  EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL;
>  
> @@ -1045,6 +1046,12 @@ InitializePageTablePool (
>  {
>VOID  *Buffer;
>BOOLEAN   IsModified;
> +  EFI_STATUSStatus;
> +
> +  Status = EfiAcquireLockOrFail ();
> +  if (EFI_ERROR (Status)) {
> +return FALSE;
> +  }
>  
>//
>// Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page 
> for
> @@ -1056,7 +1063,10 @@ InitializePageTablePool (
>Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT);
>if (Buffer == NULL) {
>  DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n"));
> +EfiReleaseLock ();

I feel that it would be safer to introduce a "Done" label at the bottom of the 
function, and release the lock there.

(Again, I'm not sure why this has to be a "lock".)

>  return FALSE;
> +  } else {
> +DEBUG ((DEBUG_INFO, "Paging: added %d pages to page table pool\r\n", 
> PoolPages));

Please don't print UINTN values with %d. Cast them to UINT64 and log them with 
%lu.

>}
>  
>//
> @@ -1092,6 +1102,8 @@ InitializePageTablePool (
>  );
>ASSERT (IsModified == TRUE);
>  
> +  EfiReleaseLock ();
> +
>return TRUE;
>  }
>  
> 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 1/3] MdeModulePkg/MdeModulePkg.dec: add new PCD for UAF detection feature

2018-10-19 Thread Laszlo Ersek
On 10/19/18 03:50, Jian J Wang wrote:
> UAF (Use-After-Free) memory detection is new feature introduced to
> detect illegal access to memory which has been freed. The principle
> behind is similar to heap guard feature, that is we'll turn all pool
> memory allocation to page allocation and mark them to be not-present
> once they are freed.
> 
> This also implies that, once a page is allocated and freed, it cannot
> be re-allocated. This will bring another issue, which is that there's
> risk that memory space will be used out. To address it, this patch
> series add logic put part (at most 64 pages a time) of freed pages
> back into page pool, so that the memory service can still have memory
> to allocate, when all memory space have been allocated once. This is
> called memory promotion. The promoted pages are always from the eldest
> pages freed.
> 
> To use this feature, one can simply set following PCD to 1
>   gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask
> 
> Please note this feature cannot be used with heap guard feature controlled
> by PCD gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask.
> 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/MdeModulePkg.dec | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 6037504fa7..83736cd761 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1029,6 +1029,12 @@
># @Prompt Enable UEFI Stack Guard.
>gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
>  
> +  ## This mask is to control Use-After-Free Memory Detection behavior.
> +  #   BIT0- Enable Use-After-Free memory detection for UEFI modules.
> +  #   BIT1..7 - Reserved for future uses.
> +  # @Prompt The Use-After-Free Memory Detection feature mask
> +  
> gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask|0x0|UINT8|0x30001056
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>## Dynamic type PCD can be registered callback function for Pcd setting 
> action.
>#  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of 
> callback function
> 

The default value looks fine to me; can't comment on the rest.

Acked-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch 0/3] Add more checker for Tianocompress and Ueficompress

2018-10-19 Thread Laszlo Ersek
On 10/19/18 08:40, Gao, Liming wrote:
> Laszlo:
>   I try to answer your question. I also include the BZ submitter
>   brent.holtsc...@intel.com. Holtsclaw, please add your comments if my
>   info is not enough.
>
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Friday, October 19, 2018 12:01 AM
>> To: Gao, Liming ; Zeng, Star
>> ; edk2-devel@lists.01.org; Ard Biesheuvel
>> 
>> Subject: Re: [edk2] [Patch 0/3] Add more checker for Tianocompress
>> and Ueficompress
>>
>> On 10/18/18 15:36, Gao, Liming wrote:
>>> Laszlo and Star:
>>>   Thank your notes. I will add CVE number in patch subject although
>>>   it will make subject long than 80 characters.
>>
>> I agree the subject will be overlong, but I also think that including
>> the CVE numbers is important enough for that.
>>
>>> Here is my proposed patch subject: CVE-2017-5731..5735 MdePkg: Add
>>> more checker in UefiDecompressLib to access the valid buffer only.
>>
>> I suggest (based on tradition) that we keep the normal subject at the
>> front, and then we append the CVE numbers at the end. Also, we should
>> spell out all those CVE identifiers individually, if the same patch
>> solves them all. It should be possible to search the subject line for
>> any one of these CVE numbers in separation, using the official CVE
>> number format.
>>
>
> So, your proposal is like:  MdePkg: Add more checker in
> UefiDecompressLib to access the valid buffer only CVE-2017-5731
> CVE-2017-5732 CVE-2017-5733 CVE-2017-5734 CVE-2017-5735

Yes:

  MdePkg: Add more checker in UefiDecompressLib to access the valid buffer only 
(CVE-2017-5731 CVE-2017-5732 CVE-2017-5733 CVE-2017-5734 CVE-2017-5735)

It looks terrible, but the real subject is still readable to the left,
and subjects with searchable CVE numbers take priority (in my opinion
anyway).

Actually: I wonder why we needed five different CVEs, if they can all be
fixed with a small, single patch.

More precisely: looking at the patch in more detail, I see that the
patch fixes multiple functions / separate buffer overflows. Is it
possible to associate each CVE with a specific, small code change in the
patch? Because if it is possible, then I think we should split the patch
*per CVE*. The subjects would go:

- MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5731)
- MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5732)
- MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5733)
- MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5734)
- MdePkg/UefiDecompressLib: fix potential buffer overflow (CVE-2017-5735)

(71 characters, in each subject)

If such separation is technically possible, then I think it would be an
improvement; minimally for documentation purposes.

>>>   In PEI phase, the recovery image is from the external device. If
>>>   the recovery image has the corrupt EFI compression section, they
>>>   will be handled by EFI Decompression PPI.
>>
>> In the PEI phase, if the recovery image is crafted, it could cause a
>> buffer overflow during decompression. However, if the recovery image
>> is crafted, it might as well decompress cleanly, and once it is
>> dispatched, do "bad things". Do the decompression and the dispatch
>> occur at different privilege levels?
>>
>
> This patch focuses on the wrong decompression data that cause the
> decompression failure or hang.  The data content can be signed and
> verified.
>
>>> In DXE phase, UEFI option ROM is the third party code. If it is EFI
>>> compression option ROM, EFI decompression protocol will be used to
>>> decode its data. I don't think SMM uses EFI decompression protocol.
>>> UefiDecompressionLib is used as EFI compression PPI/Protocol. It
>>> matches PI EFI compression section instead of GUID section. So, it
>>> has no GUID extraction PPI/Protocol.
>>
>> In the DXE phase, if the option ROM is crafted, it could cause a
>> buffer overflow when it is decompressed. But, again, how is that
>> different from when a crafted oprom decompresses cleanly, and then
>> does "bad things" when it is dispatched?
>>
>> Here (in the DXE phase), I can imagine two answers myself:
>>
>> (1) Decompression occurs before Secure Boot validation, but dispatch
>> occurs only after. Therefore a crafted UEFI image could cause
>> problems via decompression even if it would fail SB verification
>> later.
>>
>> (2) Decompression of UEFI option ROMs occurs before PlatformBDS locks
>> down SMRAM and lockboxes. However, the execution of UEFI option ROMs
>> is deferred until after the lockdown.
>>
>> Do these scenarios apply? Because, if they do, I agree the issue
>> qualifies as privilege escalation.
>>
>
> Yes. Decompression happen early. After decompression, PE image will be
> verified.

Got it now. Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg/BaseLib: AsciiStrToUnicodeStr(S) not handle EASCII properly

2018-10-19 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Wu, Hao A
> Sent: Friday, October 19, 2018 11:06 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Bin . Lain ; 
> Yao, Jiewen ; Kinney, Michael D
> ; Gao, Liming 
> Subject: [PATCH] MdePkg/BaseLib: AsciiStrToUnicodeStr(S) not handle EASCII 
> properly
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1262
> 
> Current implementation of BaseLib APIs:
> 
> AsciiStrToUnicodeStr()
> AsciiStrToUnicodeStrS()
> AsciiStrnToUnicodeStrS()
> 
> do not handle EASCII properly.
> 
> More specifically, if the value of ASCII character is larger than 0x7F,
> then the converted Unicode character will have all '1's in the higher 8
> bits.
> 
> An example:
>   0xC9 => 0xFFC9 (current implementations)
> and it should be:
>   0xC9 => 0x00C9
> 
> This commit will address this issue.
> 
> Cc: Bin.Lain 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdePkg/Library/BaseLib/SafeString.c | 4 ++--
>  MdePkg/Library/BaseLib/String.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/SafeString.c 
> b/MdePkg/Library/BaseLib/SafeString.c
> index 29310889ca..417497cbc9 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/MdePkg/Library/BaseLib/SafeString.c
> @@ -2932,7 +2932,7 @@ AsciiStrToUnicodeStrS (
>// Convert string
>//
>while (*Source != '\0') {
> -*(Destination++) = (CHAR16)*(Source++);
> +*(Destination++) = (CHAR16)(UINT8)*(Source++);
>}
>*Destination = '\0';
> 
> @@ -3045,7 +3045,7 @@ AsciiStrnToUnicodeStrS (
>// Convert string
>//
>while ((*Source != 0) && (SourceLen > 0)) {
> -*(Destination++) = (CHAR16)*(Source++);
> +*(Destination++) = (CHAR16)(UINT8)*(Source++);
>  SourceLen--;
>  (*DestinationLength)++;
>}
> diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
> index cb90774c86..e6df12797d 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -1746,7 +1746,7 @@ AsciiStrToUnicodeStr (
> 
>ReturnValue = Destination;
>while (*Source != '\0') {
> -*(Destination++) = (CHAR16) *(Source++);
> +*(Destination++) = (CHAR16)(UINT8) *(Source++);
>}
>//
>// End the Destination with a NULL.
> --
> 2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg UefiLib: Check Table against NULL in ScanTableInSDT

2018-10-19 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Zeng, Star
> Sent: Friday, October 19, 2018 4:07 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Gao, Liming ; 
> Kinney, Michael D ; Wang,
> Jian J 
> Subject: [PATCH] MdePkg UefiLib: Check Table against NULL in ScanTableInSDT
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1266
> 
> af5e95215928e052445c473f1244412dadea8252 abstracted generic functions
> from different modules (IntelVTdDxe, S3SaveStateDxe, PcRtc,
> DpDynamicCommand and PiSmmCpuDxeSmm). Some of them (IntelVTdDxe and
> PcRtc) checked Table against NULL before accessing Table->Signature,
> some (S3SaveStateDxe, DpDynamicCommand and PiSmmCpuDxeSmm did not.
> 
> The ScanTableInSDT() in Acpi.c of UefiLib was mainly from
> S3SaveStateDxe, so it does not check Table against NULL before
> accessing Table->Signature.
> 
> This patch updates ScanTableInSDT() to check Table against NULL first
> before accessing Table->Signature.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Jian J Wang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Library/UefiLib/Acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> index 4df6731ff0a4..59a828835ca0 100644
> --- a/MdePkg/Library/UefiLib/Acpi.c
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -67,7 +67,7 @@ ScanTableInSDT (
>  EntryPtr = 0;
>  CopyMem (, (VOID *)(BasePtr + Index * TablePointerSize), 
> TablePointerSize);
>  Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
> -if (Table->Signature == Signature) {
> +if ((Table != NULL) && (Table->Signature == Signature)) {
>if (PreviousTable != NULL) {
>  if (Table == PreviousTable) {
>*PreviousTableLocated = TRUE;
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation

2018-10-19 Thread Ard Biesheuvel
Commit 9dd8190e4995 ("Silicon/SynQuacer: tweak PCI I/O windows for
ACPI/Linux support") updated the min/max/offset definitions for the
PCIe I/O resource windows on SynQuacer, and updated the read path of
the platform's EfiCpuIo2 protocol implementation, but failed to update
the write path as well, resulting in spurious errors if when attempting
to write to PCIe I/O ports on PCIe RC #1, which uses translation for the
I/O BAR window.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
 | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
 
b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
index 736b20cd5129..e5cc3aef908d 100644
--- 
a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
+++ 
b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
@@ -518,12 +518,18 @@ CpuIoServiceWrite (
 return Status;
   }
 
-  if ((Address >= SYNQUACER_PCI_SEG0_PORTIO_MIN) &&
-  (Address <= SYNQUACER_PCI_SEG0_PORTIO_MAX)) {
-Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE;
-  } else if ((Address >= SYNQUACER_PCI_SEG1_PORTIO_MIN) &&
- (Address <= SYNQUACER_PCI_SEG1_PORTIO_MAX)) {
-Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE;
+  if ((Address >= (SYNQUACER_PCI_SEG0_PORTIO_MIN +
+   SYNQUACER_PCI_SEG0_PORTIO_OFFSET)) &&
+  (Address <= (SYNQUACER_PCI_SEG0_PORTIO_MAX +
+   SYNQUACER_PCI_SEG0_PORTIO_OFFSET))) {
+Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE -
+   SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
+  } else if ((Address >= (SYNQUACER_PCI_SEG1_PORTIO_MIN +
+  SYNQUACER_PCI_SEG1_PORTIO_OFFSET)) &&
+ (Address <= (SYNQUACER_PCI_SEG1_PORTIO_MAX +
+  SYNQUACER_PCI_SEG1_PORTIO_OFFSET))) {
+Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE -
+   SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
 
   } else {
 ASSERT (FALSE);
-- 
2.17.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] IntelFsp2Pkg: FSP should not override IDT

2018-10-19 Thread Chasel, Chiu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1265

FSP should not override IDT table when it is initialized
by boot loader. IDT should be re-initialized in FSP only
when it is invalid.

Test: Verified on internal platform and boots successfully.

Cc: Jiewen Yao 
Cc: Desimone Nathaniel L 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 IntelFsp2Pkg/FspSecCore/SecMain.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c 
b/IntelFsp2Pkg/FspSecCore/SecMain.c
index 37fd4dfdeb..5912221204 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -70,6 +70,7 @@ SecStartup (
   UINT32  Index;
   FSP_GLOBAL_DATA PeiFspData;
   UINT64  ExceptionHandler;
+  UINTN   IdtSize;
 
   //
   // Process all libraries constructor function linked to SecCore.
@@ -98,13 +99,24 @@ SecStartup (
   // |   |
   // |---|>  TempRamBase
   IdtTableInStack.PeiService  = NULL;
-  ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
-  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
-CopyMem ((VOID*)[Index], 
(VOID*), sizeof (UINT64));
+  AsmReadIdtr ();
+  if ((IdtDescriptor.Base == 0) && (IdtDescriptor.Limit == 0x)) {
+ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
+for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
+  CopyMem ((VOID*)[Index], 
(VOID*), sizeof (UINT64));
+}
+IdtSize = sizeof (IdtTableInStack.IdtTable);
+  } else {
+// Get minimum size
+if (IdtDescriptor.Limit + 1 > sizeof (IdtTableInStack.IdtTable)) {
+  IdtSize = sizeof (IdtTableInStack.IdtTable);
+} else {
+  IdtSize = IdtDescriptor.Limit + 1;
+}
+CopyMem ((VOID *) (UINTN) , (VOID *) 
IdtDescriptor.Base, IdtSize);
   }
-
   IdtDescriptor.Base  = (UINTN) 
-  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
+  IdtDescriptor.Limit = (UINT16)(IdtSize - 1);
 
   AsmWriteIdtr ();
 
-- 
2.13.3.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdePkg UefiLib: Check Table against NULL in ScanTableInSDT

2018-10-19 Thread Wang, Jian J
Reviewed-by: Jian J Wang 

> -Original Message-
> From: Zeng, Star
> Sent: Friday, October 19, 2018 4:07 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Gao, Liming ;
> Kinney, Michael D ; Wang, Jian J
> 
> Subject: [PATCH] MdePkg UefiLib: Check Table against NULL in ScanTableInSDT
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1266
> 
> af5e95215928e052445c473f1244412dadea8252 abstracted generic functions
> from different modules (IntelVTdDxe, S3SaveStateDxe, PcRtc,
> DpDynamicCommand and PiSmmCpuDxeSmm). Some of them (IntelVTdDxe and
> PcRtc) checked Table against NULL before accessing Table->Signature,
> some (S3SaveStateDxe, DpDynamicCommand and PiSmmCpuDxeSmm did not.
> 
> The ScanTableInSDT() in Acpi.c of UefiLib was mainly from
> S3SaveStateDxe, so it does not check Table against NULL before
> accessing Table->Signature.
> 
> This patch updates ScanTableInSDT() to check Table against NULL first
> before accessing Table->Signature.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Jian J Wang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Library/UefiLib/Acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> index 4df6731ff0a4..59a828835ca0 100644
> --- a/MdePkg/Library/UefiLib/Acpi.c
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -67,7 +67,7 @@ ScanTableInSDT (
>  EntryPtr = 0;
>  CopyMem (, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
>  Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
> -if (Table->Signature == Signature) {
> +if ((Table != NULL) && (Table->Signature == Signature)) {
>if (PreviousTable != NULL) {
>  if (Table == PreviousTable) {
>*PreviousTableLocated = TRUE;
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] MdePkg UefiLib: Check Table against NULL in ScanTableInSDT

2018-10-19 Thread Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1266

af5e95215928e052445c473f1244412dadea8252 abstracted generic functions
from different modules (IntelVTdDxe, S3SaveStateDxe, PcRtc,
DpDynamicCommand and PiSmmCpuDxeSmm). Some of them (IntelVTdDxe and
PcRtc) checked Table against NULL before accessing Table->Signature,
some (S3SaveStateDxe, DpDynamicCommand and PiSmmCpuDxeSmm did not.

The ScanTableInSDT() in Acpi.c of UefiLib was mainly from
S3SaveStateDxe, so it does not check Table against NULL before
accessing Table->Signature.

This patch updates ScanTableInSDT() to check Table against NULL first
before accessing Table->Signature.

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdePkg/Library/UefiLib/Acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
index 4df6731ff0a4..59a828835ca0 100644
--- a/MdePkg/Library/UefiLib/Acpi.c
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -67,7 +67,7 @@ ScanTableInSDT (
 EntryPtr = 0;
 CopyMem (, (VOID *)(BasePtr + Index * TablePointerSize), 
TablePointerSize);
 Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
-if (Table->Signature == Signature) {
+if ((Table != NULL) && (Table->Signature == Signature)) {
   if (PreviousTable != NULL) {
 if (Table == PreviousTable) {
   *PreviousTableLocated = TRUE;
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch] BaseTools: Fix the bug that PcdValueFromComm is not set

2018-10-19 Thread Yonghong Zhu
the bug is PcdValueFromComm is not set, but the Pcd have been override
by the command line option.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/Workspace/DscBuildData.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index e481ea4..17e6f62 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1065,10 +1065,11 @@ class DscBuildData(PlatformBuildClassObject):
 continue
 for key in BuildData.Pcds:
 PcdItem = BuildData.Pcds[key]
 if (TokenSpaceGuidCName, TokenCName) == 
(PcdItem.TokenSpaceGuidCName, PcdItem.TokenCName) and FieldName =="":
 PcdItem.DefaultValue = pcdvalue
+PcdItem.PcdValueFromComm = pcdvalue
 #In command line, the latter full assign value in commandLine should 
override the former field assign value.
 #For example, --pcd Token.pcd.field="" --pcd Token.pcd=H"{}"
 delete_assign = []
 field_assign = {}
 if GlobalData.BuildOptionPcd:
-- 
2.6.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch] BaseTools: Fix the crash issue when Dynamic structure Pcd use in FDF

2018-10-19 Thread Yonghong Zhu
The case is use Dynamic structure Pcd in the FDF file.
Current code already save the  Guid, Name and Filed info for those Pcd,
but it directly use the dict key as [Name, Guid] and cause this crash
issue.

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=1264
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index f2146a7..804f579 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -567,12 +567,12 @@ class WorkspaceAutoGen(AutoGen):
 if (Name, Guid) not in DecPcds:
 EdkLogger.error(
 'build',
 PARSER_ERROR,
 "PCD (%s.%s) used in FDF is not declared in DEC 
files." % (Guid, Name),
-File = self.FdfProfile.PcdFileLineDict[Name, Guid][0],
-Line = self.FdfProfile.PcdFileLineDict[Name, Guid][1]
+File = self.FdfProfile.PcdFileLineDict[Name, Guid, 
Fileds][0],
+Line = self.FdfProfile.PcdFileLineDict[Name, Guid, 
Fileds][1]
 )
 else:
 # Check whether Dynamic or DynamicEx PCD used in FDF file. 
If used, build break and give a error message.
 if (Name, Guid, TAB_PCDS_FIXED_AT_BUILD) in DecPcdsKey \
 or (Name, Guid, TAB_PCDS_PATCHABLE_IN_MODULE) in 
DecPcdsKey \
@@ -581,12 +581,12 @@ class WorkspaceAutoGen(AutoGen):
 elif (Name, Guid, TAB_PCDS_DYNAMIC) in DecPcdsKey or 
(Name, Guid, TAB_PCDS_DYNAMIC_EX) in DecPcdsKey:
 EdkLogger.error(
 'build',
 PARSER_ERROR,
 "Using Dynamic or DynamicEx type of PCD 
[%s.%s] in FDF file is not allowed." % (Guid, Name),
-File = self.FdfProfile.PcdFileLineDict[Name, 
Guid][0],
-Line = self.FdfProfile.PcdFileLineDict[Name, 
Guid][1]
+File = self.FdfProfile.PcdFileLineDict[Name, 
Guid, Fileds][0],
+Line = self.FdfProfile.PcdFileLineDict[Name, 
Guid, Fileds][1]
 )
 
 Pa = PlatformAutoGen(self, self.MetaFile, Target, Toolchain, Arch)
 #
 # Explicitly collect platform's dynamic PCDs
-- 
2.6.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch] BaseTools: Fix the *B and *F Flag display for Structure Pcd

2018-10-19 Thread Yonghong Zhu
Because of we newly add the PcdFieldValueFromComm and
PcdFieldValueFromFdf in early parser phase, so in the report we use
the saved value in this two variables to print it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/build/BuildReport.py | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index a66adfb..c648086 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1072,13 +1072,17 @@ class PcdReport(object):
 SkuInfoList = Pcd.SkuInfoList
 Pcd = GlobalData.gStructurePcd[self.Arch][(Pcd.TokenCName, 
Pcd.TokenSpaceGuidCName)]
 Pcd.DatumType = Pcd.StructName
 if TypeName in ('DYNVPD', 'DEXVPD'):
 Pcd.SkuInfoList = SkuInfoList
-if Pcd.PcdFieldValueFromComm:
+if Pcd.PcdValueFromComm or Pcd.PcdFieldValueFromComm:
 BuildOptionMatch = True
 DecMatch = False
+elif Pcd.PcdValueFromFdf or Pcd.PcdFieldValueFromFdf:
+DscDefaultValue = True
+DscMatch = True
+DecMatch = False
 elif Pcd.SkuOverrideValues:
 DscOverride = False
 if Pcd.DefaultFromDSC:
 DscOverride = True
 else:
@@ -1264,18 +1268,22 @@ class PcdReport(object):
 Value = '{} ({:d})'.format(Value, int(Value, 0))
 else:
 Value = "0x{:X} ({})".format(int(Value, 0), Value)
 FileWrite(File, ' %-*s   : %6s %10s = %s' % (self.MaxLen, Flag 
+ ' ' + PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', Value))
 if IsStructure:
+FiledOverrideFlag = False
 OverrideValues = Pcd.SkuOverrideValues
 if OverrideValues:
 for Data in OverrideValues.values():
 Struct = list(Data.values())
 if Struct:
 OverrideFieldStruct = self.OverrideFieldValue(Pcd, 
Struct[0])
 self.PrintStructureInfo(File, OverrideFieldStruct)
+FiledOverrideFlag = True
 break
+if not FiledOverrideFlag and (Pcd.PcdFieldValueFromComm or 
Pcd.PcdFieldValueFromFdf):
+OverrideFieldStruct = self.OverrideFieldValue(Pcd, {})
 self.PrintPcdDefault(File, Pcd, IsStructure, DscMatch, 
DscDefaultValue, InfMatch, InfDefaultValue, DecMatch, DecDefaultValue)
 else:
 FirstPrint = True
 SkuList = sorted(Pcd.SkuInfoList.keys())
 for Sku in SkuList:
@@ -1391,15 +1399,19 @@ class PcdReport(object):
 else:
 FileWrite(File, ' %-*s   : %6s %10s %10s = %s' 
% (self.MaxLen, ' ', TypeName, '(' + Pcd.DatumType + ')', '(' + SkuIdName + 
')', Value))
 if TypeName in ('DYNVPD', 'DEXVPD'):
 FileWrite(File, '%*s' % (self.MaxLen + 4, 
SkuInfo.VpdOffset))
 if IsStructure:
+FiledOverrideFlag = False
 OverrideValues = Pcd.SkuOverrideValues[Sku]
 if OverrideValues:
 Keys = OverrideValues.keys()
 OverrideFieldStruct = self.OverrideFieldValue(Pcd, 
OverrideValues[Keys[0]])
 self.PrintStructureInfo(File, OverrideFieldStruct)
+FiledOverrideFlag = True
+if not FiledOverrideFlag and 
(Pcd.PcdFieldValueFromComm or Pcd.PcdFieldValueFromFdf):
+OverrideFieldStruct = self.OverrideFieldValue(Pcd, 
{})
 self.PrintPcdDefault(File, Pcd, IsStructure, DscMatch, 
DscDefaultValue, InfMatch, InfDefaultValue, DecMatch, DecDefaultValue)
 
 def OverrideFieldValue(self, Pcd, OverrideStruct):
 OverrideFieldStruct = collections.OrderedDict()
 if OverrideStruct:
-- 
2.6.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] CVE-2018-3613 [was: MdeModulePkg Variable: Fix Timestamp zeroing issue on APPEND_WRITE]

2018-10-19 Thread Zeng, Star

Hi Laszlo,

Cc Qin also. Qin and Chao are secure boot experts, I also had some talk 
with them.


On 2018/10/19 5:45, Laszlo Ersek wrote:

Hi All,

On 10/16/18 04:41, Star Zeng wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=415

When SetVariable() to a time based auth variable with APPEND_WRITE
attribute, and if the EFI_VARIABLE_AUTHENTICATION_2.TimeStamp in
the input Data is earlier than current value, it will cause timestamp
zeroing.

This issue may bring time based auth variable downgrade problem.
For example:
A vendor released three certs at 2014, 2015, and 2016, and system
integrated the 2016 cert. User can SetVariable() with 2015 cert and
APPEND_WRITE attribute to cause timestamp zeroing first, then
SetVariable() with 2014 cert to downgrade the cert.

This patch fixes this issue.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index a2d61c8cd618..8e8db71bd201 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2462,6 +2462,8 @@ UpdateVariable (
  if (Variable->CurrPtr != NULL) {
if (VariableCompareTimeStampInternal (&(((AUTHENTICATED_VARIABLE_HEADER 
*) CacheVariable->CurrPtr)->TimeStamp), TimeStamp)) {
  CopyMem (>TimeStamp, TimeStamp, sizeof (EFI_TIME));
+  } else {
+CopyMem (>TimeStamp, &(((AUTHENTICATED_VARIABLE_HEADER *) 
CacheVariable->CurrPtr)->TimeStamp), sizeof (EFI_TIME));
}
  }
}



I believe I found a significant mitigating factor for this
vulnerability.


Very good analysis, I totally agree. :)

Yes, if the dbx signature(includes the "attribute" information) was 
generated with "APPEND" attribute (that is the case you are seeing), 
it's infeasible to apply the downgrade write since the signature 
includes the "attribute" information, the PKCS7 verification will block 
the direct write without "APPEND" attribute.


But there may be some initial dbx signature was generated without 
"APPEND" attribute. E.g. OEM may have some this kind of dbx. It should 
be rarely case, but I am not sure about that.


Another, similar situation is also for other authenticated variables 
(not PK/KEK/DB/DBX/DBT).



Thanks,
Star



(i) I tried to reproduce the issue (with this patch reverted). I indeed
managed to trigger the "timestamp zeroed" case, by *appending* a
hypothetical "2015" DBX update, to a virtual system that had the "2016"
DBX update installed first.

However, when I tried to replay the hypothetical "2014" DBX update on
top, by *writing* it (not appending it), I found that it wouldn't work:


(ii) I confirmed that the timestamp check was in fact circumvented, due
to the zeroing above. That is, the following code snippet from
VerifyTimeBasedPayload() would *not* fire:

[SecurityPkg/Library/AuthVariableLib/AuthService.c]

   1869if ((OrgTimeStamp != NULL) && ((Attributes & 
EFI_VARIABLE_APPEND_WRITE) == 0)) {
   1870  if (AuthServiceInternalCompareTimeStamp (>TimeStamp, 
OrgTimeStamp)) {
   1871//
   1872// TimeStamp check fail, suspicious replay attack, return 
EFI_SECURITY_VIOLATION.
   1873//
   1874return EFI_SECURITY_VIOLATION;
   1875  }
   1876}

(Line numbers correspond to commit 3a0329bed2a2).

Yet the replay attempt was rejected. Why?


(iii) It was rejected on the following call chain:

   VariableServiceSetVariable()   
[MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c]
 AuthVariableLibProcessVariable() 
[SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c]
   ProcessVarWithKek()
[SecurityPkg/Library/AuthVariableLib/AuthService.c]
VerifyTimeBasedPayloadAndUpdate() 
[SecurityPkg/Library/AuthVariableLib/AuthService.c]
  VerifyTimeBasedPayload()
[SecurityPkg/Library/AuthVariableLib/AuthService.c]
Pkcs7Verify() 
[CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c]

[SecurityPkg/Library/AuthVariableLib/AuthService.c]

   2032  //
   2033  // Ready to verify Pkcs7 SignedData. Go through KEK Signature 
Database to find out X.509 CertList.
   2034  //
   2035  KekDataSize  = (UINT32) DataSize;
   2036  CertList = (EFI_SIGNATURE_LIST *) Data;
   2037  while ((KekDataSize > 0) && (KekDataSize >= 
CertList->SignatureListSize)) {
   2038if (CompareGuid (>SignatureType, )) {
   2039  Cert   = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof 
(EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);
   2040  CertCount  = (CertList->SignatureListSize - sizeof 
(EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / 

Re: [edk2] [PATCH v2] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

2018-10-19 Thread Ye, Ting


Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Thursday, October 18, 2018 4:44 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting ; Wu, Jiaxin 

Subject: [PATCH v2] NetworkPkg/IpSecDxe: Fix issue to parse SA Payload.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1251

*v2: Correct the type of parameters in Ikev2ParseProposalData(), and refined 
the corresponding description.

IpSecDxe failed to create the Child SA during parsing SA Payload, the issue was 
caused by the below commit:

SHA-1: 1e0db7b11987d0ec93be7dfe26102a327860fdbd
* MdeModulePkg/NetworkPkg: Checking for NULL pointer before use.

In above commit, it changed the value of IsMatch in 
Ikev2ChildSaParseSaPayload() to FALSE. That's correct but it exposed the 
potential bug in to match the correct proposal Data, which will cause the issue 
happen.

Cc: Fu Siyuan 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/IpSecDxe/Ikev2/Utility.c | 64 ++---
 1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c 
b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 0c9c929705..63de56f09e 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -1993,34 +1993,51 @@ Ikev2IsSupportAlg (  }
 
 /**
   Get the preferred algorithm types from ProposalData.
 
-  @param[in]  ProposalData  Pointer to related IKEV2_PROPOSAL_DATA.
-  @param[out] PreferEncryptAlgorithmOutput of preferred encrypt algorithm.
-  @param[out] PreferIntegrityAlgorithm  Output of preferred integrity 
algorithm.
-  @param[out] PreferPrfAlgorithmOutput of preferred PRF algorithm. Only
-for IKE SA.
-  @param[out] PreferDhGroup Output of preferred DH group. Only for
-IKE SA.
-  @param[out] PreferEncryptKeylengthOutput of preferred encrypt key length
-in bytes.
-  @param[out] IsSupportEsn  Output of value about the Extented 
Sequence
-Number is support or not. Only for 
Child SA.
-  @param[in]  IsChildSa If it is ture, the ProposalData is for 
IKE
-SA. Otherwise the proposalData is for 
Child SA.
+  @param[in]  ProposalData  Pointer to related 
IKEV2_PROPOSAL_DATA.
+  @param[in, out] PreferEncryptAlgorithmPointer to buffer which is used to 
store the
+preferred encrypt algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred encrypt 
algorithm.
+  @param[in, out] PreferIntegrityAlgorithm  Pointer to buffer which is used to 
store the
+preferred integrity algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred integrity 
algorithm.
+  @param[in, out] PreferPrfAlgorithmPointer to buffer which is used to 
store the
+preferred PRF algorithm.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred PRF algorithm. 
Only
+for IKE SA.
+  @param[in, out] PreferDhGroup Pointer to buffer which is used to 
store the
+preferred DH group.
+Input value shall be initialized 
to zero that
+indicates to be parsed from 
ProposalData.
+Output of preferred DH group. Only 
for
+IKE SA.
+  @param[in, out] PreferEncryptKeylengthPointer to buffer which is used to 
store the
+preferred encrypt key length in 
bytes.
+  @param[in, out] IsSupportEsn  Pointer to buffer which is used to 
store the
+value about the Extented Sequence 
Number is
+support or not. Only for Child SA.
+  @param[in]  IsChildSa If it is ture, the ProposalData is 
for IKE
+SA. Otherwise the proposalData is 
for Child SA.
 
 **/
 VOID
 Ikev2ParseProposalData (
   IN 

Re: [edk2] [Patch 0/3] Add more checker for Tianocompress and Ueficompress

2018-10-19 Thread Gao, Liming
Laszlo: 
  I try to answer your question. I also include the BZ submitter 
brent.holtsc...@intel.com. Holtsclaw, please add your comments if my info is 
not enough. 

Thanks
Liming
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Friday, October 19, 2018 12:01 AM
>To: Gao, Liming ; Zeng, Star ;
>edk2-devel@lists.01.org; Ard Biesheuvel 
>Subject: Re: [edk2] [Patch 0/3] Add more checker for Tianocompress and
>Ueficompress
>
>On 10/18/18 15:36, Gao, Liming wrote:
>> Laszlo and Star:
>>   Thank your notes. I will add CVE number in patch subject although it
>>   will make subject long than 80 characters.
>
>I agree the subject will be overlong, but I also think that including
>the CVE numbers is important enough for that.
>
>> Here is my proposed patch subject: CVE-2017-5731..5735 MdePkg: Add
>> more checker in UefiDecompressLib to access the valid buffer only.
>
>I suggest (based on tradition) that we keep the normal subject at the
>front, and then we append the CVE numbers at the end. Also, we should
>spell out all those CVE identifiers individually, if the same patch
>solves them all. It should be possible to search the subject line for
>any one of these CVE numbers in separation, using the official CVE
>number format.
>

So, your proposal is like:  MdePkg: Add more checker in UefiDecompressLib to 
access the valid buffer only CVE-2017-5731 CVE-2017-5732 CVE-2017-5733 
CVE-2017-5734 CVE-2017-5735

>>   In PEI phase, the recovery image is from the external device. If the
>>   recovery image has the corrupt EFI compression section, they will be
>>   handled by EFI Decompression PPI.
>
>In the PEI phase, if the recovery image is crafted, it could cause a
>buffer overflow during decompression. However, if the recovery image is
>crafted, it might as well decompress cleanly, and once it is dispatched,
>do "bad things". Do the decompression and the dispatch occur at
>different privilege levels?
>

This patch focuses on the wrong decompression data that cause the decompression 
failure or hang.  The data content can be signed and verified.  

>> In DXE phase, UEFI option ROM is the third party code. If it is EFI
>> compression option ROM, EFI decompression protocol will be used to
>> decode its data. I don't think SMM uses EFI decompression protocol.
>> UefiDecompressionLib is used as EFI compression PPI/Protocol. It
>> matches PI EFI compression section instead of GUID section. So, it has
>> no GUID extraction PPI/Protocol.
>
>In the DXE phase, if the option ROM is crafted, it could cause a buffer
>overflow when it is decompressed. But, again, how is that different from
>when a crafted oprom decompresses cleanly, and then does "bad things"
>when it is dispatched?
>
>Here (in the DXE phase), I can imagine two answers myself:
>
>(1) Decompression occurs before Secure Boot validation, but dispatch
>occurs only after. Therefore a crafted UEFI image could cause problems
>via decompression even if it would fail SB verification later.
>
>(2) Decompression of UEFI option ROMs occurs before PlatformBDS locks
>down SMRAM and lockboxes. However, the execution of UEFI option ROMs
>is deferred until after the lockdown.
>
>Do these scenarios apply? Because, if they do, I agree the issue
>qualifies as privilege escalation.
>

Yes. Decompression happen early. After decompression, PE image will be 
verified. 

>Thank you!
>Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [staging/MicroPythonTestFramework]: MicroPython Test Framework for UEFI

2018-10-19 Thread Richardson, Brian
Leif:

Thank you for your feedback. Long Qin is a good starting contact for 
MicroPython issues.

There are readme files for the sub-components, but I agree that the missing 
top-level readme file is an issue.
https://github.com/tianocore/edk2-staging/tree/MicroPythonTestFramework/MpyTestFrameworkPkg
https://github.com/tianocore/edk2-staging/tree/MicroPythonTestFramework/MicroPythonPkg

Thanks … br
---
Brian Richardson, Firmware Ecosystem Development, Intel Software
brian.richard...@intel.com -- @intel_brian 
(Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

From: Leif Lindholm 
Sent: Friday, October 19, 2018 12:34 AM
To: Richardson, Brian 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [staging/MicroPythonTestFramework]: MicroPython Test 
Framework for UEFI

Hi Brian,

I've started having a look at this, and have a few comments:
- There is no Readme.md at the top level, as set out in 
https://github.com/tianocore/edk2-staging/blob/about/README
  Mainly, this means I don't know who I should cc on any comments I have.
- There have been substantial changes to oniguruma, and the module no longer 
builds. Can we have exact commit hashes for the two external projects added to 
the toplevel Readme.md?
- At least Uefi/modets.c and Uefi/modos.c contain Ia32/X64-specific bits. Could 
these bits be put in architecture-specific subdirectories?

Regards,

Leif

On 10 August 2018 at 03:44, Richardson, Brian 
mailto:brian.richard...@intel.com>> wrote:
The "MicroPython Test Framework for UEFI" project has been added to 
edk2-staging for community feedback.
https://github.com/tianocore/edk2-staging/tree/MicroPythonTestFramework

This includes a port of MicroPython to UEFI and a test execution environment 
that can run from the UEFI Shell.
https://github.com/tianocore/edk2-staging/tree/MicroPythonTestFramework/MicroPythonPkg
https://github.com/tianocore/edk2-staging/tree/MicroPythonTestFramework/MpyTestFrameworkPkg

Additional Info:
https://github.com/tianocore/tianocore.github.io/wiki/MicroPython-Test-Framework-for-UEFI

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com>
 -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson

___
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] MdeModulePkg/EsrtDxe: allocate ESRT table from RtServicesData memory

2018-10-19 Thread Zeng, Star

On 2018/10/19 13:28, Ard Biesheuvel wrote:

On 19 October 2018 at 13:25, Zeng, Star  wrote:

Be honest, I am not clear about the history why EfiBootServicesData is required 
for ESRT. But OS indeed cares about ESRT table, for example, I guess the 
Firmware in Device Manager for Windows is built based on ESRT table. In fact,  
I think OS loader can access either EfiBootServicesData or 
EfiRuntimeServicesData configuration table as it controls the runtime phase 
point.



The problem is not with the OS. The OS can access it it any time it
want, and create a virtual mapping for it.

The problem is with the firmware: any table that the firmware wants to
access *itself* requires a virtual mapping to be provided by the OS in
SetVirtualAdressMap(), so that the virtual address is known to the
firmware.


Only when firmware wants to access it at *runtime*.




I am concerning that even the spec could be updated, our code may still need 
caching for backward compatibility.



I don't think that should be a problem: Linux permits the ESRT to
reside in EfiRuntimeServicesData already, because certain x86
production systems do that. This means that it is highly unlikely that
Windows disallows this.


I meant firmware compatibility. If we assume ESRT is only produced by 
EsrtDxe/EsrtFmpDxe (common code), that should be ok. But if the ESRT is 
produced by other codes (non-common), it may be hard to assume they 
could be updated at the same time. I admit it is rarely case. :)



Thanks,
Star






-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Friday, October 19, 2018 1:11 PM
To: Zeng, Star 
Cc: Peter Jones ; edk2-devel@lists.01.org; Dong, Eric ; 
Leif Lindholm ; Kinney, Michael D ; Yao, 
Jiewen 
Subject: Re: [PATCH] MdeModulePkg/EsrtDxe: allocate ESRT table from 
RtServicesData memory

On 19 October 2018 at 13:01, Ard Biesheuvel  wrote:

On 19 October 2018 at 12:48, Zeng, Star  wrote:

Ok, thanks and got the case.

DxeCapsuleLibVirtualAddressChangeEvent may be too late as it could not allocate 
pool to do caching.
I meant registering gEfiSystemResourceTableGuid event group 
notification(installconfigurationtable will trigger event group) and do caching 
in the notification function.




OK, I will create a bugzilla for this.



As I understand it, the reason we require EfiBootServicesData for the ESRT is 
because the OS may not care about this table, and so we don't want to waste the 
memory. However, if we end up caching the entire table in 
EfiRuntimeServicesData anyway [so that the firmware itself can access it], is 
there still a point to keeping this requirement?
Shouldn't we update the spec regardless?


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel