[edk2] [PATCH v5 04/13] MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

This commit will add the GUID definitions for LockBox which is used to
save a list of storage devices that need to get initialized during the S3
resume.

The content of the LockBox will be a DevicePath structure that contains
zero or more DevicePath instances. Each instance denotes a storage device
that needs to get initialized during the S3 resume.

The producers of the content of this LockBox will be drivers like
OpalPassword DXE driver. This kind of drivers requires some specific
storage devices to be initialized during the PEI phase of in S3 resume.
(For the OpalPassword case, it requires the managing devices to be
automatically unlocked during the S3 resume).

The attribute of the LockBox should be set to
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY.

The consumers of the content of this LockBox will be PEI storage device
controller/bus drivers (e.g. NvmExpressPei) during S3 resume. This kind of
drivers can use the DevicePath instances stored in the LockBox to get a
list of devices that need to get initialized. In such way, an on-demand
(partial) device enumeration/initialization can be performed to benefit
the S3 resume performance.

Cc: Jian J Wang 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
---
 MdeModulePkg/MdeModulePkg.dec   |  3 +
 MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h | 64 
 2 files changed, 67 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 7f646d7702..a2130bc439 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -422,6 +422,9 @@
   ## Include/Guid/S3SmmInitDone.h
   gEdkiiS3SmmInitDoneGuid = { 0x8f9d4825, 0x797d, 0x48fc, { 0x84, 0x71, 0x84, 
0x50, 0x25, 0x79, 0x2e, 0xf6 } }
 
