Date   

Re: [PATCH v6] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

Nate DeSimone
 

Please update FSP spec version in FspiApi.h from "2.0" to "2.0 - 2.2". With that change...

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
Sent: Sunday, May 10, 2020 8:32 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>
Subject: [edk2-devel] [PATCH v6] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

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

To enhance FSP silicon initialization flexibility an optional Multi-Phase API is introduced and FSP header needs update for new API offset. Also new SecCore module created for FspMultiPhaseSiInit API

New ARCH_UPD introduced for enhancing FSP debug message flexibility now bootloader can pass its own debug handler function pointer and FSP will call the function to handle debug message.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 6 +++---
IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c | 19 ++++++++++++++++++-
IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm | 5 ++++-
IntelFsp2Pkg/Include/FspEas/FspApi.h | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
IntelFsp2Pkg/Include/FspGlobalData.h | 3 ++-
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 10 ++++++++--
IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h | 16 +++++++++++++++-
9 files changed, 321 insertions(+), 11 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index 8e0595fe9a..1334959005 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -59,7 +59,7 @@ FspApiCallingCheck (
Status = EFI_UNSUPPORTED;
}
}
- } else if (ApiIdx == FspSiliconInitApiIndex) {
+ } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx ==
+ FspMultiPhaseSiInitApiIndex)) {
//
// FspSiliconInit check
//
@@ -68,7 +68,7 @@ FspApiCallingCheck (
} else {
if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
Status = EFI_UNSUPPORTED;
- } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) {
+ } else if (EFI_ERROR (FspUpdSignatureCheck
+ (FspSiliconInitApiIndex, ApiParam))) {
Status = EFI_INVALID_PARAMETER;
}
}
diff --git a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
index f7945b5240..df8c5d121f 100644
--- a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
+++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
@@ -1,7 +1,7 @@
/** @file
Null instance of Platform Sec Lib.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -25,3 +25,20 @@ FspUpdSignatureCheck ( {
return EFI_SUCCESS;
}
+
+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
new file mode 100644
index 0000000000..0a24eb2a8b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
@@ -0,0 +1,52 @@
+## @file
+# Sec Core for FSP to support MultiPhase (SeparatePhase) SiInitialization.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> # #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Fsp22SecCoreS
+ FILE_GUID = DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.IA32]
+ Ia32/Stack.nasm
+ Ia32/Fsp22ApiEntryS.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
+
+[Ppis]
+ gEfiTemporaryRamSupportPpiGuid ## PRODUCES
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
new file mode 100644
index 0000000000..c5e73a635b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
@@ -0,0 +1,99 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> ;
+SPDX-License-Identifier: BSD-2-Clause-Patent ;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C ; extern
+ASM_PFX(FspApiCommon) extern ASM_PFX(FspMultiPhaseSiInitApiHandler)
+
+;----------------------------------------------------------------------
+------
+; NotifyPhase API
+;
+; This FSP API will notify the FSP about the different phases in the
+boot ; process ;
+;----------------------------------------------------------------------
+------
+global ASM_PFX(NotifyPhaseApi)
+ASM_PFX(NotifyPhaseApi):
+ mov eax, 2 ; FSP_API_INDEX.NotifyPhaseApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------
+------
+; FspSiliconInit API
+;
+; This FSP API initializes the CPU and the chipset including the IO ;
+controllers in the chipset to enable normal operation of these devices.
+;
+;----------------------------------------------------------------------
+------
+global ASM_PFX(FspSiliconInitApi)
+ASM_PFX(FspSiliconInitApi):
+ mov eax, 5 ; FSP_API_INDEX.FspSiliconInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------
+------
+; FspMultiPhaseSiInitApi API
+;
+; This FSP API provides multi-phase silicon initialization, which
+brings greater ; modularity beyond the existing FspSiliconInit() API.
+; Increased modularity is achieved by adding an extra API to FSP-S.
+; This allows the bootloader to add board specific initialization steps
+throughout ; the SiliconInit flow as needed.
+;
+;----------------------------------------------------------------------
+------
+global ASM_PFX(FspMultiPhaseSiInitApi)
+ASM_PFX(FspMultiPhaseSiInitApi):
+ mov eax, 6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------
+------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution ;
+;----------------------------------------------------------------------
+------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ ;
+ ; Handle FspMultiPhaseSiInitApiIndex API
+ ;
+ cmp eax, 6
+ jnz NotMultiPhaseSiInitApi
+
+ pushad
+ push DWORD [esp + (4 * 8 + 4)] ; push ApiParam
+ push eax ; push ApiIdx
+ call ASM_PFX(FspMultiPhaseSiInitApiHandler)
+ add esp, 8
+ mov dword [esp + (4 * 7)], eax
+ popad
+ ret
+
+NotMultiPhaseSiInitApi:
+ jmp $
+ ret
+
+;----------------------------------------------------------------------
+------
+; TempRamInit API
+;
+; Empty function for WHOLEARCHIVE build option ;
+;----------------------------------------------------------------------
+------
+global ASM_PFX(TempRamInitApi)
+ASM_PFX(TempRamInitApi):
+ jmp $
+ ret
+
+;----------------------------------------------------------------------
+------
+; Module Entrypoint API
+;----------------------------------------------------------------------
+------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
index bb4451b145..26ae7d9fd3 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
@@ -1,7 +1,7 @@
;; @file
; Provide FSP API entry points.
;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2016 - 2020, Intel Corporation. All rights
+reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent ;;

@@ -62,6 +62,9 @@ FspApiCommon2:
cmp eax, 3 ; FspMemoryInit API
jz FspApiCommon3

+ cmp eax, 6 ; FspMultiPhaseSiInitApiIndex API
+ jz FspApiCommon3
+
call ASM_PFX(AsmGetFspInfoHeader)
jmp ASM_PFX(Loader2PeiSwitchStack)

diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index dcf489dbe6..2887848f69 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -2,7 +2,7 @@
Intel FSP API definition from Intel Firmware Support Package External
Architecture Specification v2.0.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,6 +10,8 @@
#ifndef _FSP_API_H_
#define _FSP_API_H_

+#include <Pi/PiStatusCode.h>
+
///
/// FSP Reset Status code
/// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code @@ -24,6 +26,59 @@
#define FSP_STATUS_RESET_REQUIRED_8 0x40000008
/// @}

+/*
+ FSP may optionally include the capability of generating events messages to aid in the debugging of firmware issues.
+ These events fall under three catagories: Error, Progress, and Debug.
+The event reporting mechanism follows the
+ status code services described in section 6 and 7 of the PI Specification v1.7 Volume 3.
+
+ @param[in] Type Indicates the type of event being reported.
+ See MdePkg/Include/Pi/PiStatusCode.h for the definition of EFI_STATUS_CODE_TYPE.
+ @param[in] Value Describes the current status of a hardware or software entity.
+ This includes information about the class and subclass that is used to classify the entity as well as an operation.
+ For progress events, the operation is the current activity. For error events, it is the exception.
+ For debug events, it is not defined at this time.
+ See MdePkg/Include/Pi/PiStatusCode.h for the definition of EFI_STATUS_CODE_VALUE.
+ @param[in] Instance The enumeration of a hardware or software entity within the system.
+ A system may contain multiple entities that match a class/subclass pairing. The instance differentiates between them.
+ An instance of 0 indicates that instance information is unavailable, not meaningful, or not relevant.
+ Valid instance numbers start with 1.
+ @param[in] *CallerId This parameter can be used to identify the sub-module within the FSP generating the event.
+ This parameter may be NULL.
+ @param[in] *Data This optional parameter may be used to pass additional data. The contents can have event-specific data.
+ For example, the FSP provides a EFI_STATUS_CODE_STRING_DATA instance to this parameter when sending debug messages.
+ This parameter is NULL when no additional data is provided.
+
+ @retval EFI_SUCCESS The event was handled successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_DEVICE_ERROR The event handler failed.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_EVENT_HANDLER) (
+ IN EFI_STATUS_CODE_TYPE Type,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN OPTIONAL EFI_GUID *CallerId,
+ IN OPTIONAL EFI_STATUS_CODE_DATA *Data
+ );
+
+/*
+ Handler for FSP-T debug log messages, provided by the bootloader.
+
+ @param[in] DebugMessage A pointer to the debug message to be written to the log.
+ @param[in] MessageLength Number of bytes to written to the debug log.
+
+ @retval UINT32 The return value indicates the number of bytes actually written to
+ the debug log. If the return value is less than MessageLength,
+ an error occurred.
+*/
+typedef
+UINT32
+(EFIAPI *FSP_DEBUG_HANDLER) (
+ IN CHAR8* DebugMessage,
+ IN UINT32 MessageLength
+ );
+
#pragma pack(1)
///
/// FSP_UPD_HEADER Configuration.
@@ -77,7 +132,12 @@ typedef struct {
/// Current boot mode.
///
UINT32 BootMode;
- UINT8 Reserved1[8];
+ ///
+ /// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
+ /// This value is only valid if Revision is >= 2.
+ ///
+ FSP_EVENT_HANDLER *FspEventHandler;
+ UINT8 Reserved1[4];
} FSPM_ARCH_UPD;

///
@@ -147,6 +207,40 @@ typedef struct {
FSP_INIT_PHASE Phase;
} NOTIFY_PHASE_PARAMS;

+///
+/// Action definition for FspMultiPhaseSiInit API /// typedef enum {
+ EnumMultiPhaseGetNumberOfPhases = 0x0,
+ EnumMultiPhaseExecutePhase = 0x1
+} FSP_MULTI_PHASE_ACTION;
+
+///
+/// Data structure returned by FSP when bootloader calling ///
+FspMultiPhaseSiInit API with action 0 (EnumMultiPhaseGetNumberOfPhases)
+/// typedef struct {
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
+
+///
+/// FspMultiPhaseSiInit function parameter.
+///
+/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
+/// - PhaseIndex must be 0.
+/// - MultiPhaseParamPtr should point to an instance of FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
+///
+/// For action 1 (EnumMultiPhaseExecutePhase):
+/// - PhaseIndex will be the phase that will be executed by FSP.
+/// - MultiPhaseParamPtr shall be NULL.
+///
+typedef struct {
+ IN FSP_MULTI_PHASE_ACTION MultiPhaseAction;
+ IN UINT32 PhaseIndex;
+ IN OUT VOID *MultiPhaseParamPtr;
+} FSP_MULTI_PHASE_PARAMS;
+
#pragma pack()

/**
@@ -279,4 +373,28 @@ EFI_STATUS
IN VOID *FspsUpdDataPtr
);

+/**
+ This FSP API is expected to be called after FspSiliconInit but before FspNotifyPhase.
+ This FSP API provides multi-phase silicon initialization; which
+brings greater modularity
+ beyond the existing FspSiliconInit() API. Increased modularity is
+achieved by adding an
+ extra API to FSP-S. This allows the bootloader to add board specific
+initialization steps
+ throughout the SiliconInit flow as needed.
+
+ @param[in,out] FSP_MULTI_PHASE_PARAMS For action - EnumMultiPhaseGetNumberOfPhases:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr will contain
+ how many phases supported by FSP.
+ For action - EnumMultiPhaseExecutePhase:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr shall be NULL.
+ @retval EFI_SUCCESS FSP execution environment was initialized successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
+ @retval EFI_DEVICE_ERROR FSP initialization failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
+ IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
+);
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
index 1896b0240a..02df8463ed 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -22,6 +22,7 @@ typedef enum {
FspMemoryInitApiIndex,
TempRamExitApiIndex,
FspSiliconInitApiIndex,
+ FspMultiPhaseSiInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index 16f43a1273..3474bac1de 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -1,8 +1,8 @@
/** @file
Intel FSP Header File definition from Intel Firmware Support Package External
- Architecture Specification v2.0.
+ Architecture Specification v2.0 and above.

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -110,6 +110,12 @@ typedef struct {
/// Byte 0x44: The offset for the API to initialize the CPU and chipset.
///
UINT32 FspSiliconInitEntryOffset;
+ ///
+ /// Byte 0x48: Offset for the API for the optional Multi-Phase processor and chipset initialization.
+ /// This value is only valid if FSP HeaderRevision is >= 5.
+ /// If the value is set to 0x00000000, then this API is not available in this component.
+ ///
+ UINT32 FspMultiPhaseSiInitEntryOffset;
} FSP_INFO_HEADER;

///
diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
index 4d01b5f6d9..51a0309aed 100644
--- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
+++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -79,4 +79,18 @@ FspUpdSignatureCheck (
IN VOID *ApiParam
);

+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ );
+
#endif
--
2.13.3.windows.1


Re: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

Chiu, Chasel
 

Hi,

Found some comment changes missed in V5, please help to review V6.

Thanks,
Chasel

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
Chasel
Sent: Monday, May 11, 2020 11:24 AM
To: Desimone, Nathaniel L <nathaniel.l.desimone@...>;
devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Zeng, Star <star.zeng@...>
Subject: Re: [edk2-devel] [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support
Multi-Phase SiInit and debug handlers.


Thanks Nate, and sorry that I misunderstood something in previous patch.
I have sent V5 to address all your feedbacks, please help to review again.

Thanks,
Chasel


-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@...>
Sent: Sunday, May 10, 2020 2:22 PM
To: Chiu, Chasel <chasel.chiu@...>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Zeng, Star
<star.zeng@...>
Subject: RE: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support
Multi-Phase SiInit and debug handlers.

Hi Chasel,

A few comments:

#1) FspApi.h

The references to the FSP/PI spec sections in the comments are a bit odd.
None of the existing comments in FspApi.h make references to sections
in specs. Particularly weird are these ones:

* See Section 11.10 Appendix A - Data Structures for the definition of
EFI_STATUS_CODE_TYPE.
* See Section 11.10 Appendix A - Data Structures for the definition of
EFI_STATUS_CODE_VALUE.

This is code... why not point the reader to PiStatusCode.h directly?

#2) FspApi.h

This seems overly verbose:

@retval UINT32 The return value will be passed
back through the EAX register.
The return value indicates the
number of bytes actually written to
the debug log. If the return
value
is less than MessageLength,
an error occurred.

I think it can be shortened to:

@retval UINT32 The return value indicates the
number of bytes actually written to
the debug log. If the return
value
is less than MessageLength,
an error occurred.

#3) Please rename FspSecCoreSS.inf to Fsp22SecCoreS.inf and
FspApiEntrySS.nasm to Fsp22ApiEntryS.nasm

#4) Any reason why the entry points for FspSiliconInit() and
NotifyPhase() are absent from FspApiEntrySS.nasm/Fsp22ApiEntry.nasm?
Are we going to need 2 FspSecCoreS drivers now instead of 1? Not sure
how that would work... only 1 SecCore is allowed per FV.

Thanks,
Nate

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@...>
Sent: Thursday, May 7, 2020 6:29 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>
Subject: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support
Multi-Phase SiInit and debug handlers.

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

To enhance FSP silicon initialization flexibility an optional
Multi-Phase API is introduced and FSP header needs update for new
API offset. Also new SecCore module created for FspMultiPhaseSiInit
API

New ARCH_UPD introduced for enhancing FSP debug message flexibility
now bootloader can pass its own debug handler function pointer and
FSP will call the function to handle debug message.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
| 6 +++---
IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
|
19
++++++++++++++++++-
IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
| 48
++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
| 5
++++-
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
| 68
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++
IntelFsp2Pkg/Include/FspEas/FspApi.h
| 124
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++--
IntelFsp2Pkg/Include/FspGlobalData.h
| 3 ++-
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
| 10 ++++++++--
IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
|
16
+++++++++++++++-
9 files changed, 288 insertions(+), 11 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index 8e0595fe9a..1334959005 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2016 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -59,7 +59,7 @@ FspApiCallingCheck (
Status = EFI_UNSUPPORTED;
}
}
- } else if (ApiIdx == FspSiliconInitApiIndex) {
+ } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx ==
+ FspMultiPhaseSiInitApiIndex)) {
//
// FspSiliconInit check
//
@@ -68,7 +68,7 @@ FspApiCallingCheck (
} else {
if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
Status = EFI_UNSUPPORTED;
- } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam)))
{
+ } else if (EFI_ERROR (FspUpdSignatureCheck
+ (FspSiliconInitApiIndex, ApiParam))) {
Status = EFI_INVALID_PARAMETER;
}
}
diff --git
a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
index f7945b5240..df8c5d121f 100644
---
a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
+++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNu
+++ ll
+++ .c
@@ -1,7 +1,7 @@
/** @file
Null instance of Platform Sec Lib.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -25,3 +25,20 @@ FspUpdSignatureCheck ( {
return EFI_SUCCESS;
}
+
+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
b/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
new file mode 100644
index 0000000000..184101c7d3
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
@@ -0,0 +1,48 @@
+## @file
+# Sec Core for FSP to support MultiPhase (SeparatePhase)
SiInitialization.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = FspSecCoreSS
+ FILE_GUID =
DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required
+by the
build tools.
+#
+# VALID_ARCHITECTURES = IA32
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.IA32]
+ Ia32/Stack.nasm
+ Ia32/FspApiEntrySS.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
index bb4451b145..26ae7d9fd3 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
@@ -1,7 +1,7 @@
;; @file
; Provide FSP API entry points.
;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2016 - 2020, Intel Corporation. All rights
+reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent ;;

@@ -62,6 +62,9 @@ FspApiCommon2:
cmp eax, 3 ; FspMemoryInit API
jz FspApiCommon3

+ cmp eax, 6 ; FspMultiPhaseSiInitApiIndex API
+ jz FspApiCommon3
+
call ASM_PFX(AsmGetFspInfoHeader)
jmp ASM_PFX(Loader2PeiSwitchStack)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
new file mode 100644
index 0000000000..e6956277a9
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
@@ -0,0 +1,68 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> ;
+SPDX-License-Identifier: BSD-2-Clause-Patent ;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C ; extern
+ASM_PFX(FspApiCommon) extern
ASM_PFX(FspMultiPhaseSiInitApiHandler)
+
+;------------------------------------------------------------------
+--
+--
+------
+; FspMultiPhaseSiInitApi API
+;
+; This FSP API provides multi-phase silicon initialization, which
+brings greater ; modularity beyond the existing FspSiliconInit() API.
+; Increased modularity is achieved by adding an extra API to FSP-S.
+; This allows the bootloader to add board specific initialization
+steps throughout ; the SiliconInit flow as needed.
+;
+;------------------------------------------------------------------
+--
+--
+------
+global ASM_PFX(FspMultiPhaseSiInitApi)
+ASM_PFX(FspMultiPhaseSiInitApi):
+ mov eax, 6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;------------------------------------------------------------------
+--
+--
+------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP
+execution ;
+;------------------------------------------------------------------
+--
+--
+------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ ;
+ ; Handle FspMultiPhaseSiInitApiIndex API
+ ;
+ pushad
+ push DWORD [esp + (4 * 8 + 4)] ; push ApiParam
+ push eax ; push ApiIdx
+ call ASM_PFX(FspMultiPhaseSiInitApiHandler)
+ add esp, 8
+ mov dword [esp + (4 * 7)], eax
+ popad
+ ret
+
+;------------------------------------------------------------------
+--
+--
+------
+; TempRamInit API
+;
+; Empty function for WHOLEARCHIVE build option ;
+;------------------------------------------------------------------
+--
+--
+------
+global ASM_PFX(TempRamInitApi)
+ASM_PFX(TempRamInitApi):
+ jmp $
+ ret
+
+;------------------------------------------------------------------
+--
+--
+------
+; Module Entrypoint API
+;------------------------------------------------------------------
+--
+--
+------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+
diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index dcf489dbe6..ca908b0198 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -2,7 +2,7 @@
Intel FSP API definition from Intel Firmware Support Package External
Architecture Specification v2.0.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,6 +10,8 @@
#ifndef _FSP_API_H_
#define _FSP_API_H_

+#include <Pi/PiStatusCode.h>
+
///
/// FSP Reset Status code
/// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status
Code @@ -
24,6 +26,60 @@
#define FSP_STATUS_RESET_REQUIRED_8 0x40000008
/// @}

+/*
+ FSP may optionally include the capability of generating events
+messages to
aid in the debugging of firmware issues.
+ These events fall under three catagories: Error, Progress, and Debug.
+The event reporting mechanism follows the
+ status code services described in section 6 and 7 of the PI
+Specification v1.7
Volume 3.
+
+ @param[in] Type Indicates the type of event
being reported.
+ See Section 11.10 Appendix
A
-
+ Data Structures for the
definition of EFI_STATUS_CODE_TYPE.
+ @param[in] Value Describes the current status
of
a hardware or
software entity.
+ This includes information
about
+ the class and subclass that
is used to classify the entity as well as an operation.
+ For progress events, the
operation is the current activity.
For error events, it is the exception.
+ For debug events, it is not
defined at this time.
+ See Section 11.10 Appendix
A
-
+ Data Structures for the
definition of EFI_STATUS_CODE_VALUE.
+ @param[in] Instance The enumeration of a
hardware
or software
entity within the system.
+ A system may contain
multiple
+ entities that match a
class/subclass pairing. The instance differentiates between them.
+ An instance of 0 indicates
that
+ instance information is
unavailable, not meaningful, or not relevant.
+ Valid instance numbers start
with 1.
+ @param[in] *CallerId This parameter can be used to
identify the
sub-module within the FSP generating the event.
+ This parameter may be
NULL.
+ @param[in] *Data This optional parameter may
be used to pass
additional data. The contents can have event-specific data.
+ For example, the FSP
provides
a
EFI_STATUS_CODE_STRING_DATA instance to this parameter when
sending
debug messages.
+ This parameter is NULL
when
no
+ additional data is
provided.
+
+ @retval EFI_SUCCESS The event was handled
successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_DEVICE_ERROR The event handler failed.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_EVENT_HANDLER) (
+ IN EFI_STATUS_CODE_TYPE Type,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN OPTIONAL EFI_GUID *CallerId,
+ IN OPTIONAL EFI_STATUS_CODE_DATA *Data
+ );
+
+/*
+ Handler for FSP-T debug log messages, provided by the bootloader.
+
+ @param[in] DebugMessage A pointer to the debug
message to be
written to the log.
+ @param[in] MessageLength Number of bytes to written to
the debug
log.
+
+ @retval UINT32 The return value will be
passed
back through the
EAX register.
+ The return value indicates
the
+ number of bytes actually
written to
+ the debug log. If the return
+ value is less than
MessageLength,
+ an error occurred.
+*/
+typedef
+UINT32
+(EFIAPI *FSP_DEBUG_HANDLER) (
+ IN CHAR8* DebugMessage,
+ IN UINT32 MessageLength
+ );
+
#pragma pack(1)
///
/// FSP_UPD_HEADER Configuration.
@@ -77,7 +133,13 @@ typedef struct {
/// Current boot mode.
///
UINT32 BootMode;
- UINT8 Reserved1[8];
+ ///
+ /// A function pointer of type FSP_EVENT_HANDLER.
+ /// Optional event handler for the bootloader to be informed of
+ events
occurring during FSP execution.
+ /// Refer to Section 8.5 for more details. This value is only
+ valid if Revision is
= 2.
+ ///
+ FSP_EVENT_HANDLER *FspEventHandler;
+ UINT8 Reserved1[4];
} FSPM_ARCH_UPD;

