[edk2] [PATCH 0/2] Delete TCP, PXE, iSCSI driver in MdeModulePkg
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.
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
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
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
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
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
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
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.
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.
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.
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
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.
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
*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.
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.
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.
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
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.
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
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
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.
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
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.
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
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
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
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
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
> -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
> -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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> > > > 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
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
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
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
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
> -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.
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
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
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
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
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
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
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.
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