+  ## Include/Guid/S3StorageDeviceInitList.h
+  gS3StorageDeviceInitListGuid = { 0x310e9b8c, 0xcf90, 0x421e, { 0x8e, 0x9b, 
0x9e, 0xef, 0xb6, 0x17, 0xc8, 0xef } }
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid   = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 
0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
diff --git a/MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h 
b/MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
new file mode 100644
index 00..bd300b2696
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
@@ -0,0 +1,64 @@
+/** @file
+  Define the LockBox GUID for list of storage devices need to be initialized in
+  S3.
+
+  Copyright (c) 2019, Intel Corporation. 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 __S3_STORAGE_DEVICE_INIT_LIST_H__
+#define __S3_STORAGE_DEVICE_INIT_LIST_H__
+
+#define S3_STORAGE_DEVICE_INIT_LIST \
+  { \
+0x310e9b8c, 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17, 0xc8, 
0xef } \
+  }
+
+//
+// The LockBox will store a DevicePath structure that contains one or more
+// DevicePath instances. Each instance denotes a storage device that needs to
+// get initialized during the S3 resume.
+//
+// For example, if there is only one storage device stored in the list, the
+// content of this LockBox will be:
+//
+// +---+
+// | DevPath Instance #1   |
+// | (Terminated by an End of Hardware Device Path node|
+// |  with an End Entire Device Path sub-type) |
+// +---+
+//
+// If there are n (n > 1) storage devices in the list, the content of this
+// LockBox will be:
+//
+// +---+
+// | DevPath Instance #1   |
+// | (Terminated by an End of Hardware Device Path node|
+// |  with an End This Instance of a Device Path sub-type) |
+// +---+
+// | DevPath Instance #2   |
+// | (Terminated by an End of Hardware Device Path node|
+// |  with an End This Instance of a Device Path sub-type) |
+// +---+
+// |...|
+// +---+
+// | DevPath Instance #n   |
+// | (Terminated by an End of Hardware Device Path node|
+// |  with an End Entire Device Path sub-type) |
+// +---+
+//
+// The attribute of the LockBox should 

[edk2] [PATCH v5 06/13] MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

For the NvmExpressPei driver, this commit will add codes to produce the
Storage Security Command PPI if the underlying NVM Express controller
supports the Security Send and Security Receive commands.

Cc: Jian J Wang 
Cc: Ray Ni 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf  |  10 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h|  58 ++-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.h |  20 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.h | 247 

 MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c   | 231 
+++
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c| 143 +--
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c |  32 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiStorageSecurity.c | 423 

 8 files changed, 1075 insertions(+), 89 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
index 9591572fec..0666e5892b 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
@@ -2,7 +2,7 @@
 #  The NvmExpressPei driver is used to manage non-volatile memory subsystem
 #  which follows NVM Express specification at PEI phase.
 #
-#  Copyright (c) 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -30,6 +30,7 @@
 #
 
 [Sources]
+  DevicePath.c
   DmaMem.c
   NvmExpressPei.c
   NvmExpressPei.h
@@ -39,6 +40,8 @@
   NvmExpressPeiHci.h
   NvmExpressPeiPassThru.c
   NvmExpressPeiPassThru.h
+  NvmExpressPeiStorageSecurity.c
+  NvmExpressPeiStorageSecurity.h
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -54,11 +57,12 @@
   PeimEntryPoint
 
 [Ppis]
-  gEfiPeiVirtualBlockIoPpiGuid   ## PRODUCES
-  gEfiPeiVirtualBlockIo2PpiGuid  ## PRODUCES
   gEdkiiPeiNvmExpressHostControllerPpiGuid   ## CONSUMES
   gEdkiiIoMmuPpiGuid ## CONSUMES
   gEfiEndOfPeiSignalPpiGuid  ## CONSUMES
+  gEfiPeiVirtualBlockIoPpiGuid   ## SOMETIMES_PRODUCES
+  gEfiPeiVirtualBlockIo2PpiGuid  ## SOMETIMES_PRODUCES
+  gEdkiiPeiStorageSecurityCommandPpiGuid ## SOMETIMES_PRODUCES
 
 [Depex]
   gEfiPeiMemoryDiscoveredPpiGuid AND
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
index 0135eca6f0..92c3854c6e 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -44,6 +45,7 @@ typedef struct _PEI_NVME_CONTROLLER_PRIVATE_DATA  
PEI_NVME_CONTROLLER_PRIVATE_DA
 #include "NvmExpressPeiHci.h"
 #include "NvmExpressPeiPassThru.h"
 #include "NvmExpressPeiBlockIo.h"
+#include "NvmExpressPeiStorageSecurity.h"
 
 //
 // NVME PEI driver implementation related definitions
@@ -90,10 +92,15 @@ struct _PEI_NVME_NAMESPACE_INFO {
 struct _PEI_NVME_CONTROLLER_PRIVATE_DATA {
   UINT32Signature;
   UINTN MmioBase;
+  UINTN DevicePathLength;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
   EFI_PEI_RECOVERY_BLOCK_IO_PPI BlkIoPpi;
   EFI_PEI_RECOVERY_BLOCK_IO2_PPIBlkIo2Ppi;
+  EDKII_PEI_STORAGE_SECURITY_CMD_PPIStorageSecurityPpi;
   EFI_PEI_PPI_DESCRIPTORBlkIoPpiList;
   EFI_PEI_PPI_DESCRIPTORBlkIo2PpiList;
+  EFI_PEI_PPI_DESCRIPTORStorageSecurityPpiList;
   EFI_PEI_NOTIFY_DESCRIPTOR EndOfPeiNotifyList;
 
   //
@@ -139,11 +146,13 @@ struct _PEI_NVME_CONTROLLER_PRIVATE_DATA {
   PEI_NVME_NAMESPACE_INFO   *NamespaceInfo;
 };
 
-#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_BLKIO(a) \
+#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_BLKIO(a)   \
   CR (a, PEI_NVME_CONTROLLER_PRIVATE_DATA, BlkIoPpi, 
NVME_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE)
-#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_BLKIO2(a)\
+#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_BLKIO2(a)  \
   CR (a, PEI_NVME_CONTROLLER_PRIVATE_DATA, BlkIo2Ppi, 
NVME_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE)
-#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_NOTIFY(a)\
+#define GET_NVME_PEIM_HC_PRIVATE_DATA_FROM_THIS_STROAGE_SECURITY(a)\
+  CR (a, PEI_NVME_CONTROLLER_PRIVATE_DATA, StorageSecurityPpi, 

[edk2] [PATCH v5 01/13] MdeModulePkg: Add definitions for ATA AHCI host controller PPI

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

This commit will add the definitions for ATA AHCI host controller PPI. The
purpose of the PPI in to provide:

* MMIO base address
* Controller identification information (DevicePath)

for ATA host controllers working under AHCI mode.

Cc: Jian J Wang 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
---
 MdeModulePkg/MdeModulePkg.dec|  3 +
 MdeModulePkg/Include/Ppi/AtaAhciController.h | 89 
 2 files changed, 92 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index e5c32d15ed..4411185073 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -480,6 +480,9 @@
   ## Include/Ppi/NvmExpressHostController.h
   gEdkiiPeiNvmExpressHostControllerPpiGuid  = { 0xcae3aa63, 0x676f, 0x4da3, { 
0xbd, 0x50, 0x6c, 0xc5, 0xed, 0xde, 0x9a, 0xad } }
 
+  ## Include/Ppi/AtaAhciController.h
+  gEdkiiPeiAtaAhciHostControllerPpiGuid = { 0x61dd33ea, 0x421f, 0x4cc0, { 
0x89, 0x29, 0xff, 0xee, 0xa9, 0xa1, 0xa2, 0x61 } }
+
 [Protocols]
   ## Load File protocol provides capability to load and unload EFI image into 
memory and execute it.
   #  Include/Protocol/LoadPe32Image.h
diff --git a/MdeModulePkg/Include/Ppi/AtaAhciController.h 
b/MdeModulePkg/Include/Ppi/AtaAhciController.h
new file mode 100644
index 00..2bdd53ff36
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/AtaAhciController.h
@@ -0,0 +1,89 @@
+/** @file
+
+  Copyright (c) 2019, Intel Corporation. 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 _EDKII_ATA_AHCI_HOST_CONTROLLER_PPI_H_
+#define _EDKII_ATA_AHCI_HOST_CONTROLLER_PPI_H_
+
+#include 
+
+///
+/// Global ID for the EDKII_ATA_AHCI_HOST_CONTROLLER_PPI.
+///
+#define EDKII_ATA_AHCI_HOST_CONTROLLER_PPI_GUID \
+  { \
+0x61dd33ea, 0x421f, 0x4cc0, { 0x89, 0x29, 0xff, 0xee, 0xa9, 0xa1, 0xa2, 
0x61 } \
+  }
+
+//
+// Forward declaration for the EDKII_ATA_AHCI_HOST_CONTROLLER_PPI.
+//
+typedef struct _EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  
EDKII_ATA_AHCI_HOST_CONTROLLER_PPI;
+
+/**
+  Get the MMIO base address of ATA AHCI host controller.
+
+  @param[in]  This The PPI instance pointer.
+  @param[in]  ControllerId The ID of the ATA AHCI host controller.
+  @param[out] MmioBar  The MMIO base address of the controller.
+
+  @retval EFI_SUCCESS  The operation succeeds.
+  @retval EFI_INVALID_PARAMETERThe parameters are invalid.
+  @retval EFI_NOT_FOUNDThe specified ATA AHCI host controller not 
found.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_ATA_AHCI_HC_GET_MMIO_BAR) (
+  IN  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI*This,
+  IN  UINT8 ControllerId,
+  OUT UINTN *MmioBar
+  );
+
+/**
+  Get the device path of ATA AHCI host controller.
+
+  @param[in]  This The PPI instance pointer.
+  @param[in]  ControllerId The ID of the ATA AHCI host controller.
+  @param[out] DevicePathLength The length of the device path in bytes 
specified
+   by DevicePath.
+  @param[out] DevicePath   The device path of ATA AHCI host controller.
+   This field re-uses EFI Device Path Protocol 
as
+   defined by Section 10.2 EFI Device Path 
Protocol
+   of UEFI 2.7 Specification.
+
+  @retval EFI_SUCCESS  The operation succeeds.
+  @retval EFI_INVALID_PARAMETERThe parameters are invalid.
+  @retval EFI_NOT_FOUNDThe specified ATA AHCI host controller not 
found.
+  @retval EFI_OUT_OF_RESOURCES The operation fails due to lack of 
resources.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_ATA_AHCI_HC_GET_DEVICE_PATH) (
+  IN  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI*This,
+  IN  UINT8 ControllerId,
+  OUT UINTN *DevicePathLength,
+  OUT EFI_DEVICE_PATH_PROTOCOL  **DevicePath
+  );
+
+//
+// This PPI contains a set of services to interact with the ATA AHCI host 
controller.
+//
+struct _EDKII_ATA_AHCI_HOST_CONTROLLER_PPI {
+  EDKII_ATA_AHCI_HC_GET_MMIO_BAR   GetAhciHcMmioBar;
+  EDKII_ATA_AHCI_HC_GET_DEVICE_PATHGetAhciHcDevicePath;
+};
+
+extern EFI_GUID gEdkiiPeiAtaAhciHostControllerPpiGuid;
+
+#endif
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org

[edk2] [PATCH v5 02/13] MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

This commit will add the definitions for EDKII PEI ATA PassThru PPI. This
PPI will provide services that allow ATA commands to be sent to ATA
devices attached to an ATA controller in the PEI phase.

More specifically, the PPI will provide services to:

* Send ATA commands to an ATA device (by service 'PassThru');
* Get the list of the attached ATA device on a controller (by services
  'GetNextPort' and 'GetNextDevice');
* Get the identification information (DevicePath) of the underlying ATA
  host controller (by service 'GetDevicePath').

Cc: Jian J Wang 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
---
 MdeModulePkg/MdeModulePkg.dec  |   3 +
 MdeModulePkg/Include/Ppi/AtaPassThru.h | 219 
 2 files changed, 222 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 4411185073..8efb19e626 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -483,6 +483,9 @@
   ## Include/Ppi/AtaAhciController.h
   gEdkiiPeiAtaAhciHostControllerPpiGuid = { 0x61dd33ea, 0x421f, 0x4cc0, { 
0x89, 0x29, 0xff, 0xee, 0xa9, 0xa1, 0xa2, 0x61 } }
 
+  ## Include/Ppi/AtaPassThru.h
+  gEdkiiPeiAtaPassThruPpiGuid   = { 0xa16473fd, 0xd474, 0x4c89, { 
0xae, 0xc7, 0x90, 0xb8, 0x3c, 0x73, 0x86, 0x9  } }
+
 [Protocols]
   ## Load File protocol provides capability to load and unload EFI image into 
memory and execute it.
   #  Include/Protocol/LoadPe32Image.h
diff --git a/MdeModulePkg/Include/Ppi/AtaPassThru.h 
b/MdeModulePkg/Include/Ppi/AtaPassThru.h
new file mode 100644
index 00..78bdaef9e2
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/AtaPassThru.h
@@ -0,0 +1,219 @@
+/** @file
+
+  Copyright (c) 2019, Intel Corporation. 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 _EDKII_ATA_PASS_THRU_PPI_H_
+#define _EDKII_ATA_PASS_THRU_PPI_H_
+
+#include 
+#include 
+
+///
+/// Global ID for the EDKII_PEI_ATA_PASS_THRU_PPI.
+///
+#define EDKII_PEI_ATA_PASS_THRU_PPI_GUID \
+  { \
+0xa16473fd, 0xd474, 0x4c89, { 0xae, 0xc7, 0x90, 0xb8, 0x3c, 0x73, 0x86, 
0x9 } \
+  }
+
+//
+// Forward declaration for the EDKII_PEI_ATA_PASS_THRU_PPI.
+//
+typedef struct _EDKII_PEI_ATA_PASS_THRU_PPI  EDKII_PEI_ATA_PASS_THRU_PPI;
+
+//
+// Revision The revision to which the ATA Pass Thru PPI interface adheres.
+//  All future revisions must be backwards compatible.
+//  If a future version is not back wards compatible it is not the 
same GUID.
+//
+#define EDKII_PEI_ATA_PASS_THRU_PPI_REVISION0x0001
+
+
+/**
+  Sends an ATA command to an ATA device that is attached to the ATA controller.
+
+  @param[in] This  The PPI instance pointer.
+  @param[in] Port  The port number of the ATA device to 
send
+   the command.
+  @param[in] PortMultiplierPortThe port multiplier port number of the 
ATA
+   device to send the command.
+   If there is no port multiplier, then 
specify
+   0x.
+  @param[in,out] PacketA pointer to the ATA command to send to
+   the ATA device specified by Port and
+   PortMultiplierPort.
+
+  @retval EFI_SUCCESS  The ATA command was sent by the host. For
+   bi-directional commands, InTransferLength 
bytes
+   were transferred from InDataBuffer. For 
write
+   and bi-directional commands, 
OutTransferLength
+   bytes were transferred by OutDataBuffer.
+  @retval EFI_NOT_FOUNDThe specified ATA device is not found.
+  @retval EFI_INVALID_PARAMETERThe contents of Acb are invalid. The ATA 
command
+   was not sent, so no additional status 
information
+   is available.
+  @retval EFI_BAD_BUFFER_SIZE  The ATA command was not executed. The number
+   of bytes that could be transferred is 
returned
+   in InTransferLength. For write and 
bi-directional
+   commands, OutTransferLength bytes were 
transferred
+   by OutDataBuffer.
+  @retval EFI_NOT_READY

[edk2] [PATCH v5 05/13] MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable

2019-02-14 Thread Hao Wu
This commit is out of the scope for BZ-1409. The commit will remove the
call of RegisterForShadow() at the entry point of the driver. By doing so,
the driver is now possible to be executed without being re-loaded into
permanent memory.

Thus, this commit will update the NvmExpressPei driver to avoid updating
the content of a global variable.

Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
---
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h |  12 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c| 153 +++-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c |  11 +-
 3 files changed, 92 insertions(+), 84 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
index 0bd62c2459..0135eca6f0 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
@@ -2,7 +2,7 @@
   The NvmExpressPei driver is used to manage non-volatile memory subsystem
   which follows NVM Express specification at PEI phase.
 
-  Copyright (c) 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -147,13 +147,9 @@ struct _PEI_NVME_CONTROLLER_PRIVATE_DATA {
   CR (a, PEI_NVME_CONTROLLER_PRIVATE_DATA, EndOfPeiNotifyList, 
NVME_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE)
 
 
-/**
-  Initialize IOMMU.
-**/
-VOID
-IoMmuInit (
-  VOID
-  );
+//
+// Internal functions
+//
 
 /**
   Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
index 51b48d38dd..cb629c16b0 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/DmaMem.c
@@ -1,7 +1,7 @@
 /** @file
   The DMA memory help function.
 
-  Copyright (c) 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -16,7 +16,33 @@
 
 #include "NvmExpressPei.h"
 
-EDKII_IOMMU_PPI  *mIoMmu;
+/**
+  Get IOMMU PPI.
+
+  @return Pointer to IOMMU PPI.
+
+**/
+EDKII_IOMMU_PPI *
+GetIoMmu (
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  EDKII_IOMMU_PPI*IoMmu;
+
+  IoMmu  = NULL;
+  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **) 
+ );
+  if (!EFI_ERROR (Status) && (IoMmu != NULL)) {
+return IoMmu;
+  }
+
+  return NULL;
+}
 
 /**
   Provides the controller-specific addresses required to access system memory 
from a
@@ -46,18 +72,21 @@ IoMmuMap (
   OUT VOID  **Mapping
   )
 {
-  EFI_STATUS  Status;
-  UINT64  Attribute;
-
-  if (mIoMmu != NULL) {
-Status = mIoMmu->Map (
-   mIoMmu,
-   Operation,
-   HostAddress,
-   NumberOfBytes,
-   DeviceAddress,
-   Mapping
-   );
+  EFI_STATUS Status;
+  UINT64 Attribute;
+  EDKII_IOMMU_PPI*IoMmu;
+
+  IoMmu = GetIoMmu ();
+
+  if (IoMmu != NULL) {
+Status = IoMmu->Map (
+ IoMmu,
+ Operation,
+ HostAddress,
+ NumberOfBytes,
+ DeviceAddress,
+ Mapping
+ );
 if (EFI_ERROR (Status)) {
   return EFI_OUT_OF_RESOURCES;
 }
@@ -78,11 +107,11 @@ IoMmuMap (
   ASSERT(FALSE);
   return EFI_INVALID_PARAMETER;
 }
-Status = mIoMmu->SetAttribute (
-   mIoMmu,
-   *Mapping,
-   Attribute
-   );
+Status = IoMmu->SetAttribute (
+  IoMmu,
+  *Mapping,
+  Attribute
+  );
 if (EFI_ERROR (Status)) {
   return Status;
 }
@@ -108,11 +137,14 @@ IoMmuUnmap (
   IN VOID  *Mapping
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS Status;
+  EDKII_IOMMU_PPI*IoMmu;
+
+  IoMmu = GetIoMmu ();
 
-  if (mIoMmu != NULL) {
-Status = mIoMmu->SetAttribute (mIoMmu, Mapping, 0);
-Status = mIoMmu->Unmap (mIoMmu, Mapping);
+  if (IoMmu != NULL) {
+Status = IoMmu->SetAttribute (IoMmu, Mapping, 0);
+Status = IoMmu->Unmap (IoMmu, Mapping);
   } else {
 Status = EFI_SUCCESS;
   }
@@ -148,39 +180,42 @@ IoMmuAllocateBuffer (
   EFI_STATUSStatus;
   UINTN NumberOfBytes;
   EFI_PHYSICAL_ADDRESS  HostPhyAddress;
+  EDKII_IOMMU_PPI   *IoMmu;
 
   *HostAddress = NULL;
   *DeviceAddress = 0;
 
-  if 

[edk2] [PATCH v5 00/13] Split the S3 PEI phase HW init codes from Opal driver

2019-02-14 Thread Hao Wu
The series is also available at:
https://github.com/hwu25/edk2/tree/opal_remodel_v5

V5 changes:
For patch 11, as suggested by Star:
* Refine the UpdateLockBox() API description comment to be more precise;
* Ensure the parameter for macro 'EFI_SIZE_TO_PAGES' is of type 'UINTN';

Since the changes are minor, I still keep the origin 'Reviewed-by' from
Ray and the 'Acked-by' tag from Laszlo.

For patch 12, sync the UpdateLockBox() API description comment with the
change made in patch 11.

Since the changes are minor, I still keep the origin 'Reviewed-by' from
Laszlo an Ray.

For patch 13, additional handling logic to prevent NULL pointer
de-reference, just as what has been done in commit
d72d8561fbe03a64e01c2ad57a93777de4b9ae2f.


V4 history:

As suggested by Laszlo, add a new patch to sync the API description
comment for UpdateLockBox() in file
"OvmfPkg/Library/LockBoxLib/LockBoxLib.c" with its counterparts in
MdeModulePkg.

For patch 13 (compared with patch 12 in V3), minor type refinement in
structure definition 'OPAL_DEVICE_LOCKBOX_DATA'.

V3 history:

For patch 2, reuse the definitions within AtaPassThru protocol header file
to reduce code duplication.

For patch 4, add detailed comments to illustrate the content that will be
stored in the newly introduced LockBox.

For patch 6, device path validity check refinement within function
NvmeCheckHcDevicePath().

For patch 7:
* Remove a redundant check within function NvmeS3SkipThisController();
* Replace internal implementation of GetNextDevicePathInstance() with
  GetDevicePathInstanceSize(), which avoids unnecessary memory allocation.

For patch 8:
* Device path validity check refinement within function
  AhciCheckHcDevicePath();
* Remove a redundant check within function AhciS3GetEumeratePorts();
* Replace internal implementation of GetNextDevicePathInstance() with
  GetDevicePathInstanceSize(), which avoids unnecessary memory allocation.

For patch 11:
* Remove the ASSERT() for memory allocation failure. Since error handling
  codes already exist, no new code is added for this;
* Refine the codes to only allocate new SMM buffer when the required
  LockBox size is greater than the size of origin pages allocated;
* Add additional parameter check for 'Offset' & 'Length' to prevent
  potential numeric overflow.


V2 history:

For patch 8, the new series removes the codes to produce the Block IO PPIs
from the AhciPei driver.

The task to produce the Block IO services is out of the scope of BZ-1409
(actually covered by BZ-1483). And we will later propose seperate patch(s)
to address this.

V1 history:

For the below 2 types of storage device:

1. NVM Express devices;
2. ATA hard disk devices working under AHCI mode.

the OpalPassword driver supports auto-unlocking those devices during S3
resume.

Current implementation of the OpalPassword driver is handling the device
initialization (using boot script to restore the host controller PCI
configuration space also counts) in the PEI phase during S3 resume by
itself.

Meanwhile, the NvmExpressPei driver in MdeModulePkg also handles the NVME
device initialization during the PEI phase in order to produce the Block
IO PPI.

Moreover, there is a Bugzilla request (BZ-1483) for adding the Block IO
PPI support for ATA device as well. So there is likely to be an PEI driver
for ATA device that will handle the ATA device initialization.

In order to remove code duplication, the series will split the S3 phase
device initialization related codes out from the OpalPassword driver. And
let the existing NvmExpressPei driver and the new AhciPei driver to handle
the task.

After this remodel, NvmExpressPei and AhciPei drivers will produce a PPI
called Storage Security Command PPI. And the OpalPassword driver will
consume this PPI to perform the device auto-unlock in S3 resume.


Patch   1~4: Add the definitions of PPIs and GUIDs;
Patch 5: Refinement for the NvmExpressPei driver;
Patch   6~7: Update the NvmExpressPei driver to produce the PPI needed by
 OpalPassword;
Patch 8: Add the Ahci mode ATA device support in the PEI phase, it
 will produce the PPI needed by OpalPassword;
Patch  9~10: Refinements for the SmmLockBoxLib;
Patch11: Support LockBox enlarge for LockBoxLib API UpdateLockBox();
Patch12: Remove the hardware initialization codes from the
 OpalPassword driver. And consume the SSC PPI to unlock device
 in S3 resume.

Cc: Jian J Wang 
Cc: Ray Ni 
Cc: Eric Dong 
Cc: Star Zeng 
Cc: Chao Zhang 
Cc: Jiewen Yao 
Cc: Laszlo Ersek 

Hao Wu (12):
  MdeModulePkg: Add definitions for ATA AHCI host controller PPI
  MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI
  MdeModulePkg: Add definitions for Storage Security Command PPI
  MdeModulePkg: Add GUID for LockBox to save storage dev to init in S3
  MdeModulePkg/NvmExpressPei: Avoid updating the module-level variable
  MdeModulePkg/NvmExpressPei: Add logic to produce SSC PPI
  MdeModulePkg/NvmExpressPei: 

[edk2] [PATCH v5 03/13] MdeModulePkg: Add definitions for Storage Security Command PPI

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

This commit will add the definitions for Storage Security Command (SSC)
PPI. This PPI will be be used to abstract mass storage devices to allow
code running in the PEI phase to send security protocol commands to mass
storage devices without specific knowledge of the type of device or
controller that manages the device.

More specifically, the PPI will provide services to:

* Get the number of mass storage devices managed by a instance of the SSC
  PPI (by service 'GetNumberofDevices');
* Get the identification information (DevicePath) of a managing mass
  storage devices (by service 'GetDevicePath');
* Send security protocol commands to mass storage devices (by services
  'ReceiveData' and 'SendData').

Cc: Jian J Wang 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
---
 MdeModulePkg/MdeModulePkg.dec |   3 +
 MdeModulePkg/Include/Ppi/StorageSecurityCommand.h | 283 
 2 files changed, 286 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 8efb19e626..7f646d7702 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -483,6 +483,9 @@
   ## Include/Ppi/AtaAhciController.h
   gEdkiiPeiAtaAhciHostControllerPpiGuid = { 0x61dd33ea, 0x421f, 0x4cc0, { 
0x89, 0x29, 0xff, 0xee, 0xa9, 0xa1, 0xa2, 0x61 } }
 
+  ## Include/Ppi/StorageSecurityCommand.h
+  gEdkiiPeiStorageSecurityCommandPpiGuid= { 0x35de0b4e, 0x30fb, 0x46c3, { 
0xbd, 0x84, 0x1f, 0xdb, 0xa1, 0x58, 0xbb, 0x56 } }
+
   ## Include/Ppi/AtaPassThru.h
   gEdkiiPeiAtaPassThruPpiGuid   = { 0xa16473fd, 0xd474, 0x4c89, { 
0xae, 0xc7, 0x90, 0xb8, 0x3c, 0x73, 0x86, 0x9  } }
 
diff --git a/MdeModulePkg/Include/Ppi/StorageSecurityCommand.h 
b/MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
new file mode 100644
index 00..cc1688dabb
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
@@ -0,0 +1,283 @@
+/** @file
+
+  Copyright (c) 2019, Intel Corporation. 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 _EDKII_STORAGE_SECURITY_COMMAND_PPI_H_
+#define _EDKII_STORAGE_SECURITY_COMMAND_PPI_H_
+
+#include 
+
+///
+/// Global ID for the EDKII_PEI_STORAGE_SECURITY_CMD_PPI.
+///
+#define EDKII_PEI_STORAGE_SECURITY_CMD_PPI_GUID \
+  { \
+0x35de0b4e, 0x30fb, 0x46c3, { 0xbd, 0x84, 0x1f, 0xdb, 0xa1, 0x58, 0xbb, 
0x56 } \
+  }
+
+//
+// Forward declaration for the EDKII_PEI_STORAGE_SECURITY_CMD_PPI.
+//
+typedef struct _EDKII_PEI_STORAGE_SECURITY_CMD_PPI  
EDKII_PEI_STORAGE_SECURITY_CMD_PPI;
+
+//
+// Revision The revision to which the Storage Security Command interface 
adheres.
+//  All future revisions must be backwards compatible.
+//  If a future version is not back wards compatible it is not the 
same GUID.
+//
+#define EDKII_STORAGE_SECURITY_PPI_REVISION  0x0001
+
+
+/**
+  Gets the count of storage security devices that one specific driver detects.
+
+  @param[in]  This   The PPI instance pointer.
+  @param[out] NumberofDevicesThe number of storage security devices 
discovered.
+
+  @retval EFI_SUCCESS  The operation performed successfully.
+  @retval EFI_INVALID_PARAMETERThe parameters are invalid.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_STORAGE_SECURITY_GET_NUMBER_DEVICES) (
+  IN  EDKII_PEI_STORAGE_SECURITY_CMD_PPI*This,
+  OUT UINTN *NumberofDevices
+  );
+
+/**
+  Gets the device path of a specific storage security device.
+
+  @param[in]  This The PPI instance pointer.
+  @param[in]  DeviceIndex  Specifies the storage security device to 
which
+   the function wants to talk. Because the 
driver
+   that implements Storage Security Command 
PPIs
+   will manage multiple storage devices, the 
PPIs
+   that want to talk to a single device must 
specify
+   the device index that was assigned during 
the
+   enumeration process. This index is a number 
from
+   one to NumberofDevices.
+  @param[out] DevicePathLength The length of the device path in bytes 
specified
+   by DevicePath.
+  @param[out] DevicePath   The device path of storage security device.
+   This field 

Re: [edk2] [PATCH v4 09/13] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_'

2019-02-14 Thread Zeng, Star
Reviewed-by: Star Zeng 

-Original Message-
From: Wu, Hao A 
Sent: Monday, February 11, 2019 3:58 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Wang, Jian J ; Zeng, 
Star 
Subject: [PATCH v4 09/13] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix 
instead of 'EFI_D_'

This commit is out of the scope for BZ-1409. It is a coding style refinement 
for the SmmLockBoxLib.

More specifically, the commit will remove all the debug message display level 
macros starting with 'EFI_D_' and replace them with macros starting with 
'DEBUG_'.

Cc: Jian J Wang 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 22 +++---  
MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 26 +++  
MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c | 78 ++--
 3 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index ac8bcd2ff7..0428decbac 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions @@ -153,7 +153,7 @@ SaveLockBox (
   UINT8   *CommBuffer;
   UINTN   CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SaveLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SaveLockBox - Enter\n"));
 
   //
   // Basic check
@@ -199,7 +199,7 @@ SaveLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterSave->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SaveLockBox - Exit (%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SaveLockBox - Exit (%r)\n", 
+ Status));
 
   //
   // Done
@@ -235,7 +235,7 @@ SetLockBoxAttributes (
   UINT8 *CommBuffer;
   UINTN CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - 
+ Enter\n"));
 
   //
   // Basic check
@@ -281,7 +281,7 @@ SetLockBoxAttributes (
 
   Status = (EFI_STATUS)LockBoxParameterSetAttributes->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Exit (%r)\n", 
Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Exit 
+ (%r)\n", Status));
 
   //
   // Done
@@ -322,7 +322,7 @@ UpdateLockBox (
   UINT8 *CommBuffer;
   UINTN CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib UpdateLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib UpdateLockBox - Enter\n"));
 
   //
   // Basic check
@@ -369,7 +369,7 @@ UpdateLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterUpdate->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib UpdateLockBox - Exit (%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib UpdateLockBox - Exit (%r)\n", 
+ Status));
 
   //
   // Done
@@ -411,7 +411,7 @@ RestoreLockBox (
   UINT8  *CommBuffer;
   UINTN  CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreLockBox - Enter\n"));
 
   //
   // Basic check
@@ -467,7 +467,7 @@ RestoreLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterRestore->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreLockBox - Exit (%r)\n", 
Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreLockBox - Exit (%r)\n", 
+ Status));
 
   //
   // Done
@@ -496,7 +496,7 @@ RestoreAllLockBoxInPlace (
   UINT8   *CommBuffer;
   UINTN   CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - 
+ Enter\n"));
 
   SmmCommunication = LockBoxGetSmmCommProtocol ();
   if (SmmCommunication == NULL) {
@@ -532,7 +532,7 @@ RestoreAllLockBoxInPlace (
 
   Status = (EFI_STATUS)LockBoxParameterRestoreAllInPlace->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Exit 
(%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Exit 
+ (%r)\n", Status));
 
   //
   // Done
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 8a168663c4..9f73480070 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ 

[edk2] [PATCH v5 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox()

2019-02-14 Thread Hao Wu
This commit is out of the scope for BZ-1409. It is a refinement for the
PEI library instance within SmmLockBoxLib.

For the below ASSERT statement within function RestoreLockBox():
  Status = SmmCommunicationPpi->Communicate (
  SmmCommunicationPpi,
  [0],
  
  );
  if (Status == EFI_NOT_STARTED) {
//
// Pei SMM communication not ready yet, so we access SMRAM directly
//
DEBUG ((DEBUG_INFO, "SmmLockBoxPeiLib Communicate - (%r)\n", Status));
Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
if (Length != NULL) {
  LockBoxParameterRestore->Length = (UINT64)*Length;
}
  }
  ASSERT_EFI_ERROR (Status);

It is possible for previous codes to return an error status that is
possible for happen. One example is that, when the 'if' statement
'if (Status == EFI_NOT_STARTED) {' is entered, function
InternalRestoreLockBoxFromSmram() is possible to return 'BUFFER_TOO_SMALL'
if the caller of RestoreLockBox() provides a buffer that is too small to
hold the content of LockBox.

Thus, this commit will remove the ASSERT here.

Please note that the current implementation of RestoreLockBox() is
handling the above-mentioned error case properly, so no additional error
handling codes are needed here.

Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 9f73480070..8c3e65bc96 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -623,7 +623,6 @@ RestoreLockBox (
   LockBoxParameterRestore->Length = (UINT64)*Length;
 }
   }
-  ASSERT_EFI_ERROR (Status);
 
   if (Length != NULL) {
 *Length = (UINTN)LockBoxParameterRestore->Length;
-- 
2.12.0.windows.1

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


[edk2] [PATCH v5 12/13] OvmfPkg/LockBoxLib: Update the comments for API UpdateLockBox()

2019-02-14 Thread Hao Wu
The previous commit:
MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox()

adds the support to enlarge a LockBox when using the LockBoxLib API
UpdateLockBox().

This commit is to sync the API description comment of UpdateLockBox() with
its counterparts in MdeModulePkg.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Ray Ni 
---
 OvmfPkg/Library/LockBoxLib/LockBoxLib.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
index 45481b9230..04233059f1 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
@@ -3,7 +3,7 @@
   Library implementing the LockBox interface for OVMF
 
   Copyright (C) 2013, Red Hat, Inc.
-  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2019, Intel Corporation. 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
@@ -224,8 +224,13 @@ SetLockBoxAttributes (
   @retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or
 Length is 0.
   @retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold
-new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox without attribute
+LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY, the
+original buffer to too small to hold new
+information.
+  @retval RETURN_OUT_OF_RESOURCES   for lockbox with attribute
+LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY, no
+enough resource to save the information.
   @retval RETURN_ACCESS_DENIED  it is too late to invoke this interface
   @retval RETURN_NOT_STARTEDit is too early to invoke this interface
   @retval RETURN_UNSUPPORTEDthe service is not supported by
-- 
2.12.0.windows.1

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


[edk2] [PATCH v5 09/13] MdeModulePkg/SmmLockBoxLib: Use 'DEBUG_' prefix instead of 'EFI_D_'

2019-02-14 Thread Hao Wu
This commit is out of the scope for BZ-1409. It is a coding style
refinement for the SmmLockBoxLib.

More specifically, the commit will remove all the debug message display
level macros starting with 'EFI_D_' and replace them with macros starting
with 'DEBUG_'.

Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c | 22 +++---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 26 +++
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c | 78 ++--
 3 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index ac8bcd2ff7..0428decbac 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -153,7 +153,7 @@ SaveLockBox (
   UINT8   *CommBuffer;
   UINTN   CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SaveLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SaveLockBox - Enter\n"));
 
   //
   // Basic check
@@ -199,7 +199,7 @@ SaveLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterSave->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SaveLockBox - Exit (%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SaveLockBox - Exit (%r)\n", Status));
 
   //
   // Done
@@ -235,7 +235,7 @@ SetLockBoxAttributes (
   UINT8 *CommBuffer;
   UINTN CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Enter\n"));
 
   //
   // Basic check
@@ -281,7 +281,7 @@ SetLockBoxAttributes (
 
   Status = (EFI_STATUS)LockBoxParameterSetAttributes->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Exit (%r)\n", 
Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib SetLockBoxAttributes - Exit (%r)\n", 
Status));
 
   //
   // Done
@@ -322,7 +322,7 @@ UpdateLockBox (
   UINT8 *CommBuffer;
   UINTN CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib UpdateLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib UpdateLockBox - Enter\n"));
 
   //
   // Basic check
@@ -369,7 +369,7 @@ UpdateLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterUpdate->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib UpdateLockBox - Exit (%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib UpdateLockBox - Exit (%r)\n", Status));
 
   //
   // Done
@@ -411,7 +411,7 @@ RestoreLockBox (
   UINT8  *CommBuffer;
   UINTN  CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreLockBox - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreLockBox - Enter\n"));
 
   //
   // Basic check
@@ -467,7 +467,7 @@ RestoreLockBox (
 
   Status = (EFI_STATUS)LockBoxParameterRestore->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreLockBox - Exit (%r)\n", 
Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreLockBox - Exit (%r)\n", 
Status));
 
   //
   // Done
@@ -496,7 +496,7 @@ RestoreAllLockBoxInPlace (
   UINT8   *CommBuffer;
   UINTN   CommSize;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Enter\n"));
 
   SmmCommunication = LockBoxGetSmmCommProtocol ();
   if (SmmCommunication == NULL) {
@@ -532,7 +532,7 @@ RestoreAllLockBoxInPlace (
 
   Status = (EFI_STATUS)LockBoxParameterRestoreAllInPlace->Header.ReturnStatus;
 
-  DEBUG ((EFI_D_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Exit 
(%r)\n", Status));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxDxeLib RestoreAllLockBoxInPlace - Exit 
(%r)\n", Status));
 
   //
   // Done
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 8a168663c4..9f73480070 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ 

[edk2] [PATCH v5 07/13] MdeModulePkg/NvmExpressPei: Consume S3StorageDeviceInitList LockBox

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

For the NvmExpressPei driver, this commit will update the driver to
consume the S3StorageDeviceInitList LockBox in S3 phase. The purpose is to
perform an on-demand (partial) NVM Express device
enumeration/initialization to benefit the S3 resume performance.

Cc: Jian J Wang 
Cc: Ray Ni 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf |   8 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h   |  36 +++
 MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c  |  53 +
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c   |  20 
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiS3.c | 114 
 5 files changed, 230 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
index 0666e5892b..22b703e971 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.inf
@@ -40,6 +40,7 @@
   NvmExpressPeiHci.h
   NvmExpressPeiPassThru.c
   NvmExpressPeiPassThru.h
+  NvmExpressPeiS3.c
   NvmExpressPeiStorageSecurity.c
   NvmExpressPeiStorageSecurity.h
 
@@ -54,6 +55,7 @@
   BaseMemoryLib
   IoLib
   TimerLib
+  LockBoxLib
   PeimEntryPoint
 
 [Ppis]
@@ -64,9 +66,13 @@
   gEfiPeiVirtualBlockIo2PpiGuid  ## SOMETIMES_PRODUCES
   gEdkiiPeiStorageSecurityCommandPpiGuid ## SOMETIMES_PRODUCES
 
+[Guids]
+  gS3StorageDeviceInitListGuid   ## SOMETIMES_CONSUMES ## 
UNDEFINED
+
 [Depex]
   gEfiPeiMemoryDiscoveredPpiGuid AND
-  gEdkiiPeiNvmExpressHostControllerPpiGuid
+  gEdkiiPeiNvmExpressHostControllerPpiGuid AND
+  gEfiPeiMasterBootModePpiGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
   NvmExpressPeiExtra.uni
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
index 92c3854c6e..e2a693abe8 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.h
@@ -267,6 +267,26 @@ NvmePeimEndOfPei (
   );
 
 /**
+  Get the size of the current device path instance.
+
+  @param[in]  DevicePath A pointer to the EFI_DEVICE_PATH_PROTOCOL
+ structure.
+  @param[out] InstanceSize   The size of the current device path 
instance.
+  @param[out] EntireDevicePathEndIndicate whether the instance is the last
+ one in the device path strucure.
+
+  @retval EFI_SUCCESSThe size of the current device path instance is 
fetched.
+  @retval Others Fails to get the size of the current device path 
instance.
+
+**/
+EFI_STATUS
+GetDevicePathInstanceSize (
+  IN  EFI_DEVICE_PATH_PROTOCOL*DevicePath,
+  OUT UINTN   *InstanceSize,
+  OUT BOOLEAN *EntireDevicePathEnd
+  );
+
+/**
   Check the validity of the device path of a NVM Express host controller.
 
   @param[in] DevicePath  A pointer to the EFI_DEVICE_PATH_PROTOCOL
@@ -309,4 +329,20 @@ NvmeBuildDevicePath (
   OUT EFI_DEVICE_PATH_PROTOCOL**DevicePath
   );
 
+/**
+  Determine if a specific NVM Express controller can be skipped for S3 phase.
+
+  @param[in]  HcDevicePath  Device path of the controller.
+  @param[in]  HcDevicePathLengthLength of the device path specified by
+HcDevicePath.
+
+  @retvalThe number of ports that need to be enumerated.
+
+**/
+BOOLEAN
+NvmeS3SkipThisController (
+  IN  EFI_DEVICE_PATH_PROTOCOL*HcDevicePath,
+  IN  UINTN   HcDevicePathLength
+  );
+
 #endif
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c 
b/MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c
index 5dab447f09..ed05d7a2be 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/DevicePath.c
@@ -89,6 +89,59 @@ NextDevicePathNode (
 }
 
 /**
+  Get the size of the current device path instance.
+
+  @param[in]  DevicePath A pointer to the EFI_DEVICE_PATH_PROTOCOL
+ structure.
+  @param[out] InstanceSize   The size of the current device path 
instance.
+  @param[out] EntireDevicePathEndIndicate whether the instance is the last
+ one in the device path strucure.
+
+  @retval EFI_SUCCESSThe size of the current device path instance is 
fetched.
+  @retval Others Fails to get the size of the current device path 
instance.
+
+**/
+EFI_STATUS
+GetDevicePathInstanceSize (
+  IN  EFI_DEVICE_PATH_PROTOCOL*DevicePath,
+  OUT UINTN   *InstanceSize,
+  OUT BOOLEAN *EntireDevicePathEnd
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL*Walker;
+
+  if (DevicePath == NULL || InstanceSize == 

[edk2] [PATCH v5 11/13] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox()

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409

This commit will add the support to enlarge a LockBox when using the
LockBoxLib API UpdateLockBox().

Please note that the new support will ONLY work for LockBox with attribute
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY set.

The functional uni-test for the commit is available at:
https://github.com/hwu25/edk2/tree/lockbox_unitest

Cc: Jian J Wang 
Cc: Ray Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Acked-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
---
 MdeModulePkg/Include/Library/LockBoxLib.h |  7 +-
 MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c  |  7 +-
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c |  5 +-
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c |  5 +-
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c | 72 ++--
 5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h 
b/MdeModulePkg/Include/Library/LockBoxLib.h
index 5921731419..b457bd4241 100644
--- a/MdeModulePkg/Include/Library/LockBoxLib.h
+++ b/MdeModulePkg/Include/Library/LockBoxLib.h
@@ -2,7 +2,7 @@
   This library is only intended to be used by DXE modules that need save
   confidential information to LockBox and get it by PEI modules in S3 phase.
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -85,7 +85,10 @@ SetLockBoxAttributes (
   @retval RETURN_SUCCESSthe information is saved successfully.
   @retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
   @retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold 
new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox without attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+the original buffer to too small to hold 
new information.
+  @retval RETURN_OUT_OF_RESOURCES   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+no enough resource to save the information.
   @retval RETURN_ACCESS_DENIED  it is too late to invoke this interface
   @retval RETURN_NOT_STARTEDit is too early to invoke this interface
   @retval RETURN_UNSUPPORTEDthe service is not supported by 
implementaion.
diff --git a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c 
b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
index c40dfea398..0c3a762a6f 100644
--- a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
+++ b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -76,7 +76,10 @@ SetLockBoxAttributes (
   @retval RETURN_SUCCESSthe information is saved successfully.
   @retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
   @retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold 
new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox without attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+the original buffer to too small to hold 
new information.
+  @retval RETURN_OUT_OF_RESOURCES   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+no enough resource to save the information.
   @retval RETURN_ACCESS_DENIED  it is too late to invoke this interface
   @retval RETURN_NOT_STARTEDit is too early to invoke this interface
   @retval RETURN_UNSUPPORTEDthe service is not supported by 
implementaion.
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index 0428decbac..db8322631c 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -300,7 +300,10 @@ SetLockBoxAttributes (
   @retval RETURN_SUCCESSthe information is saved successfully.
   @retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
   @retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold 
new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox without attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+the original buffer to too small to hold 
new 

Re: [edk2] [PATCH v4 11/13] MdeModulePkg/SmmLockBoxLib: Support LockBox enlarge in UpdateLockBox()

2019-02-14 Thread Zeng, Star

Some minor comments.
With them addressed, Reviewed-by: Star Zeng .

On 2019/2/11 15:57, Hao Wu wrote:

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

This commit will add the support to enlarge a LockBox when using the
LockBoxLib API UpdateLockBox().

Please note that the new support will ONLY work for LockBox with attribute
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY set.

The functional uni-test for the commit is available at:
https://github.com/hwu25/edk2/tree/lockbox_unitest

Cc: Jian J Wang 
Cc: Ray Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Acked-by: Laszlo Ersek 
---
  MdeModulePkg/Include/Library/LockBoxLib.h |  7 +-
  MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c  |  7 +-
  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c |  5 +-
  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c |  5 +-
  MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c | 72 
++--
  5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h 
b/MdeModulePkg/Include/Library/LockBoxLib.h
index 5921731419..addce3bd4a 100644
--- a/MdeModulePkg/Include/Library/LockBoxLib.h
+++ b/MdeModulePkg/Include/Library/LockBoxLib.h
@@ -2,7 +2,7 @@
This library is only intended to be used by DXE modules that need save
confidential information to LockBox and get it by PEI modules in S3 phase.
  
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.

+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
  
  This program and the accompanying materials

  are licensed and made available under the terms and conditions
@@ -85,7 +85,10 @@ SetLockBoxAttributes (
@retval RETURN_SUCCESSthe information is saved successfully.
@retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
@retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold 
new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,


Here, the description should be "without attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY".



+the original buffer to too small to hold 
new information.
+  @retval RETURN_OUT_OF_RESOURCES   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+no enough resource to save the information.
@retval RETURN_ACCESS_DENIED  it is too late to invoke this interface
@retval RETURN_NOT_STARTEDit is too early to invoke this interface
@retval RETURN_UNSUPPORTEDthe service is not supported by 
implementaion.
diff --git a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c 
b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
index c40dfea398..0adda1e2a9 100644
--- a/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
+++ b/MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.c
@@ -1,6 +1,6 @@
  /** @file
  
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.

+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
  
  This program and the accompanying materials

  are licensed and made available under the terms and conditions
@@ -76,7 +76,10 @@ SetLockBoxAttributes (
@retval RETURN_SUCCESSthe information is saved successfully.
@retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
@retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too small to hold 
new information.
+  @retval RETURN_BUFFER_TOO_SMALL   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE,
+the original buffer to too small to hold 
new information.
+  @retval RETURN_OUT_OF_RESOURCES   for lockbox with attribute 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY,
+no enough resource to save the information.
@retval RETURN_ACCESS_DENIED  it is too late to invoke this interface
@retval RETURN_NOT_STARTEDit is too early to invoke this interface
@retval RETURN_UNSUPPORTEDthe service is not supported by 
implementaion.
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index 0428decbac..5ee563b71f 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -300,7 +300,10 @@ SetLockBoxAttributes (
@retval RETURN_SUCCESSthe information is saved successfully.
@retval RETURN_INVALID_PARAMETER  the Guid is NULL, or Buffer is NULL, or 
Length is 0.
@retval RETURN_NOT_FOUND  the requested GUID not found.
-  @retval RETURN_BUFFER_TOO_SMALL   the original buffer to too 

Re: [edk2] [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in RestoreLockBox()

2019-02-14 Thread Zeng, Star
Reviewed-by: Star Zeng 

-Original Message-
From: Wu, Hao A 
Sent: Monday, February 11, 2019 3:58 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Wang, Jian J ; Zeng, 
Star 
Subject: [PATCH v4 10/13] MdeModulePkg/SmmLockBox(PEI): Remove an ASSERT in 
RestoreLockBox()

This commit is out of the scope for BZ-1409. It is a refinement for the PEI 
library instance within SmmLockBoxLib.

For the below ASSERT statement within function RestoreLockBox():
  Status = SmmCommunicationPpi->Communicate (
  SmmCommunicationPpi,
  [0],
  
  );
  if (Status == EFI_NOT_STARTED) {
//
// Pei SMM communication not ready yet, so we access SMRAM directly
//
DEBUG ((DEBUG_INFO, "SmmLockBoxPeiLib Communicate - (%r)\n", Status));
Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
if (Length != NULL) {
  LockBoxParameterRestore->Length = (UINT64)*Length;
}
  }
  ASSERT_EFI_ERROR (Status);

It is possible for previous codes to return an error status that is possible 
for happen. One example is that, when the 'if' statement 'if (Status == 
EFI_NOT_STARTED) {' is entered, function
InternalRestoreLockBoxFromSmram() is possible to return 'BUFFER_TOO_SMALL'
if the caller of RestoreLockBox() provides a buffer that is too small to hold 
the content of LockBox.

Thus, this commit will remove the ASSERT here.

Please note that the current implementation of RestoreLockBox() is handling the 
above-mentioned error case properly, so no additional error handling codes are 
needed here.

Cc: Jian J Wang 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index 9f73480070..8c3e65bc96 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -623,7 +623,6 @@ RestoreLockBox (
   LockBoxParameterRestore->Length = (UINT64)*Length;
 }
   }
-  ASSERT_EFI_ERROR (Status);
 
   if (Length != NULL) {
 *Length = (UINTN)LockBoxParameterRestore->Length;
--
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 v2 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chiu, Chasel



> -Original Message-
> From: Ni, Ray
> Sent: Thursday, February 14, 2019 4:25 PM
> To: Chiu, Chasel ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Laszlo Ersek 
> Subject: RE: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> 
> 
> > -Original Message-
> > From: Chiu, Chasel 
> > Sent: Wednesday, February 13, 2019 5:47 PM
> > To: edk2-devel@lists.01.org
> > Cc: Dong, Eric ; Ni, Ray ;
> > Laszlo Ersek ; Chiu, Chasel 
> > Subject: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform when PeiCore
> > not in BFV so SecCore has to search PeiCore either from the FV
> > location provided by EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Eric Dong 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu 
> > ---
> >  UefiCpuPkg/SecCore/SecMain.c   | 35
> > +--
> >  UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
> >  UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
> >  3 files changed, 33 insertions(+), 8 deletions(-)
> >
> > diff --git a/UefiCpuPkg/SecCore/SecMain.c
> > b/UefiCpuPkg/SecCore/SecMain.c index b24e190617..b99072599d 100644
> > --- a/UefiCpuPkg/SecCore/SecMain.c
> > +++ b/UefiCpuPkg/SecCore/SecMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >C functions in SEC
> >
> > -  Copyright (c) 2008 - 2018, Intel Corporation. All rights
> > reserved.
> > +  Copyright (c) 2008 - 2019, Intel Corporation. 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 @@ -228,26 +228,49 @@ SecStartupPhase2(  {
> >EFI_SEC_PEI_HAND_OFF*SecCoreData;
> >EFI_PEI_PPI_DESCRIPTOR  *PpiList;
> > +  EFI_PEI_PPI_DESCRIPTOR  *PpiListTmp;
> 
> 
> 1. Maybe this local variable is not needed. We can use PpiList[Index].
Agree! I will correct this.

> 
> >UINT32  Index;
> >EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
> >EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
> >
> > +  PeiCoreEntryPoint = NULL;
> >SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
> >AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData-
> > >PeiTemporaryRamBase;
> > +
> >//
> >// Find Pei Core entry point. It will report SEC and Pei Core debug
> > information if remote debug
> >// is enabled.
> >//
> > -  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> > SecCoreData->BootFirmwareVolumeBase, );
> > -  if (PeiCoreEntryPoint == NULL)
> > -  {
> > -CpuDeadLoop ();
> > +  PpiList = SecPlatformMain (SecCoreData);  PpiListTmp = PpiList;
> > + for
> > + (;;) {
> 
> 2. Similar comments as above. Maybe we can just use PpiList[Index] in the 
> loop.
> By the way, original code logic checks PpiList against NULL.
> Do we still need to make sure to deference PpiList after checking against 
> NULL?
Good catch! I will update.

> 
> > +if (CompareGuid (PpiListTmp->Guid, )
> > + &&
> > (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)-
> > >PeiCoreFvLocation != 0)) {
> > +  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> > ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation,
> > );
> > +  if (PeiCoreEntryPoint != NULL) {
> > +break;
> 
> 3. Is it valid that PeiCore cannot be found in the PeiCoreFvLocation?
> If no, can we just dead-loop here when PeiCoreEntryPoint is NULL?
Yes. I will add dead-loop.

> 
> > +  }
> > +}
> > +if ((PpiListTmp->Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST)
> > + ==
> > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) {
> > +  //
> > +  // Continue until the end of the PPI List.
> > +  //
> > +  break;
> > +}
> > +PpiListTmp++;
> > +  }
> > +  //
> > +  // If EFI_PEI_CORE_FV_LOCATION_PPI not found or no PeiCore found by
> > the pointer in provided PPI, try to locate PeiCore from BFV.
> > +  //
> > +  if (PeiCoreEntryPoint == NULL) {
> > +FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> > SecCoreData->BootFirmwareVolumeBase, );
> > +if (PeiCoreEntryPoint == NULL) {
> > +  CpuDeadLoop ();
> > +}
> >}
> >
> >//
> >// Perform platform specific initialization before entering PeiCore.
> >//
> > -  PpiList = SecPlatformMain (SecCoreData);
> >if (PpiList != NULL) {
> >  //
> >  // Remove the terminal flag from the terminal PPI diff --git
> > a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
> > index 442f663911..fc1485b5cb 100644
> > --- a/UefiCpuPkg/SecCore/SecCore.inf
> > +++ b/UefiCpuPkg/SecCore/SecCore.inf
> > @@ -7,7 +7,7 @@
> >  #  protected mode, setup 

Re: [edk2] [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chiu, Chasel



> -Original Message-
> From: Ni, Ray
> Sent: Thursday, February 14, 2019 4:20 PM
> To: Chiu, Chasel ; edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Zeng, Star ; Gao, Liming 
> Subject: RE: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> Sorry, please ignore my other review mail and just check this mail.
> 3 minor comments below.
> 
> > -Original Message-
> > From: Chiu, Chasel 
> > Sent: Tuesday, February 12, 2019 9:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J ; Wu, Hao A
> > ; Ni, Ray ; Zeng, Star
> > ; Gao, Liming ; Chiu,
> > Chasel 
> > Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> > EFI_PEI_CORE_FV_LOCATION_PPI
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> >
> > When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> > checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore
> > from BFV.
> >
> > Test: Verified on internal platform and booting successfully.
> >
> > Cc: Jian J Wang 
> > Cc: Hao Wu 
> > Cc: Ray Ni 
> > Cc: Star Zeng 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chasel Chiu 
> > ---
> >  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> > +-
> >  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
> >  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
> >  3 files changed, 49 insertions(+), 15 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > index 4da80a8222..408f24c216 100644
> > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >Pei Core Main Entry Point
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) 2006 - 2019, Intel Corporation. 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 @@ -80,23 +80,55 @@ ShadowPeiCore (
> >IN PEI_CORE_INSTANCE  *PrivateData
> >)
> >  {
> > -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > -  EFI_PHYSICAL_ADDRESS EntryPoint;
> > -  EFI_STATUS   Status;
> > -  UINT32   AuthenticationState;
> > +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> > +  EFI_PHYSICAL_ADDRESS EntryPoint;
> > +  EFI_STATUS   Status;
> > +  UINT32   AuthenticationState;
> > +  UINTNIndex;
> > +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> >
> >PeiCoreFileHandle = NULL;
> >
> >//
> > -  // Find the PEI Core in the BFV
> > +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> > + indicated FV or BFV
> >//
> > -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > -   PrivateData->Fv[0].FvPpi,
> > -   EFI_FV_FILETYPE_PEI_CORE,
> > -   PrivateData->Fv[0].FvHandle,
> > -   
> > -   );
> > -  ASSERT_EFI_ERROR (Status);
> > +  Status = PeiServicesLocatePpi (
> > + ,
> > + 0,
> > + NULL,
> > + (VOID **) 
> > + );
> > +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation
> > + !=
> > NULL)) {
> > +//
> > +// If PeiCoreFvLocation present, the PEI Core should be found
> > + from
> > indicated FV.
> > +//
> > +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> > +  if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> > PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> > +continue;
> > +  }
> > +  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> > +   
> > PrivateData->Fv[Index].FvPpi,
> > +   EFI_FV_FILETYPE_PEI_CORE,
> > +   
> > PrivateData->Fv[Index].FvHandle,
> > +   
> > +   );
> > +  if (!EFI_ERROR (Status)) {
> > +break;
> 1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
> If no, why not assert here?
Agree. I will update the code.

> 
> > +  }
> > +}
> > +ASSERT (Index < PrivateData->FvCount);  } else {
> > +//
> > +// Find PEI Core from BFV
> > +//
> > +Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> > + PrivateData->Fv[0].FvPpi,
> > + EFI_FV_FILETYPE_PEI_CORE,
> > + PrivateData->Fv[0].FvHandle,
> > +

Re: [edk2] [PATCH v2 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray



> -Original Message-
> From: Chiu, Chasel 
> Sent: Wednesday, February 13, 2019 5:47 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Chiu, Chasel 
> Subject: [PATCH v2 3/3] UefiCpuPkg/SecCore: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform when PeiCore
> not in BFV so SecCore has to search PeiCore either from the FV location
> provided by EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  UefiCpuPkg/SecCore/SecMain.c   | 35
> +--
>  UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
>  UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
>  3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/SecCore/SecMain.c
> b/UefiCpuPkg/SecCore/SecMain.c index b24e190617..b99072599d 100644
> --- a/UefiCpuPkg/SecCore/SecMain.c
> +++ b/UefiCpuPkg/SecCore/SecMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>C functions in SEC
> 
> -  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) 2008 - 2019, Intel Corporation. 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 @@ -228,26 +228,49 @@ SecStartupPhase2(  {
>EFI_SEC_PEI_HAND_OFF*SecCoreData;
>EFI_PEI_PPI_DESCRIPTOR  *PpiList;
> +  EFI_PEI_PPI_DESCRIPTOR  *PpiListTmp;


1. Maybe this local variable is not needed. We can use PpiList[Index].

>UINT32  Index;
>EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
>EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
> 
> +  PeiCoreEntryPoint = NULL;
>SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
>AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData-
> >PeiTemporaryRamBase;
> +
>//
>// Find Pei Core entry point. It will report SEC and Pei Core debug
> information if remote debug
>// is enabled.
>//
> -  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> SecCoreData->BootFirmwareVolumeBase, );
> -  if (PeiCoreEntryPoint == NULL)
> -  {
> -CpuDeadLoop ();
> +  PpiList = SecPlatformMain (SecCoreData);  PpiListTmp = PpiList;  for
> + (;;) {

2. Similar comments as above. Maybe we can just use PpiList[Index] in the loop.
By the way, original code logic checks PpiList against NULL.
Do we still need to make sure to deference PpiList after checking against NULL?

> +if (CompareGuid (PpiListTmp->Guid, ) &&
> (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)-
> >PeiCoreFvLocation != 0)) {
> +  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiListTmp->Ppi)->PeiCoreFvLocation,
> );
> +  if (PeiCoreEntryPoint != NULL) {
> +break;

3. Is it valid that PeiCore cannot be found in the PeiCoreFvLocation?
If no, can we just dead-loop here when PeiCoreEntryPoint is NULL?

> +  }
> +}
> +if ((PpiListTmp->Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) ==
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) {
> +  //
> +  // Continue until the end of the PPI List.
> +  //
> +  break;
> +}
> +PpiListTmp++;
> +  }
> +  //
> +  // If EFI_PEI_CORE_FV_LOCATION_PPI not found or no PeiCore found by
> the pointer in provided PPI, try to locate PeiCore from BFV.
> +  //
> +  if (PeiCoreEntryPoint == NULL) {
> +FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> SecCoreData->BootFirmwareVolumeBase, );
> +if (PeiCoreEntryPoint == NULL) {
> +  CpuDeadLoop ();
> +}
>}
> 
>//
>// Perform platform specific initialization before entering PeiCore.
>//
> -  PpiList = SecPlatformMain (SecCoreData);
>if (PpiList != NULL) {
>  //
>  // Remove the terminal flag from the terminal PPI diff --git
> a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
> index 442f663911..fc1485b5cb 100644
> --- a/UefiCpuPkg/SecCore/SecCore.inf
> +++ b/UefiCpuPkg/SecCore/SecCore.inf
> @@ -7,7 +7,7 @@
>  #  protected mode, setup flat memory model, enable temporary memory
> and  #  call into SecStartup().
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2006 - 2019, Intel Corporation. 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
> @@ -73,6 +73,7 @@
>## NOTIFY
>## SOMETIMES_CONSUMES
>gPeiSecPerformancePpiGuid
> +  gEfiPeiCoreFvLocationPpiGuid
> 
>  [Guids]
>## 

Re: [edk2] [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.

2019-02-14 Thread Ni, Ray

Reviewed-by: Ray Ni 

On 2/13/2019 10:04 AM, Eric Dong wrote:

PcdCpuFeaturesSupport used to specify the platform policy about
what CPU features this platform supports. This value is decide by
platform owner, not by hardware. After this change, this PCD will
be used in IsCpuFeatureSupported function only.

Now RegisterCpuFeaturesLib use this PCD as an template to Get the
pcd size. Update the code logic to replace it with
PcdCpuFeaturesSetting.

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375

Cc: Ray Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 66 +++---
  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
  .../RegisterCpuFeaturesLib.c   | 10 ++--
  3 files changed, 24 insertions(+), 53 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 4ebd0025b4..762eaec277 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -74,27 +74,6 @@ GetSettingPcd (
return SettingBitMask;
  }
  
-/**

-  Worker function to get PcdCpuFeaturesSupport.
-
-  @return  The pointer to CPU feature bits mask buffer.
-**/
-UINT8 *
-GetSupportPcd (
-  VOID
-  )
-{
-  UINT8  *SupportBitMask;
-
-  SupportBitMask = AllocateCopyPool (
-  PcdGetSize (PcdCpuFeaturesSupport),
-  PcdGetPtr (PcdCpuFeaturesSupport)
-  );
-  ASSERT (SupportBitMask != NULL);
-
-  return SupportBitMask;
-}
-
  /**
Collects CPU type and feature information.
  
@@ -282,11 +261,6 @@ CpuInitDataInitialize (

ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL);
CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) 
* CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount);
ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL);
-
-  //
-  // Get support and configuration PCDs
-  //
-  CpuFeaturesData->SupportPcd   = GetSupportPcd ();
  }
  
  /**

@@ -306,7 +280,7 @@ SupportedMaskOr (
UINT8  *Data1;
UINT8  *Data2;
  
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);

+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
Data1 = SupportedFeatureMask;
Data2 = OrFeatureBitMask;
for (Index = 0; Index < BitMaskSize; Index++) {
@@ -331,7 +305,7 @@ SupportedMaskAnd (
UINT8  *Data1;
UINT8  *Data2;
  
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);

+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
Data1 = SupportedFeatureMask;
Data2 = AndFeatureBitMask;
for (Index = 0; Index < BitMaskSize; Index++) {
@@ -356,7 +330,7 @@ SupportedMaskCleanBit (
UINT8  *Data1;
UINT8  *Data2;
  
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);

+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
Data1 = SupportedFeatureMask;
Data2 = AndFeatureBitMask;
for (Index = 0; Index < BitMaskSize; Index++) {
@@ -387,7 +361,7 @@ IsBitMaskMatch (
UINT8  *Data1;
UINT8  *Data2;
  
-  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);

+  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
  
Data1 = SupportedFeatureMask;

Data2 = ComparedFeatureBitMask;
@@ -426,22 +400,22 @@ CollectProcessorData (
Entry = GetFirstNode (>FeatureList);
while (!IsNull (>FeatureList, Entry)) {
  CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
-if (IsBitMaskMatch (CpuFeaturesData->SupportPcd, CpuFeature->FeatureMask)) 
{
-  if (CpuFeature->SupportFunc == NULL) {
-//
-// If SupportFunc is NULL, then the feature is supported.
-//
-SupportedMaskOr (
-  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-  CpuFeature->FeatureMask
-  );
-  } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
CpuFeature->ConfigData)) {
-SupportedMaskOr (
-  CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
-  CpuFeature->FeatureMask
-  );
-  }
+
+if (CpuFeature->SupportFunc == NULL) {
+  //
+  // If SupportFunc is NULL, then the feature is supported.
+  //
+  SupportedMaskOr (
+CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
+CpuFeature->FeatureMask
+);
+} else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, 
CpuFeature->ConfigData)) {
+  SupportedMaskOr (
+CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask,
+CpuFeature->FeatureMask
+);
  }
+
  Entry = Entry->ForwardLink;
}
  }
@@ -646,8 +620,6 @@ AnalysisProcessorFeatures (
DumpCpuFeature 

Re: [edk2] [Patch 0/3] Simplify CPU Features solution.

2019-02-14 Thread Ni, Ray
Eric,
Please update the copyright year to 2019 for every file you changed.

> -Original Message-
> From: Dong, Eric 
> Sent: Wednesday, February 13, 2019 10:04 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: [Patch 0/3] Simplify CPU Features solution.
> 
> Changes includes:
> 1. Optimize PCD PcdCpuFeaturesUserConfiguration 2. Limit useage of
> PcdCpuFeaturesSupport 3. Remove some useless APIs.
> Detail explanation please check each patch's introduction.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> 
> Eric Dong (3):
>   UefiCpuPkg/RegisterCpuFeaturesLib: Remove useless functions.
>   UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD
> PcdCpuFeaturesUserConfiguration.
>   UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.
> 
>  .../Include/Library/RegisterCpuFeaturesLib.h   | 34 
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 95 -
> -
>  .../DxeRegisterCpuFeaturesLib.inf  |  1 -
>  .../PeiRegisterCpuFeaturesLib.inf  |  1 -
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  2 -
>  .../RegisterCpuFeaturesLib.c   | 60 ++
>  UefiCpuPkg/UefiCpuPkg.dec  |  7 +-
>  7 files changed, 42 insertions(+), 158 deletions(-)
> 
> --
> 2.15.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] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
Sorry, please ignore my other review mail and just check this mail.
3 minor comments below.

> -Original Message-
> From: Chiu, Chasel 
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Zeng, Star ; Gao, Liming
> ; Chiu, Chasel 
> Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore from
> BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +-
>  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..408f24c216 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. 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
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>IN PEI_CORE_INSTANCE  *PrivateData
>)
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS   Status;
> -  UINT32   AuthenticationState;
> +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS EntryPoint;
> +  EFI_STATUS   Status;
> +  UINT32   AuthenticationState;
> +  UINTNIndex;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>PeiCoreFileHandle = NULL;
> 
>//
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> + indicated FV or BFV
>//
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -   PrivateData->Fv[0].FvPpi,
> -   EFI_FV_FILETYPE_PEI_CORE,
> -   PrivateData->Fv[0].FvHandle,
> -   
> -   );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **) 
> + );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +//
> +// If PeiCoreFvLocation present, the PEI Core should be found from
> indicated FV.
> +//
> +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +  if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> +continue;
> +  }
> +  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +   PrivateData->Fv[Index].FvPpi,
> +   EFI_FV_FILETYPE_PEI_CORE,
> +   
> PrivateData->Fv[Index].FvHandle,
> +   
> +   );
> +  if (!EFI_ERROR (Status)) {
> +break;
1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
If no, why not assert here?

> +  }
> +}
> +ASSERT (Index < PrivateData->FvCount);  } else {
> +//
> +// Find PEI Core from BFV
> +//
> +Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> + PrivateData->Fv[0].FvPpi,
> + EFI_FV_FILETYPE_PEI_CORE,
> + PrivateData->Fv[0].FvHandle,
> + 
> + );
> +ASSERT_EFI_ERROR (Status);

2. Seems to have code duplication here. Can we have a local PeiCoreFvIndex to 
be 0 as default?
And when the gEfiPeiCoreFvLocationPpiGuid PPI exists, PeiCoreFvIndex is updated 
to the correct value.
So that FindFileByType() is only used once in source code with first parameter
PrivateData->Fv[PeiCoreFvIndex].FvPpi.

> +  }
> 
>//
>// Shadow PEI Core into memory so it will run faster diff --git
> a/MdeModulePkg/Core/Pei/PeiMain.h
> 

Re: [edk2] [PATCH 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
1. please move the #include adjacent to other #include
2. Can you explain why 

> -Original Message-
> From: Chiu, Chasel 
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Zeng, Star ; Gao, Liming
> ; Chiu, Chasel 
> Subject: [PATCH 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore from
> BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 58
> +-
>  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..408f24c216 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. 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
> @@ -80,23 +80,55 @@ ShadowPeiCore (
>IN PEI_CORE_INSTANCE  *PrivateData
>)
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS   Status;
> -  UINT32   AuthenticationState;
> +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS EntryPoint;
> +  EFI_STATUS   Status;
> +  UINT32   AuthenticationState;
> +  UINTNIndex;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> 
>PeiCoreFileHandle = NULL;
> 
>//
> -  // Find the PEI Core in the BFV
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> + indicated FV or BFV
>//
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -   PrivateData->Fv[0].FvPpi,
> -   EFI_FV_FILETYPE_PEI_CORE,
> -   PrivateData->Fv[0].FvHandle,
> -   
> -   );
> -  ASSERT_EFI_ERROR (Status);
> +  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **) 
> + );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +//
> +// If PeiCoreFvLocation present, the PEI Core should be found from
> indicated FV.
> +//
> +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +  if ((UINT32) PrivateData->Fv[Index].FvHandle != (UINT32)
> PeiCoreFvLocationPpi->PeiCoreFvLocation) {
> +continue;
> +  }
> +  Status = PrivateData->Fv[Index].FvPpi->FindFileByType (
> +   PrivateData->Fv[Index].FvPpi,
> +   EFI_FV_FILETYPE_PEI_CORE,
> +   
> PrivateData->Fv[Index].FvHandle,
> +   
> +   );
> +  if (!EFI_ERROR (Status)) {
> +break;

1. Is it valid that PEI_CORE cannot be found in the PeiCoreFvLocation?
If no, why not assert here?

> +  }
> +}
> +ASSERT (Index < PrivateData->FvCount);  } else {
> +//
> +// Find PEI Core from BFV
> +//
> +Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> + PrivateData->Fv[0].FvPpi,
> + EFI_FV_FILETYPE_PEI_CORE,
> + PrivateData->Fv[0].FvHandle,
> + 
> + );
> +ASSERT_EFI_ERROR (Status);
> +  }
> 
>//
>// Shadow PEI Core into memory so it will run faster diff --git
> a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h index 322e7cd845..38542ab072
> 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel 

Re: [edk2] [Patch 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD PcdCpuFeaturesUserConfiguration.

2019-02-14 Thread Ni, Ray



> -Original Message-
> From: Dong, Eric 
> Sent: Wednesday, February 13, 2019 10:04 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: [Patch 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD
> PcdCpuFeaturesUserConfiguration.
> 
> In current implementation, PCD PcdCpuFeaturesUserConfiguration used as
> user enabled CPU features list. It is initialzied in platform driver and as an
> input for CpuFeatures driver. PCD PcdCpuFeaturesSetting used as an output
> for the final enabled CPU features list. For now,
> PcdCpuFeaturesUserConfiguration is only used as an input and
> PcdCpuFeaturesSetting only used as an output.
> 
> This change merge PcdCpuFeaturesUserConfiguration into
> PcdCpuFeaturesSetting.
> Use PcdCpuFeaturesSetting as input for the user input feature setting Use
> PcdCpuFeaturesSetting as output for the final CPU feature setting
> 
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 37 --
> 
>  .../DxeRegisterCpuFeaturesLib.inf  |  1 -
>  .../PeiRegisterCpuFeaturesLib.inf  |  1 -
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
>  UefiCpuPkg/UefiCpuPkg.dec  |  7 ++--
>  5 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index bae92b89a6..4ebd0025b4 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -54,41 +54,41 @@ SetSettingPcd (
>  }
> 
>  /**
> -  Worker function to get PcdCpuFeaturesSupport.
> +  Worker function to get PcdCpuFeaturesSetting.
> 
>@return  The pointer to CPU feature bits mask buffer.
>  **/
>  UINT8 *
> -GetSupportPcd (
> +GetSettingPcd (
>VOID
>)
>  {
> -  UINT8  *SupportBitMask;
> +  UINT8  *SettingBitMask;
> 
> -  SupportBitMask = AllocateCopyPool (
> -  PcdGetSize (PcdCpuFeaturesSupport),
> -  PcdGetPtr (PcdCpuFeaturesSupport)
> +  SettingBitMask = AllocateCopyPool (
> +  PcdGetSize (PcdCpuFeaturesSetting),
> +  PcdGetPtr (PcdCpuFeaturesSetting)
>);
> -  ASSERT (SupportBitMask != NULL);
> +  ASSERT (SettingBitMask != NULL);
> 
> -  return SupportBitMask;
> +  return SettingBitMask;
>  }
> 
>  /**
> -  Worker function to get PcdCpuFeaturesUserConfiguration.
> +  Worker function to get PcdCpuFeaturesSupport.
> 
>@return  The pointer to CPU feature bits mask buffer.
>  **/
>  UINT8 *
> -GetConfigurationPcd (
> +GetSupportPcd (
>VOID
>)
>  {
>UINT8  *SupportBitMask;
> 
>SupportBitMask = AllocateCopyPool (
> -  PcdGetSize (PcdCpuFeaturesUserConfiguration),
> -  PcdGetPtr (PcdCpuFeaturesUserConfiguration)
> +  PcdGetSize (PcdCpuFeaturesSupport),
> +  PcdGetPtr (PcdCpuFeaturesSupport)
>);
>ASSERT (SupportBitMask != NULL);
> 
> @@ -287,7 +287,6 @@ CpuInitDataInitialize (
>// Get support and configuration PCDs
>//
>CpuFeaturesData->SupportPcd   = GetSupportPcd ();
> -  CpuFeaturesData->ConfigurationPcd = GetConfigurationPcd ();  }
> 
>  /**
> @@ -595,6 +594,9 @@ AnalysisProcessorFeatures (
>CPU_FEATURE_DEPENDENCE_TYPE  AfterDep;
>CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibBeforeDep;
>CPU_FEATURE_DEPENDENCE_TYPE  NoneNeibAfterDep;
> +  UINT8*ConfigurationPcd;

1. Can we use the name "SettingPcd"? To use the same term.

> +
> +  ConfigurationPcd = NULL;
> 
>CpuFeaturesData = GetCpuFeaturesData ();
>CpuFeaturesData->CapabilityPcd = AllocatePool (CpuFeaturesData-
> >BitMaskSize); @@ -610,10 +612,13 @@ AnalysisProcessorFeatures (
>//
>// Calculate the last setting
>//
> -
>CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> >BitMaskSize, CpuFeaturesData->CapabilityPcd);
>ASSERT (CpuFeaturesData->SettingPcd != NULL);
> -  SupportedMaskAnd (CpuFeaturesData->SettingPcd, CpuFeaturesData-
> >ConfigurationPcd);
> +  ConfigurationPcd = GetSettingPcd ();

2. SuportedMaskAnd() function doesn't change the parameter 2.
So how about we pass PcdGetPtr (PcdCpuFeaturesSetting) directly to
SupportedMaskAnd()?

> +  SupportedMaskAnd (CpuFeaturesData->SettingPcd, ConfigurationPcd);  if
> + (ConfigurationPcd != NULL) {
> +FreePool (ConfigurationPcd);
> +  }
> 
>//
>// Save PCDs and display CPU PCDs
> @@ -643,8 +648,6 @@ AnalysisProcessorFeatures (
>  }
>  DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n"));
>  DumpCpuFeatureMask (CpuFeaturesData->SupportPcd);
> -DEBUG ((DEBUG_INFO, "PcdCpuFeaturesUserConfiguration:\n"));

[edk2] [PATCH] BaseTools:Function application error

2019-02-14 Thread Fan, ZhijuX
Error due to incorrect function parameters and attributes
FileWrite() The first argument it needs is a list, not a file
This patch abandons this function and saves the file independently

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/Eot/EotMain.py| 5 +++--
 BaseTools/Source/Python/Workspace/BuildClassObject.py | 1 +
 BaseTools/Source/Python/build/BuildReport.py  | 9 ++---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/Eot/EotMain.py 
b/BaseTools/Source/Python/Eot/EotMain.py
index fd4bee6f90..8c2bfc45e4 100644
--- a/BaseTools/Source/Python/Eot/EotMain.py
+++ b/BaseTools/Source/Python/Eot/EotMain.py
@@ -21,7 +21,7 @@ import Eot.EotGlobalData as EotGlobalData
 from optparse import OptionParser
 from Common.StringUtils import NormPath
 from Common import BuildToolError
-from Common.Misc import GuidStructureStringToGuidString, sdict
+from Common.Misc import GuidStructureStringToGuidString
 from Eot.Parser import *
 from Eot.InfParserLite import EdkInfParser
 from Common.StringUtils import GetSplitValueList
@@ -32,6 +32,7 @@ from Eot.Report import Report
 from Common.BuildVersion import gBUILD_VERSION
 from Eot.Parser import ConvertGuid
 from Common.LongFilePathSupport import OpenLongFilePath as open
+import collections
 import struct
 import uuid
 import copy
@@ -57,7 +58,7 @@ class Image(array):
 self._LEN_ = None
 self._OFF_ = None
 
-self._SubImages = sdict() # {offset: Image()}
+self._SubImages = collections.OrderedDict() # {offset: Image()}
 
 array.__init__(self)
 
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index cff77a71ae..6f8a09e87c 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -261,6 +261,7 @@ class StructurePcd(PcdClassObject):
 self.PackageDecs = Packages
 self.DefaultStoreName = [default_store]
 self.DefaultValues = OrderedDict()
+self.DefaultFromDSC = None
 self.PcdMode = None
 self.SkuOverrideValues = OrderedDict()
 self.StructName = None
diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index 0b98d62cb6..70584570a5 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1651,14 +1651,17 @@ class PredictionReport(object):
 SourceList = os.path.join(self._EotDir, "SourceFile.txt")
 GuidList = os.path.join(self._EotDir, "GuidList.txt")
 DispatchList = os.path.join(self._EotDir, "Dispatch.txt")
-
+TempList = []
 TempFile = open(SourceList, "w+")
 for Item in self._SourceList:
-FileWrite(TempFile, Item)
+TempList.append(Item + TAB_LINE_BREAK)
+TempFile.writelines(TempList)
 TempFile.close()
+TempList = []
 TempFile = open(GuidList, "w+")
 for Key in self._GuidMap:
-FileWrite(TempFile, "%s %s" % (Key, self._GuidMap[Key]))
+TempList.append("%s %s %s" % (Key, self._GuidMap[Key], 
TAB_LINE_BREAK))
+TempFile.writelines(TempList)
 TempFile.close()
 
 try:
-- 
2.14.1.windows.1

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


Re: [edk2] [PATCH edk2-platforms v1 01/16] Hisilicon/D0x: Remove SerdesLib

2019-02-14 Thread Ming Huang



On 2/13/2019 5:42 PM, Leif Lindholm wrote:
> On Wed, Feb 13, 2019 at 02:36:11PM +0800, Ming Huang wrote:
>>> Should it not then also delete #include  from
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c,
>>> Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c and
>>> Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/Type09/MiscSystemSlotDesignationFunction.c
>>> ?
>>>
>>> Meanwhile,
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/BoardFeature2PHi1610.c
>>> and
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/BoardFeatureD05.c
>>> both include this header, but
>>> Platform/Hisilicon/D03/Library/OemMiscLib2P/OemMiscLib2PHi1610.inf
>>> and 
>>> Platform/Hisilicon/D05/Library/OemMiscLibD05/OemMiscLibD05.inf
>>> do not declare the dependency.
>>
>> OemMiscLibD06.c can remove the SerdesLib.h. As using the definitions in
>> SerdesLib.h, other .c files can not remove the header file.
> 
> If they are using definitions from the library header, but not the
> library itself, there is something suspicious about the code
> structuring.
> 
> But in the meantime, if they are referencing library header files,
> they need to list those libraryclasses in their .inf.
> 
>>> Can you investigate and submit an updated patch addressing all of the
>>> unnecessary references?
>>
>> This may takes a lot of time, as Hi1620(D06) is our important project,
>> maybe we should focus on D06.
> 
> Feel free to submit deletions for all and any platforms you are
> unwilling to maintain.

Maybe there are no enough time to investigate this. Is ok to do this for 19.06?
When should I send the v2 out?

Thanks

> 
> Best Regards,
> 
> Leif
> 
> 
>> Thanks
>>
>>>
>>> Best Regards,
>>>
>>> Leif
>>>
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Ming Huang 
 ---
  Platform/Hisilicon/D06/D06.dsc   | 2 --
  Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 1 -
  2 files changed, 3 deletions(-)

 diff --git a/Platform/Hisilicon/D06/D06.dsc 
 b/Platform/Hisilicon/D06/D06.dsc
 index 396bd03c9d24..cbbd99e4a659 100644
 --- a/Platform/Hisilicon/D06/D06.dsc
 +++ b/Platform/Hisilicon/D06/D06.dsc
 @@ -64,8 +64,6 @@ [LibraryClasses.common]
  
CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
  
 -  
 SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
 -
TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf

 RealTimeClockLib|Silicon/Hisilicon/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf

 OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
 diff --git 
 a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf 
 b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 index 61cead7779b9..8e5c56fa41fd 100644
 --- a/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 +++ b/Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
 @@ -77,7 +77,6 @@ [LibraryClasses]
  
IpmiCmdLib
  
 -  SerdesLib
  
  [Protocols]
gEfiSmbiosProtocolGuid   # PROTOCOL ALWAYS_CONSUMED
 -- 
 2.9.5

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


[edk2] [PATCH v3 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chasel, Chiu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on
PI spec 1.7, Section 6.3.9.
This PPI can support the secnario that PEI Foundation
not in BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdePkg/Include/Ppi/PeiCoreFvLocation.h | 48 

 MdePkg/MdePkg.dec  | 11 +--
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h 
b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 00..3bea6819ec
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,48 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI 
Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass this 
PPI as a part
+  of the PPI list provided to the PEI Foundation Entry Point, otherwise the 
PEI Foundation
+  shall assume that it resides within the BFV.
+
+  Copyright (c) 2019, Intel Corporation. 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.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 
1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI
+///
+#define EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 
0xf4 } \
+  }
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+typedef struct {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI 
Foundation.
+  ///
+  VOID*PeiCoreFvLocation;
+} EFI_PEI_CORE_FV_LOCATION_PPI;
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index a485408310..c859b4a511 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
 # This Package provides all definitions, library classes and libraries 
instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
-# EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
 # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 #
@@ -929,6 +929,13 @@
   ## Include/Ppi/SecHobData.h
   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
 
+  #
+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 
0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
 [Protocols]
   ## Include/Protocol/Pcd.h
   gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 
0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
-- 
2.13.3.windows.1

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


[edk2] [PATCH v3 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chasel, Chiu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform
when PeiCore not in BFV so SecCore has to search PeiCore
either from the FV location provided by
EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 UefiCpuPkg/SecCore/SecMain.c   | 35 +--
 UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index b24e190617..d84eb675f5 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -1,7 +1,7 @@
 /** @file
   C functions in SEC
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, Intel Corporation. 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
@@ -232,22 +232,45 @@ SecStartupPhase2(
   EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
   EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
 
+  PeiCoreEntryPoint = NULL;
   SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
   AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData->PeiTemporaryRamBase;
+
   //
   // Find Pei Core entry point. It will report SEC and Pei Core debug 
information if remote debug
   // is enabled.
   //
-  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
-  if (PeiCoreEntryPoint == NULL)
-  {
-CpuDeadLoop ();
+  PpiList = SecPlatformMain (SecCoreData);
+  if (PpiList != NULL) {
+for (Index = 0;
+  (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
+  Index++) {
+  if (CompareGuid (PpiList[Index].Guid, ) && 
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation != 
0)) {
+FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation, 
);
+if (PeiCoreEntryPoint != NULL) {
+  break;
+} else {
+  //
+  // PeiCore not found
+  //
+  CpuDeadLoop ();
+}
+  }
+}
+  }
+  //
+  // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from BFV.
+  //
+  if (PeiCoreEntryPoint == NULL) {
+FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
+if (PeiCoreEntryPoint == NULL) {
+  CpuDeadLoop ();
+}
   }
 
   //
   // Perform platform specific initialization before entering PeiCore.
   //
-  PpiList = SecPlatformMain (SecCoreData);
   if (PpiList != NULL) {
 //
 // Remove the terminal flag from the terminal PPI
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 442f663911..fc1485b5cb 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -7,7 +7,7 @@
 #  protected mode, setup flat memory model, enable temporary memory and
 #  call into SecStartup().
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. 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
@@ -73,6 +73,7 @@
   ## NOTIFY
   ## SOMETIMES_CONSUMES
   gPeiSecPerformancePpiGuid
+  gEfiPeiCoreFvLocationPpiGuid
 
 [Guids]
   ## SOMETIMES_PRODUCES   ## HOB
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index 83244af119..80045d34f4 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -1,7 +1,7 @@
 /** @file
   Master header file for SecCore.
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, Intel Corporation. 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
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
-- 
2.13.3.windows.1

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


[edk2] [PATCH v3 0/3] Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chasel, Chiu
PI spec 1.7 section 6.3.9 has defined a PPI to support the scenario
that PEI Foundation not in BFV. EFI_PEI_CORE_FV_LOCATION_PPI reports
the FV which contains PEI Foundation and should be passed by SEC as
part of PPI list. Otherwise PEI Foundation shall assume that it
resides in BFV.

Patch1: Add EFI_PEI_CORE_FV_LOCATION_PPI definition.
Patch2: Support PeiCore not in BFV scenario when shadowing.
Patch3: SecCore to find PeiCore from either non-BFV or BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 

Chasel, Chiu (3):
  MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI
  MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI
  UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 53 
-
 UefiCpuPkg/SecCore/SecMain.c| 35 
+--
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 MdePkg/Include/Ppi/PeiCoreFvLocation.h  | 48 

 MdePkg/MdePkg.dec   | 11 +--
 UefiCpuPkg/SecCore/SecCore.inf  |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h|  3 ++-
 8 files changed, 134 insertions(+), 25 deletions(-)
 create mode 100644 MdePkg/Include/Ppi/PeiCoreFvLocation.h

-- 
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 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
Chasel,
I don't think the below forward definition is needed.
 typedef struct _EFI_PEI_CORE_FV_LOCATION_PPI EFI_PEI_CORE_FV_LOCATION_PPI;
It is needed when the PPI contains methods and the methods requires This pointer
as the first parameter.


> -Original Message-
> From: edk2-devel  On Behalf Of Chasel,
> Chiu
> Sent: Tuesday, February 12, 2019 9:20 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> 
> Subject: [edk2] [PATCH 1/3] MdePkg: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7,
> Section 6.3.9.
> This PPI can support the secnario that PEI Foundation not in BFV.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdePkg/Include/Ppi/PeiCoreFvLocation.h | 53
> +
>  MdePkg/MdePkg.dec  | 11 +--
>  2 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> new file mode 100644
> index 00..c7bbbfb265
> --- /dev/null
> +++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> @@ -0,0 +1,53 @@
> +/** @file
> +  Header file for Pei Core FV Location PPI.
> +
> +  This PPI contains a pointer to the firmware volume which contains the PEI
> Foundation.
> +  If the PEI Foundation does not reside in the BFV, then SEC must pass
> + this PPI as a part  of the PPI list provided to the PEI Foundation
> + Entry Point, otherwise the PEI Foundation  shall assume that it resides
> within the BFV.
> +
> +  Copyright (c) 2019, Intel Corporation. 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.
> +
> +  @par Revision Reference:
> +  This PPI is defined in UEFI Platform Initialization Specification 1.7 
> Volume 1:
> +  Standards
> +
> +**/
> +
> +
> +#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
> +#define _EFI_PEI_CORE_FV_LOCATION_H_
> +
> +///
> +/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define
> +EFI_PEI_CORE_FV_LOCATION_GUID \
> +  { \
> +0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0,
> +0xca, 0xf4 } \
> +  }
> +
> +///
> +/// Forward declaration for EFI_PEI_CORE_FV_LOCATION_PPI /// typedef
> +struct _EFI_PEI_CORE_FV_LOCATION_PPI
> EFI_PEI_CORE_FV_LOCATION_PPI;
> +
> +///
> +/// This PPI provides location of EFI PeiCoreFv.
> +///
> +struct _EFI_PEI_CORE_FV_LOCATION_PPI {
> +  ///
> +  /// Pointer to the first byte of the firmware volume which contains the PEI
> Foundation.
> +  ///
> +  VOID*PeiCoreFvLocation;
> +};
> +
> +extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
> +
> +#endif // _EFI_PEI_CORE_FV_LOCATION_H_
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> a485408310..c859b4a511 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2,9 +2,9 @@
>  # This Package provides all definitions, library classes and libraries 
> instances.
>  #
>  # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -#
> EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
> +# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
>  #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights
> +reserved.
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.  # 
> (C)
> Copyright 2016 Hewlett Packard Enterprise Development LP  # @@ -
> 929,6 +929,13 @@
>## Include/Ppi/SecHobData.h
>gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5,
> 0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
> 
> +  #
> +  # PPIs defined in PI 1.7.
> +  #
> +
> +  ## Include/Ppi/PeiCoreFvLocation.h
> +  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8,
> 0x7f, 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
> +
>  [Protocols]
>## Include/Protocol/Pcd.h
>gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 
> 0x90,
> 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
> --
> 2.13.3.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


[edk2] [PATCH v3 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Chasel, Chiu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524

When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI
should be checked to see if PeiCore not in BFV, otherwise
just shadowing PeiCore from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 53 
-
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c 
b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 4da80a8222..a7da90e149 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Main Entry Point
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. 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
@@ -80,22 +80,49 @@ ShadowPeiCore (
   IN PEI_CORE_INSTANCE  *PrivateData
   )
 {
-  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
-  EFI_PHYSICAL_ADDRESS EntryPoint;
-  EFI_STATUS   Status;
-  UINT32   AuthenticationState;
+  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
+  EFI_PHYSICAL_ADDRESS EntryPoint;
+  EFI_STATUS   Status;
+  UINT32   AuthenticationState;
+  UINTNIndex;
+  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
+  UINTNPeiCoreFvIndex;
 
   PeiCoreFileHandle = NULL;
-
   //
-  // Find the PEI Core in the BFV
+  // Default PeiCore is in BFV
+  //
+  PeiCoreFvIndex = 0;
+  //
+  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI indicated FV 
or BFV
+  //
+  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **) 
+ );
+  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != 
NULL)) {
+//
+// If PeiCoreFvLocation present, the PEI Core should be found from 
indicated FV
+//
+for (Index = 0; Index < PrivateData->FvCount; Index ++) {
+  if (PrivateData->Fv[Index].FvHandle == 
PeiCoreFvLocationPpi->PeiCoreFvLocation) {
+PeiCoreFvIndex = Index;
+break;
+  }
+}
+ASSERT (Index < PrivateData->FvCount);
+  }
+  //
+  // Find PEI Core from the given FV index
   //
-  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
-   PrivateData->Fv[0].FvPpi,
-   EFI_FV_FILETYPE_PEI_CORE,
-   PrivateData->Fv[0].FvHandle,
-   
-   );
+  Status = PrivateData->Fv[PeiCoreFvIndex].FvPpi->FindFileByType (
+
PrivateData->Fv[PeiCoreFvIndex].FvPpi,
+EFI_FV_FILETYPE_PEI_CORE,
+
PrivateData->Fv[PeiCoreFvIndex].FvHandle,
+
+);
   ASSERT_EFI_ERROR (Status);
 
   //
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index 322e7cd845..a61da73fd8 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1,7 +1,7 @@
 /** @file
   Definition of Pei Core Structures and Services
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. 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
@@ -31,6 +31,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf 
b/MdeModulePkg/Core/Pei/PeiMain.inf
index 140cb1..5bab2aab8c 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -6,7 +6,7 @@
 # 2) Dispatch PEIM from discovered FV.
 # 3) Handoff control to DxeIpl to load DXE core and enter DXE phase.
 #
-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -102,6 +102,7 

Re: [edk2] [PATCH v3 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
Reviewed-by: Ray Ni 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Chasel, Chiu
> Sent: Thursday, February 14, 2019 6:59 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> 
> Subject: [edk2] [PATCH v3 1/3] MdePkg: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7, Section
> 6.3.9.
> This PPI can support the secnario that PEI Foundation not in BFV.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdePkg/Include/Ppi/PeiCoreFvLocation.h | 48
> 
>  MdePkg/MdePkg.dec  | 11 +--
>  2 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> new file mode 100644
> index 00..3bea6819ec
> --- /dev/null
> +++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
> @@ -0,0 +1,48 @@
> +/** @file
> +  Header file for Pei Core FV Location PPI.
> +
> +  This PPI contains a pointer to the firmware volume which contains the PEI
> Foundation.
> +  If the PEI Foundation does not reside in the BFV, then SEC must pass
> + this PPI as a part  of the PPI list provided to the PEI Foundation
> + Entry Point, otherwise the PEI Foundation  shall assume that it resides 
> within
> the BFV.
> +
> +  Copyright (c) 2019, Intel Corporation. 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.
> +
> +  @par Revision Reference:
> +  This PPI is defined in UEFI Platform Initialization Specification 1.7 
> Volume 1:
> +  Standards
> +
> +**/
> +
> +
> +#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
> +#define _EFI_PEI_CORE_FV_LOCATION_H_
> +
> +///
> +/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define
> +EFI_PEI_CORE_FV_LOCATION_GUID \
> +  { \
> +0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0,
> +0xca, 0xf4 } \
> +  }
> +
> +///
> +/// This PPI provides location of EFI PeiCoreFv.
> +///
> +typedef struct {
> +  ///
> +  /// Pointer to the first byte of the firmware volume which contains the PEI
> Foundation.
> +  ///
> +  VOID*PeiCoreFvLocation;
> +} EFI_PEI_CORE_FV_LOCATION_PPI;
> +
> +extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
> +
> +#endif // _EFI_PEI_CORE_FV_LOCATION_H_
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> a485408310..c859b4a511 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2,9 +2,9 @@
>  # This Package provides all definitions, library classes and libraries 
> instances.
>  #
>  # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -#
> EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
> +# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
>  #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2019, Intel Corporation. All rights
> +reserved.
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.  # 
> (C)
> Copyright 2016 Hewlett Packard Enterprise Development LP  # @@ -929,6
> +929,13 @@
>## Include/Ppi/SecHobData.h
>gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5,
> 0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
> 
> +  #
> +  # PPIs defined in PI 1.7.
> +  #
> +
> +  ## Include/Ppi/PeiCoreFvLocation.h
> +  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 
> 0x7f,
> 0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
> +
>  [Protocols]
>## Include/Protocol/Pcd.h
>gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 
> 0x90,
> 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
> --
> 2.13.3.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 1/3] BaseTools: Fixed a build report issue.

2019-02-14 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Feng, Bob C
> Sent: Monday, February 4, 2019 2:48 PM
> To: edk2-devel@lists.01.org
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [Patch 1/3] BaseTools: Fixed a build report issue.
> 
> Generate report fail when -Y EXECUTION_ORDER in build command.
> This patch is going to fix this issue.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/Eot/EotMain.py   | 3 ++-
>  BaseTools/Source/Python/build/BuildReport.py | 8 
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Eot/EotMain.py 
> b/BaseTools/Source/Python/Eot/EotMain.py
> index fd4bee6f90..57f96c89c9 100644
> --- a/BaseTools/Source/Python/Eot/EotMain.py
> +++ b/BaseTools/Source/Python/Eot/EotMain.py
> @@ -19,11 +19,12 @@ import Common.LongFilePathOs as os, time, glob  import 
> Common.EdkLogger as EdkLogger  import
> Eot.EotGlobalData as EotGlobalData  from optparse import OptionParser  from 
> Common.StringUtils import NormPath  from Common
> import BuildToolError -from Common.Misc import 
> GuidStructureStringToGuidString, sdict
> +from Common.Misc import GuidStructureStringToGuidString from
> +collections import OrderedDict as sdict
>  from Eot.Parser import *
>  from Eot.InfParserLite import EdkInfParser  from Common.StringUtils import 
> GetSplitValueList  from Eot import c  from Eot import
> Database diff --git a/BaseTools/Source/Python/build/BuildReport.py 
> b/BaseTools/Source/Python/build/BuildReport.py
> index e457660fce..52764a6c55 100644
> --- a/BaseTools/Source/Python/build/BuildReport.py
> +++ b/BaseTools/Source/Python/build/BuildReport.py
> @@ -1650,18 +1650,18 @@ class PredictionReport(object):
>  #
>  SourceList = os.path.join(self._EotDir, "SourceFile.txt")
>  GuidList = os.path.join(self._EotDir, "GuidList.txt")
>  DispatchList = os.path.join(self._EotDir, "Dispatch.txt")
> 
> -TempFile = open(SourceList, "w+")
> +TempFile = []
>  for Item in self._SourceList:
>  FileWrite(TempFile, Item)
> -TempFile.close()
> -TempFile = open(GuidList, "w+")
> +SaveFileOnChange(SourceList, "".join(TempFile), False)
> +TempFile = []
>  for Key in self._GuidMap:
>  FileWrite(TempFile, "%s %s" % (Key, self._GuidMap[Key]))
> -TempFile.close()
> +SaveFileOnChange(GuidList, "".join(TempFile), False)
> 
>  try:
>  from Eot.EotMain import Eot
> 
>  #
> --
> 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] BaseTools: Fix the build report issue about Structure PCD

2019-02-14 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Feng, Bob C
> Sent: Friday, February 1, 2019 5:01 PM
> To: edk2-devel@lists.01.org
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [Patch] BaseTools: Fix the build report issue about Structure PCD
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1472
> build report use incorrect method to parse DynamicDefault/DynamicExDefault
> and DynamicVpd/DynamicExVpd structure Pcd value.
> 
> This patch is to fix this issue.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/build/BuildReport.py | 21 +++-
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/build/BuildReport.py 
> b/BaseTools/Source/Python/build/BuildReport.py
> index 8d3b030151..64f8a68516 100644
> --- a/BaseTools/Source/Python/build/BuildReport.py
> +++ b/BaseTools/Source/Python/build/BuildReport.py
> @@ -1109,24 +1109,17 @@ class PcdReport(object):
>  break
>  else:
>  SkuList = sorted(Pcd.SkuInfoList.keys())
>  for Sku in SkuList:
>  SkuInfo = Pcd.SkuInfoList[Sku]
> -if TypeName in ('DYNHII', 'DEXHII'):
> -if SkuInfo.DefaultStoreDict:
> -DefaultStoreList = 
> sorted(SkuInfo.DefaultStoreDict.keys())
> -for DefaultStore in 
> DefaultStoreList:
> -OverrideValues = 
> Pcd.SkuOverrideValues[Sku]
> -DscOverride = 
> self.ParseStruct(OverrideValues[DefaultStore])
> -if DscOverride:
> -break
> -else:
> -OverrideValues = 
> Pcd.SkuOverrideValues[Sku]
> -if OverrideValues:
> -Keys = 
> list(OverrideValues.keys())
> -OverrideFieldStruct = 
> self.OverrideFieldValue(Pcd, OverrideValues[Keys[0]])
> -DscOverride = 
> self.ParseStruct(OverrideFieldStruct)
> +if SkuInfo.DefaultStoreDict:
> +DefaultStoreList = 
> sorted(SkuInfo.DefaultStoreDict.keys())
> +for DefaultStore in 
> DefaultStoreList:
> +OverrideValues = 
> Pcd.SkuOverrideValues[Sku]
> +DscOverride = 
> self.ParseStruct(OverrideValues[DefaultStore])
> +if DscOverride:
> +break
>  if DscOverride:
>  break
>  if DscOverride:
>  DscDefaultValue = True
>  DscMatch = True
> --
> 2.20.1.windows.1

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


Re: [edk2] [PATCH v3 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
Reviewed-by: Ray Ni 

> -Original Message-
> From: Chiu, Chasel
> Sent: Thursday, February 14, 2019 6:59 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Zeng, Star ; Gao, Liming
> ; Chiu, Chasel 
> Subject: [PATCH v3 2/3] MdeModulePkg/PeiMain: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be
> checked to see if PeiCore not in BFV, otherwise just shadowing PeiCore from
> BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Jian J Wang 
> Cc: Hao Wu 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 53
> -
>  MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
>  MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
>  3 files changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 4da80a8222..a7da90e149 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>Pei Core Main Entry Point
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. 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 @@ -80,22 +80,49 
> @@
> ShadowPeiCore (
>IN PEI_CORE_INSTANCE  *PrivateData
>)
>  {
> -  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> -  EFI_PHYSICAL_ADDRESS EntryPoint;
> -  EFI_STATUS   Status;
> -  UINT32   AuthenticationState;
> +  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
> +  EFI_PHYSICAL_ADDRESS EntryPoint;
> +  EFI_STATUS   Status;
> +  UINT32   AuthenticationState;
> +  UINTNIndex;
> +  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> +  UINTNPeiCoreFvIndex;
> 
>PeiCoreFileHandle = NULL;
> -
>//
> -  // Find the PEI Core in the BFV
> +  // Default PeiCore is in BFV
> +  //
> +  PeiCoreFvIndex = 0;
> +  //
> +  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI
> + indicated FV or BFV  //  Status = PeiServicesLocatePpi (
> + ,
> + 0,
> + NULL,
> + (VOID **) 
> + );
> +  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation !=
> NULL)) {
> +//
> +// If PeiCoreFvLocation present, the PEI Core should be found from 
> indicated
> FV
> +//
> +for (Index = 0; Index < PrivateData->FvCount; Index ++) {
> +  if (PrivateData->Fv[Index].FvHandle == PeiCoreFvLocationPpi-
> >PeiCoreFvLocation) {
> +PeiCoreFvIndex = Index;
> +break;
> +  }
> +}
> +ASSERT (Index < PrivateData->FvCount);  }  //  // Find PEI Core
> + from the given FV index
>//
> -  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
> -   PrivateData->Fv[0].FvPpi,
> -   EFI_FV_FILETYPE_PEI_CORE,
> -   PrivateData->Fv[0].FvHandle,
> -   
> -   );
> +  Status = PrivateData->Fv[PeiCoreFvIndex].FvPpi->FindFileByType (
> +
> PrivateData->Fv[PeiCoreFvIndex].FvPpi,
> +EFI_FV_FILETYPE_PEI_CORE,
> +
> PrivateData->Fv[PeiCoreFvIndex].FvHandle,
> +
> +);
>ASSERT_EFI_ERROR (Status);
> 
>//
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h index 322e7cd845..a61da73fd8 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>Definition of Pei Core Structures and Services
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2019, Intel Corporation. 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 @@ -31,6 +31,7 @@
> WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf
> 

Re: [edk2] [Patch] BaseTools: Correct the error message for UPT

2019-02-14 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: Feng, Bob C
> Sent: Saturday, February 2, 2019 9:36 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Hess ; Gao, Liming 
> Subject: [Patch] BaseTools: Correct the error message for UPT
> 
> This patch is going to correct the error message
> for UPT.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hess Chen 
> Cc: Liming Gao 
> ---
>  .../Python/UPT/Library/UniClassObject.py   | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/UPT/Library/UniClassObject.py 
> b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
> index cd575d5a34..7a056b349f 100644
> --- a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
> +++ b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # Collect all defined strings in multiple uni files.
>  #
> -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2014 - 2019, Intel Corporation. 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
> @@ -593,10 +593,19 @@ class UniFileClassObject(object):
>  StringEntryExistsFlag = 1
>  if not Line.endswith('"'):
>  EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID,
>  ExtraData='''The line %s misses '"' at 
> the end of it in file %s'''
>% (LineCount, File.Path))
> +
> +#
> +# Check the situation that there has more than 2 '"' for the 
> language entry
> +#
> +if Line.strip() and Line.replace(u'\\"', '').count(u'"') > 2:
> +EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID,
> +ExtraData='''The line %s has more than 2 
> '"' for language entry in file %s'''
> +% (LineCount, File.Path))
> +
>  elif Line.startswith(u'#language'):
>  if StringEntryExistsFlag == 2:
>  EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID,
>  Message=ST.ERR_UNI_MISS_STRING_ENTRY % 
> Line, ExtraData=File.Path)
>  StringEntryExistsFlag = 0
> @@ -741,17 +750,10 @@ class UniFileClassObject(object):
>  NewLines.append((Line[:Line.find(u'"')]).strip())
>  NewLines.append((Line[Line.find(u'"'):]).strip())
>  else:
>  EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID, ExtraData=File.Path)
>  elif Line.startswith(u'"'):
> -#
> -# Check the situation that there has more than 2 '"' for the 
> language entry
> -#
> -if Line.replace(u'\\"', '').count(u'"') > 2:
> -EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID,
> -ExtraData='''The line %s has more than 2 
> '"' for language entry in file %s'''
> -% (LineCount, File.Path))
>  if u'#string' in Line  or u'#language' in Line:
>  EdkLogger.Error("Unicode File Parser", 
> ToolError.FORMAT_INVALID, ExtraData=File.Path)
>  NewLines.append(Line)
>  else:
>  print(Line)
> --
> 2.19.1.windows.1

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