///
@@ -147,6 +209,40 @@ typedef struct {
FSP_INIT_PHASE Phase;
} NOTIFY_PHASE_PARAMS;

+///
+/// Action definition for FspMultiPhaseSiInit API /// typedef enum
+{
+ EnumMultiPhaseGetNumberOfPhases = 0x0,
+ EnumMultiPhaseExecutePhase = 0x1
+} FSP_MULTI_PHASE_ACTION;
+
+///
+/// Data structure returned by FSP when bootloader calling ///
+FspMultiPhaseSiInit API with action 0
(EnumMultiPhaseGetNumberOfPhases)
+/// typedef struct {
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
+
+///
+/// FspMultiPhaseSiInit function parameter.
+///
+/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
+/// - PhaseIndex must be 0.
+/// - MultiPhaseParamPtr should point to an instance of
FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
+///
+/// For action 1 (EnumMultiPhaseExecutePhase):
+/// - PhaseIndex will be the phase that will be executed by FSP.
+/// - MultiPhaseParamPtr shall be NULL.
+///
+typedef struct {
+ IN FSP_MULTI_PHASE_ACTION MultiPhaseAction;
+ IN UINT32 PhaseIndex;
+ IN OUT VOID *MultiPhaseParamPtr;
+} FSP_MULTI_PHASE_PARAMS;
+
#pragma pack()

/**
@@ -279,4 +375,28 @@ EFI_STATUS
IN VOID *FspsUpdDataPtr
);

+/**
+ This FSP API is expeted to be called after FspSiliconInit but
+before
FspNotifyPhase.
+ This FSP API provides multi-phase silicon initialization; which
+brings greater modularity
+ beyond the existing FspSiliconInit() API. Increased modularity is
+achieved by adding an
+ extra API to FSP-S. This allows the bootloader to add board
+specific initialization steps
+ throughout the SiliconInit flow as needed.
+
+ @param[in,out] FSP_MULTI_PHASE_PARAMS For action -
EnumMultiPhaseGetNumberOfPhases:
+
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
will contain
+ how many phases
supported by FSP.
+ For action -
EnumMultiPhaseExecutePhase:
+
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
shall be NULL.
+ @retval EFI_SUCCESS FSP execution
environment was initialized
successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are
invalid.
+ @retval EFI_UNSUPPORTED The FSP calling
conditions were not
met.
+ @retval EFI_DEVICE_ERROR FSP initialization
failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required.
These
status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
+ IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
+);
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
b/IntelFsp2Pkg/Include/FspGlobalData.h
index 1896b0240a..02df8463ed 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2014 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -22,6 +22,7 @@ typedef enum {
FspMemoryInitApiIndex,
TempRamExitApiIndex,
FspSiliconInitApiIndex,
+ FspMultiPhaseSiInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index 16f43a1273..3474bac1de 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -1,8 +1,8 @@
/** @file
Intel FSP Header File definition from Intel Firmware Support
Package External
- Architecture Specification v2.0.
+ Architecture Specification v2.0 and above.

- Copyright (c) 2014 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -110,6 +110,12 @@ typedef struct {
/// Byte 0x44: The offset for the API to initialize the CPU and chipset.
///
UINT32 FspSiliconInitEntryOffset;
+ ///
+ /// Byte 0x48: Offset for the API for the optional Multi-Phase
+ processor
and chipset initialization.
+ /// This value is only valid if FSP HeaderRevision is >= 5.
+ /// If the value is set to 0x00000000, then this API is
not
available in
this component.
+ ///
+ UINT32 FspMultiPhaseSiInitEntryOffset;
} FSP_INFO_HEADER;

///
diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
index 4d01b5f6d9..51a0309aed 100644
--- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
+++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2015 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2015 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -79,4 +79,18 @@ FspUpdSignatureCheck (
IN VOID *ApiParam
);

+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ );
+
#endif
--
2.13.3.windows.1


[PATCH v6] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

Chiu, Chasel
 

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

To enhance FSP silicon initialization flexibility an optional
Multi-Phase API is introduced and FSP header needs update for
new API offset. Also new SecCore module created for
FspMultiPhaseSiInit API

New ARCH_UPD introduced for enhancing FSP debug message
flexibility now bootloader can pass its own debug handler
function pointer and FSP will call the function to handle
debug message.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 6 +++---
IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c | 19 ++++++++++++++++++-
IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm | 5 ++++-
IntelFsp2Pkg/Include/FspEas/FspApi.h | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
IntelFsp2Pkg/Include/FspGlobalData.h | 3 ++-
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 10 ++++++++--
IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h | 16 +++++++++++++++-
9 files changed, 321 insertions(+), 11 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index 8e0595fe9a..1334959005 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -59,7 +59,7 @@ FspApiCallingCheck (
Status = EFI_UNSUPPORTED;
}
}
- } else if (ApiIdx == FspSiliconInitApiIndex) {
+ } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx == FspMultiPhaseSiInitApiIndex)) {
//
// FspSiliconInit check
//
@@ -68,7 +68,7 @@ FspApiCallingCheck (
} else {
if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
Status = EFI_UNSUPPORTED;
- } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) {
+ } else if (EFI_ERROR (FspUpdSignatureCheck (FspSiliconInitApiIndex, ApiParam))) {
Status = EFI_INVALID_PARAMETER;
}
}
diff --git a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
index f7945b5240..df8c5d121f 100644
--- a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
+++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
@@ -1,7 +1,7 @@
/** @file
Null instance of Platform Sec Lib.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -25,3 +25,20 @@ FspUpdSignatureCheck (
{
return EFI_SUCCESS;
}
+
+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
new file mode 100644
index 0000000000..0a24eb2a8b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
@@ -0,0 +1,52 @@
+## @file
+# Sec Core for FSP to support MultiPhase (SeparatePhase) SiInitialization.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Fsp22SecCoreS
+ FILE_GUID = DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.IA32]
+ Ia32/Stack.nasm
+ Ia32/Fsp22ApiEntryS.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
+
+[Ppis]
+ gEfiTemporaryRamSupportPpiGuid ## PRODUCES
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
new file mode 100644
index 0000000000..c5e73a635b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
@@ -0,0 +1,99 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C
+;
+extern ASM_PFX(FspApiCommon)
+extern ASM_PFX(FspMultiPhaseSiInitApiHandler)
+
+;----------------------------------------------------------------------------
+; NotifyPhase API
+;
+; This FSP API will notify the FSP about the different phases in the boot
+; process
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(NotifyPhaseApi)
+ASM_PFX(NotifyPhaseApi):
+ mov eax, 2 ; FSP_API_INDEX.NotifyPhaseApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspSiliconInit API
+;
+; This FSP API initializes the CPU and the chipset including the IO
+; controllers in the chipset to enable normal operation of these devices.
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspSiliconInitApi)
+ASM_PFX(FspSiliconInitApi):
+ mov eax, 5 ; FSP_API_INDEX.FspSiliconInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspMultiPhaseSiInitApi API
+;
+; This FSP API provides multi-phase silicon initialization, which brings greater
+; modularity beyond the existing FspSiliconInit() API.
+; Increased modularity is achieved by adding an extra API to FSP-S.
+; This allows the bootloader to add board specific initialization steps throughout
+; the SiliconInit flow as needed.
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspMultiPhaseSiInitApi)
+ASM_PFX(FspMultiPhaseSiInitApi):
+ mov eax, 6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ ;
+ ; Handle FspMultiPhaseSiInitApiIndex API
+ ;
+ cmp eax, 6
+ jnz NotMultiPhaseSiInitApi
+
+ pushad
+ push DWORD [esp + (4 * 8 + 4)] ; push ApiParam
+ push eax ; push ApiIdx
+ call ASM_PFX(FspMultiPhaseSiInitApiHandler)
+ add esp, 8
+ mov dword [esp + (4 * 7)], eax
+ popad
+ ret
+
+NotMultiPhaseSiInitApi:
+ jmp $
+ ret
+
+;----------------------------------------------------------------------------
+; TempRamInit API
+;
+; Empty function for WHOLEARCHIVE build option
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(TempRamInitApi)
+ASM_PFX(TempRamInitApi):
+ jmp $
+ ret
+
+;----------------------------------------------------------------------------
+; Module Entrypoint API
+;----------------------------------------------------------------------------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
index bb4451b145..26ae7d9fd3 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
@@ -1,7 +1,7 @@
;; @file
; Provide FSP API entry points.
;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;;

@@ -62,6 +62,9 @@ FspApiCommon2:
cmp eax, 3 ; FspMemoryInit API
jz FspApiCommon3

+ cmp eax, 6 ; FspMultiPhaseSiInitApiIndex API
+ jz FspApiCommon3
+
call ASM_PFX(AsmGetFspInfoHeader)
jmp ASM_PFX(Loader2PeiSwitchStack)

diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index dcf489dbe6..2887848f69 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -2,7 +2,7 @@
Intel FSP API definition from Intel Firmware Support Package External
Architecture Specification v2.0.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,6 +10,8 @@
#ifndef _FSP_API_H_
#define _FSP_API_H_

+#include <Pi/PiStatusCode.h>
+
///
/// FSP Reset Status code
/// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code
@@ -24,6 +26,59 @@
#define FSP_STATUS_RESET_REQUIRED_8 0x40000008
/// @}

+/*
+ FSP may optionally include the capability of generating events messages to aid in the debugging of firmware issues.
+ These events fall under three catagories: Error, Progress, and Debug. The event reporting mechanism follows the
+ status code services described in section 6 and 7 of the PI Specification v1.7 Volume 3.
+
+ @param[in] Type Indicates the type of event being reported.
+ See MdePkg/Include/Pi/PiStatusCode.h for the definition of EFI_STATUS_CODE_TYPE.
+ @param[in] Value Describes the current status of a hardware or software entity.
+ This includes information about the class and subclass that is used to classify the entity as well as an operation.
+ For progress events, the operation is the current activity. For error events, it is the exception.
+ For debug events, it is not defined at this time.
+ See MdePkg/Include/Pi/PiStatusCode.h for the definition of EFI_STATUS_CODE_VALUE.
+ @param[in] Instance The enumeration of a hardware or software entity within the system.
+ A system may contain multiple entities that match a class/subclass pairing. The instance differentiates between them.
+ An instance of 0 indicates that instance information is unavailable, not meaningful, or not relevant.
+ Valid instance numbers start with 1.
+ @param[in] *CallerId This parameter can be used to identify the sub-module within the FSP generating the event.
+ This parameter may be NULL.
+ @param[in] *Data This optional parameter may be used to pass additional data. The contents can have event-specific data.
+ For example, the FSP provides a EFI_STATUS_CODE_STRING_DATA instance to this parameter when sending debug messages.
+ This parameter is NULL when no additional data is provided.
+
+ @retval EFI_SUCCESS The event was handled successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_DEVICE_ERROR The event handler failed.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_EVENT_HANDLER) (
+ IN EFI_STATUS_CODE_TYPE Type,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN OPTIONAL EFI_GUID *CallerId,
+ IN OPTIONAL EFI_STATUS_CODE_DATA *Data
+ );
+
+/*
+ Handler for FSP-T debug log messages, provided by the bootloader.
+
+ @param[in] DebugMessage A pointer to the debug message to be written to the log.
+ @param[in] MessageLength Number of bytes to written to the debug log.
+
+ @retval UINT32 The return value indicates the number of bytes actually written to
+ the debug log. If the return value is less than MessageLength,
+ an error occurred.
+*/
+typedef
+UINT32
+(EFIAPI *FSP_DEBUG_HANDLER) (
+ IN CHAR8* DebugMessage,
+ IN UINT32 MessageLength
+ );
+
#pragma pack(1)
///
/// FSP_UPD_HEADER Configuration.
@@ -77,7 +132,12 @@ typedef struct {
/// Current boot mode.
///
UINT32 BootMode;
- UINT8 Reserved1[8];
+ ///
+ /// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
+ /// This value is only valid if Revision is >= 2.
+ ///
+ FSP_EVENT_HANDLER *FspEventHandler;
+ UINT8 Reserved1[4];
} FSPM_ARCH_UPD;

