Date   

Re: [PATCH 3/6] NetworkPkg/IScsiDxe: distinguish "maximum" and "selected" CHAP digest sizes

Maciej Rabeda
 

To cut to the chase on this patch:
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 09-Jun-21 15:46, Laszlo Ersek wrote:
On 06/09/21 12:43, Philippe Mathieu-Daudé wrote:
Hi Laszlo,

On 6/8/21 3:06 PM, Laszlo Ersek wrote:
IScsiDxe uses the ISCSI_CHAP_RSP_LEN macro for expressing the size of the
digest (16) that it solely supports at this point (MD5).
ISCSI_CHAP_RSP_LEN is used for both (a) *allocating* digest-related
buffers (binary buffers and hex encodings alike), and (b) *processing*
binary digest buffers (comparing them, filling them, reading them).

In preparation for adding other hash algorithms, split purpose (a) from
purpose (b). For purpose (a) -- buffer allocation --, introduce
ISCSI_CHAP_MAX_DIGEST_SIZE. For purpose (b) -- processing --, rely on
MD5_DIGEST_SIZE from <BaseCryptLib.h>.
Matter of taste probably, I'd rather see this patch split in 2, as you
identified. (b) first then (a). Regardless:
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Interesting; I thought that showing all uses of ISCSI_CHAP_RSP_LEN in
one patch, and classifying each use one way or another at once, was the
best for reviewer understanding. Basically it's a single "mental loop"
over all uses, and in the "loop body", we have an "if" (classification)
-- allocation vs. processing.

What you propose is basically "two loops". In that approach, in the
first patch (= the first "mental loop"), only "processing" uses would be
updated; the "allocation sites" wouldn't be shown at all. I feel that
this approach is counter-intuitive:

From the body of the first patch,

- the reviewer can check the *correctness* of the patch (that is,
whether everything that I converted is indeed "processing"),

- but they can't check the *completeness* of the patch (that is, whether
there is a "processing" site that I should have converted, but missed).

For the reviewer to verify the completeness of the first patch, they
have to apply it (or check out the branch at that stage), and go over
all the *remaining* ISCSI_CHAP_RSP_LEN instances, to see if I missed
something. And, if the reviewer has to check every single instance of
ISCSI_CHAP_RSP_LEN right after the first patch, they end up doing the
same work as if they had just reviewed this particular patch.

I think your approach boils down to the following idea:

The completeness of the first patch would be proved by the correctness
of the second patch.

That is, *after* you reviewed the second patch (and see that every site
converted is indeed an allocation site, and that the ISCSI_CHAP_RSP_LEN
macro definition is being removed, so no other ISCSI_CHAP_RSP_LEN
instance remains), you could be sure that no processing site was missed
in the first patch.

Technically / mathematically, this is entirely true; I just prefer
avoiding situations where you have to review patch (N+X) to see the
validity (completeness) of patch (N). I quite dislike jumping between
patches during review.

Does my explanation make sense?

If you still prefer the split, I'm OK to do it.

Thanks!
Laszlo

Distinguishing these purposes is justified because purpose (b) --
processing -- must depend on the hashing algorithm negotiated between
initiator and target, while for purpose (a) -- allocation --, using the
maximum supported digest size is suitable. For now, because only MD5 is
supported, introduce ISCSI_CHAP_MAX_DIGEST_SIZE *as* MD5_DIGEST_SIZE.

Note that the argument for using the digest size as the size of the
outgoing challenge (in case mutual authentication is desired by the
initiator) remains in place. Because of this, the above two purposes are
distinguished for the "ISCSI_CHAP_AUTH_DATA.OutChallenge" field as well.

This patch is functionally a no-op, just yet.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiCHAP.h | 17 +++++++++------
NetworkPkg/IScsiDxe/IScsiCHAP.c | 22 ++++++++++----------
2 files changed, 22 insertions(+), 17 deletions(-)


Re: [PUBLIC edk2 PATCH v2 01/10] NetworkPkg/IScsiDxe: wrap IScsiCHAP source files to 80 characters

Ni, Ray
 

No objection on the patch. Just curious what editor you are using😊

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, June 8, 2021 8:13 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Maciej Rabeda <maciej.rabeda@linux.intel.com>; Philippe Mathieu-Daudé
<philmd@redhat.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [edk2-devel] [PUBLIC edk2 PATCH v2 01/10] NetworkPkg/IScsiDxe: wrap IScsiCHAP source files to 80 characters

Working with overlong lines is difficult for me; rewrap the CHAP-related
source files in IScsiDxe to 80 characters width. No functional changes.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiCHAP.h | 3 +-
NetworkPkg/IScsiDxe/IScsiCHAP.c | 90 +++++++++++++++-----
2 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h
index 140bba0dcd76..5e59fb678bd7 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
@@ -72,31 +72,32 @@ typedef struct _ISCSI_CHAP_AUTH_DATA {

@retval EFI_SUCCESS The Login Response passed the CHAP validation.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.
@retval Others Other errors as indicated.

**/
EFI_STATUS
IScsiCHAPOnRspReceived (
IN ISCSI_CONNECTION *Conn
);
/**
This function fills the CHAP authentication information into the login PDU
during the security negotiation stage in the iSCSI connection login.

@param[in] Conn The iSCSI connection.
@param[in, out] Pdu The PDU to send out.

@retval EFI_SUCCESS All check passed and the phase-related CHAP
- authentication info is filled into the iSCSI PDU.
+ authentication info is filled into the iSCSI
+ PDU.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.

**/
EFI_STATUS
IScsiCHAPToSendReq (
IN ISCSI_CONNECTION *Conn,
IN OUT NET_BUF *Pdu
);

#endif
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c
index 355c6f129f68..cbbc56ae5b43 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
@@ -1,42 +1,45 @@
/** @file
- This file is for Challenge-Handshake Authentication Protocol (CHAP) Configuration.
+ This file is for Challenge-Handshake Authentication Protocol (CHAP)
+ Configuration.

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

**/

#include "IScsiImpl.h"

/**
Initiator calculates its own expected hash value.

@param[in] ChapIdentifier iSCSI CHAP identifier sent by authenticator.
@param[in] ChapSecret iSCSI CHAP secret of the authenticator.
@param[in] SecretLength The length of iSCSI CHAP secret.
@param[in] ChapChallenge The challenge message sent by authenticator.
@param[in] ChallengeLength The length of iSCSI CHAP challenge message.
@param[out] ChapResponse The calculation of the expected hash value.

- @retval EFI_SUCCESS The expected hash value was calculatedly successfully.
- @retval EFI_PROTOCOL_ERROR The length of the secret should be at least the
- length of the hash value for the hashing algorithm chosen.
+ @retval EFI_SUCCESS The expected hash value was calculatedly
+ successfully.
+ @retval EFI_PROTOCOL_ERROR The length of the secret should be at least
+ the length of the hash value for the hashing
+ algorithm chosen.
@retval EFI_PROTOCOL_ERROR MD5 hash operation fail.
@retval EFI_OUT_OF_RESOURCES Fail to allocate resource to complete MD5.

**/
EFI_STATUS
IScsiCHAPCalculateResponse (
IN UINT32 ChapIdentifier,
IN CHAR8 *ChapSecret,
IN UINT32 SecretLength,
IN UINT8 *ChapChallenge,
IN UINT32 ChallengeLength,
OUT UINT8 *ChapResponse
)
{
UINTN Md5ContextSize;
VOID *Md5Ctx;
CHAR8 IdByte[1];
EFI_STATUS Status;

@@ -78,40 +81,42 @@ IScsiCHAPCalculateResponse (
goto Exit;
}

if (Md5Final (Md5Ctx, ChapResponse)) {
Status = EFI_SUCCESS;
}

Exit:
FreePool (Md5Ctx);
return Status;
}

/**
The initiator checks the CHAP response replied by target against its own
calculation of the expected hash value.

@param[in] AuthData iSCSI CHAP authentication data.
@param[in] TargetResponse The response from target.

- @retval EFI_SUCCESS The response from target passed authentication.
- @retval EFI_SECURITY_VIOLATION The response from target was not expected value.
+ @retval EFI_SUCCESS The response from target passed
+ authentication.
+ @retval EFI_SECURITY_VIOLATION The response from target was not expected
+ value.
@retval Others Other errors as indicated.

**/
EFI_STATUS
IScsiCHAPAuthTarget (
IN ISCSI_CHAP_AUTH_DATA *AuthData,
IN UINT8 *TargetResponse
)
{
EFI_STATUS Status;
UINT32 SecretSize;
UINT8 VerifyRsp[ISCSI_CHAP_RSP_LEN];

Status = EFI_SUCCESS;

SecretSize = (UINT32) AsciiStrLen (AuthData->AuthConfig->ReverseCHAPSecret);
Status = IScsiCHAPCalculateResponse (
AuthData->OutIdentifier,
AuthData->AuthConfig->ReverseCHAPSecret,
@@ -177,187 +182,211 @@ IScsiCHAPOnRspReceived (
//
NetbufQueCopy (&Conn->RspQue, 0, Len, Data);

//
// Build the key-value list from the data segment of the Login Response.
//
KeyValueList = IScsiBuildKeyValueList ((CHAR8 *) Data, Len);
if (KeyValueList == NULL) {
Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}

Status = EFI_PROTOCOL_ERROR;

switch (Conn->AuthStep) {
case ISCSI_AUTH_INITIAL:
//
// The first Login Response.
//
- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_TARGET_PORTAL_GROUP_TAG);
+ Value = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_TARGET_PORTAL_GROUP_TAG
+ );
if (Value == NULL) {
goto ON_EXIT;
}

Result = IScsiNetNtoi (Value);
if (Result > 0xFFFF) {
goto ON_EXIT;
}

Session->TargetPortalGroupTag = (UINT16) Result;

- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_AUTH_METHOD);
+ Value = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_AUTH_METHOD
+ );
if (Value == NULL) {
goto ON_EXIT;
}
//
- // Initiator mandates CHAP authentication but target replies without "CHAP", or
- // initiator suggets "None" but target replies with some kind of auth method.
+ // Initiator mandates CHAP authentication but target replies without
+ // "CHAP", or initiator suggets "None" but target replies with some kind of
+ // auth method.
//
if (Session->AuthType == ISCSI_AUTH_TYPE_NONE) {
if (AsciiStrCmp (Value, ISCSI_KEY_VALUE_NONE) != 0) {
goto ON_EXIT;
}
} else if (Session->AuthType == ISCSI_AUTH_TYPE_CHAP) {
if (AsciiStrCmp (Value, ISCSI_AUTH_METHOD_CHAP) != 0) {
goto ON_EXIT;
}
} else {
goto ON_EXIT;
}

//
// Transit to CHAP step one.
//
Conn->AuthStep = ISCSI_CHAP_STEP_ONE;
Status = EFI_SUCCESS;
break;

case ISCSI_CHAP_STEP_TWO:
//
// The Target replies with CHAP_A=<A> CHAP_I=<I> CHAP_C=<C>
//
- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_ALGORITHM);
+ Value = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_CHAP_ALGORITHM
+ );
if (Value == NULL) {
goto ON_EXIT;
}