Re: [edk2] [PATCH v3 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Ni, Ray
Reviewed-by: Ray Ni 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Chasel, Chiu
> Sent: Thursday, February 14, 2019 6:59 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Dong, Eric 
> Subject: [edk2] [PATCH v3 3/3] UefiCpuPkg/SecCore: Support
> EFI_PEI_CORE_FV_LOCATION_PPI
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1524
> 
> EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform
> when PeiCore not in BFV so SecCore has to search PeiCore
> either from the FV location provided by
> EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.
> 
> Test: Verified on internal platform and booting successfully.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  UefiCpuPkg/SecCore/SecMain.c   | 35 +--
>  UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
>  UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
>  3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
> index b24e190617..d84eb675f5 100644
> --- a/UefiCpuPkg/SecCore/SecMain.c
> +++ b/UefiCpuPkg/SecCore/SecMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>C functions in SEC
> 
> -  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) 2008 - 2019, Intel Corporation. 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
> @@ -232,22 +232,45 @@ SecStartupPhase2(
>EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
>EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
> 
> +  PeiCoreEntryPoint = NULL;
>SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
>AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData-
> >PeiTemporaryRamBase;
> +
>//
>// Find Pei Core entry point. It will report SEC and Pei Core debug 
> information
> if remote debug
>// is enabled.
>//
> -  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> SecCoreData->BootFirmwareVolumeBase, );
> -  if (PeiCoreEntryPoint == NULL)
> -  {
> -CpuDeadLoop ();
> +  PpiList = SecPlatformMain (SecCoreData);
> +  if (PpiList != NULL) {
> +for (Index = 0;
> +  (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> +  Index++) {
> +  if (CompareGuid (PpiList[Index].Guid, ) &&
> (((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)-
> >PeiCoreFvLocation != 0)) {
> +FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> ((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation,
> );
> +if (PeiCoreEntryPoint != NULL) {
> +  break;
> +} else {
> +  //
> +  // PeiCore not found
> +  //
> +  CpuDeadLoop ();
> +}
> +  }
> +}
> +  }
> +  //
> +  // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from
> BFV.
> +  //
> +  if (PeiCoreEntryPoint == NULL) {
> +FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *)
> SecCoreData->BootFirmwareVolumeBase, );
> +if (PeiCoreEntryPoint == NULL) {
> +  CpuDeadLoop ();
> +}
>}
> 
>//
>// Perform platform specific initialization before entering PeiCore.
>//
> -  PpiList = SecPlatformMain (SecCoreData);
>if (PpiList != NULL) {
>  //
>  // Remove the terminal flag from the terminal PPI
> diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
> index 442f663911..fc1485b5cb 100644
> --- a/UefiCpuPkg/SecCore/SecCore.inf
> +++ b/UefiCpuPkg/SecCore/SecCore.inf
> @@ -7,7 +7,7 @@
>  #  protected mode, setup flat memory model, enable temporary memory and
>  #  call into SecStartup().
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2006 - 2019, Intel Corporation. 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
> @@ -73,6 +73,7 @@
>## NOTIFY
>## SOMETIMES_CONSUMES
>gPeiSecPerformancePpiGuid
> +  gEfiPeiCoreFvLocationPpiGuid
> 
>  [Guids]
>## SOMETIMES_PRODUCES   ## HOB
> diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
> index 83244af119..80045d34f4 100644
> --- a/UefiCpuPkg/SecCore/SecMain.h
> +++ b/UefiCpuPkg/SecCore/SecMain.h
> @@ -1,7 +1,7 @@
>  /** @file
>Master header file for SecCore.
> 
> -  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the 

Re: [edk2] [Patch 2/3] BaseTools: Fixed an issue about StructurePcd

2019-02-14 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng, 
> Bob C
> Sent: Monday, February 4, 2019 2:48 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch 2/3] BaseTools: Fixed an issue about StructurePcd
> 
> If use a structure pcd in fdf, build tool crash
> This is a regression issue introduced by py3 patch set.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/Workspace/BuildClassObject.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py 
> b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> index cff77a71ae..41759b8785 100644
> --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> @@ -268,10 +268,11 @@ class StructurePcd(PcdClassObject):
>  self.PkgPath = ""
>  self.DefaultValueFromDec = ""
>  self.ValueChain = set()
>  self.PcdFieldValueFromComm = OrderedDict()
>  self.PcdFieldValueFromFdf = OrderedDict()
> +self.DefaultFromDSC=None
>  def __repr__(self):
>  return self.TypeName
> 
>  def AddDefaultValue (self, FieldName, Value, FileName="", 
> LineNo=0,DimensionAttr ="-1"):
>  if DimensionAttr not in self.DefaultValues:
> @@ -324,11 +325,10 @@ class StructurePcd(PcdClassObject):
>  if isinstance(PcdObject, StructurePcd):
>  self.StructuredPcdIncludeFile = 
> PcdObject.StructuredPcdIncludeFile if PcdObject.StructuredPcdIncludeFile else
> self.StructuredPcdIncludeFile
>  self.PackageDecs = PcdObject.PackageDecs if 
> PcdObject.PackageDecs else self.PackageDecs
>  self.DefaultValues = PcdObject.DefaultValues if 
> PcdObject.DefaultValues else self.DefaultValues
>  self.PcdMode = PcdObject.PcdMode if PcdObject.PcdMode else 
> self.PcdMode
> -self.DefaultFromDSC=None
>  self.DefaultValueFromDec = PcdObject.DefaultValueFromDec if 
> PcdObject.DefaultValueFromDec else
> self.DefaultValueFromDec
>  self.SkuOverrideValues = PcdObject.SkuOverrideValues if 
> PcdObject.SkuOverrideValues else self.SkuOverrideValues
>  self.StructName = PcdObject.DatumType if PcdObject.DatumType 
> else self.StructName
>  self.PcdDefineLineNo = PcdObject.PcdDefineLineNo if 
> PcdObject.PcdDefineLineNo else self.PcdDefineLineNo
>  self.PkgPath = PcdObject.PkgPath if PcdObject.PkgPath else 
> self.PkgPath
> --
> 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 3/3] BaseTools: Fix a bug about PcdArray

2019-02-14 Thread Gao, Liming
Bob:
  Could you give more message on this issue?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng, 
> Bob C
> Sent: Monday, February 4, 2019 2:48 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch 3/3] BaseTools: Fix a bug about PcdArray
> 
> This patch is going to fix the bug that
> there is an incorrect variable access method.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob 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 f472fa177f..a286232f7c 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -2394,11 +2394,11 @@ class DscBuildData(PlatformBuildClassObject):
>  skuinfo = Pcd.SkuInfoList[skuname]
>  if skuinfo.VariableName:
>  for defaultstore in skuinfo.DefaultStoreDict:
>  pcddscrawdefaultvalue = 
> self.GetPcdDscRawDefaultValue(Pcd, skuname, defaultstore)
>  if pcddscrawdefaultvalue:
> -Value = skuinfo[defaultstore]
> +Value = skuinfo.DefaultStoreDict[defaultstore]
>  if "{CODE(" in Value:
>  realvalue = Value.strip()[6:-2] # 
> "{CODE(").rstrip(")}"
>  CApp += "static %s %s_%s_%s_%s_Value%s = 
> %s;\n" %
> (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore,Demesion,realvalue)
>  else:
>  pcddscrawdefaultvalue = 
> self.GetPcdDscRawDefaultValue(Pcd, skuname, TAB_DEFAULT_STORES_DEFAULT)
> --
> 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] BaseTools: Implement splitquoted function

2019-02-14 Thread Laszlo Ersek
Hi Bob,

On 02/14/19 03:51, Feng, Bob C wrote:
> Hi Laszlo, Liming, Jaben and Dandan,
>
> I found this is a Ubuntu18 bug. Refer to
> https://bugs.launchpad.net/ubuntu/+source/fdroidserver/+bug/1762183

That Ubuntu bug is *related*, but it's not a bug that matters for us.

The bug above is about another package; it is called "fdroidserver". The
"fdroidserver" package has a hard runtime dependency on the python
"distutils.util" module.

Package management systems on Linux distributions track inter-package
dependencies. That is, if the meta-data on the "fdroidserver" package
explicitly lists the "python3-distutils" package as a dependency, then
the package management system will automatically install in
"python3-distutils" when the user requests "fdroidserver".

However, if the meta-data on the "fdroidserver" package are incorrect
(buggy), then the package management system will have no clue. And, if
the "python3-distutils" package is not already installed for some other
reason, then the user will get an installation or runtime error, when
they try to install or run "fdroidserver".

So, to be clear, the bug report you reference describes a *similar*
situation to ours (a missing dependency), but it's not the same case.
The bug you refer to is in the "fdroidserver" package, and they fixed it
in Ubuntu by updating / correcting the meta-data on the "fdroidserver"
package. The "python3-distutils" package, or other parts of the OS, were
not touched.

> And Ubuntu fixed this bug via a Ubuntu18.04.1 update package which was
> published on 2018-08-09. Refer to
> https://launchpad.net/ubuntu/+source/fdroidserver/1.0.9-1~18.04.1

The link you provide confirms what I wrote above. It is a changelog for
the "fdroidserver" package, and the relevant entry says "fix missing
Python distutils dependency".

> While the latest Ubuntu 18.04 release
> (ubuntu-18.04.1-desktop-amd64.iso)  on
> http://releases.ubuntu.com/18.04/ was published on 2018-07-25.  So
> there is no distutils.util library on Ubuntu18.04 default
> installation.

No, this statement can't be correct. I'm pretty sure that the
"python3-distutils" package *was* available when Ubuntu 18.04 -- not
18.04.1 -- was originally released.

I've just downloaded

  http://old-releases.ubuntu.com/releases/bionic/ubuntu-18.04-desktop-amd64.iso

While the package is not on the ISO, a whole lot of *other* packages are
also not there -- for example I can't see any python at all. So I'm
thinking that python3-distutils was only available from the network.

But, I'm pretty sure python3-distutils *was* available from the network,
when 18.04 was originally released.

What was indeed broken in the original Ubuntu 18.04 release was the
meta-data on the "fdroidserver" package.

> But I think it's clear that distutils.util is not *3rd party* python
> library.

I agree; and that is what matters.

> I have tried that the command "sudo apt upgrade"  can't fix this bug

This "apt" command would only be relevant if you had the *old*
fdroidserver package installed (with the missing dependency in its
meta-data). Then, "apt" would install the *new* fdroidserver package for
you, and it would also act on the now-visible "python3-distutils"
dependency. Thus, "apt" would automatically install in
python3-distutils, as a dependency.

For installing "python3-distutils" *in itself*, the above "apt" command
is totally useless.

> while the command "sudo apt-get install python3-distutils" works.

Yes, it does, because here you are specifically requesting the
python3-distutils package.

And that's what matters. Ubuntu users simply need to install
python3-distutils manually, from their official package repositories, if
they want to use BaseTools (as part of the upstream edk2 git repo).

So I think this edk2 patch set is not necessary, after all.

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


Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

2019-02-14 Thread Laszlo Ersek
On 02/14/19 04:23, Gao, Liming wrote:
> Bob:
>   So, this is OS issue. I prefer to update wiki page to describe how to 
> resolve it. 

Great idea. I think this is probably the affected section:

https://github.com/tianocore/tianocore.github.io/wiki/Using-EDK-II-with-Native-GCC#Install_required_software_from_apt

We should simply add "python3-distutils" to the list.

Possibly, also mention "python3-distutils" in
"BaseTools/Conf/tools_def.template".

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


Re: [edk2] [Patch] BaseTools: Correct the PcdDatabase Info in AutoGen

2019-02-14 Thread Gao, Liming
Zhiju:

1. Please make sure struct PEI_PCD_DATABASE_INIT in AutoGen.h to match 
gPEIPcdDbInit in AutoGen.c. 
2. Please make sure struct DXE_PCD_DATABASE_INIT in AutoGen.h to match 
gDXEPcdDbinit in AutoGen.c
3. gDXEPcdDbinit in AutoGen.c
  a. Keep origin style of /* LocalTokenNumberTable */ and /* VariableHead */
  b. Remove /* SkuHead */. It is not in PCD data bin. 
  c. Use the real string for the value in /* PcdTokenTable */ and /* 
PcdCNameTable */

Thanks
Liming
> -Original Message-
> From: Feng, Bob C
> Sent: Saturday, February 2, 2019 9:52 AM
> To: edk2-devel@lists.01.org
> Cc: Zhao, ZhiqiangX ; Feng, Bob C 
> ; Gao, Liming 
> Subject: [Patch] BaseTools: Correct the PcdDatabase Info in AutoGen
> 
> From: Zhaozh1x 
> 
> PcdDriver AutoGen code shows PCD DB format for debug purpose only.
> But now, AutoGen code doesn't exactly match the generated
> PCD DB binary file. It brings the complex for Pcd driver debug.
> 
> This patch is going to fix that issue.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
>  BaseTools/Source/Python/AutoGen/GenPcdDb.py | 41 +
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenPcdDb.py 
> b/BaseTools/Source/Python/AutoGen/GenPcdDb.py
> index cbf7a39dd5..2ca9001417 100644
> --- a/BaseTools/Source/Python/AutoGen/GenPcdDb.py
> +++ b/BaseTools/Source/Python/AutoGen/GenPcdDb.py
> @@ -52,23 +52,32 @@ ${END}
>  ${BEGIN}{ ${EXMAPPING_TABLE_EXTOKEN}, ${EXMAPPING_TABLE_LOCAL_TOKEN}, 
> ${EXMAPPING_TABLE_GUID_INDEX} },
>  ${END}
>},
>/* LocalTokenNumberTable */
>{
> -${BEGIN}offsetof(${PHASE}_PCD_DATABASE, 
> ${TOKEN_INIT}.${TOKEN_CNAME}_${TOKEN_GUID}${VARDEF_HEADER}) |
> ${TOKEN_TYPE},
> +${BEGIN}${LOCAL_TOKEN_NUMBER_DB_VALUE}, /* 
> offsetof(${PHASE}_PCD_DATABASE,
> ${TOKEN_INIT}.${TOKEN_CNAME}_${TOKEN_GUID}${VARDEF_HEADER}) | ${TOKEN_TYPE} */
>  ${END}
>},
>/* GuidTable */
>{
>  ${BEGIN}${GUID_STRUCTURE},
>  ${END}
>},
> +  /* StringHead */
>  ${BEGIN}  { ${STRING_HEAD_VALUE} }, /*
> ${STRING_HEAD_CNAME_DECL}_${STRING_HEAD_GUID_DECL}[${STRING_HEAD_NUMSKUS_DECL}]
>  */
>  ${END}
> +  /* PcdNameTable */
> +  /* TokenSpaceCNameIndex, PcdCNameIndex, TokenSpaceCNameIndex, 
> PcdCNameIndex, .. */
> +  {
> +${BEGIN}${PCD_NAME_OFFSET},
> +${END}
> +  },
> +  /* VariableHead */
> +  /* StringIndex, DefaultValueOffset, GuidTableIndex, Offset, Attributes, 
> Property */
>  ${BEGIN}  /* 
> ${VARIABLE_HEAD_CNAME_DECL}_${VARIABLE_HEAD_GUID_DECL}_Variable_Header[${VARIABLE_HEAD_NUMSKUS_DECL}]
> */
>{
> -${VARIABLE_HEAD_VALUE}
> +${VARIABLE_DB_VALUE}
>},
>  ${END}
>  /* SkuHead */
>{
>${BEGIN} offsetof (${PHASE}_PCD_DATABASE, 
> ${TOKEN_INIT}.${TOKEN_CNAME}_${TOKEN_GUID}${VARDEF_HEADER}) |
> ${TOKEN_TYPE}, /* */
> @@ -76,10 +85,20 @@ ${END}
>${END}
>},
>   /* StringTable */
>  ${BEGIN}  ${STRING_TABLE_VALUE}, /* 
> ${STRING_TABLE_CNAME}_${STRING_TABLE_GUID} */
>  ${END}
> +  /* PcdTokenTable */
> +   {
> +${BEGIN}${PCD_TOKENSPACE}, /* PCD_TOKENSPACE */
> +${END}
> +  },
> +  /* PcdCNameTable */
> +  {
> +${BEGIN}${PCD_CNAME}, /* PCD_CNAME */
> +${END}
> +  },
>/* SizeTable */
>{
>  ${BEGIN}${SIZE_TABLE_MAXIMUM_LENGTH}, ${SIZE_TABLE_CURRENT_LENGTH}, /* 
> ${SIZE_TABLE_CNAME}_${SIZE_TABLE_GUID} */
>  ${END}
>},
> @@ -1221,18 +1240,10 @@ def CreatePcdDatabasePhaseSpecificAutoGen (Platform, 
> DynamicPcdList, Phase):
>  if VariableGuid not in GuidList:
>  GuidList.append(VariableGuid)
>  Dict['GUID_STRUCTURE'].append(VariableGuidStructure)
>  VariableHeadGuidIndex = GuidList.index(VariableGuid)
> 
> -if "PCD_TYPE_STRING" in Pcd.TokenTypeList:
> -VariableHeadValueList.append('%dU, 
> offsetof(%s_PCD_DATABASE, Init.%s_%s), %dU, %sU' %
> - (VariableHeadStringIndex, 
> Phase, CName, TokenSpaceGuid,
> - VariableHeadGuidIndex, 
> Sku.VariableOffset))
> -else:
> -VariableHeadValueList.append('%dU, 
> offsetof(%s_PCD_DATABASE, Init.%s_%s_VariableDefault_%s), %dU, %sU' %
> - (VariableHeadStringIndex, 
> Phase, CName, TokenSpaceGuid, SkuIdIndex,
> - VariableHeadGuidIndex, 
> Sku.VariableOffset))
>  Dict['VARDEF_CNAME_'+Pcd.DatumType].append(CName)
>  Dict['VARDEF_GUID_'+Pcd.DatumType].append(TokenSpaceGuid)
>  Dict['VARDEF_SKUID_'+Pcd.DatumType].append(SkuIdIndex)
>  if "PCD_TYPE_STRING" in  Pcd.TokenTypeList:
>  Dict['VARDEF_VALUE_' + Pcd.DatumType].append("%s_%s[%d]" 
> % (Pcd.TokenCName, TokenSpaceGuid,
> SkuIdIndex))
> @@ -1571,13 +1582,11 @@ 

Re: [edk2] [PATCH] BaseTools:Function application error

2019-02-14 Thread Carsey, Jaben
I am really confused by this patch and how it ever worked.

I see that you remove the import for sdict.  But I see that sdict is used in 
other places in that file.

I also cant find a class definition for sdict anywhere (certainly not in 
Common/Misc).

So maybe refactor and remove all the other uses of sdict from the file?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Fan, ZhijuX
> Sent: Thursday, February 14, 2019 1:12 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH] BaseTools:Function application error
> 
> Error due to incorrect function parameters and attributes
> FileWrite() The first argument it needs is a list, not a file
> This patch abandons this function and saves the file independently
> 
> Cc: Bob Feng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhiju.Fan 
> ---
>  BaseTools/Source/Python/Eot/EotMain.py| 5 +++--
>  BaseTools/Source/Python/Workspace/BuildClassObject.py | 1 +
>  BaseTools/Source/Python/build/BuildReport.py  | 9 ++---
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Eot/EotMain.py
> b/BaseTools/Source/Python/Eot/EotMain.py
> index fd4bee6f90..8c2bfc45e4 100644
> --- a/BaseTools/Source/Python/Eot/EotMain.py
> +++ b/BaseTools/Source/Python/Eot/EotMain.py
> @@ -21,7 +21,7 @@ import Eot.EotGlobalData as EotGlobalData
>  from optparse import OptionParser
>  from Common.StringUtils import NormPath
>  from Common import BuildToolError
> -from Common.Misc import GuidStructureStringToGuidString, sdict
> +from Common.Misc import GuidStructureStringToGuidString
>  from Eot.Parser import *
>  from Eot.InfParserLite import EdkInfParser
>  from Common.StringUtils import GetSplitValueList
> @@ -32,6 +32,7 @@ from Eot.Report import Report
>  from Common.BuildVersion import gBUILD_VERSION
>  from Eot.Parser import ConvertGuid
>  from Common.LongFilePathSupport import OpenLongFilePath as open
> +import collections
>  import struct
>  import uuid
>  import copy
> @@ -57,7 +58,7 @@ class Image(array):
>  self._LEN_ = None
>  self._OFF_ = None
> 
> -self._SubImages = sdict() # {offset: Image()}
> +self._SubImages = collections.OrderedDict() # {offset: Image()}
> 
>  array.__init__(self)
> 
> diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> index cff77a71ae..6f8a09e87c 100644
> --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> @@ -261,6 +261,7 @@ class StructurePcd(PcdClassObject):
>  self.PackageDecs = Packages
>  self.DefaultStoreName = [default_store]
>  self.DefaultValues = OrderedDict()
> +self.DefaultFromDSC = None
>  self.PcdMode = None
>  self.SkuOverrideValues = OrderedDict()
>  self.StructName = None
> diff --git a/BaseTools/Source/Python/build/BuildReport.py
> b/BaseTools/Source/Python/build/BuildReport.py
> index 0b98d62cb6..70584570a5 100644
> --- a/BaseTools/Source/Python/build/BuildReport.py
> +++ b/BaseTools/Source/Python/build/BuildReport.py
> @@ -1651,14 +1651,17 @@ class PredictionReport(object):
>  SourceList = os.path.join(self._EotDir, "SourceFile.txt")
>  GuidList = os.path.join(self._EotDir, "GuidList.txt")
>  DispatchList = os.path.join(self._EotDir, "Dispatch.txt")
> -
> +TempList = []
>  TempFile = open(SourceList, "w+")
>  for Item in self._SourceList:
> -FileWrite(TempFile, Item)
> +TempList.append(Item + TAB_LINE_BREAK)
> +TempFile.writelines(TempList)
>  TempFile.close()
> +TempList = []
>  TempFile = open(GuidList, "w+")
>  for Key in self._GuidMap:
> -FileWrite(TempFile, "%s %s" % (Key, self._GuidMap[Key]))
> +TempList.append("%s %s %s" % (Key, self._GuidMap[Key],
> TAB_LINE_BREAK))
> +TempFile.writelines(TempList)
>  TempFile.close()
> 
>  try:
> --
> 2.14.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 v5 edk2-platforms 00/22] Platform/RaspberryPi: Add Raspberry Pi 3 support