///
@@ -147,6 +207,40 @@ typedef struct {
FSP_INIT_PHASE Phase;
} NOTIFY_PHASE_PARAMS;

+///
+/// Action definition for FspMultiPhaseSiInit API
+///
+typedef enum {
+ EnumMultiPhaseGetNumberOfPhases = 0x0,
+ EnumMultiPhaseExecutePhase = 0x1
+} FSP_MULTI_PHASE_ACTION;
+
+///
+/// Data structure returned by FSP when bootloader calling
+/// FspMultiPhaseSiInit API with action 0 (EnumMultiPhaseGetNumberOfPhases)
+///
+typedef struct {
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
+
+///
+/// FspMultiPhaseSiInit function parameter.
+///
+/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
+/// - PhaseIndex must be 0.
+/// - MultiPhaseParamPtr should point to an instance of FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
+///
+/// For action 1 (EnumMultiPhaseExecutePhase):
+/// - PhaseIndex will be the phase that will be executed by FSP.
+/// - MultiPhaseParamPtr shall be NULL.
+///
+typedef struct {
+ IN FSP_MULTI_PHASE_ACTION MultiPhaseAction;
+ IN UINT32 PhaseIndex;
+ IN OUT VOID *MultiPhaseParamPtr;
+} FSP_MULTI_PHASE_PARAMS;
+
#pragma pack()

/**
@@ -279,4 +373,28 @@ EFI_STATUS
IN VOID *FspsUpdDataPtr
);

+/**
+ This FSP API is expected to be called after FspSiliconInit but before FspNotifyPhase.
+ This FSP API provides multi-phase silicon initialization; which brings greater modularity
+ beyond the existing FspSiliconInit() API. Increased modularity is achieved by adding an
+ extra API to FSP-S. This allows the bootloader to add board specific initialization steps
+ throughout the SiliconInit flow as needed.
+
+ @param[in,out] FSP_MULTI_PHASE_PARAMS For action - EnumMultiPhaseGetNumberOfPhases:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr will contain
+ how many phases supported by FSP.
+ For action - EnumMultiPhaseExecutePhase:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr shall be NULL.
+ @retval EFI_SUCCESS FSP execution environment was initialized successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
+ @retval EFI_DEVICE_ERROR FSP initialization failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
+ IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
+);
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
index 1896b0240a..02df8463ed 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -22,6 +22,7 @@ typedef enum {
FspMemoryInitApiIndex,
TempRamExitApiIndex,
FspSiliconInitApiIndex,
+ FspMultiPhaseSiInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index 16f43a1273..3474bac1de 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -1,8 +1,8 @@
/** @file
Intel FSP Header File definition from Intel Firmware Support Package External
- Architecture Specification v2.0.
+ Architecture Specification v2.0 and above.

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -110,6 +110,12 @@ typedef struct {
/// Byte 0x44: The offset for the API to initialize the CPU and chipset.
///
UINT32 FspSiliconInitEntryOffset;
+ ///
+ /// Byte 0x48: Offset for the API for the optional Multi-Phase processor and chipset initialization.
+ /// This value is only valid if FSP HeaderRevision is >= 5.
+ /// If the value is set to 0x00000000, then this API is not available in this component.
+ ///
+ UINT32 FspMultiPhaseSiInitEntryOffset;
} FSP_INFO_HEADER;

///
diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
index 4d01b5f6d9..51a0309aed 100644
--- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
+++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -79,4 +79,18 @@ FspUpdSignatureCheck (
IN VOID *ApiParam
);

+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ );
+
#endif
--
2.13.3.windows.1


Re: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

Chiu, Chasel
 

Thanks Nate, and sorry that I misunderstood something in previous patch.
I have sent V5 to address all your feedbacks, please help to review again.

Thanks,
Chasel

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@...>
Sent: Sunday, May 10, 2020 2:22 PM
To: Chiu, Chasel <chasel.chiu@...>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Zeng, Star <star.zeng@...>
Subject: RE: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support Multi-Phase
SiInit and debug handlers.

Hi Chasel,

A few comments:

#1) FspApi.h

The references to the FSP/PI spec sections in the comments are a bit odd.
None of the existing comments in FspApi.h make references to sections in
specs. Particularly weird are these ones:

* See Section 11.10 Appendix A - Data Structures for the definition of
EFI_STATUS_CODE_TYPE.
* See Section 11.10 Appendix A - Data Structures for the definition of
EFI_STATUS_CODE_VALUE.

This is code... why not point the reader to PiStatusCode.h directly?

#2) FspApi.h

This seems overly verbose:

@retval UINT32 The return value will be passed
back through the EAX register.
The return value indicates the
number of bytes actually written to
the debug log. If the return value
is less than MessageLength,
an error occurred.

I think it can be shortened to:

@retval UINT32 The return value indicates the
number of bytes actually written to
the debug log. If the return value
is less than MessageLength,
an error occurred.

#3) Please rename FspSecCoreSS.inf to Fsp22SecCoreS.inf and
FspApiEntrySS.nasm to Fsp22ApiEntryS.nasm

#4) Any reason why the entry points for FspSiliconInit() and NotifyPhase()
are absent from FspApiEntrySS.nasm/Fsp22ApiEntry.nasm? Are we going to
need 2 FspSecCoreS drivers now instead of 1? Not sure how that would
work... only 1 SecCore is allowed per FV.

Thanks,
Nate

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@...>
Sent: Thursday, May 7, 2020 6:29 PM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>
Subject: [edk2-platforms: PATCH v4] IntelFsp2Pkg: Support Multi-Phase
SiInit and debug handlers.

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

To enhance FSP silicon initialization flexibility an optional
Multi-Phase API is introduced and FSP header needs update for new API
offset. Also new SecCore module created for FspMultiPhaseSiInit API

New ARCH_UPD introduced for enhancing FSP debug message flexibility
now bootloader can pass its own debug handler function pointer and FSP
will call the function to handle debug message.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
| 6 +++---
IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c |
19
++++++++++++++++++-
IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
| 48
++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
| 5
++++-
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
| 68
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++
IntelFsp2Pkg/Include/FspEas/FspApi.h
| 124
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++--
IntelFsp2Pkg/Include/FspGlobalData.h
| 3 ++-
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
| 10 ++++++++--
IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h |
16
+++++++++++++++-
9 files changed, 288 insertions(+), 11 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index 8e0595fe9a..1334959005 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2016 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -59,7 +59,7 @@ FspApiCallingCheck (
Status = EFI_UNSUPPORTED;
}
}
- } else if (ApiIdx == FspSiliconInitApiIndex) {
+ } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx ==
+ FspMultiPhaseSiInitApiIndex)) {
//
// FspSiliconInit check
//
@@ -68,7 +68,7 @@ FspApiCallingCheck (
} else {
if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
Status = EFI_UNSUPPORTED;
- } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) {
+ } else if (EFI_ERROR (FspUpdSignatureCheck
+ (FspSiliconInitApiIndex, ApiParam))) {
Status = EFI_INVALID_PARAMETER;
}
}
diff --git
a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
index f7945b5240..df8c5d121f 100644
---
a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
+++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull
+++ .c
@@ -1,7 +1,7 @@
/** @file
Null instance of Platform Sec Lib.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -25,3 +25,20 @@ FspUpdSignatureCheck ( {
return EFI_SUCCESS;
}
+
+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
b/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
new file mode 100644
index 0000000000..184101c7d3
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreSS.inf
@@ -0,0 +1,48 @@
+## @file
+# Sec Core for FSP to support MultiPhase (SeparatePhase)
SiInitialization.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> #
+#
+SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = FspSecCoreSS
+ FILE_GUID =
DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by
+the
build tools.
+#
+# VALID_ARCHITECTURES = IA32
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.IA32]
+ Ia32/Stack.nasm
+ Ia32/FspApiEntrySS.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
index bb4451b145..26ae7d9fd3 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
@@ -1,7 +1,7 @@
;; @file
; Provide FSP API entry points.
;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2016 - 2020, Intel Corporation. All rights
+reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent ;;

@@ -62,6 +62,9 @@ FspApiCommon2:
cmp eax, 3 ; FspMemoryInit API
jz FspApiCommon3

+ cmp eax, 6 ; FspMultiPhaseSiInitApiIndex API
+ jz FspApiCommon3
+
call ASM_PFX(AsmGetFspInfoHeader)
jmp ASM_PFX(Loader2PeiSwitchStack)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
new file mode 100644
index 0000000000..e6956277a9
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntrySS.nasm
@@ -0,0 +1,68 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> ;
+SPDX-License-Identifier: BSD-2-Clause-Patent ;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C ; extern
+ASM_PFX(FspApiCommon) extern
ASM_PFX(FspMultiPhaseSiInitApiHandler)
+
+;--------------------------------------------------------------------
+--
+------
+; FspMultiPhaseSiInitApi API
+;
+; This FSP API provides multi-phase silicon initialization, which
+brings greater ; modularity beyond the existing FspSiliconInit() API.
+; Increased modularity is achieved by adding an extra API to FSP-S.
+; This allows the bootloader to add board specific initialization
+steps throughout ; the SiliconInit flow as needed.
+;
+;--------------------------------------------------------------------
+--
+------
+global ASM_PFX(FspMultiPhaseSiInitApi)
+ASM_PFX(FspMultiPhaseSiInitApi):
+ mov eax, 6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;--------------------------------------------------------------------
+--
+------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution
+;
+;--------------------------------------------------------------------
+--
+------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ ;
+ ; Handle FspMultiPhaseSiInitApiIndex API
+ ;
+ pushad
+ push DWORD [esp + (4 * 8 + 4)] ; push ApiParam
+ push eax ; push ApiIdx
+ call ASM_PFX(FspMultiPhaseSiInitApiHandler)
+ add esp, 8
+ mov dword [esp + (4 * 7)], eax
+ popad
+ ret
+
+;--------------------------------------------------------------------
+--
+------
+; TempRamInit API
+;
+; Empty function for WHOLEARCHIVE build option ;
+;--------------------------------------------------------------------
+--
+------
+global ASM_PFX(TempRamInitApi)
+ASM_PFX(TempRamInitApi):
+ jmp $
+ ret
+
+;--------------------------------------------------------------------
+--
+------
+; Module Entrypoint API
+;--------------------------------------------------------------------
+--
+------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+
diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index dcf489dbe6..ca908b0198 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -2,7 +2,7 @@
Intel FSP API definition from Intel Firmware Support Package External
Architecture Specification v2.0.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,6 +10,8 @@
#ifndef _FSP_API_H_
#define _FSP_API_H_

+#include <Pi/PiStatusCode.h>
+
///
/// FSP Reset Status code
/// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status
Code @@ -
24,6 +26,60 @@
#define FSP_STATUS_RESET_REQUIRED_8 0x40000008
/// @}

+/*
+ FSP may optionally include the capability of generating events
+messages to
aid in the debugging of firmware issues.
+ These events fall under three catagories: Error, Progress, and Debug.
+The event reporting mechanism follows the
+ status code services described in section 6 and 7 of the PI
+Specification v1.7
Volume 3.
+
+ @param[in] Type Indicates the type of event
being reported.
+ See Section 11.10 Appendix A
-
+ Data Structures for the
definition of EFI_STATUS_CODE_TYPE.
+ @param[in] Value Describes the current status of
a hardware or
software entity.
+ This includes information
about
+ the class and subclass that
is used to classify the entity as well as an operation.
+ For progress events, the
operation is the current activity.
For error events, it is the exception.
+ For debug events, it is not
defined at this time.
+ See Section 11.10 Appendix A
-
+ Data Structures for the
definition of EFI_STATUS_CODE_VALUE.
+ @param[in] Instance The enumeration of a hardware
or software
entity within the system.
+ A system may contain multiple
+ entities that match a
class/subclass pairing. The instance differentiates between them.
+ An instance of 0 indicates that
+ instance information is
unavailable, not meaningful, or not relevant.
+ Valid instance numbers start
with 1.
+ @param[in] *CallerId This parameter can be used to
identify the
sub-module within the FSP generating the event.
+ This parameter may be NULL.
+ @param[in] *Data This optional parameter may
be used to pass
additional data. The contents can have event-specific data.
+ For example, the FSP provides
a
EFI_STATUS_CODE_STRING_DATA instance to this parameter when sending
debug messages.
+ This parameter is NULL when
no
+ additional data is
provided.
+
+ @retval EFI_SUCCESS The event was handled
successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_DEVICE_ERROR The event handler failed.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_EVENT_HANDLER) (
+ IN EFI_STATUS_CODE_TYPE Type,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN OPTIONAL EFI_GUID *CallerId,
+ IN OPTIONAL EFI_STATUS_CODE_DATA *Data
+ );
+
+/*
+ Handler for FSP-T debug log messages, provided by the bootloader.
+
+ @param[in] DebugMessage A pointer to the debug
message to be
written to the log.
+ @param[in] MessageLength Number of bytes to written to
the debug
log.
+
+ @retval UINT32 The return value will be passed
back through the
EAX register.
+ The return value indicates the
+ number of bytes actually
written to
+ the debug log. If the return
+ value is less than
MessageLength,
+ an error occurred.
+*/
+typedef
+UINT32
+(EFIAPI *FSP_DEBUG_HANDLER) (
+ IN CHAR8* DebugMessage,
+ IN UINT32 MessageLength
+ );
+
#pragma pack(1)
///
/// FSP_UPD_HEADER Configuration.
@@ -77,7 +133,13 @@ typedef struct {
/// Current boot mode.
///
UINT32 BootMode;
- UINT8 Reserved1[8];
+ ///
+ /// A function pointer of type FSP_EVENT_HANDLER.
+ /// Optional event handler for the bootloader to be informed of
+ events
occurring during FSP execution.
+ /// Refer to Section 8.5 for more details. This value is only valid
+ if Revision is
= 2.
+ ///
+ FSP_EVENT_HANDLER *FspEventHandler;
+ UINT8 Reserved1[4];
} FSPM_ARCH_UPD;

///
@@ -147,6 +209,40 @@ typedef struct {
FSP_INIT_PHASE Phase;
} NOTIFY_PHASE_PARAMS;

+///
+/// Action definition for FspMultiPhaseSiInit API /// typedef enum {
+ EnumMultiPhaseGetNumberOfPhases = 0x0,
+ EnumMultiPhaseExecutePhase = 0x1
+} FSP_MULTI_PHASE_ACTION;
+
+///
+/// Data structure returned by FSP when bootloader calling ///
+FspMultiPhaseSiInit API with action 0
(EnumMultiPhaseGetNumberOfPhases)
+/// typedef struct {
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
+
+///
+/// FspMultiPhaseSiInit function parameter.
+///
+/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
+/// - PhaseIndex must be 0.
+/// - MultiPhaseParamPtr should point to an instance of
FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
+///
+/// For action 1 (EnumMultiPhaseExecutePhase):
+/// - PhaseIndex will be the phase that will be executed by FSP.
+/// - MultiPhaseParamPtr shall be NULL.
+///
+typedef struct {
+ IN FSP_MULTI_PHASE_ACTION MultiPhaseAction;
+ IN UINT32 PhaseIndex;
+ IN OUT VOID *MultiPhaseParamPtr;
+} FSP_MULTI_PHASE_PARAMS;
+
#pragma pack()