Algorithm = IScsiNetNtoi (Value);
if (Algorithm != ISCSI_CHAP_ALGORITHM_MD5) {
//
// Unsupported algorithm is chosen by target.
//
goto ON_EXIT;
}

- Identifier = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_IDENTIFIER);
+ Identifier = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_CHAP_IDENTIFIER
+ );
if (Identifier == NULL) {
goto ON_EXIT;
}

- Challenge = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_CHALLENGE);
+ Challenge = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_CHAP_CHALLENGE
+ );
if (Challenge == NULL) {
goto ON_EXIT;
}
//
// Process the CHAP identifier and CHAP Challenge from Target.
// Calculate Response value.
//
Result = IScsiNetNtoi (Identifier);
if (Result > 0xFF) {
goto ON_EXIT;
}

AuthData->InIdentifier = (UINT32) Result;
AuthData->InChallengeLength = ISCSI_CHAP_AUTH_MAX_LEN;
- IScsiHexToBin ((UINT8 *) AuthData->InChallenge, &AuthData->InChallengeLength, Challenge);
+ IScsiHexToBin (
+ (UINT8 *) AuthData->InChallenge,
+ &AuthData->InChallengeLength,
+ Challenge
+ );
Status = IScsiCHAPCalculateResponse (
AuthData->InIdentifier,
AuthData->AuthConfig->CHAPSecret,
(UINT32) AsciiStrLen (AuthData->AuthConfig->CHAPSecret),
AuthData->InChallenge,
AuthData->InChallengeLength,
AuthData->CHAPResponse
);

//
// Transit to next step.
//
Conn->AuthStep = ISCSI_CHAP_STEP_THREE;
break;

case ISCSI_CHAP_STEP_THREE:
//
// One way CHAP authentication and the target would like to
// authenticate us.
//
Status = EFI_SUCCESS;
break;

case ISCSI_CHAP_STEP_FOUR:
ASSERT (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL);
//
// The forth step, CHAP_N=<N> CHAP_R=<R> is received from Target.
//
Name = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_NAME);
if (Name == NULL) {
goto ON_EXIT;
}

- Response = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_RESPONSE);
+ Response = IScsiGetValueByKeyFromList (
+ KeyValueList,
+ ISCSI_KEY_CHAP_RESPONSE
+ );
if (Response == NULL) {
goto ON_EXIT;
}

RspLen = ISCSI_CHAP_RSP_LEN;
IScsiHexToBin (TargetRsp, &RspLen, Response);

//
// Check the CHAP Name and Response replied by Target.
//
Status = IScsiCHAPAuthTarget (AuthData, TargetRsp);
break;

default:
break;
}

ON_EXIT:

if (KeyValueList != NULL) {
IScsiFreeKeyValueList (KeyValueList);
}

FreePool (Data);

return Status;
}


/**
This function fills the CHAP authentication information into the login PDU
during the security negotiation stage in the iSCSI connection login.

@param[in] Conn The iSCSI connection.
@param[in, out] Pdu The PDU to send out.

@retval EFI_SUCCESS All check passed and the phase-related CHAP
- authentication info is filled into the iSCSI PDU.
+ authentication info is filled into the iSCSI
+ PDU.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred.

**/
EFI_STATUS
IScsiCHAPToSendReq (
IN ISCSI_CONNECTION *Conn,
IN OUT NET_BUF *Pdu
)
{
EFI_STATUS Status;
ISCSI_SESSION *Session;
ISCSI_LOGIN_REQUEST *LoginReq;
ISCSI_CHAP_AUTH_DATA *AuthData;
CHAR8 *Value;
CHAR8 ValueStr[256];
CHAR8 *Response;
UINT32 RspLen;
CHAR8 *Challenge;
@@ -376,95 +405,114 @@ IScsiCHAPToSendReq (
RspLen = 2 * ISCSI_CHAP_RSP_LEN + 3;
Response = AllocateZeroPool (RspLen);
if (Response == NULL) {
return EFI_OUT_OF_RESOURCES;
}

ChallengeLen = 2 * ISCSI_CHAP_RSP_LEN + 3;
Challenge = AllocateZeroPool (ChallengeLen);
if (Challenge == NULL) {
FreePool (Response);
return EFI_OUT_OF_RESOURCES;
}

switch (Conn->AuthStep) {
case ISCSI_AUTH_INITIAL:
//
// It's the initial Login Request. Fill in the key=value pairs mandatory
// for the initial Login Request.
//
- IScsiAddKeyValuePair (Pdu, ISCSI_KEY_INITIATOR_NAME, mPrivate->InitiatorName);
+ IScsiAddKeyValuePair (
+ Pdu,
+ ISCSI_KEY_INITIATOR_NAME,
+ mPrivate->InitiatorName
+ );
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_SESSION_TYPE, "Normal");
IScsiAddKeyValuePair (
Pdu,
ISCSI_KEY_TARGET_NAME,
Session->ConfigData->SessionConfigData.TargetName
);

if (Session->AuthType == ISCSI_AUTH_TYPE_NONE) {
Value = ISCSI_KEY_VALUE_NONE;
ISCSI_SET_FLAG (LoginReq, ISCSI_LOGIN_REQ_PDU_FLAG_TRANSIT);
} else {
Value = ISCSI_AUTH_METHOD_CHAP;
}

IScsiAddKeyValuePair (Pdu, ISCSI_KEY_AUTH_METHOD, Value);

break;

case ISCSI_CHAP_STEP_ONE:
//
- // First step, send the Login Request with CHAP_A=<A1,A2...> key-value pair.
+ // First step, send the Login Request with CHAP_A=<A1,A2...> key-value
+ // pair.
//
AsciiSPrint (ValueStr, sizeof (ValueStr), "%d", ISCSI_CHAP_ALGORITHM_MD5);
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_ALGORITHM, ValueStr);

Conn->AuthStep = ISCSI_CHAP_STEP_TWO;
break;

case ISCSI_CHAP_STEP_THREE:
//
// Third step, send the Login Request with CHAP_N=<N> CHAP_R=<R> or
// CHAP_N=<N> CHAP_R=<R> CHAP_I=<I> CHAP_C=<C> if target authentication is
// required too.
//
// CHAP_N=<N>
//
- IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_NAME, (CHAR8 *) &AuthData->AuthConfig->CHAPName);
+ IScsiAddKeyValuePair (
+ Pdu,
+ ISCSI_KEY_CHAP_NAME,
+ (CHAR8 *) &AuthData->AuthConfig->CHAPName
+ );
//
// CHAP_R=<R>
//
- IScsiBinToHex ((UINT8 *) AuthData->CHAPResponse, ISCSI_CHAP_RSP_LEN, Response, &RspLen);
+ IScsiBinToHex (
+ (UINT8 *) AuthData->CHAPResponse,
+ ISCSI_CHAP_RSP_LEN,
+ Response,
+ &RspLen
+ );
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_RESPONSE, Response);