2019-02-14 Thread Leif Lindholm
On Tue, Feb 05, 2019 at 04:25:15PM +, Pete Batard wrote:
> Changes for v5:
> 
> * Raspberry/Pi3 -> RaspberryPi/RPi3
> * Remove VirtualRealTimeClockLib as well as BUILD_EPOCH macro (use the 
> upcoming
>   EmbeddedPkg Virtual RTC from EDK2 instead)
> * Use $(PLATFORM_NAME) where possible in .dsc and .fdf
> * Update Readme to remove build instructions, describe ACPI limitations, fix
>   ATF Readme link and split OS installation & test notes into a separate file.
> * Add -Wl,--fix-cortex-a53-843419 to LINK_FLAGS
> 
> IMPORTANT: Due to the removal of VirtualRealTimeClockLib this series requires
> https://lists.01.org/pipermail/edk2-devel/2019-February/036301.html to have
> been applied to your edk2 repository.

All of the comments I raised on previous versions of this set have
been addressed. The VirtualRtc driver has been moved separately and I
have provided feedback on that for the new edk2 version.
Other than that, I gave an R-b for the added documentation, but I
cannot claim to have properly looked at the rest of the platform
support.

I'll just mention here formally that I see no reason for Ard not to
push this support once he is happy with it (and the Rtc driver is in
edk2).