/**
@@ -279,4 +375,28 @@ EFI_STATUS
IN VOID *FspsUpdDataPtr
);

+/**
+ This FSP API is expeted to be called after FspSiliconInit but
+before
FspNotifyPhase.
+ This FSP API provides multi-phase silicon initialization; which
+brings greater modularity
+ beyond the existing FspSiliconInit() API. Increased modularity is
+achieved by adding an
+ extra API to FSP-S. This allows the bootloader to add board
+specific initialization steps
+ throughout the SiliconInit flow as needed.
+
+ @param[in,out] FSP_MULTI_PHASE_PARAMS For action -
EnumMultiPhaseGetNumberOfPhases:
+
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
will contain
+ how many phases
supported by FSP.
+ For action -
EnumMultiPhaseExecutePhase:
+
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr
shall be NULL.
+ @retval EFI_SUCCESS FSP execution
environment was initialized
successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are
invalid.
+ @retval EFI_UNSUPPORTED The FSP calling
conditions were not
met.
+ @retval EFI_DEVICE_ERROR FSP initialization failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required.
These
status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
+ IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
+);
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
b/IntelFsp2Pkg/Include/FspGlobalData.h
index 1896b0240a..02df8463ed 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2014 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -22,6 +22,7 @@ typedef enum {
FspMemoryInitApiIndex,
TempRamExitApiIndex,
FspSiliconInitApiIndex,
+ FspMultiPhaseSiInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index 16f43a1273..3474bac1de 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -1,8 +1,8 @@
/** @file
Intel FSP Header File definition from Intel Firmware Support
Package External
- Architecture Specification v2.0.
+ Architecture Specification v2.0 and above.

- Copyright (c) 2014 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -110,6 +110,12 @@ typedef struct {
/// Byte 0x44: The offset for the API to initialize the CPU and chipset.
///
UINT32 FspSiliconInitEntryOffset;
+ ///
+ /// Byte 0x48: Offset for the API for the optional Multi-Phase
+ processor
and chipset initialization.
+ /// This value is only valid if FSP HeaderRevision is >= 5.
+ /// If the value is set to 0x00000000, then this API is not
available in
this component.
+ ///
+ UINT32 FspMultiPhaseSiInitEntryOffset;
} FSP_INFO_HEADER;

///
diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
index 4d01b5f6d9..51a0309aed 100644
--- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
+++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2015 - 2019, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2015 - 2020, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -79,4 +79,18 @@ FspUpdSignatureCheck (
IN VOID *ApiParam
);

+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ );
+
#endif
--
2.13.3.windows.1


[PATCH v5] IntelFsp2Pkg: Support Multi-Phase SiInit and debug handlers.

Chiu, Chasel
 

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

To enhance FSP silicon initialization flexibility an optional
Multi-Phase API is introduced and FSP header needs update for
new API offset. Also new SecCore module created for
FspMultiPhaseSiInit API

New ARCH_UPD introduced for enhancing FSP debug message
flexibility now bootloader can pass its own debug handler
function pointer and FSP will call the function to handle
debug message.

Cc: Maurice Ma <maurice.ma@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 6 +++---
IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c | 19 ++++++++++++++++++-
IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm | 5 ++++-
IntelFsp2Pkg/Include/FspEas/FspApi.h | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
IntelFsp2Pkg/Include/FspGlobalData.h | 3 ++-
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 10 ++++++++--
IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h | 16 +++++++++++++++-
9 files changed, 322 insertions(+), 11 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
index 8e0595fe9a..1334959005 100644
--- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
+++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -59,7 +59,7 @@ FspApiCallingCheck (
Status = EFI_UNSUPPORTED;
}
}
- } else if (ApiIdx == FspSiliconInitApiIndex) {
+ } else if ((ApiIdx == FspSiliconInitApiIndex) || (ApiIdx == FspMultiPhaseSiInitApiIndex)) {
//
// FspSiliconInit check
//
@@ -68,7 +68,7 @@ FspApiCallingCheck (
} else {
if (FspData->Signature != FSP_GLOBAL_DATA_SIGNATURE) {
Status = EFI_UNSUPPORTED;
- } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) {
+ } else if (EFI_ERROR (FspUpdSignatureCheck (FspSiliconInitApiIndex, ApiParam))) {
Status = EFI_INVALID_PARAMETER;
}
}
diff --git a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
index f7945b5240..df8c5d121f 100644
--- a/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
+++ b/IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/PlatformSecLibNull.c
@@ -1,7 +1,7 @@
/** @file
Null instance of Platform Sec Lib.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -25,3 +25,20 @@ FspUpdSignatureCheck (
{
return EFI_SUCCESS;
}
+
+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ )
+{
+ return EFI_SUCCESS;
+}
diff --git a/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
new file mode 100644
index 0000000000..0a24eb2a8b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
@@ -0,0 +1,52 @@
+## @file
+# Sec Core for FSP to support MultiPhase (SeparatePhase) SiInitialization.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Fsp22SecCoreS
+ FILE_GUID = DF0FCD70-264A-40BF-BBD4-06C76DB19CB1
+ MODULE_TYPE = SEC
+ VERSION_STRING = 1.0
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32
+#
+
+[Sources]
+ SecFspApiChk.c
+ SecFsp.h
+
+[Sources.IA32]
+ Ia32/Stack.nasm
+ Ia32/Fsp22ApiEntryS.nasm
+ Ia32/FspApiEntryCommon.nasm
+ Ia32/FspHelper.nasm
+
+[Binaries.Ia32]
+ RAW|Vtf0/Bin/ResetVec.ia32.raw |GCC
+
+[Packages]
+ MdePkg/MdePkg.dec
+ IntelFsp2Pkg/IntelFsp2Pkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ BaseLib
+ PciCf8Lib
+ SerialPortLib
+ FspSwitchStackLib
+ FspCommonLib
+ FspSecPlatformLib
+
+[Ppis]
+ gEfiTemporaryRamSupportPpiGuid ## PRODUCES
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
new file mode 100644
index 0000000000..c5e73a635b
--- /dev/null
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp22ApiEntryS.nasm
@@ -0,0 +1,99 @@
+;; @file
+; Provide FSP API entry points.
+;
+; Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;;
+
+ SECTION .text
+
+;
+; Following functions will be provided in C
+;
+extern ASM_PFX(FspApiCommon)
+extern ASM_PFX(FspMultiPhaseSiInitApiHandler)
+
+;----------------------------------------------------------------------------
+; NotifyPhase API
+;
+; This FSP API will notify the FSP about the different phases in the boot
+; process
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(NotifyPhaseApi)
+ASM_PFX(NotifyPhaseApi):
+ mov eax, 2 ; FSP_API_INDEX.NotifyPhaseApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspSiliconInit API
+;
+; This FSP API initializes the CPU and the chipset including the IO
+; controllers in the chipset to enable normal operation of these devices.
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspSiliconInitApi)
+ASM_PFX(FspSiliconInitApi):
+ mov eax, 5 ; FSP_API_INDEX.FspSiliconInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspMultiPhaseSiInitApi API
+;
+; This FSP API provides multi-phase silicon initialization, which brings greater
+; modularity beyond the existing FspSiliconInit() API.
+; Increased modularity is achieved by adding an extra API to FSP-S.
+; This allows the bootloader to add board specific initialization steps throughout
+; the SiliconInit flow as needed.
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspMultiPhaseSiInitApi)
+ASM_PFX(FspMultiPhaseSiInitApi):
+ mov eax, 6 ; FSP_API_INDEX.FspMultiPhaseSiInitApiIndex
+ jmp ASM_PFX(FspApiCommon)
+
+;----------------------------------------------------------------------------
+; FspApiCommonContinue API
+;
+; This is the FSP API common entry point to resume the FSP execution
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(FspApiCommonContinue)
+ASM_PFX(FspApiCommonContinue):
+ ;
+ ; Handle FspMultiPhaseSiInitApiIndex API
+ ;
+ cmp eax, 6
+ jnz NotMultiPhaseSiInitApi
+
+ pushad
+ push DWORD [esp + (4 * 8 + 4)] ; push ApiParam
+ push eax ; push ApiIdx
+ call ASM_PFX(FspMultiPhaseSiInitApiHandler)
+ add esp, 8
+ mov dword [esp + (4 * 7)], eax
+ popad
+ ret
+
+NotMultiPhaseSiInitApi:
+ jmp $
+ ret
+
+;----------------------------------------------------------------------------
+; TempRamInit API
+;
+; Empty function for WHOLEARCHIVE build option
+;
+;----------------------------------------------------------------------------
+global ASM_PFX(TempRamInitApi)
+ASM_PFX(TempRamInitApi):
+ jmp $
+ ret
+
+;----------------------------------------------------------------------------
+; Module Entrypoint API
+;----------------------------------------------------------------------------
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+ jmp $
+
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
index bb4451b145..26ae7d9fd3 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm
@@ -1,7 +1,7 @@
;; @file
; Provide FSP API entry points.
;
-; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;;

@@ -62,6 +62,9 @@ FspApiCommon2:
cmp eax, 3 ; FspMemoryInit API
jz FspApiCommon3

+ cmp eax, 6 ; FspMultiPhaseSiInitApiIndex API
+ jz FspApiCommon3
+
call ASM_PFX(AsmGetFspInfoHeader)
jmp ASM_PFX(Loader2PeiSwitchStack)

diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h b/IntelFsp2Pkg/Include/FspEas/FspApi.h
index dcf489dbe6..e79995ed89 100644
--- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
+++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
@@ -2,7 +2,7 @@
Intel FSP API definition from Intel Firmware Support Package External
Architecture Specification v2.0.

- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,6 +10,8 @@
#ifndef _FSP_API_H_
#define _FSP_API_H_

+#include <Pi/PiStatusCode.h>
+
///
/// FSP Reset Status code
/// These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code
@@ -24,6 +26,59 @@
#define FSP_STATUS_RESET_REQUIRED_8 0x40000008
/// @}

+/*
+ FSP may optionally include the capability of generating events messages to aid in the debugging of firmware issues.
+ These events fall under three catagories: Error, Progress, and Debug. The event reporting mechanism follows the
+ status code services described in section 6 and 7 of the PI Specification v1.7 Volume 3.
+
+ @param[in] Type Indicates the type of event being reported.
+ See MdePkg/Include/Pi/PiStatusCode.h - Data Structures for the definition of EFI_STATUS_CODE_TYPE.
+ @param[in] Value Describes the current status of a hardware or software entity.
+ This includes information about the class and subclass that is used to classify the entity as well as an operation.
+ For progress events, the operation is the current activity. For error events, it is the exception.
+ For debug events, it is not defined at this time.
+ See MdePkg/Include/Pi/PiStatusCode.h - Data Structures for the definition of EFI_STATUS_CODE_VALUE.
+ @param[in] Instance The enumeration of a hardware or software entity within the system.
+ A system may contain multiple entities that match a class/subclass pairing. The instance differentiates between them.
+ An instance of 0 indicates that instance information is unavailable, not meaningful, or not relevant.
+ Valid instance numbers start with 1.
+ @param[in] *CallerId This parameter can be used to identify the sub-module within the FSP generating the event.
+ This parameter may be NULL.
+ @param[in] *Data This optional parameter may be used to pass additional data. The contents can have event-specific data.
+ For example, the FSP provides a EFI_STATUS_CODE_STRING_DATA instance to this parameter when sending debug messages.
+ This parameter is NULL when no additional data is provided.
+
+ @retval EFI_SUCCESS The event was handled successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_DEVICE_ERROR The event handler failed.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_EVENT_HANDLER) (
+ IN EFI_STATUS_CODE_TYPE Type,
+ IN EFI_STATUS_CODE_VALUE Value,
+ IN UINT32 Instance,
+ IN OPTIONAL EFI_GUID *CallerId,
+ IN OPTIONAL EFI_STATUS_CODE_DATA *Data
+ );
+
+/*
+ Handler for FSP-T debug log messages, provided by the bootloader.
+
+ @param[in] DebugMessage A pointer to the debug message to be written to the log.
+ @param[in] MessageLength Number of bytes to written to the debug log.
+
+ @retval UINT32 The return value indicates the number of bytes actually written to
+ the debug log. If the return value is less than MessageLength,
+ an error occurred.
+*/
+typedef
+UINT32
+(EFIAPI *FSP_DEBUG_HANDLER) (
+ IN CHAR8* DebugMessage,
+ IN UINT32 MessageLength
+ );
+
#pragma pack(1)
///
/// FSP_UPD_HEADER Configuration.
@@ -77,7 +132,13 @@ typedef struct {
/// Current boot mode.
///
UINT32 BootMode;
- UINT8 Reserved1[8];
+ ///
+ /// A function pointer of type FSP_EVENT_HANDLER.
+ /// Optional event handler for the bootloader to be informed of events occurring during FSP execution.
+ /// Refer to Section 8.5 for more details. This value is only valid if Revision is >= 2.
+ ///
+ FSP_EVENT_HANDLER *FspEventHandler;
+ UINT8 Reserved1[4];
} FSPM_ARCH_UPD;

///
@@ -147,6 +208,40 @@ typedef struct {
FSP_INIT_PHASE Phase;
} NOTIFY_PHASE_PARAMS;

+///
+/// Action definition for FspMultiPhaseSiInit API
+///
+typedef enum {
+ EnumMultiPhaseGetNumberOfPhases = 0x0,
+ EnumMultiPhaseExecutePhase = 0x1
+} FSP_MULTI_PHASE_ACTION;
+
+///
+/// Data structure returned by FSP when bootloader calling
+/// FspMultiPhaseSiInit API with action 0 (EnumMultiPhaseGetNumberOfPhases)
+///
+typedef struct {
+ UINT32 NumberOfPhases;
+ UINT32 PhasesExecuted;
+} FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS;
+
+///
+/// FspMultiPhaseSiInit function parameter.
+///
+/// For action 0 (EnumMultiPhaseGetNumberOfPhases):
+/// - PhaseIndex must be 0.
+/// - MultiPhaseParamPtr should point to an instance of FSP_MULTI_PHASE_GET_NUMBER_OF_PHASES_PARAMS.
+///
+/// For action 1 (EnumMultiPhaseExecutePhase):
+/// - PhaseIndex will be the phase that will be executed by FSP.
+/// - MultiPhaseParamPtr shall be NULL.
+///
+typedef struct {
+ IN FSP_MULTI_PHASE_ACTION MultiPhaseAction;
+ IN UINT32 PhaseIndex;
+ IN OUT VOID *MultiPhaseParamPtr;
+} FSP_MULTI_PHASE_PARAMS;
+
#pragma pack()

/**
@@ -279,4 +374,28 @@ EFI_STATUS
IN VOID *FspsUpdDataPtr
);

+/**
+ This FSP API is expeted to be called after FspSiliconInit but before FspNotifyPhase.
+ This FSP API provides multi-phase silicon initialization; which brings greater modularity
+ beyond the existing FspSiliconInit() API. Increased modularity is achieved by adding an
+ extra API to FSP-S. This allows the bootloader to add board specific initialization steps
+ throughout the SiliconInit flow as needed.
+
+ @param[in,out] FSP_MULTI_PHASE_PARAMS For action - EnumMultiPhaseGetNumberOfPhases:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr will contain
+ how many phases supported by FSP.
+ For action - EnumMultiPhaseExecutePhase:
+ FSP_MULTI_PHASE_PARAMS->MultiPhaseParamPtr shall be NULL.
+ @retval EFI_SUCCESS FSP execution environment was initialized successfully.
+ @retval EFI_INVALID_PARAMETER Input parameters are invalid.
+ @retval EFI_UNSUPPORTED The FSP calling conditions were not met.
+ @retval EFI_DEVICE_ERROR FSP initialization failed.
+ @retval FSP_STATUS_RESET_REQUIREDx A reset is required. These status codes will not be returned during S3.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *FSP_MULTI_PHASE_SI_INIT) (
+ IN FSP_MULTI_PHASE_PARAMS *MultiPhaseSiInitParamPtr
+);
+
#endif
diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h b/IntelFsp2Pkg/Include/FspGlobalData.h
index 1896b0240a..02df8463ed 100644
--- a/IntelFsp2Pkg/Include/FspGlobalData.h
+++ b/IntelFsp2Pkg/Include/FspGlobalData.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -22,6 +22,7 @@ typedef enum {
FspMemoryInitApiIndex,
TempRamExitApiIndex,
FspSiliconInitApiIndex,
+ FspMultiPhaseSiInitApiIndex,
FspApiIndexMax
} FSP_API_INDEX;

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
index 16f43a1273..3474bac1de 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -1,8 +1,8 @@
/** @file
Intel FSP Header File definition from Intel Firmware Support Package External
- Architecture Specification v2.0.
+ Architecture Specification v2.0 and above.

- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -110,6 +110,12 @@ typedef struct {
/// Byte 0x44: The offset for the API to initialize the CPU and chipset.
///
UINT32 FspSiliconInitEntryOffset;
+ ///
+ /// Byte 0x48: Offset for the API for the optional Multi-Phase processor and chipset initialization.
+ /// This value is only valid if FSP HeaderRevision is >= 5.
+ /// If the value is set to 0x00000000, then this API is not available in this component.
+ ///
+ UINT32 FspMultiPhaseSiInitEntryOffset;
} FSP_INFO_HEADER;

///
diff --git a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
index 4d01b5f6d9..51a0309aed 100644
--- a/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
+++ b/IntelFsp2Pkg/Include/Library/FspSecPlatformLib.h
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -79,4 +79,18 @@ FspUpdSignatureCheck (
IN VOID *ApiParam
);

+/**
+ This function handles FspMultiPhaseSiInitApi.
+
+ @param[in] ApiIdx Internal index of the FSP API.
+ @param[in] ApiParam Parameter of the FSP API.
+
+**/
+EFI_STATUS
+EFIAPI
+FspMultiPhaseSiInitApiHandler (
+ IN UINT32 ApiIdx,
+ IN VOID *ApiParam
+ );
+
#endif
--
2.13.3.windows.1


[PATCH v2] MdeModulePkg/RegularExpressionDxe: Optimize the code infrastructure

