[edk2] [PATCH 0/2] Delete TCP, PXE, iSCSI driver in MdeModulePkg

2018-12-18 Thread Fu Siyuan
This patch series is to delete the Tcp4Dxe, UefiPxeBcDxe and IScsi4Dxe
drivers in MdeModulePkg. These drivers will not be maintained and can't
co-work with the dual-stack drivers in NetworkPkg.

In future, people should use below NetworkPkg drivers instead:
  NetworkPkg/IScsiDxe/IScsiDxe.inf
  NetworkPkg/TcpDxe/TcpDxe.inf
  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
These drivers are actively maintained with more bug fixes and new feature
support.

All edk2 platforms DSC/FDF have already been updated to use the NetworkPkg
drivers in privious patch.

Bugzilla link: https://bugzilla.tianocore.org/show_bug.cgi?id=1278

Fu Siyuan (2):
  MdeModulePkg: Delete the TCP/PXE/ISCSI drivers in MdeModulePkg.
  NetworkPkg: Remove some clarification from TCP/PXE/ISCSI driver INF.

 .../Network/IScsiDxe/ComponentName.c  |  283 --
 .../Universal/Network/IScsiDxe/IScsiCHAP.c|  430 ---
 .../Universal/Network/IScsiDxe/IScsiConfig.c  | 1264 ---
 .../Universal/Network/IScsiDxe/IScsiDhcp.c|  472 ---
 .../Universal/Network/IScsiDxe/IScsiDriver.c  |  676 
 .../Network/IScsiDxe/IScsiExtScsiPassThru.c   |  412 ---
 .../Universal/Network/IScsiDxe/IScsiIbft.c|  539 ---
 .../Network/IScsiDxe/IScsiInitiatorName.c |  116 -
 .../Universal/Network/IScsiDxe/IScsiMisc.c|  948 --
 .../Universal/Network/IScsiDxe/IScsiProto.c   | 2799 ---
 .../Universal/Network/IScsiDxe/IScsiTcp4Io.c  |  487 ---
 MdeModulePkg/Universal/Network/IScsiDxe/Md5.c |  350 --
 .../Universal/Network/Tcp4Dxe/ComponentName.c |  433 ---
 .../Universal/Network/Tcp4Dxe/SockImpl.c  | 1201 ---
 .../Universal/Network/Tcp4Dxe/SockInterface.c |  990 --
 .../Network/Tcp4Dxe/Tcp4Dispatcher.c  |  717 
 .../Universal/Network/Tcp4Dxe/Tcp4Driver.c|  782 -
 .../Universal/Network/Tcp4Dxe/Tcp4Input.c | 1497 -
 .../Universal/Network/Tcp4Dxe/Tcp4Io.c|  112 -
 .../Universal/Network/Tcp4Dxe/Tcp4Main.c  |  674 
 .../Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  940 --
 .../Universal/Network/Tcp4Dxe/Tcp4Option.c|  352 --
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c| 1238 ---
 .../Universal/Network/Tcp4Dxe/Tcp4Timer.c |  584 
 .../Network/UefiPxeBcDxe/ComponentName.c  |  365 --
 .../Network/UefiPxeBcDxe/PxeBcDhcp.c  | 1999 ---
 .../Network/UefiPxeBcDxe/PxeBcDriver.c|  665 
 .../Network/UefiPxeBcDxe/PxeBcImpl.c  | 2989 -
 .../Network/UefiPxeBcDxe/PxeBcMtftp.c |  454 ---
 .../Network/UefiPxeBcDxe/PxeBcSupport.c   |  221 --
 MdeModulePkg/MdeModulePkg.dsc |3 -
 .../Network/IScsiDxe/ComponentName.h  |  165 -
 .../Universal/Network/IScsiDxe/IScsi4Dxe.uni  |   25 -
 .../Network/IScsiDxe/IScsi4DxeExtra.uni   |   20 -
 .../Universal/Network/IScsiDxe/IScsiCHAP.h|  106 -
 .../Universal/Network/IScsiDxe/IScsiCommon.h  |   22 -
 .../Universal/Network/IScsiDxe/IScsiConfig.h  |  166 -
 .../Network/IScsiDxe/IScsiConfigDxe.vfr   |  219 --
 .../IScsiDxe/IScsiConfigDxeStrings.uni|   62 -
 .../Network/IScsiDxe/IScsiConfigNVDataStruc.h |  109 -
 .../Universal/Network/IScsiDxe/IScsiDhcp.h|   55 -
 .../Universal/Network/IScsiDxe/IScsiDriver.h  |  140 -
 .../Universal/Network/IScsiDxe/IScsiDxe.inf   |  134 -
 .../Network/IScsiDxe/IScsiExtScsiPassThru.h   |   22 -
 .../Universal/Network/IScsiDxe/IScsiIbft.h|   38 -
 .../Universal/Network/IScsiDxe/IScsiImpl.h|  168 -
 .../Network/IScsiDxe/IScsiInitiatorName.h |   74 -
 .../Universal/Network/IScsiDxe/IScsiMisc.h|  317 --
 .../Universal/Network/IScsiDxe/IScsiProto.h   | 1005 --
 .../Universal/Network/IScsiDxe/IScsiTcp4Io.h  |  142 -
 MdeModulePkg/Universal/Network/IScsiDxe/Md5.h |   80 -
 .../Universal/Network/Tcp4Dxe/SockImpl.h  |  131 -
 .../Universal/Network/Tcp4Dxe/Socket.h|  954 --
 .../Universal/Network/Tcp4Dxe/Tcp4Driver.h|  342 --
 .../Universal/Network/Tcp4Dxe/Tcp4Dxe.inf |   94 -
 .../Universal/Network/Tcp4Dxe/Tcp4Dxe.uni |   23 -
 .../Network/Tcp4Dxe/Tcp4DxeExtra.uni  |   20 -
 .../Universal/Network/Tcp4Dxe/Tcp4Func.h  |  781 -
 .../Universal/Network/Tcp4Dxe/Tcp4Main.h  |  494 ---
 .../Universal/Network/Tcp4Dxe/Tcp4Option.h|  130 -
 .../Universal/Network/Tcp4Dxe/Tcp4Proto.h |  357 --
 .../Network/UefiPxeBcDxe/PxeBcDhcp.h  |  502 ---
 .../Network/UefiPxeBcDxe/PxeBcDriver.h|  102 -
 .../Network/UefiPxeBcDxe/PxeBcImpl.h  |  189 --
 .../Network/UefiPxeBcDxe/PxeBcMtftp.h |  137 -
 .../Network/UefiPxeBcDxe/PxeBcSupport.h   |  134 -
 .../Network/UefiPxeBcDxe/UefiPxe4BcDxe.uni|   25 -
 .../UefiPxeBcDxe/UefiPxe4BcDxeExtra.uni   |   20 -
 .../Network/UefiPxeBcDxe/UefiPxeBcDxe.inf |  102 -
 NetworkPkg/IScsiDxe/IScsiDxe.inf  |   10 -
 NetworkPkg/TcpDxe/TcpDxe.inf  |6 -
 NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf  |6 -
 72 files changed, 32620 deletions(-)
 delete 

[edk2] [PATCH 2/2] NetworkPkg: Remove some clarification from TCP/PXE/ISCSI driver INF.

2018-12-18 Thread Fu Siyuan
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1278

This patch is to remove the clarification about usage/difference between
those drivers in MdeModulePkg and NetworkPkg, since the MdeModulePkg ones
have been deleted now.

Cc: Jiaxin Wu 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/IScsiDxe/IScsiDxe.inf | 10 --
 NetworkPkg/TcpDxe/TcpDxe.inf |  6 --
 NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf |  6 --
 3 files changed, 22 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
index 195dc191250f..bdf1313aa957 100644
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
@@ -4,16 +4,6 @@
 #  The iSCSI driver provides iSCSI service in the preboot environment and 
supports
 #  booting over iSCSI. This driver supports both IPv4 and IPv6 network stack.
 #
-#  Notes:
-#  1) This driver can't co-work with the IScsiDxe driver in MdeModulePkg.
-#  2) This driver includes more bug fixes and supports more features (e.g. 
IPv6, Dns
-# support for target URL configuration, iSCSI keyword support) than the 
IscsiDxe
-# driver in MdeModulePkg. So, we recommend using this driver even though 
both of
-# them can be used.
-#  3) This driver depends on OpenSSL. To use this driver, please follow the
-# instructions found in the file "OpenSSL-HOWTO.txt" located in
-# CryptoPkg\Library\OpensslLib to enable the OpenSSL building first.
-#
 # Copyright (c) 2004 - 2018, 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
diff --git a/NetworkPkg/TcpDxe/TcpDxe.inf b/NetworkPkg/TcpDxe/TcpDxe.inf
index 9433fb875cba..c4e3de7ec5ce 100644
--- a/NetworkPkg/TcpDxe/TcpDxe.inf
+++ b/NetworkPkg/TcpDxe/TcpDxe.inf
@@ -5,12 +5,6 @@
 #  It might provide TCPv4 Protocol or TCPv6 Protocol or both of them that 
depends on which network
 #  stack has been loaded in system. This driver supports both IPv4 and IPv6 
network stack.
 #
-#  Notes:
-#  1) This driver can't co-work with the Tcp4Dxe driver in MdeModulePkg.
-#  2) This driver includes more bug fixes and supports more features (e.g. 
IPv6, TCP Cancel
-# function) than the Tcp4Dxe driver in MdeModulePkg. So, we recommend 
using this driver
-# even though both of them can be used.
-#
 #  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
diff --git a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf 
b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
index 130a5456e2c1..63430711e71b 100644
--- a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
+++ b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
@@ -5,12 +5,6 @@
 #  PXE-compatible device for network access or booting. This driver supports
 #  both IPv4 and IPv6 network stack.
 #
-#  Notes:
-#  1) This driver can't co-work with the UefiPxeBcDxe driver in MdeModulePkg.
-#  2) This driver includes more bug fixes and supports more features (e.g. 
IPv6,
-# MTFTP windowsize) than the UefiPxeBcDxe driver in MdeModulePkg. So, we
-# recommend using this driver even though both of them can be used.
-#
 #  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
-- 
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 V2 3/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxFvSupported

2018-12-18 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 


-Original Message-
From: Zeng, Star 
Sent: Tuesday, December 18, 2018 2:08 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Wang, Jian J ; Wu, 
Hao A ; Gao, Liming ; Ni, Ruiyu 
; Kinney, Michael D ; Desimone, 
Nathaniel L ; Chiu, Chasel 

Subject: [PATCH V2 3/7] MdeModulePkg PeiCore: Remove the using of 
PcdPeiCoreMaxFvSupported

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

Background as below.

Problem:
As static configuration from the PCDs, the binary PeiCore (for example in FSP 
binary with dispatch mode) could not predict how many FVs, Files or PPIs for 
different platforms.

Burden:
Platform developers need configure the PCDs accordingly for different platforms.

To solve the problem and remove the burden, we can update PeiCore to remove the 
using of PcdPeiCoreMaxFvSupported, PcdPeiCoreMaxPeimPerFv and 
PcdPeiCoreMaxPpiSupported by extending buffer dynamically for FV, File and PPI 
management.

This patch removes the using of PcdPeiCoreMaxFvSupported in PeiCore.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Nate DeSimone 
Cc: Chasel Chiu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Chasel Chiu 
Reviewed-by: Jian J Wang 
---
 MdeModulePkg/Core/Pei/FwVol/FwVol.c | 67 ++---
 MdeModulePkg/Core/Pei/PeiMain.h | 15 +++-
 MdeModulePkg/Core/Pei/PeiMain.inf   |  1 -
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 16 
 4 files changed, 75 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c 
b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
index 5629c9a1ce20..0a67b96bf1e3 100644
--- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c
@@ -503,6 +503,10 @@ PeiInitializeFv (
 );
   ASSERT_EFI_ERROR (Status);
 
+  PrivateData->Fv = AllocateZeroPool (sizeof (PEI_CORE_FV_HANDLE) * 
+ FV_GROWTH_STEP);  ASSERT (PrivateData->Fv != NULL);  
+ PrivateData->MaxFvCount = FV_GROWTH_STEP;
+
   //
   // Update internal PEI_CORE_FV array.
   //
@@ -560,6 +564,7 @@ FirmwareVolmeInfoPpiNotifyCallback (
   VOID  *DepexData;
   BOOLEAN   IsFvInfo2;
   UINTN CurFvCount;
+  VOID  *TempPtr;
 
   Status   = EFI_SUCCESS;
   PrivateData  = PEI_CORE_INSTANCE_FROM_PS_THIS (PeiServices); @@ -626,10 
+631,21 @@ FirmwareVolmeInfoPpiNotifyCallback (
   }
 }
 