Regards,

Leif


> Regards,
> 
> /Pete
> 
> 
> Pete Batard (22):
>   Silicon/Broadcom/Bcm283x: Add interrupt driver
>   Silicon/Broadcom/Bcm283x: Add GpioLib
>   Platform/RaspberryPi/RPi3: Add ACPI tables
>   Platform/RaspberryPi/RPi3: Add reset and memory init libraries
>   Platform/RaspberryPi/RPi3: Add platform library
>   Platform/RaspberryPi/RPi3: Add firmware driver
>   Platform/RaspberryPi/RPi3: Add platform config driver
>   Platform/RaspberryPi/RPi3: Add SMBIOS driver
>   Platform/RaspberryPi/RPi3: Add display driver
>   Platform/RaspberryPi/RPi3: Add console driver
>   Platform/RaspberryPi/RPi3: Add NV storage driver
>   Platform/RaspberryPi/RPi3: Add Device Tree driver
>   Platform/RaspberryPi/RPi3: Add base MMC driver
>   Platform/RaspberryPi/RPi3: Add Arasan MMC driver
>   Platform/RaspberryPi/RPi3: Add SD Host driver
>   Platform/RaspberryPi/RPi3: Add platform boot manager and helper libs
>   Platform/RaspberryPi/RPi3: Add USB host driver
>   Platform/RaspberryPi/RPi3 *NON-OSI*: Add ATF binaries
>   Platform/RaspberryPi/RPi3 *NON-OSI*: Add Device Tree binaries
>   Platform/RaspberryPi/RPi3 *NON-OSI*: Add logo driver
>   Platform/RaspberryPi/RPi3: Add platform
>   Platform/RaspberryPi/RPi3: Add platform readme's
> 
>  .../RaspberryPi/RPi3/AcpiTables/AcpiTables.h  |   82 +
>  .../RPi3/AcpiTables/AcpiTables.inf|   46 +
>  .../RaspberryPi/RPi3/AcpiTables/Csrt.aslc |  332 +++
>  .../RaspberryPi/RPi3/AcpiTables/Dbg2.aslc |   34 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Dsdt.asl |  511 +
>  .../RaspberryPi/RPi3/AcpiTables/Fadt.aslc |   52 +
>  .../RaspberryPi/RPi3/AcpiTables/Gtdt.aslc |   33 +
>  .../RaspberryPi/RPi3/AcpiTables/Madt.aslc |   62 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Pep.asl  |   95 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Pep.c|   84 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Pep.h|  126 ++
>  Platform/RaspberryPi/RPi3/AcpiTables/Rhpx.asl |  201 ++
>  Platform/RaspberryPi/RPi3/AcpiTables/Sdhc.asl |  105 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Spcr.asl |   53 +
>  Platform/RaspberryPi/RPi3/AcpiTables/Uart.asl |  158 ++
>  .../RaspberryPi/RPi3/DeviceTree/License.txt   |  340 +++
>  .../RPi3/DeviceTree/bcm2710-rpi-3-b-plus.dtb  |  Bin 0 -> 25617 bytes
>  .../RPi3/DeviceTree/bcm2710-rpi-3-b-plus.dts  | 1263 
>  .../RPi3/DeviceTree/bcm2710-rpi-3-b.dtb   |  Bin 0 -> 25354 bytes
>  .../RPi3/DeviceTree/bcm2710-rpi-3-b.dts   | 1259 +++
>  .../ArasanMmcHostDxe/ArasanMmcHostDxe.c   |  723 +++
>  .../ArasanMmcHostDxe/ArasanMmcHostDxe.h   |   50 +
>  .../ArasanMmcHostDxe/ArasanMmcHostDxe.inf |   52 +
>  .../RPi3/Drivers/ConfigDxe/ConfigDxe.c|  351 
>  .../RPi3/Drivers/ConfigDxe/ConfigDxe.inf  |   78 +
>  .../Drivers/ConfigDxe/ConfigDxeFormSetGuid.h  |   23 +
>  .../RPi3/Drivers/ConfigDxe/ConfigDxeHii.uni   |  100 +
>  .../RPi3/Drivers/ConfigDxe/ConfigDxeHii.vfr   |  306 +++
>  .../RPi3/Drivers/DisplayDxe/ComponentName.c   |  222 ++
>  .../RPi3/Drivers/DisplayDxe/DisplayDxe.c  |  606 ++
>  .../RPi3/Drivers/DisplayDxe/DisplayDxe.h  |   42 +
>  .../RPi3/Drivers/DisplayDxe/DisplayDxe.inf|   71 +
>  .../RPi3/Drivers/DisplayDxe/Screenshot.c  |  375 
>  .../RPi3/Drivers/DwUsbHostDxe/ComponentName.c |  225 ++
>  .../RPi3/Drivers/DwUsbHostDxe/DriverBinding.c |  274 +++
>  .../RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.c  | 1635 +++
>  .../RPi3/Drivers/DwUsbHostDxe/DwUsbHostDxe.h  |  162 ++
>  .../Drivers/DwUsbHostDxe/DwUsbHostDxe.inf |   59 +
>  .../RPi3/Drivers/DwUsbHostDxe/DwcHw.h |  788 +++
>  .../RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c  |  364 
>  .../RPi3/Drivers/FdtDxe/FdtDxe.inf|   53 +
>  .../GraphicsConsoleDxe/ComponentName.c|  183 ++
>  