Zhang, Shenglei
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2712
OnigurumaIntrinsics.c is now not used improperly. So the implements
of function 'memcpy' and 'memset' are not linked for all tool chains,
which causes build failure with CLANG9 and XCODE. I remove
OnigurumaIntrinsics.c and move the necessary function
implements to OnigurumaUefiPort.c/h.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
.../OnigurumaIntrinsics.c | 48 -------------------
.../RegularExpressionDxe/OnigurumaUefiPort.c | 10 ++++
.../RegularExpressionDxe/OnigurumaUefiPort.h | 2 +
.../RegularExpressionDxe.inf | 1 -
4 files changed, 12 insertions(+), 49 deletions(-)
delete mode 100644 MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c
deleted file mode 100644
index 536c7e0f1b26..000000000000
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c
+++ /dev/null
@@ -1,48 +0,0 @@
-/** @file
-
- Provide intrinsics within Oniguruma
-
- (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
- Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Library/BaseMemoryLib.h>
-
-//
-// From CryptoPkg/IntrinsicLib
-//
-
-/* Copies bytes between buffers */
-#pragma function(memcpy)
-void * memcpy (void *dest, const void *src, unsigned int count)
-{
- return CopyMem (dest, src, (UINTN)count);
-}
-
-/* Sets buffers to a specified character */
-#pragma function(memset)
-void * memset (void *dest, char ch, unsigned int count)
-{
- //
- // NOTE: Here we use one base implementation for memset, instead of the direct
- // optimized SetMem() wrapper. Because the IntrinsicLib has to be built
- // without whole program optimization option, and there will be some
- // potential register usage errors when calling other optimized codes.
- //
-
- //
- // Declare the local variables that actually move the data elements as
- // volatile to prevent the optimizer from replacing this function with
- // the intrinsic memset()
- //
- volatile UINT8 *Pointer;
-
- Pointer = (UINT8 *)dest;
- while (count-- != 0) {
- *(Pointer++) = ch;
- }
-
- return dest;
-}
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
index b1604b378af3..743f16612cc6 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
@@ -86,3 +86,13 @@ void * realloc (void *ptr, size_t size)
return NULL;
}

+void * memcpy (void *dest, const void *src, unsigned int count)
+{
+ return CopyMem (dest, src, (UINTN)count);
+}
+
+void * memset (void *dest, char ch, unsigned int count)
+{
+ return SetMem(dest, ch, count);
+}
+
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
index 1a0b2daf473d..de98934c83dc 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
@@ -96,6 +96,8 @@ int EFIAPI sprintf_s (char *str, size_t sizeOfBuffer, char const *fmt, ...);
int strlen(const char* str);
void* malloc(size_t size);
void* realloc(void *ptr, size_t size);
+void * memcpy (void *dest, const void *src, unsigned int count);
+void * memset (void *dest, char ch, unsigned int count);

#define exit(n) ASSERT(FALSE);

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
index da63aa6eb418..84489c294279 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
@@ -20,7 +20,6 @@ [Sources]
RegularExpressionDxe.h
OnigurumaUefiPort.h
OnigurumaUefiPort.c
- OnigurumaIntrinsics.c | MSFT

# Wrapper header files start #
stdio.h
--
2.18.0.windows.1


Re: [PATCH 1/1] MdeModulePkg/RegularExpressionDxe: Optimize the code infrastructure

Liming Gao
 

Reviewed-by: Liming Gao <liming.gao@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, Shenglei
Sent: 2020年5月9日 14:34
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>
Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/RegularExpressionDxe: Optimize the code infrastructure

OnigurumaIntrinsics.c is now not used. So the implement of function 'memcpy' is now not., which causes build failure with CLANG9 and XCODE. I remove OnigurumaIntrinsics.c and move the necessary function implement to OnigurumaUefiPort.c/h.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
.../OnigurumaIntrinsics.c | 48 -------------------
.../RegularExpressionDxe/OnigurumaUefiPort.c | 5 ++ .../RegularExpressionDxe/OnigurumaUefiPort.h | 1 +
.../RegularExpressionDxe.inf | 1 -
4 files changed, 6 insertions(+), 49 deletions(-) delete mode 100644 MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c
deleted file mode 100644
index 536c7e0f1b26..000000000000
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaIntrinsics.c
+++ /dev/null
@@ -1,48 +0,0 @@
-/** @file
-
- Provide intrinsics within Oniguruma
-
- (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
- Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Library/BaseMemoryLib.h>
-
-//
-// From CryptoPkg/IntrinsicLib
-//
-
-/* Copies bytes between buffers */
-#pragma function(memcpy)
-void * memcpy (void *dest, const void *src, unsigned int count) -{
- return CopyMem (dest, src, (UINTN)count); -}
-
-/* Sets buffers to a specified character */ -#pragma function(memset) -void * memset (void *dest, char ch, unsigned int count) -{
- //
- // NOTE: Here we use one base implementation for memset, instead of the direct
- // optimized SetMem() wrapper. Because the IntrinsicLib has to be built
- // without whole program optimization option, and there will be some
- // potential register usage errors when calling other optimized codes.
- //
-
- //
- // Declare the local variables that actually move the data elements as
- // volatile to prevent the optimizer from replacing this function with
- // the intrinsic memset()
- //
- volatile UINT8 *Pointer;
-
- Pointer = (UINT8 *)dest;
- while (count-- != 0) {
- *(Pointer++) = ch;
- }
-
- return dest;
-}
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
index b1604b378af3..a893ac86959f 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c
@@ -86,3 +86,8 @@ void * realloc (void *ptr, size_t size)
return NULL;
}

+void * memcpy (void *dest, const void *src, unsigned int count) {
+ return CopyMem (dest, src, (UINTN)count); }
+
diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
index 1a0b2daf473d..dfe4498ce57f 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.h
@@ -96,6 +96,7 @@ int EFIAPI sprintf_s (char *str, size_t sizeOfBuffer, char const *fmt, ...); int strlen(const char* str);
void* malloc(size_t size);
void* realloc(void *ptr, size_t size);
+void * memcpy (void *dest, const void *src, unsigned int count);

#define exit(n) ASSERT(FALSE);

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
index da63aa6eb418..84489c294279 100644
--- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
+++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.i
+++ nf
@@ -20,7 +20,6 @@ [Sources]
RegularExpressionDxe.h
OnigurumaUefiPort.h
OnigurumaUefiPort.c
- OnigurumaIntrinsics.c | MSFT

# Wrapper header files start #
stdio.h
--
2.18.0.windows.1


Re: [edk2-rfc] GitHub Pull Request based Code Review Process

Michael D Kinney
 

Rebecca,

I agree that the first version should rerun CI checks
on every time commits are added to a PR or there is a
forced push to the PR.

Perhaps we should use Draft Pull Requests as a way
to indicate the content is not ready for code review
or CI checks yet.

https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests#draft-pull-requests

We also want emails added to the email archive when
the pull request is either abandoned or merged.
merify can add comments to a PR that are picked up
by the webhook.

I agree with reducing the number of services required.
There was feedback from Laszlo related to rebase for
pull requests using the current CI process. I will
do more investigations of GitHub features, webhook
features, and Mergify features to see if there is
simpler overall solution.

Mike

-----Original Message-----
From: Rebecca Cran <rebecca@...>
Sent: Sunday, May 10, 2020 2:44 PM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; rfc@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] GitHub Pull
Request based Code Review Process

Mike,

On 5/10/20 3:29 PM, Michael D Kinney wrote:

There is no difference between CI checks run during
code review
and the final CI checks before merge. I think it is
an interesting
conversation to decide how many times those CI checks
should be
run and if they should run automatically on every
change during
review or on demand.
I'd suggest following what other Github projects do,
which I think is to
run the CI checks automatically on every change that's
made in a pull
request - I don't know if it might also be necessary to
run them during
the merge, if master has changed in the meantime. That
gives the
_submitter_ feedback about any changes they need to
make, instead of
having to wait until the maintainer tells them their
change has broken
something: it speeds up the development process.

Mergify is more flexible. We want to make sure the
git history
is linear with not git merges and supports both
single patches
and patch series without squashing. GitHub merge
button by
default squashes all commits into a single commit.
Wouldn't disabling all but "Allow rebase merging" do
the same thing
without the additional potential failure point? Though
it sounds like
we've resolved the problems with Mergify, so it's not
important.

https://help.github.com/en/github/administering-a-
repository/configuring-commit-squashing-for-pull-
requests


--
Rebecca Cran


[edk2-staging/EdkRepo] [PATCH 4/4] EdkRepo: Remove support for deprecated Manifest-Repo content in edkrepo.cfg

Ashley E Desimone
 

Remove the remaining uses of manifest_repo_abs_local_path()
and update to use manifest_repo_abs_path where appropriate.

Remove the config prop definitions for the Manifest-Repo
section of the edkrepo.cfg file.

Remove the Manifest-Repo entry from the edkrepo.cfg file.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Puja Pandya <puja.pandya@...>
Cc: Erik Bjorge <erik.c.bjorge@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Prince Agyeman <prince.agyeman@...>
---
edkrepo/commands/clone_command.py | 3 ++-
edkrepo/commands/sync_command.py | 8 +++---
edkrepo/common/common_repo_functions.py | 45 ++++++++++++++++++++-------------
edkrepo/config/config_factory.py | 12 ++-------
edkrepo_installer/Vendor/edkrepo.cfg | 5 ----
5 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/edkrepo/commands/clone_command.py b/edkrepo/commands/clone_command.py
index b7f3158..179aaf7 100644
--- a/edkrepo/commands/clone_command.py
+++ b/edkrepo/commands/clone_command.py
@@ -66,7 +66,6 @@ class CloneCommand(EdkrepoCommand):

def run_command(self, args, config):
pull_all_manifest_repos(config['cfg_file'], config['user_cfg_file'], False)
- update_editor_config(config)

name_or_manifest = args.ProjectNameOrManifestFile
workspace_dir = args.Workspace
@@ -92,8 +91,10 @@ class CloneCommand(EdkrepoCommand):
# If this manifest is in a defined manifest repository validate the manifest within the manifest repo
if manifest_repo in cfg:
verify_single_manifest(config['cfg_file'], manifest_repo, global_manifest_path)
+ update_editor_config(config, config['cfg_file'].manifest_repo_abs_path(manifest_repo))
elif manifest_repo in user_cfg:
verify_single_manifest(config['user_cfg_file'], manifest_repo, global_manifest_path)
+ update_editor_config(config, config['user_cfg_file'].manifest_repo_abs_path(manifest_repo))

# Copy project manifest to local manifest dir and rename it Manifest.xml.
local_manifest_dir = os.path.join(workspace_dir, "repo")
diff --git a/edkrepo/commands/sync_command.py b/edkrepo/commands/sync_command.py
index d1db4f1..da99397 100644
--- a/edkrepo/commands/sync_command.py
+++ b/edkrepo/commands/sync_command.py
@@ -79,8 +79,6 @@ class SyncCommand(EdkrepoCommand):
return metadata

def run_command(self, args, config):
- update_editor_config(config)
-
workspace_path = get_workspace_path()
initial_manifest = get_workspace_manifest()
current_combo = initial_manifest.general_config.current_combo
@@ -97,6 +95,8 @@ class SyncCommand(EdkrepoCommand):
global_manifest_directory = config['user_cfg_file'].manifest_repo_abs_path(source_global_manifest_repo)
verify_single_manifest(config['user_cfg_file'], source_global_manifest_repo, get_workspace_manifest_file(), args.verbose)

+ update_editor_config(config, global_manifest_directory)
+
if not args.update_local_manifest:
self.__check_for_new_manifest(args, config, initial_manifest, workspace_path, global_manifest_directory)
check_dirty_repos(initial_manifest, workspace_path)
@@ -145,7 +145,7 @@ class SyncCommand(EdkrepoCommand):
for repo_to_sync in repo_sources_to_sync:
local_repo_path = os.path.join(workspace_path, repo_to_sync.root)
# Update any hooks
- update_hooks(hooks_add, hooks_update, hooks_uninstall, local_repo_path, repo_to_sync, config)
+ update_hooks(hooks_add, hooks_update, hooks_uninstall, local_repo_path, repo_to_sync, config, global_manifest_directory)
repo = Repo(local_repo_path)
#Fetch notes
repo.remotes.origin.fetch("refs/notes/*:refs/notes/*")
@@ -200,7 +200,7 @@ class SyncCommand(EdkrepoCommand):
# Perform submodule updates and url redirection
maintain_submodules(repo_to_sync, repo)
# Update commit message templates
- update_repo_commit_template(workspace_path, repo, repo_to_sync, config)
+ update_repo_commit_template(workspace_path, repo, repo_to_sync, config, source_global_manifest_repo)