if (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL) {
//
// CHAP_I=<I>
//
IScsiGenRandom ((UINT8 *) &AuthData->OutIdentifier, 1);
AsciiSPrint (ValueStr, sizeof (ValueStr), "%d", AuthData->OutIdentifier);
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_IDENTIFIER, ValueStr);
//
// CHAP_C=<C>
//
IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN);
AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN;
- IScsiBinToHex ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN, Challenge, &ChallengeLen);
+ IScsiBinToHex (
+ (UINT8 *) AuthData->OutChallenge,
+ ISCSI_CHAP_RSP_LEN,
+ Challenge,
+ &ChallengeLen
+ );
IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_CHALLENGE, Challenge);

Conn->AuthStep = ISCSI_CHAP_STEP_FOUR;
}
//
// Set the stage transition flag.
//
ISCSI_SET_FLAG (LoginReq, ISCSI_LOGIN_REQ_PDU_FLAG_TRANSIT);
break;

default:
Status = EFI_PROTOCOL_ERROR;
break;
}

FreePool (Response);
FreePool (Challenge);

return Status;
--
2.19.1.3.g30247aa5d201






Re: [PATCH 2/6] NetworkPkg/IScsiDxe: add horizontal whitespace to IScsiCHAP files

Maciej Rabeda
 

Nicely aligned spacing...

Feels Good Man by Daniel 168

Reviewed-by: Maciej Rabeda <maciej.rabeda@...>