Re: [edk2] Issues with EDK-II-Debugging wiki page, and how to submit a pull request for wiki pages

2019-02-14 Thread Laszlo Ersek
On 02/14/19 18:39, Rebecca Cran via edk2-devel wrote:
> I noticed a couple of issues with the page
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Debugging -
> 'SNI' should be 'SNP', and the |PcdDebugPrintErrorLevel is missing some
> entries.
> |
> 
> 
> How do I fork the wiki repo to make a pull request?

I suggest the following:

(1) clone the wiki to your local workstation. The repo URL is

  git://github.com/tianocore/tianocore.github.io.wiki

(2) In any project that you have *on github*, enable the wiki (specific
to that project). Then, add *that* wiki as a "remote" to your local
clone that was made in step (1):

[remote "mine"]
url = g...@github.com:GITHUB_USERNAME/PROJECT.wiki.git
tagopt = --no-tags

The command I recommend is

  git remote add --no-tags mine \
g...@github.com:GITHUB_USERNAME/PROJECT.wiki.git

(3) You can develop wiki article changes in your local repo, on a
dedicated topic branch. Simply commit and rebase as you see fit.

(4) For rendering your local changes, force-push your local topic branch
to the master branch of your own wiki:

  git push --force mine TOPIC_BRANCH:master

Then, if you refresh your own wiki page, on github, in your browser, you
should see the changes.

Please note that this step is destructive, with regard to the personal
wiki that you push to.