-if (PrivateData->FvCount >= PcdGet32 (PcdPeiCoreMaxFvSupported)) {
-  DEBUG ((EFI_D_ERROR, "The number of Fv Images (%d) exceed the max 
supported FVs (%d) in Pei", PrivateData->FvCount + 1, PcdGet32 
(PcdPeiCoreMaxFvSupported)));
-  DEBUG ((EFI_D_ERROR, "PcdPeiCoreMaxFvSupported value need be 
reconfigurated in DSC"));
-  ASSERT (FALSE);
+if (PrivateData->FvCount >= PrivateData->MaxFvCount) {
+  //
+  // Run out of room, grow the buffer.
+  //
+  TempPtr = AllocateZeroPool (
+  sizeof (PEI_CORE_FV_HANDLE) * (PrivateData->MaxFvCount + 
FV_GROWTH_STEP)
+  );
+  ASSERT (TempPtr != NULL);
+  CopyMem (
+TempPtr,
+PrivateData->Fv,
+sizeof (PEI_CORE_FV_HANDLE) * PrivateData->MaxFvCount
+);
+  PrivateData->Fv = TempPtr;
+  PrivateData->MaxFvCount = PrivateData->MaxFvCount + 
+ FV_GROWTH_STEP;
 }
 
 //
@@ -2157,7 +2173,6 @@ FindNextCoreFvHandle (
 }
   }
 
-  ASSERT (Private->FvCount <= PcdGet32 (PcdPeiCoreMaxFvSupported));
   if (Instance >= Private->FvCount) {
 return NULL;
   }
@@ -2205,7 +2220,7 @@ PeiReinitializeFv (
   //
   // Fixup all FvPpi pointers for the implementation in flash to permanent 
memory.
   //
-  for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
+  for (Index = 0; Index < PrivateData->FvCount; Index ++) {
 if (PrivateData->Fv[Index].FvPpi == OldFfsFvPpi) {
   PrivateData->Fv[Index].FvPpi = 
 }
@@ -2233,7 +2248,7 @@ PeiReinitializeFv (
   //
   // Fixup all FvPpi pointers for the implementation in flash to permanent 
memory.
   //
-  for (Index = 0; Index < PcdGet32 (PcdPeiCoreMaxFvSupported); Index ++) {
+  for (Index = 0; Index < PrivateData->FvCount; Index ++) {
 if (PrivateData->Fv[Index].FvPpi == OldFfsFvPpi) {
   PrivateData->Fv[Index].FvPpi = 
 }
@@ -2263,9 +2278,23 @@ AddUnknownFormatFvInfo (
   )
 {
   PEI_CORE_UNKNOW_FORMAT_FV_INFO*NewUnknownFv;
+  VOID  *TempPtr;
 
-  if (PrivateData->UnknownFvInfoCount + 1 >= PcdGet32 
(PcdPeiCoreMaxFvSupported)) {
-return EFI_OUT_OF_RESOURCES;
+  if (PrivateData->UnknownFvInfoCount >= PrivateData->MaxUnknownFvInfoCount) {
+//
+// Run out of room, grow the buffer.
+//
+TempPtr = AllocateZeroPool (
+sizeof (PEI_CORE_UNKNOW_FORMAT_FV_INFO) * 
(PrivateData->MaxUnknownFvInfoCount + FV_GROWTH_STEP)
+ 

Re: [edk2] [PATCH V2 4/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPpiSupported

2018-12-18 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 


-Original Message-
From: Zeng, Star 
Sent: Tuesday, December 18, 2018 2:08 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Wang, Jian J ; Wu, 
Hao A ; Gao, Liming ; Ni, Ruiyu 
; Kinney, Michael D ; Desimone, 
Nathaniel L ; Chiu, Chasel 

Subject: [PATCH V2 4/7] MdeModulePkg PeiCore: Remove the using of 
PcdPeiCoreMaxPpiSupported

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

Background as below.

Problem:
As static configuration from the PCDs, the binary PeiCore (for example in FSP 
binary with dispatch mode) could not predict how many FVs, Files or PPIs for 
different platforms.

Burden:
Platform developers need configure the PCDs accordingly for different platforms.

To solve the problem and remove the burden, we can update code to remove the 
using of PcdPeiCoreMaxFvSupported, PcdPeiCoreMaxPeimPerFv and 
PcdPeiCoreMaxPpiSupported by extending buffer dynamically for FV, File and PPI 
management.

This patch removes the using of PcdPeiCoreMaxPpiSupported in PeiCore.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Nate DeSimone 
Cc: Chasel Chiu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Chasel Chiu 
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c |   8 +-
 MdeModulePkg/Core/Pei/PeiMain.h   |  59 +++--
 MdeModulePkg/Core/Pei/PeiMain.inf |   1 -
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |  30 ++-
 MdeModulePkg/Core/Pei/Ppi/Ppi.c   | 355 ++
 5 files changed, 254 insertions(+), 199 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 9692e2f6bf51..68670f43e0b3 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1036,7 +1036,7 @@ PeiDispatcher (
   // Process the Notify list and dispatch any notifies for
   // newly installed PPIs.
   //
-  ProcessNotifyList (Private);
+  ProcessDispatchNotifyList (Private);
 }
   }
 }
@@ -1183,10 +1183,10 @@ PeiDispatcher (
 // Process the Notify list and dispatch any notifies for
 // newly installed PPIs.
 //
-ProcessNotifyList (Private);
+ProcessDispatchNotifyList (Private);
 
 //
-// Recheck SwitchStackSignal after ProcessNotifyList()
+// Recheck SwitchStackSignal after 
+ ProcessDispatchNotifyList()
 // in case PeiInstallPeiMemory() is done in a callback with
 // EFI_PEI_PPI_DESCRIPTOR_NOTIFY_DISPATCH.
 //
@@ -1227,7 +1227,7 @@ PeiDispatcher (
   // Process the Notify list and dispatch any notifies for
   // newly installed PPIs.
   //
-  ProcessNotifyList (Private);
+  ProcessDispatchNotifyList (Private);
 }
   }
 }
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h 
index b103215d81f7..322e7cd84524 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -66,37 +66,60 @@ typedef union {
 } PEI_PPI_LIST_POINTERS;
 
 ///
-/// PPI database structure which contains two link: PpiList and NotifyList. 
PpiList -/// is in head of PpiListPtrs array and notify is in end of 
PpiListPtrs.
+/// Number of PEI_PPI_LIST_POINTERS to grow by each time we run out of 
+room
 ///
+#define PPI_GROWTH_STEP 64
+#define CALLBACK_NOTIFY_GROWTH_STEP 32
+#define DISPATCH_NOTIFY_GROWTH_STEP 8
+
 typedef struct {
+  UINTN CurrentCount;
+  UINTN MaxCount;
+  UINTN LastDispatchedCount;
   ///
-  /// index of end of PpiList link list.
+  /// MaxCount number of entries.
   ///
-  INTNPpiListEnd;
+  PEI_PPI_LIST_POINTERS *PpiPtrs;
+} PEI_PPI_LIST;
+
+typedef struct {
+  UINTN CurrentCount;
+  UINTN MaxCount;
   ///
-  /// index of end of notify link list.
+  /// MaxCount number of entries.
   ///
-  INTNNotifyListEnd;
+  PEI_PPI_LIST_POINTERS *NotifyPtrs;
+} PEI_CALLBACK_NOTIFY_LIST;
+
+typedef struct {
+  UINTN CurrentCount;
+  UINTN MaxCount;
+  UINTN LastDispatchedCount;
   ///
-  /// index of the dispatched notify list.
+  /// MaxCount number of entries.
   ///
-  INTNDispatchListEnd;
+  PEI_PPI_LIST_POINTERS *NotifyPtrs;
+} PEI_DISPATCH_NOTIFY_LIST;
+
+///
+/// PPI database structure which contains three links:
+/// PpiList, CallbackNotifyList and DispatchNotifyList.
+///
+typedef struct {
   ///
-  /// index of last installed Ppi description in PpiList link list.
+  /// PPI List.
   ///
-  INTNLastDispatchedInstall;
+  PEI_PPI_LIST  PpiList;
   ///
-  /// index of last dispatched notify in Notify link 

Re: [edk2] [PATCH V2 7/7] MdeModulePkg: Remove PcdPeiCoreMaxXXX PCDs

2018-12-18 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 


-Original Message-
From: Zeng, Star 
Sent: Tuesday, December 18, 2018 2:08 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Wang, Jian J ; Wu, 
Hao A ; Gao, Liming ; Ni, Ruiyu 
; Kinney, Michael D ; Desimone, 
Nathaniel L ; Chiu, Chasel 

Subject: [PATCH V2 7/7] MdeModulePkg: Remove PcdPeiCoreMaxXXX PCDs

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

The codes have been updated to not use PcdPeiCoreMaxFvSupported, 
PcdPeiCoreMaxPeimPerFv and PcdPeiCoreMaxPpiSupported.

The patch removes them in MdeModulePkg.dec.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Nate DeSimone 
Cc: Chasel Chiu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Chasel Chiu 
---
 MdeModulePkg/MdeModulePkg.dec | 13 -  
MdeModulePkg/MdeModulePkg.uni | 12 
 2 files changed, 25 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index 0abacc1a901f..5585ce603596 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1053,23 +1053,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt VPD base address.
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0|UINT32|0x00010010
 
-  ## Maximum number of FV is supported by PeiCore's dispatching.
-  # @Prompt Maximum number of FV supported by PeiCore.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6|UINT32|0x00010030
-
-  ## Maximum File count in every FV is supported by PeiCore's dispatching.
-  #  PeiCore supported File type includes PEIM, Combined PEIM and FV.
-  # @Prompt Maximum File count per FV supported by PeiCore.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32|UINT32|0x00010031
-
   ## Maximum stack size for PeiCore.
   # @Prompt Maximum stack size for PeiCore.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize|0x2|UINT32|0x00010032
 
-  ## Maximum PPI count is supported by PeiCore's PPI database.
-  # @Prompt Maximum PPI count supported by PeiCore.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|64|UINT32|0x00010033
-
   ## The maximum size of a single non-HwErr type variable.
   # @Prompt Maximum variable size.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400|UINT32|0x3003
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni 
index 038e8485054b..fbea62dac8da 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -69,22 +69,10 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdVpdBaseAddress_HELP  #language 
en-US "VPD type PCD allows a developer to point to an absolute physical address 
PCDVPDBASEADDRESS to store PCD value."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxFvSupported_PROMPT  
#language en-US "Maximum number of FV supported by PeiCore"
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxFvSupported_HELP  
#language en-US "Maximum number of FV is supported by PeiCore's dispatching."
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPeimPerFv_PROMPT  
#language en-US "Maximum File count per FV supported by PeiCore"
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPeimPerFv_HELP  
#language en-US "Maximum File count in every FV is supported by PeiCore's 
dispatching. PeiCore supported File type includes PEIM, Combined PEIM and FV."
-
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPeiStackSize_PROMPT  
#language en-US "Maximum stack size for PeiCore"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPeiStackSize_HELP  
#language en-US "Maximum stack size for PeiCore."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPpiSupported_PROMPT  
#language en-US "Maximum PPI count supported by PeiCore"
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPeiCoreMaxPpiSupported_HELP  
#language en-US "Maximum PPI count is supported by PeiCore's PPI database."
-
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVariableSize_PROMPT  
#language en-US "Maximum variable size"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVariableSize_HELP  #language 
en-US "The maximum size of a single non-HwErr type variable."
--
2.7.0.windows.1

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


Re: [edk2] [PATCH V2 1/7] MdeModulePkg PeiCore: Remove the using of PcdPeiCoreMaxPeimPerFv

2018-12-18 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 


-Original Message-
From: Zeng, Star 
Sent: Tuesday, December 18, 2018 2:08 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Wang, Jian J ; Wu, 
Hao A ; Gao, Liming ; Ni, Ruiyu 
; Kinney, Michael D ; Desimone, 
Nathaniel L ; Chiu, Chasel 

Subject: [PATCH V2 1/7] MdeModulePkg PeiCore: Remove the using of 
PcdPeiCoreMaxPeimPerFv

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

Background as below.

Problem:
As static configuration from the PCDs, the binary PeiCore (for example in FSP 
binary with dispatch mode) could not predict how many FVs, Files or PPIs for 
different platforms.

Burden:
Platform developers need configure the PCDs accordingly for different platforms.

To solve the problem and remove the burden, we can update code to remove the 
using of PcdPeiCoreMaxFvSupported, PcdPeiCoreMaxPeimPerFv and 
PcdPeiCoreMaxPpiSupported by extending buffer dynamically for FV, File and PPI 
management.

This patch removes the using of PcdPeiCoreMaxPeimPerFv in PeiCore.

Cc: Jian J Wang 
Cc: Hao Wu 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Nate DeSimone 
Cc: Chasel Chiu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 173 +++---
 MdeModulePkg/Core/Pei/PeiMain.h   |  22 ++--
 MdeModulePkg/Core/Pei/PeiMain.inf |   1 -
 MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |  48 ---
 4 files changed, 136 insertions(+), 108 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index f6bb35a5fe8d..9692e2f6bf51 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -41,7 +41,7 @@ DiscoverPeimsAndOrderWithApriori (
   UINTN   PeimCount;
   EFI_GUID*Guid;
   EFI_PEI_FILE_HANDLE *TempFileHandles;
-  EFI_GUID*FileGuid;
+  EFI_GUID*TempFileGuid;
   EFI_PEI_FIRMWARE_VOLUME_PPI *FvPpi;
   EFI_FV_FILE_INFOFileInfo;
 
@@ -51,38 +51,81 @@ DiscoverPeimsAndOrderWithApriori (
   // Walk the FV and find all the PEIMs and the Apriori file.
   //
   AprioriFileHandle = NULL;
-  Private->CurrentFvFileHandles[0] = NULL;
+  Private->CurrentFvFileHandles = NULL;
   Guid = NULL;
-  FileHandle = NULL;
-  TempFileHandles = Private->FileHandles;
-  FileGuid= Private->FileGuid;
 
   //
-  // If the current Fv has been scanned, directly get its cachable record.
+  // If the current Fv has been scanned, directly get its cached records.
   //
-  if (Private->Fv[Private->CurrentPeimFvCount].ScanFv) {
-CopyMem (Private->CurrentFvFileHandles, 
Private->Fv[Private->CurrentPeimFvCount].FvFileHandles, sizeof 
(EFI_PEI_FILE_HANDLE) * PcdGet32 (PcdPeiCoreMaxPeimPerFv));
+  if (CoreFileHandle->ScanFv) {
+Private->CurrentFvFileHandles = CoreFileHandle->FvFileHandles;
 return;
   }
 
+  TempFileHandles = Private->TempFileHandles;
+  TempFileGuid= Private->TempFileGuid;
+
   //
-  // Go ahead to scan this Fv, and cache FileHandles within it.
+  // Go ahead to scan this Fv, get PeimCount and cache FileHandles within it 
to TempFileHandles.
   //
-  Status = EFI_NOT_FOUND;
-  for (PeimCount = 0; PeimCount <= PcdGet32 (PcdPeiCoreMaxPeimPerFv); 
PeimCount++) {
+  PeimCount = 0;
+  FileHandle = NULL;
+  do {
 Status = FvPpi->FindFileByType (FvPpi, 
PEI_CORE_INTERNAL_FFS_FILE_DISPATCH_TYPE, CoreFileHandle->FvHandle, 
);
-if (Status != EFI_SUCCESS || PeimCount == PcdGet32 
(PcdPeiCoreMaxPeimPerFv)) {
-  break;
+if (!EFI_ERROR (Status)) {
+  if (PeimCount >= Private->TempPeimCount) {
+//
+// Run out of room, grow the buffer.
+//
+TempFileHandles = AllocatePool (
+sizeof (EFI_PEI_FILE_HANDLE) * 
(Private->TempPeimCount + TEMP_FILE_GROWTH_STEP));
+ASSERT (TempFileHandles != NULL);
+CopyMem (
+  TempFileHandles,
+  Private->TempFileHandles,
+  sizeof (EFI_PEI_FILE_HANDLE) * Private->TempPeimCount
+  );
+Private->TempFileHandles = TempFileHandles;
+TempFileGuid = AllocatePool (
+ sizeof (EFI_GUID) * (Private->TempPeimCount + 
TEMP_FILE_GROWTH_STEP));
+ASSERT (TempFileGuid != NULL);
+CopyMem (
+  TempFileGuid,
+  Private->TempFileGuid,
+  sizeof (EFI_GUID) * Private->TempPeimCount
+  );
+Private->TempFileGuid = TempFileGuid;
+Private->TempPeimCount = Private->TempPeimCount + 
TEMP_FILE_GROWTH_STEP;
+  }
+
+  TempFileHandles[PeimCount++] = FileHandle;
 }
+  } while (!EFI_ERROR (Status));
+
+  DEBUG ((
+DEBUG_INFO,
+"%a(): Found 0x%x PEI FFS files in the %dth FV\n",
+__FUNCTION__,
+PeimCount,
+Private->CurrentPeimFvCount
+

Re: [edk2] [PATCH] Upgrade OpenSSL to 1.1.0j

2018-12-18 Thread Wei, Gang
Reviewed-by: Gang Wei 

> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, December 19, 2018 11:03 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Wei, Gang 
> Subject: [PATCH] Upgrade OpenSSL to 1.1.0j
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1393
> 
> BZ#1089 (https://bugzilla.tianocore.org/show_bug.cgi?id=1089) requests
> to upgrade the OpenSSL to the latest 1.1.1 release. Since OpenSSL-1.1.1
> has many changes, more porting efforts and feature evaluation are needed.
> This might lead to a situation that it cannot catch the Q1'19 stable tag.
> 
> One of the solution is upgrade current version (1.1.0h) to 1.1.0j.
> According to following web page in openssl.org, all security issues
> solved in 1.1.1 have been also back-ported to 1.1.0.j. This can make
> sure that no security vulnerabilities left in edk2 master before 1.1.1.
> 
> https://www.openssl.org/news/vulnerabilities-1.1.1.html
> 
> Cc: Ting Ye 
> Cc: Gang Wei 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  CryptoPkg/CryptoPkg.dsc   |  1 +
>  .../Library/Include/openssl/opensslconf.h | 20 ---
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf   |  3 +++
>  .../Library/OpensslLib/OpensslLibCrypto.inf   |  3 +++
>  CryptoPkg/Library/OpensslLib/openssl  |  2 +-
>  CryptoPkg/Library/OpensslLib/process_files.pl |  0
>  6 files changed, 21 insertions(+), 8 deletions(-)
>  mode change 100644 => 100755
> CryptoPkg/Library/OpensslLib/process_files.pl
> 
> diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
> index a0334d628b..321abe4d4c 100644
> --- a/CryptoPkg/CryptoPkg.dsc
> +++ b/CryptoPkg/CryptoPkg.dsc
> @@ -121,6 +121,7 @@
>CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
>CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>CryptoPkg/Library/TlsLib/TlsLib.inf
> +  CryptoPkg/Library/OpensslLib/OpensslLib.inf
> 
>  [Components.IA32, Components.X64]
>CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h
> b/CryptoPkg/Library/Include/openssl/opensslconf.h
> index 1917d7ab24..28dd9ab93c 100644
> --- a/CryptoPkg/Library/Include/openssl/opensslconf.h
> +++ b/CryptoPkg/Library/Include/openssl/opensslconf.h
> @@ -2,7 +2,7 @@
>   * WARNING: do not edit!
>   * Generated from include/openssl/opensslconf.h.in
>   *
> - * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
> + * Copyright 2016-2018 The OpenSSL Project Authors. All Rights Reserved.
>   *
>   * Licensed under the OpenSSL license (the "License").  You may not use
>   * this file except in compliance with the License.  You can obtain a copy
> @@ -235,12 +235,18 @@ extern "C" {
>   * still won't see them if the library has been built to disable deprecated
>   * functions.
>   */
> -#if defined(OPENSSL_NO_DEPRECATED)
> -# define DECLARE_DEPRECATED(f)
> -#elif __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
> -# define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
> -#else
> -# define DECLARE_DEPRECATED(f)   f;
> +#ifndef DECLARE_DEPRECATED
> +# if defined(OPENSSL_NO_DEPRECATED)
> +#  define DECLARE_DEPRECATED(f)
> +# else
> +#  define DECLARE_DEPRECATED(f)   f;
> +#  ifdef __GNUC__
> +#   if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
> +#undef DECLARE_DEPRECATED
> +#define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
> +#   endif
> +#  endif
> +# endif
>  #endif
> 
>  #ifndef OPENSSL_FILE
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index 0300856cf2..6162d29143 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -175,6 +175,7 @@
>$(OPENSSL_PATH)/crypto/conf/conf_mall.c
>$(OPENSSL_PATH)/crypto/conf/conf_mod.c
>$(OPENSSL_PATH)/crypto/conf/conf_sap.c
> +  $(OPENSSL_PATH)/crypto/conf/conf_ssl.c
>$(OPENSSL_PATH)/crypto/cpt_err.c
>$(OPENSSL_PATH)/crypto/cryptlib.c
>$(OPENSSL_PATH)/crypto/cversion.c
> @@ -281,6 +282,7 @@
>$(OPENSSL_PATH)/crypto/evp/pmeth_lib.c
>$(OPENSSL_PATH)/crypto/evp/scrypt.c
>$(OPENSSL_PATH)/crypto/ex_data.c
> +  $(OPENSSL_PATH)/crypto/getenv.c
>$(OPENSSL_PATH)/crypto/hmac/hm_ameth.c
>$(OPENSSL_PATH)/crypto/hmac/hm_pmeth.c
>$(OPENSSL_PATH)/crypto/hmac/hmac.c
> @@ -418,6 +420,7 @@
>$(OPENSSL_PATH)/crypto/x509/x509_err.c
>$(OPENSSL_PATH)/crypto/x509/x509_ext.c
>$(OPENSSL_PATH)/crypto/x509/x509_lu.c
> +  $(OPENSSL_PATH)/crypto/x509/x509_meth.c
>$(OPENSSL_PATH)/crypto/x509/x509_obj.c
>$(OPENSSL_PATH)/crypto/x509/x509_r2x.c
>$(OPENSSL_PATH)/crypto/x509/x509_req.c
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 23be4e1e14..b04bf62b4e 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf

[edk2] [PATCH] Upgrade OpenSSL to 1.1.0j

2018-12-18 Thread Jian J Wang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1393

BZ#1089 (https://bugzilla.tianocore.org/show_bug.cgi?id=1089) requests
to upgrade the OpenSSL to the latest 1.1.1 release. Since OpenSSL-1.1.1
has many changes, more porting efforts and feature evaluation are needed.
This might lead to a situation that it cannot catch the Q1'19 stable tag.

One of the solution is upgrade current version (1.1.0h) to 1.1.0j.
According to following web page in openssl.org, all security issues
solved in 1.1.1 have been also back-ported to 1.1.0.j. This can make
sure that no security vulnerabilities left in edk2 master before 1.1.1.

https://www.openssl.org/news/vulnerabilities-1.1.1.html

Cc: Ting Ye 
Cc: Gang Wei 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 CryptoPkg/CryptoPkg.dsc   |  1 +
 .../Library/Include/openssl/opensslconf.h | 20 ---
 CryptoPkg/Library/OpensslLib/OpensslLib.inf   |  3 +++
 .../Library/OpensslLib/OpensslLibCrypto.inf   |  3 +++
 CryptoPkg/Library/OpensslLib/openssl  |  2 +-
 CryptoPkg/Library/OpensslLib/process_files.pl |  0
 6 files changed, 21 insertions(+), 8 deletions(-)
 mode change 100644 => 100755 CryptoPkg/Library/OpensslLib/process_files.pl

diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index a0334d628b..321abe4d4c 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -121,6 +121,7 @@
   CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
   CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   CryptoPkg/Library/TlsLib/TlsLib.inf
+  CryptoPkg/Library/OpensslLib/OpensslLib.inf
 
 [Components.IA32, Components.X64]
   CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h 
b/CryptoPkg/Library/Include/openssl/opensslconf.h
index 1917d7ab24..28dd9ab93c 100644
--- a/CryptoPkg/Library/Include/openssl/opensslconf.h
+++ b/CryptoPkg/Library/Include/openssl/opensslconf.h
@@ -2,7 +2,7 @@
  * WARNING: do not edit!
  * Generated from include/openssl/opensslconf.h.in
  *
- * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2016-2018 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -235,12 +235,18 @@ extern "C" {
  * still won't see them if the library has been built to disable deprecated
  * functions.
  */
-#if defined(OPENSSL_NO_DEPRECATED)
-# define DECLARE_DEPRECATED(f)
-#elif __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
-# define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
-#else
-# define DECLARE_DEPRECATED(f)   f;
+#ifndef DECLARE_DEPRECATED
+# if defined(OPENSSL_NO_DEPRECATED)
+#  define DECLARE_DEPRECATED(f)
+# else
+#  define DECLARE_DEPRECATED(f)   f;
+#  ifdef __GNUC__
+#   if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
+#undef DECLARE_DEPRECATED
+#define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
+#   endif
+#  endif
+# endif
 #endif
 
 #ifndef OPENSSL_FILE
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index 0300856cf2..6162d29143 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -175,6 +175,7 @@
   $(OPENSSL_PATH)/crypto/conf/conf_mall.c
   $(OPENSSL_PATH)/crypto/conf/conf_mod.c
   $(OPENSSL_PATH)/crypto/conf/conf_sap.c
+  $(OPENSSL_PATH)/crypto/conf/conf_ssl.c
   $(OPENSSL_PATH)/crypto/cpt_err.c
   $(OPENSSL_PATH)/crypto/cryptlib.c
   $(OPENSSL_PATH)/crypto/cversion.c
@@ -281,6 +282,7 @@
   $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c
   $(OPENSSL_PATH)/crypto/evp/scrypt.c
   $(OPENSSL_PATH)/crypto/ex_data.c
+  $(OPENSSL_PATH)/crypto/getenv.c
   $(OPENSSL_PATH)/crypto/hmac/hm_ameth.c
   $(OPENSSL_PATH)/crypto/hmac/hm_pmeth.c
   $(OPENSSL_PATH)/crypto/hmac/hmac.c
@@ -418,6 +420,7 @@
   $(OPENSSL_PATH)/crypto/x509/x509_err.c
   $(OPENSSL_PATH)/crypto/x509/x509_ext.c
   $(OPENSSL_PATH)/crypto/x509/x509_lu.c
+  $(OPENSSL_PATH)/crypto/x509/x509_meth.c
   $(OPENSSL_PATH)/crypto/x509/x509_obj.c
   $(OPENSSL_PATH)/crypto/x509/x509_r2x.c
   $(OPENSSL_PATH)/crypto/x509/x509_req.c
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 23be4e1e14..b04bf62b4e 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -175,6 +175,7 @@
   $(OPENSSL_PATH)/crypto/conf/conf_mall.c
   $(OPENSSL_PATH)/crypto/conf/conf_mod.c
   $(OPENSSL_PATH)/crypto/conf/conf_sap.c
+  $(OPENSSL_PATH)/crypto/conf/conf_ssl.c
   $(OPENSSL_PATH)/crypto/cpt_err.c
   $(OPENSSL_PATH)/crypto/cryptlib.c
   $(OPENSSL_PATH)/crypto/cversion.c
@@ -281,6 +282,7 @@
   $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c
   $(OPENSSL_PATH)/crypto/evp/scrypt.c
   $(OPENSSL_PATH)/crypto/ex_data.c
+  

Re: [edk2] [PATCH] BaseTools: Fix GenFds error doesn't break build.

2018-12-18 Thread Lin, Derek (HPS SW)
Hi Bob, 

Please try attached patch. Based on today's master.

Thanks,
Derek


-Original Message-
From: Feng, Bob C [mailto:bob.c.f...@intel.com] 
Sent: Wednesday, December 19, 2018 8:25 AM
To: Lin, Derek (HPS SW) ; edk2-devel@lists.01.org; Zhao, 
ZhiqiangX 
Subject: RE: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Hi Derek,

This patch looks good to me. But I failed to apply this patch on master. Could 
you update this patch?

Thanks,
Bob

-Original Message-
From: Lin, Derek (HPS SW) [mailto:derek.l...@hpe.com] 
Sent: Tuesday, December 18, 2018 4:52 PM
To: edk2-devel@lists.01.org; Feng, Bob C ; Zhao, 
ZhiqiangX 
Cc: Lin, Derek (HPS SW) 
Subject: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Fix a bug because of b3497bad1221704a5dbc5da0b10f42625f1ad2ed.
Before the patch, when GenFds fail, the build continue and return success.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Derek Lin 
---
 BaseTools/Source/Python/build/build.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index cbbb291b2c..97271e634e 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.  #  
Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2018, Hewlett Packard Enterprise Development, L.P.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -1384,7 +1385,8 
@@ class Build():
 
 # genfds
 if Target == 'fds':
-GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db)
+if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 return True
 
 # run
@@ -2122,7 +2124,8 @@ class Build():
 # Generate FD image if there's a FDF file found
 #
 GenFdsStart = time.time()
-GenFdsApi(Wa.GenFdsCommandDict, self.Db)
+if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 
 #
 # Create MAP file for all platform FVs after GenFds.
--
2.17.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 GenFds error doesn't break build.

2018-12-18 Thread Feng, Bob C
Thanks. This patch works.

Reviewed-by: Bob Feng 


Thanks,
Bob

-Original Message-
From: Lin, Derek (HPS SW) [mailto:derek.l...@hpe.com] 
Sent: Wednesday, December 19, 2018 9:55 AM
To: Feng, Bob C ; edk2-devel@lists.01.org; Zhao, 
ZhiqiangX 
Subject: RE: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Hi Bob, 

Please try attached patch. Based on today's master.

Thanks,
Derek


-Original Message-
From: Feng, Bob C [mailto:bob.c.f...@intel.com] 
Sent: Wednesday, December 19, 2018 8:25 AM
To: Lin, Derek (HPS SW) ; edk2-devel@lists.01.org; Zhao, 
ZhiqiangX 
Subject: RE: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Hi Derek,

This patch looks good to me. But I failed to apply this patch on master. Could 
you update this patch?

Thanks,
Bob

-Original Message-
From: Lin, Derek (HPS SW) [mailto:derek.l...@hpe.com] 
Sent: Tuesday, December 18, 2018 4:52 PM
To: edk2-devel@lists.01.org; Feng, Bob C ; Zhao, 
ZhiqiangX 
Cc: Lin, Derek (HPS SW) 
Subject: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Fix a bug because of b3497bad1221704a5dbc5da0b10f42625f1ad2ed.
Before the patch, when GenFds fail, the build continue and return success.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Derek Lin 
---
 BaseTools/Source/Python/build/build.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index cbbb291b2c..97271e634e 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.  #  
Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2018, Hewlett Packard Enterprise Development, L.P.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -1384,7 +1385,8 
@@ class Build():
 
 # genfds
 if Target == 'fds':
-GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db)
+if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 return True
 
 # run
@@ -2122,7 +2124,8 @@ class Build():
 # Generate FD image if there's a FDF file found
 #
 GenFdsStart = time.time()
-GenFdsApi(Wa.GenFdsCommandDict, self.Db)
+if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 
 #
 # Create MAP file for all platform FVs after GenFds.
--
2.17.0.windows.1



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


Re: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.

2018-12-18 Thread Wu, Hao A
Hi,

Could you grant me some time for reviewing this patch?

I will try to give you the feedbacks before the end of WW01 2019. Sorry for the
potential delay.

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Wednesday, December 19, 2018 5:29 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC
> HC v4 and above Support.
> 
> Add SDMA, ADMA2 and 26b data length support.
> 
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> SDMA registers supporting 64 bit addresses.
> 
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> ADMA descriptors supporting 64 bit addresses.
> 
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c|   2 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |   4 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  21 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   7 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 328
> +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  84 --
>  6 files changed, 363 insertions(+), 83 deletions(-)
>  mode change 100755 => 100644
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> old mode 100755
> new mode 100644
> index 2d3fb68..0c5646f
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
>//
>// Convert the clock freq unit from MHz to KHz.
>//
> -  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot]);
> +  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 68485c8..cdcdfa3 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -864,7 +864,7 @@ SdCardSetBusMode (
>  return Status;
>}
> 
> -  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot]);
> +  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -1064,7 +1064,7 @@ SdCardIdentification (
>  goto Error;
>}
> 
> -  SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
> +  SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> >ControllerVersion[Slot]);
> 
>gBS->Stall (1000);
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index a87f8de..b5bc260 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -62,7 +62,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate =
> {
>{ // MaxCurrent
>  0,
>},
> -  0 // ControllerVersion
> +  {
> +0   // ControllerVersion
> +  }
>  };
> 
>  SD_DEVICE_PATHmSdDpTemplate = {
> @@ -621,6 +623,14 @@ SdMmcPciHcDriverBindingStart (
>for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
>  Private->Slot[Slot].Enable = TRUE;
> 
> +//
> +// Get SD/MMC Pci Host Controller Version
> +//
> +Status = SdMmcHcGetControllerVersion (PciIo, Slot, 
> >ControllerVersion[Slot]);
> +if (EFI_ERROR (Status)) {
> +  goto Done;
> +}
> +
>  Status = SdMmcHcGetCapability (PciIo, Slot, >Capability[Slot]);
>  if (EFI_ERROR (Status)) {
>continue;
> @@ -649,7 +659,14 @@ SdMmcPciHcDriverBindingStart (
>Private->BaseClkFreq[Slot]
>));
> 
> -Support64BitDma &= Private->Capability[Slot].SysBus64;
> +//
> +// If any of the slots does not support 64b system bus
> +// do not enable 64b DMA in the PCI layer.
> +//
> +if (Private->Capability[Slot].SysBus64V3 == 0 &&
> +Private->Capability[Slot].SysBus64V4 == 0) {
> +  Support64BitDma = FALSE;
> +}
> 
>  Status = SdMmcHcGetMaxCurrent (PciIo, Slot, 
> 

Re: [edk2] [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' statement

2018-12-18 Thread Zeng, Star
Yi, Zailiang and David,

So, is it ok to give RB to the patch from you? :)


Thanks,
Star
-Original Message-
From: Qian, Yi 
Sent: Wednesday, December 19, 2018 9:21 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Sun, Zailiang ; Wei, David 
Subject: RE: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' 
statement

It's fine. Thank you for your update.

Thanks
Qian Yi

-Original Message-
From: Zeng, Star 
Sent: Tuesday, December 18, 2018 7:01 PM
To: Qian, Yi ; edk2-devel@lists.01.org
Cc: Sun, Zailiang ; Wei, David ; 
Zeng, Star 
Subject: RE: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' 
statement

Yi,

Good question.

With the patch, the code can be compiled and the image can boot to setup.
The image could not find shell, but that is not related to the patch.
It is because 
https://github.com/tianocore/edk2/commit/2840bb51040bb79c1ad53b1eb1cbb86e5edf80ca#diff-0318cca23f8f1c46d1076b3a5891fadd
 updated the platform dsc and fdf to use 2.0 shell source build (with 2.0 shell 
file GUID), but the code at 
https://github.com/tianocore/edk2/blob/master/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c#L1365
 is still using PcdShellFile (which matches to EDK shell) to find shell file.


Thanks,
Star
-Original Message-
From: Qian, Yi 
Sent: Tuesday, December 18, 2018 10:08 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Sun, Zailiang 
Subject: RE: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' 
statement

Hi Star,
   This patch is good to me. Only one thing I need to know is whether you have 
done the unit test, at least, compilation, and boot into shell. 

Thanks
Qian Yi

-Original Message-
From: Zeng, Star 
Sent: Friday, December 14, 2018 6:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Sun, Zailiang ; 
Qian, Yi 
Subject: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' statement

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

The codes have been updated to not use PcdPeiCoreMaxFvSupported, 
PcdPeiCoreMaxPeimPerFv and PcdPeiCoreMaxPpiSupported, so their statement in 
platform DSC could be removed.

Cc: Zailiang Sun 
Cc: Yi Qian 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 --
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 2 --
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 2 --
 3 files changed, 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index f8ad29df5986..d43611550285 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index ca3b2ff90287..a33816c4d18b 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 81f36bd73b28..b50731f25ffb 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   

Re: [edk2] [PATCH] BaseTools: Fix GenFds error doesn't break build.

2018-12-18 Thread Feng, Bob C
Hi Derek,

This patch looks good to me. But I failed to apply this patch on master. Could 
you update this patch?

Thanks,
Bob

-Original Message-
From: Lin, Derek (HPS SW) [mailto:derek.l...@hpe.com] 
Sent: Tuesday, December 18, 2018 4:52 PM
To: edk2-devel@lists.01.org; Feng, Bob C ; Zhao, 
ZhiqiangX 
Cc: Lin, Derek (HPS SW) 
Subject: [PATCH] BaseTools: Fix GenFds error doesn't break build.

Fix a bug because of b3497bad1221704a5dbc5da0b10f42625f1ad2ed.
Before the patch, when GenFds fail, the build continue and return success.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Derek Lin 
---
 BaseTools/Source/Python/build/build.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index cbbb291b2c..97271e634e 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.  #  
Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2018, Hewlett Packard Enterprise Development, L.P.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -1384,7 +1385,8 
@@ class Build():
 
 # genfds
 if Target == 'fds':
-GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db)
+if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 return True
 
 # run
@@ -2122,7 +2124,8 @@ class Build():
 # Generate FD image if there's a FDF file found
 #
 GenFdsStart = time.time()
-GenFdsApi(Wa.GenFdsCommandDict, self.Db)
+if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 
 #
 # Create MAP file for all platform FVs after GenFds.
--
2.17.0.windows.1


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


[edk2] [PATCH 1/1] UefiCpuPkg/CpuExceptionHandlerLib: Fix spelling issue

2018-12-18 Thread Mike Maslenkin
*Excpetion* should be *Exception*

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Mike Maslenkin 
CC: Eric Dong 
CC: Laszlo Ersek 
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h  | 2 +-
 UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c| 2 +-
 .../Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c  | 2 +-
 .../Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index e10d9379d596..cefa779b7e8a 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -310,7 +310,7 @@ CommonExceptionHandlerWorker (
   @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid content.
 **/
 EFI_STATUS
-ArchSetupExcpetionStack (
+ArchSetupExceptionStack (
   IN CPU_EXCEPTION_INIT_DATA*StackSwitchData
   );
 
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 2a090782fc22..70ee7dd8bfb3 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -285,7 +285,7 @@ InitializeCpuExceptionHandlersEx (
 
 InitData = 
   }
-  Status = ArchSetupExcpetionStack (InitData);
+  Status = ArchSetupExceptionStack (InitData);
 }
   }
 
diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index 04f2ab593c3e..8d1326c4ba71 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -119,7 +119,7 @@ ArchRestoreExceptionContext (
 
 **/
 EFI_STATUS
-ArchSetupExcpetionStack (
+ArchSetupExceptionStack (
   IN CPU_EXCEPTION_INIT_DATA  *StackSwitchData
   )
 {
diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 56180f4c17e4..02dfa50fc7d0 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -124,7 +124,7 @@ ArchRestoreExceptionContext (
 
 **/
 EFI_STATUS
-ArchSetupExcpetionStack (
+ArchSetupExceptionStack (
   IN CPU_EXCEPTION_INIT_DATA  *StackSwitchData
   )
 {
-- 
2.19.2

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


[edk2] [PATCH v7] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.

2018-12-18 Thread Ashish Singhal
Add SDMA, ADMA2 and 26b data length support.

If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
SDMA registers supporting 64 bit addresses.

If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.

If host controller version is above V4.0, enable ADMA2 with 26b data
length support for better performance. HC 2 register is configured to
use 26 bit data lengths and ADMA2 descriptors are configured appropriately.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c|   2 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |   4 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  21 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   7 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 328 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  84 --
 6 files changed, 363 insertions(+), 83 deletions(-)
 mode change 100755 => 100644 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
old mode 100755
new mode 100644
index 2d3fb68..0c5646f
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -707,7 +707,7 @@ EmmcSwitchClockFreq (
   //
   // Convert the clock freq unit from MHz to KHz.
   //
-  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, 
Private->BaseClkFreq[Slot]);
+  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, 
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
   if (EFI_ERROR (Status)) {
 return Status;
   }
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 68485c8..cdcdfa3 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -864,7 +864,7 @@ SdCardSetBusMode (
 return Status;
   }
 
-  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, 
Private->BaseClkFreq[Slot]);
+  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, 
Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -1064,7 +1064,7 @@ SdCardIdentification (
 goto Error;
   }
 
-  SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
+  SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], 
Private->ControllerVersion[Slot]);
 
   gBS->Stall (1000);
 
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a87f8de..b5bc260 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -62,7 +62,9 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = {
   { // MaxCurrent
 0,
   },
-  0 // ControllerVersion
+  {
+0   // ControllerVersion
+  }
 };
 
 SD_DEVICE_PATHmSdDpTemplate = {
@@ -621,6 +623,14 @@ SdMmcPciHcDriverBindingStart (
   for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
 Private->Slot[Slot].Enable = TRUE;
 
+//
+// Get SD/MMC Pci Host Controller Version
+//
+Status = SdMmcHcGetControllerVersion (PciIo, Slot, 
>ControllerVersion[Slot]);
+if (EFI_ERROR (Status)) {
+  goto Done;
+}
+
 Status = SdMmcHcGetCapability (PciIo, Slot, >Capability[Slot]);
 if (EFI_ERROR (Status)) {
   continue;
@@ -649,7 +659,14 @@ SdMmcPciHcDriverBindingStart (
   Private->BaseClkFreq[Slot]
   ));
 
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+//
+// If any of the slots does not support 64b system bus
+// do not enable 64b DMA in the PCI layer.
+//
+if (Private->Capability[Slot].SysBus64V3 == 0 &&
+Private->Capability[Slot].SysBus64V4 == 0) {
+  Support64BitDma = FALSE;
+}
 
 Status = SdMmcHcGetMaxCurrent (PciIo, Slot, >MaxCurrent[Slot]);
 if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 8c1a589..1bb701a 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
 
   Provides some data structure definitions used by the SD/MMC host controller 
driver.
 
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
 Copyright (c) 2015, 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
@@ -116,8 +117,7 @@ typedef struct {
   SD_MMC_HC_SLOT

Re: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.

2018-12-18 Thread Ashish Singhal
Hello Hao,


I have made all the changes as recommended and have submitted patch v7. I have 
completed all verification as suggested by you and it all seems to be working. 
I have also made changes for passing controller version through private 
structure.


Thanks

Ashish


From: Wu, Hao A 
Sent: Friday, November 30, 2018 1:03:15 AM
To: Ashish Singhal; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 
and above Support.

Hello,

Please refer to the inline comments below:

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 3:15 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add
> SDMMC HC v4 and above Support.
>
> Add SDMA, ADMA2 and 26b data length support.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> SDMA registers supporting 64 bit addresses.
>
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> ADMA descriptors supporting 64 bit addresses.
>
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   4 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 328
> ++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  48 ++-
>  3 files changed, 322 insertions(+), 58 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 8c1a589..3af7c95 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,6 +2,7 @@
>
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
>
> +Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>  Copyright (c) 2015, 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
> @@ -150,7 +151,8 @@ typedef struct {
>BOOLEAN Started;
>UINT64  Timeout;
>
> -  SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
> +  SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> +  SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
>EFI_PHYSICAL_ADDRESSAdmaDescPhy;
>VOID*AdmaMap;
>UINT32  AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 598b6a3..debc3be 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -4,6 +4,7 @@
>
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
>
> +  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2017, 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
> @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
>  }
>
>  /**
> +  Get the controller version information from the specified slot.
> +
> +  @param[in]  PciIo   The PCI IO protocol instance.
> +  @param[in]  SlotThe slot number of the SD card to send the
> command to.
> +  @param[out] Version The buffer to store the version information.
> +
> +  @retval EFI_SUCCESS The operation executes successfully.
> +  @retval Others  The operation fails.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcGetControllerVersion (
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN UINT8Slot,
> + OUT UINT16   *Version
> +  )
> +{
> +  EFI_STATUSStatus;
> +
> +  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE,
> sizeof (UINT16), Version);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  *Version &= 0xFF;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
>Software reset the specified SD/MMC host controller and enable all
> interrupts.
>
>@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA
> instance.
> @@ -776,18 +807,19 @@ SdMmcHcClockSupply (
>
>DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d
> ClockFreq %dKhz\n", BaseClkFreq, Divisor, ClockFreq));
>
> -  Status = SdMmcHcRwMmio (PciIo, Slot, 

Re: [edk2] [PATCH edk2-platforms 12/41] LS1043/BoardLib : Add support for LS1043 BoardLib.

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:26PM +0530, Meenakshi Aggarwal wrote:
> BoardLib will contain functions specific for LS1043aRdb board.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> Reviewed-by: Leif Lindholm 

Sorry, I do have a few more (trivial) comments.
But feel free to keep the reviewed-by if you address them.

> ---
>  .../NXP/LS1043aRdbPkg/Include/IfcBoardSpecific.h   | 109 
> +

Ifc->NxpIfc?

>  .../NXP/LS1043aRdbPkg/Library/BoardLib/BoardLib.c  |  69 +
>  .../LS1043aRdbPkg/Library/BoardLib/BoardLib.inf|  31 ++
>  3 files changed, 209 insertions(+)
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/Include/IfcBoardSpecific.h
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/Library/BoardLib/BoardLib.c
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/Library/BoardLib/BoardLib.inf
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/Include/IfcBoardSpecific.h 
> b/Platform/NXP/LS1043aRdbPkg/Include/IfcBoardSpecific.h
> new file mode 100644
> index 000..261867a
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/Include/IfcBoardSpecific.h
> @@ -0,0 +1,109 @@
> +/** IfcBoardSpecificLib.h
> +
> +  IFC Flash Board Specific Macros and structure
> +
> +  Copyright 2017 NXP
> +
> +  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 __IFC__BOARD_SPECIFIC_H__
> +#define __IFC__BOARD_SPECIFIC_H__

NXP_ prefix?

> +
> +#include 
> +
> +// On board flash support
> +#define IFC_NAND_BUF_BASE0x7E80
> +
> +// On board Inegrated flash Controller chip select configuration
> +#define IFC_NOR_CSIFC_CS0
> +#define IFC_NAND_CS   IFC_CS1
> +#define IFC_FPGA_CS   IFC_CS2
> +
> +// board-specific NAND timing
> +#define NAND_FTIM0(IFC_FTIM0_NAND_TCCST(0x7) | \
> +  IFC_FTIM0_NAND_TWP(0x18)   | \
> +  IFC_FTIM0_NAND_TWCHT(0x7) | \
> +  IFC_FTIM0_NAND_TWH(0xa))
> +
> +#define NAND_FTIM1(IFC_FTIM1_NAND_TADLE(0x32) | \
> +  IFC_FTIM1_NAND_TWBE(0x39)  | \
> +  IFC_FTIM1_NAND_TRR(0xe)   | \
> +  IFC_FTIM1_NAND_TRP(0x18))
> +
> +#define NAND_FTIM2(IFC_FTIM2_NAND_TRAD(0xf) | \
> +  IFC_FTIM2_NAND_TREH(0xa) | \
> +  IFC_FTIM2_NAND_TWHRE(0x1e))
> +
> +#define NAND_FTIM30x0
> +
> +#define NAND_CSPR   (IFC_CSPR_PHYS_ADDR(IFC_NAND_BUF_BASE) \
> +| IFC_CSPR_PORT_SIZE_8 \
> +| IFC_CSPR_MSEL_NAND \
> +| IFC_CSPR_V)
> +
> +#define NAND_CSPR_EXT   0x0
> +#define NAND_AMASK  0x
> +
> +#define NAND_CSOR (IFC_CSOR_NAND_ECC_ENC_EN /* ECC on encode */ \
> +  | IFC_CSOR_NAND_ECC_DEC_EN /* ECC on decode */ \
> +  | IFC_CSOR_NAND_ECC_MODE_4 /* 4-bit ECC */ \
> +  | IFC_CSOR_NAND_RAL_3   /* RAL = 3 Bytes */ \
> +  | IFC_CSOR_NAND_PGS_2K  /* Page Size = 2K */ \
> +  | IFC_CSOR_NAND_SPRZ_64 /* Spare size = 64 */ \
> +  | IFC_CSOR_NAND_PB(6)) /* 2^6 Pages Per Block */
> +
> +// board-specific NOR timing
> +#define NOR_FTIM0 (IFC_FTIM0_NOR_TACSE(0x1) | \
> +  IFC_FTIM0_NOR_TEADC(0x1) | \
> +  IFC_FTIM0_NOR_TAVDS(0x0) | \
> +  IFC_FTIM0_NOR_TEAHC(0xc))
> +#define NOR_FTIM1 (IFC_FTIM1_NOR_TACO(0x1c) | \
> +  IFC_FTIM1_NOR_TRAD_NOR(0xb) |\
> +  IFC_FTIM1_NOR_TSEQRAD_NOR(0x9))
> +#define NOR_FTIM2 (IFC_FTIM2_NOR_TCS(0x1) | \
> +  IFC_FTIM2_NOR_TCH(0x4) | \
> +  IFC_FTIM2_NOR_TWPH(0x8) | \
> +  IFC_FTIM2_NOR_TWP(0x10))
> +#define NOR_FTIM3 0x0
> +
> +#define NOR_CSPR  (IFC_CSPR_PHYS_ADDR(FixedPcdGet64 
> (PcdIfcRegion1BaseAddr)) \
> +  | IFC_CSPR_PORT_SIZE_16 \
> +  | IFC_CSPR_MSEL_NOR\
> +  | IFC_CSPR_V)
> +
> +#define NOR_CSPR_EXT  0x0
> +#define NOR_AMASK IFC_AMASK(128*1024*1024)
> +#define NOR_CSOR  (IFC_CSOR_NOR_ADM_SHIFT(4) | \
> +  IFC_CSOR_NOR_TRHZ_80)
> +
> +// board-specific fpga timing
> +#define FPGA_BASE_PHYS  0x7fb0
> +#define FPGA_CSPR_EXT   0x0
> +#define FPGA_CSPR   (IFC_CSPR_PHYS_ADDR(FPGA_BASE_PHYS) | \
> +IFC_CSPR_PORT_SIZE_8 | \
> +IFC_CSPR_MSEL_GPCM | \
> +IFC_CSPR_V)
> +
> +#define 

Re: [edk2] [PATCH edk2-platforms 11/41] IFC : Add Header file for IFC controller

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:25PM +0530, Meenakshi Aggarwal wrote:
> This header file contain IFC controller timing structure,
> chip select enum and other IFC macros.

Please expand the IFC acronym here (like is done in file header below).

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  Silicon/NXP/Include/Ifc.h | 423 
> ++

Please Update at least filename Ifc.h->NxpIfc.h 

>  1 file changed, 423 insertions(+)
>  create mode 100644 Silicon/NXP/Include/Ifc.h
> 
> diff --git a/Silicon/NXP/Include/Ifc.h b/Silicon/NXP/Include/Ifc.h
> new file mode 100644
> index 000..6babb22
> --- /dev/null
> +++ b/Silicon/NXP/Include/Ifc.h
> @@ -0,0 +1,423 @@
> +/** @Ifc.h
> +
> +  The integrated flash controller (IFC) is used to interface with external 
> asynchronous
> +  NAND flash, asynchronous NOR flash, SRAM, generic ASIC memories and EPROM.
> +
> +  Copyright 2017 NXP
> +
> +  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 __IFC_H__
> +#define __IFC_H__

Please add NXP_ prefix.

(I am less concerned about the actual defines below.)

> +
> +#include 

Is BaseLib.h really used by this file?
If not, please drop it.

> +#include 
> +
> +#define IFC_BANK_COUNT4
> +
> +#define IFC_CSPR_REG_LEN  148
> +#define IFC_AMASK_REG_LEN 144
> +#define IFC_CSOR_REG_LEN  144
> +#define IFC_FTIM_REG_LEN  576
> +
> +#define IFC_CSPR_USED_LEN sizeof (IFC_CSPR) * \
> +  IFC_BANK_COUNT
> +
> +#define IFC_AMASK_USED_LENsizeof (IFC_AMASK) * \
> +  IFC_BANK_COUNT
> +
> +#define IFC_CSOR_USED_LEN sizeof (IFC_CSOR) * \
> +  IFC_BANK_COUNT
> +
> +#define IFC_FTIM_USED_LEN sizeof (IFC_FTIM) * \
> +  IFC_BANK_COUNT
> +
> +/* List of commands */
> +#define IFC_NAND_CMD_RESET0xFF
> +#define IFC_NAND_CMD_READID   0x90
> +#define IFC_NAND_CMD_STATUS   0x70
> +#define IFC_NAND_CMD_READ00x00
> +#define IFC_NAND_CMD_READSTART0x30
> +#define IFC_NAND_CMD_ERASE1   0x60
> +#define IFC_NAND_CMD_ERASE2   0xD0
> +#define IFC_NAND_CMD_SEQIN0x80
> +#define IFC_NAND_CMD_PAGEPROG 0x10
> +#define MAX_RETRY_COUNT   15
> +
> +
> +#define IFC_NAND_SEQ_STRT_FIR_STRT  0x8000
> +
> +/*
> + * NAND Event and Error Status Register (NAND_EVTER_STAT)
> + */
> +
> +/* Operation Complete */
> +#define IFC_NAND_EVTER_STAT_OPC 0x8000
> +
> +/* Flash Timeout Error */
> +#define IFC_NAND_EVTER_STAT_FTOER   0x0800
> +
> +/* Write Protect Error */
> +#define IFC_NAND_EVTER_STAT_WPER0x0400
> +
> +/* ECC Error */
> +#define IFC_NAND_EVTER_STAT_ECCER   0x0200
> +
> +/*
> + * NAND Flash Instruction Registers (NAND_FIR0/NAND_FIR1/NAND_FIR2)
> + */
> +
> +/* NAND Machine specific opcodes OP0-OP14*/
> +#define IFC_NAND_FIR0_OP0   0xFC00
> +#define IFC_NAND_FIR0_OP0_SHIFT 26
> +#define IFC_NAND_FIR0_OP1   0x03F0
> +#define IFC_NAND_FIR0_OP1_SHIFT 20
> +#define IFC_NAND_FIR0_OP2   0x000FC000
> +#define IFC_NAND_FIR0_OP2_SHIFT 14
> +#define IFC_NAND_FIR0_OP3   0x3F00
> +#define IFC_NAND_FIR0_OP3_SHIFT 8
> +#define IFC_NAND_FIR0_OP4   0x00FC
> +#define IFC_NAND_FIR0_OP4_SHIFT 2
> +#define IFC_NAND_FIR1_OP5   0xFC00
> +#define IFC_NAND_FIR1_OP5_SHIFT 26
> +#define IFC_NAND_FIR1_OP6   0x03F0
> +#define IFC_NAND_FIR1_OP6_SHIFT 20
> +#define IFC_NAND_FIR1_OP7   0x000FC000
> +#define IFC_NAND_FIR1_OP7_SHIFT 14
> +#define IFC_NAND_FIR1_OP8   0x3F00
> +#define IFC_NAND_FIR1_OP8_SHIFT 8
> +#define IFC_NAND_FIR1_OP9   0x00FC
> +#define IFC_NAND_FIR1_OP9_SHIFT 2
> +#define IFC_NAND_FIR2_OP10  0xFC00
> +#define IFC_NAND_FIR2_OP10_SHIFT26
> +#define IFC_NAND_FIR2_OP11  0x03F0
> +#define IFC_NAND_FIR2_OP11_SHIFT20
> +#define IFC_NAND_FIR2_OP12  0x000FC000
> +#define IFC_NAND_FIR2_OP12_SHIFT14
> +#define IFC_NAND_FIR2_OP13  0x3F00
> +#define IFC_NAND_FIR2_OP13_SHIFT8
> +#define IFC_NAND_FIR2_OP14  0x00FC
> +#define IFC_NAND_FIR2_OP14_SHIFT2
> +
> +/*
> + * NAND Flash Command Registers (NAND_FCR0/NAND_FCR1)
> + */
> +
> +/* General purpose FCM flash command bytes CMD0-CMD7 */
> +#define IFC_NAND_FCR0_CMD0  0xFF00
> +#define IFC_NAND_FCR0_CMD0_SHIFT24
> +#define IFC_NAND_FCR0_CMD1  0x00FF
> +#define 

Re: [edk2] [PATCH edk2-platforms 10/41] Readme : Add Readme.md file.

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:24PM +0530, Meenakshi Aggarwal wrote:
> Readme.md to explain how to build NXP board packages.
> 

Could you add a link to this file from top-level Readme.md (towards
the very end)?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  Platform/NXP/Readme.md | 24 
>  1 file changed, 24 insertions(+)
>  create mode 100644 Platform/NXP/Readme.md
> 
> diff --git a/Platform/NXP/Readme.md b/Platform/NXP/Readme.md
> new file mode 100644
> index 000..902bafe
> --- /dev/null
> +++ b/Platform/NXP/Readme.md
> @@ -0,0 +1,24 @@
> +Support for all NXP boards is available in this directory.
> +
> +# How to build
> +
> +1. Set toolchain path.
> +
> +   export 
> PATH=/gcc-linaro-4.9-2016.02-x86_64_aarch64-linux-gnu/bin/:$PATH
> +
> +2. Export following variables needed for compilation.
> +
> +   export CROSS_COMPILE=aarch64-linux-gnu-
> +   export GCC_ARCH_PREFIX=GCC49_AARCH64_PREFIX
> +   export GCC49_AARCH64_PREFIX=aarch64-linux-gnu-
> +   export PACKAGES_PATH=/edk2/edk2-platforms
> +

Can you check whether you are happy with the generic build
instructions in the top-level Readme.md and improve those if not?
Then you could reference those rather than repeating.

/
Leif

> +3. Build desired board package
> +
> +   source edksetup.sh
> +   build -p "path to package's description (.dsc) file" -a AARCH64 -t GCC49 
> -b DEBUG/RELEASE clean
> +
> +   e.g.
> +   build -p "$PACKAGES_PATH/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc" -a 
> AARCH64 -t GCC49 -b DEBUG clean
> +   build -p "$PACKAGES_PATH/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc" -a 
> AARCH64 -t GCC49 -b DEBUG
> +
> -- 
> 1.9.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] ArmPkg: drop ArmBds remnant Pcds from .dec

2018-12-18 Thread Leif Lindholm
The following Pcds
- gArmTokenSpaceGuid.PcdArmLinuxSpinTable
- gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset
- gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset
- gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment
remained defined, without actual users.
So get rid of them.

One reference to be deleted separately from edk2-platforms.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 ArmPkg/ArmPkg.dec | 20 
 1 file changed, 20 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index d99eb6769f..0ec5e8efd6 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -80,10 +80,6 @@ [PcdsFeatureFlag.common]
   # it has been configured by the CPU DXE
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport|FALSE|BOOLEAN|0x0032
 
-  # Define if the spin-table mechanism is used by the secondary cores when 
booting
-  # Linux (instead of PSCI)
-  gArmTokenSpaceGuid.PcdArmLinuxSpinTable|FALSE|BOOLEAN|0x0033
-
   # Define if the GICv3 controller should use the GICv2 legacy
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x0042
 
@@ -173,16 +169,6 @@ [PcdsFixedAtBuild.ARM]
   # By default we do not do a transition to non-secure mode
   gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x0|UINT32|0x003E
 
-  # The Linux ATAGs are expected to be under 0x4000 (16KB) from the beginning 
of the System Memory
-  gArmTokenSpaceGuid.PcdArmLinuxAtagMaxOffset|0x4000|UINT32|0x0020
-
-  # If the fixed FDT address is not available, then it should be loaded below 
the kernel.
-  # The recommendation from the Linux kernel is to have the FDT below 16KB.
-  # (see the kernel doc: Documentation/arm/Booting)
-  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x4000|UINT32|0x0023
-  # The FDT blob must be loaded at a 64bit aligned address.
-  gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x8|UINT32|0x0026
-
   # Non Secure Access Control Register
   # - BIT15 : NSASEDIS - Disable Non-secure Advanced SIMD functionality
   # - BIT14 : NSD32DIS - Disable Non-secure use of D16-D31
@@ -221,12 +207,6 @@ [PcdsFixedAtBuild.AARCH64]
   # Other modes include using SP0 or switching to Aarch32, but these are
   # not currently supported.
   gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x3c9|UINT32|0x003E
-  # If the fixed FDT address is not available, then it should be loaded above 
the kernel.
-  # The recommendation from the AArch64 Linux kernel is to have the FDT below 
512MB.
-  # (see the kernel doc: Documentation/arm64/booting.txt)
-  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset|0x2000|UINT32|0x0023
-  # The FDT blob must be loaded at a 2MB aligned address.
-  gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment|0x0020|UINT32|0x0026
 
 
 #
-- 
2.11.0

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


[edk2] [PATCH edk2-platforms] Silicon/AMD: drop ArmBds remnant Pcd imports from StyxDtbLoaderLib

2018-12-18 Thread Leif Lindholm
ArmPkg PcdArmLinuxFdtMaxOffset and PcdArmLinuxFdtAlignment are still
referenced in StyxDtbLoaderLib.inf, but not actually used anywhere -
so drop them.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf 
b/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf
index 23c5e563bb..3fbc0548aa 100644
--- a/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf
+++ b/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.inf
@@ -52,8 +52,6 @@ [Pcd]
 [FixedPcd]
   gAmdModulePkgTokenSpaceGuid.PcdXgbeEnable
   gArmPlatformTokenSpaceGuid.PcdCoreCount
-  gArmTokenSpaceGuid.PcdArmLinuxFdtMaxOffset
-  gArmTokenSpaceGuid.PcdArmLinuxFdtAlignment
   gAmdStyxTokenSpaceGuid.PcdEnableKcs
   gAmdStyxTokenSpaceGuid.PcdSata1PortCount
 
-- 
2.11.0

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


Re: [edk2] [PATCH edk2-platforms 09/41] Compilation : Add the fdf, dsc and dec files.

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:23PM +0530, Meenakshi Aggarwal wrote:
> The firmware device, description and declaration files.
> 

I won't give a reviewed-by yet since there will be at least some
changes related to watchdog - but I have a couple of comments in
addition to that.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  Platform/NXP/FVRules.fdf.inc |  99 +++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec |  29 ++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc |  80 ++
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 198 +
>  Platform/NXP/NxpQoriqLs.dsc.inc  | 412 
> +++
>  Silicon/NXP/LS1043A/LS1043A.dec  |  22 ++
>  Silicon/NXP/LS1043A/LS1043A.dsc.inc  |  73 +
>  Silicon/NXP/NxpQoriqLs.dec   | 117 
>  8 files changed, 1030 insertions(+)
>  create mode 100644 Platform/NXP/FVRules.fdf.inc
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dec
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
>  create mode 100644 Platform/NXP/NxpQoriqLs.dsc.inc
>  create mode 100644 Silicon/NXP/LS1043A/LS1043A.dec
>  create mode 100644 Silicon/NXP/LS1043A/LS1043A.dsc.inc
>  create mode 100644 Silicon/NXP/NxpQoriqLs.dec
> 
> diff --git a/Platform/NXP/FVRules.fdf.inc b/Platform/NXP/FVRules.fdf.inc
> new file mode 100644
> index 000..d0e17cb
> --- /dev/null
> +++ b/Platform/NXP/FVRules.fdf.inc
> @@ -0,0 +1,99 @@
> +#  FvRules.fdf.inc
> +#
> +#  Rules for creating FD.
> +#
> +#  Copyright 2017 NXP
> +#
> +#  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.
> +#
> +
> +
> +#
> +# Rules are use with the [FV] section's module INF type to define
> +# how an FFS file is created for a given INF file. The following Rule are 
> the default
> +# rules for the different module type. User can add the customized rules to 
> define the
> +# content of the FFS file.
> +#
> +
> +
> +[Rule.Common.SEC]
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> +TE  TEAlign = 32$(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +[Rule.Common.PEI_CORE]
> +  FILE PEI_CORE = $(NAMED_GUID) {
> +TE TE   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +UI STRING ="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.PEIM]
> +  FILE PEIM = $(NAMED_GUID) {
> + PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
> + PE32  PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
> + UI   STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.PEIM.TIANOCOMPRESSED]
> +  FILE PEIM = $(NAMED_GUID) DEBUG_MYTOOLS_IA32 {
> +PEI_DEPEX PEI_DEPEX Optional$(INF_OUTPUT)/$(MODULE_NAME).depex
> +GUIDED A31280AD-481E-41B6-95E8-127F4C984779 PROCESSING_REQUIRED = TRUE {
> +  PE32  PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
> +  UISTRING="$(MODULE_NAME)" Optional
> +}
> +  }
> +
> +[Rule.Common.DXE_CORE]
> +  FILE DXE_CORE = $(NAMED_GUID) {
> +PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +UI   STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +
> +[Rule.Common.UEFI_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +DXE_DEPEXDXE_DEPEX  Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
> +PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +UI   STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.DXE_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +DXE_DEPEXDXE_DEPEX  Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
> +PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +UI   STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.DXE_RUNTIME_DRIVER]
> +  FILE DRIVER = $(NAMED_GUID) {
> +DXE_DEPEXDXE_DEPEX  Optional 
> $(INF_OUTPUT)/$(MODULE_NAME).depex
> +PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
> +UI   STRING="$(MODULE_NAME)" Optional
> +  }
> +
> +[Rule.Common.UEFI_APPLICATION]
> +  FILE APPLICATION = $(NAMED_GUID) {
> +UI STRING ="$(MODULE_NAME)" Optional
> +PE32   PE32 $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  }
> +
> +[Rule.Common.UEFI_DRIVER.BINARY]
> +  FILE DRIVER = 

Re: [edk2] [PATCH edk2-platforms 08/41] Platform/NXP: Add Platform driver for LS1043 RDB board

2018-12-18 Thread Leif Lindholm
Please add a commit message.
What does the platform driver _do_?

(No comments on the code.)

/
Leif

On Wed, Nov 28, 2018 at 08:31:22PM +0530, Meenakshi Aggarwal wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  .../Drivers/PlatformDxe/PlatformDxe.c  | 119 
> +
>  .../Drivers/PlatformDxe/PlatformDxe.inf|  58 ++
>  2 files changed, 177 insertions(+)
>  create mode 100644 
> Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
>  create mode 100644 
> Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> 
> diff --git a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c 
> b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> new file mode 100644
> index 000..7ce7318
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -0,0 +1,119 @@
> +/** @file
> +  LS1043 DXE platform driver.
> +
> +  Copyright 2018 NXP
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +typedef struct {
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR StartDesc;
> +  UINT8 EndDesc;
> +} ADDRESS_SPACE_DESCRIPTOR;
> +
> +STATIC ADDRESS_SPACE_DESCRIPTOR mI2cDesc[FixedPcdGet64 
> (PcdNumI2cController)];
> +
> +STATIC
> +EFI_STATUS
> +RegisterDevice (
> +  IN  EFI_GUID*TypeGuid,
> +  IN  ADDRESS_SPACE_DESCRIPTOR*Desc,
> +  OUT EFI_HANDLE  *Handle
> +  )
> +{
> +  NON_DISCOVERABLE_DEVICE *Device;
> +  EFI_STATUS  Status;
> +
> +  Device = (NON_DISCOVERABLE_DEVICE *)AllocateZeroPool (sizeof (*Device));
> +  if (Device == NULL) {
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Device->Type = TypeGuid;
> +  Device->DmaType = NonDiscoverableDeviceDmaTypeNonCoherent;
> +  Device->Resources = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)Desc;
> +
> +  Status = gBS->InstallMultipleProtocolInterfaces (Handle,
> +  , Device,
> +  NULL);
> +  if (EFI_ERROR (Status)) {
> +goto FreeDevice;
> +  }
> +  return EFI_SUCCESS;
> +
> +FreeDevice:
> +  FreePool (Device);
> +
> +  return Status;
> +}
> +
> +VOID
> +PopulateI2cInformation (
> +  IN VOID
> +  )
> +{
> +  UINT32 Index;
> +
> +  for (Index = 0; Index < FixedPcdGet32 (PcdNumI2cController); Index++) {
> +mI2cDesc[Index].StartDesc.Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +mI2cDesc[Index].StartDesc.Len = sizeof 
> (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
> +mI2cDesc[Index].StartDesc.ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +mI2cDesc[Index].StartDesc.GenFlag = 0;
> +mI2cDesc[Index].StartDesc.SpecificFlag = 0;
> +mI2cDesc[Index].StartDesc.AddrSpaceGranularity = 32;
> +mI2cDesc[Index].StartDesc.AddrRangeMin = FixedPcdGet64 (PcdI2c0BaseAddr) 
> +
> + (Index * FixedPcdGet32 
> (PcdI2cSize));
> +mI2cDesc[Index].StartDesc.AddrRangeMax = 
> mI2cDesc[Index].StartDesc.AddrRangeMin +
> + FixedPcdGet32 (PcdI2cSize) - 1;
> +mI2cDesc[Index].StartDesc.AddrTranslationOffset = 0;
> +mI2cDesc[Index].StartDesc.AddrLen = FixedPcdGet32 (PcdI2cSize);
> +
> +mI2cDesc[Index].EndDesc = ACPI_END_TAG_DESCRIPTOR;
> +  }
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +PlatformDxeEntryPoint (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_HANDLE  Handle;
> +
> +  Handle = NULL;
> +
> +  PopulateI2cInformation ();
> +
> +  Status = RegisterDevice (,
> + [0], );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Install the DS1307 I2C Master protocol on this handle so the RTC driver
> +  // can identify it as the I2C master it can invoke directly.
> +  //
> +  Status = gBS->InstallProtocolInterface (,
> +  ,
> +  EFI_NATIVE_INTERFACE, NULL);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf 
> b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> new file mode 100644
> index 000..91d6ad3
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -0,0 +1,58 @@
> +## @file
> +#
> +#  Component description file for LS1043 DXE platform driver.
> +#
> +#  Copyright 2018 NXP
> +#
> +#  This program and the 

Re: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg PXE/iSCSI/TCP with NetworkPkg Drivers.

2018-12-18 Thread Kubacki, Michael A
Reviewed-by: Michael Kubacki 

-Original Message-
From: Yao, Jiewen 
Sent: Tuesday, December 18, 2018 3:42 AM
To: Fu, Siyuan ; edk2-devel@lists.01.org
Cc: Kubacki, Michael A 
Subject: RE: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg PXE/iSCSI/TCP 
with NetworkPkg Drivers.

Looks good.
Reviewed-by: jiewen@intel.com


> -Original Message-
> From: Fu, Siyuan
> Sent: Tuesday, December 18, 2018 3:33 PM
> To: Fu, Siyuan ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Kubacki, Michael A 
> 
> Subject: RE: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg 
> PXE/iSCSI/TCP with NetworkPkg Drivers.
> 
> Hi, Jiewen and Kubacki
> 
> Do you have any comments for this patch?
> 
> BestRegards
> Fu Siyuan
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of
> Fu
> > Siyuan
> > Sent: Friday, December 14, 2018 2:32 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen ; Kubacki, Michael A 
> > 
> > Subject: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg
> PXE/iSCSI/TCP with
> > NetworkPkg Drivers.
> >
> > The PXE/iSCSI/TCP drivers in MdeModulePkg are going to be deprecated.
> All
> > platform DSC/FDF files should be updated to use the dual-stack 
> > drivers in NetworkPkg.
> >
> > Cc: Michael A Kubacki 
> > Cc: Jiewen Yao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Fu Siyuan 
> > ---
> >
> Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclude.
> dsc  | 7
> > ++-
> >
> Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclude.
> fdf | 7
> > ++-
> >  Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> | 3
> > +--
> >  3 files changed, 5 insertions(+), 12 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > index 4d70db6062..6764d46131 100644
> > ---
> a/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > +++
> b/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  Platform description.
> >  #
> > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2017 - 2018, 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.
> > @@ -26,10 +26,7 @@
> >MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> >MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> >MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > -  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -  #MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >
> >NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> >NetworkPkg/TcpDxe/TcpDxe.inf
> > @@ -42,7 +39,7 @@
> >NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> >NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> >
> > -  #NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +  NetworkPkg/IScsiDxe/IScsiDxe.inf
> >NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >  !endif
> >
> > diff --git
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > index 0be408d13b..64f1dd5872 100644
> > ---
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > +++
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  FDF file of Platform.
> >  #
> > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2017 - 2018, 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.
> > @@ -27,9 +27,6 @@ INF
> MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > -#INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > -INF  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -#INF
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >
> >  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> >  INF  NetworkPkg/TcpDxe/TcpDxe.inf
> > @@ -42,7 +39,7 @@ INF  NetworkPkg/HttpDxe/HttpDxe.inf  INF  
> > NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> >  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> >
> > -#INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> >  INF  

Re: [edk2] [PATCH edk2-platforms 05/41] Silicon/NXP: Add support for I2c driver

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:19PM +0530, Meenakshi Aggarwal wrote:
> I2C driver produces gEfiI2cMasterProtocolGuid which can be
> used by other modules.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 

Many thanks for the non-trivial rework.

A few style comments below.

> ---
>  Silicon/NXP/Drivers/I2cDxe/ComponentName.c | 185 
>  Silicon/NXP/Drivers/I2cDxe/DriverBinding.c | 241 ++
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.c| 693 
> +
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.h|  96 
>  Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf  |  64 +++
>  5 files changed, 1279 insertions(+)
>  create mode 100644 Silicon/NXP/Drivers/I2cDxe/ComponentName.c
>  create mode 100644 Silicon/NXP/Drivers/I2cDxe/DriverBinding.c
>  create mode 100644 Silicon/NXP/Drivers/I2cDxe/I2cDxe.c
>  create mode 100644 Silicon/NXP/Drivers/I2cDxe/I2cDxe.h
>  create mode 100644 Silicon/NXP/Drivers/I2cDxe/I2cDxe.inf
> 
> diff --git a/Silicon/NXP/Drivers/I2cDxe/ComponentName.c 
> b/Silicon/NXP/Drivers/I2cDxe/ComponentName.c
> new file mode 100644
> index 000..efed6b9
> --- /dev/null
> +++ b/Silicon/NXP/Drivers/I2cDxe/ComponentName.c
> @@ -0,0 +1,185 @@
> +/** @file
> +
> +  Copyright 2018 NXP
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include "I2cDxe.h"
> +
> +STATIC EFI_UNICODE_STRING_TABLE mNxpI2cDriverNameTable[] = {
> +  {
> +"en",
> +(CHAR16 *)L"Nxp I2C Driver"
> +  },
> +  { }
> +};
> +
> +STATIC EFI_UNICODE_STRING_TABLE mNxpI2cControllerNameTable[] = {
> +  {
> +"en",
> +(CHAR16 *)L"Nxp I2C Controller"
> +  },
> +  { }
> +};
> +
> +/**
> +  Retrieves a Unicode string that is the user readable name of the driver.
> +
> +  This function retrieves the user readable name of a driver in the form of a
> +  Unicode string. If the driver specified by This has a user readable name in
> +  the language specified by Language, then a pointer to the driver name is
> +  returned in DriverName, and EFI_SUCCESS is returned. If the driver 
> specified
> +  by This does not support the language specified by Language,
> +  then EFI_UNSUPPORTED is returned.
> +
> +  @param  This[in]  A pointer to the 
> EFI_COMPONENT_NAME2_PROTOCOL or
> +EFI_COMPONENT_NAME_PROTOCOL instance.
> +
> +  @param  Language[in]  A pointer to a Null-terminated ASCII string
> +array indicating the language. This is the
> +language of the driver name that the caller 
> is
> +requesting, and it must match one of the
> +languages specified in SupportedLanguages. 
> The
> +number of languages supported by a driver is 
> up
> +to the driver writer. Language is specified
> +in RFC 4646 or ISO 639-2 language code 
> format.
> +
> +  @param  DriverName[out]   A pointer to the Unicode string to return.
> +This Unicode string is the name of the
> +driver specified by This in the language
> +specified by Language.
> +
> +  @retval EFI_SUCCESS   The Unicode string for the Driver specified 
> by
> +This and the language specified by Language 
> was
> +returned in DriverName.
> +
> +  @retval EFI_INVALID_PARAMETER Language is NULL.
> +
> +  @retval EFI_INVALID_PARAMETER DriverName is NULL.
> +
> +  @retval EFI_UNSUPPORTED   The driver specified by This does not support
> +the language specified by Language.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +NxpI2cGetDriverName (
> +  IN  EFI_COMPONENT_NAME2_PROTOCOL  *This,
> +  IN  CHAR8 *Language,
> +  OUT CHAR16**DriverName
> +  )
> +{
> +  return LookupUnicodeString2 (Language,
> +   This->SupportedLanguages,
> +   mNxpI2cDriverNameTable,
> +   DriverName,
> +   FALSE);
> +}
> +
> +/**
> +  Retrieves a Unicode string that is the user readable name of the controller
> +  that is being managed by a driver.
> +
> +  This function retrieves the user readable name of the controller specified 
> by
> +  ControllerHandle and ChildHandle in the form of a Unicode string. If the
> + 

[edk2] [PATCH] Platform/FVP-AArch64: switch to the SBSA watchdog

2018-12-18 Thread Ard Biesheuvel
On the FVP Foundation model, the SP805 watchdog appears to be 'wired'
incorrectly, resulting in a watchdog counter that decrements at the
APB clock rate of 24 MHz instead of the usual 32 kHz. Since the
timer start value is only 32-bits wide, this makes the watchdog
unusable in UEFI, since the default timeout set by the DXE core is
5 minutes, which is not representable in 32-bit at this clock rate.

So switch to the SBSA watchdog instead, which is wired up to the
generic timer, and ticks at the correct rate.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc   | 7 
---
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf   | 2 +-
 Platform/ARM/VExpressPkg/Include/Platform/RTSM/ArmPlatform.h   | 3 +++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf | 3 +++
 Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c  | 7 
++-
 5 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index 7db1c675c3d9..0941edeaf53c 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -133,9 +133,10 @@
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x1C17
 
   ## SBSA Watchdog Count
-!ifndef DISABLE_SBSA_WATCHDOG
   gArmPlatformTokenSpaceGuid.PcdWatchdogCount|1
-!endif
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x2a44
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x2a45
+  gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|59
 
 !ifdef EDK2_ENABLE_PL111
   ## PL111 Versatile Express Motherboard controller
@@ -265,7 +266,7 @@
 !ifdef EDK2_ENABLE_PL111
   ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
 !endif
-  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf
+  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
 
   # SMBIOS Support
 
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf
index 239029d05cf1..c3e573e1bb4f 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf
@@ -116,7 +116,7 @@ FvNameGuid = 87940482-fc81-41c3-87e6-399cf85ac8a0
 !ifdef EDK2_ENABLE_PL111
   INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
 !endif
-  INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf
+  INF ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
 
   #
   # Semi-hosting filesystem
diff --git a/Platform/ARM/VExpressPkg/Include/Platform/RTSM/ArmPlatform.h 
b/Platform/ARM/VExpressPkg/Include/Platform/RTSM/ArmPlatform.h
index d856b6daa1d7..e267912ef5f5 100644
--- a/Platform/ARM/VExpressPkg/Include/Platform/RTSM/ArmPlatform.h
+++ b/Platform/ARM/VExpressPkg/Include/Platform/RTSM/ArmPlatform.h
@@ -76,4 +76,7 @@
 #define PL111_CLCD_MOTHERBOARD_VIDEO_MODE_OSC_ID  1
 #define PL111_CLCD_CORE_TILE_VIDEO_MODE_OSC_ID  1
 
+#define SBSA_WATCHDOG_BASE  0x2a44
+#define SBSA_WATCHDOG_SIZE  (2 * SIZE_64KB)
+
 #endif
diff --git 
a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf 
b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
index 53898c5e957e..511a2ac99b75 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib.inf
@@ -60,5 +60,8 @@
   gArmPlatformTokenSpaceGuid.PcdArmMaliDpBase
   gArmPlatformTokenSpaceGuid.PcdArmMaliDpMemoryRegionLength
 
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
+
 [Ppis]
   gArmMpCoreInfoPpiGuid
diff --git a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c 
b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
index c8eefa0cf28b..eb8f6a48cd02 100644
--- a/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
+++ b/Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
@@ -23,7 +23,7 @@
 #define DP_BASE_DESCRIPTOR  ((FixedPcdGet64 (PcdArmMaliDpBase) != 0) ? 1 : 
0)
 
 // Number of Virtual Memory Map Descriptors
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (9 + DP_BASE_DESCRIPTOR)
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS (10 + DP_BASE_DESCRIPTOR)
 
 // DDR attributes
 #define DDR_ATTRIBUTES_CACHED   ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
@@ -175,6 +175,11 @@ ArmPlatformGetVirtualMemoryMap (
 VirtualMemoryTable[Index].Attributes = CacheAttributes;
   }
 
+  VirtualMemoryTable[++Index].PhysicalBase = SBSA_WATCHDOG_BASE;
+  VirtualMemoryTable[Index].VirtualBase = SBSA_WATCHDOG_BASE;
+  VirtualMemoryTable[Index].Length = SBSA_WATCHDOG_SIZE;
+  VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
   // End 

Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 14:39, Leif Lindholm  wrote:
>
> On Tue, Dec 18, 2018 at 02:10:12PM +0100, Ard Biesheuvel wrote:
> > The SP805 watchdog driver doesn't implement the PI watchdog protocol
> > fully, but always simply resets the system if the watchdog time runs
> > out.
> >
> > However, the hardware does support the intended usage model, as long
> > as the SP805 is wired up correctly. So let's implement interrupt based
> > mode involving a handler that is registered by the DXE core and invoked
> > when the watchdog runs out. In the interrupt handler, we invoke the
> > notify function if one was registered, or call the ResetSystem()
> > runtime service otherwise (as per the UEFI spec)
>
> The only question mark from my end is - what happens when the
> interrupt isn't wired up correctly? Would it be worth to bail out and
> refuse to register the driver if PcdSP805WatchdogInterrupt is set to
> 0?
>
> Thomas?
>

I have left the code in place that enables the hard reset, but the
timeout is double the programmed value (since the countdown timer is
restarted on an interrupt, and the hard reset is generated when it
reaches zero the second time)

This should cover both the miswired interrupt scenario, and the
scenario where ResetSystem() (or the handler) gets stuck and never
returns.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 16:58, Udit Kumar  wrote:
>
>
>
> > -Original Message-
> > From: Ard Biesheuvel 
> > Sent: Tuesday, December 18, 2018 6:40 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel ; Leif Lindholm
> > ; Sami Mujawar ; Thomas
> > Panakamattam Abraham ; Meenakshi Aggarwal
> > ; Udit Kumar ; Matteo
> > Carlini ; Nariman Poushin
> > 
> > Subject: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt
> > mode
> >
> > The SP805 watchdog driver doesn't implement the PI watchdog protocol fully,
> > but always simply resets the system if the watchdog time runs out.
> >
> > However, the hardware does support the intended usage model, as long as the
> > SP805 is wired up correctly. So let's implement interrupt based mode 
> > involving a
> > handler that is registered by the DXE core and invoked when the watchdog 
> > runs
> > out. In the interrupt handler, we invoke the notify function if one was 
> > registered,
> > or call the ResetSystem() runtime service otherwise (as per the UEFI spec)
>
> Specs, says
> If no handler has been registered, or the registered handler returns, then 
> the system will be reset by calling the Runtime Service ResetSystem().
> My understanding is that we have to ResetSystem() always irrespective of 
> notify function
>

Indeed. Thanks for spotting that.

>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
> >  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95
> > ++--
> >  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
> >  3 files changed, 75 insertions(+), 27 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> > b/ArmPlatformPkg/ArmPlatformPkg.dec
> > index 5f67e7415469..44c00bd0c133 100644
> > --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> > @@ -70,6 +70,7 @@
> >## SP805 Watchdog
> >
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x00
> > 23
> >
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|
> > UINT32|0x0021
> > +
> > +
> > gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x
> > 00
> > + 2E
> >
> >## PL011 UART
> >
> > gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x00
> > 1F
> > diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > index 12c2f0a1fe49..4f09acf1fa28 100644
> > --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> > @@ -21,12 +21,17 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> > +#include 
> >  #include 
> >
> >  #include "SP805Watchdog.h"
> >
> > -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> > +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> > +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> > +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> > +STATIC UINT32   mTimerPeriod;
> >
> >  /**
> >Make sure the SP805 registers are unlocked for writing.
> > @@ -65,6 +70,33 @@ SP805Lock (
> >}
> >  }
> >
> > +STATIC
> > +VOID
> > +EFIAPI
> > +SP805InterruptHandler (
> > +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> > +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> > +  )
> > +{
> > +  //
> > +  // The notify function should be called with the elapsed number of
> > +ticks
> > +  // since the watchdog was armed, which should exceed the timer period.
> > +  // We don't actually know the elapsed number of ticks, so let's
> > +return
> > +  // the timer period plus 1.
> > +  //
> > +  if (mWatchdogNotify != NULL) {
> > +mWatchdogNotify (mTimerPeriod + 1);
> > +  } else {
> > +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> > +  }
>
> Shouldn't we ResetSystem in all cases.
>
>
> > +
> > +  SP805Unlock ();
> > +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears
> > + the irq  SP805Lock ();
> > +
> > +  mInterrupt->EndOfInterrupt (mInterrupt, Source); }
> > +
>
> IMO, this routine should be
> 1/ Handle IT
> 2/ Do callback mWatchdogNotify
> 3/ Reset the system.
>
> >  /**
> >Stop the SP805 watchdog timer from counting down by disabling interrupts.
> >  **/
> > @@ -149,9 +181,16 @@ SP805RegisterHandler (
> >IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
> >)
> >  {
> > -  // ERROR: This function is not supported.
> > -  // The hardware watchdog will reset the board
> > -  return EFI_INVALID_PARAMETER;
> > +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> > +return EFI_ALREADY_STARTED;
> > +  }
> > +
> > +  mWatchdogNotify = NotifyFunction;
> > +  return EFI_SUCCESS;
> > 

Re: [edk2] [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement RegisterHandler() method

2018-12-18 Thread Udit Kumar



> -Original Message-
> From: Ard Biesheuvel 
> Sent: Tuesday, December 18, 2018 6:40 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Leif Lindholm
> ; Sami Mujawar ; Thomas
> Panakamattam Abraham ; Meenakshi Aggarwal
> ; Udit Kumar ; Matteo
> Carlini ; Nariman Poushin
> 
> Subject: [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement
> RegisterHandler() method
> 
> Even though UEFI does not appear to use it, let's implement the complete PI
> watchdog protocol, including handler registration, which will be invoked 
> instead
> of the ResetSystem() runtime service when the watchdog timer expires.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34
> ++--
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> index 717a180a64ec..21118a3c88d1 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0;  STATIC UINT64
> mNumTimerTicks = 0;
> 
>  STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> 
>  STATIC
>  VOID
> @@ -107,17 +108,25 @@ WatchdogInterruptHandler (
>)
>  {
>STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out.";
> +  UINT64  TimerPeriod;
> 
>WatchdogDisable ();
> 
>mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source);
> 
> -  gRT->ResetSystem (
> - EfiResetCold,
> - EFI_TIMEOUT,
> - StrSize (ResetString),
> - (VOID *) 
> - );
> +  //
> +  // The notify function should be called with the elapsed number of
> + ticks  // since the watchdog was armed, which should exceed the timer 
> period.
> +  // We don't actually know the elapsed number of ticks, so let's
> + return  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) *
> mNumTimerTicks);
> +mWatchdogNotify (TimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString),
> +   (CHAR16 *)ResetString);
> +  }

Here too, please handle reset in all cases 
 
>// If we got here then the reset didn't work
>ASSERT (FALSE);
> @@ -155,9 +164,16 @@ WatchdogRegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The watchdog will reset the board
> -  return EFI_UNSUPPORTED;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
> 
>  /**
> --
> 2.17.1

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


Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Udit Kumar



> -Original Message-
> From: Ard Biesheuvel 
> Sent: Tuesday, December 18, 2018 6:40 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Leif Lindholm
> ; Sami Mujawar ; Thomas
> Panakamattam Abraham ; Meenakshi Aggarwal
> ; Udit Kumar ; Matteo
> Carlini ; Nariman Poushin
> 
> Subject: [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt
> mode
> 
> The SP805 watchdog driver doesn't implement the PI watchdog protocol fully,
> but always simply resets the system if the watchdog time runs out.
> 
> However, the hardware does support the intended usage model, as long as the
> SP805 is wired up correctly. So let's implement interrupt based mode 
> involving a
> handler that is registered by the DXE core and invoked when the watchdog runs
> out. In the interrupt handler, we invoke the notify function if one was 
> registered,
> or call the ResetSystem() runtime service otherwise (as per the UEFI spec)

Specs, says 
If no handler has been registered, or the registered handler returns, then the 
system will be reset by calling the Runtime Service ResetSystem().
My understanding is that we have to ResetSystem() always irrespective of notify 
function

 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95
> ++--
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
>  3 files changed, 75 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 5f67e7415469..44c00bd0c133 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -70,6 +70,7 @@
>## SP805 Watchdog
> 
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x00
> 23
> 
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|
> UINT32|0x0021
> +
> +
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x
> 00
> + 2E
> 
>## PL011 UART
> 
> gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x00
> 1F
> diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> index 12c2f0a1fe49..4f09acf1fa28 100644
> --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> @@ -21,12 +21,17 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
> +#include 
>  #include 
> 
>  #include "SP805Watchdog.h"
> 
> -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> +STATIC UINT32   mTimerPeriod;
> 
>  /**
>Make sure the SP805 registers are unlocked for writing.
> @@ -65,6 +70,33 @@ SP805Lock (
>}
>  }
> 
> +STATIC
> +VOID
> +EFIAPI
> +SP805InterruptHandler (
> +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> +  )
> +{
> +  //
> +  // The notify function should be called with the elapsed number of
> +ticks
> +  // since the watchdog was armed, which should exceed the timer period.
> +  // We don't actually know the elapsed number of ticks, so let's
> +return
> +  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +mWatchdogNotify (mTimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> +  }

Shouldn't we ResetSystem in all cases.
 

> +
> +  SP805Unlock ();
> +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears
> + the irq  SP805Lock ();
> +
> +  mInterrupt->EndOfInterrupt (mInterrupt, Source); }
> +

IMO, this routine should be 
1/ Handle IT
2/ Do callback mWatchdogNotify
3/ Reset the system.

>  /**
>Stop the SP805 watchdog timer from counting down by disabling interrupts.
>  **/
> @@ -149,9 +181,16 @@ SP805RegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The hardware watchdog will reset the board
> -  return EFI_INVALID_PARAMETER;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
> 
>  /**
> @@ -202,19 +241,16 @@ SP805SetTimerPeriod (
>  SP805Stop ();
>} else {
>  // Calculate the Watchdog ticks required for a delay of (TimerTicks * 
> 100)
> nanoseconds
> -// The SP805 will count down to ZERO once, generate an interrupt and
> -// then it will again reload the initial value and start again.
> -// On the second time 

Re: [edk2] [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 04:19:20PM +0100, Ard Biesheuvel wrote:
> > > @@ -178,8 +186,8 @@ WatchdogSetTimerPeriod (
> > >
> > >// if TimerPeriod is 0, this is a request to stop the watchdog.
> > >if (TimerPeriod == 0) {
> > > -mNumTimerTicks = 0;
> > > -WatchdogDisable ();
> > > +//mNumTimerTicks = 0;
> > > +//WatchdogDisable ();
> >
> > Should these be deletions?
> >
> 
> Nope. I commented them out to test whether the watchdog actually
> fires, since most shell tools (and GRUB) disable it while running in
> interactive mode.
> 
> So this whole hunk should be dropped.

Works for me.

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


Re: [edk2] [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 14:43, Leif Lindholm  wrote:
>
> On Tue, Dec 18, 2018 at 02:10:13PM +0100, Ard Biesheuvel wrote:
> > Clean up the code, by adding missing STATIC modifiers, drop
> > redundant casts, and get rid of the 'success handling' anti
> > pattern in the entry point code.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 111 
> > +++-
> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf |  11 +-
> >  2 files changed, 64 insertions(+), 58 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > index 8ccf15366dfa..717a180a64ec 100644
> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > @@ -34,15 +34,16 @@
> >  #define TIME_UNITS_PER_SECOND 1000
> >
> >  // Tick frequency of the generic timer basis of the generic watchdog.
> > -UINTN mTimerFrequencyHz = 0;
> > +STATIC UINTN mTimerFrequencyHz = 0;
> >
> >  /* In cases where the compare register was set manually, information about
> > how long the watchdog was asked to wait cannot be retrieved from 
> > hardware.
> > It is therefore stored here. 0 means the timer is not running. */
> > -UINT64 mNumTimerTicks = 0;
> > +STATIC UINT64 mNumTimerTicks = 0;
> >
> > -EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
> > +STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
> >
> > +STATIC
> >  VOID
> >  WatchdogWriteOffsetRegister (
> >UINT32  Value
> > @@ -51,6 +52,7 @@ WatchdogWriteOffsetRegister (
> >MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
> >  }
> >
> > +STATIC
> >  VOID
> >  WatchdogWriteCompareRegister (
> >UINT64  Value
> > @@ -60,6 +62,7 @@ WatchdogWriteCompareRegister (
> >MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
> > MAX_UINT32);
> >  }
> >
> > +STATIC
> >  VOID
> >  WatchdogEnable (
> >VOID
> > @@ -68,6 +71,7 @@ WatchdogEnable (
> >MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);
> >  }
> >
> > +STATIC
> >  VOID
> >  WatchdogDisable (
> >VOID
> > @@ -79,6 +83,7 @@ WatchdogDisable (
> >  /** On exiting boot services we must make sure the Watchdog Timer
> >  is stopped.
> >  **/
> > +STATIC
> >  VOID
> >  EFIAPI
> >  WatchdogExitBootServicesEvent (
> > @@ -93,6 +98,7 @@ WatchdogExitBootServicesEvent (
> >  /* This function is called when the watchdog's first signal (WS0) goes 
> > high.
> > It uses the ResetSystem Runtime Service to reset the board.
> >  */
> > +STATIC
> >  VOID
> >  EFIAPI
> >  WatchdogInterruptHandler (
> > @@ -141,10 +147,11 @@ WatchdogInterruptHandler (
> >@retval EFI_UNSUPPORTED   The code does not support NotifyFunction.
> >
> >  **/
> > +STATIC
> >  EFI_STATUS
> >  EFIAPI
> >  WatchdogRegisterHandler (
> > -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> > +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
> >IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
> >)
> >  {
> > @@ -167,10 +174,11 @@ WatchdogRegisterHandler (
> >  in TimerPeriod 100ns units.
> >
> >  **/
> > +STATIC
> >  EFI_STATUS
> >  EFIAPI
> >  WatchdogSetTimerPeriod (
> > -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> > +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
> >IN UINT64   TimerPeriod   // In 100ns 
> > units
> >)
> >  {
> > @@ -178,8 +186,8 @@ WatchdogSetTimerPeriod (
> >
> >// if TimerPeriod is 0, this is a request to stop the watchdog.
> >if (TimerPeriod == 0) {
> > -mNumTimerTicks = 0;
> > -WatchdogDisable ();
> > +//mNumTimerTicks = 0;
> > +//WatchdogDisable ();
>
> Should these be deletions?
>

Nope. I commented them out to test whether the watchdog actually
fires, since most shell tools (and GRUB) disable it while running in
interactive mode.

So this whole hunk should be dropped.


> >  return EFI_SUCCESS;
> >}
> >
> > @@ -222,10 +230,11 @@ WatchdogSetTimerPeriod (
> >@retval EFI_INVALID_PARAMETER TimerPeriod is NULL.
> >
> >  **/
> > +STATIC
> >  EFI_STATUS
> >  EFIAPI
> >  WatchdogGetTimerPeriod (
> > -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> > +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
> >OUT UINT64  *TimerPeriod
> >)
> >  {
> > @@ -270,13 +279,13 @@ WatchdogGetTimerPeriod (
> >Retrieves the period of the timer interrupt in 100ns units.
> >
> >  **/
> > -EFI_WATCHDOG_TIMER_ARCH_PROTOCOLgWatchdogTimer = {
> > -  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
> > -  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
> > -  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
> > +STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {
> > +  

Re: [edk2] [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 02:10:13PM +0100, Ard Biesheuvel wrote:
> Clean up the code, by adding missing STATIC modifiers, drop
> redundant casts, and get rid of the 'success handling' anti
> pattern in the entry point code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 111 
> +++-
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf |  11 +-
>  2 files changed, 64 insertions(+), 58 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> index 8ccf15366dfa..717a180a64ec 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -34,15 +34,16 @@
>  #define TIME_UNITS_PER_SECOND 1000
>  
>  // Tick frequency of the generic timer basis of the generic watchdog.
> -UINTN mTimerFrequencyHz = 0;
> +STATIC UINTN mTimerFrequencyHz = 0;
>  
>  /* In cases where the compare register was set manually, information about
> how long the watchdog was asked to wait cannot be retrieved from hardware.
> It is therefore stored here. 0 means the timer is not running. */
> -UINT64 mNumTimerTicks = 0;
> +STATIC UINT64 mNumTimerTicks = 0;
>  
> -EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
> +STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
>  
> +STATIC
>  VOID
>  WatchdogWriteOffsetRegister (
>UINT32  Value
> @@ -51,6 +52,7 @@ WatchdogWriteOffsetRegister (
>MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
>  }
>  
> +STATIC
>  VOID
>  WatchdogWriteCompareRegister (
>UINT64  Value
> @@ -60,6 +62,7 @@ WatchdogWriteCompareRegister (
>MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
> MAX_UINT32);
>  }
>  
> +STATIC
>  VOID
>  WatchdogEnable (
>VOID
> @@ -68,6 +71,7 @@ WatchdogEnable (
>MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);
>  }
>  
> +STATIC
>  VOID
>  WatchdogDisable (
>VOID
> @@ -79,6 +83,7 @@ WatchdogDisable (
>  /** On exiting boot services we must make sure the Watchdog Timer
>  is stopped.
>  **/
> +STATIC
>  VOID
>  EFIAPI
>  WatchdogExitBootServicesEvent (
> @@ -93,6 +98,7 @@ WatchdogExitBootServicesEvent (
>  /* This function is called when the watchdog's first signal (WS0) goes high.
> It uses the ResetSystem Runtime Service to reset the board.
>  */
> +STATIC
>  VOID
>  EFIAPI
>  WatchdogInterruptHandler (
> @@ -141,10 +147,11 @@ WatchdogInterruptHandler (
>@retval EFI_UNSUPPORTED   The code does not support NotifyFunction.
>  
>  **/
> +STATIC
>  EFI_STATUS
>  EFIAPI
>  WatchdogRegisterHandler (
> -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> @@ -167,10 +174,11 @@ WatchdogRegisterHandler (
>  in TimerPeriod 100ns units.
>  
>  **/
> +STATIC
>  EFI_STATUS
>  EFIAPI
>  WatchdogSetTimerPeriod (
> -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
>IN UINT64   TimerPeriod   // In 100ns units
>)
>  {
> @@ -178,8 +186,8 @@ WatchdogSetTimerPeriod (
>  
>// if TimerPeriod is 0, this is a request to stop the watchdog.
>if (TimerPeriod == 0) {
> -mNumTimerTicks = 0;
> -WatchdogDisable ();
> +//mNumTimerTicks = 0;
> +//WatchdogDisable ();

Should these be deletions?

>  return EFI_SUCCESS;
>}
>  
> @@ -222,10 +230,11 @@ WatchdogSetTimerPeriod (
>@retval EFI_INVALID_PARAMETER TimerPeriod is NULL.
>  
>  **/
> +STATIC
>  EFI_STATUS
>  EFIAPI
>  WatchdogGetTimerPeriod (
> -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
>OUT UINT64  *TimerPeriod
>)
>  {
> @@ -270,13 +279,13 @@ WatchdogGetTimerPeriod (
>Retrieves the period of the timer interrupt in 100ns units.
>  
>  **/
> -EFI_WATCHDOG_TIMER_ARCH_PROTOCOLgWatchdogTimer = {
> -  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
> -  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
> -  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
> +STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {
> +  WatchdogRegisterHandler,
> +  WatchdogSetTimerPeriod,
> +  WatchdogGetTimerPeriod
>  };
>  
> -EFI_EVENT   EfiExitBootServicesEvent = 
> (EFI_EVENT)NULL;
> +STATIC EFI_EVENT mEfiExitBootServicesEvent;
>  
>  EFI_STATUS
>  EFIAPI
> @@ -288,6 +297,10 @@ GenericWatchdogEntry (
>EFI_STATUS  Status;
>EFI_HANDLE  Handle;
>  
> +  Status = gBS->LocateProtocol (, NULL,
> +  (VOID **));
> +  ASSERT_EFI_ERROR (Status);
> +
>/* Make sure 

Re: [edk2] [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement RegisterHandler() method

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 02:10:14PM +0100, Ard Biesheuvel wrote:
> Even though UEFI does not appear to use it, let's implement the
> complete PI watchdog protocol, including handler registration,
> which will be invoked instead of the ResetSystem() runtime service
> when the watchdog timer expires.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 
Thanks!

> ---
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34 
> ++--
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> index 717a180a64ec..21118a3c88d1 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0;
>  STATIC UINT64 mNumTimerTicks = 0;
>  
>  STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
>  
>  STATIC
>  VOID
> @@ -107,17 +108,25 @@ WatchdogInterruptHandler (
>)
>  {
>STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out.";
> +  UINT64  TimerPeriod;
>  
>WatchdogDisable ();
>  
>mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source);
>  
> -  gRT->ResetSystem (
> - EfiResetCold,
> - EFI_TIMEOUT,
> - StrSize (ResetString),
> - (VOID *) 
> - );
> +  //
> +  // The notify function should be called with the elapsed number of ticks
> +  // since the watchdog was armed, which should exceed the timer period.
> +  // We don't actually know the elapsed number of ticks, so let's return
> +  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
> mNumTimerTicks);
> +mWatchdogNotify (TimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString),
> +   (CHAR16 *)ResetString);
> +  }
>  
>// If we got here then the reset didn't work
>ASSERT (FALSE);
> @@ -155,9 +164,16 @@ WatchdogRegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The watchdog will reset the board
> -  return EFI_UNSUPPORTED;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
>  
>  /**
> -- 
> 2.17.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 02:10:12PM +0100, Ard Biesheuvel wrote:
> The SP805 watchdog driver doesn't implement the PI watchdog protocol
> fully, but always simply resets the system if the watchdog time runs
> out.
> 
> However, the hardware does support the intended usage model, as long
> as the SP805 is wired up correctly. So let's implement interrupt based
> mode involving a handler that is registered by the DXE core and invoked
> when the watchdog runs out. In the interrupt handler, we invoke the
> notify function if one was registered, or call the ResetSystem()
> runtime service otherwise (as per the UEFI spec)

The only question mark from my end is - what happens when the
interrupt isn't wired up correctly? Would it be worth to bail out and
refuse to register the driver if PcdSP805WatchdogInterrupt is set to
0?

Thomas?

/
Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95 
> ++--
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
>  3 files changed, 75 insertions(+), 27 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
> b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 5f67e7415469..44c00bd0c133 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -70,6 +70,7 @@
>## SP805 Watchdog
>gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x0023
>
> gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|UINT32|0x0021
> +  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x002E
>  
>## PL011 UART
>gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x001F
> diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
> b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> index 12c2f0a1fe49..4f09acf1fa28 100644
> --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> @@ -21,12 +21,17 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
> +#include 
>  #include 
>  
>  #include "SP805Watchdog.h"
>  
> -STATIC EFI_EVENT  mEfiExitBootServicesEvent;
> +STATIC EFI_EVENTmEfiExitBootServicesEvent;
> +STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
> +STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
> +STATIC UINT32   mTimerPeriod;
>  
>  /**
>Make sure the SP805 registers are unlocked for writing.
> @@ -65,6 +70,33 @@ SP805Lock (
>}
>  }
>  
> +STATIC
> +VOID
> +EFIAPI
> +SP805InterruptHandler (
> +  IN  HARDWARE_INTERRUPT_SOURCE   Source,
> +  IN  EFI_SYSTEM_CONTEXT  SystemContext
> +  )
> +{
> +  //
> +  // The notify function should be called with the elapsed number of ticks
> +  // since the watchdog was armed, which should exceed the timer period.
> +  // We don't actually know the elapsed number of ticks, so let's return
> +  // the timer period plus 1.
> +  //
> +  if (mWatchdogNotify != NULL) {
> +mWatchdogNotify (mTimerPeriod + 1);
> +  } else {
> +gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
> +  }
> +
> +  SP805Unlock ();
> +  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears the 
> irq
> +  SP805Lock ();
> +
> +  mInterrupt->EndOfInterrupt (mInterrupt, Source);
> +}
> +
>  /**
>Stop the SP805 watchdog timer from counting down by disabling interrupts.
>  **/
> @@ -149,9 +181,16 @@ SP805RegisterHandler (
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> -  // ERROR: This function is not supported.
> -  // The hardware watchdog will reset the board
> -  return EFI_INVALID_PARAMETER;
> +  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
> +return EFI_ALREADY_STARTED;
> +  }
> +
> +  mWatchdogNotify = NotifyFunction;
> +  return EFI_SUCCESS;
>  }
>  
>  /**
> @@ -202,19 +241,16 @@ SP805SetTimerPeriod (
>  SP805Stop ();
>} else {
>  // Calculate the Watchdog ticks required for a delay of (TimerTicks * 
> 100) nanoseconds
> -// The SP805 will count down to ZERO once, generate an interrupt and
> -// then it will again reload the initial value and start again.
> -// On the second time when it reaches ZERO, it will actually reset the 
> board.
> -// Therefore, we need to load half the required delay.
> +// The SP805 will count down to zero and generate an interrupt.
>  //
> -// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz) 
> / 2 ;
> +// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz);
>  //
>  // i.e.:
>  //
> -// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
> +// 

Re: [edk2] [PATCH 1/4] ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 02:10:11PM +0100, Ard Biesheuvel wrote:
> Before fixing the SP805 driver, let's clean it up a bit. No
> functional changes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 97 
> ++--
>  ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf | 13 +--
>  2 files changed, 53 insertions(+), 57 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
> b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> index 0a9f64095bf8..12c2f0a1fe49 100644
> --- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> +++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
> @@ -1,6 +1,7 @@
>  /** @file
>  *
>  *  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018, Linaro Limited. All rights reserved.
>  *
>  *  This program and the accompanying materials
>  *  are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -19,16 +20,13 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> -#include 
> -#include 
>  
>  #include 
>  
>  #include "SP805Watchdog.h"
>  
> -EFI_EVENT   EfiExitBootServicesEvent = 
> (EFI_EVENT)NULL;
> +STATIC EFI_EVENT  mEfiExitBootServicesEvent;
>  
>  /**
>Make sure the SP805 registers are unlocked for writing.
> @@ -43,8 +41,8 @@ SP805Unlock (
>VOID
>)
>  {
> -  if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED ) {
> -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE);
> +  if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED) {
> +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE);
>}
>  }
>  
> @@ -61,9 +59,9 @@ SP805Lock (
>VOID
>)
>  {
> -  if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED ) {
> +  if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED) {
>  // To lock it, just write in any number (except the special unlock code).
> -MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED);
> +MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED);
>}
>  }
>  
> @@ -77,8 +75,8 @@ SP805Stop (
>)
>  {
>// Disable interrupts
> -  if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0 ) {
> -MmioAnd32(SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN);
> +  if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0) {
> +MmioAnd32 (SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN);
>}
>  }
>  
> @@ -94,8 +92,8 @@ SP805Start (
>)
>  {
>// Enable interrupts
> -  if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0 ) {
> -MmioOr32(SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN);
> +  if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0) {
> +MmioOr32 (SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN);
>}
>  }
>  
> @@ -103,6 +101,7 @@ SP805Start (
>  On exiting boot services we must make sure the SP805 Watchdog Timer
>  is stopped.
>  **/
> +STATIC
>  VOID
>  EFIAPI
>  ExitBootServicesEvent (
> @@ -110,9 +109,9 @@ ExitBootServicesEvent (
>IN VOID   *Context
>)
>  {
> -  SP805Unlock();
> -  SP805Stop();
> -  SP805Lock();
> +  SP805Unlock ();
> +  SP805Stop ();
> +  SP805Lock ();
>  }
>  
>  /**
> @@ -142,10 +141,11 @@ ExitBootServicesEvent (
>  previously registered.
>  
>  **/
> +STATIC
>  EFI_STATUS
>  EFIAPI
>  SP805RegisterHandler (
> -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
>IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
>)
>  {
> @@ -182,22 +182,24 @@ SP805RegisterHandler (
>@retval EFI_DEVICE_ERROR  The timer period could not be changed due to 
> a device error.
>  
>  **/
> +STATIC
>  EFI_STATUS
>  EFIAPI
>  SP805SetTimerPeriod (
> -  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
> +  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
>IN UINT64   TimerPeriod   // In 100ns units
>)
>  {
> -  EFI_STATUS  Status = EFI_SUCCESS;
> +  EFI_STATUS  Status;
>UINT64  Ticks64bit;
>  
> -  SP805Unlock();
> +  SP805Unlock ();
>  
> -  if( TimerPeriod == 0 ) {
> +  Status = EFI_SUCCESS;
> +
> +  if (TimerPeriod == 0) {
>  // This is a watchdog stop request
> -SP805Stop();
> -goto EXIT;
> +SP805Stop ();
>} else {
>  // Calculate the Watchdog ticks required for a delay of (TimerTicks * 
> 100) nanoseconds
>  // The SP805 will count down to ZERO once, generate an interrupt and
> @@ -211,10 +213,11 @@ SP805SetTimerPeriod (
>  //
>  // WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
>  
> -Ticks64bit = DivU64x32(MultU64x32(TimerPeriod, 
> (UINTN)PcdGet32(PcdSP805WatchdogClockFrequencyInHz)), 2000);
> 

Re: [edk2] SP805 driver

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 12:57:22PM +, Udit Kumar wrote:
> > Are you referring to
> > MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf?
> 
> Yes !! 

:)

> > That is certainly what most of the platforms in edk2-platforms use.
> > 
> > The EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is used by core code.
> > 
> > If you want to use your hardware watchdog as part of your platform specific
> > code, that is absolutely fine and probably a very good idea - but it has 
> > nothing to
> > do with this protocol.  There is nothing forcing you to use the platform-
> > independent EFI_WATCHDOG_TIMER_ARCH_PROTOCOL for this.
> 
> Yes, this was idea to use 
> MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> and if needed, install a custom protocol for hardware wdt. And this to be 
> used by platform specific code.

Ah, yes - exactly!
This would be the ideal solution.

Regards,

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


[edk2] [PATCH 3/4] ArmPkg/GenericWatchdogDxe: clean up the code

2018-12-18 Thread Ard Biesheuvel
Clean up the code, by adding missing STATIC modifiers, drop
redundant casts, and get rid of the 'success handling' anti
pattern in the entry point code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c   | 111 
+++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf |  11 +-
 2 files changed, 64 insertions(+), 58 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 8ccf15366dfa..717a180a64ec 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -34,15 +34,16 @@
 #define TIME_UNITS_PER_SECOND 1000
 
 // Tick frequency of the generic timer basis of the generic watchdog.
-UINTN mTimerFrequencyHz = 0;
+STATIC UINTN mTimerFrequencyHz = 0;
 
 /* In cases where the compare register was set manually, information about
how long the watchdog was asked to wait cannot be retrieved from hardware.
It is therefore stored here. 0 means the timer is not running. */
-UINT64 mNumTimerTicks = 0;
+STATIC UINT64 mNumTimerTicks = 0;
 
-EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
+STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
 
+STATIC
 VOID
 WatchdogWriteOffsetRegister (
   UINT32  Value
@@ -51,6 +52,7 @@ WatchdogWriteOffsetRegister (
   MmioWrite32 (GENERIC_WDOG_OFFSET_REG, Value);
 }
 
+STATIC
 VOID
 WatchdogWriteCompareRegister (
   UINT64  Value
@@ -60,6 +62,7 @@ WatchdogWriteCompareRegister (
   MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
MAX_UINT32);
 }
 
+STATIC
 VOID
 WatchdogEnable (
   VOID
@@ -68,6 +71,7 @@ WatchdogEnable (
   MmioWrite32 (GENERIC_WDOG_CONTROL_STATUS_REG, GENERIC_WDOG_ENABLED);
 }
 
+STATIC
 VOID
 WatchdogDisable (
   VOID
@@ -79,6 +83,7 @@ WatchdogDisable (
 /** On exiting boot services we must make sure the Watchdog Timer
 is stopped.
 **/
+STATIC
 VOID
 EFIAPI
 WatchdogExitBootServicesEvent (
@@ -93,6 +98,7 @@ WatchdogExitBootServicesEvent (
 /* This function is called when the watchdog's first signal (WS0) goes high.
It uses the ResetSystem Runtime Service to reset the board.
 */
+STATIC
 VOID
 EFIAPI
 WatchdogInterruptHandler (
@@ -141,10 +147,11 @@ WatchdogInterruptHandler (
   @retval EFI_UNSUPPORTED   The code does not support NotifyFunction.
 
 **/
+STATIC
 EFI_STATUS
 EFIAPI
 WatchdogRegisterHandler (
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
   IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
   )
 {
@@ -167,10 +174,11 @@ WatchdogRegisterHandler (
 in TimerPeriod 100ns units.
 
 **/
+STATIC
 EFI_STATUS
 EFIAPI
 WatchdogSetTimerPeriod (
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
   IN UINT64   TimerPeriod   // In 100ns units
   )
 {
@@ -178,8 +186,8 @@ WatchdogSetTimerPeriod (
 
   // if TimerPeriod is 0, this is a request to stop the watchdog.
   if (TimerPeriod == 0) {
-mNumTimerTicks = 0;
-WatchdogDisable ();
+//mNumTimerTicks = 0;
+//WatchdogDisable ();
 return EFI_SUCCESS;
   }
 
@@ -222,10 +230,11 @@ WatchdogSetTimerPeriod (
   @retval EFI_INVALID_PARAMETER TimerPeriod is NULL.
 
 **/
+STATIC
 EFI_STATUS
 EFIAPI
 WatchdogGetTimerPeriod (
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
   OUT UINT64  *TimerPeriod
   )
 {
@@ -270,13 +279,13 @@ WatchdogGetTimerPeriod (
   Retrieves the period of the timer interrupt in 100ns units.
 
 **/
-EFI_WATCHDOG_TIMER_ARCH_PROTOCOLgWatchdogTimer = {
-  (EFI_WATCHDOG_TIMER_REGISTER_HANDLER)WatchdogRegisterHandler,
-  (EFI_WATCHDOG_TIMER_SET_TIMER_PERIOD)WatchdogSetTimerPeriod,
-  (EFI_WATCHDOG_TIMER_GET_TIMER_PERIOD)WatchdogGetTimerPeriod
+STATIC EFI_WATCHDOG_TIMER_ARCH_PROTOCOL mWatchdogTimer = {
+  WatchdogRegisterHandler,
+  WatchdogSetTimerPeriod,
+  WatchdogGetTimerPeriod
 };
 
-EFI_EVENT   EfiExitBootServicesEvent = (EFI_EVENT)NULL;
+STATIC EFI_EVENT mEfiExitBootServicesEvent;
 
 EFI_STATUS
 EFIAPI
@@ -288,6 +297,10 @@ GenericWatchdogEntry (
   EFI_STATUS  Status;
   EFI_HANDLE  Handle;
 
+  Status = gBS->LocateProtocol (, NULL,
+  (VOID **));
+  ASSERT_EFI_ERROR (Status);
+
   /* Make sure the Watchdog Timer Architectural Protocol has not been installed
  in the system yet.
  This will avoid conflicts with the universal watchdog */
@@ -296,51 +309,45 @@ GenericWatchdogEntry (
   mTimerFrequencyHz = ArmGenericTimerGetTimerFreq ();
   ASSERT (mTimerFrequencyHz != 0);
 
-  // Register for an ExitBootServicesEvent
-  Status = gBS->CreateEvent (
-  

[edk2] [PATCH 2/4] ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode

2018-12-18 Thread Ard Biesheuvel
The SP805 watchdog driver doesn't implement the PI watchdog protocol
fully, but always simply resets the system if the watchdog time runs
out.

However, the hardware does support the intended usage model, as long
as the SP805 is wired up correctly. So let's implement interrupt based
mode involving a handler that is registered by the DXE core and invoked
when the watchdog runs out. In the interrupt handler, we invoke the
notify function if one was registered, or call the ResetSystem()
runtime service otherwise (as per the UEFI spec)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmPlatformPkg.dec|  1 +
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 95 
++--
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf |  6 +-
 3 files changed, 75 insertions(+), 27 deletions(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index 5f67e7415469..44c00bd0c133 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -70,6 +70,7 @@
   ## SP805 Watchdog
   gArmPlatformTokenSpaceGuid.PcdSP805WatchdogBase|0x0|UINT32|0x0023
   
gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|32000|UINT32|0x0021
+  gArmPlatformTokenSpaceGuid.PcdSP805WatchdogInterrupt|0|UINT32|0x002E
 
   ## PL011 UART
   gArmPlatformTokenSpaceGuid.PL011UartClkInHz|2400|UINT32|0x001F
diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
index 12c2f0a1fe49..4f09acf1fa28 100644
--- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
+++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
@@ -21,12 +21,17 @@
 #include 
 #include 
 #include 
+#include 
 
+#include 
 #include 
 
 #include "SP805Watchdog.h"
 
-STATIC EFI_EVENT  mEfiExitBootServicesEvent;
+STATIC EFI_EVENTmEfiExitBootServicesEvent;
+STATIC EFI_HARDWARE_INTERRUPT_PROTOCOL  *mInterrupt;
+STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
+STATIC UINT32   mTimerPeriod;
 
 /**
   Make sure the SP805 registers are unlocked for writing.
@@ -65,6 +70,33 @@ SP805Lock (
   }
 }
 
+STATIC
+VOID
+EFIAPI
+SP805InterruptHandler (
+  IN  HARDWARE_INTERRUPT_SOURCE   Source,
+  IN  EFI_SYSTEM_CONTEXT  SystemContext
+  )
+{
+  //
+  // The notify function should be called with the elapsed number of ticks
+  // since the watchdog was armed, which should exceed the timer period.
+  // We don't actually know the elapsed number of ticks, so let's return
+  // the timer period plus 1.
+  //
+  if (mWatchdogNotify != NULL) {
+mWatchdogNotify (mTimerPeriod + 1);
+  } else {
+gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, 0, NULL);
+  }
+
+  SP805Unlock ();
+  MmioWrite32 (SP805_WDOG_INT_CLR_REG, 0); // write of any value clears the irq
+  SP805Lock ();
+
+  mInterrupt->EndOfInterrupt (mInterrupt, Source);
+}
+
 /**
   Stop the SP805 watchdog timer from counting down by disabling interrupts.
 **/
@@ -149,9 +181,16 @@ SP805RegisterHandler (
   IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
   )
 {
-  // ERROR: This function is not supported.
-  // The hardware watchdog will reset the board
-  return EFI_INVALID_PARAMETER;
+  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
+return EFI_ALREADY_STARTED;
+  }
+
+  mWatchdogNotify = NotifyFunction;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -202,19 +241,16 @@ SP805SetTimerPeriod (
 SP805Stop ();
   } else {
 // Calculate the Watchdog ticks required for a delay of (TimerTicks * 100) 
nanoseconds
-// The SP805 will count down to ZERO once, generate an interrupt and
-// then it will again reload the initial value and start again.
-// On the second time when it reaches ZERO, it will actually reset the 
board.
-// Therefore, we need to load half the required delay.
+// The SP805 will count down to zero and generate an interrupt.
 //
-// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz) / 
2 ;
+// WatchdogTicks = ((TimerPeriod * 100 * SP805_CLOCK_FREQUENCY) / 1GHz);
 //
 // i.e.:
 //
-// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
+// WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 10 MHz ;
 
 Ticks64bit = MultU64x32 (TimerPeriod, PcdGet32 
(PcdSP805WatchdogClockFrequencyInHz));
-Ticks64bit = DivU64x32 (Ticks64bit, 2000);
+Ticks64bit = DivU64x32 (Ticks64bit, 10 * 1000 * 1000);
 
 // The registers in the SP805 are only 32 bits
 if (Ticks64bit > MAX_UINT32) {
@@ -233,9 +269,12 @@ SP805SetTimerPeriod (
 SP805Start ();
   }
 
+  mTimerPeriod = TimerPeriod;
+
 EXIT:
   // Ensure the watchdog is locked before exiting.
   SP805Lock ();
+  

[edk2] [PATCH 1/4] ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup

2018-12-18 Thread Ard Biesheuvel
Before fixing the SP805 driver, let's clean it up a bit. No
functional changes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 97 
++--
 ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf | 13 +--
 2 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c 
b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
index 0a9f64095bf8..12c2f0a1fe49 100644
--- a/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
+++ b/ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805Watchdog.c
@@ -1,6 +1,7 @@
 /** @file
 *
 *  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+*  Copyright (c) 2018, Linaro Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD 
License
@@ -19,16 +20,13 @@
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
-#include 
 
 #include 
 
 #include "SP805Watchdog.h"
 
-EFI_EVENT   EfiExitBootServicesEvent = (EFI_EVENT)NULL;
+STATIC EFI_EVENT  mEfiExitBootServicesEvent;
 
 /**
   Make sure the SP805 registers are unlocked for writing.
@@ -43,8 +41,8 @@ SP805Unlock (
   VOID
   )
 {
-  if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED ) {
-MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE);
+  if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_LOCKED) {
+MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_SPECIAL_UNLOCK_CODE);
   }
 }
 
@@ -61,9 +59,9 @@ SP805Lock (
   VOID
   )
 {
-  if( MmioRead32(SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED ) {
+  if (MmioRead32 (SP805_WDOG_LOCK_REG) == SP805_WDOG_LOCK_IS_UNLOCKED) {
 // To lock it, just write in any number (except the special unlock code).
-MmioWrite32(SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED);
+MmioWrite32 (SP805_WDOG_LOCK_REG, SP805_WDOG_LOCK_IS_LOCKED);
   }
 }
 
@@ -77,8 +75,8 @@ SP805Stop (
   )
 {
   // Disable interrupts
-  if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0 ) {
-MmioAnd32(SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN);
+  if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) != 0) {
+MmioAnd32 (SP805_WDOG_CONTROL_REG, ~SP805_WDOG_CTRL_INTEN);
   }
 }
 
@@ -94,8 +92,8 @@ SP805Start (
   )
 {
   // Enable interrupts
-  if ( (MmioRead32(SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0 ) {
-MmioOr32(SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN);
+  if ((MmioRead32 (SP805_WDOG_CONTROL_REG) & SP805_WDOG_CTRL_INTEN) == 0) {
+MmioOr32 (SP805_WDOG_CONTROL_REG, SP805_WDOG_CTRL_INTEN);
   }
 }
 
@@ -103,6 +101,7 @@ SP805Start (
 On exiting boot services we must make sure the SP805 Watchdog Timer
 is stopped.
 **/
+STATIC
 VOID
 EFIAPI
 ExitBootServicesEvent (
@@ -110,9 +109,9 @@ ExitBootServicesEvent (
   IN VOID   *Context
   )
 {
-  SP805Unlock();
-  SP805Stop();
-  SP805Lock();
+  SP805Unlock ();
+  SP805Stop ();
+  SP805Lock ();
 }
 
 /**
@@ -142,10 +141,11 @@ ExitBootServicesEvent (
 previously registered.
 
 **/
+STATIC
 EFI_STATUS
 EFIAPI
 SP805RegisterHandler (
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
   IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
   )
 {
@@ -182,22 +182,24 @@ SP805RegisterHandler (
   @retval EFI_DEVICE_ERROR  The timer period could not be changed due to a 
device error.
 
 **/
+STATIC
 EFI_STATUS
 EFIAPI
 SP805SetTimerPeriod (
-  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_ARCH_PROTOCOL *This,
   IN UINT64   TimerPeriod   // In 100ns units
   )
 {
-  EFI_STATUS  Status = EFI_SUCCESS;
+  EFI_STATUS  Status;
   UINT64  Ticks64bit;
 
-  SP805Unlock();
+  SP805Unlock ();
 
-  if( TimerPeriod == 0 ) {
+  Status = EFI_SUCCESS;
+
+  if (TimerPeriod == 0) {
 // This is a watchdog stop request
-SP805Stop();
-goto EXIT;
+SP805Stop ();
   } else {
 // Calculate the Watchdog ticks required for a delay of (TimerTicks * 100) 
nanoseconds
 // The SP805 will count down to ZERO once, generate an interrupt and
@@ -211,10 +213,11 @@ SP805SetTimerPeriod (
 //
 // WatchdogTicks = (TimerPeriod * SP805_CLOCK_FREQUENCY) / 20 MHz ;
 
-Ticks64bit = DivU64x32(MultU64x32(TimerPeriod, 
(UINTN)PcdGet32(PcdSP805WatchdogClockFrequencyInHz)), 2000);
+Ticks64bit = MultU64x32 (TimerPeriod, PcdGet32 
(PcdSP805WatchdogClockFrequencyInHz));
+Ticks64bit = DivU64x32 (Ticks64bit, 2000);
 
 // The registers in the SP805 are only 32 bits
-if(Ticks64bit > (UINT64)0x) {
+if (Ticks64bit > MAX_UINT32) {
   // We could load the watchdog with the maximum supported value but
   // if a smaller value was 

[edk2] [PATCH 0/4] ArmPkg, ArmPlatformPkg: watchdog driver cleanup

2018-12-18 Thread Ard Biesheuvel
This series cleans up the code of the two watchdog drivers we have for
ARM systems, and brings them in compliance with the PI spec, which
specifies that the default action of the watchdog can be overridden
by registering a handler.

Note that the TC2 code in edk2-platforms will have to be brought up to
date. The SP805 on the FVP model seems terminally broken (it is 'wired'
to the 24 MHz APB clock instead of the 32 kHz WDOG clock, so I'll switch
that one over to use the SBSA watchdog instead)

Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Thomas Panakamattam Abraham 
Cc: Meenakshi Aggarwal 
Cc: Udit Kumar 
Cc: Matteo Carlini 
Cc: Nariman Poushin 

Ard Biesheuvel (4):
  ArmPlatformPkg/SP805WatchdogDxe: cosmetic cleanup
  ArmPlatformPkg/SP805WatchdogDxe: switch to interrupt mode
  ArmPkg/GenericWatchdogDxe: clean up the code
  ArmPkg/GenericWatchdogDxe: implement RegisterHandler() method

 .../GenericWatchdogDxe/GenericWatchdogDxe.c   | 145 --
 .../GenericWatchdogDxe/GenericWatchdogDxe.inf |  11 +-
 ArmPlatformPkg/ArmPlatformPkg.dec |   1 +
 .../Drivers/SP805WatchdogDxe/SP805Watchdog.c  | 182 +++---
 .../SP805WatchdogDxe/SP805WatchdogDxe.inf |  17 +-
 5 files changed, 211 insertions(+), 145 deletions(-)

-- 
2.17.1

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


[edk2] [PATCH 4/4] ArmPkg/GenericWatchdogDxe: implement RegisterHandler() method

2018-12-18 Thread Ard Biesheuvel
Even though UEFI does not appear to use it, let's implement the
complete PI watchdog protocol, including handler registration,
which will be invoked instead of the ResetSystem() runtime service
when the watchdog timer expires.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 34 
++--
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 717a180a64ec..21118a3c88d1 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -42,6 +42,7 @@ STATIC UINTN mTimerFrequencyHz = 0;
 STATIC UINT64 mNumTimerTicks = 0;
 
 STATIC EFI_HARDWARE_INTERRUPT2_PROTOCOL *mInterruptProtocol;
+STATIC EFI_WATCHDOG_TIMER_NOTIFYmWatchdogNotify;
 
 STATIC
 VOID
@@ -107,17 +108,25 @@ WatchdogInterruptHandler (
   )
 {
   STATIC CONST CHAR16 ResetString[]= L"The generic watchdog timer ran out.";
+  UINT64  TimerPeriod;
 
   WatchdogDisable ();
 
   mInterruptProtocol->EndOfInterrupt (mInterruptProtocol, Source);
 
-  gRT->ResetSystem (
- EfiResetCold,
- EFI_TIMEOUT,
- StrSize (ResetString),
- (VOID *) 
- );
+  //
+  // The notify function should be called with the elapsed number of ticks
+  // since the watchdog was armed, which should exceed the timer period.
+  // We don't actually know the elapsed number of ticks, so let's return
+  // the timer period plus 1.
+  //
+  if (mWatchdogNotify != NULL) {
+TimerPeriod = ((TIME_UNITS_PER_SECOND / mTimerFrequencyHz) * 
mNumTimerTicks);
+mWatchdogNotify (TimerPeriod + 1);
+  } else {
+gRT->ResetSystem (EfiResetCold, EFI_TIMEOUT, StrSize (ResetString),
+   (CHAR16 *)ResetString);
+  }
 
   // If we got here then the reset didn't work
   ASSERT (FALSE);
@@ -155,9 +164,16 @@ WatchdogRegisterHandler (
   IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
   )
 {
-  // ERROR: This function is not supported.
-  // The watchdog will reset the board
-  return EFI_UNSUPPORTED;
+  if (mWatchdogNotify == NULL && NotifyFunction == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (mWatchdogNotify != NULL && NotifyFunction != NULL) {
+return EFI_ALREADY_STARTED;
+  }
+
+  mWatchdogNotify = NotifyFunction;
+  return EFI_SUCCESS;
 }
 
 /**
-- 
2.17.1

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


Re: [edk2] [PATCH edk2-non-osi 1/1] Platform/Socionext: update ARM-TF binary to include OP-TEE

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 14:04, Sumit Garg  wrote:
>
> On Tue, 18 Dec 2018 at 18:30, Ard Biesheuvel  
> wrote:
> >
> > On Tue, 18 Dec 2018 at 13:58, Sumit Garg  wrote:
> > >
> > > On Tue, 18 Dec 2018 at 18:16, Ard Biesheuvel  
> > > wrote:
> > > >
> > > > On Tue, 18 Dec 2018 at 13:09, Sumit Garg  wrote:
> > > > >
> > > > > Include a prebuilt binary of OP-TEE OS built from commit
> > > > > a5d528c7e54fd7726230483bd4cd5c4786d7703f.
> > > > > (https://github.com/OP-TEE/optee_os.git master)
> > > > >
> > > > > Also update ARM-TF RELEASE build to commit 47577cbaaf4b.
> > > > >
> > > > > Cc: Ard Biesheuvel 
> > > > > Cc: Leif Lindholm 
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Sumit Garg 
> > > > > ---
> > > > >  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 46824 -> 
> > > > > 415251 bytes
> > > >
> > > > Thanks Sumit!
> > > >
> > > > In the future, please use --no-binary when sending patches like this 
> > > > one.
> > > >
> > >
> > > Sure will take care of this in future.
> > >
> > > > However, do you have any explanation why the size increases 10x? This
> > > > patch adds the pseudo-TA, but OP-TEE was already included in the
> > > > previous build, so this is a bit unexpected.
> > > >
> > >
> > > Actually I think ARM-TF was not updated earlier to include OP-TEE in
> > > upstream (tee.bin size: 316392 bytes).
> > >
> >
> > Ah, right. So we added the OP-TEE aware ATF build but not OP-TEE
> > itself? That would indeed explain it.
>
> We didn't updated ATF build at all in upstream. I think you have
> updated ATF build in local repo [1] only.
>
> [1] git://git.linaro.org/leg/noupstream/edk2-non-osi.git
>

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


Re: [edk2] [PATCH edk2-non-osi 1/1] Platform/Socionext: update ARM-TF binary to include OP-TEE

2018-12-18 Thread Sumit Garg
On Tue, 18 Dec 2018 at 18:30, Ard Biesheuvel  wrote:
>
> On Tue, 18 Dec 2018 at 13:58, Sumit Garg  wrote:
> >
> > On Tue, 18 Dec 2018 at 18:16, Ard Biesheuvel  
> > wrote:
> > >
> > > On Tue, 18 Dec 2018 at 13:09, Sumit Garg  wrote:
> > > >
> > > > Include a prebuilt binary of OP-TEE OS built from commit
> > > > a5d528c7e54fd7726230483bd4cd5c4786d7703f.
> > > > (https://github.com/OP-TEE/optee_os.git master)
> > > >
> > > > Also update ARM-TF RELEASE build to commit 47577cbaaf4b.
> > > >
> > > > Cc: Ard Biesheuvel 
> > > > Cc: Leif Lindholm 
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Sumit Garg 
> > > > ---
> > > >  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 46824 -> 
> > > > 415251 bytes
> > >
> > > Thanks Sumit!
> > >
> > > In the future, please use --no-binary when sending patches like this one.
> > >
> >
> > Sure will take care of this in future.
> >
> > > However, do you have any explanation why the size increases 10x? This
> > > patch adds the pseudo-TA, but OP-TEE was already included in the
> > > previous build, so this is a bit unexpected.
> > >
> >
> > Actually I think ARM-TF was not updated earlier to include OP-TEE in
> > upstream (tee.bin size: 316392 bytes).
> >
>
> Ah, right. So we added the OP-TEE aware ATF build but not OP-TEE
> itself? That would indeed explain it.

We didn't updated ATF build at all in upstream. I think you have
updated ATF build in local repo [1] only.

[1] git://git.linaro.org/leg/noupstream/edk2-non-osi.git

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


Re: [edk2] [PATCH edk2-non-osi 1/1] Platform/Socionext: update ARM-TF binary to include OP-TEE

2018-12-18 Thread Sumit Garg
On Tue, 18 Dec 2018 at 18:16, Ard Biesheuvel  wrote:
>
> On Tue, 18 Dec 2018 at 13:09, Sumit Garg  wrote:
> >
> > Include a prebuilt binary of OP-TEE OS built from commit
> > a5d528c7e54fd7726230483bd4cd5c4786d7703f.
> > (https://github.com/OP-TEE/optee_os.git master)
> >
> > Also update ARM-TF RELEASE build to commit 47577cbaaf4b.
> >
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Sumit Garg 
> > ---
> >  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 46824 -> 415251 
> > bytes
>
> Thanks Sumit!
>
> In the future, please use --no-binary when sending patches like this one.
>

Sure will take care of this in future.

> However, do you have any explanation why the size increases 10x? This
> patch adds the pseudo-TA, but OP-TEE was already included in the
> previous build, so this is a bit unexpected.
>

Actually I think ARM-TF was not updated earlier to include OP-TEE in
upstream (tee.bin size: 316392 bytes).

-Sumit

>
>
>
> >  1 file changed, 0 insertions(+), 0 deletions(-)
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-non-osi 1/1] Platform/Socionext: update ARM-TF binary to include OP-TEE

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 13:58, Sumit Garg  wrote:
>
> On Tue, 18 Dec 2018 at 18:16, Ard Biesheuvel  
> wrote:
> >
> > On Tue, 18 Dec 2018 at 13:09, Sumit Garg  wrote:
> > >
> > > Include a prebuilt binary of OP-TEE OS built from commit
> > > a5d528c7e54fd7726230483bd4cd5c4786d7703f.
> > > (https://github.com/OP-TEE/optee_os.git master)
> > >
> > > Also update ARM-TF RELEASE build to commit 47577cbaaf4b.
> > >
> > > Cc: Ard Biesheuvel 
> > > Cc: Leif Lindholm 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Sumit Garg 
> > > ---
> > >  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 46824 -> 415251 
> > > bytes
> >
> > Thanks Sumit!
> >
> > In the future, please use --no-binary when sending patches like this one.
> >
>
> Sure will take care of this in future.
>
> > However, do you have any explanation why the size increases 10x? This
> > patch adds the pseudo-TA, but OP-TEE was already included in the
> > previous build, so this is a bit unexpected.
> >
>
> Actually I think ARM-TF was not updated earlier to include OP-TEE in
> upstream (tee.bin size: 316392 bytes).
>

Ah, right. So we added the OP-TEE aware ATF build but not OP-TEE
itself? That would indeed explain it.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] SP805 driver

2018-12-18 Thread Udit Kumar
> > > > Coming back to hardware, which does not have mask around wdt, how
> > > > to implement this feature.
> > >
> > > Simple - you can't.
> > >
> > > You can absolutely implement exactly the functionality you have
> > > today, with minimal changes to the protocol - it just should not be
> > > registered as an implementation of
> EFI_WATCHDOG_TIMER_ARCH_PROTOCOL.
> >
> > I believe,  EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is must Are you
> > suggesting to EFI_WATCHDOG_TIMER_ARCH_PROTOCOL from
> MdeModulePkg and
> > hook platform specific code with this.
> > Or simply register EFI_WATCHDOG_TIMER_ARCH_PROTOCOL with dummy
> functions.
> 
> Are you referring to
> MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf?

Yes !! 
 
> That is certainly what most of the platforms in edk2-platforms use.
> 
> The EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is used by core code.
> 
> If you want to use your hardware watchdog as part of your platform specific
> code, that is absolutely fine and probably a very good idea - but it has 
> nothing to
> do with this protocol.  There is nothing forcing you to use the platform-
> independent EFI_WATCHDOG_TIMER_ARCH_PROTOCOL for this.

Yes, this was idea to use 
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
and if needed, install a custom protocol for hardware wdt. And this to be used 
by platform specific code.

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


Re: [edk2] SP805 driver

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 12:30:34PM +, Udit Kumar wrote:
> > > >"If no handler has been registered, or the registered handler
> > > >returns, then the system will be reset by calling the Runtime Service
> > > >ResetSystem()."
> > > I believe,  this handler needs to be called by wdt driver after wdt is
> > > expired. Meaning , we want wdt to work without resetting the system.
> > > Therefore a mask is needed with wdt, which will prevent to reset the
> > > system.   I see it working, at least for SP805, because of PMU mask
> > > bits.
> > 
> > Yes, if the WDT can be configured to generate an interrupt instead of a
> > hardware reset, it can be used to implement this protocol.
> 
> Here I am just thinking of one condition , some application started the wdt
> and CPU got stuck somewhere in ISR routine, 
> Now this wdt is expired. We will end up in hang system. 
> I agree doing reset in graceful manner is always great, but In this case, 
> resetting the 
> as it is, will be more useful. 
> 
> Thought ? 

The short answer is:
That would violate the specification, so what is the purpose of
debating the merit?

The longer answer is:
The comparison conflates software and hardware watchdogs. There is
nothing saying a system couldn't have both.

But giving someone a hardware watchdog when they explicitly ask for a
software one is not OK.

> > > Also ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c has this
> > limitation.
> > > I haven’t read spec of this wdt.  I hope there should a mask around.
> > 
> > No, the situation for the GenericWatchdogDxe is not as dire.
> > 
> > Correct: it does not permit registering software handlers (which perhaps we
> > should do something about, but is ... acceptable).
> > 
> > _But_, it still conforms to the above text; when the timer expires, it 
> > resets the
> > system by calling the ResetSystem() service. It does not directly force a
> > hardware reset.
> 
> Hmm, this does partly by calling  ResetSystem(), however this does not 
> Allow to install handler in WatchdogRegisterHandler.

As I said, it's acceptable - it's not ideal.

> > The severe problem is not the lack of the ability to register the software 
> > handler
> > (which does remove much of the utility), but the removal of reset control 
> > from
> > the system firmware.
> > 
> > > Coming back to hardware, which does not have mask around wdt, how to
> > > implement this feature.
> > 
> > Simple - you can't.
> > 
> > You can absolutely implement exactly the functionality you have today, with
> > minimal changes to the protocol - it just should not be registered as an
> > implementation of EFI_WATCHDOG_TIMER_ARCH_PROTOCOL.
> 
> I believe,  EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is must 
> Are you suggesting to EFI_WATCHDOG_TIMER_ARCH_PROTOCOL from MdeModulePkg
> and hook platform specific code with this.
> Or simply register EFI_WATCHDOG_TIMER_ARCH_PROTOCOL with dummy functions.

Are you referring to
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf?

That is certainly what most of the platforms in edk2-platforms use.

The EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is used by core code.

If you want to use your hardware watchdog as part of your platform
specific code, that is absolutely fine and probably a very good idea -
but it has nothing to do with this protocol. There is nothing forcing
you to use the platform-independent EFI_WATCHDOG_TIMER_ARCH_PROTOCOL
for this.

Regards,

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


Re: [edk2] [PATCH edk2-platforms v1 1/1] Silicon/SynQuacer: add OP-TEE based RNG driver

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 13:17, Sumit Garg  wrote:
>
> Hi Ard,
>
> On Wed, 12 Dec 2018 at 11:05, Sumit Garg  wrote:
> >
> > On Tue, 11 Dec 2018 at 21:37, Ard Biesheuvel  
> > wrote:
> > >
> > > On Tue, 11 Dec 2018 at 10:06, Ard Biesheuvel  
> > > wrote:
> > > >
> > > > On Tue, 11 Dec 2018 at 10:04, Sumit Garg  wrote:
> > > > >
> > > > > On Tue, 11 Dec 2018 at 13:56, Ard Biesheuvel 
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, 11 Dec 2018 at 08:46, Sumit Garg  
> > > > > > wrote:
> > > > > > >
> > > > > > > This driver uses OpteeLib to interface with OP-TEE based RNG 
> > > > > > > service
> > > > > > > (pseudo trusted application) to implement EFI_RNG_PROTOCOL that 
> > > > > > > is used
> > > > > > > to seed kernel entropy pool.
> > > > > > >
> > > > > > > Cc: Ard Biesheuvel 
> > > > > > > Cc: Leif Lindholm 
> > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > > Signed-off-by: Sumit Garg 
> > > > > >
> > > > > > Excellent! Happy to see this code going upstream.
> > > > > >
> > > > >
> > > > > Thanks Ard.
> > > > >
> > > > > > This code looks fine to me, but I'd like to test it with an updated
> > > > > > OP-TEE build first before I merge it.
> > > > > >
> > > > >
> > > > > Agree, will wait for OP-TEE PR to be merged. BTW, if you need I could
> > > > > share updated "fip_all_arm_tf.bin" using OP-TEE PR for testing.
> > > > >
> > > >
> > > > Yes please, that would be useful.
> > > >
> > > >
> > >
> > > OK, I successfully tested this patch on my SynQuacer box. Very nice work!
> > >
> >
> > Thanks Ard for testing this patch.
> >
> > > So this is ready to go in as soon as the OP-TEE side is merged as
> > > well, so please keep me informed about that (and contribute a new
> > > ARM-TF + OP-TEE build to edk2-non-osi)
> >
> > Sure will let you know about OP-TEE PR merge and yes will contribute a
> > new ARM-TF + OP-TEE build to edk2-non-osi.
> >
>
> OP-TEE PR [1] has been merged. Also I have sent a patch to contribute
> new ARM-TF + OP-TEE build. Please have a look.
>
> [1] https://github.com/OP-TEE/optee_os/pull/2564
>

Wonderful! Thanks a lot.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-non-osi 1/1] Platform/Socionext: update ARM-TF binary to include OP-TEE

2018-12-18 Thread Ard Biesheuvel
On Tue, 18 Dec 2018 at 13:09, Sumit Garg  wrote:
>
> Include a prebuilt binary of OP-TEE OS built from commit
> a5d528c7e54fd7726230483bd4cd5c4786d7703f.
> (https://github.com/OP-TEE/optee_os.git master)
>
> Also update ARM-TF RELEASE build to commit 47577cbaaf4b.
>
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sumit Garg 
> ---
>  Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin | Bin 46824 -> 415251 
> bytes

Thanks Sumit!

In the future, please use --no-binary when sending patches like this one.

However, do you have any explanation why the size increases 10x? This
patch adds the pseudo-TA, but OP-TEE was already included in the
previous build, so this is a bit unexpected.




>  1 file changed, 0 insertions(+), 0 deletions(-)
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH edk2-platforms 03/41] SocLib : Add support for initialization of peripherals

2018-12-18 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 08:31:17PM +0530, Meenakshi Aggarwal wrote:
> Add SocInit function that initializes peripherals
> and print board and soc information.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  Silicon/NXP/Include/Chassis2/SerDes.h|  68 +
>  Silicon/NXP/Include/Chassis2/Soc.h   | 367 ++
>  Silicon/NXP/LS1043A/Include/SocSerDes.h  |  57 
>  Silicon/NXP/Library/SocLib/Chassis.c | 372 
> +++
>  Silicon/NXP/Library/SocLib/Chassis.h | 144 +++
>  Silicon/NXP/Library/SocLib/Chassis2/Soc.c| 167 
>  Silicon/NXP/Library/SocLib/LS1043aSocLib.inf |  49 
>  Silicon/NXP/Library/SocLib/SerDes.c  | 271 +++
>  8 files changed, 1495 insertions(+)
>  create mode 100644 Silicon/NXP/Include/Chassis2/SerDes.h
>  create mode 100644 Silicon/NXP/Include/Chassis2/Soc.h
>  create mode 100644 Silicon/NXP/LS1043A/Include/SocSerDes.h
>  create mode 100644 Silicon/NXP/Library/SocLib/Chassis.c
>  create mode 100644 Silicon/NXP/Library/SocLib/Chassis.h
>  create mode 100644 Silicon/NXP/Library/SocLib/Chassis2/Soc.c
>  create mode 100644 Silicon/NXP/Library/SocLib/LS1043aSocLib.inf
>  create mode 100644 Silicon/NXP/Library/SocLib/SerDes.c
> 
> diff --git a/Silicon/NXP/Include/Chassis2/SerDes.h 
> b/Silicon/NXP/Include/Chassis2/SerDes.h
> new file mode 100644
> index 000..4c874aa
> --- /dev/null
> +++ b/Silicon/NXP/Include/Chassis2/SerDes.h
> @@ -0,0 +1,68 @@
> +/** SerDes.h
> + The Header file of SerDes Module for Chassis 2
> +
> + Copyright 2017 NXP
> +
> + 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
> + 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 __SERDES_H__
> +#define __SERDES_H__

SERDES is a bit too generic. Please add an NXP_ prefix, or more if you
can come up with something that pins it down to the group of users of
this header (QORIQ?).

> +
> +#include 
> +
> +#define SRDS_MAX_LANES 4
> +
> +typedef enum {
> +  NONE = 0,
> +  PCIE1,
> +  PCIE2,
> +  PCIE3,
> +  SATA,
> +  SGMII_FM1_DTSEC1,
> +  SGMII_FM1_DTSEC2,
> +  SGMII_FM1_DTSEC5,
> +  SGMII_FM1_DTSEC6,
> +  SGMII_FM1_DTSEC9,
> +  SGMII_FM1_DTSEC10,
> +  QSGMII_FM1_A,
> +  XFI_FM1_MAC9,
> +  XFI_FM1_MAC10,
> +  SGMII_2500_FM1_DTSEC2,
> +  SGMII_2500_FM1_DTSEC5,
> +  SGMII_2500_FM1_DTSEC9,
> +  SGMII_2500_FM1_DTSEC10,
> +  SERDES_PRTCL_COUNT

As per
https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/56_declarations_and_types.html#5622-enumerated-types
please rename enum members to CamelCase. (Please apply throughout set.)

Yes, I missed this in my previous review - apologies for that.

> +} SERDES_PROTOCOL;
> +
> +typedef enum {
> +  SRDS_1  = 0,
> +  SRDS_2,
> +  SRDS_MAX_NUM
> +} SERDES_NUMBER;
> +
> +typedef struct {
> +  UINT16 Protocol;
> +  UINT8  SrdsLane[SRDS_MAX_LANES];
> +} SERDES_CONFIG;
> +
> +typedef VOID
> +(*SERDES_PROBE_LANES_CALLBACK) (
> +  IN SERDES_PROTOCOL LaneProtocol,
> +  IN VOID *Arg
> +  );
> +
> +VOID
> +SerDesProbeLanes(
> +  IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback,
> +  IN VOID *Arg
> +  );
> +
> +#endif /* __SERDES_H */
> diff --git a/Silicon/NXP/Include/Chassis2/Soc.h 
> b/Silicon/NXP/Include/Chassis2/Soc.h
> new file mode 100644
> index 000..10e99ab
> --- /dev/null
> +++ b/Silicon/NXP/Include/Chassis2/Soc.h
> @@ -0,0 +1,367 @@
> +/** Soc.h
> +*  Header defining the Base addresses, sizes, flags etc for chassis 1
> +*
> +*  Copyright 2017 NXP
> +*
> +*  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 __SOC_H__
> +#define __SOC_H__

NXP_PREFIX please.

> +
> +#define HWA_CGA_M1_CLK_SEL 0xe000
> +#define HWA_CGA_M1_CLK_SHIFT   29
> +
> +#define TP_CLUSTER_EOC_MASK0xc000  /* end of clusters mask */
> +#define NUM_CC_PLLS2
> +#define CLK_FREQ   1
> +#define MAX_CPUS   4
> +#define NUM_FMAN   1
> +#define CHECK_CLUSTER(Cluster)((Cluster & TP_CLUSTER_EOC_MASK) == 0x0)
> +
> +/* RCW SERDES MACRO */
> +#define RCWSR_INDEX4
> +#define RCWSR_SRDS1_PRTCL_MASK 0x
> 

Re: [edk2] SP805 driver

2018-12-18 Thread Udit Kumar


> -Original Message-
> From: Leif Lindholm 
> Sent: Tuesday, December 18, 2018 2:56 PM


> 
> On Tue, Dec 18, 2018 at 05:20:59AM +, Udit Kumar wrote:
> > Thanks for note Leif,
> >
> > I am trying to understand this behavior  of specification.
> >
> > >"If no handler has been registered, or the registered handler
> > >returns, then the system will be reset by calling the Runtime Service
> > >ResetSystem()."
> > I believe,  this handler needs to be called by wdt driver after wdt is
> > expired. Meaning , we want wdt to work without resetting the system.
> > Therefore a mask is needed with wdt, which will prevent to reset the
> > system.   I see it working, at least for SP805, because of PMU mask
> > bits.
> 
> Yes, if the WDT can be configured to generate an interrupt instead of a
> hardware reset, it can be used to implement this protocol.

Here I am just thinking of one condition , some application started the wdt
and CPU got stuck somewhere in ISR routine, 
Now this wdt is expired. We will end up in hang system. 
I agree doing reset in graceful manner is always great, but In this case, 
resetting the 
as it is, will be more useful. 

Thought ? 
 
> > Also ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c has this
> limitation.
> > I haven’t read spec of this wdt.  I hope there should a mask around.
> 
> No, the situation for the GenericWatchdogDxe is not as dire.
> 
> Correct: it does not permit registering software handlers (which perhaps we
> should do something about, but is ... acceptable).
> 
> _But_, it still conforms to the above text; when the timer expires, it resets 
> the
> system by calling the ResetSystem() service. It does not directly force a
> hardware reset.

Hmm, this does partly by calling  ResetSystem(), however this does not 
Allow to install handler in WatchdogRegisterHandler.

> The severe problem is not the lack of the ability to register the software 
> handler
> (which does remove much of the utility), but the removal of reset control from
> the system firmware.
> 
> > Coming back to hardware, which does not have mask around wdt, how to
> > implement this feature.
> 
> Simple - you can't.
> 
> You can absolutely implement exactly the functionality you have today, with
> minimal changes to the protocol - it just should not be registered as an
> implementation of EFI_WATCHDOG_TIMER_ARCH_PROTOCOL.

I believe,  EFI_WATCHDOG_TIMER_ARCH_PROTOCOL is must 
Are you suggesting to EFI_WATCHDOG_TIMER_ARCH_PROTOCOL from MdeModulePkg
and hook platform specific code with this.
Or simply register EFI_WATCHDOG_TIMER_ARCH_PROTOCOL with dummy functions.

> Then your platform code can invoke this custom protocol as needed.
 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg PXE/iSCSI/TCP with NetworkPkg Drivers.

2018-12-18 Thread Yao, Jiewen
Looks good.
Reviewed-by: jiewen@intel.com


> -Original Message-
> From: Fu, Siyuan
> Sent: Tuesday, December 18, 2018 3:33 PM
> To: Fu, Siyuan ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Kubacki, Michael A
> 
> Subject: RE: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg
> PXE/iSCSI/TCP with NetworkPkg Drivers.
> 
> Hi, Jiewen and Kubacki
> 
> Do you have any comments for this patch?
> 
> BestRegards
> Fu Siyuan
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Fu
> > Siyuan
> > Sent: Friday, December 14, 2018 2:32 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen ; Kubacki, Michael A
> > 
> > Subject: [edk2] [PATCH] Edk2Platforms: Replace MdeModulePkg
> PXE/iSCSI/TCP with
> > NetworkPkg Drivers.
> >
> > The PXE/iSCSI/TCP drivers in MdeModulePkg are going to be deprecated.
> All
> > platform DSC/FDF files should be updated to use the dual-stack drivers in
> > NetworkPkg.
> >
> > Cc: Michael A Kubacki 
> > Cc: Jiewen Yao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Fu Siyuan 
> > ---
> >
> Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclude.
> dsc  | 7
> > ++-
> >
> Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclude.
> fdf | 7
> > ++-
> >  Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> | 3
> > +--
> >  3 files changed, 5 insertions(+), 12 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > index 4d70db6062..6764d46131 100644
> > ---
> a/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > +++
> b/Platform/Intel/AdvancedFeaturePkg/Include/Dsc/CoreAdvancedDxeInclud
> e.dsc
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  Platform description.
> >  #
> > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2017 - 2018, 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.
> > @@ -26,10 +26,7 @@
> >MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> >MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> >MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> >MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > -  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -  #MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >
> >NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> >NetworkPkg/TcpDxe/TcpDxe.inf
> > @@ -42,7 +39,7 @@
> >NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> >NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> >
> > -  #NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +  NetworkPkg/IScsiDxe/IScsiDxe.inf
> >NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >  !endif
> >
> > diff --git
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > index 0be408d13b..64f1dd5872 100644
> > ---
> >
> a/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > +++
> >
> b/Platform/Intel/AdvancedFeaturePkg/Include/Fdf/CoreAdvancedLateInclud
> e.fdf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  FDF file of Platform.
> >  #
> > -# Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2017 - 2018, 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.
> > @@ -27,9 +27,6 @@ INF
> MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> >  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > -#INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> > -INF  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> > -#INF
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >
> >  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> >  INF  NetworkPkg/TcpDxe/TcpDxe.inf
> > @@ -42,7 +39,7 @@ INF  NetworkPkg/HttpDxe/HttpDxe.inf
> >  INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> >  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> >
> > -#INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> >  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> >  !endif
> >
> > diff --git
> a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> > index 2174eaa609..dd0173a1af 100644
> > --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc

Re: [edk2] [PATCH 00/13] Extend secure variable service to be usable from Standalone MM

2018-12-18 Thread Jagadeesh Ujja
Hi Liming,

On Tue, Dec 18, 2018 at 10:07 AM Gao, Liming  wrote:
>
> Jagadeesh:
>   StandaloneMmServicesTableLib library class header file is added into 
> MdePkg. Its library implementation is in MdePkg and StandaloneMmPkg. The one 
> in MdePkg produces the dummy implementation, and the one in StandaloneMmPkg 
> produces the real implementation. I don't see the reason to separate this 
> library class.
>

In this patchset series, the Variable service/Fault tolerant/Nor Flash
driver are refactored to be usable as MM_STANDALONE driver. These
drivers uses the following libraries from “StandaloneMmPkg”.

- 
MmServicesTableLib|StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
- 
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

Variable MM_STANDALONE driver is located at
- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf

FaultTolerant MM_STANDALONE is driver located at
- MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
- 
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf

These drivers look for “gMmst” which is defined in
“MmServicesTableLib”. Ideally,
“StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h”
should have defined “gMmst” as an “extern EFI_MM_SYSTEM_TABLE
*gMmst;”.
In which case, we would have to add
“StandaloneMmPkg/StandaloneMmPkg.dec” in other drivers listed below.

- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
- MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf

This will make “edk2 packages” to be depended on
"StandaloneMmPkg/StandaloneMmPkg.dec".

To avoid this, “StandaloneMmPkg/Include/Library/StandaloneMmServicesTableLib.h”
is moved to “MdePkg/Include/Library/StandaloneMmServicesTableLib.h”.
But, the implementation of “MmServicesTableLib” comes from
“StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf”.

Thanks
Jagadeesh

> Thanks
> Liming
> >-Original Message-
> >From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> >Sent: Monday, December 17, 2018 7:47 PM
> >To: Gao, Liming 
> >Cc: edk2-devel@lists.01.org; Zhang, Chao B ;
> >leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> >Subject: Re: [edk2] [PATCH 00/13] Extend secure variable service to be usable
> >from Standalone MM
> >
> >Hi Liming,
> >
> >On Mon, Dec 17, 2018 at 7:15 AM Gao, Liming  wrote:
> >>
> >> One question here. Why separate StandaloneMmServicesTableLib to two
> >library classes? Current MdePkg\Include\Library\SmmServicesTableLib.h is
> >one library class.
> >MdePkg\Library\SmmServicesTableLib\SmmServicesTableLib.inf is its
> >implementation. StandaloneMmServicesTableLib should be same to it.
> >> StandaloneMmServicesTableLib is the library class.
> >MdePkg\Library\StandaloneMmRuntimeDxe is its library instance.
> >>
> >Thanks for your review.
> >
> >The implementation of the "StandaloneMmServicesTableLib" library class
> >is at "StandaloneMmPkg/Library/StandaloneMmServicesTableLib/". As this
> >patchset reuses some of the DXE_DRIVER drivers as MM_STANDALONE
> >drivers, the "StandaloneMmServicesTableLib" library class definition
> >was placed within MdePkg. The reason for splitting the library class
> >definition (in MdePkg) and its implementation (in StandaloneMmPkg) was
> >due to your comment that "edk2 packages" should not have any reference
> >to StandaloneMmPkg.dec.
> >
> >The "StandaloneMmRuntimeDxe" library now just has an implementation of
> >InMm(). And so, this can be kept as a separate library with no
> >dependency on StandaloneMmPkg. So this was the reason to split
> >"StandaloneMmRuntimeDxe" and "StandaloneMmServicesTableLib" into two
> >separate libraries.
> >
> >thanks
> >Jagadeesh
> >> Thanks
> >> Liming
> >> >-Original Message-
> >> >From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> >> >Sent: Friday, December 14, 2018 8:13 PM
> >> >To: edk2-devel@lists.01.org; Gao, Liming ; Zhang,
> >> >Chao B ; leif.lindh...@linaro.org;
> >> >ard.biesheu...@linaro.org
> >> >Subject: [PATCH 00/13] Extend secure variable service to be usable from
> >> >Standalone MM
> >> >
> >> >Changes since RFC v4:
> >> >- Addressed all the comments from Liming Gao
> >> >  - Added an additional PCD 'PcdStandaloneMmCodeEnabled' to indicate
> >> >presence of StandaloneMM support.
> >> >  - MdePkg.dec file updated to include StandaloneMmServiceTableLib and
> >> >StandaloneMmRuntimeDxe library.
> >> >  - Platform specific changes will be posted in a seperate patchset.
> >> >  - AsmLfence wrapper function is supported for AArch64 platforms.
> >> >  - All the patches in this series can be pulled from
> >> >https://github.com/jagadeeshujja/edk2 (branch:
> >> >topics/aarch64_secure_vars)
> >> >
> >> >Changes since RFC v3:
> >> >- Addressed all the comments from Liming Gao
> >> >  - Added a AArch64 

Re: [edk2] [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' statement

2018-12-18 Thread Zeng, Star
Yi,

Good question.

With the patch, the code can be compiled and the image can boot to setup.
The image could not find shell, but that is not related to the patch.
It is because 
https://github.com/tianocore/edk2/commit/2840bb51040bb79c1ad53b1eb1cbb86e5edf80ca#diff-0318cca23f8f1c46d1076b3a5891fadd
 updated the platform dsc and fdf to use 2.0 shell source build (with 2.0 shell 
file GUID), but the code at 
https://github.com/tianocore/edk2/blob/master/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c#L1365
 is still using PcdShellFile (which matches to EDK shell) to find shell file.


Thanks,
Star
-Original Message-
From: Qian, Yi 
Sent: Tuesday, December 18, 2018 10:08 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Sun, Zailiang 
Subject: RE: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' 
statement

Hi Star,
   This patch is good to me. Only one thing I need to know is whether you have 
done the unit test, at least, compilation, and boot into shell. 

Thanks
Qian Yi

-Original Message-
From: Zeng, Star 
Sent: Friday, December 14, 2018 6:29 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Sun, Zailiang ; 
Qian, Yi 
Subject: [PATCH 6/7] Vlv2TbltDevicePkg: Remove PcdPeiCoreMaxXXX PCDs' statement

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

The codes have been updated to not use PcdPeiCoreMaxFvSupported, 
PcdPeiCoreMaxPeimPerFv and PcdPeiCoreMaxPpiSupported, so their statement in 
platform DSC could be removed.

Cc: Zailiang Sun 
Cc: Yi Qian 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 2 --
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 2 --
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 2 --
 3 files changed, 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index f8ad29df5986..d43611550285 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index ca3b2ff90287..a33816c4d18b 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 81f36bd73b28..b50731f25ffb 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -663,10 +663,8 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdS3AcpiReservedMemorySize|0x1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|50
   gEfiMdeModulePkgTokenSpaceGuid.PcdSrIovSupport|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport|FALSE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiCpuTokenSpaceGuid.PcdCpuSmmApSyncTimeout|1000
 !if $(S4_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE
--
2.7.0.windows.1

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


Re: [edk2] [edk2-platforms PATCH 2/2] PurleyOpenBoardPkg: Remove PcdPeiCoreMaxXXX PCDs' statement

2018-12-18 Thread Zeng, Star
Could you help take a review to the patch? :)

Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Friday, December 14, 2018 6:43 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Lu, Shifei A ; 
Zhou, Bowen ; Oram, Isaac W 
Subject: [edk2-platforms PATCH 2/2] PurleyOpenBoardPkg: Remove PcdPeiCoreMaxXXX 
PCDs' statement

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

The codes have been updated to not use PcdPeiCoreMaxFvSupported, 
PcdPeiCoreMaxPeimPerFv and PcdPeiCoreMaxPpiSupported, so their statement in 
platform DSC could be removed.

Cc: Shifei A Lu 
Cc: Xiaohu Zhou 
Cc: Isaac W Oram 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/PlatformPkgPcd.dsc | 2 --
 1 file changed, 2 deletions(-)

diff --git 
a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/PlatformPkgPcd.dsc 
b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/PlatformPkgPcd.dsc
index bfe32aba1c5c..bfdf5c0c3874 100644
--- a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/PlatformPkgPcd.dsc
+++ b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/PlatformPkgPcd.ds
+++ c
@@ -91,8 +91,6 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0x0
   gEfiMdePkgTokenSpaceGuid.PcdFSBClock|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeiStackSize|0x10
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|32
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPpiSupported|128
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|0x170
 
   gEfiCpuTokenSpaceGuid.PcdCpuIEDRamSize|0x40
--
2.7.0.windows.1

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


Re: [edk2] [PATCH] ArmPlatformPkg/PL011SerialPortLib: use untyped PCD for register base

2018-12-18 Thread Philippe Mathieu-Daudé
On 12/17/18 7:51 PM, Ard Biesheuvel wrote:
> Use an untyped PCD reference for PcdSerialRegisterBase, so that the
> library gets built without hardcoded values, permitting modules to
> override the default serial port. This allows SerialDxe to use a
> different serial port from the one used for DEBUG output (which
> often gets occluded due to the console driver clearing the screen)

You missed the trailing '.' :)

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c   | 14 
> +++---
>  ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf |  4 +++-
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c 
> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> index 212991d63859..d576f79c3e6e 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c
> @@ -48,7 +48,7 @@ SerialPortInitialize (
>StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
>  
>return PL011UartInitializePort (
> -   (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> +   (UINTN)PcdGet64 (PcdSerialRegisterBase),
> PL011UartClockGetFreq(),
> ,
> ,
> @@ -75,7 +75,7 @@ SerialPortWrite (
>IN UINTN NumberOfBytes
>)
>  {
> -  return PL011UartWrite ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), 
> Buffer, NumberOfBytes);
> +  return PL011UartWrite ((UINTN)PcdGet64 (PcdSerialRegisterBase), Buffer, 
> NumberOfBytes);
>  }
>  
>  /**
> @@ -95,7 +95,7 @@ SerialPortRead (
>IN  UINTN NumberOfBytes
>  )
>  {
> -  return PL011UartRead ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), 
> Buffer, NumberOfBytes);
> +  return PL011UartRead ((UINTN)PcdGet64 (PcdSerialRegisterBase), Buffer, 
> NumberOfBytes);
>  }
>  
>  /**
> @@ -111,7 +111,7 @@ SerialPortPoll (
>VOID
>)
>  {
> -  return PL011UartPoll ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase));
> +  return PL011UartPoll ((UINTN)PcdGet64 (PcdSerialRegisterBase));
>  }
>  /**
>Set new attributes to PL011.
> @@ -156,7 +156,7 @@ SerialPortSetAttributes (
>)
>  {
>return PL011UartInitializePort (
> -   (UINTN)FixedPcdGet64 (PcdSerialRegisterBase),
> +   (UINTN)PcdGet64 (PcdSerialRegisterBase),
> PL011UartClockGetFreq(),
> BaudRate,
> ReceiveFifoDepth,
> @@ -198,7 +198,7 @@ SerialPortSetControl (
>IN UINT32  Control
>)
>  {
> -  return PL011UartSetControl ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), 
> Control);
> +  return PL011UartSetControl ((UINTN)PcdGet64 (PcdSerialRegisterBase), 
> Control);
>  }
>  
>  /**
> @@ -239,5 +239,5 @@ SerialPortGetControl (
>OUT UINT32  *Control
>)
>  {
> -  return PL011UartGetControl ((UINTN)FixedPcdGet64 (PcdSerialRegisterBase), 
> Control);
> +  return PL011UartGetControl ((UINTN)PcdGet64 (PcdSerialRegisterBase), 
> Control);
>  }
> diff --git a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf 
> b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> index 5ce5b2f5304c..bca7bed875c6 100644
> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> @@ -36,8 +36,10 @@
>MdeModulePkg/MdeModulePkg.dec
>ArmPlatformPkg/ArmPlatformPkg.dec
>  
> -[FixedPcd]
> +[Pcd]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
> +
> +[FixedPcd]
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] Platform/FVP-AArch64: use different serial ports for DEBUG and console

2018-12-18 Thread Philippe Mathieu-Daudé
On 12/17/18 7:53 PM, Ard Biesheuvel wrote:
> The FVP models expose several emulated serial ports, and always start with
> Xterm windows connected to at least two of them. So let's switch to the
> second one for DEBUG output, leaving the original one for console output
> via SerialDxe.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
> b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> index 7094e57ee13a..7db1c675c3d9 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> @@ -125,7 +125,7 @@
>gArmPlatformTokenSpaceGuid.PcdSP805WatchdogClockFrequencyInHz|2400
>  
>## PL011 - Serial Terminal
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c09
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0a
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>  
> @@ -239,7 +239,10 @@
>MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
>MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
> -  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +  MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c09
> +  }
>  
>MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] SP805 driver

2018-12-18 Thread Leif Lindholm
On Tue, Dec 18, 2018 at 05:20:59AM +, Udit Kumar wrote:
> Thanks for note Leif,
> 
> I am trying to understand this behavior  of specification.
> 
> >"If no handler has been registered, or the registered handler
> >returns, then the system will be reset by calling the Runtime
> >Service ResetSystem()."
> I believe,  this handler needs to be called by wdt driver after wdt
> is expired. Meaning , we want wdt to work without resetting the
> system.
> Therefore a mask is needed with wdt, which will prevent to reset the
> system.   I see it working, at least for SP805, because of PMU mask
> bits.

Yes, if the WDT can be configured to generate an interrupt instead of
a hardware reset, it can be used to implement this protocol.

> Also ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c has this 
> limitation.
> I haven’t read spec of this wdt.  I hope there should a mask around.

No, the situation for the GenericWatchdogDxe is not as dire.

Correct: it does not permit registering software handlers (which
perhaps we should do something about, but is ... acceptable).

_But_, it still conforms to the above text; when the timer expires,
it resets the system by calling the ResetSystem() service. It does not
directly force a hardware reset.

The severe problem is not the lack of the ability to register the
software handler (which does remove much of the utility), but the
removal of reset control from the system firmware.

> Coming back to hardware, which does not have mask around wdt, how to
> implement this feature.

Simple - you can't.

You can absolutely implement exactly the functionality you have today,
with minimal changes to the protocol - it just should not be registered
as an implementation of EFI_WATCHDOG_TIMER_ARCH_PROTOCOL.

Then your platform code can invoke this custom protocol as needed.

Regards,

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


[edk2] [PATCH] BaseTools: Fix GenFds error doesn't break build.

2018-12-18 Thread Lin, Derek (HPS SW)
Fix a bug because of b3497bad1221704a5dbc5da0b10f42625f1ad2ed.
Before the patch, when GenFds fail, the build continue and return success.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Derek Lin 
---
 BaseTools/Source/Python/build/build.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index cbbb291b2c..97271e634e 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.  #  
Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2018, Hewlett Packard Enterprise Development, L.P.
 #
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License @@ -1384,7 +1385,8 
@@ class Build():
 
 # genfds
 if Target == 'fds':
-GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db)
+if GenFdsApi(AutoGenObject.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 return True
 
 # run
@@ -2122,7 +2124,8 @@ class Build():
 # Generate FD image if there's a FDF file found
 #
 GenFdsStart = time.time()
-GenFdsApi(Wa.GenFdsCommandDict, self.Db)
+if GenFdsApi(Wa.GenFdsCommandDict, self.Db):
+EdkLogger.error("build", COMMAND_FAILURE)
 
 #
 # Create MAP file for all platform FVs after GenFds.
--
2.17.0.windows.1

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