On 08-Jun-21 15:06, Laszlo Ersek wrote:
In the next patches, we'll need more room for various macro and parameter
names. For maintaining the current visual alignments, insert some
horizontal whitespace in preparation. "git show -b" produces no output for
this patch; the patch introduces no functional changes.

Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Maciej Rabeda <maciej.rabeda@...>
Cc: Philippe Mathieu-Daudé <philmd@...>
Cc: Siyuan Fu <siyuan.fu@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@...>
---
 NetworkPkg/IScsiDxe/IScsiCHAP.h | 24 ++++++++++----------
 NetworkPkg/IScsiDxe/IScsiCHAP.c | 12 +++++-----
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h
index 35d5d6ec29e3..d6a90fc27fc3 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h
@@ -1,49 +1,49 @@
 /** @file
   The header file of CHAP configuration.
 
 Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #ifndef _ISCSI_CHAP_H_
 #define _ISCSI_CHAP_H_
 
-#define ISCSI_AUTH_METHOD_CHAP    "CHAP"
+#define ISCSI_AUTH_METHOD_CHAP                    "CHAP"
 
-#define ISCSI_KEY_CHAP_ALGORITHM  "CHAP_A"
-#define ISCSI_KEY_CHAP_IDENTIFIER "CHAP_I"
-#define ISCSI_KEY_CHAP_CHALLENGE  "CHAP_C"
-#define ISCSI_KEY_CHAP_NAME       "CHAP_N"
-#define ISCSI_KEY_CHAP_RESPONSE   "CHAP_R"
+#define ISCSI_KEY_CHAP_ALGORITHM                  "CHAP_A"
+#define ISCSI_KEY_CHAP_IDENTIFIER                 "CHAP_I"
+#define ISCSI_KEY_CHAP_CHALLENGE                  "CHAP_C"
+#define ISCSI_KEY_CHAP_NAME                       "CHAP_N"
+#define ISCSI_KEY_CHAP_RESPONSE                   "CHAP_R"
 
-#define ISCSI_CHAP_ALGORITHM_MD5  5
+#define ISCSI_CHAP_ALGORITHM_MD5                  5
 
 ///
 /// MD5_HASHSIZE
 ///
-#define ISCSI_CHAP_RSP_LEN        16
+#define ISCSI_CHAP_RSP_LEN                        16
 
-#define ISCSI_CHAP_STEP_ONE       1
-#define ISCSI_CHAP_STEP_TWO       2
-#define ISCSI_CHAP_STEP_THREE     3
-#define ISCSI_CHAP_STEP_FOUR      4
+#define ISCSI_CHAP_STEP_ONE                       1
+#define ISCSI_CHAP_STEP_TWO                       2
+#define ISCSI_CHAP_STEP_THREE                     3
+#define ISCSI_CHAP_STEP_FOUR                      4
 
 
 #pragma pack(1)
 
 typedef struct _ISCSI_CHAP_AUTH_CONFIG_NVDATA {
   UINT8 CHAPType;
   CHAR8 CHAPName[ISCSI_CHAP_NAME_STORAGE];
   CHAR8 CHAPSecret[ISCSI_CHAP_SECRET_STORAGE];
   CHAR8 ReverseCHAPName[ISCSI_CHAP_NAME_STORAGE];
   CHAR8 ReverseCHAPSecret[ISCSI_CHAP_SECRET_STORAGE];
 } ISCSI_CHAP_AUTH_CONFIG_NVDATA;
 
 #pragma pack()
 
 ///
 /// ISCSI CHAP Authentication Data
 ///
 typedef struct _ISCSI_CHAP_AUTH_DATA {
   ISCSI_CHAP_AUTH_CONFIG_NVDATA *AuthConfig;
diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c
index 7e930c0d1eab..bb84f4359d35 100644
--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c
+++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c
@@ -14,44 +14,44 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @param[in]   ChapIdentifier     iSCSI CHAP identifier sent by authenticator.
   @param[in]   ChapSecret         iSCSI CHAP secret of the authenticator.
   @param[in]   SecretLength       The length of iSCSI CHAP secret.
   @param[in]   ChapChallenge      The challenge message sent by authenticator.
   @param[in]   ChallengeLength    The length of iSCSI CHAP challenge message.
   @param[out]  ChapResponse       The calculation of the expected hash value.
 
   @retval EFI_SUCCESS             The expected hash value was calculatedly
                                   successfully.
   @retval EFI_PROTOCOL_ERROR      The length of the secret should be at least
                                   the length of the hash value for the hashing
                                   algorithm chosen.
   @retval EFI_PROTOCOL_ERROR      MD5 hash operation fail.
   @retval EFI_OUT_OF_RESOURCES    Fail to allocate resource to complete MD5.
 
 **/
 EFI_STATUS
 IScsiCHAPCalculateResponse (
-  IN  UINT32  ChapIdentifier,
-  IN  CHAR8   *ChapSecret,
-  IN  UINT32  SecretLength,
-  IN  UINT8   *ChapChallenge,
-  IN  UINT32  ChallengeLength,
-  OUT UINT8   *ChapResponse
+  IN  UINT32          ChapIdentifier,
+  IN  CHAR8           *ChapSecret,
+  IN  UINT32          SecretLength,
+  IN  UINT8           *ChapChallenge,
+  IN  UINT32          ChallengeLength,
+  OUT UINT8           *ChapResponse
   )
 {
   UINTN       Md5ContextSize;
   VOID        *Md5Ctx;
   CHAR8       IdByte[1];
   EFI_STATUS  Status;
 
   if (SecretLength < ISCSI_CHAP_SECRET_MIN_LEN) {
     return EFI_PROTOCOL_ERROR;
   }
 
   Md5ContextSize = Md5GetContextSize ();
   Md5Ctx = AllocatePool (Md5ContextSize);
   if (Md5Ctx == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
   Status = EFI_PROTOCOL_ERROR;
 


Re: [PATCH 1/6] NetworkPkg/IScsiDxe: re-set session-level authentication state before login

Maciej Rabeda
 

Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 08-Jun-21 15:06, Laszlo Ersek wrote:
RFC 7143 explains that a single iSCSI session may use multiple TCP
connections. The first connection established is called the leading
connection. The login performed on the leading connection is called the
leading login. Before the session is considered full-featured, the leading
login must succeed. Further (non-leading) connections can be associated
with the session later.

(It's unclear to me from RFC 7143 whether the non-leading connections
require individual (non-leading) logins as well, but that particular
question is irrelevant from the perspective of this patch; see below.)

The data model in IScsiDxe exhibits some confusion, regarding connection /
session association:

- On one hand, the "ISCSI_SESSION.Conns" field is a *set* (it has type
LIST_ENTRY), and accordingly, connections can be added to, and removed
from, a session, with the IScsiAttatchConnection() and
IScsiDetatchConnection() functions.

- On the other hand, ISCSI_MAX_CONNS_PER_SESSION has value 1, therefore no
session will ever use more than 1 connection at a time (refer to
instances of "Session->MaxConnections" in
"NetworkPkg/IScsiDxe/IScsiProto.c").

This one-to-many confusion between ISCSI_SESSION and ISCSI_CONNECTION is
very visible in the CHAP logic, where the progress of the authentication
is maintained *per connection*, in the "ISCSI_CONNECTION.AuthStep" field
(with values such as ISCSI_AUTH_INITIAL, ISCSI_CHAP_STEP_ONE, etc), but
the *data* for the authentication are maintained *per session*, in the
"AuthType" and "AuthData" fields of ISCSI_SESSION. Clearly, this makes no
sense if multiple connections are eligible for logging in.

Knowing that IScsiDxe uses only one connection per session (put
differently: knowing that any connection is a leading connection, and any
login is a leading login), there is no functionality bug. But the data
model is still broken: "AuthType", "AuthData", and "AuthStep" should be
maintained at the *same* level -- be it "session-level" or "(leading)
connection-level".

Fixing this data model bug is more than what I'm signing up for. However,
I do need to add one function, in preparation for multi-hash support:
whenever a new login is attempted (put differently: whenever the leading
login is re-attempted), which always happens with a fresh connection, the
session-level authentication data needs to be rewound to a sane initial
state.

Introduce the IScsiSessionResetAuthData() function. Call it from the
central -- session-level -- IScsiSessionLogin() function, just before the
latter calls the -- connection-level -- IScsiConnLogin() function.

Right now, do nothing in IScsiSessionResetAuthData(); so functionally
speaking, the patch is a no-op.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/NetworkPkg/IScsiDxe/IScsiProto.c b/NetworkPkg/IScsiDxe/IScsiProto.c
index 6983f0fa5973..69d1b39dbb1f 100644
--- a/NetworkPkg/IScsiDxe/IScsiProto.c
+++ b/NetworkPkg/IScsiDxe/IScsiProto.c
@@ -401,38 +401,55 @@ IScsiGetIp6NicInfo (
if (Ip6ModeData.GroupTable!= NULL) {
FreePool (Ip6ModeData.GroupTable);
}
if (Ip6ModeData.RouteTable!= NULL) {
FreePool (Ip6ModeData.RouteTable);
}
if (Ip6ModeData.NeighborCache!= NULL) {
FreePool (Ip6ModeData.NeighborCache);
}
if (Ip6ModeData.PrefixTable!= NULL) {
FreePool (Ip6ModeData.PrefixTable);
}
if (Ip6ModeData.IcmpTypeList!= NULL) {
FreePool (Ip6ModeData.IcmpTypeList);
}
return Status;
}
+/**
+ Re-set any stateful session-level authentication information that is used by
+ the leading login / leading connection.
+
+ (Note that this driver only supports a single connection per session -- see
+ ISCSI_MAX_CONNS_PER_SESSION.)
+
+ @param[in,out] Session The iSCSI session.
+**/
+STATIC
+VOID
+IScsiSessionResetAuthData (
+ IN OUT ISCSI_SESSION *Session
+ )
+{
+}
+
/**
Login the iSCSI session.
@param[in] Session The iSCSI session.
@retval EFI_SUCCESS The iSCSI session login procedure finished.
@retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
@retval EFI_NO_MEDIA There was a media error.
@retval Others Other errors as indicated.
**/
EFI_STATUS
IScsiSessionLogin (
IN ISCSI_SESSION *Session
)
{
EFI_STATUS Status;
ISCSI_CONNECTION *Conn;
VOID *Tcp;
@@ -454,38 +471,39 @@ IScsiSessionLogin (
//
CopyMem (Session->Isid, Session->ConfigData->SessionConfigData.IsId, 6);
RetryCount = 0;
do {
//
// Create a connection for the session.
//
Conn = IScsiCreateConnection (Session);
if (Conn == NULL) {
return EFI_OUT_OF_RESOURCES;
}
IScsiAttatchConnection (Session, Conn);
//
// Login through the newly created connection.
//
+ IScsiSessionResetAuthData (Session);
Status = IScsiConnLogin (Conn, Session->ConfigData->SessionConfigData.ConnectTimeout);
if (EFI_ERROR (Status)) {
IScsiConnReset (Conn);
IScsiDetatchConnection (Conn);
IScsiDestroyConnection (Conn);
}
if (Status != EFI_TIMEOUT) {
break;
}
RetryCount++;
} while (RetryCount <= Session->ConfigData->SessionConfigData.ConnectRetryCount);
if (!EFI_ERROR (Status)) {
Session->State = SESSION_STATE_LOGGED_IN;
if (!Conn->Ipv6Flag) {
ProtocolGuid = &gEfiTcp4ProtocolGuid;


[edk2-platforms][PATCH v2 5/5] Platform/Sgi: Cleanup build options for StandaloneMM context

Pranav Madhu
 

From: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>

The Arm reference design platforms support only AArch64 mode for
StandaloneMM execution context. So cleanup the existing build options
specified for StandaloneMM.

Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
---
Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc b/Platform/ARM/Sgi=
Pkg/SgiPlatformMm.dsc.inc
index 6839ec35da8a..2b461d5afbcb 100644
--- a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
@@ -144,7 +144,5 @@
#
########################################################################=
###########################
[BuildOptions.AARCH64]
- GCC:*_*_*_DLINK_FLAGS =3D -z common-page-size=3D0x1000 -march=3Darmv8-=
a+nofp
-
-[BuildOptions]
- *_*_*_CC_FLAGS =3D -D DISABLE_NEW_DEPRECATED_INTERFACES
+ GCC:*_*_*_CC_FLAGS =3D -mstrict-align -march=3Darmv8-a+nofp -D DISABLE=
_NEW_DEPRECATED_INTERFACES
+ GCC:*_*_*_DLINK_FLAGS =3D -z common-page-size=3D0x1000
--=20
2.17.1


[edk2-platforms][PATCH v2 4/5] Platform/Sgi: update _OSC control method to control LPI and CPPC

Pranav Madhu
 

Define and use the global macro LPI_EN and CPPC_EN to enable low power
idle and CPPC support for reference design platforms. Update platform
wide _OSC control method to enable/disable low power idle and CPPC
support based on pcd PcdOscLpiEnable and PcdOscCppcEnable. The pcds
are controlled by the global macros LPI_EN and CPPC_EN.

Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
---
Platform/ARM/SgiPkg/SgiPlatform.dec | 4 ++++
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 14 +++++++++++=
+++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf | 1 +
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf | 1 +
Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 2 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 2 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf | 2 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf | 2 ++
Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 1 +
Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 2 ++
Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl | 8 ++++++++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl | 8 ++++++++
Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl | 15 +++++++++++=
++++
Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl | 15 +++++++++++=
++++
Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl | 15 +++++++++++=
++++
Platform/ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl | 15 +++++++++++=
++++
Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl | 8 ++++++++
17 files changed, 115 insertions(+)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/Sg=
iPlatform.dec
index ffbbb24f1c33..8cd818a9bf64 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -86,5 +86,9 @@
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize|0|UINT32|0x00000023
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt|0|UINT32|0x00000024
=20
+ # ACPI platform wide _OSC
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable|0|UINT32|0x00000025
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable|0|UINT32|0x00000026
+
[Ppis]
gNtFwConfigDtInfoPpiGuid =3D { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8,=
0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPk=
g/SgiPlatform.dsc.inc
index 2851cf180c0e..7e37732fb93c 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
@@ -17,6 +17,10 @@
# To allow HDLCD display using the Graphics Output Protocol, set this =
to TRUE.
DEFINE ENABLE_GOP =3D FALSE
=20
+ # To enable LPI and CPPC power management functionality, set this to T=
RUE.
+ DEFINE LPI_EN =3D FALSE
+ DEFINE CPPC_EN =3D FALSE
+
[BuildOptions]
*_*_*_CC_FLAGS =3D -D DISABLE_NEW_DEPRECATED_INTERFACES
=20
@@ -108,6 +112,16 @@
gArmSgiTokenSpaceGuid.PcdDramBlock2Base|0x8080000000
gArmSgiTokenSpaceGuid.PcdDramBlock2Size|0x180000000
=20
+!if $(LPI_EN) =3D=3D TRUE
+ # Allow use of LPI in the response to _OSC method call
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable|1
+!endif
+
+!if $(CPPC_EN) =3D=3D TRUE
+ # Allow use of CPPC in the response to _OSC method call
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable|1
+!endif
+
# NV Storage PCDs. Use base of 0x08000000 for NOR0, 0xC0000000 for NOR=
1
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x0140000=
0
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x01400=
000
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf b/Plat=
form/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
index 8d46b001444c..ce89aa93ea7b 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
@@ -57,6 +57,7 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf b/Pl=
atform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
index 473c9eff0f55..1999bc1553e9 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
@@ -66,6 +66,7 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform=
/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
index c537db45e08f..25be2e276e85 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
@@ -57,6 +57,8 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Plat=
form/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
index 6bbc3fc230ae..4b36c3e5ceb2 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
@@ -57,6 +57,8 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable
gArmSgiTokenSpaceGuid.PcdSmmuBase
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf b/Platform=
/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
index d461cbe54c68..97a87462932b 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
@@ -57,6 +57,8 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf b/Platfo=
rm/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
index 3b699b0acbb8..deaca3719ae4 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
@@ -66,6 +66,8 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
+ gArmSgiTokenSpaceGuid.PcdOscCppcEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf b/Platfo=
rm/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
index 3ee66b1dfd5a..a1bd71fde761 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
@@ -57,6 +57,7 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdOscLpiEnable
gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
diff --git a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h b/Platform/ARM/S=
giPkg/Include/SgiAcpiHeader.h
index 7b8c16b172c0..d75d54055436 100644
--- a/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
+++ b/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h
@@ -37,6 +37,8 @@
=20
// ACPI OSC for Platform-Wide Capability
#define OSC_CAP_CPPC_SUPPORT (1U << 5)
+#define OSC_CAP_CPPC2_SUPPORT (1U << 6)
+#define OSC_CAP_PLAT_COORDINATED_LPI (1U << 7)
#define OSC_CAP_OS_INITIATED_LPI (1U << 8)
=20
#pragma pack(1)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl b/Platform/=
ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl
index a2258f61aeca..bd8efa544a59 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl
@@ -29,6 +29,14 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD",=
"ARMSGI",
And (CAP0, Not (OSC_CAP_OS_INITIATED_LPI), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl b/Platfor=
m/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl
index 5807658e7815..9cb2b175418c 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl
@@ -31,6 +31,14 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD",=
"ARMSGI",
And (CAP0, Not (OSC_CAP_OS_INITIATED_LPI), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl b/Platform/ARM/=
SgiPkg/AcpiTables/RdN2/Dsdt.asl
index a318ef48ded9..ccd98f829652 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl
@@ -30,11 +30,26 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD"=
, "ARMSGI",
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
=20
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
If (And (CAP0, OSC_CAP_CPPC_SUPPORT)) {
/* CPPC revision 1 and below not supported */
And (CAP0, Not (OSC_CAP_CPPC_SUPPORT), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_CPPC2_SUPPORT)) {
+ if (LEqual (FixedPcdGet32 (PcdOscCppcEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_CPPC2_SUPPORT), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl b/Platform/=
ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl
index 411eff84334a..b6decc77f480 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl
@@ -36,11 +36,26 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD"=
, "ARMSGI",
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
=20
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
If (And (CAP0, OSC_CAP_CPPC_SUPPORT)) {
/* CPPC revision 1 and below not supported */
And (CAP0, Not (OSC_CAP_CPPC_SUPPORT), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_CPPC2_SUPPORT)) {
+ if (LEqual (FixedPcdGet32 (PcdOscCppcEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_CPPC2_SUPPORT), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl b/Platform/ARM/=
SgiPkg/AcpiTables/RdV1/Dsdt.asl
index 0f632673d050..db9c19780e16 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl
@@ -30,11 +30,26 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD"=
, "ARMSGI",
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
=20
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
If (And (CAP0, OSC_CAP_CPPC_SUPPORT)) {
/* CPPC revision 1 and below not supported */
And (CAP0, Not (OSC_CAP_CPPC_SUPPORT), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_CPPC2_SUPPORT)) {
+ if (LEqual (FixedPcdGet32 (PcdOscCppcEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_CPPC2_SUPPORT), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl b/Platform/AR=
M/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl
index 622d522532a3..e084d82de7c0 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl
@@ -30,11 +30,26 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 2, "ARMLTD"=
, "ARMSGI",
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
=20
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
If (And (CAP0, OSC_CAP_CPPC_SUPPORT)) {
/* CPPC revision 1 and below not supported */
And (CAP0, Not (OSC_CAP_CPPC_SUPPORT), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_CPPC2_SUPPORT)) {
+ if (LEqual (FixedPcdGet32 (PcdOscCppcEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_CPPC2_SUPPORT), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl b/Platform/AR=
M/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
index e879a681fabf..a292d20d8afb 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl
@@ -28,6 +28,14 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 2, "ARMLTD", =
"ARMSGI", EFI_ACPI_ARM_OEM
And (CAP0, Not (OSC_CAP_OS_INITIATED_LPI), CAP0)
Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
}
+
+ If (And (CAP0, OSC_CAP_PLAT_COORDINATED_LPI)) {
+ if (LEqual (FixedPcdGet32 (PcdOscLpiEnable), Zero)) {
+ And (CAP0, Not (OSC_CAP_PLAT_COORDINATED_LPI), CAP0)
+ Or (STS0, OSC_STS_CAPABILITY_MASKED, STS0)
+ }
+ }
+
} Else {
And (STS0, Not (OSC_STS_MASK), STS0)
Or (STS0, Or (OSC_STS_FAILURE, OSC_STS_UNRECOGNIZED_REV), STS0=
)
--=20
2.17.1


[edk2-platforms][PATCH v2 3/5] Platform/Sgi: define the macro ENABLE_GOP

Pranav Madhu
 

From: Thomas Abraham <thomas.abraham@arm.com>

Define and use the global macro ENABLE_GOP to enable the use of the
Graphics Output Protocol (GOP). Enabling this macro allows GOP protocol
to be used for display on the HDLCD controller of the platform. This
macro is set to false by default for the all supported platforms.

Signed-off-by: Thomas Abraham <thomas.abraham@arm.com>
Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
---
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 5 +++++
Platform/ARM/SgiPkg/SgiPlatform.fdf | 2 ++
2 files changed, 7 insertions(+)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPk=
g/SgiPlatform.dsc.inc
index e4aee7a09acf..2851cf180c0e 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
@@ -14,6 +14,9 @@
# Secure boot requires secure storage to be enabled as well.
DEFINE SECURE_BOOT_ENABLE =3D FALSE
=20
+ # To allow HDLCD display using the Graphics Output Protocol, set this =
to TRUE.
+ DEFINE ENABLE_GOP =3D FALSE
+
[BuildOptions]
*_*_*_CC_FLAGS =3D -D DISABLE_NEW_DEPRECATED_INTERFACES
=20
@@ -234,7 +237,9 @@
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
ArmPkg/Drivers/TimerDxe/TimerDxe.inf
ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+!if $(ENABLE_GOP) =3D=3D TRUE
ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
+!endif
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
EmbeddedPkg/MetronomeDxe/MetronomeDxe.inf
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/Sg=
iPlatform.fdf
index d94e4633e36c..8227ae03330c 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -153,7 +153,9 @@ READ_LOCK_STATUS =3D TRUE
INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
=20
# Graphics Output Protocol
+!if $(ENABLE_GOP) =3D=3D TRUE
INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.i=
nf
+!endif
=20
INF Platform/ARM/Drivers/BootMonFs/BootMonFs.inf
INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
--=20
2.17.1


[edk2-platforms][PATCH v2 2/5] Platform/Sgi: Add GED support

Pranav Madhu
 

Add ACPI Generic Event Device (GED) support for Arm's reference design
platforms. The SP804 dual-timer interrupt is used as the event source
for GED.

Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
---
Platform/ARM/SgiPkg/SgiPlatform.dec | 5 ++
Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc | 5 ++
Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 5 ++
Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 3 ++
Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl | 49 ++++++++++++++++++++
12 files changed, 88 insertions(+)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index e0aabc566d88..ffbbb24f1c33 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -81,5 +81,10 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Size|0|UINT32|0x00000020
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt|0|UINT32|0x00000021

+ # SP804 Dual Timer
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress|0|UINT32|0x00000022
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize|0|UINT32|0x00000023
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt|0|UINT32|0x00000024
+
[Ppis]
gNtFwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
index a567af8537ec..76707be73d7b 100644
--- a/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
@@ -51,6 +51,11 @@
gArmSgiTokenSpaceGuid.PcdWdogWS0Gsiv|93
gArmSgiTokenSpaceGuid.PcdWdogWS1Gsiv|94

+ # SP804 dual timer
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress|0x1C110000
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize|0x00010000
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt|230
+
# SMMU
gArmSgiTokenSpaceGuid.PcdSmmuBase|0x4F000000
gArmSgiTokenSpaceGuid.PcdSmmuSize|0x01000000
diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
index 5c137c0991e7..2d612f9b9674 100644
--- a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
@@ -51,6 +51,11 @@
gArmSgiTokenSpaceGuid.PcdWdogWS0Gsiv|110
gArmSgiTokenSpaceGuid.PcdWdogWS1Gsiv|111

+ # SP804 dual timer
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress|0x0C110000
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize|0x00010000
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt|486
+
# SMMU
gArmSgiTokenSpaceGuid.PcdSmmuBase|0x40000000
gArmSgiTokenSpaceGuid.PcdSmmuSize|0x10000000
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
index 56b80f418398..8c34c2fa73e4 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
@@ -57,6 +57,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
index fa6692bc86f6..8d46b001444c 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
@@ -57,6 +57,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
index d0ee125fa1de..473c9eff0f55 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
@@ -66,6 +66,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
index 232b58eb012f..c537db45e08f 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
@@ -57,6 +57,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
index 5713ef1ce3a9..6bbc3fc230ae 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
@@ -58,6 +58,9 @@
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdSmmuBase
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
index e7b702962abe..d461cbe54c68 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
@@ -57,6 +57,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
index 901391fba70e..3b699b0acbb8 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
@@ -66,6 +66,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
index d59aefde735b..3ee66b1dfd5a 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
@@ -57,6 +57,9 @@
gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerSize
+ gArmSgiTokenSpaceGuid.PcdSp804DualTimerInterrupt
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
gArmSgiTokenSpaceGuid.PcdVirtioBlkInterrupt
diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl
index 28c7ce4e7f3c..58efe62c93ea 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl
+++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl
@@ -9,12 +9,20 @@
Interrupt is received by OSPM and that GPIO Interrupt Connection is listed in
a GPIO controller device’s _AEI object.

+ Interrupt Signalled ACPI event is another method of signalling events in
+ HW-Reduced ACPI model. In this method, ACPI event is generated when an
+ interrupt is received by the OSPM which is listed in the Generic Event Device
+ (GED) _CRS object.
+
Copyright (c) 2021, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Specification Reference:
- ACPI 6.4, Chapter 5.6.5, GPIO-signaled ACPI Events
- Arm Base Boot Requirements 1.0, Issue F, Chapter 8.5.3, GPIO controllers
+ - ACPI 6.4, Chapter 5.6.9, Interrupt-signaled ACPI events
+ - Arm Base Boot Requirements 1.0, Issue F, Chapter 8.5.4 Generic Event
+ Devices
**/

#include "SgiAcpiHeader.h"
@@ -64,4 +72,45 @@ DefinitionBlock("SsdtEvent.aml", "SSDT", 2, "ARMLTD", "ARMSGI", EFI_ACPI_ARM_OEM
INC0, 8
}
}
+
+ /* ACPI GED object Template. Arm's reference design platforms include a SP804
+ dual timer which is implemented as part of the RoS sub-system. The SP804
+ interrupt is used in GED as interrupt source. */
+ Device (\_SB.GED0) {
+ Name (_HID, "ACPI0013")
+ Name (_UID, 0)
+
+ /* Resource setting for GED */
+ Name (_CRS, ResourceTemplate () {
+ Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
+ FixedPcdGet32 (PcdSp804DualTimerInterrupt)
+ }
+ })
+
+ Method (_STA, 0x0, NotSerialized) {
+ return (0xF);
+ }
+
+ /* Register map for interrupt clear register */
+ OperationRegion (
+ DTIM,
+ SystemMemory,
+ FixedPcdGet32 (PcdSp804DualTimerBaseAddress),
+ FixedPcdGet32 (PcdSp804DualTimerSize))
+ Field (DTIM, DWordAcc, NoLock, Preserve) {
+ Offset (0x0C),
+ T1IC, 32, /* 0x0C Timer 1 Interrupt clear */
+ }
+
+ /* GED event handler */
+ Method (_EVT, 1, Serialized) {
+ switch (ToInteger (Arg0))
+ {
+ case (FixedPcdGet32 (PcdSp804DualTimerInterrupt)) {
+ Store (0x01, T1IC)
+ }
+ }
+ }
+ } /* Device (\_SB.GED0) */
+
}
--
2.17.1


[edk2-platforms][PATCH v2 1/5] Platform/Sgi: Enable PrimeCell GPIO

Pranav Madhu
 

The HW-Reduced ACPI model has specific requirements for GPIO
controllers. Arm's reference design Platforms has PrimeCell GPIO
(PL061) integrated in the RoS subsystem to provide GPIO support. Add
GPIO device entry and also add GPIO signalled ACPI event template for
reference.

Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
---
Platform/ARM/SgiPkg/SgiPlatform.dec | 5 ++
Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc | 5 ++
Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 5 ++
Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf | 4 ++
Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 5 ++
Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl | 67 ++++++++++++++++++++
12 files changed, 115 insertions(+)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index af08ed153eae..e0aabc566d88 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -76,5 +76,10 @@
gArmSgiTokenSpaceGuid.PcdSmmuBase|0|UINT32|0x0000001D
gArmSgiTokenSpaceGuid.PcdSmmuSize|0|UINT32|0x0000001E

+ # GPIO Controller
+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress|0|UINT32|0x0000001F
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size|0|UINT32|0x00000020
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt|0|UINT32|0x00000021
+
[Ppis]
gNtFwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
index d3d650323891..a567af8537ec 100644
--- a/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc
@@ -54,3 +54,8 @@
# SMMU
gArmSgiTokenSpaceGuid.PcdSmmuBase|0x4F000000
gArmSgiTokenSpaceGuid.PcdSmmuSize|0x01000000
+
+ # GPIO Controller
+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress|0x1C1D0000
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size|0x00010000
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt|136
diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
index c593156e17be..5c137c0991e7 100644
--- a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
@@ -54,3 +54,8 @@
# SMMU
gArmSgiTokenSpaceGuid.PcdSmmuBase|0x40000000
gArmSgiTokenSpaceGuid.PcdSmmuSize|0x10000000
+
+ # GPIO controller
+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress|0x0C1D0000
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size|0x00010000
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt|392
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
index 04ef2bfcaa26..56b80f418398 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf
@@ -26,6 +26,7 @@
RdE1Edge/Pptt.aslc
Spcr.aslc
Ssdt.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -51,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
index eecb64186473..fa6692bc86f6 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf
@@ -26,6 +26,7 @@
RdN1Edge/Pptt.aslc
Spcr.aslc
Ssdt.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -51,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
index 617519d9dd38..d0ee125fa1de 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN1EdgeX2AcpiTables.inf
@@ -28,6 +28,7 @@
RdN1EdgeX2/Srat.aslc
Spcr.aslc
Ssdt.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -60,6 +61,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
index c1282a3422ab..232b58eb012f 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
@@ -26,6 +26,7 @@
Spcr.aslc
Ssdt.asl
SsdtRos.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -51,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
index 58468096de4f..5713ef1ce3a9 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
@@ -26,6 +26,7 @@
Spcr.aslc
Ssdt.asl
SsdtRos.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -51,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdSmmuBase
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
index a3e558cf1535..e7b702962abe 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf
@@ -26,6 +26,7 @@
RdV1/Pptt.aslc
Spcr.aslc
Ssdt.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -51,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
index ffda4f925b19..901391fba70e 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdV1McAcpiTables.inf
@@ -28,6 +28,7 @@
RdV1Mc/Srat.aslc
Spcr.aslc
Ssdt.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -60,6 +61,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
index b1ee16e98ea3..d59aefde735b 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
@@ -25,6 +25,8 @@
Sgi575/Pptt.aslc
Spcr.aslc
Ssdt.asl
+ SsdtRos.asl
+ SsdtEvents.asl

[Packages]
ArmPkg/ArmPkg.dec
@@ -50,6 +52,9 @@
gArmTokenSpaceGuid.PcdPciBusMin
gArmTokenSpaceGuid.PcdPciBusMax

+ gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
+ gArmSgiTokenSpaceGuid.PcdGpioController0Size
+ gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl
new file mode 100644
index 000000000000..28c7ce4e7f3c
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl
@@ -0,0 +1,67 @@
+/** @file
+ Secondary System Description Table (SSDT) for hardware reduced events.
+
+ Arm Reference Design platforms implement the HW-Reduced ACPI model and do not
+ support legacy ACPI Fixed Hardware interfaces.
+
+ GPIO Signalled ACPI event is one of the methods for signalling events in
+ HW-Reduced ACPI model. In this method, ACPI events can be signaled when a GPIO
+ Interrupt is received by OSPM and that GPIO Interrupt Connection is listed in
+ a GPIO controller device’s _AEI object.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - ACPI 6.4, Chapter 5.6.5, GPIO-signaled ACPI Events
+ - Arm Base Boot Requirements 1.0, Issue F, Chapter 8.5.3, GPIO controllers
+**/
+
+#include "SgiAcpiHeader.h"
+
+DefinitionBlock("SsdtEvent.aml", "SSDT", 2, "ARMLTD", "ARMSGI", EFI_ACPI_ARM_OEM_REVISION) {
+ /* GPIO Controller 0 device. Use _AEI object to configure pin 0 for
+ signalling HW-Reduced events and the _L00 method to handle the event
+ generated by pin 0.
+ */
+ Device (\_SB.GPI0)
+ {
+ Name (_HID, "ARMH0061") /* PrimeCell GPIO */
+ Name (_UID, 0)
+
+ /* Resource setting for GPIO controller 0 */
+ Name (_CRS, ResourceTemplate () {
+ Memory32Fixed (
+ ReadWrite,
+ FixedPcdGet32 (PcdGpioController0BaseAddress),
+ FixedPcdGet32 (PcdGpioController0Size)
+ )
+ Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
+ FixedPcdGet32 (PcdGpioController0Interrupt)
+ }
+ })
+
+ /* ACPI Event information for GPIO controller 0 */
+ Name (_AEI, ResourceTemplate() {
+ GpioInt (Level, ActiveHigh, Exclusive, PullDown, , "\\_SB.GPI0") {0}
+ })
+
+ /* Event handler for pin0 */
+ Method (_L00) {
+ Printf ("GPIO0 Pin0 Toggled")
+ Store (1, INC0)
+ }
+
+ /* Mapping for interrupt clear register */
+ OperationRegion (
+ GIO0,
+ SystemMemory,
+ FixedPcdGet32 (PcdGpioController0BaseAddress),
+ FixedPcdGet32 (PcdGpioController0Size)
+ )
+ Field (GIO0, ByteAcc, NoLock, Preserve) {
+ Offset (0x41C), /* WO Intr clear on writing 1 to resp bit */
+ INC0, 8
+ }
+ }
+}
--
2.17.1


[edk2-platforms][PATCH v2 0/5] Platform/Sgi: Miscellaneous updates

Pranav Madhu
 

Changes since V1:
- Rebase the patches on top of latest master branch

This patch series introduces few miscellaneous updates for Arm's
Reference Design platforms. The first two patches add a template
support for hardware reduced ACPI events. This allows helps the
platform get better compliance coverage with SystemReady test
suite. The third patch adds a build macro for enabling graphics
output via the HDLCD. The fourth patch allow build time control
for CPPC and LPI features on the platform. The fifth patch fixes
StandaloneMM build options for the RD platforms.

Link to github branch with the patches in this series -
https://github.com/Pranav-Madhu/edk2-platforms/tree/topic/rd_misc

Omkar Anand Kulkarni (1):
Platform/Sgi: Cleanup build options for StandaloneMM context

Pranav Madhu (3):
Platform/Sgi: Enable PrimeCell GPIO
Platform/Sgi: Add GED support
Platform/Sgi: update _OSC control method to control LPI and CPPC

Thomas Abraham (1):
Platform/Sgi: define the macro ENABLE_GOP

Platform/ARM/SgiPkg/SgiPlatform.dec | 14 +++
Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc | 10 ++
Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 10 ++
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 19 +++
Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 6 +-
Platform/ARM/SgiPkg/SgiPlatform.fdf | 2 +
.../SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf | 7 ++
.../SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf | 8 ++
.../AcpiTables/RdN1EdgeX2AcpiTables.inf | 8 ++
.../ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 9 ++
.../ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/RdV1McAcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 9 ++
Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 2 +
.../ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl | 8 ++
.../ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl | 8 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl | 15 +++
Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl | 8 ++
Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl | 116 ++++++++++++++++++
23 files changed, 327 insertions(+), 4 deletions(-)
create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl

--=20
2.17.1


Re: [edk2-test][PATCH v2 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Heinrich Schuchardt
 

On 11.06.21 10:35, Sunny Wang wrote:
The commits a9d1fb58 and ae7e5477b555 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commits a9d1fb58 and
ae7e5477b555 to not create event with TPL_HIGH_LEVEL to be compliant
with UEFI Spec and fix the failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
.../EventTimerTaskPriorityServicesBBTestCreateEventEx.c | 4 +---
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index eb458de5..03b7ae6e 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2016 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -228,7 +229,6 @@ BBTestCreateEventEx_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {
@@ -318,7 +318,6 @@ BBTestCreateEventEx_Conf_Sub2 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {
@@ -413,7 +412,6 @@ BBTestCreateEventEx_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {


Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

Thanks for catching this, Heinrich.
I sent v2 to also revert TPL_HIGH_LEVEL changes in ae7e5477b555.

Best Regards,
Sunny Wang

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Heinrich Schuchardt via groups.io
Sent: Friday, June 11, 2021 4:24 PM
To: Sunny Wang <Sunny.Wang@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; G Edhaya Chandran <Edhaya.Chandran@arm.com>; Barton Gao <gaojie@byosoft.com.cn>; Michael D Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

On 11.06.21 09:07, Sunny Wang wrote:
The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338
Please, have a look at commit ae7e5477b555 ("uefi-sct/SctPkg: allowable
NotifyTpl in CreateEventEx") too.

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;





IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-test][PATCH v2 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

The commits a9d1fb58 and ae7e5477b555 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commits a9d1fb58 and
ae7e5477b555 to not create event with TPL_HIGH_LEVEL to be compliant
with UEFI Spec and fix the failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
.../EventTimerTaskPriorityServicesBBTestCreateEventEx.c | 4 +---
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
Event.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPr=
iorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEve=
nt.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
EventEx.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTask=
PriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateE=
ventEx.c
index eb458de5..03b7ae6e 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx=
.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx=
.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2016 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -228,7 +229,6 @@ BBTestCreateEventEx_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
@@ -318,7 +318,6 @@ BBTestCreateEventEx_Conf_Sub2 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
@@ -413,7 +412,6 @@ BBTestCreateEventEx_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
--=20
2.31.0.windows.1


Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Heinrich Schuchardt
 

On 11.06.21 09:07, Sunny Wang wrote:
The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338
Please, have a look at commit ae7e5477b555 ("uefi-sct/SctPkg: allowable
NotifyTpl in CreateEventEx") too.

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;


Re: [PATCH v1 2/5] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate

Wu, Hao A
 

A couple of minor comments below:

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1 2/5] MdeModulePkg: PiSmmIpl: Update MessageLength
calculation for MmCommunicate

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

This change updated calculation routine for MM communication in PiSmmIpl.
It removes ambiguity brought in by UINTN variables from this routine and
paves way for updating definition of field MessageLength in
EFI_MM_COMMUNICATE_HEADER to definitive size.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++++++++++++-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 599a0cd01d80..9508715fda24 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -34,6 +34,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h> // BZ3398

I suggest to remove the comment "// BZ3398" here.

I think users can use the 'blame' feature of the version control systems
together with the commit log message to find out the information.



#include "PiSmmCorePrivateData.h"

@@ -515,6 +516,7 @@ SmmCommunicationCommunicate (
EFI_STATUS Status;
EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
BOOLEAN OldInSmm;
+ UINT64 BZ3398_LongCommSize;

Suggest to drop the "BZ3398_" prefix for the variable name.


UINTN TempCommSize;

//
@@ -527,7 +529,16 @@ SmmCommunicationCommunicate (
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)
CommBuffer;

if (CommSize == NULL) {
- TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)
+ CommunicateHeader->MessageLength;
+ // BZ3398 Starts: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.

Suggest to drop the "// BZ3398 Starts:" and "// BZ3398 Ends" comments pair here.

With above handled:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


+ Status = SafeUint64Add (OFFSET_OF
(EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader-
MessageLength, &BZ3398_LongCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ Status = SafeUint64ToUintn (BZ3398_LongCommSize, &TempCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ // BZ3398 Ends
} else {
TempCommSize = *CommSize;
//
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..87142e27fa47 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib #BZ3398

[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
--
2.31.1.windows.1


Re: [PATCH v1 4/5] MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation

Wu, Hao A
 

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1 4/5] MdeModulePkg: SmiHandlerProfileInfo: Updated
MessageLength calculation

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to consumers.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
| 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
index 4153074b7a80..56d80d1a9ce1 100644
---
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
+++
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileIn
+++ fo.c
@@ -116,7 +116,9 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.

Similar comments as in patch 3/5.

Best Regards,
Hao Wu


+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication,
CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status); @@ -
149,7 +151,9 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;

--
2.31.1.windows.1


Re: [PATCH v1 3/5] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation

Wu, Hao A
 

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH v1 3/5] MdeModulePkg: MemoryProfileInfo: Updated
MessageLength calculation

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to consumers.

I think there is one missing place in function GetSmramProfileData():

MinimalSizeNeeded = sizeof (EFI_GUID) +
sizeof (UINTN) +
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_INFO),
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET),
sizeof (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)));

More inline comments below:



Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 20
+++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git
a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
index 191c31068545..39ed8b2e0484 100644
--- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+++
b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
@@ -1190,7 +1190,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.

Please help to drop the explicit mention of BZ3398 in the comment.
How about using:
//
// The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
//

There are 4 more similar cases below.

Best Regards,
Hao Wu


+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication,
CommBuffer, &CommSize);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n",
Status)); @@ -1213,7 +1215,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer,
&CommSize);
}

@@ -1230,7 +1234,9 @@ GetSmramProfileData (
CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1;
CommGetProfileInfo->ProfileSize = 0;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication,
CommBuffer, &CommSize);
ASSERT_EFI_ERROR (Status);

@@ -1261,7 +1267,9 @@ GetSmramProfileData (
CommGetProfileData->Header.DataLength = sizeof
(*CommGetProfileData);
CommGetProfileData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *) CommHeader + CommSize;
Size -= CommSize;

@@ -1320,7 +1328,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_ENABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer,
&CommSize);
}

--
2.31.1.windows.1


Re: [PATCH v1 1/5] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update

Wu, Hao A
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
Andrew Fish <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>; Leif
Lindholm <leif@nuviainc.com>
Subject: [edk2-devel] [PATCH v1 1/5] EDK2 Code First: PI Specification:
EFI_MM_COMMUNICATE_HEADER Update

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

This change includes specification update markdown file that describes the
proposed PI Specification v1.7 Errata A in detail and potential impact to the
existing codebase.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
BZ3430-SpecChange.md | 88 ++++++++++++++++++++
1 file changed, 88 insertions(+)

diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md new file mode
100644 index 000000000000..33a1ffda447b
--- /dev/null
+++ b/BZ3430-SpecChange.md
@@ -0,0 +1,88 @@
+# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER
to
+UINT64
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7
+Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER`
from UINTN to UINT64 to remove architecture dependency:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+## Benefits of the change
+
+In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the
MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as
`EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN.
+
+But this structure, as a generic definition, could be used for both PEI and
DXE MM communication. Thus for a system that supports PEI MM launch,
but operates PEI in 32bit mode and MM foundation in 64bit, the current
`EFI_MM_COMMUNICATE_HEADER` definition will cause structure parse
error due to UINTN used.
+
+## Impact of the change
+
+This change will impact the known structure consumers including:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl
+MdeModulePkg/Application/SmiHandlerProfileInfo
+MdeModulePkg/Application/MemoryProfileInfo
+```
+
+For consumers that are not using
`OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing
explicit addition such as the existing
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c,
one will need to change code implementation to match new structure
definition. Otherwise, the code compiled on IA32 architecture will
experience structure field dereference error.
+
+User who currently uses UINTN local variables as place holder of
MessageLength will need to use caution to make cast from UINTN to UINT64
and vice versa. It is recommended to use `SafeUint64ToUintn` for such
operations when the value is indeterministic.
+
+Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also
consuming this structure, but it handled this size discrepancy internally. If this

Hello Kun,

Sorry for a question.
I am not sure why the current codes in file SmmLockBoxDxeLib.c will work properly after the UINTN -> UINT64 change.

For example:
LockBoxGetSmmCommBuffer():
MinimalSizeNeeded = sizeof (EFI_GUID) +
sizeof (UINTN) +
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SAVE),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE),
sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)))));

SaveLockBox():
UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];

Is the series missed changes for SmmLockBoxDxeLib or I got something wrong?

Best Regards,
Hao Wu


potential spec change is not applied, all applicable PEI MM communicate
callers will need to use the same routine as that of SmmLockBoxPeiLib to
invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used
in X64 MM foundation.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of
`EFI_MM_COMMUNICATE_HEADER` should be changed from current:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINTN MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+to:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+### Code Changes
+
+1. Remove the explicit calculation of the offset of `Data` in
`EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of
`sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with
`OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar
alternatives. These calculations are identified in:
+
+```bash
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+```
+
+1. Resolve potentially mismatched type between `UINTN` and `UINT64`.
This would occur when `MessageLength` or its derivitive are used for local
calculation with existing `UINTN` typed variables. Code change regarding this
perspective is per case evaluation: if the variables involved are all
deterministic values, and there is no overflow or underflow risk, a cast
operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the
calculation will be performed in `UINT64` bitwidth and then convert to
`UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These
operations are identified in:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+```
+
+1. After all above changes applied and specification updated,
`MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to
match new definition that includes the field type update.
--
2.31.1.windows.1





Re: Commit a9d1fb58 - uefi-sct/SctPkg: allowable NotifyTpl in CreateEvent

Sunny Wang
 

I just sent a patch for this. https://edk2.groups.io/g/devel/message/76368
Please help review the patch so that we can pick this up in SCT stable release.

Best Regards,
Sunny Wang

-----Original Message-----
From: Andrew Fish <afish@apple.com>
Sent: Friday, June 11, 2021 2:50 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; xypron.glpk@gmx.de
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Barton Gao <gaojie@byosoft.com.cn>; G Edhaya Chandran <Edhaya.Chandran@arm.com>; Mike Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] Commit a9d1fb58 - uefi-sct/SctPkg: allowable NotifyTpl in CreateEvent



On Jun 10, 2021, at 11:05 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

On 6/10/21 7:22 PM, Samer El-Haj-Mahmoud wrote:
+ Mike

I agree with Sunny's evaluation. The original EDK2 patch from Jan had good feedback from Mike K.. The exact language from the UEFI spec section 7.1 is:

" Consequently, there is a fourth TPL, TPL_HIGH_LEVEL, designed for use exclusively by the firmware."
Firmware includes drivers. Drivers use CreateEvent().


" This is the highest priority level. It is not interruptible (interrupts are disabled) and is used sparingly by firmware to synchronize operations that need to be accessible from any priority level. For example, it must be possible to signal events while executing at any priority level. Therefore, firmware manipulates the internal event structure while at this priority level."

Given this, I think the following needs to happen:

1. The SCT commit (https://github.com/tianocore/edk2-test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58 ) needs to be reverted
2. The SCT change can be re-submitted as a new patch, without adding TPL_HIGH_LEVEL
3. The original EDK2 patch on Jan 6 (https://edk2.groups.io/g/devel/topic/79479680#69853) needs to be re-submitted per Mike's feedback. This means not adding the TPL_HIGH_LEVEL support in CreateEvent

In my opinion, (1) needs to be reverted right way, as it is causing SCT failures on many (all?) EDK2 based systems. This needs to happen before the edk2-test stable tag is created
Table 7-3 TPL Restrictions in UEFI spec 2.9 explicitly has:

Event Notification Levels
TPL_APPLICATION
<= TPL_HIGH_LEVEL

Nowhere does the specification say that CreateEvent() cannot be called
with with TPL_HIGH_LEVEL.

I can understand why Michael suggests that CreateEvent() should not be
callable at TPL_HIGH_LEVEL. But this requires a change request updating
the specification.
This is the other bit of the spec that might be relevant….

7.1 Event, Timer, and Task Priority Services
...
Execution in the boot services environment occurs at different task priority levels, or TPLs. The boot services environment exposes only three of these levels to UEFI applications and drivers:
• TPL_APPLICATION, the lowest priority level
• TPL_CALLBACK, an intermediate priority level
• TPL_NOTIFY, the highest priority level

Thanks,

Andrew Fish

I am missing this step in the action list above.

1) and 2) can be done with a single patch. Just don't revert the line
adding TPL_APPLICATION.

Best regards

Heinrich


Any thought about these suggested next steps?

Thanks,
--Samer

-----Original Message-----
From: Sunny Wang <Sunny.Wang@arm.com>
Sent: Thursday, June 10, 2021 4:26 AM
To: Heinrich Schuchardt <xypron.glpk@gmx.de>; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>; Barton Gao
<gaojie@byosoft.com.cn>; G Edhaya Chandran
<Edhaya.Chandran@arm.com>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Subject: RE: Commit a9d1fb58

Thanks for the information, Heinrich.

I think we need to further work on this for Mike's comment with your
ongoing edk2 patch.
-
https://edk2.groups.io/g/devel/message/69853?p=,,,20,0,0,0::relevance,,M
deModulePkg%3A+correct+TPL+level+check+CoreCreateEventEx,20,2,0,794
79680
For addressing Mike's comment, we will need to update both of your edk2
and edk2-test patches.
Therefore, I would suggest NOT picking up your edk2-test commit into the
upcoming stable tag/release. What do you guys think?

In other words, the main problem is that SCT should not create an event with
TPL_HIGH_LEVEL, so we need to revert TPL_HIGH_LEVEL related change in
Heinrich's commit https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58.

By the way, I confirmed this issue on two of my AARCH64 platforms (RPi4 and
another one).

Best Regards,
Sunny Wang

-----Original Message-----
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Sent: Thursday, June 10, 2021 1:04 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Barton
Gao <gaojie@byosoft.com.cn>; G Edhaya Chandran
<Edhaya.Chandran@arm.com>; Sunny Wang <Sunny.Wang@arm.com>
Subject: Re: Commit a9d1fb58

On 6/10/21 4:10 AM, Samer El-Haj-Mahmoud wrote:
With commit
https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58
<https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58>
, I am seeing the following new failure on some EDK2 based systems:

BS.CreateEvent - Create event with all valid event type and supported TPL.
--
*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000100, NotifyTpl -
31

BS.CreateEvent - Create event with all valid event type and supported TPL.
--
*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000200, NotifyTpl -
31

Are we sure this is implemented properly in EDK2? I imagine this code is
common in TianoCore/EDK2, and all systems will experience such failure.

I confirmed this on one AARCH64 system. I am trying to confirm on
another AARCH64 as well.

Can anyone confirm on other systems/archs?

If there is going to be a stable tag/release of edk2-test, we may want
to confirm this change does not cause failures in many systems before we
pick it up.

Thanks,

--Samer
An EDK II patch is available since 2021-01-06 via
https://bugzilla.tianocore.org/show_bug.cgi?id=3058
Cf. https://edk2.groups.io/g/devel/message/69851

Best regards

Heinrich
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
Event.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPr=
iorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEve=
nt.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
--=20
2.31.0.windows.1

4601 - 4620 of 80913