In addition, IIRC, this step requires SSH pubkey auth to be enabled in
your github profile. (Step 2 could require it too; I'm not certain.)

(5) Once you are done with development, simply format the patches as
usual, and submit them to edk2-devel with git-send-email. For
formatting, use the parameter

  --subject-prefix='edk2-wiki PATCH'

This will tell reviewers the patches are for the wiki.

I can help apply the changes when they are suitably reviewed.

I'm not sure if we have designated maintainers/reviewers for wiki pages.
Usually there is a related area in edk2, so I guess the edk2
maintainer(s) could double in the docs reviewer role too.

> On each page there's
> a "Clone this wiki locally" box where I can clone
> https://github.com/tianocore/tianocore.github.io.wiki.git, but when I
> try and fork it I instead fork
> https://github.com/tianocore/tianocore.github.io which doesn't have the
> same files either on the master or gh-pages branches - it looks like
> https://github.com/tianocore/tianocore.github.io has the files for
> www.tianocore.org, not the wiki?

The wikis on github are non-intuitive, if you intend to edit them in a
normal text editor, locally. I struggled a lot until I came up with the
list on top. (It's possible I relied on others' advice in that; I no
longer remember.) I hope it helps.

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


Re: [edk2] [PATCH v5 edk2-platforms 22/22] Platform/RaspberryPi/RPi3: Add platform readme's

2019-02-14 Thread Leif Lindholm
On Tue, Feb 05, 2019 at 04:25:37PM +, Pete Batard wrote:
> Documentation is split between general plaform data and OS testing
> and installation details.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pete Batard 

Reviewed-by: Leif Lindholm 

> ---
>  Platform/RaspberryPi/RPi3/Readme.md  | 167 
>  Platform/RaspberryPi/RPi3/Systems.md |  65 
>  Readme.md|   3 +
>  3 files changed, 235 insertions(+)
> 
> diff --git a/Platform/RaspberryPi/RPi3/Readme.md 
> b/Platform/RaspberryPi/RPi3/Readme.md
> new file mode 100644
> index ..7434233df0fb
> --- /dev/null
> +++ b/Platform/RaspberryPi/RPi3/Readme.md
> @@ -0,0 +1,167 @@
> +Raspberry Pi Platform
> +=
> +
> +# Summary
> +
> +This is a port of 64-bit Tiano Core UEFI firmware for the Raspberry Pi 3/3B+ 
> platforms,
> +based on [Ard Bisheuvel's 
> 64-bit](http://www.workofard.com/2017/02/uefi-on-the-pi/)
> +and [Microsoft's 
> 32-bit](https://github.com/ms-iot/RPi-UEFI/tree/ms-iot/Pi3BoardPkg)
> +implementations, as maintained by [Andrei 
> Warkentin](https://github.com/andreiw/RaspberryPiPkg).
> +
> +This is meant as a generally useful 64-bit ATF + UEFI implementation for the 
> Raspberry
> +Pi 3/3B+ which should be good enough for most kind of UEFI development, as 
> well as for
> +running consummer Operating Systems in such as Linux or Windows.
> +
> +Raspberry Pi is a trademark of the [Raspberry Pi 
> Foundation](http://www.raspberrypi.org).
> +
> +# Status
> +
> +This firmware, that has been validated to compile against the current
> +[edk2](https://github.com/tianocore/edk2)/[edk2-platforms](https://github.com/tianocore/edk2-platforms),
> +should be able to boot Linux (SUSE, Ubuntu), NetBSD, FreeBSD as well as 
> Windows 10 ARM64
> +(full GUI version).
> +
> +It also provides support for ATF ([Arm Trusted 
> Platform](https://github.com/ARM-software/arm-trusted-firmware)).
> +
> +HDMI and the mini-UART serial port can be used for output devices, with 
> mirrored output.
> +USB keyboards and the mini-UART serial port can be used as input.
> +
> +On a freshly built firmware, the default is to boot the UEFI shell.
> +To change the default boot order (for instance to boot uSD media by default) 
> you
> +will need to edit the preferences in _Boot Maintenance Manager_.
> +
> +For additional information about the tested systems and how to set them up,
> +please see [Systems.md](./Systems.md).
> +
> +# Building
> +
> +Build instructions from the top level edk2-platforms Readme.md apply.
> +
> +# Booting the firmware
> +
> +1. Format a uSD card as FAT32
> +2. Copy the generated `RPI_EFI.fd` firmware onto the partition
> +3. Download and copy the following files from 
> https://github.com/raspberrypi/firmware/tree/master/boot
> +  - `bootcode.bin`
> +  - `fixup.dat`
> +  - `start.elf`
> +4. Create a `config.txt` with the following content:
> +  ```
> +  arm_control=0x200
> +  enable_uart=1
> +  armstub=RPI_EFI.fd
> +  disable_commandline_tags=1
> +  ```
> +5. Insert the uSD card and power up the Pi.
> +
> +Note that if you have a model 3+ or a model 3 where you enabled USB boot 
> through OTP
> +(see 
> [here](https://www.raspberrypi.org/documentation/hardware/raspberrypi/bootmodes/msd.md))
> +you may also be able to boot from a FAT32 USB driver rather than uSD.
> +
> +# Notes
> +
> +## ARM Trusted Firmware (ATF)
> +
> +The ATF binaries being used were compiled from the latest ATF source.
> +No aleration to the official source have been applied.
> +
> +For more details on the ATF compilation, see the 
> [Readme](./TrustedFirmware/Readme.md)
> +in the `TrustedFirmware/` directory.
> +
> +## Custom Device Tree
> +
> +The default Device Tree included in the firmware is the one for a Raspberry 
> Pi 3 Model B (not B+).
> +If you want to use a different Device Tree, to boot a Pi 3 Model B+ for 
> instance (for which a
> +DTB is also provided under `DeviceTree/`), you should copy the relevant 
> `.dtb` into the root of
> +the SD or USB, and then edit your `config.txt` so that it looks like:
> +
> +```
> +(...)
> +disable_commandline_tags=2
> +device_tree_address=0x1
> +device_tree_end=0x2
> +device_tree=bcm2710-rpi-3-b-plus.dtb
> +```
> +
> +Note: the address range **must** be `[0x1:0x2]`.
> +`dtoverlay` and `dtparam` parameters are also supported **when** providing a 
> Device Tree`.
> +
> +## Custom `bootargs`
> +
> +This firmware will honor the command line passed by the GPU via 
> `cmdline.txt`.
> +
> +Note, that the ultimate contents of `/chosen/bootargs` are a combination of 
> several pieces:
> +- Original `/chosen/bootargs` if using the internal DTB. Seems to be 
> completely discarded by GPU when booting with a custom device tree.
> +- GPU-passed hardware configuration. This one is always present.
> +- Additional boot options passed via `cmdline.txt`.
> +
> +# Limitations
> +
> +## HDMI
> +
> +The UEFI HDMI video support relies 

Re: [edk2] [edk2-announce] Community Meeting Minutes

2019-02-14 Thread Jeremiah Cox via edk2-devel
Hi Ard,
My apologies as no insult was intended.  The suggestion is that, similar to 
today, folks encountering difficulties with our systems could reach out to the 
TianoCore discussion venue (our mailing list or groups.io), and our friendly 
community members (we have many) will surely assist them.

You are correct that my focus is not casual contributors, rather I want to 
encourage a large number of UEFI developers who are currently closed to stop 
their fork-modify-ship model, which is inefficient to service, go open to share 
their learnings, get current, stay current, upstream their changes (where it 
makes sense to the community), but not throw garbage over the wall.   I think 
there is some value in this endeavor.

Kind Regards,
Jeremiah


From: Ard Biesheuvel 
Sent: Friday, February 8, 2019 5:58 AM
To: Jeremiah Cox
Cc: stephano; edk2-devel@lists.01.org; Kinney, Michael D; Laszlo Ersek
Subject: Re: [edk2] [edk2-announce] Community Meeting Minutes

On Thu, 7 Feb 2019 at 18:52, Jeremiah Cox via edk2-devel
 wrote:
>
> Apologies on the late reply, I was on vacation for several weeks and just got 
> back to this.
>
> Regarding "Patch Review System Evaluation", on the call, I disagreed with 
> your conclusion, but that note is not captured below.  My reading of the 
> email and call discussions, I did not hear our community reject GitHub, 
> rather observations that it was not "perfect", that it does not transparently 
> interact with folks who prefer mailing list patch systems, but it would be 
> acceptable to try.  On the call you mentioned a second justification for 
> staying with the mailing list system, that GitHub (really all modern patch 
> management systems) exclude folks who have limited internet connectivity.  I 
> hypothesize that this theoretical group of Tianocore contributors would be a 
> very small group of folks.  Should our community optimize our systems for a 
> very small, theoretical group, penalizing the overwhelming majority?  I would 
> propose that we try a modern patch management system, GitHub had the best 
> reviews in my reading, and folks with limited internet connectivity find a 
> friend to act as a go between translating their email diffs into GitHub PRs.

I find this unnecessarily condescending. Finding a friend, seriously?

Very serious concerns have been raised about the lack of transparency
with the various systems, and the fact that I am able to consult my
own local copy of the entire review history, including all email
exchanges is a very important aspect of the current model to me, as
opposed to GitHub deciding what is important enough to keep and what
is not. In an open source project, the code base is *not* the HEAD
commit, it is the entire repository, including history, and logged
email threads with technical discussions, since they are usually not
captured in other ways.

The push to GitHub is being sold to us as a way to attract more
contributors, but it seems to me (and I have stated this multiple
times) that the mailing list is not the steep part of the learning
curve when contributing to TianoCore. So is this really about getting
outsiders to contribute to Tianocore? Or is it about reducing the
impedance mismatch with what internal teams at MicroSoft (and Intel?)
are doing, which probably already went through the learning curve when
it comes to other aspects of Tianocore.

>From a high level, it might seem that using a mailing list is the
impediment here. But in reality, contributing to open source in
general is not about whether you use GitHub or a mailing list to throw
your stuff over the fence. It is about collaborating with the
community to find common ground between the various sometimes
conflicting interests, and permitting your engineers to engage with
the community.

Tianocore has always been a rather peculiar open source project, since
it wasn't more than just that, i.e., openly available source code.
This has been changing for the better during the time I have been
involved, and we have worked very hard with the Intel firmware team
and other contributors to collaborate better on the mailing list.

To summarize my concern here: it seems that this push is not about
making it easier for contributors that already know how to do open
source collaboration to contribute to Tianocore, it is about making it
easier for currently closed code to be contributed to Tianocore by
teams who have no prior experience with open source.

Apologies if I have the wrong end of the stick here. If not, why don't
we consult a few casual contributors (which should be easy to find on
the mailing list) and ask them what their biggest issues were with
contributing to Tianocore?






>
> -Original Message-
> From: edk2-devel  On Behalf Of stephano
> Sent: Friday, January 11, 2019 11:27 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Laszlo Ersek 
> 
> Subject: [edk2] [edk2-announce] Community Meeting Minutes
>
> An HTML 

[edk2] Issues with EDK-II-Debugging wiki page, and how to submit a pull request for wiki pages

2019-02-14 Thread Rebecca Cran via edk2-devel
I noticed a couple of issues with the page 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Debugging - 
'SNI' should be 'SNP', and the |PcdDebugPrintErrorLevel is missing some 
entries.

|


How do I fork the wiki repo to make a pull request? On each page there's 
a "Clone this wiki locally" box where I can clone 
https://github.com/tianocore/tianocore.github.io.wiki.git, but when I 
try and fork it I instead fork 
https://github.com/tianocore/tianocore.github.io which doesn't have the 
same files either on the master or gh-pages branches - it looks like 
https://github.com/tianocore/tianocore.github.io has the files for 
www.tianocore.org, not the wiki?



--
Rebecca Cran

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


Re: [edk2] [edk2-announce] Community Meeting Minutes

2019-02-14 Thread Rebecca Cran via edk2-devel
As a casual contributor, for me the biggest complaint I have is how busy 
the mailing list gets. I don't think a new 'announce' list is what's 
needed, perhaps a 'reviews' or 'discussion' list to split out 
discussions (from anyone) from day-to-day patches? Also, I'd be anxious 
about jumping to a new service like groups.io: most open source 
developers understand plain email, and personally I'd like that to stay. 
For example FreeBSD set up web forums, but most contributors continue to 
use the existing mailman based lists, and I suspect tend to forget the 
web interface exists.



One thing I feel that's missing from the current Github-based 
infrastructure of the web site and wiki is that as far as I know there's 
no API documentation built regularly, or automated builds etc. I'm 
hosting the API documentation at e.g. 
https://code.bluestop.org/edk2/docs/master/ . Also, one thing a review 
system like Gerrit, Github, Phabricator, Review Board etc. would give us 
is the ability to run tests (lint, build/run OVMF etc.) against patches 
and have it comment on the review about its status to give committers 
more confidence in it.



--

Rebecca Cran


On 2/14/19 12:07 PM, Jeremiah Cox via edk2-devel wrote:

Hi Ard,
My apologies as no insult was intended.  The suggestion is that, similar to 
today, folks encountering difficulties with our systems could reach out to the 
TianoCore discussion venue (our mailing list or groups.io), and our friendly 
community members (we have many) will surely assist them.

You are correct that my focus is not casual contributors, rather I want to 
encourage a large number of UEFI developers who are currently closed to stop 
their fork-modify-ship model, which is inefficient to service, go open to share 
their learnings, get current, stay current, upstream their changes (where it 
makes sense to the community), but not throw garbage over the wall.   I think 
there is some value in this endeavor.

Kind Regards,
Jeremiah


From: Ard Biesheuvel 
Sent: Friday, February 8, 2019 5:58 AM
To: Jeremiah Cox
Cc: stephano; edk2-devel@lists.01.org; Kinney, Michael D; Laszlo Ersek
Subject: Re: [edk2] [edk2-announce] Community Meeting Minutes

On Thu, 7 Feb 2019 at 18:52, Jeremiah Cox via edk2-devel
 wrote:

Apologies on the late reply, I was on vacation for several weeks and just got 
back to this.

Regarding "Patch Review System Evaluation", on the call, I disagreed with your 
conclusion, but that note is not captured below.  My reading of the email and call discussions, I 
did not hear our community reject GitHub, rather observations that it was not "perfect", 
that it does not transparently interact with folks who prefer mailing list patch systems, but it 
would be acceptable to try.  On the call you mentioned a second justification for staying with the 
mailing list system, that GitHub (really all modern patch management systems) exclude folks who 
have limited internet connectivity.  I hypothesize that this theoretical group of Tianocore 
contributors would be a very small group of folks.  Should our community optimize our systems for a 
very small, theoretical group, penalizing the overwhelming majority?  I would propose that we try a 
modern patch management system, GitHub had the best reviews in my reading, and folks with limited 
internet connectivity find a friend to act as a go between translating their email diffs into 
GitHub PRs.

I find this unnecessarily condescending. Finding a friend, seriously?

Very serious concerns have been raised about the lack of transparency
with the various systems, and the fact that I am able to consult my
own local copy of the entire review history, including all email
exchanges is a very important aspect of the current model to me, as
opposed to GitHub deciding what is important enough to keep and what
is not. In an open source project, the code base is *not* the HEAD
commit, it is the entire repository, including history, and logged
email threads with technical discussions, since they are usually not
captured in other ways.

The push to GitHub is being sold to us as a way to attract more
contributors, but it seems to me (and I have stated this multiple
times) that the mailing list is not the steep part of the learning
curve when contributing to TianoCore. So is this really about getting
outsiders to contribute to Tianocore? Or is it about reducing the
impedance mismatch with what internal teams at MicroSoft (and Intel?)
are doing, which probably already went through the learning curve when
it comes to other aspects of Tianocore.

 From a high level, it might seem that using a mailing list is the
impediment here. But in reality, contributing to open source in
general is not about whether you use GitHub or a mailing list to throw
your stuff over the fence. It is about collaborating with the
community to find common ground between the various sometimes
conflicting interests, and permitting your engineers to engage 

Re: [edk2] [edk2-announce] Community Meeting Minutes

2019-02-14 Thread Kinney, Michael D
Rebecca,

You can review the groups.io features.  I think what you 
want is available.  Stephano has also setup an edk2 area
in groups.io for community member to try out.

https://groups.io/static/help#features

There are a number of CI services that are integrated with
GitHub.

https://github.com/marketplace/category/continuous-integration

There is work to be done to enable one of these CI services
for edk2.  Stephano has a community task to evaluate and
select a CI service.

Best regards,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Rebecca Cran via
> edk2-devel
> Sent: Thursday, February 14, 2019 12:28 PM
> To: Jeremiah Cox ; Ard
> Biesheuvel 
> Cc: Kinney, Michael D ;
> edk2-devel@lists.01.org; Laszlo Ersek
> 
> Subject: Re: [edk2] [edk2-announce] Community Meeting
> Minutes
> 
> As a casual contributor, for me the biggest complaint I
> have is how busy
> the mailing list gets. I don't think a new 'announce'
> list is what's
> needed, perhaps a 'reviews' or 'discussion' list to
> split out
> discussions (from anyone) from day-to-day patches?
> Also, I'd be anxious
> about jumping to a new service like groups.io: most
> open source
> developers understand plain email, and personally I'd
> like that to stay.
> For example FreeBSD set up web forums, but most
> contributors continue to
> use the existing mailman based lists, and I suspect
> tend to forget the
> web interface exists.
> 
> 
> One thing I feel that's missing from the current
> Github-based
> infrastructure of the web site and wiki is that as far
> as I know there's
> no API documentation built regularly, or automated
> builds etc. I'm
> hosting the API documentation at e.g.
> https://code.bluestop.org/edk2/docs/master/ . Also, one
> thing a review
> system like Gerrit, Github, Phabricator, Review Board
> etc. would give us
> is the ability to run tests (lint, build/run OVMF etc.)
> against patches
> and have it comment on the review about its status to
> give committers
> more confidence in it.
> 
> 
> --
> 
> Rebecca Cran
> 
> 
> On 2/14/19 12:07 PM, Jeremiah Cox via edk2-devel wrote:
> > Hi Ard,
> > My apologies as no insult was intended.  The
> suggestion is that, similar to today, folks
> encountering difficulties with our systems could reach
> out to the TianoCore discussion venue (our mailing list
> or groups.io), and our friendly community members (we
> have many) will surely assist them.
> >
> > You are correct that my focus is not casual
> contributors, rather I want to encourage a large number
> of UEFI developers who are currently closed to stop
> their fork-modify-ship model, which is inefficient to
> service, go open to share their learnings, get current,
> stay current, upstream their changes (where it makes
> sense to the community), but not throw garbage over the
> wall.   I think there is some value in this endeavor.
> >
> > Kind Regards,
> > Jeremiah
> >
> > 
> > From: Ard Biesheuvel 
> > Sent: Friday, February 8, 2019 5:58 AM
> > To: Jeremiah Cox
> > Cc: stephano; edk2-devel@lists.01.org; Kinney,
> Michael D; Laszlo Ersek
> > Subject: Re: [edk2] [edk2-announce] Community Meeting
> Minutes
> >
> > On Thu, 7 Feb 2019 at 18:52, Jeremiah Cox via edk2-
> devel
> >  wrote:
> >> Apologies on the late reply, I was on vacation for
> several weeks and just got back to this.
> >>
> >> Regarding "Patch Review System Evaluation", on the
> call, I disagreed with your conclusion, but that note
> is not captured below.  My reading of the email and
> call discussions, I did not hear our community reject
> GitHub, rather observations that it was not "perfect",
> that it does not transparently interact with folks who
> prefer mailing list patch systems, but it would be
> acceptable to try.  On the call you mentioned a second
> justification for staying with the mailing list system,
> that GitHub (really all modern patch management
> systems) exclude folks who have limited internet
> connectivity.  I hypothesize that this theoretical
> group of Tianocore contributors would be a very small
> group of folks.  Should our community optimize our
> systems for a very small, theoretical group, penalizing
> the overwhelming majority?  I would propose that we try
> a modern patch management system, GitHub had the best
> reviews in my reading, and folks with limited internet
> connectivity find a friend to act as a go between
> translating their email diffs into GitHub PRs.
> > I find this unnecessarily condescending. Finding a
> friend, seriously?
> >
> > Very serious concerns have been raised about the lack
> of transparency
> > with the various systems, and the fact that I am able
> to consult my
> > own local copy of the entire review history,
> including all email
> > exchanges is a very important aspect of the current
> model to me, as
> > opposed to GitHub deciding what is important enough
> to keep and what
> > is not. In an open 

[edk2] FW: [PATCH] BaseTools:Function application error

2019-02-14 Thread Fan, ZhijuX


The sdict() was originally defined as common/Misc.py and UPT/library/Misc.py.

However, the sdict() defined previously in common/Misc.py has been removed.

It is not as good as the OrderedDict() when running in Python3.


Any question, please let me know. Thanks.

Best Regards
Fan Zhiju



-Original Message-
From: Carsey, Jaben 
Sent: Thursday, February 14, 2019 11:31 PM
To: Fan, ZhijuX ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [PATCH] BaseTools:Function application error

I am really confused by this patch and how it ever worked.

I see that you remove the import for sdict.  But I see that sdict is used in 
other places in that file.

I also cant find a class definition for sdict anywhere (certainly not in 
Common/Misc).

So maybe refactor and remove all the other uses of sdict from the file?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Fan, ZhijuX
> Sent: Thursday, February 14, 2019 1:12 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH] BaseTools:Function application error
> 
> Error due to incorrect function parameters and attributes
> FileWrite() The first argument it needs is a list, not a file This 
> patch abandons this function and saves the file independently
> 
> Cc: Bob Feng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhiju.Fan 
> ---
>  BaseTools/Source/Python/Eot/EotMain.py| 5 +++--
>  BaseTools/Source/Python/Workspace/BuildClassObject.py | 1 +
>  BaseTools/Source/Python/build/BuildReport.py  | 9 ++---
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Eot/EotMain.py
> b/BaseTools/Source/Python/Eot/EotMain.py
> index fd4bee6f90..8c2bfc45e4 100644
> --- a/BaseTools/Source/Python/Eot/EotMain.py
> +++ b/BaseTools/Source/Python/Eot/EotMain.py
> @@ -21,7 +21,7 @@ import Eot.EotGlobalData as EotGlobalData  from 
> optparse import OptionParser  from Common.StringUtils import NormPath  
> from Common import BuildToolError -from Common.Misc import 
> GuidStructureStringToGuidString, sdict
> +from Common.Misc import GuidStructureStringToGuidString
>  from Eot.Parser import *
>  from Eot.InfParserLite import EdkInfParser  from Common.StringUtils 
> import GetSplitValueList @@ -32,6 +32,7 @@ from Eot.Report import 
> Report  from Common.BuildVersion import gBUILD_VERSION  from 
> Eot.Parser import ConvertGuid  from Common.LongFilePathSupport import 
> OpenLongFilePath as open
> +import collections
>  import struct
>  import uuid
>  import copy
> @@ -57,7 +58,7 @@ class Image(array):
>  self._LEN_ = None
>  self._OFF_ = None
> 
> -self._SubImages = sdict() # {offset: Image()}
> +self._SubImages = collections.OrderedDict() # {offset: 
> + Image()}
> 
>  array.__init__(self)
> 
> diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> index cff77a71ae..6f8a09e87c 100644
> --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> @@ -261,6 +261,7 @@ class StructurePcd(PcdClassObject):
>  self.PackageDecs = Packages
>  self.DefaultStoreName = [default_store]
>  self.DefaultValues = OrderedDict()
> +self.DefaultFromDSC = None
>  self.PcdMode = None
>  self.SkuOverrideValues = OrderedDict()
>  self.StructName = None
> diff --git a/BaseTools/Source/Python/build/BuildReport.py
> b/BaseTools/Source/Python/build/BuildReport.py
> index 0b98d62cb6..70584570a5 100644
> --- a/BaseTools/Source/Python/build/BuildReport.py
> +++ b/BaseTools/Source/Python/build/BuildReport.py
> @@ -1651,14 +1651,17 @@ class PredictionReport(object):
>  SourceList = os.path.join(self._EotDir, "SourceFile.txt")
>  GuidList = os.path.join(self._EotDir, "GuidList.txt")
>  DispatchList = os.path.join(self._EotDir, "Dispatch.txt")
> -
> +TempList = []
>  TempFile = open(SourceList, "w+")
>  for Item in self._SourceList:
> -FileWrite(TempFile, Item)
> +TempList.append(Item + TAB_LINE_BREAK)
> +TempFile.writelines(TempList)
>  TempFile.close()
> +TempList = []
>  TempFile = open(GuidList, "w+")
>  for Key in self._GuidMap:
> -FileWrite(TempFile, "%s %s" % (Key, self._GuidMap[Key]))
> +TempList.append("%s %s %s" % (Key, self._GuidMap[Key],
> TAB_LINE_BREAK))
> +TempFile.writelines(TempList)
>  TempFile.close()
> 
>  try:
> --
> 2.14.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

Re: [edk2] [PATCH v3 2/3] MdeModulePkg/PeiMain: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Zeng, Star
Reviewed-by: Star Zeng 

-Original Message-
From: Chiu, Chasel 
Sent: Thursday, February 14, 2019 6:59 PM
To: edk2-devel@lists.01.org
Cc: Wang, Jian J ; Wu, Hao A ; Ni, 
Ray ; Zeng, Star ; Gao, Liming 
; Chiu, Chasel 
Subject: [PATCH v3 2/3] MdeModulePkg/PeiMain: Support 
EFI_PEI_CORE_FV_LOCATION_PPI

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

When shadowing PeiCore the EFI_PEI_CORE_FV_LOCATION_PPI should be checked to 
see if PeiCore not in BFV, otherwise just shadowing PeiCore from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Ray Ni 
Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 53 
-
 MdeModulePkg/Core/Pei/PeiMain.h |  3 ++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  3 ++-
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c 
b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 4da80a8222..a7da90e149 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -1,7 +1,7 @@
 /** @file
   Pei Core Main Entry Point
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. 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 @@ -80,22 +80,49 @@ 
ShadowPeiCore (
   IN PEI_CORE_INSTANCE  *PrivateData
   )
 {
-  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
-  EFI_PHYSICAL_ADDRESS EntryPoint;
-  EFI_STATUS   Status;
-  UINT32   AuthenticationState;
+  EFI_PEI_FILE_HANDLE  PeiCoreFileHandle;
+  EFI_PHYSICAL_ADDRESS EntryPoint;
+  EFI_STATUS   Status;
+  UINT32   AuthenticationState;
+  UINTNIndex;
+  EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
+  UINTNPeiCoreFvIndex;
 
   PeiCoreFileHandle = NULL;
-
   //
-  // Find the PEI Core in the BFV
+  // Default PeiCore is in BFV
+  //
+  PeiCoreFvIndex = 0;
+  //
+  // Find the PEI Core either from EFI_PEI_CORE_FV_LOCATION_PPI 
+ indicated FV or BFV  //  Status = PeiServicesLocatePpi (
+ ,
+ 0,
+ NULL,
+ (VOID **) 
+ );
+  if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != 
NULL)) {
+//
+// If PeiCoreFvLocation present, the PEI Core should be found from 
indicated FV
+//
+for (Index = 0; Index < PrivateData->FvCount; Index ++) {
+  if (PrivateData->Fv[Index].FvHandle == 
PeiCoreFvLocationPpi->PeiCoreFvLocation) {
+PeiCoreFvIndex = Index;
+break;
+  }
+}
+ASSERT (Index < PrivateData->FvCount);  }  //  // Find PEI Core 
+ from the given FV index
   //
-  Status = PrivateData->Fv[0].FvPpi->FindFileByType (
-   PrivateData->Fv[0].FvPpi,
-   EFI_FV_FILETYPE_PEI_CORE,
-   PrivateData->Fv[0].FvHandle,
-   
-   );
+  Status = PrivateData->Fv[PeiCoreFvIndex].FvPpi->FindFileByType (
+
PrivateData->Fv[PeiCoreFvIndex].FvPpi,
+EFI_FV_FILETYPE_PEI_CORE,
+
PrivateData->Fv[PeiCoreFvIndex].FvHandle,
+
+);
   ASSERT_EFI_ERROR (Status);
 
   //
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h 
index 322e7cd845..a61da73fd8 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -1,7 +1,7 @@
 /** @file
   Definition of Pei Core Structures and Services
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. 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 @@ -31,6 +31,7 @@ 
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf 
b/MdeModulePkg/Core/Pei/PeiMain.inf
index 140cb1..5bab2aab8c 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -6,7 +6,7 @@
 # 2) Dispatch PEIM from discovered FV.
 # 3) Handoff control to DxeIpl to load DXE core and enter 

Re: [edk2] [PATCH v3 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Zeng, Star
Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chasel, 
Chiu
Sent: Thursday, February 14, 2019 6:59 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Gao, Liming 

Subject: [edk2] [PATCH v3 1/3] MdePkg: Support EFI_PEI_CORE_FV_LOCATION_PPI

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

Add EFI_PEI_CORE_FV_LOCATION_PPI definition basing on PI spec 1.7, Section 
6.3.9.
This PPI can support the secnario that PEI Foundation not in BFV.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 MdePkg/Include/Ppi/PeiCoreFvLocation.h | 48 

 MdePkg/MdePkg.dec  | 11 +--
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Ppi/PeiCoreFvLocation.h 
b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
new file mode 100644
index 00..3bea6819ec
--- /dev/null
+++ b/MdePkg/Include/Ppi/PeiCoreFvLocation.h
@@ -0,0 +1,48 @@
+/** @file
+  Header file for Pei Core FV Location PPI.
+
+  This PPI contains a pointer to the firmware volume which contains the PEI 
Foundation.
+  If the PEI Foundation does not reside in the BFV, then SEC must pass 
+ this PPI as a part  of the PPI list provided to the PEI Foundation 
+ Entry Point, otherwise the PEI Foundation  shall assume that it resides 
within the BFV.
+
+  Copyright (c) 2019, Intel Corporation. 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.
+
+  @par Revision Reference:
+  This PPI is defined in UEFI Platform Initialization Specification 1.7 Volume 
1:
+  Standards
+
+**/
+
+
+#ifndef _EFI_PEI_CORE_FV_LOCATION_H_
+#define _EFI_PEI_CORE_FV_LOCATION_H_
+
+///
+/// Global ID for EFI_PEI_CORE_FV_LOCATION_PPI /// #define 
+EFI_PEI_CORE_FV_LOCATION_GUID \
+  { \
+0x52888eae, 0x5b10, 0x47d0, {0xa8, 0x7f, 0xb8, 0x22, 0xab, 0xa0, 
+0xca, 0xf4 } \
+  }
+
+///
+/// This PPI provides location of EFI PeiCoreFv.
+///
+typedef struct {
+  ///
+  /// Pointer to the first byte of the firmware volume which contains the PEI 
Foundation.
+  ///
+  VOID*PeiCoreFvLocation;
+} EFI_PEI_CORE_FV_LOCATION_PPI;
+
+extern EFI_GUID gEfiPeiCoreFvLocationPpiGuid;
+
+#endif // _EFI_PEI_CORE_FV_LOCATION_H_
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index a485408310..c859b4a511 
100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,9 +2,9 @@
 # This Package provides all definitions, library classes and libraries 
instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of -# 
EFI1.10/UEFI2.7/PI1.6 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.7 and some Industry Standards.
 #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2019, Intel Corporation. All rights 
+reserved.
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.  # 
(C) Copyright 2016 Hewlett Packard Enterprise Development LP  # @@ -929,6 
+929,13 @@
   ## Include/Ppi/SecHobData.h
   gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4, 0xee, 0xf5, 
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }
 
+  #
+  # PPIs defined in PI 1.7.
+  #
+
+  ## Include/Ppi/PeiCoreFvLocation.h
+  gEfiPeiCoreFvLocationPpiGuid   = { 0x52888eae, 0x5b10, 0x47d0, { 0xa8, 0x7f, 
0xb8, 0x22, 0xab, 0xa0, 0xca, 0xf4 }}
+
 [Protocols]
   ## Include/Protocol/Pcd.h
   gPcdProtocolGuid   = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 
0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
--
2.13.3.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


[edk2] [PATCH v1 1/1] SecurityPkg/HddPassword: Add Security feature set support for ATA dev

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1529

This commit will add the 'Security feature set' support for ATA devices.

According to the AT Attachment 8 - ATA/ATAPI Command Set (ATA8-ACS)
specification, the Security feature set is an optional feature. In
summary, the feature is a password system that restricts access to user
data stored on an ATA device. A more detailed introduction of this feature
can be referred from the ATA8-ACS spec.

The HddPassword driver is composed of 2 parts:
* A DXE driver and
* A PEI driver

The DXE driver consumes EFI_ATA_PASS_THRU_PROTOCOL instances and installs
an HII GUI to manage the devices. If the managing device supports Security
feature set, the HII page will provide the user with the ability to
set/update/disable the password for this device. Also, if a password is
being set via the Security feature set, a popup window will show during
boot requesting the user to input password.

Another feature supported by this driver is that for those managing
devices with password set, they will be automatically unlocked during the
S3 resume. This is done by the co-work of the DXE driver and the PEI
driver:

The DXE driver will save the password and the identication information for
these devices into a LockBox, which is only allowed to restore during S3
resume.

The PEI driver, during S3 resume, will restore the content in the LockBox
and will consume EDKII_PEI_ATA_PASS_THRU_PPI instances to unlock devices.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Chao Zhang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 SecurityPkg/SecurityPkg.dsc   |6 +
 SecurityPkg/HddPassword/HddPasswordDxe.inf|   75 +
 SecurityPkg/HddPassword/HddPasswordPei.inf|   54 +
 SecurityPkg/HddPassword/HddPasswordCommon.h   |   61 +
 SecurityPkg/HddPassword/HddPasswordDxe.h  |  148 +
 SecurityPkg/HddPassword/HddPasswordHiiDataStruc.h |   63 +
 SecurityPkg/HddPassword/HddPasswordPei.h  |   64 +
 SecurityPkg/HddPassword/HddPassword.vfr   |  188 ++
 SecurityPkg/HddPassword/HddPasswordDxe.c  | 2816 
 SecurityPkg/HddPassword/HddPasswordPei.c  |  461 
 SecurityPkg/HddPassword/HddPasswordStrings.uni|   48 +
 11 files changed, 3984 insertions(+)

diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index ab887e8c4d..5577ff0687 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -287,6 +287,12 @@
   SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf
   SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.inf
 
+  #
+  # HDD Password solution
+  #
+  SecurityPkg/HddPassword/HddPasswordDxe.inf
+  SecurityPkg/HddPassword/HddPasswordPei.inf
+
 [BuildOptions]
MSFT:*_*_IA32_DLINK_FLAGS = /ALIGN:256
   INTEL:*_*_IA32_DLINK_FLAGS = /ALIGN:256
diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf 
b/SecurityPkg/HddPassword/HddPasswordDxe.inf
new file mode 100644
index 00..7a3fc2f88c
--- /dev/null
+++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
@@ -0,0 +1,75 @@
+## @file
+#  HddPasswordDxe driver which is used to set/clear hdd password at attached 
harddisk
+#  devices.
+#
+#  Copyright (c) 2019, Intel Corporation. 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= 0x00010005
+  BASE_NAME  = HddPasswordDxe
+  FILE_GUID  = 9BD549CD-86D1-4925-9F7D-3686DDD876FC
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= HddPasswordDxeInit
+
+#
+# The following information is for reference only and not required by the 
build tools.
+#
+#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
+#
+
+[Sources]
+  HddPasswordDxe.c
+  HddPasswordDxe.h
+  HddPasswordHiiDataStruc.h
+  HddPassword.vfr
+  HddPasswordStrings.uni
+  HddPasswordCommon.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  CryptoPkg/CryptoPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiHiiServicesLib
+  UefiRuntimeServicesTableLib
+  DxeServicesTableLib
+  BaseMemoryLib
+  DebugLib
+  HiiLib
+  PrintLib
+  UefiLib
+  LockBoxLib
+  S3BootScriptLib
+  PciLib
+  BaseCryptLib
+
+[Guids]
+  gEfiIfrTianoGuid  ## CONSUMES ## GUID
+  gEfiEndOfDxeEventGroupGuid## CONSUMES ## Event
+  gS3StorageDeviceInitListGuid  ## SOMETIMES_PRODUCES ## 
UNDEFINED
+

[edk2] [PATCH v1 0/1] Add Security feature set support for ATA devices

2019-02-14 Thread Hao Wu
This patch actually depends on a series that is currently under review:
https://www.mail-archive.com/edk2-devel@lists.01.org/msg51142.html

One can get this patch together with the above-mentioned series at:
https://github.com/hwu25/edk2/tree/hddpassword_v1

Uni-tests performed for the patch:
* Password can be set/update/disabled to an ATA device via the HII page;
* A popup windows will show up if a device is set with password. And after
  inputting the correct password, the data can be accessed. Otherwise,
  data cannot be accessed;
* A device with password set can be auto-unlocked during the S3 resume.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Chao Zhang 
Cc: Jiewen Yao 

Hao Wu (1):
  SecurityPkg/HddPassword: Add Security feature set support for ATA dev

 SecurityPkg/SecurityPkg.dsc   |6 +
 SecurityPkg/HddPassword/HddPasswordDxe.inf|   75 +
 SecurityPkg/HddPassword/HddPasswordPei.inf|   54 +
 SecurityPkg/HddPassword/HddPasswordCommon.h   |   61 +
 SecurityPkg/HddPassword/HddPasswordDxe.h  |  148 +
 SecurityPkg/HddPassword/HddPasswordHiiDataStruc.h |   63 +
 SecurityPkg/HddPassword/HddPasswordPei.h  |   64 +
 SecurityPkg/HddPassword/HddPassword.vfr   |  188 ++
 SecurityPkg/HddPassword/HddPasswordDxe.c  | 2816 
 SecurityPkg/HddPassword/HddPasswordPei.c  |  461 
 SecurityPkg/HddPassword/HddPasswordStrings.uni|   48 +
 11 files changed, 3984 insertions(+)
 create mode 100644 SecurityPkg/HddPassword/HddPasswordDxe.inf
 create mode 100644 SecurityPkg/HddPassword/HddPasswordPei.inf
 create mode 100644 SecurityPkg/HddPassword/HddPasswordCommon.h
 create mode 100644 SecurityPkg/HddPassword/HddPasswordDxe.h
 create mode 100644 SecurityPkg/HddPassword/HddPasswordHiiDataStruc.h
 create mode 100644 SecurityPkg/HddPassword/HddPasswordPei.h
 create mode 100644 SecurityPkg/HddPassword/HddPassword.vfr
 create mode 100644 SecurityPkg/HddPassword/HddPasswordDxe.c
 create mode 100644 SecurityPkg/HddPassword/HddPasswordPei.c
 create mode 100644 SecurityPkg/HddPassword/HddPasswordStrings.uni

-- 
2.12.0.windows.1

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


[edk2] [PATCH v1] MdeModulePkg/UfsBlockIoPei: Correct use of 'DeviceIndex' in BlkIO PPI

2019-02-14 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1474

Within UfsBlockIoPei, the current implementation of the Block IO(2)
services:

UfsBlockIoPeimGetMediaInfo
UfsBlockIoPeimReadBlocks
UfsBlockIoPeimGetMediaInfo2
UfsBlockIoPeimReadBlocks2

does not handle the input parameter 'DeviceIndex' properly.

According to both of the PI spec and the function description comments:

> DeviceIndexSpecifies the block device to which the function wants
>to talk. ... This index is a number from one to
>NumberBlockDevices.

But current codes incorrectly treat the valid range of 'DeviceIndex' as 0
to (NumberBlockDevices - 1).

This commit is to address this issue.

Cc: Jian J Wang 
Cc: Ray Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 56 +++-
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c 
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index 204e456413..c969ab45f5 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2014 - 2019, Intel Corporation. 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
@@ -587,15 +587,17 @@ UfsBlockIoPeimGetMediaInfo (
   EFI_SCSI_DISK_CAPACITY_DATA16  Capacity16;
   UINTN  DataLength;
   BOOLEANNeedRetry;
+  UINTN  Lun;
 
   Private   = GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS (This);
   NeedRetry = TRUE;
 
-  if (DeviceIndex >= UFS_PEIM_MAX_LUNS) {
+  if ((DeviceIndex == 0) || (DeviceIndex > UFS_PEIM_MAX_LUNS)) {
 return EFI_INVALID_PARAMETER;
   }
 
-  if ((Private->Luns.BitMask & (BIT0 << DeviceIndex)) == 0) {
+  Lun = DeviceIndex - 1;
+  if ((Private->Luns.BitMask & (BIT0 << Lun)) == 0) {
 return EFI_ACCESS_DENIED;
   }
 
@@ -609,7 +611,7 @@ UfsBlockIoPeimGetMediaInfo (
   do {
 Status = UfsPeimTestUnitReady (
Private,
-   DeviceIndex,
+   Lun,
,

);
@@ -621,7 +623,7 @@ UfsBlockIoPeimGetMediaInfo (
   continue;
 }
 
-Status = UfsPeimParsingSenseKeys (&(Private->Media[DeviceIndex]), 
, );
+Status = UfsPeimParsingSenseKeys (&(Private->Media[Lun]), , 
);
 if (EFI_ERROR (Status)) {
   return EFI_DEVICE_ERROR;
 }
@@ -630,7 +632,7 @@ UfsBlockIoPeimGetMediaInfo (
 
   DataLength  = sizeof (EFI_SCSI_DISK_CAPACITY_DATA);
   SenseDataLength = 0;
-  Status = UfsPeimReadCapacity (Private, DeviceIndex, , (UINT32 
*), NULL, );
+  Status = UfsPeimReadCapacity (Private, Lun, , (UINT32 
*), NULL, );
   if (EFI_ERROR (Status)) {
 return EFI_DEVICE_ERROR;
   }
@@ -639,22 +641,22 @@ UfsBlockIoPeimGetMediaInfo (
   (Capacity.LastLba1 == 0xff) && (Capacity.LastLba0 == 0xff)) {
 DataLength  = sizeof (EFI_SCSI_DISK_CAPACITY_DATA16);
 SenseDataLength = 0;
-Status = UfsPeimReadCapacity16 (Private, DeviceIndex, , (UINT32 
*), NULL, );
+Status = UfsPeimReadCapacity16 (Private, Lun, , (UINT32 
*), NULL, );
 if (EFI_ERROR (Status)) {
   return EFI_DEVICE_ERROR;
 }
-Private->Media[DeviceIndex].LastBlock  = ((UINT32)Capacity16.LastLba3 << 
24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | 
Capacity16.LastLba0;
-Private->Media[DeviceIndex].LastBlock |= LShiftU64 
((UINT64)Capacity16.LastLba7, 56) | LShiftU64((UINT64)Capacity16.LastLba6, 48) 
| LShiftU64 ((UINT64)Capacity16.LastLba5, 40) | LShiftU64 
((UINT64)Capacity16.LastLba4, 32);
-Private->Media[DeviceIndex].BlockSize  = (Capacity16.BlockSize3 << 24) | 
(Capacity16.BlockSize2 << 16) | (Capacity16.BlockSize1 << 8) | 
Capacity16.BlockSize0;
+Private->Media[Lun].LastBlock  = ((UINT32)Capacity16.LastLba3 << 24) | 
(Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | Capacity16.LastLba0;
+Private->Media[Lun].LastBlock |= LShiftU64 ((UINT64)Capacity16.LastLba7, 
56) | LShiftU64((UINT64)Capacity16.LastLba6, 48) | LShiftU64 
((UINT64)Capacity16.LastLba5, 40) | LShiftU64 ((UINT64)Capacity16.LastLba4, 32);
+Private->Media[Lun].BlockSize  = (Capacity16.BlockSize3 << 24) | 
(Capacity16.BlockSize2 << 16) | (Capacity16.BlockSize1 << 8) | 
Capacity16.BlockSize0;
   } else {
-Private->Media[DeviceIndex].LastBlock  = ((UINT32)Capacity.LastLba3 << 24) 
| (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0;
-Private->Media[DeviceIndex].BlockSize  = (Capacity.BlockSize3 << 24) | 
(Capacity.BlockSize2 << 16) | (Capacity.BlockSize1 << 8) | Capacity.BlockSize0;
+

Re: [edk2] [edk2-announce] Community Meeting Minutes

2019-02-14 Thread Rebecca Cran via edk2-devel

On 2/14/19 3:13 PM, Kinney, Michael D wrote:

You can review the groups.io features.  I think what you
want is available.  Stephano has also setup an edk2 area
in groups.io for community member to try out.

https://groups.io/static/help#features

There are a number of CI services that are integrated with
GitHub.

https://github.com/marketplace/category/continuous-integration

There is work to be done to enable one of these CI services
for edk2.  Stephano has a community task to evaluate and
select a CI service.



Thanks, I'm cautiously optimistic that groups.io will maintain a nice 
email interface to the list(s). However I don't see any messages/topics 
(in https://edk2.groups.io/g/main), and it appears my test posts are 
being moderated: are there plans to start active testing at some point?



I'm not sure I'll ever be a fan of Github but hopefully it's something 
we can move forward with - and I'll continue providing other services I 
feel are missing, from the server in my basement :)



--
Rebecca Cran

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


Re: [edk2] [PATCH v3 3/3] UefiCpuPkg/SecCore: Support EFI_PEI_CORE_FV_LOCATION_PPI

2019-02-14 Thread Zeng, Star
   //
   // Perform platform specific initialization before entering PeiCore.
   //

This comment block should be also moved together with SecPlatformMain() 
calling, right?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chasel, 
Chiu
Sent: Thursday, February 14, 2019 6:59 PM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek ; Dong, Eric 
Subject: [edk2] [PATCH v3 3/3] UefiCpuPkg/SecCore: Support 
EFI_PEI_CORE_FV_LOCATION_PPI

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

EFI_PEI_CORE_FV_LOCATION_PPI may be passed by platform when PeiCore not in BFV 
so SecCore has to search PeiCore either from the FV location provided by 
EFI_PEI_CORE_FV_LOCATION_PPI or from BFV.

Test: Verified on internal platform and booting successfully.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 UefiCpuPkg/SecCore/SecMain.c   | 35 +--
 UefiCpuPkg/SecCore/SecCore.inf |  3 ++-
 UefiCpuPkg/SecCore/SecMain.h   |  3 ++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c index 
b24e190617..d84eb675f5 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -1,7 +1,7 @@
 /** @file
   C functions in SEC
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, Intel Corporation. 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 @@ -232,22 +232,45 @@ SecStartupPhase2(
   EFI_PEI_PPI_DESCRIPTOR  *AllSecPpiList;
   EFI_PEI_CORE_ENTRY_POINTPeiCoreEntryPoint;
 
+  PeiCoreEntryPoint = NULL;
   SecCoreData   = (EFI_SEC_PEI_HAND_OFF *) Context;
   AllSecPpiList = (EFI_PEI_PPI_DESCRIPTOR *) SecCoreData->PeiTemporaryRamBase;
+
   //
   // Find Pei Core entry point. It will report SEC and Pei Core debug 
information if remote debug
   // is enabled.
   //
-  FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
-  if (PeiCoreEntryPoint == NULL)
-  {
-CpuDeadLoop ();
+  PpiList = SecPlatformMain (SecCoreData);  if (PpiList != NULL) {
+for (Index = 0;
+  (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) != 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
+  Index++) {
+  if (CompareGuid (PpiList[Index].Guid, ) && 
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation != 
0)) {
+FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)->PeiCoreFvLocation, 
);
+if (PeiCoreEntryPoint != NULL) {
+  break;
+} else {
+  //
+  // PeiCore not found
+  //
+  CpuDeadLoop ();
+}
+  }
+}
+  }
+  //
+  // If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from BFV.
+  //
+  if (PeiCoreEntryPoint == NULL) {
+FindAndReportEntryPoints ((EFI_FIRMWARE_VOLUME_HEADER *) 
SecCoreData->BootFirmwareVolumeBase, );
+if (PeiCoreEntryPoint == NULL) {
+  CpuDeadLoop ();
+}
   }
 
   //
   // Perform platform specific initialization before entering PeiCore.
   //
-  PpiList = SecPlatformMain (SecCoreData);
   if (PpiList != NULL) {
 //
 // Remove the terminal flag from the terminal PPI
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 442f663911..fc1485b5cb 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -7,7 +7,7 @@
 #  protected mode, setup flat memory model, enable temporary memory and
 #  call into SecStartup().
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. 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
@@ -73,6 +73,7 @@
   ## NOTIFY
   ## SOMETIMES_CONSUMES
   gPeiSecPerformancePpiGuid
+  gEfiPeiCoreFvLocationPpiGuid
 
 [Guids]
   ## SOMETIMES_PRODUCES   ## HOB
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index 83244af119..80045d34f4 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -1,7 +1,7 @@
 /** @file
   Master header file for SecCore.
 
-  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 2019, Intel Corporation. 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
@@ 

Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

2019-02-14 Thread Feng, Bob C
Hi Laszlo,

Thanks for you detailed clarification. 

This patch will not be pushed to edk2 master and Ubuntu user need to install 
python3-distutils package via "sudo apt-get install python3-distutils" to 
resolve this issue.

Thanks,
Bob

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, February 14, 2019 11:32 PM
To: Feng, Bob C 
Cc: Gao, Liming ; Bi, Dandan ; 
Carsey, Jaben ; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch 0/3] BaseTools: Implement splitquoted function

Hi Bob,

On 02/14/19 03:51, Feng, Bob C wrote:
> Hi Laszlo, Liming, Jaben and Dandan,
>
> I found this is a Ubuntu18 bug. Refer to
> https://bugs.launchpad.net/ubuntu/+source/fdroidserver/+bug/1762183

That Ubuntu bug is *related*, but it's not a bug that matters for us.

The bug above is about another package; it is called "fdroidserver". The 
"fdroidserver" package has a hard runtime dependency on the python 
"distutils.util" module.

Package management systems on Linux distributions track inter-package 
dependencies. That is, if the meta-data on the "fdroidserver" package 
explicitly lists the "python3-distutils" package as a dependency, then the 
package management system will automatically install in "python3-distutils" 
when the user requests "fdroidserver".

However, if the meta-data on the "fdroidserver" package are incorrect (buggy), 
then the package management system will have no clue. And, if the 
"python3-distutils" package is not already installed for some other reason, 
then the user will get an installation or runtime error, when they try to 
install or run "fdroidserver".

So, to be clear, the bug report you reference describes a *similar* situation 
to ours (a missing dependency), but it's not the same case.
The bug you refer to is in the "fdroidserver" package, and they fixed it in 
Ubuntu by updating / correcting the meta-data on the "fdroidserver"
package. The "python3-distutils" package, or other parts of the OS, were not 
touched.

> And Ubuntu fixed this bug via a Ubuntu18.04.1 update package which was 
> published on 2018-08-09. Refer to
> https://launchpad.net/ubuntu/+source/fdroidserver/1.0.9-1~18.04.1

The link you provide confirms what I wrote above. It is a changelog for the 
"fdroidserver" package, and the relevant entry says "fix missing Python 
distutils dependency".

> While the latest Ubuntu 18.04 release
> (ubuntu-18.04.1-desktop-amd64.iso)  on 
> http://releases.ubuntu.com/18.04/ was published on 2018-07-25.  So 
> there is no distutils.util library on Ubuntu18.04 default 
> installation.

No, this statement can't be correct. I'm pretty sure that the 
"python3-distutils" package *was* available when Ubuntu 18.04 -- not
18.04.1 -- was originally released.

I've just downloaded

  http://old-releases.ubuntu.com/releases/bionic/ubuntu-18.04-desktop-amd64.iso

While the package is not on the ISO, a whole lot of *other* packages are also 
not there -- for example I can't see any python at all. So I'm thinking that 
python3-distutils was only available from the network.

But, I'm pretty sure python3-distutils *was* available from the network, when 
18.04 was originally released.

What was indeed broken in the original Ubuntu 18.04 release was the meta-data 
on the "fdroidserver" package.

> But I think it's clear that distutils.util is not *3rd party* python 
> library.

I agree; and that is what matters.

> I have tried that the command "sudo apt upgrade"  can't fix this bug

This "apt" command would only be relevant if you had the *old* fdroidserver 
package installed (with the missing dependency in its meta-data). Then, "apt" 
would install the *new* fdroidserver package for you, and it would also act on 
the now-visible "python3-distutils"
dependency. Thus, "apt" would automatically install in python3-distutils, as a 
dependency.

For installing "python3-distutils" *in itself*, the above "apt" command is 
totally useless.

> while the command "sudo apt-get install python3-distutils" works.

Yes, it does, because here you are specifically requesting the 
python3-distutils package.

And that's what matters. Ubuntu users simply need to install python3-distutils 
manually, from their official package repositories, if they want to use 
BaseTools (as part of the upstream edk2 git repo).

So I think this edk2 patch set is not necessary, after all.

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


[edk2] [PATCH] BaseTools:BaseTools supports to the driver combination.

2019-02-14 Thread Fan, ZhijuX
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1520

To save the image size without the compression, more than
one drivers can be combined into single one. When more than
one drivers are combined, their depex will be AND together.
Below is the example to combine BootManagerPolicyDxe into
DriverHealthManagerDxe.

Besides this patch, BaseTools also needs to check the module
type and make sure all module type are same. Otherwise,
BaseTools will report the error.
DRIVER INF has the parameter ENTRY_POINT 
LIBRARY INF has the parameter LIBRARY_CLASS 

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/AutoGen/GenC.py | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 9700bf8527..93e8d78375 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -1455,10 +1455,24 @@ def CreateLibraryDestructorCode(Info, AutoGenC, 
AutoGenH):
 def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
 if Info.IsLibrary or Info.ModuleType in [SUP_MODULE_USER_DEFINED, 
SUP_MODULE_SEC]:
 return
+ModuleEntryPointList = []
+for Lib in Info.DependentLibraryList:
+if len(Lib.ModuleEntryPointList) > 0:
+if Lib.ModuleType == Info.ModuleType:
+ModuleEntryPointList = ModuleEntryPointList + 
Lib.ModuleEntryPointList
+else:
+EdkLogger.error(
+"build",
+CODE_ERROR,
+"%s \nDriver's ModuleType must be consistent" % Lib,
+File=str(Info)
+)
+ModuleEntryPointList = ModuleEntryPointList + 
Info.Module.ModuleEntryPointList
+
 #
 # Module Entry Points
 #
-NumEntryPoints = len(Info.Module.ModuleEntryPointList)
+NumEntryPoints = len(ModuleEntryPointList)
 if 'PI_SPECIFICATION_VERSION' in Info.Module.Specification:
 PiSpecVersion = Info.Module.Specification['PI_SPECIFICATION_VERSION']
 else:
@@ -1468,7 +1482,7 @@ def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
 else:
 UefiSpecVersion = '0x'
 Dict = {
-'Function'   :   Info.Module.ModuleEntryPointList,
+'Function'   :   ModuleEntryPointList,
 'PiSpecVersion'  :   PiSpecVersion + 'U',
 'UefiSpecVersion':   UefiSpecVersion + 'U'
 }
@@ -1481,7 +1495,7 @@ def CreateModuleEntryPointCode(Info, AutoGenC, AutoGenH):
   AUTOGEN_ERROR,
   '%s must have exactly one entry point' % Info.ModuleType,
   File=str(Info),
-  ExtraData= ", ".join(Info.Module.ModuleEntryPointList)
+  ExtraData= ", ".join(ModuleEntryPointList)
   )
 if Info.ModuleType == SUP_MODULE_PEI_CORE:
 AutoGenC.Append(gPeiCoreEntryPointString.Replace(Dict))
@@ -1535,11 +1549,18 @@ def CreateModuleEntryPointCode(Info, AutoGenC, 
AutoGenH):
 def CreateModuleUnloadImageCode(Info, AutoGenC, AutoGenH):
 if Info.IsLibrary or Info.ModuleType in [SUP_MODULE_USER_DEFINED, 
SUP_MODULE_SEC]:
 return
+
+ModuleUnloadImageList = []
+for Lib in Info.DependentLibraryList:
+if len(Lib.ModuleUnloadImageList) > 0:
+ModuleUnloadImageList = ModuleUnloadImageList + 
Lib.ModuleUnloadImageList
+ModuleUnloadImageList = ModuleUnloadImageList + 
Info.Module.ModuleUnloadImageList
+
 #
 # Unload Image Handlers
 #
-NumUnloadImage = len(Info.Module.ModuleUnloadImageList)
-Dict = {'Count':str(NumUnloadImage) + 'U', 
'Function':Info.Module.ModuleUnloadImageList}
+NumUnloadImage = len(ModuleUnloadImageList)
+Dict = {'Count':str(NumUnloadImage) + 'U', 
'Function':ModuleUnloadImageList}
 if NumUnloadImage < 2:
 AutoGenC.Append(gUefiUnloadImageString[NumUnloadImage].Replace(Dict))
 else:
-- 
2.14.1.windows.1

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


Re: [edk2] [PATCH 1/1] EmbeddedPkg/Library: Add VirtualRealTimeClockLib

2019-02-14 Thread Pete Batard

Hi Leif,

On 2019-02-12 19:14, Leif Lindholm wrote:

On Mon, Feb 04, 2019 at 12:47:36PM +, Pete Batard wrote:

This is designed to be used on platforms where a a real RTC is not
available and relies on an RtcEpochSeconds variable having been set or,
if that is not the case, falls back to using the epoch embedded at
compilation time.

Note that, in order to keep things simple for the setting of the
compilation time variable, only GCC environments with UNIX-like shells
and where a 'date' command is available are meant to be supported for
now.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard 


On the whole, this looks good to me.


Thanks for the review.


One addition we'll need, so that we can build this library standalone
is an entry in EmbeddedPkg.dsc:

diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 4d9e6399d5..dc5040e611 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -218,6 +218,7 @@ [Components.common]
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf

EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
+  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf


Well, I actually tried just that but got a build failure when trying to 
compile it (sorry can't remember exactly what was the issue and I don't 
have access to an edk2 test env at the moment) and I saw that some 
libraries in the same location, such as TimeLib which I believe is used 
by some edk2-platforms, are also not referenced at all in the dsc.


So my take was that not all of the libs are required to be included in 
the dsc, especially if they are meant to be referenced directly.


If you feel this is important, I can look into adding the .dsc ref 
again, though provided I can figure out my issue (which may very well 
boil down to me not being too familiar with compiling a standalone 
EmbeddedPkg) it might be a week or two before I can send an updated patch.


Regards,

/Pete


I don't have any strong opinions on either of Phil's suggestions, but
if you could give some feedback on those and fold the above in, this
could go in.

Regards,

Leif


---
  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 400 

  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf |  43 
+++
  2 files changed, 443 insertions(+)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
new file mode 100644
index ..4c354730d02b
--- /dev/null
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -0,0 +1,400 @@
+/** @file
+ *
+ *  Implement virtual EFI RealTimeClock runtime services.
+ *
+ *  Coypright (c) 2019, Pete Batard 
+ *  Copyright (c) 2018, Andrei Warkentin 
+ *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
+ *  Copyright (c) Microsoft Corporation. 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.
+ *
+ *  Based on 
ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+STATIC CONST CHAR16  mEpochVariableName[] = L"RtcEpochSeconds";
+STATIC CONST CHAR16  mTimeZoneVariableName[]  = L"RtcTimeZone";
+STATIC CONST CHAR16  mDaylightVariableName[]  = L"RtcDaylight";
+
+/**
+   Returns the current time and date information, and the time-keeping 
capabilities
+   of the virtual RTC.
+
+   @param  Time  A pointer to storage to receive a snapshot of 
the current time.
+   @param  Capabilities  An optional pointer to a buffer to receive 
the real time clock
+ device's capabilities.
+
+   @retval EFI_SUCCESS   The operation completed successfully.
+   @retval EFI_INVALID_PARAMETER Time is NULL.
+   @retval EFI_DEVICE_ERROR  The time could not be retrieved due to 
hardware error.
+
+**/
+EFI_STATUS
+EFIAPI
+LibGetTime (
+  OUT EFI_TIME   *Time,
+  OUT EFI_TIME_CAPABILITIES  *Capabilities
+  )
+{
+  EFI_STATUS  Status;
+  UINT32  EpochSeconds;
+  INT16   TimeZone;
+  UINT8   Daylight;
+  UINT64  Freq;
+  UINT64

Re: [edk2] [PATCH 1/1] EmbeddedPkg/Library: Add VirtualRealTimeClockLib

2019-02-14 Thread Pete Batard

Hi Philippe,

Thanks fort reviewing this patch, and apologies for the late reply.

On 2019-02-05 20:57, Philippe Mathieu-Daudé wrote:

Hi Pete,

On 2/4/19 1:47 PM, Pete Batard wrote:

This is designed to be used on platforms where a a real RTC is not
available and relies on an RtcEpochSeconds variable having been set or,
if that is not the case, falls back to using the epoch embedded at
compilation time.

Note that, in order to keep things simple for the setting of the
compilation time variable, only GCC environments with UNIX-like shells
and where a 'date' command is available are meant to be supported for
now.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard 
---
  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 400 

  EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf |  43 
+++
  2 files changed, 443 insertions(+)

diff --git 
a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
new file mode 100644
index ..4c354730d02b
--- /dev/null
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -0,0 +1,400 @@
+/** @file
+ *
+ *  Implement virtual EFI RealTimeClock runtime services.
+ *
+ *  Coypright (c) 2019, Pete Batard 
+ *  Copyright (c) 2018, Andrei Warkentin 
+ *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ *  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
+ *  Copyright (c) Microsoft Corporation. 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.
+ *
+ *  Based on 
ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+STATIC CONST CHAR16  mEpochVariableName[] = L"RtcEpochSeconds";
+STATIC CONST CHAR16  mTimeZoneVariableName[]  = L"RtcTimeZone";
+STATIC CONST CHAR16  mDaylightVariableName[]  = L"RtcDaylight";
+
+/**
+   Returns the current time and date information, and the time-keeping 
capabilities
+   of the virtual RTC.
+
+   @param  Time  A pointer to storage to receive a snapshot of 
the current time.
+   @param  Capabilities  An optional pointer to a buffer to receive 
the real time clock
+ device's capabilities.
+
+   @retval EFI_SUCCESS   The operation completed successfully.
+   @retval EFI_INVALID_PARAMETER Time is NULL.
+   @retval EFI_DEVICE_ERROR  The time could not be retrieved due to 
hardware error.
+
+**/
+EFI_STATUS
+EFIAPI
+LibGetTime (
+  OUT EFI_TIME   *Time,
+  OUT EFI_TIME_CAPABILITIES  *Capabilities
+  )
+{
+  EFI_STATUS  Status;
+  UINT32  EpochSeconds;
+  INT16   TimeZone;
+  UINT8   Daylight;
+  UINT64  Freq;
+  UINT64  Counter;
+  UINT64  Remainder;
+  UINTN   ElapsedSeconds;
+  UINTN   Size;
+
+  if (Time == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  // Get the counter frequency
+  Freq = GetPerformanceCounterProperties (NULL, NULL);
+  if (Freq == 0) {
+return EFI_DEVICE_ERROR;
+  }
+
+  // Get the epoch time from non-volatile storage
+  Size = sizeof (UINTN);
+  ElapsedSeconds = 0;
+  Status = EfiGetVariable (
+ (CHAR16 *)mEpochVariableName,
+ ,
+ NULL,
+ ,
+ (VOID *)
+ );
+  // Fall back to compilation-time epoch if not set
+  if (EFI_ERROR (Status)) {
+ASSERT(Status != EFI_INVALID_PARAMETER);
+ASSERT(Status != EFI_BUFFER_TOO_SMALL);
+//
+// The following is intended to produce a compilation error on build
+// environments where BUILD_EPOCH can not be set from inline shell.
+// If you are attempting to use this library on such an environment, please
+// contact the edk2 mailing list, so we can try to add support for it.
+//


What about:

#ifndef BUILD_EPOCH
#define BUILD_EPOCH 1549396000 /* As of this commit */
#endif


Well, the plan is to see what we can actually do for platforms that 
can't define BUILD_EPOCH to the actual compilation time through shell 
invocation.


Rather than assume that we won't be able to do anything for those, and 
therefore default to an epoch that would have a very large offset even 
when the user went the trouble of recompiling the firmware, I'd rather 
we get a better idea of the environments this might apply to, and see if 
there's anything that could be achieved there.


Of course, if there are other people here that support your