if sync_error:
print(SYNC_ERROR)
diff --git a/edkrepo/common/common_repo_functions.py b/edkrepo/common/common_repo_functions.py
index 5ec834b..61133f1 100644
--- a/edkrepo/common/common_repo_functions.py
+++ b/edkrepo/common/common_repo_functions.py
@@ -59,8 +59,8 @@ from edkrepo.common.edkrepo_exception import EdkrepoInvalidParametersException
from edkrepo_manifest_parser.edk_manifest import CiIndexXml, ManifestXml
from edkrepo.common.edkrepo_exception import EdkrepoNotFoundException, EdkrepoGitException, EdkrepoWarningException
from edkrepo.common.edkrepo_exception import EdkrepoFoundMultipleException, EdkrepoHookNotFoundException
-from edkrepo.common.edkrepo_exception import EdkrepoGitConfigSetupException, EdkrepoManifestInvalidException
-from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import pull_single_manifest_repo
+from edkrepo.common.edkrepo_exception import EdkrepoGitConfigSetupException, EdkrepoManifestInvalidException, EdkrepoManifestNotFoundException
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import find_source_manifest_repo, list_available_manifest_repos
from edkrepo.common.workspace_maintenance.workspace_maintenance import case_insensitive_single_match
from edkrepo.common.ui_functions import init_color_console
from edkrepo_manifest_parser import edk_manifest
@@ -112,11 +112,27 @@ def clone_repos(args, workspace_dir, repos_to_clone, project_client_side_hooks,
if repo_to_clone.enable_submodule:
maintain_submodules(repo_to_clone, repo, args.verbose)

- # Install git hooks
- install_hooks(project_client_side_hooks, local_repo_path, repo_to_clone, config)
+ try:
+ if 'source_manifest_repo' in vars(args).keys():
+ src_manifest_repo = find_source_manifest_repo(manifest, config['cfg_file'], config['user_cfg_file'], args.source_manifest_repo)
+ else:
+ src_manifest_repo = find_source_manifest_repo(manifest, config['cfg_file'], config['user_cfg_file'], None)
+ except EdkrepoManifestNotFoundException:
+ src_manifest_repo = None
+ if src_manifest_repo:
+ cfg, user_cfg, conflicts = list_available_manifest_repos(config['cfg_file'], config['user_cfg_file'])
+ if src_manifest_repo in cfg:
+ global_manifest_directory = config['cfg_file'].manifest_repo_abs_path(src_manifest_repo)
+ elif src_manifest_repo in user_cfg:
+ global_manifest_directory = config['user_cfg_file'].manifest_repo_abs_path(src_manifest_repo)
+ else:
+ global_manifest_directory = None
+ if global_manifest_directory:
+ # Install git hooks if there is a manifest repo associated with the manifest being cloned
+ install_hooks(project_client_side_hooks, local_repo_path, repo_to_clone, config, global_manifest_directory)

- # Add the commit template if it exists.
- update_repo_commit_template(workspace_dir, repo, repo_to_clone, config)
+ # Add the commit template if it exists.
+ update_repo_commit_template(workspace_dir, repo, repo_to_clone, config, global_manifest_directory)

# Check to see if mirror is in sync with primary repo
if not in_sync_with_primary(repo, repo_to_clone, args.verbose):
@@ -194,7 +210,7 @@ def maintain_submodules(repo_sources, repo, verbose = False):
except:
raise EdkrepoGitException(SUBMODULE_FAILURE.format(repo_sources.remote_name))

-def install_hooks(hooks, local_repo_path, repo_for_install, config):
+def install_hooks(hooks, local_repo_path, repo_for_install, config, global_manifest_directory):
# Determine the which hooks are for the repo in question and which are from a URL based source or are in a global
# manifest repo relative path
hooks_url = []
@@ -222,7 +238,6 @@ def install_hooks(hooks, local_repo_path, repo_for_install, config):

# Copy any global manifest repository relative path source based hooks
for hook in hooks_path:
- global_manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
man_dir_rel_hook_path = os.path.join(global_manifest_directory, hook.source)
if not os.path.exists(man_dir_rel_hook_path):
raise EdkrepoHookNotFoundException(HOOK_NOT_FOUND_ERROR.format(hook.source, repo_for_install.root))
@@ -258,11 +273,11 @@ def uninstall_hooks(hooks, local_repo_path, repo_for_uninstall):
hook_file = os.path.join(destination, (os.path.basename(str(hook.source))))
os.remove(hook_file)

-def update_hooks (hooks_add, hooks_update, hooks_uninstall, local_repo_path, repo, config):
+def update_hooks (hooks_add, hooks_update, hooks_uninstall, local_repo_path, repo, config, global_manifest_directory):
if hooks_add:
- install_hooks(hooks_add, local_repo_path, repo, config)
+ install_hooks(hooks_add, local_repo_path, repo, config, global_manifest_directory)
if hooks_update:
- install_hooks(hooks_update, local_repo_path, repo, config)
+ install_hooks(hooks_update, local_repo_path, repo, config, global_manifest_directory)
if hooks_uninstall:
uninstall_hooks(hooks_uninstall, local_repo_path, repo)

@@ -562,15 +577,11 @@ def get_latest_sha(repo, branch, remote_or_url='origin'):
latest_sha = None
return latest_sha

-def update_repo_commit_template(workspace_dir, repo, repo_info, config):
+def update_repo_commit_template(workspace_dir, repo, repo_info, config, global_manifest_directory):
# Open the local manifest and get any templates
manifest = edk_manifest.ManifestXml(os.path.join(workspace_dir, 'repo', 'Manifest.xml'))
templates = manifest.commit_templates

- # Need to know where the global manifest directory is located at this point
- global_manifest_directory = os.path.join(get_edkrepo_global_data_directory(),
- config['cfg_file'].manifest_repo_local_path)
-
# Apply the template based on current manifest
with repo.config_writer() as cw:
if cw.has_option(section='commit', option='template'):
@@ -590,7 +601,7 @@ def update_repo_commit_template(workspace_dir, repo, repo_info, config):
if cw.has_option(section='commit', option='template'):
cw.remove_option(section='commit', option='template')

-def update_editor_config(config):
+def update_editor_config(config, global_manifest_directory):
return

def has_primary_repo_remote(repo, verbose=False):
diff --git a/edkrepo/config/config_factory.py b/edkrepo/config/config_factory.py
index 6d56ee6..c0b9f68 100644
--- a/edkrepo/config/config_factory.py
+++ b/edkrepo/config/config_factory.py
@@ -119,7 +119,7 @@ class BaseConfig():
"""Returns a list of available manifest repos"""
if self.cfg.has_section('manifest-repos'):
return self.cfg.options('manifest-repos')
-
+
def manifest_repo_props(self, manifest_repo):
"""
Returns a list of cfg_prop objects that pertain to a given manifest
@@ -128,7 +128,7 @@ class BaseConfig():
return [x for x in self.prop_list if manifest_repo in x.name]

def get_manifest_repo_url(self, manifest_repo):
- """
+ """
Returns the URL value for a given manifest repo based on config
file contents.
"""
@@ -172,9 +172,6 @@ class GlobalConfig(BaseConfig):
def __init__(self):
self.filename = os.path.join(get_edkrepo_global_data_directory(), "edkrepo.cfg")
self.prop_list = [
- CfgProp('manifest-repo', 'URL', 'manifest_repo_url', None, True),
- CfgProp('manifest-repo', 'Branch', 'manifest_repo_branch', None, True),
- CfgProp('manifest-repo', 'LocalPath', 'manifest_repo_local_path', None, True),
CfgProp('sparsecheckout', 'always_include', 'sparsecheckout_always_include', None, True),
CfgProp('sparsecheckout', 'always_exclude', 'sparsecheckout_always_exclude', None, True),
CfgProp('f2f-cherry-pick', 'ignored_folder_substrings', 'f2f_cp_ignored_folder_substrings'),
@@ -199,11 +196,6 @@ class GlobalConfig(BaseConfig):
pkg_list.append(pkg.strip())
return pkg_list

- @property
- def manifest_repo_abs_local_path(self):
- """Provides an absolute path to the manifest repo based on configuration file values."""
- return os.path.join(self.global_data_dir, self.manifest_repo_local_path)
-
@property
def sparsecheckout_data(self):
always_include = self.sparsecheckout_always_include.split('|')
diff --git a/edkrepo_installer/Vendor/edkrepo.cfg b/edkrepo_installer/Vendor/edkrepo.cfg
index 38d7b35..738b832 100644
--- a/edkrepo_installer/Vendor/edkrepo.cfg
+++ b/edkrepo_installer/Vendor/edkrepo.cfg
@@ -6,11 +6,6 @@ URL = https://github.com/tianocore/edk2-staging.git
Branch = EdkRepo-Manifest
LocalPath = edk2-staging-manifest-master

-[manifest-repo]
-URL = https://github.com/tianocore/edk2-staging.git
-Branch = EdkRepo-Manifest
-LocalPath = manifest-master
-
[git-ver]
minimum = 2.13.0
recommended = 2.16.2
--
2.16.2.windows.1


[edk2-staging/EdkRepo] [PATCH 3/4] EdkRepo: Add support for multiple manifest repostories to command completions

Ashley E Desimone
 

Update the command completions for the checkout pin command to support
multiple manifest repositories. If a source manifest repository cannot
be found for the current workspace then no completions will be provided.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Puja Pandya <puja.pandya@...>
Cc: Erik Bjorge <erik.c.bjorge@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Prince Agyeman <prince.agyeman@...>
---
edkrepo/command_completion_edkrepo.py | 48 +++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/edkrepo/command_completion_edkrepo.py b/edkrepo/command_completion_edkrepo.py
index 1220924..8a2c0e6 100644
--- a/edkrepo/command_completion_edkrepo.py
+++ b/edkrepo/command_completion_edkrepo.py
@@ -15,6 +15,9 @@ import traceback

from edkrepo_manifest_parser.edk_manifest import ManifestXml
from edkrepo.common.common_repo_functions import combinations_in_manifest
+from edkrepo.common.edkrepo_exception import EdkrepoManifestNotFoundException
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import list_available_manifest_repos
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import find_source_manifest_repo
from edkrepo.config import config_factory
from edkrepo.config.config_factory import get_workspace_manifest

@@ -28,23 +31,36 @@ def current_combo(parsed_args, config):

def checkout_pin(parsed_args, config):
pins = []
- manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
manifest = get_workspace_manifest()
- pin_folder = os.path.normpath(os.path.join(manifest_directory, manifest.general_config.pin_path))
- for dirpath, _, filenames in os.walk(pin_folder):
- for file in filenames:
- pin_file = os.path.join(dirpath, file)
- # Capture error output from manifest parser stdout so it is hidden unless verbose is enabled
- stdout = sys.stdout
- sys.stdout = io.StringIO()
- pin = ManifestXml(pin_file)
- parse_output = sys.stdout.getvalue()
- sys.stdout = stdout
- if parsed_args.verbose and parse_output.strip() != '':
- print('Pin {} Parsing Errors: {}\n'.format(file, parse_output.strip()))
- if pin.project_info.codename == manifest.project_info.codename:
- pins.append(file)
- print(' '.join(pins))
+ manifest_directory = None
+ try:
+ source_manifest_repo = find_source_manifest_repo(manifest, config['cfg_file'], config['user_cfg_file'], )
+ if source_manifest_repo:
+ cfg, user_cfg, conflicts = list_available_manifest_repos(config['cfg_file'], config['user_cfgFile'])
+ if source_manifest_repo in cfg:
+ manifest_directory = config['cfg_file'].manifest_repo_abs_path(source_manifest_repo)
+ elif source_manifest_repo in user_cfg:
+ manifest_directory = config['user_cfg_file'].manifest_repo_abs_path(source_manifest_repo)
+ else:
+ manifest_directory = None
+ except EdkrepoManifestNotFoundException:
+ manifest_directory = None
+ if manifest_directory:
+ pin_folder = os.path.normpath(os.path.join(manifest_directory, manifest.general_config.pin_path))
+ for dirpath, _, filenames in os.walk(pin_folder):
+ for file in filenames:
+ pin_file = os.path.join(dirpath, file)
+ # Capture error output from manifest parser stdout so it is hidden unless verbose is enabled
+ stdout = sys.stdout
+ sys.stdout = io.StringIO()
+ pin = ManifestXml(pin_file)
+ parse_output = sys.stdout.getvalue()
+ sys.stdout = stdout
+ if parsed_args.verbose and parse_output.strip() != '':
+ print('Pin {} Parsing Errors: {}\n'.format(file, parse_output.strip()))
+ if pin.project_info.codename == manifest.project_info.codename:
+ pins.append(file)
+ print(' '.join(pins))

# To add command completions for a new command, add an entry to this dictionary.
command_completions = {
--
2.16.2.windows.1


[edk2-staing/EdkRepo] [PATCH 2/4] EdkRepo: Remove unused functions from common_repo_functions.py

Ashley E Desimone
 

Remove unused functions from common_repo_functions.py and remove
any remaining imports of them.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Puja Pandya <puja.pandya@...>
Cc: Erik Bjorge <erik.c.bjorge@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Prince Agyeman <prince.agyeman@...>
---
edkrepo/commands/manifest_command.py | 2 +-
edkrepo/common/common_repo_functions.py | 40 ---------------------------------
edkrepo/common/humble.py | 4 ++--
3 files changed, 3 insertions(+), 43 deletions(-)

diff --git a/edkrepo/commands/manifest_command.py b/edkrepo/commands/manifest_command.py
index bb6252d..5c6184f 100644
--- a/edkrepo/commands/manifest_command.py
+++ b/edkrepo/commands/manifest_command.py
@@ -16,7 +16,7 @@ from edkrepo.commands.edkrepo_command import EdkrepoCommand
from edkrepo.commands.edkrepo_command import ColorArgument
import edkrepo.commands.arguments.manifest_args as arguments
from edkrepo.common.edkrepo_exception import EdkrepoWorkspaceInvalidException, EdkrepoManifestNotFoundException
-from edkrepo.common.common_repo_functions import pull_latest_manifest_repo, validate_manifest_repo
+from edkrepo.common.common_repo_functions import validate_manifest_repo
from edkrepo.common.ui_functions import init_color_console
from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import list_available_manifest_repos
from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import pull_all_manifest_repos
diff --git a/edkrepo/common/common_repo_functions.py b/edkrepo/common/common_repo_functions.py
index eb6c4c0..5ec834b 100644
--- a/edkrepo/common/common_repo_functions.py
+++ b/edkrepo/common/common_repo_functions.py
@@ -73,14 +73,6 @@ CLEAR_LINE = '\x1b[K'
DEFAULT_REMOTE_NAME = 'origin'
PRIMARY_REMOTE_NAME = 'primary'

-def pull_latest_manifest_repo(args, config, reset_hard=False):
- repo_url = config['cfg_file'].manifest_repo_url
- branch = config['cfg_file'].manifest_repo_branch
- local_path = config['cfg_file'].manifest_repo_local_path
- init_color_console(False)
- pull_single_manifest_repo(repo_url, branch, local_path, reset_hard)
-
-
def clone_repos(args, workspace_dir, repos_to_clone, project_client_side_hooks, config, skip_submodule, manifest):
for repo_to_clone in repos_to_clone:
local_repo_path = os.path.join(workspace_dir, repo_to_clone.root)
@@ -395,38 +387,6 @@ def checkout_repos(verbose, override, repos_to_checkout, workspace_path, manifes
if repo_to_checkout.enable_submodule:
maintain_submodules(repo_to_checkout, repo, verbose)

-def verify_manifest_data(global_manifest_directory, config, verbose=False, verify_all=False, verify_proj=None, verify_archived=False):
- # Validate the project individual project selected
- if verify_proj:
- print(VERIFY_PROJ.format(verify_proj))
- ci_index_path = os.path.join(config['cfg_file'].manifest_repo_abs_local_path, 'CiIndex.xml')
- ci_index = CiIndexXml(ci_index_path)
- try:
- proj_path = find_project_in_index(verify_proj, ci_index, config['cfg_file'].manifest_repo_abs_local_path, VERIFY_PROJ_NOT_IN_INDEX.format(verify_proj))
- except EdkrepoInvalidParametersException:
- raise
- if proj_path:
- proj_val_data = validate_manifestfiles([proj_path])
- proj_val_error = get_manifest_validation_status(proj_val_data)
- if proj_val_error:
- if verbose:
- print_manifest_errors(proj_val_data)
- raise EdkrepoManifestInvalidException(VERIFY_PROJ_FAIL.format(verify_proj))
-
- # Validate the entire global manifest repository.
- if verify_all:
- print(VERIFY_GLOBAL)
- if verify_archived:
- print(VERIFY_ARCHIVED)
- # Attempt to make sure the manifest data is good
- manifestfile_validation_data = validate_manifestrepo(global_manifest_directory, verify_archived)
- manifest_repo_error = get_manifest_validation_status(manifestfile_validation_data)
- # Display errors
- if manifest_repo_error:
- print(VERIFY_GLOBAL_FAIL)
- if verbose:
- print_manifest_errors(manifestfile_validation_data)
-
def validate_manifest_repo(manifest_repo, verbose=False, archived=False):
print(VERIFY_GLOBAL)
if archived:
diff --git a/edkrepo/common/humble.py b/edkrepo/common/humble.py
index 8ca38bb..f905357 100644
--- a/edkrepo/common/humble.py
+++ b/edkrepo/common/humble.py
@@ -41,7 +41,7 @@ SYNC_COMMITS_ON_MASTER = 'Commits were found on {0} branch.\n (use the "--overr
SYNC_ERROR = '\nError: Some repositories were not updated.'
SYNC_MANIFEST_NOT_FOUND = 'A manifest for project, {0}, was not found.\nTo complete this operation please rerun the command with the --override flag\n' + SYNC_EXIT
SYNC_URL_CHANGE = 'The URL for the remote, {0} has changed.\n' + SYNC_EXIT
-SYNC_COMBO_CHANGE = 'The current checked out combination, {0}, does not exist in the latest manifest for project, {1}\n'
+SYNC_COMBO_CHANGE = 'The current checked out combination, {0}, does not exist in the latest manifest for project, {1}\n'
SYNC_REPO_CHANGE = 'The latest manifest for project, {0}, requires a change in currently cloned repositories.\nTo complete this operation please rerun the command with the --override flag\n' + SYNC_EXIT
SYNC_SOURCE_MOVE_WARNING = '{}{}WARNING:{}{} {{}} being moved to {{}}'.format(Style.BRIGHT, Fore.RED, Style.RESET_ALL, Fore.RED)
SYNC_REMOVE_WARNING = '{}{}WARNING:{}{} The following repos no longer exist in the new manifest and can no \nlonger be used for submitting code. Please manually delete the following \ndirectories after saving any work you have in them:'.format(Style.BRIGHT, Fore.RED, Style.RESET_ALL, Fore.RED)
@@ -130,7 +130,7 @@ ERROR_WRITING_INCLUDE = 'An error occured while writting the URL redirection con
#Error messages for squash.py
SQUASH_COMMON_ANCESTOR_REQUIRED = '{} is not in the same branch history as {}, unable to operate on this commit range.'

-# Messages for common_repo_functions.verify_manifest_data()
+# Messages for common_repo_functions.
VERIFY_GLOBAL = 'Verifying the active projects in the global manifest repository\n'
VERIFY_ARCHIVED = 'Verifying the archived projects in the global manifest repository\n'
VERIFY_GLOBAL_FAIL = 'Unable to verify the contents of the global manifest repository\n'
--
2.16.2.windows.1


[edk2-stagin/EdkRepo] [PATCH 1/4] EdkRepo: Update sync to support multiple manifest repositories

Ashley E Desimone
 

Update the sync command to only update the global manifest repository
that the workspace is based on and to only check the source manfiest
repository for manifest updates.

Update the manifest parser to ignore the 'SourceManifestRepository'
field when conducting an equality operation.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Puja Pandya <puja.pandya@...>
Cc: Erik Bjorge <erik.c.bjorge@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Prince Agyeman <prince.agyeman@...>
---
edkrepo/commands/sync_command.py | 36 +++++++++++++++++-----------
edkrepo_manifest_parser/edk_manifest.py | 42 +++++++++++++++++++++++++--------
2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/edkrepo/commands/sync_command.py b/edkrepo/commands/sync_command.py
index 82f5247..d1db4f1 100644
--- a/edkrepo/commands/sync_command.py
+++ b/edkrepo/commands/sync_command.py
@@ -18,7 +18,7 @@ from git import Repo

# Our modules
from edkrepo.commands.edkrepo_command import EdkrepoCommand
-from edkrepo.commands.edkrepo_command import DryRunArgument, SubmoduleSkipArgument
+from edkrepo.commands.edkrepo_command import DryRunArgument, SubmoduleSkipArgument, SourceManifestRepoArgument
import edkrepo.commands.arguments.sync_args as arguments
from edkrepo.common.progress_handler import GitProgressHandler
from edkrepo.common.edkrepo_exception import EdkrepoUncommitedChangesException, EdkrepoManifestNotFoundException
@@ -32,8 +32,8 @@ from edkrepo.common.humble import MIRROR_BEHIND_PRIMARY_REPO, SYNC_NEEDS_REBASE,
from edkrepo.common.humble import SYNC_BRANCH_CHANGE_ON_LOCAL, SYNC_INCOMPATIBLE_COMBO
from edkrepo.common.humble import SYNC_REBASE_CALC_FAIL
from edkrepo.common.pathfix import get_actual_path
-from edkrepo.common.common_repo_functions import pull_latest_manifest_repo, clone_repos, sparse_checkout_enabled
-from edkrepo.common.common_repo_functions import reset_sparse_checkout, sparse_checkout, verify_manifest_data
+from edkrepo.common.common_repo_functions import clone_repos, sparse_checkout_enabled
+from edkrepo.common.common_repo_functions import reset_sparse_checkout, sparse_checkout, verify_single_manifest
from edkrepo.common.common_repo_functions import checkout_repos, check_dirty_repos
from edkrepo.common.common_repo_functions import update_editor_config
from edkrepo.common.common_repo_functions import update_repo_commit_template, get_latest_sha
@@ -41,8 +41,12 @@ from edkrepo.common.common_repo_functions import has_primary_repo_remote, fetch_
from edkrepo.common.common_repo_functions import update_hooks, maintain_submodules, combinations_in_manifest
from edkrepo.common.common_repo_functions import write_included_config, remove_included_config
from edkrepo.common.workspace_maintenance.workspace_maintenance import generate_name_for_obsolete_backup
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import pull_workspace_manifest_repo
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import find_source_manifest_repo
+from edkrepo.common.workspace_maintenance.manifest_repos_maintenance import list_available_manifest_repos
from edkrepo.common.ui_functions import init_color_console
from edkrepo.config.config_factory import get_workspace_path, get_workspace_manifest, get_edkrepo_global_data_directory
+from edkrepo.config.config_factory import get_workspace_manifest_file
from edkrepo_manifest_parser.edk_manifest import CiIndexXml, ManifestXml


@@ -71,23 +75,30 @@ class SyncCommand(EdkrepoCommand):
'required' : False,
'help-text' : arguments.OVERRIDE_HELP})
args.append(SubmoduleSkipArgument)
+ args.append(SourceManifestRepoArgument)
return metadata

def run_command(self, args, config):
update_editor_config(config)
+
workspace_path = get_workspace_path()
initial_manifest = get_workspace_manifest()
current_combo = initial_manifest.general_config.current_combo
initial_sources = initial_manifest.get_repo_sources(current_combo)
initial_hooks = initial_manifest.repo_hooks

- pull_latest_manifest_repo(args, config)
+ source_global_manifest_repo = find_source_manifest_repo(initial_manifest, config['cfg_file'], config['user_cfg_file'], args.source_manifest_repo)
+ pull_workspace_manifest_repo(initial_manifest, config['cfg_file'], config['user_cfg_file'], args.source_manifest_repo, False)
+ cfg_manifest_repos, user_cfg_manifest_repos, conflicts = list_available_manifest_repos(config['cfg_file'], config['user_cfg_file'])
+ if source_global_manifest_repo in cfg_manifest_repos:
+ global_manifest_directory = config['cfg_file'].manifest_repo_abs_path(source_global_manifest_repo)
+ verify_single_manifest(config['cfg_file'], source_global_manifest_repo, get_workspace_manifest_file(), args.verbose)
+ elif source_global_manifest_repo in user_cfg_manifest_repos:
+ global_manifest_directory = config['user_cfg_file'].manifest_repo_abs_path(source_global_manifest_repo)
+ verify_single_manifest(config['user_cfg_file'], source_global_manifest_repo, get_workspace_manifest_file(), args.verbose)

- # Verify that the latest version of the manifest in the global manifest repository is not broken
- global_manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
- verify_manifest_data(global_manifest_directory, config, verbose=args.verbose, verify_proj=initial_manifest.project_info.codename)
if not args.update_local_manifest:
- self.__check_for_new_manifest(args, config, initial_manifest, workspace_path)
+ self.__check_for_new_manifest(args, config, initial_manifest, workspace_path, global_manifest_directory)
check_dirty_repos(initial_manifest, workspace_path)
# Determine if sparse checkout needs to be disabled for this operation
sparse_settings = initial_manifest.sparse_settings
@@ -103,7 +114,7 @@ class SyncCommand(EdkrepoCommand):

# Get the latest manifest if requested
if args.update_local_manifest: #NOTE: hyphens in arg name replaced with underscores due to argparse
- self.__update_local_manifest(args, config, initial_manifest, workspace_path)
+ self.__update_local_manifest(args, config, initial_manifest, workspace_path, global_manifest_directory)
manifest = get_workspace_manifest()
if args.update_local_manifest:
try:
@@ -199,7 +210,7 @@ class SyncCommand(EdkrepoCommand):
print(SPARSE_CHECKOUT)
sparse_checkout(workspace_path, repo_sources_to_sync, manifest)

- def __update_local_manifest(self, args, config, initial_manifest, workspace_path):
+ def __update_local_manifest(self, args, config, initial_manifest, workspace_path, global_manifest_directory):
local_manifest_dir = os.path.join(workspace_path, 'repo')
current_combo = initial_manifest.general_config.current_combo
initial_sources = initial_manifest.get_repo_sources(current_combo)
@@ -210,8 +221,6 @@ class SyncCommand(EdkrepoCommand):
origin = repo.remotes.origin
origin.fetch()

- global_manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
-
#see if there is an entry in CiIndex.xml that matches the prject name of the current manifest
index_path = os.path.join(global_manifest_directory, 'CiIndex.xml')
ci_index_xml = CiIndexXml(index_path)
@@ -364,8 +373,7 @@ class SyncCommand(EdkrepoCommand):
break
return repos_to_checkout

- def __check_for_new_manifest(self, args, config, initial_manifest, workspace_path):
- global_manifest_directory = config['cfg_file'].manifest_repo_abs_local_path
+ def __check_for_new_manifest(self, args, config, initial_manifest, workspace_path, global_manifest_directory):
#see if there is an entry in CiIndex.xml that matches the prject name of the current manifest
index_path = os.path.join(global_manifest_directory, 'CiIndex.xml')
ci_index_xml = CiIndexXml(index_path)
diff --git a/edkrepo_manifest_parser/edk_manifest.py b/edkrepo_manifest_parser/edk_manifest.py
index 69583b1..1e2b111 100644
--- a/edkrepo_manifest_parser/edk_manifest.py
+++ b/edkrepo_manifest_parser/edk_manifest.py
@@ -425,6 +425,7 @@ class ManifestXml(BaseXmlHelper):
element = subroot.find('SourceManifestRepository')
if element is None:
element = ET.SubElement(subroot, 'SourceManifestRepository')
+ element.tail = '\n'
element.attrib['manifest_repo'] = manifest_repo
self._tree.write(filename)
self.__general_config.source_manifest_repo = manifest_repo
@@ -549,7 +550,16 @@ class ManifestXml(BaseXmlHelper):
if element1.text != element2.text:
return False
if element1.tail != element2.tail:
- return False
+ if element1.tail is not None:
+ tail1 = element1.tail.strip()
+ else:
+ tail1 = ''
+ if element2.tail is not None:
+ tail2 = element2.tail.strip()
+ else:
+ tail2 = ''
+ if tail1 != tail2:
+ return False
if element1.attrib != element2.attrib:
return False
if len(element1) != len(element2):
@@ -558,25 +568,37 @@ class ManifestXml(BaseXmlHelper):

def equals(self, other, ignore_current_combo=False):
status = self._compare_elements(self._tree.getroot(), other._tree.getroot())
- if not status and ignore_current_combo:
+ if not status:
tree1 = copy.deepcopy(self._tree.getroot())
tree2 = copy.deepcopy(other._tree.getroot())
subroot = tree1.find('GeneralConfig')
if subroot is None:
return False
- element = subroot.find('CurrentClonedCombo')
+ if ignore_current_combo:
+ element = subroot.find('CurrentClonedCombo')
+ if element is None:
+ element = ET.SubElement(subroot, 'CurrentClonedCombo')
+ element.tail = '\n'
+ element.attrib['combination'] = ''
+ element = subroot.find('SourceManifestRepository')
if element is None:
- element = ET.SubElement(subroot, 'CurrentClonedCombo')
- element.tail = '\n'
- element.attrib['combination'] = ''
+ element = ET.SubElement(subroot, 'SourceManifestRepository')
+ element.tail ='\n'
+ element.attrib['manifest_repo'] = ''
subroot = tree2.find('GeneralConfig')
if subroot is None:
return False
- element = subroot.find('CurrentClonedCombo')
+ if ignore_current_combo:
+ element = subroot.find('CurrentClonedCombo')
+ if element is None:
+ element = ET.SubElement(subroot, 'CurrentClonedCombo')
+ element.tail = '\n'
+ element.attrib['combination'] = ''
+ element = subroot.find('SourceManifestRepository')
if element is None:
- element = ET.SubElement(subroot, 'CurrentClonedCombo')
- element.tail = '\n'
- element.attrib['combination'] = ''
+ element = ET.SubElement(subroot, 'SourceManifestRepository')
+ element.tail ='\n'
+ element.attrib['manifest_repo'] = ''
status = self._compare_elements(tree1, tree2)
return status

--
2.16.2.windows.1


[edk2-staging/EdkRepo] [PATCH 0/4] EdkRepo: Finalize multiple manifest repository support

Ashley E Desimone
 

Finalize integration of multiple manifest repository
support. By adding support to remaining commands and
by removing support for the Manifest-Repo section of
the edkrepo.cfg file.

Signed-off-by: Ashley E Desimone <ashley.e.desimone@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Puja Pandya <puja.pandya@...>
Cc: Erik Bjorge <erik.c.bjorge@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Prince Agyeman <prince.agyeman@...>

Ashley E Desimone (4):
EdkRepo: Update sync to support multiple manifest repositories
EdkRepo: Remove unused functions from common_repo_functions.py
EdkRepo: Add support for multiple manifest repostories to command
completions
EdkRepo: Remove support for deprecated Manifest-Repo content in
edkrepo.cfg

edkrepo/command_completion_edkrepo.py | 48 ++++++++++++-------
edkrepo/commands/clone_command.py | 3 +-
edkrepo/commands/manifest_command.py | 2 +-
edkrepo/commands/sync_command.py | 42 +++++++++-------
edkrepo/common/common_repo_functions.py | 85 +++++++++++----------------------
edkrepo/common/humble.py | 4 +-
edkrepo/config/config_factory.py | 12 +----
edkrepo_installer/Vendor/edkrepo.cfg | 5 --
edkrepo_manifest_parser/edk_manifest.py | 42 ++++++++++++----
9 files changed, 124 insertions(+), 119 deletions(-)

--
2.16.2.windows.1


Re: [edk2-rfc] GitHub Pull Request based Code Review Process

Rebecca Cran
 

Mike,

On 5/10/20 3:29 PM, Michael D Kinney wrote:

There is no difference between CI checks run during code review
and the final CI checks before merge. I think it is an interesting
conversation to decide how many times those CI checks should be
run and if they should run automatically on every change during
review or on demand.
I'd suggest following what other Github projects do, which I think is to run the CI checks automatically on every change that's made in a pull request - I don't know if it might also be necessary to run them during the merge, if master has changed in the meantime. That gives the _submitter_ feedback about any changes they need to make, instead of having to wait until the maintainer tells them their change has broken something: it speeds up the development process.

Mergify is more flexible. We want to make sure the git history
is linear with not git merges and supports both single patches
and patch series without squashing. GitHub merge button by
default squashes all commits into a single commit.
Wouldn't disabling all but "Allow rebase merging" do the same thing without the additional potential failure point? Though it sounds like we've resolved the problems with Mergify, so it's not important.

https://help.github.com/en/github/administering-a-repository/configuring-commit-squashing-for-pull-requests


--
Rebecca Cran


Re: [edk2-platforms][PATCH 1/1] RPi: add Gpio output set/clear functions to GpioLib

Ard Biesheuvel
 

On 5/10/20 11:36 PM, Andrei Warkentin wrote:
GpioSet and GpioClear. Using hw of course (not mailbox).
Signed-off-by: Andrei Warkentin <andrey.warkentin@...>
Can you add this patch to the series that actually introduces a user for this new functionality?

Also, please don't hide unrelated changes in your patches like this.

---
Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h | 10 +++++
Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c | 42 +++++++++++++++++++-
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
index 014c6b07..10c9cdfb 100644
--- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
+++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
@@ -24,4 +24,14 @@ GpioPinFuncGet (
IN UINTN Pin
);
+VOID
+GpioSet (
+ IN UINTN Pin
+ );
+
+VOID
+GpioClear (
+ IN UINTN Pin
+ );
+
#endif /* __GPIO_LIB__ */
diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
index 542b6e8f..716b05be 100644
--- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
+++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
@@ -18,7 +18,7 @@
STATIC
VOID
-GpioFSELModify (
+GpioFSelModify (
IN UINTN RegIndex,
IN UINT32 ModifyMask,
IN UINT32 FunctionMask
@@ -38,6 +38,44 @@ GpioFSELModify (
MmioWrite32 (Reg, Val);
}
+VOID
+GpioSet (
+ IN UINTN Pin
+ )
+{
+ UINT32 Val;
+ EFI_PHYSICAL_ADDRESS Reg;
+ UINT8 RegIndex = Pin / 32;
+ UINT8 SelIndex = Pin % 32;
+
+ Reg = RegIndex * sizeof (UINT32) + GPIO_GPSET0;
+
+ ASSERT (Reg <= GPIO_GPSET1);
+
+ Val = MmioRead32 (Reg);
+ Val |= 1 << SelIndex;
+ MmioWrite32 (Reg, Val);
+}
+
+VOID
+GpioClear (
+ IN UINTN Pin
+ )
+{
+ UINT32 Val;
+ EFI_PHYSICAL_ADDRESS Reg;
+ UINT8 RegIndex = Pin / 32;
+ UINT8 SelIndex = Pin % 32;
+
+ Reg = RegIndex * sizeof (UINT32) + GPIO_GPCLR0;
+
+ ASSERT (Reg <= GPIO_GPCLR1);
+
+ Val = MmioRead32 (Reg);
+ Val |= 1 << SelIndex;
+ MmioWrite32 (Reg, Val);
+}
+
VOID
GpioPinFuncSet (
IN UINTN Pin,
@@ -57,7 +95,7 @@ GpioPinFuncSet (
ModifyMask = GPIO_FSEL_MASK << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
FunctionMask = Function << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
- GpioFSELModify (RegIndex, ModifyMask, FunctionMask);
+ GpioFSelModify (RegIndex, ModifyMask, FunctionMask);
}
UINTN


[edk2-platforms][PATCH 1/1] RPi: add Gpio output set/clear functions to GpioLib

Andrei Warkentin
 

GpioSet and GpioClear. Using hw of course (not mailbox).

Signed-off-by: Andrei Warkentin <andrey.warkentin@...>
---
Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h | 10 +++++
Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c | 42 +++++++++++++++++++-
2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
index 014c6b07..10c9cdfb 100644
--- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
+++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
@@ -24,4 +24,14 @@ GpioPinFuncGet (
IN UINTN Pin
);

+VOID
+GpioSet (
+ IN UINTN Pin
+ );
+
+VOID
+GpioClear (
+ IN UINTN Pin
+ );
+
#endif /* __GPIO_LIB__ */
diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
index 542b6e8f..716b05be 100644
--- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
+++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
@@ -18,7 +18,7 @@

STATIC
VOID
-GpioFSELModify (
+GpioFSelModify (
IN UINTN RegIndex,
IN UINT32 ModifyMask,
IN UINT32 FunctionMask
@@ -38,6 +38,44 @@ GpioFSELModify (
MmioWrite32 (Reg, Val);
}

+VOID
+GpioSet (
+ IN UINTN Pin
+ )
+{
+ UINT32 Val;
+ EFI_PHYSICAL_ADDRESS Reg;
+ UINT8 RegIndex = Pin / 32;
+ UINT8 SelIndex = Pin % 32;
+
+ Reg = RegIndex * sizeof (UINT32) + GPIO_GPSET0;
+
+ ASSERT (Reg <= GPIO_GPSET1);
+
+ Val = MmioRead32 (Reg);
+ Val |= 1 << SelIndex;
+ MmioWrite32 (Reg, Val);
+}
+
+VOID
+GpioClear (
+ IN UINTN Pin
+ )
+{
+ UINT32 Val;
+ EFI_PHYSICAL_ADDRESS Reg;
+ UINT8 RegIndex = Pin / 32;
+ UINT8 SelIndex = Pin % 32;
+
+ Reg = RegIndex * sizeof (UINT32) + GPIO_GPCLR0;
+
+ ASSERT (Reg <= GPIO_GPCLR1);
+
+ Val = MmioRead32 (Reg);
+ Val |= 1 << SelIndex;
+ MmioWrite32 (Reg, Val);
+}
+
VOID
GpioPinFuncSet (
IN UINTN Pin,
@@ -57,7 +95,7 @@ GpioPinFuncSet (

ModifyMask = GPIO_FSEL_MASK << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
FunctionMask = Function << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
- GpioFSELModify (RegIndex, ModifyMask, FunctionMask);
+ GpioFSelModify (RegIndex, ModifyMask, FunctionMask);
}

UINTN
--
2.17.1


Re: [PATCH edk2-platforms v2 0/4] BCM genet fixes

Ard Biesheuvel
 

On 5/10/20 12:42 PM, Ard Biesheuvel wrote:
This fixes the multicast/broadcast/promisc handling, and switches to
ordinary page allocations for RX buffers. Patch #1 is cosmetic only.
https://github.com/pftf/edk2-platforms/tree/rpi4_genet_v2_ardb
Cc: Pete Batard <pete@...>
Cc: Jared McNeill <jmcneill@...>
Cc: Andrei Warkentin <awarkentin@...>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
NOTE: build tested only.
Samer is reporting that this breaks HTTP boot. I will have a look into this tomorrow.


[edk2-platforms][PATCH 1/1] RPi3: default DisplayDxe to just native mode

Andrei Warkentin
 

The scaled resolutions are useful, but are not the default expected
by most users. Linux and BSDs don't set preferred resolution in
their OS loader, so when booting via setup UI, the OS is left
running at 800x600, not the native resolution. This looks crummy.

Signed-off-by: Andrei Warkentin <andrey.warkentin@...>
---
Platform/RaspberryPi/RPi3/RPi3.dsc | 6 +++++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index a138c874..8dd4cbff 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -471,7 +471,11 @@
#
# Display-related.
#
- gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0x3f
+
+ #
+ # Just enable native resolution by default.
+ #
+ gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0x20
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1

#
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 75867f03..9413fe66 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -486,7 +486,11 @@
#
# Display-related.
#
- gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0x3f
+
+ #
+ # Just enable native resolution by default.
+ #
+ gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0x20
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1

#
--
2.17.1


[edk2-platforms][PATCH 2/2] RPi: allow selecting DT-only mode

Andrei Warkentin
 

Today the Pies can be booted in a way where only ACPI is exposed,
or both ACPI and DT are exposed.

This adds one more mode - DT only, no ACPI. The target audience
is developers. When both are exposed, it's up to the OS to decide
which gets used, and that choice can differ between OSes,

Note: this does _not_ change defaults. Pi 3 still defaults to
ACPI + DT, while Pi 4 still defaults to ACPI only.

We don't really want to remove DT + ACPI mode - it is the default
on Pi 3, and removing it is bound to just annoy users - WoA and
NetBSD (voa UEFI) on Pi 3 only work with ACPI, while everything
else (Linux, FreeBSD) only work with DT. I'd make an analogy of
MPS and ACPI being exposed for the longest time ever together on
PCs.

Testing: OpenBSD on Pi 4 with DT-only and ACPI-only boots.

Signed-off-by: Andrei Warkentin <andrey.warkentin@...>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11 +++++++----
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 2 +-
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 9 +++++----
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 ++++++++-------
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 3 ++-
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf | 2 +-
Platform/RaspberryPi/Include/ConfigVars.h | 11 +++++------
Platform/RaspberryPi/RPi3/RPi3.dsc | 8 ++++++--
Platform/RaspberryPi/RPi4/RPi4.dsc | 8 ++++++--
Platform/RaspberryPi/RaspberryPi.dec | 2 +-
10 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 8c9609f3..29701db2 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -155,11 +155,11 @@ SetupVariables (
}

Size = sizeof (UINT32);
- Status = gRT->GetVariable (L"OptDeviceTree",
+ Status = gRT->GetVariable (L"SystemTableMode",
&gConfigDxeFormSetGuid,
NULL, &Size, &Var32);
if (EFI_ERROR (Status)) {
- PcdSet32 (PcdOptDeviceTree, PcdGet32 (PcdOptDeviceTree));
+ PcdSet32 (PcdSystemTableMode, PcdGet32 (PcdSystemTableMode));
}

Size = sizeof (UINT32);
@@ -488,8 +488,11 @@ ConfigInitialize (
DEBUG ((DEBUG_ERROR, "Couldn't install ConfigDxe configuration pages: %r\n", Status));
}

- Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
- ASSERT_EFI_ERROR (Status);
+ if (PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_ACPI ||
+ PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {
+ Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
+ ASSERT_EFI_ERROR (Status);
+ }


return EFI_SUCCESS;
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 57963baf..e47f3af6 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -77,7 +77,7 @@
gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
+ gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 07660072..db36e200 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -41,10 +41,11 @@
#string STR_ADVANCED_3GB_OFF #language en-US "Disabled"
#string STR_ADVANCED_3GB_ON #language en-US "Enabled"

-#string STR_ADVANCED_DT_PROMPT #language en-US "Device Tree"
-#string STR_ADVANCED_DT_HELP #language en-US "Disable this option to force OSes such as GNU/Linux to use ACPI"
-#string STR_ADVANCED_DT_OFF #language en-US "Disabled (Force ACPI)"
-#string STR_ADVANCED_DT_ON #language en-US "Enabled"
+#string STR_ADVANCED_SYSTAB_PROMPT #language en-US "System Table Selection"
+#string STR_ADVANCED_SYSTAB_HELP #language en-US "ACPI/DT choice for specific OSes"
+#string STR_ADVANCED_SYSTAB_ACPI #language en-US "ACPI"
+#string STR_ADVANCED_SYSTAB_BOTH #language en-US "ACPI + Devicetree"
+#string STR_ADVANCED_SYSTAB_DT #language en-US "Devicetree"

/*
* MMC/SD configuration.
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 576eabe9..91a0ea2d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -44,9 +44,9 @@ formset
name = RamLimitTo3GB,
guid = CONFIGDXE_FORM_SET_GUID;

- efivarstore ADVANCED_DEVICE_TREE_VARSTORE_DATA,
+ efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
- name = OptDeviceTree,
+ name = SystemTableMode,
guid = CONFIGDXE_FORM_SET_GUID;

efivarstore MMC_SD_VARSTORE_DATA,
@@ -164,12 +164,13 @@ formset
endoneof;
endif;

- oneof varid = OptDeviceTree.Enabled,
- prompt = STRING_TOKEN(STR_ADVANCED_DT_PROMPT),
- help = STRING_TOKEN(STR_ADVANCED_DT_HELP),
+ oneof varid = SystemTableMode.Mode,
+ prompt = STRING_TOKEN(STR_ADVANCED_SYSTAB_PROMPT),
+ help = STRING_TOKEN(STR_ADVANCED_SYSTAB_HELP),
flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
- option text = STRING_TOKEN(STR_ADVANCED_DT_OFF), value = 0, flags = 0;
- option text = STRING_TOKEN(STR_ADVANCED_DT_ON), value = 1, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_ACPI), value = SYSTEM_TABLE_MODE_ACPI, flags = 0;
+ option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_BOTH), value = SYSTEM_TABLE_MODE_BOTH, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
endoneof;
endform;

diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index 3aaa0a7f..44c6c87c 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -355,7 +355,8 @@ FdtDxeInitialize (
UINTN FdtSize;
VOID *FdtImage = NULL;

- if (PcdGet32 (PcdOptDeviceTree) == 0) {
+ if (PcdGet32 (PcdSystemTableMode) != SYSTEM_TABLE_MODE_BOTH &&
+ PcdGet32 (PcdSystemTableMode) != SYSTEM_TABLE_MODE_DT) {
DEBUG ((DEBUG_INFO, "Device Tree disabled per user configuration\n"));
return EFI_SUCCESS;
}
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index 49ba5ba1..26f84e59 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -46,4 +46,4 @@
gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress

[Pcd]
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree
+ gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index a0959b4b..28837f98 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -76,12 +76,11 @@ typedef struct {
} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;

typedef struct {
- /*
- * 0 - Do not provide a Device Tree to the OS
- * 1 - Provide a Device Tree to the OS
- */
- UINT32 Enabled;
-} ADVANCED_DEVICE_TREE_VARSTORE_DATA;
+#define SYSTEM_TABLE_MODE_ACPI 0
+#define SYSTEM_TABLE_MODE_BOTH 1
+#define SYSTEM_TABLE_MODE_DT 2
+ UINT32 Mode;
+} SYSTEM_TABLE_MODE_VARSTORE_DATA;

typedef struct {
/*
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 563fb891..a138c874 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -481,9 +481,13 @@
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0

#
- # Device Tree
+ # Device Tree and ACPI selection.
#
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|1
+ # 0 - SYSTEM_TABLE_MODE_ACPI
+ # 1 - SYSTEM_TABLE_MODE_BOTH (default)
+ # 2 - SYSTEM_TABLE_MODE_DT
+ #
+ gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1

#
# Common UEFI ones.
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 4deccd9d..75867f03 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -496,9 +496,13 @@
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1

#
- # Device Tree
+ # Device Tree and ACPI selection.
#
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|L"OptDeviceTree"|gConfigDxeFormSetGuid|0x0|0
+ # 0 - SYSTEM_TABLE_MODE_ACPI (default)
+ # 1 - SYSTEM_TABLE_MODE_BOTH
+ # 2 - SYSTEM_TABLE_MODE_DT
+ #
+ gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0

#
# Common UEFI ones.
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index 66ef6186..1a3c44e0 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -65,6 +65,6 @@
gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0x3F|UINT8|0x00000017
gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018
- gRaspberryPiTokenSpaceGuid.PcdOptDeviceTree|1|UINT32|0x0000001B
+ gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B
gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
--
2.17.1


[edk2-platforms][PATCH 1/2] RPi: move varstore structure defs to ConfigVars.h

Andrei Warkentin
 

To avoid hardcoding constants for non-obvious fields (e.g. enum
instead of basic enable/disable), move variable structure and value
definitions into a separate header, ConfigVars.h.

Signed-off-by: Andrei Warkentin <andrey.warkentin@...>
---
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 +-
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 131 +------------------
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 3 +-
Platform/RaspberryPi/Include/ConfigVars.h | 132 ++++++++++++++++++++
4 files changed, 144 insertions(+), 132 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index c90c2530..8c9609f3 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -22,6 +22,7 @@
#include <IndustryStandard/Bcm2836Gpio.h>
#include <Library/GpioLib.h>
#include <Protocol/RpiFirmware.h>
+#include <ConfigVars.h>
#include "ConfigDxeFormSetGuid.h"

#define FREQ_1_MHZ 1000000
@@ -259,10 +260,10 @@ ApplyVariables (
UINT64 SystemMemorySize;

switch (CpuClock) {
- case 0: // Low
+ case CHIPSET_CPU_CLOCK_LOW:
Rate = FixedPcdGet32 (PcdCpuLowSpeedMHz) * FREQ_1_MHZ;
break;
- case 1: // Default
+ case CHIPSET_CPU_CLOCK_DEFAULT:
/*
* What the Raspberry Pi Foundation calls "max clock rate" is really the default value
* from: https://www.raspberrypi.org/documentation/configuration/config-txt/overclocking.md
@@ -272,10 +273,10 @@ ApplyVariables (
DEBUG ((DEBUG_ERROR, "Couldn't read default CPU speed %r\n", Status));
}
break;
- case 2: // Max
+ case CHIPSET_CPU_CLOCK_MAX:
Rate = FixedPcdGet32 (PcdCpuMaxSpeedMHz) * FREQ_1_MHZ;
break;
- case 3: // Custom
+ case CHIPSET_CPU_CLOCK_CUSTOM:
Rate = CustomCpuClock * FREQ_1_MHZ;
break;
}
@@ -490,5 +491,6 @@ ConfigInitialize (
Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);
ASSERT_EFI_ERROR (Status);

+
return EFI_SUCCESS;
}
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 0a650a94..576eabe9 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -8,128 +8,7 @@

#include <Guid/HiiPlatformSetupFormset.h>
#include "ConfigDxeFormSetGuid.h"
-
-#pragma pack(1)
-typedef struct {
- /*
- * One bit for each scaled resolution supported,
- * these are ordered exactly like mGopModeData
- * in DisplayDxe.
- *
- * 800x600, 640x480, 1024x768, 720p, 1080p, native.
- */
- UINT8 v640 : 1;
- UINT8 v800 : 1;
- UINT8 v1024 : 1;
- UINT8 v720p : 1;
- UINT8 v1080p : 1;
- UINT8 Physical : 1;
-} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
-#pragma pack()
-
-typedef struct {
- /*
- * 0 - No screenshot support.
- * 1 - Screenshot support via hotkey.
- */
- UINT32 Enable;
-} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - No JTAG.
- * 1 - JTAG mode.
- */
- UINT32 Enable;
-} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't show UEFI exit message.
- * 1 - Show UEFI exit message.
- */
- UINT32 Show;
-} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - low.
- * 1 - default.
- * 2 - maximum.
- * 3 - custom.
- */
- UINT32 Clock;
-} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
-
-typedef struct {
- UINT32 Clock;
-} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
-
-typedef struct {
- /*
- * Always set by ConfigDxe prior to HII init to reflect
- * platform capability.
- */
- UINT32 Supported;
-} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
-
-typedef struct {
- UINT32 Enabled;
-} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Do not provide a Device Tree to the OS
- * 1 - Provide a Device Tree to the OS
- */
- UINT32 Enabled;
-} ADVANCED_DEVICE_TREE_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
- * 1 - uSD slot routed to Arasan SDHCI.
- */
- UINT32 Routing;
-} MMC_SD_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't disable multi-block.
- * 1 - Disable multi-block commands.
- */
- UINT32 DisableMulti;
-} MMC_DISMULTI_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't force 1 bit mode.
- * 1 - Force 1 bit mode.
- */
- UINT32 Force1Bit;
-} MMC_FORCE1BIT_VARSTORE_DATA;
-
-typedef struct {
- /*
- * 0 - Don't force default speed.
- * 1 - Force default speed.
- */
- UINT32 ForceDS;
-} MMC_FORCEDS_VARSTORE_DATA;
-
-typedef struct {
- /*
- * Default Speed MHz override (25MHz default).
- */
- UINT32 MHz;
-} MMC_SD_DS_MHZ_VARSTORE_DATA;
-
-typedef struct {
- /*
- * High Speed MHz override (50MHz default).
- */
- UINT32 MHz;
-} MMC_SD_HS_MHZ_VARSTORE_DATA;
+#include <ConfigVars.h>

//
// EFI Variable attributes
@@ -253,10 +132,10 @@ formset
prompt = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_PROMPT),
help = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_HELP),
flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = 0, flags = 0;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = 1, flags = DEFAULT;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = 2, flags = 0;
- option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = 3, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_LOW), value = CHIPSET_CPU_CLOCK_LOW, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_DEF), value = CHIPSET_CPU_CLOCK_DEFAULT, flags = DEFAULT;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_MAX), value = CHIPSET_CPU_CLOCK_MAX, flags = 0;
+ option text = STRING_TOKEN(STR_CHIPSET_CLOCK_CPU_CUSTOM), value = CHIPSET_CPU_CLOCK_CUSTOM, flags = 0;
endoneof;

grayoutif NOT ideqval CpuClock.Clock == 3;
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cb11256e..3aaa0a7f 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -15,10 +15,9 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <libfdt.h>
-
#include <Protocol/RpiFirmware.h>
-
#include <Guid/Fdt.h>
+#include <ConfigVars.h>

STATIC VOID *mFdtImage;

diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
new file mode 100644
index 00000000..a0959b4b
--- /dev/null
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -0,0 +1,132 @@
+/** @file
+ *
+ * Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@...>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#ifndef CONFIG_VARS_H
+#define CONFIG_VARS_H
+
+#pragma pack(1)
+typedef struct {
+ /*
+ * One bit for each scaled resolution supported,
+ * these are ordered exactly like mGopModeData
+ * in DisplayDxe.
+ *
+ * 800x600, 640x480, 1024x768, 720p, 1080p, native.
+ */
+ UINT8 v640 : 1;
+ UINT8 v800 : 1;
+ UINT8 v1024 : 1;
+ UINT8 v720p : 1;
+ UINT8 v1080p : 1;
+ UINT8 Physical : 1;
+} DISPLAY_ENABLE_SCALED_VMODES_VARSTORE_DATA;
+#pragma pack()
+
+typedef struct {
+ /*
+ * 0 - No screenshot support.
+ * 1 - Screenshot support via hotkey.
+ */
+ UINT32 Enable;
+} DISPLAY_ENABLE_SSHOT_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - No JTAG.
+ * 1 - JTAG mode.
+ */
+ UINT32 Enable;
+} DEBUG_ENABLE_JTAG_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't show UEFI exit message.
+ * 1 - Show UEFI exit message.
+ */
+ UINT32 Show;
+} DEBUG_SHOW_UEFI_EXIT_VARSTORE_DATA;
+
+typedef struct {
+#define CHIPSET_CPU_CLOCK_LOW 0
+#define CHIPSET_CPU_CLOCK_DEFAULT 1
+#define CHIPSET_CPU_CLOCK_MAX 2
+#define CHIPSET_CPU_CLOCK_CUSTOM 3
+ UINT32 Clock;
+} CHIPSET_CPU_CLOCK_VARSTORE_DATA;
+
+typedef struct {
+ UINT32 Clock;
+} CHIPSET_CUSTOM_CPU_CLOCK_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * Always set by ConfigDxe prior to HII init to reflect
+ * platform capability.
+ */
+ UINT32 Supported;
+} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA;
+
+typedef struct {
+ UINT32 Enabled;
+} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Do not provide a Device Tree to the OS
+ * 1 - Provide a Device Tree to the OS
+ */
+ UINT32 Enabled;
+} ADVANCED_DEVICE_TREE_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
+ * 1 - uSD slot routed to Arasan SDHCI.
+ */
+ UINT32 Routing;
+} MMC_SD_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't disable multi-block.
+ * 1 - Disable multi-block commands.
+ */
+ UINT32 DisableMulti;
+} MMC_DISMULTI_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't force 1 bit mode.
+ * 1 - Force 1 bit mode.
+ */
+ UINT32 Force1Bit;
+} MMC_FORCE1BIT_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * 0 - Don't force default speed.
+ * 1 - Force default speed.
+ */
+ UINT32 ForceDS;
+} MMC_FORCEDS_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * Default Speed MHz override (25MHz default).
+ */
+ UINT32 MHz;
+} MMC_SD_DS_MHZ_VARSTORE_DATA;
+
+typedef struct {
+ /*
+ * High Speed MHz override (50MHz default).
+ */
+ UINT32 MHz;
+} MMC_SD_HS_MHZ_VARSTORE_DATA;
+
+#endif /* CONFIG_VARS_H */
--
2.17.1