-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner via groups.io
Sent: Thursday, October 27, 2022 9:54 AM
To: devel@edk2.groups.io
Cc: Abner Chang <abner.chang@...>
Subject: [edk2-devel] [edk2-CCodingStandardsSpecification] Create release/2.30 branch

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Abner Chang <abner.chang@...>
---
book.json | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/book.json b/book.json
index d112b26..1fdd570 100644
--- a/book.json
+++ b/book.json
@@ -1,8 +1,7 @@
{

"variables" : {

- "draft" : "yes",

"title" : "EDK II C Coding Standards Specification",

- "version" : "Revision 2.2"

+ "version" : "Revision 2.3"

},

"plugins": ["puml-aleung"],

"pluginsConfig": {}

--
2.37.1.windows.1

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor
Kulchytskyy via groups.io
Sent: Thursday, October 27, 2022 10:42 PM
To: devel@edk2.groups.io; nicklew@...; Chang, Abner
<Abner.Chang@...>
Cc: Nick Ramirez <nramirez@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


Hi Nickle,
Pleased, see my comments on your questions below.

Another point I missed in my previous mail.
You have that function in the library to get credentials and it is
used by RedfishCredentialsDxe driver to create the protocol which in
its turn will be used by other Redfish modules.
We do not know how many modules and how many times will call this
function during boot. Right?
And on each call of this function you call IPMI command. That command
will create new Redfish account on BMC side according to Redfish HI specification:

" If the Get Bootstrap Account Credentials command has been issued and
responds with the completion code 00h, a bootstrap account shall be
added to the manager's account collection and enabled. If the Get
Bootstrap Account Credentials command is sent subsequent times and
responds with the completion code 00h, a new account shall be created
based on the newly generated credentials. Any existing bootstrap accounts shall remain active."

As I know BMC may have some restrictions on the number of Redfish
accounts they can support.
And because of that BOS may hit this limit. Which is not good.
On the other hand I'm not sure we need to have different credentials
for different modules? All those modules are part of FW. And all of
them will be associated with the same RoleID (FW role) on BMC side.
So, all of them may use the same credentials.
Could we cash the credentials on first call of that

RedfishCredentialGetAuthInfo and then use those cashed credentials on
subsequential calls?
It will also may save a boot time, since there is no need to send IPMI
command.

Thank you,
Igor

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle
Wang via groups.io
Sent: Thursday, October 27, 2022 9:26 AM
To: devel@edk2.groups.io; Igor Kulchytskyy <igork@...>;
abner.chang@...
Cc: Nick Ramirez <nramirez@...>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH]
RedfishPkg/RedfishPlatformCredentialLib: IPMI implementation


**CAUTION: The e-mail below is from an external source. Please
exercise caution before opening attachments, clicking links, or
following guidance.**

Hi Igor,

Thank you for your help to review my changes.

And it will be blocked by our IPMI call.

I see your point. So, BIOS should never be the person to shutdown
credential service because BIOS always get executed prior to OS, right?

Igor: Yes, my point is that we should not shutdown credential service
from BIOS. Even if OS sends that IPMI command, new account will be
created and BIOS credentials will not be compromised.

Should it be configured with some PCD? Maybe user may select in
Setup

what method should be used? Or it could be build time configuration?

I have below assumption while I implemented the library. I admit this
is not always true.

No Auth: I think this is rare case for Redfish service which gives
anonymous privilege to change BIOS settings.
Basic Auth: this is the authentication method which uses username and
password to build base64 encoded string.
Session Auth: I assume that client must have a session token first and
then use this authentication method. Can we use username and password
to generate session token on our own? If my memory serves me
correctly, client has to do a login with username and password first
and then client can receive session token from server.

Igor: BIOS will use the credentials to create session. It should send
POST request to the session URI with user name and password to create session.
If a session created successfully then on response BMC returns header
"X- Auth-Token" which then used for the subsequential calls.

If we really like to know what authentication method that Redfish
service used, we can issue a HTTP query to "/redfish/v1/Systems" with "No Auth".
Then we can know what authentication method is required by reading the
"WWW-Authenticate " filed in returned HTTP header.

Igor: As my understanding, even if you include authentication header
(Base64 encoded) in the request to BMC and BMC has NoAuth
configuration, then that authentication header would be just ignored by BMC.

Thanks,
Nickle

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor
Kulchytskyy via groups.io
Sent: Wednesday, October 26, 2022 11:26 PM
To: Nickle Wang <nicklew@...>; devel@edk2.groups.io;
abner.chang@...
Cc: Nick Ramirez <nramirez@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

External email: Use caution opening links or attachments


Hi Nickle,
I would like to discuss that DisableBootstrapControl flag and how it
is used in our implementation.
According to Redfish HI specification we can use this flag to disable
credential bootstrapping control.
It can be disabled permanently or till next reboot of the host or
service. That depend on the EnableAfterReset setting on BMC side:
CredentialBootstrapping (v1.3+)
{ object The credential bootstrapping settings for this interface.
EnableAfterReset (v1.3+) Boolean read-write (null) An
indication of whether credential bootstrapping is enabled after a reset for this interface.
Enabled (v1.3+) Boolean read-write (null) An indication of
whether credential bootstrapping is enabled for this interface.
RoleId (v1.3+) string read-write The role used for the
bootstrap account created for this interface.
}
So, if EnableAfterReset set to false, that means BMC will response
with 0x80 error and will not return any credentials after reboot. And
BIOS BMC communication will fail.
Another concern with disabling credential bootstrapping control is
that we do it on Exit Boot event before passing a control to OS.
But OS may also need to communicate to BMC through Redfish Host
Interface to post some information. And it will be blocked by our IPMI call.
We create that SMBIOS Type 42 table with Redfish Host Interface
settings which can be used by OS to communicate with BMC. But without
the credentials it will not be possible.

Another question is AuthMethod parameter you initialize in this library:
*AuthMethod = AuthMethodHttpBasic;
According to Redfish HI specification 3 methods may be used - No Auth,
Basic Auth and Session Auth.
Basic Auth and Session Auth methods are required the credentials to be
used by BIOS. And both of them should be supported by BMC.
And your high level function RedfishCreateLibredfishService also
supports of creation Basic or Session Auth service.
I'm not sure why low level library which is created to get credentials
from BMC should decide what Authentication method should be used?
Should it be configured with some PCD? Maybe user may select in Setup
what method should be used? Or it could be build time configuration?

Thank you,
Igor

-----Original Message-----
From: Nickle Wang <nicklew@...>
Sent: Tuesday, October 25, 2022 4:24 AM
To: devel@edk2.groups.io; abner.chang@...
Cc: Nick Ramirez <nramirez@...>; Igor Kulchytskyy
<igork@...>
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH]
RedfishPkg/RedfishPlatformCredentialLib: IPMI implementation


**CAUTION: The e-mail below is from an external source. Please
exercise caution before opening attachments, clicking links, or
following guidance.**

Thanks for your review comments, Abner! I will update new version
patch later. The CI build error will be handled together.

please add Igor as reviewer too

Sure!

+ *UserId = AllocateZeroPool (sizeof (CHAR8) * USERNAME_MAX_SIZE);
+ if

[Chang, Abner]
Allocation memory with the size (USERNAME_MAX_LENGTH + 1) for both
BootUsername and BootstrapPassword? Because the maximum number of
characters defined in the spec is USERNAME_MAX_LENGTH for the
user/password.

Yes, the additional one byte is for NULL terminator.
USERNAME_MAX_LENGTH is defined as 16 and follow host interface
specification.

Regards,
Nickle

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
Abner via groups.io
Sent: Saturday, October 22, 2022 3:01 PM
To: Nickle Wang <nicklew@...>; devel@edk2.groups.io
Cc: Nick Ramirez <nramirez@...>; Igor Kulchytskyy
<igork@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

Hi Nickle, please add Igor as reviewer too. My comments is in below,

-----Original Message-----
From: Nickle Wang <nicklew@...>
Sent: Thursday, October 20, 2022 10:55 AM
To: devel@edk2.groups.io
Cc: Chang, Abner <Abner.Chang@...>; Nick Ramirez
<nramirez@...>
Subject: [PATCH] RedfishPkg/RedfishPlatformCredentialLib: IPMI
implementation

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


This library follows Redfish Host Interface specification and use
IPMI command to get bootstrap account credential(NetFn 2Ch, Command
02h)

from BMC.

RedfishHostInterfaceDxe will use this credential for the following
communication between BIOS and BMC.

Cc: Abner Chang <abner.chang@...>
Cc: Nick Ramirez <nramirez@...>
Signed-off-by: Nickle Wang <nicklew@...>
---
.../RedfishPlatformCredentialLib.c | 273 ++++++++++++++++++
.../RedfishPlatformCredentialLib.h | 75 +++++
.../RedfishPlatformCredentialLib.inf | 37 +++

[Chang, Abner]
Could we name this library RedfishPlatformCredentialIpmi so the naming
style is consistent with RedfishPlatformCredentialNull?

3 files changed, 385 insertions(+)
create mode 100644

RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCredent
ial
Lib.

c
create mode 100644

RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCredent
ial
Lib.

h
create mode 100644
RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCrede
nt
ialLib.i
nf

diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.c
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.c
new file mode 100644
index 0000000000..23a15ab1fa
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.c
@@ -0,0 +1,273 @@
+/** @file
+*
+* Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent

[Chang, Abner]
We can have "@par Revision Reference:" in the file header to point
out the spec.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam1
2.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fnam1
&amp;data=05%7C01%7Cnicklew%40nvidia.com%7C90696ea8811e49e371a708dab8e
be2d8%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638025620543993279%
7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=CyvRgMZREOk5Yf0hU3CmH%2
B08ktxMYw%2Bixr4UVywfrAQ%3D&amp;reserved=0
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fww&a
mp;data=05%7C01%7Cigork%40ami.com%7C2b5b562c02c04c90d51208dab8b8c0fd%7
C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C638025400915295621%7CUnkno
wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Uy%2B3HS336N3rgNSESPcyOPGX3eOR
48hekdz08nLtJU4%3D&amp;reserved=0
w.dmtf.org%2Fsites%2Fdefault%2Ffiles%2Fstandards%2Fdocuments%2FDSP
0270_1.3.0.pdf&amp;data=05%7C01%7Cabner.chang%40amd.com%7C074aa
e162fba49409af408dab8297c03%7C3dd8961fe4884e608e11a82d994e183d%7C
0%7C0%7C638024786060127888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
%7C%7C%7C&amp;sdata=yY6hhKjQfVqmNuufbeDNk%2B2FKrebHyIAyS9Ya4
szE3Y%3D&amp;reserved=0

+*
+**/
+
+#include "RedfishPlatformCredentialLib.h"
+
+//
+// Global flag of controlling credential service // BOOLEAN
+mRedfishServiceStopped = FALSE;
+
+/**
+ Notify the Redfish service provide to stop provide configuration
+service to this
platform.
+
+ This function should be called when the platfrom is about to
+ leave the safe
environment.
+ It will notify the Redfish service provider to abort all logined
+ session, and prohibit further login with original auth info.
+ GetAuthInfo() will return EFI_UNSUPPORTED once this function is

returned.

+
+ @param[in] This Pointer to
EDKII_REDFISH_CREDENTIAL_PROTOCOL instance.
+ @param[in] ServiceStopType Reason of stopping Redfish service.
+
+ @retval EFI_SUCCESS Service has been stoped successfully.
+ @retval EFI_INVALID_PARAMETER This is NULL.
+ @retval Others Some error happened.
+
+**/
+EFI_STATUS
+EFIAPI
+LibStopRedfishService (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This,
+ IN EDKII_REDFISH_CREDENTIAL_STOP_SERVICE_TYPE

ServiceStopType

+ )
+{
+ EFI_STATUS Status;
+
+ if ((ServiceStopType <= ServiceStopTypeNone) || (ServiceStopType
+ >=
ServiceStopTypeMax)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Raise flag first
+ //
+ mRedfishServiceStopped = TRUE;
+
+ //
+ // Notify BMC to disable credential bootstrapping support.
+ //
+ Status = GetBootstrapAccountCredentials (TRUE, NULL, NULL); if
+ (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: fail to disable bootstrap credential:
+ %r\n",
__FUNCTION__, Status));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Notification of Exit Boot Service.
+
+ @param[in] This Pointer to EDKII_REDFISH_CREDENTIAL_PROTOCOL.
+**/
+VOID
+EFIAPI
+LibCredentialExitBootServicesNotify (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This
+ )
+{
+ //
+ // Stop the credential support when system is about to enter OS.
+ //
+ LibStopRedfishService (This, ServiceStopTypeExitBootService); }
+
+/**
+ Notification of End of DXe.
+
+ @param[in] This Pointer to EDKII_REDFISH_CREDENTIAL_PROTOCOL.
+**/
+VOID
+EFIAPI
+LibCredentialEndOfDxeNotify (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This
+ )
+{
+ //
+ // Do nothing now.
+ // We can stop credential support when system reach end-of-dxe
+for security
reason.
+ //
+}
+
+/**
+ Function to retrieve temporary use credentials for the UEFI
+redfish client

[Chang, Abner]
We miss the functionality to disable bootstrap credential service in
the function description.

+
+ @param[in] DisableBootstrapControl
+ TRUE - Tell the BMC to disable the bootstrap credential
+ service to ensure no one else gains credentials
+ FALSE Allow the bootstrap
+ credential service to continue @param[out] BootstrapUsername
+ A pointer to a UTF-8 encoded
+ string for the credential
username
+ When DisableBootstrapControl
+ is TRUE, this pointer can be NULL
+
+ @param[out] BootstrapPassword
+ A pointer to a UTF-8 encoded
+ string for the credential
password
+ When DisableBootstrapControl
+ is TRUE, this pointer can be NULL
+
+ @retval EFI_SUCCESS Credentials were successfully fetched and
returned
+ @retval EFI_INVALID_PARAMETER BootstrapUsername or
BootstrapPassword is NULL when DisableBootstrapControl
+ is set to FALSE
+ @retval EFI_DEVICE_ERROR An IPMI failure occurred

[Chang, Abner]
The return status should also include the status of disabling
bootstrap credential.

+**/
+EFI_STATUS
+GetBootstrapAccountCredentials (
+ IN BOOLEAN DisableBootstrapControl,
+ IN OUT CHAR8 *BootstrapUsername, OPTIONAL
+ IN OUT CHAR8 *BootstrapPassword OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ IPMI_BOOTSTRAP_CREDENTIALS_COMMAND_DATA CommandData;
+ IPMI_BOOTSTRAP_CREDENTIALS_RESULT_RESPONSE ResponseData;
+ UINT32 ResponseSize;
+
+ if (!PcdGetBool (PcdIpmiFeatureEnable)) {
+ DEBUG ((DEBUG_ERROR, "%a: IPMI is not enabled! Unable to fetch
+ Redfish
credentials\n", __FUNCTION__));
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // NULL buffer check
+ //
+ if (!DisableBootstrapControl && ((BootstrapUsername == NULL) ||
(BootstrapPassword == NULL))) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: Disable bootstrap control: 0x%x\n",
+ __FUNCTION__, DisableBootstrapControl));
+
+ //
+ // IPMI callout to NetFn 2C, command 02
+ // Request data:
+ // Byte 1: REDFISH_IPMI_GROUP_EXTENSION
+ // Byte 2: DisableBootstrapControl
+ //
+ CommandData.GroupExtensionId =

REDFISH_IPMI_GROUP_EXTENSION;

+ CommandData.DisableBootstrapControl = (DisableBootstrapControl ?
+ REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_DISABLE :
+ REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_ENABLE);
+
+ ResponseSize = sizeof (ResponseData);
+
+ //
+ // Response data:
+ // Byte 1 : Completion code
+ // Byte 2 : REDFISH_IPMI_GROUP_EXTENSION
+ // Byte 3-18 : Username
+ // Byte 19-34: Password
+ //
+ Status = IpmiSubmitCommand (
+ IPMI_NETFN_GROUP_EXT,
+ REDFISH_IPMI_GET_BOOTSTRAP_CREDENTIALS_CMD,
+ (UINT8 *)&CommandData,
+ sizeof (CommandData),
+ (UINT8 *)&ResponseData,
+ &ResponseSize
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: IPMI transaction failure.
+ Returning\n",
__FUNCTION__));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ } else {
+ if (ResponseData.CompletionCode != IPMI_COMP_CODE_NORMAL) {
+ if (ResponseData.CompletionCode ==
REDFISH_IPMI_COMP_CODE_BOOTSTRAP_CREDENTIAL_DISABLED) {
+ DEBUG ((DEBUG_ERROR, "%a: bootstrap credential support was
disabled\n", __FUNCTION__));
+ return EFI_ACCESS_DENIED;
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a: Completion code = 0x%x.
+ Returning\n",
__FUNCTION__, ResponseData.CompletionCode));
+ return EFI_PROTOCOL_ERROR;
+ } else if (ResponseData.GroupExtensionId !=
REDFISH_IPMI_GROUP_EXTENSION) {
+ DEBUG ((DEBUG_ERROR, "%a: Group Extension Response = 0x%x.
Returning\n", __FUNCTION__, ResponseData.GroupExtensionId));
+ return EFI_DEVICE_ERROR;
+ } else {
+ if (BootstrapUsername != NULL) {
+ CopyMem (BootstrapUsername, ResponseData.Username,
USERNAME_MAX_LENGTH);
+ //
+ // Manually append null-terminator in case 16 characters
+ username
returned.
+ //
+ BootstrapUsername[USERNAME_MAX_LENGTH] = '\0';
+ }
+
+ if (BootstrapPassword != NULL) {
+ CopyMem (BootstrapPassword, ResponseData.Password,
PASSWORD_MAX_LENGTH);
+ //
+ // Manually append null-terminator in case 16 characters
+ password
returned.
+ //
+ BootstrapPassword[PASSWORD_MAX_LENGTH] = '\0';
+ }
+ }
+ }
+
+ return Status;
+}
+
+/**
+ Retrieve platform's Redfish authentication information.
+
+ This functions returns the Redfish authentication method together
+ with the user Id and password.
+ - For AuthMethodNone, the UserId and Password could be used for
+ HTTP
header authentication
+ as defined by RFC7235.
+ - For AuthMethodRedfishSession, the UserId and Password could be
+ used for
Redfish
+ session login as defined by Redfish API specification (DSP0266).
+
+ Callers are responsible for and freeing the returned string storage.
+
+ @param[in] This Pointer to
EDKII_REDFISH_CREDENTIAL_PROTOCOL instance.
+ @param[out] AuthMethod Type of Redfish authentication method.
+ @param[out] UserId The pointer to store the returned UserId

string.

+ @param[out] Password The pointer to store the returned

Password

string.
+
+ @retval EFI_SUCCESS Get the authentication information

successfully.

+ @retval EFI_ACCESS_DENIED SecureBoot is disabled after EndOfDxe.
+ @retval EFI_INVALID_PARAMETER This or AuthMethod or UserId or
Password is NULL.
+ @retval EFI_OUT_OF_RESOURCES There are not enough memory

resources.

+ @retval EFI_UNSUPPORTED Unsupported authentication method is
found.
+
+**/
+EFI_STATUS
+EFIAPI
+LibCredentialGetAuthInfo (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This,
+ OUT EDKII_REDFISH_AUTH_METHOD *AuthMethod,
+ OUT CHAR8 **UserId,
+ OUT CHAR8 **Password
+ )
+{
+ EFI_STATUS Status;
+
+ if ((AuthMethod == NULL) || (UserId == NULL) || (Password == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *UserId = NULL;
+ *Password = NULL;
+
+ if (mRedfishServiceStopped) {
+ DEBUG ((DEBUG_ERROR, "%a: credential service is stopped due to
+ security
reason\n", __FUNCTION__));
+ return EFI_ACCESS_DENIED;
+ }
+
+ *AuthMethod = AuthMethodHttpBasic;
+
+ *UserId = AllocateZeroPool (sizeof (CHAR8) * USERNAME_MAX_SIZE);
+ if

[Chang, Abner]
Allocation memory with the size (USERNAME_MAX_LENGTH + 1) for both
BootUsername and BootstrapPassword? Because the maximum number of
characters defined in the spec is USERNAME_MAX_LENGTH for the
user/password.

+ (*UserId == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ *Password = AllocateZeroPool (sizeof (CHAR8) *
+ PASSWORD_MAX_SIZE); if (*Password == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = GetBootstrapAccountCredentials (FALSE, *UserId,
+ *Password); if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: fail to get bootstrap credential:
+ %r\n",
__FUNCTION__, Status));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.h
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.h
new file mode 100644
index 0000000000..5b448e01be
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.h
@@ -0,0 +1,75 @@
+/** @file
+*
+* Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+#include <Uefi.h>
+#include <IndustryStandard/Ipmi.h>
+#include <Protocol/EdkIIRedfishCredential.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IpmiBaseLib.h>
+#include <Library/MemoryAllocationLib.h> #include
+<Library/RedfishCredentialLib.h>
+
+#define REDFISH_IPMI_GROUP_EXTENSION 0x52
+#define REDFISH_IPMI_GET_BOOTSTRAP_CREDENTIALS_CMD 0x02
+#define REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_ENABLE 0xA5
+#define REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_DISABLE 0x00
+#define

REDFISH_IPMI_COMP_CODE_BOOTSTRAP_CREDENTIAL_DISABLED

0x80
+
+//
+// Per Redfish Host Interface Specification 1.3, The maximum lenght
+of // username and password is 16 characters long.
+//
+#define USERNAME_MAX_LENGTH 16
+#define PASSWORD_MAX_LENGTH 16
+#define USERNAME_MAX_SIZE (USERNAME_MAX_LENGTH + 1) //

NULL

terminator
+#define PASSWORD_MAX_SIZE (PASSWORD_MAX_LENGTH + 1) //

NULL

terminator
+
+#pragma pack(1)
+///
+/// The definition of IPMI command to get bootstrap account
+credentials /// typedef struct {
+ UINT8 GroupExtensionId;
+ UINT8 DisableBootstrapControl;
+} IPMI_BOOTSTRAP_CREDENTIALS_COMMAND_DATA;
+
+///
+/// The response data of getting bootstrap credential /// typedef
+struct {
+ UINT8 CompletionCode;
+ UINT8 GroupExtensionId;
+ CHAR8 Username[USERNAME_MAX_LENGTH];
+ CHAR8 Password[PASSWORD_MAX_LENGTH];
+} IPMI_BOOTSTRAP_CREDENTIALS_RESULT_RESPONSE;
+
+#pragma pack()
+
+/**
+ Function to retrieve temporary use credentials for the UEFI
+redfish client

[Chang, Abner]
We miss the functionality to disable bootstrap credential service in
the function description.

+
+ @param[in] DisableBootstrapControl
+ TRUE - Tell the BMC to disable the bootstrap credential
+ service to ensure no one else gains credentials
+ FALSE Allow the bootstrap
+ credential service to continue @param[out] BootstrapUsername
+ A pointer to a UTF-8 encoded
+ string for the credential username
+
+ @param[out] BootstrapPassword
+ A pointer to a UTF-8 encoded
+ string for the credential password
+
+ @retval EFI_SUCCESS Credentials were successfully fetched and
returned

[Chang, Abner]
Or the bootstrap credential service is disabled successfully, right?

+ @retval EFI_DEVICE_ERROR An IPMI failure occurred
+**/
+EFI_STATUS
+GetBootstrapAccountCredentials (
+ IN BOOLEAN DisableBootstrapControl,
+ IN OUT CHAR8 *BootstrapUsername,
+ IN OUT CHAR8 *BootstrapPassword
+ );
diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.inf
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.inf
new file mode 100644
index 0000000000..a990d28363
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.inf
@@ -0,0 +1,37 @@
+## @file
+#
+# Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x0001000b
+ BASE_NAME = RedfishPlatformCredentialLib
+ FILE_GUID = 9C45D622-4C66-417F-814C-F76246D97233
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = RedfishPlatformCredentialLib
+
+[Sources]
+ RedfishPlatformCredentialLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ RedfishPkg/RedfishPkg.dec
+ IpmiFeaturePkg/IpmiFeaturePkg.dec

[Chang, Abner]
Could you please add a comment to the reference of IpmiFeaturePkg? We
have to give customers a notice that the dependence of "edk2-
platforms/Features/Intel/OutOfBandManagement/". They have to add the
path to PACKAGES_PATH. You also have to skip this dependence in the
RedfishPkg.yaml to avoid the CI error.

Another thing is I propose to move out IpmiFeaturePkg from edk2-
platforms/Features/Intel/OutOfBandManagement to edk2-
platforms/Features/ManageabilityPkg that also provides the
implementation of PLDM/MCTP/IPMI/KCS. I had an initial talk with
IpmiFeaturePkg owner and get the positive response on this proposal. I
will kick off the discussion on the dev mailing list. That is to say
this module may need a little bit change later, however that is good
to me having this implementation now.
Thanks
Abner

+
+[LibraryClasses]
+ UefiLib
+ DebugLib
+ IpmiBaseLib
+ MemoryAllocationLib
+ BaseMemoryLib
+
+[Pcd]
+ gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
+
+[Depex]
+ TRUE
--
2.17.1

-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is
intended to be read only by the individual or entity to whom it is
addressed or by their designee. If the reader of this message is not
the intended recipient, you are on notice that any distribution of
this message, in any form, is strictly prohibited. Please promptly
notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.










-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is
intended to be read only by the individual or entity to whom it is
addressed or by their designee. If the reader of this message is not
the intended recipient, you are on notice that any distribution of
this message, in any form, is strictly prohibited. Please promptly
notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.

Hi Pavamana,

I found there are few patch format issues.  Please use edk2\BaseTools\Scripts\PatchCheck.py to check the patch and fix them. After that, I’ll merge it.

Thanks,

Bob

Michael:

 Thanks for your sharing.

Pavamana:

 FIT spec 1.4 has been published. So, this change can be merged now.

Thanks

Liming

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor
Kulchytskyy via groups.io
Sent: Thursday, October 27, 2022 10:42 PM
To: devel@edk2.groups.io; nicklew@...; Chang, Abner
<Abner.Chang@...>
Cc: Nick Ramirez <nramirez@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


Hi Nickle,
Pleased, see my comments on your questions below.

Another point I missed in my previous mail.
You have that function in the library to get credentials and it is
used by RedfishCredentialsDxe driver to create the protocol which in
its turn will be used by other Redfish modules.
We do not know how many modules and how many times will call this
function during boot. Right?
And on each call of this function you call IPMI command. That command
will create new Redfish account on BMC side according to Redfish HI specification:

" If the Get Bootstrap Account Credentials command has been issued and
responds with the completion code 00h, a bootstrap account shall be
added to the manager's account collection and enabled. If the Get
Bootstrap Account Credentials command is sent subsequent times and
responds with the completion code 00h, a new account shall be created
based on the newly generated credentials. Any existing bootstrap accounts shall remain active."

As I know BMC may have some restrictions on the number of Redfish
accounts they can support.
And because of that BOS may hit this limit. Which is not good.
On the other hand I'm not sure we need to have different credentials
for different modules? All those modules are part of FW. And all of
them will be associated with the same RoleID (FW role) on BMC side.
So, all of them may use the same credentials.
Could we cash the credentials on first call of that

RedfishCredentialGetAuthInfo and then use those cashed credentials on
subsequential calls?
It will also may save a boot time, since there is no need to send IPMI
command.

Thank you,
Igor

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle
Wang via groups.io
Sent: Thursday, October 27, 2022 9:26 AM
To: devel@edk2.groups.io; Igor Kulchytskyy <igork@...>;
abner.chang@...
Cc: Nick Ramirez <nramirez@...>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH]
RedfishPkg/RedfishPlatformCredentialLib: IPMI implementation


**CAUTION: The e-mail below is from an external source. Please
exercise caution before opening attachments, clicking links, or
following guidance.**

Hi Igor,

Thank you for your help to review my changes.

And it will be blocked by our IPMI call.

I see your point. So, BIOS should never be the person to shutdown
credential service because BIOS always get executed prior to OS, right?

Igor: Yes, my point is that we should not shutdown credential service
from BIOS. Even if OS sends that IPMI command, new account will be
created and BIOS credentials will not be compromised.

Should it be configured with some PCD? Maybe user may select in
Setup

what method should be used? Or it could be build time configuration?

I have below assumption while I implemented the library. I admit this
is not always true.

No Auth: I think this is rare case for Redfish service which gives
anonymous privilege to change BIOS settings.
Basic Auth: this is the authentication method which uses username and
password to build base64 encoded string.
Session Auth: I assume that client must have a session token first and
then use this authentication method. Can we use username and password
to generate session token on our own? If my memory serves me
correctly, client has to do a login with username and password first
and then client can receive session token from server.

Igor: BIOS will use the credentials to create session. It should send
POST request to the session URI with user name and password to create session.
If a session created successfully then on response BMC returns header
"X- Auth-Token" which then used for the subsequential calls.

If we really like to know what authentication method that Redfish
service used, we can issue a HTTP query to "/redfish/v1/Systems" with "No Auth".
Then we can know what authentication method is required by reading the
"WWW-Authenticate " filed in returned HTTP header.

Igor: As my understanding, even if you include authentication header
(Base64 encoded) in the request to BMC and BMC has NoAuth
configuration, then that authentication header would be just ignored by BMC.

Thanks,
Nickle

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Igor
Kulchytskyy via groups.io
Sent: Wednesday, October 26, 2022 11:26 PM
To: Nickle Wang <nicklew@...>; devel@edk2.groups.io;
abner.chang@...
Cc: Nick Ramirez <nramirez@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

External email: Use caution opening links or attachments


Hi Nickle,
I would like to discuss that DisableBootstrapControl flag and how it
is used in our implementation.
According to Redfish HI specification we can use this flag to disable
credential bootstrapping control.
It can be disabled permanently or till next reboot of the host or
service. That depend on the EnableAfterReset setting on BMC side:
CredentialBootstrapping (v1.3+)
{ object The credential bootstrapping settings for this interface.
EnableAfterReset (v1.3+) Boolean read-write (null) An
indication of whether credential bootstrapping is enabled after a reset for this interface.
Enabled (v1.3+) Boolean read-write (null) An indication of
whether credential bootstrapping is enabled for this interface.
RoleId (v1.3+) string read-write The role used for the
bootstrap account created for this interface.
}
So, if EnableAfterReset set to false, that means BMC will response
with 0x80 error and will not return any credentials after reboot. And
BIOS BMC communication will fail.
Another concern with disabling credential bootstrapping control is
that we do it on Exit Boot event before passing a control to OS.
But OS may also need to communicate to BMC through Redfish Host
Interface to post some information. And it will be blocked by our IPMI call.
We create that SMBIOS Type 42 table with Redfish Host Interface
settings which can be used by OS to communicate with BMC. But without
the credentials it will not be possible.

Another question is AuthMethod parameter you initialize in this library:
*AuthMethod = AuthMethodHttpBasic;
According to Redfish HI specification 3 methods may be used - No Auth,
Basic Auth and Session Auth.
Basic Auth and Session Auth methods are required the credentials to be
used by BIOS. And both of them should be supported by BMC.
And your high level function RedfishCreateLibredfishService also
supports of creation Basic or Session Auth service.
I'm not sure why low level library which is created to get credentials
from BMC should decide what Authentication method should be used?
Should it be configured with some PCD? Maybe user may select in Setup
what method should be used? Or it could be build time configuration?

Thank you,
Igor

-----Original Message-----
From: Nickle Wang <nicklew@...>
Sent: Tuesday, October 25, 2022 4:24 AM
To: devel@edk2.groups.io; abner.chang@...
Cc: Nick Ramirez <nramirez@...>; Igor Kulchytskyy
<igork@...>
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH]
RedfishPkg/RedfishPlatformCredentialLib: IPMI implementation


**CAUTION: The e-mail below is from an external source. Please
exercise caution before opening attachments, clicking links, or
following guidance.**

Thanks for your review comments, Abner! I will update new version
patch later. The CI build error will be handled together.

please add Igor as reviewer too

Sure!

+ *UserId = AllocateZeroPool (sizeof (CHAR8) * USERNAME_MAX_SIZE);
+ if

[Chang, Abner]
Allocation memory with the size (USERNAME_MAX_LENGTH + 1) for both
BootUsername and BootstrapPassword? Because the maximum number of
characters defined in the spec is USERNAME_MAX_LENGTH for the
user/password.

Yes, the additional one byte is for NULL terminator.
USERNAME_MAX_LENGTH is defined as 16 and follow host interface
specification.

Regards,
Nickle

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
Abner via groups.io
Sent: Saturday, October 22, 2022 3:01 PM
To: Nickle Wang <nicklew@...>; devel@edk2.groups.io
Cc: Nick Ramirez <nramirez@...>; Igor Kulchytskyy
<igork@...>
Subject: Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformCredentialLib:
IPMI implementation

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

Hi Nickle, please add Igor as reviewer too. My comments is in below,

-----Original Message-----
From: Nickle Wang <nicklew@...>
Sent: Thursday, October 20, 2022 10:55 AM
To: devel@edk2.groups.io
Cc: Chang, Abner <Abner.Chang@...>; Nick Ramirez
<nramirez@...>
Subject: [PATCH] RedfishPkg/RedfishPlatformCredentialLib: IPMI
implementation

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


This library follows Redfish Host Interface specification and use
IPMI command to get bootstrap account credential(NetFn 2Ch, Command
02h)

from BMC.

RedfishHostInterfaceDxe will use this credential for the following
communication between BIOS and BMC.

Cc: Abner Chang <abner.chang@...>
Cc: Nick Ramirez <nramirez@...>
Signed-off-by: Nickle Wang <nicklew@...>
---
.../RedfishPlatformCredentialLib.c | 273 ++++++++++++++++++
.../RedfishPlatformCredentialLib.h | 75 +++++
.../RedfishPlatformCredentialLib.inf | 37 +++

[Chang, Abner]
Could we name this library RedfishPlatformCredentialIpmi so the naming
style is consistent with RedfishPlatformCredentialNull?

3 files changed, 385 insertions(+)
create mode 100644

RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCredent
ial
Lib.

c
create mode 100644

RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCredent
ial
Lib.

h
create mode 100644
RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCrede
nt
ialLib.i
nf

diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.c
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.c
new file mode 100644
index 0000000000..23a15ab1fa
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.c
@@ -0,0 +1,273 @@
+/** @file
+*
+* Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent

[Chang, Abner]
We can have "@par Revision Reference:" in the file header to point
out the spec.
https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam1
1.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fww&a
mp;data=05%7C01%7Cigork%40ami.com%7C2b5b562c02c04c90d51208dab8b8c0fd%7
C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C638025400915295621%7CUnkno
wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Uy%2B3HS336N3rgNSESPcyOPGX3eOR
48hekdz08nLtJU4%3D&amp;reserved=0
w.dmtf.org%2Fsites%2Fdefault%2Ffiles%2Fstandards%2Fdocuments%2FDSP
0270_1.3.0.pdf&amp;data=05%7C01%7Cabner.chang%40amd.com%7C074aa
e162fba49409af408dab8297c03%7C3dd8961fe4884e608e11a82d994e183d%7C
0%7C0%7C638024786060127888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
%7C%7C%7C&amp;sdata=yY6hhKjQfVqmNuufbeDNk%2B2FKrebHyIAyS9Ya4
szE3Y%3D&amp;reserved=0

+*
+**/
+
+#include "RedfishPlatformCredentialLib.h"
+
+//
+// Global flag of controlling credential service // BOOLEAN
+mRedfishServiceStopped = FALSE;
+
+/**
+ Notify the Redfish service provide to stop provide configuration
+service to this
platform.
+
+ This function should be called when the platfrom is about to
+ leave the safe
environment.
+ It will notify the Redfish service provider to abort all logined
+ session, and prohibit further login with original auth info.
+ GetAuthInfo() will return EFI_UNSUPPORTED once this function is

returned.

+
+ @param[in] This Pointer to
EDKII_REDFISH_CREDENTIAL_PROTOCOL instance.
+ @param[in] ServiceStopType Reason of stopping Redfish service.
+
+ @retval EFI_SUCCESS Service has been stoped successfully.
+ @retval EFI_INVALID_PARAMETER This is NULL.
+ @retval Others Some error happened.
+
+**/
+EFI_STATUS
+EFIAPI
+LibStopRedfishService (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This,
+ IN EDKII_REDFISH_CREDENTIAL_STOP_SERVICE_TYPE

ServiceStopType

+ )
+{
+ EFI_STATUS Status;
+
+ if ((ServiceStopType <= ServiceStopTypeNone) || (ServiceStopType
+ >=
ServiceStopTypeMax)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Raise flag first
+ //
+ mRedfishServiceStopped = TRUE;
+
+ //
+ // Notify BMC to disable credential bootstrapping support.
+ //
+ Status = GetBootstrapAccountCredentials (TRUE, NULL, NULL); if
+ (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: fail to disable bootstrap credential:
+ %r\n",
__FUNCTION__, Status));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Notification of Exit Boot Service.
+
+ @param[in] This Pointer to EDKII_REDFISH_CREDENTIAL_PROTOCOL.
+**/
+VOID
+EFIAPI
+LibCredentialExitBootServicesNotify (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This
+ )
+{
+ //
+ // Stop the credential support when system is about to enter OS.
+ //
+ LibStopRedfishService (This, ServiceStopTypeExitBootService); }
+
+/**
+ Notification of End of DXe.
+
+ @param[in] This Pointer to EDKII_REDFISH_CREDENTIAL_PROTOCOL.
+**/
+VOID
+EFIAPI
+LibCredentialEndOfDxeNotify (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This
+ )
+{
+ //
+ // Do nothing now.
+ // We can stop credential support when system reach end-of-dxe
+for security
reason.
+ //
+}
+
+/**
+ Function to retrieve temporary use credentials for the UEFI
+redfish client

[Chang, Abner]
We miss the functionality to disable bootstrap credential service in
the function description.

+
+ @param[in] DisableBootstrapControl
+ TRUE - Tell the BMC to disable the bootstrap credential
+ service to ensure no one else gains credentials
+ FALSE Allow the bootstrap
+ credential service to continue @param[out] BootstrapUsername
+ A pointer to a UTF-8 encoded
+ string for the credential
username
+ When DisableBootstrapControl
+ is TRUE, this pointer can be NULL
+
+ @param[out] BootstrapPassword
+ A pointer to a UTF-8 encoded
+ string for the credential
password
+ When DisableBootstrapControl
+ is TRUE, this pointer can be NULL
+
+ @retval EFI_SUCCESS Credentials were successfully fetched and
returned
+ @retval EFI_INVALID_PARAMETER BootstrapUsername or
BootstrapPassword is NULL when DisableBootstrapControl
+ is set to FALSE
+ @retval EFI_DEVICE_ERROR An IPMI failure occurred

[Chang, Abner]
The return status should also include the status of disabling
bootstrap credential.

+**/
+EFI_STATUS
+GetBootstrapAccountCredentials (
+ IN BOOLEAN DisableBootstrapControl,
+ IN OUT CHAR8 *BootstrapUsername, OPTIONAL
+ IN OUT CHAR8 *BootstrapPassword OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ IPMI_BOOTSTRAP_CREDENTIALS_COMMAND_DATA CommandData;
+ IPMI_BOOTSTRAP_CREDENTIALS_RESULT_RESPONSE ResponseData;
+ UINT32 ResponseSize;
+
+ if (!PcdGetBool (PcdIpmiFeatureEnable)) {
+ DEBUG ((DEBUG_ERROR, "%a: IPMI is not enabled! Unable to fetch
+ Redfish
credentials\n", __FUNCTION__));
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // NULL buffer check
+ //
+ if (!DisableBootstrapControl && ((BootstrapUsername == NULL) ||
(BootstrapPassword == NULL))) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: Disable bootstrap control: 0x%x\n",
+ __FUNCTION__, DisableBootstrapControl));
+
+ //
+ // IPMI callout to NetFn 2C, command 02
+ // Request data:
+ // Byte 1: REDFISH_IPMI_GROUP_EXTENSION
+ // Byte 2: DisableBootstrapControl
+ //
+ CommandData.GroupExtensionId =

REDFISH_IPMI_GROUP_EXTENSION;

+ CommandData.DisableBootstrapControl = (DisableBootstrapControl ?
+ REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_DISABLE :
+ REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_ENABLE);
+
+ ResponseSize = sizeof (ResponseData);
+
+ //
+ // Response data:
+ // Byte 1 : Completion code
+ // Byte 2 : REDFISH_IPMI_GROUP_EXTENSION
+ // Byte 3-18 : Username
+ // Byte 19-34: Password
+ //
+ Status = IpmiSubmitCommand (
+ IPMI_NETFN_GROUP_EXT,
+ REDFISH_IPMI_GET_BOOTSTRAP_CREDENTIALS_CMD,
+ (UINT8 *)&CommandData,
+ sizeof (CommandData),
+ (UINT8 *)&ResponseData,
+ &ResponseSize
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: IPMI transaction failure.
+ Returning\n",
__FUNCTION__));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ } else {
+ if (ResponseData.CompletionCode != IPMI_COMP_CODE_NORMAL) {
+ if (ResponseData.CompletionCode ==
REDFISH_IPMI_COMP_CODE_BOOTSTRAP_CREDENTIAL_DISABLED) {
+ DEBUG ((DEBUG_ERROR, "%a: bootstrap credential support was
disabled\n", __FUNCTION__));
+ return EFI_ACCESS_DENIED;
+ }
+
+ DEBUG ((DEBUG_ERROR, "%a: Completion code = 0x%x.
+ Returning\n",
__FUNCTION__, ResponseData.CompletionCode));
+ return EFI_PROTOCOL_ERROR;
+ } else if (ResponseData.GroupExtensionId !=
REDFISH_IPMI_GROUP_EXTENSION) {
+ DEBUG ((DEBUG_ERROR, "%a: Group Extension Response = 0x%x.
Returning\n", __FUNCTION__, ResponseData.GroupExtensionId));
+ return EFI_DEVICE_ERROR;
+ } else {
+ if (BootstrapUsername != NULL) {
+ CopyMem (BootstrapUsername, ResponseData.Username,
USERNAME_MAX_LENGTH);
+ //
+ // Manually append null-terminator in case 16 characters
+ username
returned.
+ //
+ BootstrapUsername[USERNAME_MAX_LENGTH] = '\0';
+ }
+
+ if (BootstrapPassword != NULL) {
+ CopyMem (BootstrapPassword, ResponseData.Password,
PASSWORD_MAX_LENGTH);
+ //
+ // Manually append null-terminator in case 16 characters
+ password
returned.
+ //
+ BootstrapPassword[PASSWORD_MAX_LENGTH] = '\0';
+ }
+ }
+ }
+
+ return Status;
+}
+
+/**
+ Retrieve platform's Redfish authentication information.
+
+ This functions returns the Redfish authentication method together
+ with the user Id and password.
+ - For AuthMethodNone, the UserId and Password could be used for
+ HTTP
header authentication
+ as defined by RFC7235.
+ - For AuthMethodRedfishSession, the UserId and Password could be
+ used for
Redfish
+ session login as defined by Redfish API specification (DSP0266).
+
+ Callers are responsible for and freeing the returned string storage.
+
+ @param[in] This Pointer to
EDKII_REDFISH_CREDENTIAL_PROTOCOL instance.
+ @param[out] AuthMethod Type of Redfish authentication method.
+ @param[out] UserId The pointer to store the returned UserId

string.

+ @param[out] Password The pointer to store the returned

Password

string.
+
+ @retval EFI_SUCCESS Get the authentication information

successfully.

+ @retval EFI_ACCESS_DENIED SecureBoot is disabled after EndOfDxe.
+ @retval EFI_INVALID_PARAMETER This or AuthMethod or UserId or
Password is NULL.
+ @retval EFI_OUT_OF_RESOURCES There are not enough memory

resources.

+ @retval EFI_UNSUPPORTED Unsupported authentication method is
found.
+
+**/
+EFI_STATUS
+EFIAPI
+LibCredentialGetAuthInfo (
+ IN EDKII_REDFISH_CREDENTIAL_PROTOCOL *This,
+ OUT EDKII_REDFISH_AUTH_METHOD *AuthMethod,
+ OUT CHAR8 **UserId,
+ OUT CHAR8 **Password
+ )
+{
+ EFI_STATUS Status;
+
+ if ((AuthMethod == NULL) || (UserId == NULL) || (Password == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *UserId = NULL;
+ *Password = NULL;
+
+ if (mRedfishServiceStopped) {
+ DEBUG ((DEBUG_ERROR, "%a: credential service is stopped due to
+ security
reason\n", __FUNCTION__));
+ return EFI_ACCESS_DENIED;
+ }
+
+ *AuthMethod = AuthMethodHttpBasic;
+
+ *UserId = AllocateZeroPool (sizeof (CHAR8) * USERNAME_MAX_SIZE);
+ if

[Chang, Abner]
Allocation memory with the size (USERNAME_MAX_LENGTH + 1) for both
BootUsername and BootstrapPassword? Because the maximum number of
characters defined in the spec is USERNAME_MAX_LENGTH for the
user/password.

+ (*UserId == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ *Password = AllocateZeroPool (sizeof (CHAR8) *
+ PASSWORD_MAX_SIZE); if (*Password == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = GetBootstrapAccountCredentials (FALSE, *UserId,
+ *Password); if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: fail to get bootstrap credential:
+ %r\n",
__FUNCTION__, Status));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.h
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.h
new file mode 100644
index 0000000000..5b448e01be
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.h
@@ -0,0 +1,75 @@
+/** @file
+*
+* Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+#include <Uefi.h>
+#include <IndustryStandard/Ipmi.h>
+#include <Protocol/EdkIIRedfishCredential.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IpmiBaseLib.h>
+#include <Library/MemoryAllocationLib.h> #include
+<Library/RedfishCredentialLib.h>
+
+#define REDFISH_IPMI_GROUP_EXTENSION 0x52
+#define REDFISH_IPMI_GET_BOOTSTRAP_CREDENTIALS_CMD 0x02
+#define REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_ENABLE 0xA5
+#define REDFISH_IPMI_BOOTSTRAP_CREDENTIAL_DISABLE 0x00
+#define

REDFISH_IPMI_COMP_CODE_BOOTSTRAP_CREDENTIAL_DISABLED

0x80
+
+//
+// Per Redfish Host Interface Specification 1.3, The maximum lenght
+of // username and password is 16 characters long.
+//
+#define USERNAME_MAX_LENGTH 16
+#define PASSWORD_MAX_LENGTH 16
+#define USERNAME_MAX_SIZE (USERNAME_MAX_LENGTH + 1) //

NULL

terminator
+#define PASSWORD_MAX_SIZE (PASSWORD_MAX_LENGTH + 1) //

NULL

terminator
+
+#pragma pack(1)
+///
+/// The definition of IPMI command to get bootstrap account
+credentials /// typedef struct {
+ UINT8 GroupExtensionId;
+ UINT8 DisableBootstrapControl;
+} IPMI_BOOTSTRAP_CREDENTIALS_COMMAND_DATA;
+
+///
+/// The response data of getting bootstrap credential /// typedef
+struct {
+ UINT8 CompletionCode;
+ UINT8 GroupExtensionId;
+ CHAR8 Username[USERNAME_MAX_LENGTH];
+ CHAR8 Password[PASSWORD_MAX_LENGTH];
+} IPMI_BOOTSTRAP_CREDENTIALS_RESULT_RESPONSE;
+
+#pragma pack()
+
+/**
+ Function to retrieve temporary use credentials for the UEFI
+redfish client

[Chang, Abner]
We miss the functionality to disable bootstrap credential service in
the function description.

+
+ @param[in] DisableBootstrapControl
+ TRUE - Tell the BMC to disable the bootstrap credential
+ service to ensure no one else gains credentials
+ FALSE Allow the bootstrap
+ credential service to continue @param[out] BootstrapUsername
+ A pointer to a UTF-8 encoded
+ string for the credential username
+
+ @param[out] BootstrapPassword
+ A pointer to a UTF-8 encoded
+ string for the credential password
+
+ @retval EFI_SUCCESS Credentials were successfully fetched and
returned

[Chang, Abner]
Or the bootstrap credential service is disabled successfully, right?

+ @retval EFI_DEVICE_ERROR An IPMI failure occurred
+**/
+EFI_STATUS
+GetBootstrapAccountCredentials (
+ IN BOOLEAN DisableBootstrapControl,
+ IN OUT CHAR8 *BootstrapUsername,
+ IN OUT CHAR8 *BootstrapPassword
+ );
diff --git
a/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.inf
b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatformCre
de
ntialLi
b.inf
new file mode 100644
index 0000000000..a990d28363
--- /dev/null
+++ b/RedfishPkg/Library/RedfishPlatformCredentialLib/RedfishPlatfor
+++ mC
+++ re
+++ dentialLib.inf
@@ -0,0 +1,37 @@
+## @file
+#
+# Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights

reserved.

+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent # ##
+
+[Defines]
+ INF_VERSION = 0x0001000b
+ BASE_NAME = RedfishPlatformCredentialLib
+ FILE_GUID = 9C45D622-4C66-417F-814C-F76246D97233
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = RedfishPlatformCredentialLib
+
+[Sources]
+ RedfishPlatformCredentialLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ RedfishPkg/RedfishPkg.dec
+ IpmiFeaturePkg/IpmiFeaturePkg.dec

[Chang, Abner]
Could you please add a comment to the reference of IpmiFeaturePkg? We
have to give customers a notice that the dependence of "edk2-
platforms/Features/Intel/OutOfBandManagement/". They have to add the
path to PACKAGES_PATH. You also have to skip this dependence in the
RedfishPkg.yaml to avoid the CI error.

Another thing is I propose to move out IpmiFeaturePkg from edk2-
platforms/Features/Intel/OutOfBandManagement to edk2-
platforms/Features/ManageabilityPkg that also provides the
implementation of PLDM/MCTP/IPMI/KCS. I had an initial talk with
IpmiFeaturePkg owner and get the positive response on this proposal. I
will kick off the discussion on the dev mailing list. That is to say
this module may need a little bit change later, however that is good
to me having this implementation now.
Thanks
Abner

+
+[LibraryClasses]
+ UefiLib
+ DebugLib
+ IpmiBaseLib
+ MemoryAllocationLib
+ BaseMemoryLib
+
+[Pcd]
+ gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
+
+[Depex]
+ TRUE
--
2.17.1

-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is
intended to be read only by the individual or entity to whom it is
addressed or by their designee. If the reader of this message is not
the intended recipient, you are on notice that any distribution of
this message, in any form, is strictly prohibited. Please promptly
notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.










-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is
intended to be read only by the individual or entity to whom it is
addressed or by their designee. If the reader of this message is not
the intended recipient, you are on notice that any distribution of
this message, in any form, is strictly prohibited. Please promptly
notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
Abner via groups.io
Sent: Friday, September 30, 2022 5:52 PM
To: devel@edk2.groups.io
Cc: Abdul Lateef Attar <abdattar@...>; Garrett Kirkendall
<garrett.kirkendall@...>; Paul Grimes <paul.grimes@...>; Dong,
Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Kumar, Rahul R
<rahul.r.kumar@...>
Subject: [edk2-devel] [PATCH 1/2] UefiCpuPkg/SmmCpuFeaturesLib:
Abstract arch dependent code

From: Abner Chang <abner.chang@...>

This change strips away Intel X86 implementation and put
it in the IntelSmmCpuFeatureLib

Signed-off-by: Abner Chang <abner.chang@...>
Cc: Abdul Lateef Attar <abdattar@...>
Cc: Garrett Kirkendall <garrett.kirkendall@...>
Cc: Paul Grimes <paul.grimes@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
---
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 1 +
.../SmmCpuFeaturesLibStm.inf | 1 +
.../StandaloneMmCpuFeaturesLib.inf | 1 +
.../SmmCpuFeaturesLib/CpuFeaturesLib.h | 6 +
.../IntelSmmCpuFeaturesLib.c | 403 ++++++++++++++++++
.../SmmCpuFeaturesLibCommon.c | 391 +----------------
6 files changed, 413 insertions(+), 390 deletions(-)
create mode 100644
UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 7b5cef97008..9ac7dde78f8 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -18,6 +18,7 @@

[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
SmmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmCpuFeaturesLibNoStm.c
diff --git
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 85214ee31cd..86d367e0a09 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -19,6 +19,7 @@

[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmStm.c
SmmStm.h
diff --git
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
nf
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
nf
index 3eacab48db3..61890205e18 100644
---
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
nf
+++
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.i
nf
@@ -20,6 +20,7 @@

[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
StandaloneMmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmCpuFeaturesLibNoStm.c
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index 8a1c2adc5c4..fd3e902547c 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -9,6 +9,12 @@
#ifndef CPU_FEATURES_LIB_H_
#define CPU_FEATURES_LIB_H_

+#include <Library/SmmCpuFeaturesLib.h>
+#include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+
/**
Performs library initialization.

diff --git
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
new file mode 100644
index 00000000000..cb4897b21e3
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
@@ -0,0 +1,403 @@
+/** @file
+Implementation shared across all library instances.
+
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "CpuFeaturesLib.h"
+
+#include <Library/MtrrLib.h>
+#include <Register/Intel/Cpuid.h>
+#include <Register/Intel/SmramSaveStateMap.h>
+
+//
+// Machine Specific Registers (MSRs)
+//
+#define SMM_FEATURES_LIB_IA32_MTRR_CAP 0x0FE
+#define SMM_FEATURES_LIB_IA32_FEATURE_CONTROL 0x03A
+#define SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE 0x1F2
+#define SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK 0x1F3
+#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE 0x0A0
+#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK 0x0A1
+#define EFI_MSR_SMRR_MASK 0xFFFFF000
+#define EFI_MSR_SMRR_PHYS_MASK_VALID BIT11
+#define SMM_FEATURES_LIB_SMM_FEATURE_CONTROL 0x4E0
+
+//
+// MSRs required for configuration of SMM Code Access Check
+//
+#define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D
+#define SMM_CODE_ACCESS_CHK_BIT BIT58
+
+//
+// Set default value to assume IA-32 Architectural MSRs are used
+//
+UINT32 mSmrrPhysBaseMsr =
SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
+UINT32 mSmrrPhysMaskMsr =
SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
+
+//
+// Set default value to assume MTRRs need to be configured on each SMI
+//
+BOOLEAN mNeedConfigureMtrrs = TRUE;
+
+//
+// Array for state of SMRR enable on all CPUs
+//
+BOOLEAN *mSmrrEnabled;
+
+/**
+ Performs library initialization.
+
+ This initialization function contains common functionality shared betwen all
+ library instance constructors.
+
+**/
+VOID
+CpuFeaturesLibInitialization (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ UINT32 RegEdx;
+ UINTN FamilyId;
+ UINTN ModelId;
+
+ //
+ // Retrieve CPU Family and Model
+ //
+ AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+ FamilyId = (RegEax >> 8) & 0xf;
+ ModelId = (RegEax >> 4) & 0xf;
+ if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
+ ModelId = ModelId | ((RegEax >> 12) & 0xf0);
+ }
+
+ //
+ // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
+ //
+ if ((RegEdx & BIT12) != 0) {
+ //
+ // Check MTRR_CAP MSR bit 11 for SMRR support
+ //
+ if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) !=
0) {
+ ASSERT (FeaturePcdGet (PcdSmrrEnable));
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor
Family
+ //
+ // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H,
then
+ // SMRR Physical Base and SMM Physical Mask MSRs are not available.
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) ||
(ModelId == 0x35) || (ModelId == 0x36)) {
+ ASSERT (!FeaturePcdGet (PcdSmrrEnable));
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor
Family
+ //
+ // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
+ // Processor Family MSRs
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x17) || (ModelId == 0x0f)) {
+ mSmrrPhysBaseMsr =
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
+ mSmrrPhysMaskMsr =
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 34.4.2 SMRAM Caching
+ // An IA-32 processor does not automatically write back and invalidate its
+ // caches before entering SMM or before exiting SMM. Because of this
behavior,
+ // care must be taken in the placement of the SMRAM in system memory
and in
+ // the caching of the SMRAM to prevent cache incoherence when
switching back
+ // and forth between SMM and protected mode operation.
+ //
+ // An IA-32 processor is a processor that does not support the Intel 64
+ // Architecture. Support for the Intel 64 Architecture can be detected
from
+ // CPUID(CPUID_EXTENDED_CPU_SIG).EDX[29]
+ //
+ // If an IA-32 processor is detected, then set mNeedConfigureMtrrs to
TRUE,
+ // so caches are flushed on SMI entry and SMI exit, the interrupted code
+ // MTRRs are saved/restored, and MTRRs for SMM are loaded.
+ //
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
+ AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & BIT29) != 0) {
+ mNeedConfigureMtrrs = FALSE;
+ }
+ }
+
+ //
+ // Allocate array for state of SMRR enable on all CPUs
+ //
+ mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) *
GetCpuMaxLogicalProcessorNumber ());
+ ASSERT (mSmrrEnabled != NULL);
+}
+
+/**
+ Called during the very first SMI into System Management Mode to initialize
+ CPU features, including SMBASE, for the currently executing CPU. Since
this
+ is the first SMI, the SMRAM Save State Map is at the default address of
+ SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET. The
currently executing
+ CPU is specified by CpuIndex and CpuIndex can be used to access
information
+ about the currently executing CPU in the ProcessorInfo array and the
+ HotPlugCpuData data structure.
+
+ @param[in] CpuIndex The index of the CPU to initialize. The value
+ must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] IsMonarch TRUE if the CpuIndex is the index of the CPU
that
+ was elected as monarch during System Management
+ Mode initialization.
+ FALSE if the CpuIndex is not the index of the CPU
+ that was elected as monarch during System
+ Management Mode initialization.
+ @param[in] ProcessorInfo Pointer to an array of
EFI_PROCESSOR_INFORMATION
+ structures. ProcessorInfo[CpuIndex] contains the
+ information for the currently executing CPU.
+ @param[in] CpuHotPlugData Pointer to the CPU_HOT_PLUG_DATA
structure that
+ contains the ApidId and SmBase arrays.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesInitializeProcessor (
+ IN UINTN CpuIndex,
+ IN BOOLEAN IsMonarch,
+ IN EFI_PROCESSOR_INFORMATION *ProcessorInfo,
+ IN CPU_HOT_PLUG_DATA *CpuHotPlugData
+ )
+{
+ SMRAM_SAVE_STATE_MAP *CpuState;
+ UINT64 FeatureControl;
+ UINT32 RegEax;
+ UINT32 RegEdx;
+ UINTN FamilyId;
+ UINTN ModelId;
+
+ //
+ // Configure SMBASE.
+ //
+ CpuState = (SMRAM_SAVE_STATE_MAP
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
+ CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor
Family
+ //
+ // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used,
then
+ // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A)
is set before
+ // accessing SMRR base/mask MSRs. If Lock(BIT0) of
MSR_FEATURE_CONTROL MSR(0x3A)
+ // is set, then the MSR is locked and can not be modified.
+ //
+ if ((FeaturePcdGet (PcdSmrrEnable)) && (mSmrrPhysBaseMsr ==
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {
+ FeatureControl = AsmReadMsr64
(SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
+ if ((FeatureControl & BIT3) == 0) {
+ ASSERT ((FeatureControl & BIT0) == 0);
+ if ((FeatureControl & BIT0) == 0) {
+ AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL,
FeatureControl | BIT3);
+ }
+ }
+ }
+
+ //
+ // If SMRR is supported, then program SMRR base/mask MSRs.
+ // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first
normal SMI.
+ // The code that initializes SMM environment is running in normal mode
+ // from SMRAM region. If SMRR is enabled here, then the SMRAM region
+ // is protected and the normal mode code execution will fail.
+ //
+ if (FeaturePcdGet (PcdSmrrEnable)) {
+ //
+ // SMRR size cannot be less than 4-KBytes
+ // SMRR size must be of length 2^n
+ // SMRR base alignment cannot be less than SMRR length
+ //
+ if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
+ (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData-

SmrrSize)) ||

+ ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) !=
CpuHotPlugData->SmrrBase))
+ {
+ //
+ // Print message and halt if CPU is Monarch
+ //
+ if (IsMonarch) {
+ DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet
alignment/size requirement!\n"));
+ CpuDeadLoop ();
+ }
+ } else {
+ AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase |
MTRR_CACHE_WRITE_BACK);
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1)
& EFI_MSR_SMRR_MASK));
+ mSmrrEnabled[CpuIndex] = FALSE;
+ }
+ }
+
+ //
+ // Retrieve CPU Family and Model
+ //
+ AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+ FamilyId = (RegEax >> 8) & 0xf;
+ ModelId = (RegEax >> 4) & 0xf;
+ if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
+ ModelId = ModelId | ((RegEax >> 12) & 0xf0);
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
+ // Processor Family.
+ //
+ // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
+ // Intel(R) Core(TM) Processor Family MSRs.
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
+ (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) ||
(ModelId == 0x4F) ||
+ (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) ||
(ModelId == 0x5C) ||
+ (ModelId == 0x8C))
+ {
+ //
+ // Check to see if the CPU supports the SMM Code Access Check feature
+ // Do not access this MSR unless the CPU supports the
SmmRegFeatureControl
+ //
+ if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) &
SMM_CODE_ACCESS_CHK_BIT) != 0) {
+ ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable));
+ }
+ }
+ }
+
+ //
+ // Call internal worker function that completes the CPU initialization
+ //
+ FinishSmmCpuFeaturesInitializeProcessor ();
+}
+
+/**
+ Determines if MTRR registers must be configured to set SMRAM cache-
ability
+ when executing in System Management Mode.
+
+ @retval TRUE MTRR registers must be configured to set SMRAM cache-
ability.
+ @retval FALSE MTRR registers do not need to be configured to set
SMRAM
+ cache-ability.
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesNeedConfigureMtrrs (
+ VOID
+ )
+{
+ return mNeedConfigureMtrrs;
+}
+
+/**
+ Disable SMRR register if SMRR is supported and
SmmCpuFeaturesNeedConfigureMtrrs()
+ returns TRUE.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableSmrr (
+ VOID
+ )
+{
+ if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
+ }
+}
+
+/**
+ Enable SMRR register if SMRR is supported and
SmmCpuFeaturesNeedConfigureMtrrs()
+ returns TRUE.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesReenableSmrr (
+ VOID
+ )
+{
+ if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+ }
+}
+
+/**
+ Processor specific hook point each time a CPU enters System Management
Mode.
+
+ @param[in] CpuIndex The index of the CPU that has entered SMM. The
value
+ must be between 0 and the NumberOfCpus field in the
+ System Management System Table (SMST).
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesRendezvousEntry (
+ IN UINTN CpuIndex
+ )
+{
+ //
+ // If SMRR is supported and this is the first normal SMI, then enable SMRR
+ //
+ if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+ mSmrrEnabled[CpuIndex] = TRUE;
+ }
+}
+
+/**
+ Returns the current value of the SMM register for the specified CPU.
+ If the SMM register is not supported, then 0 is returned.
+
+ @param[in] CpuIndex The index of the CPU to read the SMM register.
The
+ value must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] RegName Identifies the SMM register to read.
+
+ @return The value of the SMM register specified by RegName from the
CPU
+ specified by CpuIndex.
+**/
+UINT64
+EFIAPI
+SmmCpuFeaturesGetSmmRegister (
+ IN UINTN CpuIndex,
+ IN SMM_REG_NAME RegName
+ )
+{
+ if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
SmmRegFeatureControl)) {
+ return AsmReadMsr64
(SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
+ }
+
+ return 0;
+}
+
+/**
+ Sets the value of an SMM register on a specified CPU.
+ If the SMM register is not supported, then no action is performed.
+
+ @param[in] CpuIndex The index of the CPU to write the SMM register.
The
+ value must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] RegName Identifies the SMM register to write.
+ registers are read-only.
+ @param[in] Value The value to write to the SMM register.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesSetSmmRegister (
+ IN UINTN CpuIndex,
+ IN SMM_REG_NAME RegName,
+ IN UINT64 Value
+ )
+{
+ if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
SmmRegFeatureControl)) {
+ AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL,
Value);
+ }
+}
+
diff --git
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 75a0ec8e948..7777e52740e 100644
---
a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -14,278 +14,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/PcdLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/DebugLib.h>
-#include <Register/Intel/Cpuid.h>
-#include <Register/Intel/SmramSaveStateMap.h>
-#include "CpuFeaturesLib.h"
-
-//
-// Machine Specific Registers (MSRs)
-//
-#define SMM_FEATURES_LIB_IA32_MTRR_CAP 0x0FE
-#define SMM_FEATURES_LIB_IA32_FEATURE_CONTROL 0x03A
-#define SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE 0x1F2
-#define SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK 0x1F3
-#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE 0x0A0
-#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK 0x0A1
-#define EFI_MSR_SMRR_MASK 0xFFFFF000
-#define EFI_MSR_SMRR_PHYS_MASK_VALID BIT11
-#define SMM_FEATURES_LIB_SMM_FEATURE_CONTROL 0x4E0
-
-//
-// MSRs required for configuration of SMM Code Access Check
-//
-#define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D
-#define SMM_CODE_ACCESS_CHK_BIT BIT58
-
-//
-// Set default value to assume IA-32 Architectural MSRs are used
-//
-UINT32 mSmrrPhysBaseMsr =
SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
-UINT32 mSmrrPhysMaskMsr =
SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
-
-//
-// Set default value to assume MTRRs need to be configured on each SMI
-//
-BOOLEAN mNeedConfigureMtrrs = TRUE;
-
-//
-// Array for state of SMRR enable on all CPUs
-//
-BOOLEAN *mSmrrEnabled;
-
-/**
- Performs library initialization.
-
- This initialization function contains common functionality shared betwen all
- library instance constructors.
-
-**/
-VOID
-CpuFeaturesLibInitialization (
- VOID
- )
-{
- UINT32 RegEax;
- UINT32 RegEdx;
- UINTN FamilyId;
- UINTN ModelId;
-
- //
- // Retrieve CPU Family and Model
- //
- AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
- FamilyId = (RegEax >> 8) & 0xf;
- ModelId = (RegEax >> 4) & 0xf;
- if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
- ModelId = ModelId | ((RegEax >> 12) & 0xf0);
- }
-
- //
- // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
- //
- if ((RegEdx & BIT12) != 0) {
- //
- // Check MTRR_CAP MSR bit 11 for SMRR support
- //
- if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) != 0)
{
- ASSERT (FeaturePcdGet (PcdSmrrEnable));
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor Family
- //
- // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H, then
- // SMRR Physical Base and SMM Physical Mask MSRs are not available.
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) ||
(ModelId == 0x35) || (ModelId == 0x36)) {
- ASSERT (!FeaturePcdGet (PcdSmrrEnable));
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor
Family
- //
- // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
- // Processor Family MSRs
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x17) || (ModelId == 0x0f)) {
- mSmrrPhysBaseMsr =
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
- mSmrrPhysMaskMsr =
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 34.4.2 SMRAM Caching
- // An IA-32 processor does not automatically write back and invalidate its
- // caches before entering SMM or before exiting SMM. Because of this
behavior,
- // care must be taken in the placement of the SMRAM in system memory
and in
- // the caching of the SMRAM to prevent cache incoherence when
switching back
- // and forth between SMM and protected mode operation.
- //
- // An IA-32 processor is a processor that does not support the Intel 64
- // Architecture. Support for the Intel 64 Architecture can be detected from
- // CPUID(CPUID_EXTENDED_CPU_SIG).EDX[29]
- //
- // If an IA-32 processor is detected, then set mNeedConfigureMtrrs to
TRUE,
- // so caches are flushed on SMI entry and SMI exit, the interrupted code
- // MTRRs are saved/restored, and MTRRs for SMM are loaded.
- //
- AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
- if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
- AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
- if ((RegEdx & BIT29) != 0) {
- mNeedConfigureMtrrs = FALSE;
- }
- }
-
- //
- // Allocate array for state of SMRR enable on all CPUs
- //
- mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) *
GetCpuMaxLogicalProcessorNumber ());
- ASSERT (mSmrrEnabled != NULL);
-}
-
-/**
- Called during the very first SMI into System Management Mode to initialize
- CPU features, including SMBASE, for the currently executing CPU. Since
this
- is the first SMI, the SMRAM Save State Map is at the default address of
- SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET. The
currently executing
- CPU is specified by CpuIndex and CpuIndex can be used to access
information
- about the currently executing CPU in the ProcessorInfo array and the
- HotPlugCpuData data structure.
-
- @param[in] CpuIndex The index of the CPU to initialize. The value
- must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] IsMonarch TRUE if the CpuIndex is the index of the CPU
that
- was elected as monarch during System Management
- Mode initialization.
- FALSE if the CpuIndex is not the index of the CPU
- that was elected as monarch during System
- Management Mode initialization.
- @param[in] ProcessorInfo Pointer to an array of
EFI_PROCESSOR_INFORMATION
- structures. ProcessorInfo[CpuIndex] contains the
- information for the currently executing CPU.
- @param[in] CpuHotPlugData Pointer to the CPU_HOT_PLUG_DATA
structure that
- contains the ApidId and SmBase arrays.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesInitializeProcessor (
- IN UINTN CpuIndex,
- IN BOOLEAN IsMonarch,
- IN EFI_PROCESSOR_INFORMATION *ProcessorInfo,
- IN CPU_HOT_PLUG_DATA *CpuHotPlugData
- )
-{
- SMRAM_SAVE_STATE_MAP *CpuState;
- UINT64 FeatureControl;
- UINT32 RegEax;
- UINT32 RegEdx;
- UINTN FamilyId;
- UINTN ModelId;
-
- //
- // Configure SMBASE.
- //
- CpuState = (SMRAM_SAVE_STATE_MAP
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
- CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor
Family
- //
- // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used,
then
- // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is
set before
- // accessing SMRR base/mask MSRs. If Lock(BIT0) of
MSR_FEATURE_CONTROL MSR(0x3A)
- // is set, then the MSR is locked and can not be modified.
- //
- if ((FeaturePcdGet (PcdSmrrEnable)) && (mSmrrPhysBaseMsr ==
SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {
- FeatureControl = AsmReadMsr64
(SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
- if ((FeatureControl & BIT3) == 0) {
- ASSERT ((FeatureControl & BIT0) == 0);
- if ((FeatureControl & BIT0) == 0) {
- AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL,
FeatureControl | BIT3);
- }
- }
- }

- //
- // If SMRR is supported, then program SMRR base/mask MSRs.
- // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first
normal SMI.
- // The code that initializes SMM environment is running in normal mode
- // from SMRAM region. If SMRR is enabled here, then the SMRAM region
- // is protected and the normal mode code execution will fail.
- //
- if (FeaturePcdGet (PcdSmrrEnable)) {
- //
- // SMRR size cannot be less than 4-KBytes
- // SMRR size must be of length 2^n
- // SMRR base alignment cannot be less than SMRR length
- //
- if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
- (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData-

SmrrSize)) ||

- ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) !=
CpuHotPlugData->SmrrBase))
- {
- //
- // Print message and halt if CPU is Monarch
- //
- if (IsMonarch) {
- DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet
alignment/size requirement!\n"));
- CpuDeadLoop ();
- }
- } else {
- AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase |
MTRR_CACHE_WRITE_BACK);
- AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1)
& EFI_MSR_SMRR_MASK));
- mSmrrEnabled[CpuIndex] = FALSE;
- }
- }
-
- //
- // Retrieve CPU Family and Model
- //
- AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
- FamilyId = (RegEax >> 8) & 0xf;
- ModelId = (RegEax >> 4) & 0xf;
- if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
- ModelId = ModelId | ((RegEax >> 12) & 0xf0);
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
- // Processor Family.
- //
- // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
- // Intel(R) Core(TM) Processor Family MSRs.
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
- (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) ||
(ModelId == 0x4F) ||
- (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) ||
(ModelId == 0x5C) ||
- (ModelId == 0x8C))
- {
- //
- // Check to see if the CPU supports the SMM Code Access Check feature
- // Do not access this MSR unless the CPU supports the
SmmRegFeatureControl
- //
- if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) &
SMM_CODE_ACCESS_CHK_BIT) != 0) {
- ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable));
- }
- }
- }
-
- //
- // Call internal worker function that completes the CPU initialization
- //
- FinishSmmCpuFeaturesInitializeProcessor ();
-}
+#include "CpuFeaturesLib.h"

/**
This function updates the SMRAM save state on the currently executing
CPU
@@ -345,75 +75,6 @@ SmmCpuFeaturesSmmRelocationComplete (
{
}

-/**
- Determines if MTRR registers must be configured to set SMRAM cache-
ability
- when executing in System Management Mode.
-
- @retval TRUE MTRR registers must be configured to set SMRAM cache-
ability.
- @retval FALSE MTRR registers do not need to be configured to set SMRAM
- cache-ability.
-**/
-BOOLEAN
-EFIAPI
-SmmCpuFeaturesNeedConfigureMtrrs (
- VOID
- )
-{
- return mNeedConfigureMtrrs;
-}
-
-/**
- Disable SMRR register if SMRR is supported and
SmmCpuFeaturesNeedConfigureMtrrs()
- returns TRUE.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesDisableSmrr (
- VOID
- )
-{
- if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
- }
-}
-
-/**
- Enable SMRR register if SMRR is supported and
SmmCpuFeaturesNeedConfigureMtrrs()
- returns TRUE.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesReenableSmrr (
- VOID
- )
-{
- if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
- }
-}
-
-/**
- Processor specific hook point each time a CPU enters System Management
Mode.
-
- @param[in] CpuIndex The index of the CPU that has entered SMM. The
value
- must be between 0 and the NumberOfCpus field in the
- System Management System Table (SMST).
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesRendezvousEntry (
- IN UINTN CpuIndex
- )
-{
- //
- // If SMRR is supported and this is the first normal SMI, then enable SMRR
- //
- if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64
(mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
- mSmrrEnabled[CpuIndex] = TRUE;
- }
-}
-
/**
Processor specific hook point each time a CPU exits System Management
Mode.

@@ -456,56 +117,6 @@ SmmCpuFeaturesIsSmmRegisterSupported (
return FALSE;
}

-/**
- Returns the current value of the SMM register for the specified CPU.
- If the SMM register is not supported, then 0 is returned.
-
- @param[in] CpuIndex The index of the CPU to read the SMM register.
The
- value must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] RegName Identifies the SMM register to read.
-
- @return The value of the SMM register specified by RegName from the
CPU
- specified by CpuIndex.
-**/
-UINT64
-EFIAPI
-SmmCpuFeaturesGetSmmRegister (
- IN UINTN CpuIndex,
- IN SMM_REG_NAME RegName
- )
-{
- if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
SmmRegFeatureControl)) {
- return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
- }
-
- return 0;
-}
-
-/**
- Sets the value of an SMM register on a specified CPU.
- If the SMM register is not supported, then no action is performed.
-
- @param[in] CpuIndex The index of the CPU to write the SMM register.
The
- value must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] RegName Identifies the SMM register to write.
- registers are read-only.
- @param[in] Value The value to write to the SMM register.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesSetSmmRegister (
- IN UINTN CpuIndex,
- IN SMM_REG_NAME RegName,
- IN UINT64 Value
- )
-{
- if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName ==
SmmRegFeatureControl)) {
- AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL,
Value);
- }
-}
-
/**
Read an SMM Save State register on the target processor. If this function
returns EFI_UNSUPPORTED, then the caller is responsible for reading the
--
2.37.1.windows.1

-----Original Message-----
From: Tan, Dun <dun.tan@...>
Sent: Friday, October 28, 2022 11:51 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Kumar,
Rahul R <rahul.r.kumar@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: [Patch V2] UefiCpuPkg: Restore HpetTimer after
CpuExceptionHandlerLib test

Disable/Restore HpetTimer before and after running the Dxe
CpuExceptionHandlerLib unit test module. During the UnitTest, a
new Idt is initialized for the test. There is no handler for timer
intrrupt in this new idt. After the test module, HpetTimer does
not work any more since the comparator value register and main
counter value register for timer does not match. To fix this issue,
disable/restore HpetTimer before and after Unit Test if HpetTimer
driver has been dispatched. We don't need to send Apic Eoi in this
unit test module.When disabling timer, after RaiseTPL(), if there
is a pending timer interrupt, bit64 of Interrupt Request Register
(IRR) will be set to 1 to indicate there is a pending timer
interrupt. After RestoreTPL(), CPU will handle the pending
interrupt in IRR.Then TimerInterruptHandler calls SendApicEoi().

Signed-off-by: Dun Tan <dun.tan@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---

UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan
dlerLibUnitTest.inf | 1 +

UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionHan
dlerUnitTest.c | 30 +++++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerLibUnitTest.inf
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerLibUnitTest.inf
index e3dbe7b9ab..a904eb2504 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerLibUnitTest.inf
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerLibUnitTest.inf
@@ -53,6 +53,7 @@

[Protocols]
gEfiMpServiceProtocolGuid
+ gEfiTimerArchProtocolGuid

[Depex]
gEfiMpServiceProtocolGuid
diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerUnitTest.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerUnitTest.c
index 917fc549bf..1cec3ed809 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerUnitTest.c
+++
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/UnitTest/DxeCpuExceptionH
andlerUnitTest.c
@@ -8,6 +8,7 @@

#include "CpuExceptionHandlerTest.h"
#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/Timer.h>

/**
Initialize Bsp Idt with a new Idt table and return the IA32_DESCRIPTOR
buffer.
@@ -162,8 +163,12 @@ CpuExceptionHandlerTestEntry (
{
EFI_STATUS Status;
UNIT_TEST_FRAMEWORK_HANDLE Framework;
+ EFI_TIMER_ARCH_PROTOCOL *TimerArchProtocol;
+ UINT64 TimerPeriod;

- Framework = NULL;
+ Framework = NULL;
+ TimerArchProtocol = NULL;
+ TimerPeriod = 0;

DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME,
UNIT_TEST_APP_VERSION));

@@ -182,11 +187,34 @@ CpuExceptionHandlerTestEntry (
goto EXIT;
}

+ //
+ // If HpetTimer driver has been dispatched, disable HpetTimer before Unit
Test.
+ //
+ gBS->LocateProtocol (&gEfiTimerArchProtocolGuid, NULL, (VOID
**)&TimerArchProtocol);
+ if (TimerArchProtocol != NULL) {
+ Status = TimerArchProtocol->GetTimerPeriod (TimerArchProtocol,
&TimerPeriod);
+ ASSERT_EFI_ERROR (Status);
+ if (TimerPeriod > 0) {
+ DEBUG ((DEBUG_INFO, "HpetTimer has been dispatched. Disable
HpetTimer.\n"));
+ Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol, 0);
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+
//
// Execute the tests.
//
Status = RunAllTestSuites (Framework);

+ //
+ // Restore HpetTimer after Unit Test.
+ //
+ if ((TimerArchProtocol != NULL) && (TimerPeriod > 0)) {
+ DEBUG ((DEBUG_INFO, "Restore HpetTimer after
DxeCpuExceptionHandlerLib UnitTest.\n"));
+ Status = TimerArchProtocol->SetTimerPeriod (TimerArchProtocol,
TimerPeriod);
+ ASSERT_EFI_ERROR (Status);
+ }
+
EXIT:
if (Framework) {
FreeUnitTestFramework (Framework);
--
2.31.1.windows.1

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yuwei
Chen
Sent: Tuesday, October 25, 2022 4:51 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@...>; Gao, Liming
<gaoliming@...>
Subject: [edk2-devel] [PATCH] BaseTools: Fix build option overrides Pcd
Feature Flag issue

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

INF [Sources] section Feature Flag Expressions do not use override values
from build --pcd option currently.
This patch fix this issue.

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Signed-off-by: Yuwei Chen <yuwei.chen@...>
---
edk2basetools/Workspace/InfBuildData.py | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/edk2basetools/Workspace/InfBuildData.py
b/edk2basetools/Workspace/InfBuildData.py
index c97a70e..8631373 100644
--- a/edk2basetools/Workspace/InfBuildData.py
+++ b/edk2basetools/Workspace/InfBuildData.py
@@ -1055,6 +1055,7 @@ def IsBinaryModule(self):
return False
def CheckFeatureFlagPcd(self,Instance):
Pcds = GlobalData.gPlatformFinalPcds.copy()
+ BuildOptionPcds = GlobalData.BuildOptionPcd
if PcdPattern.search(Instance):
PcdTuple = tuple(Instance.split('.')[::-1])
if PcdTuple in self.Pcds:
@@ -1062,6 +1063,14 @@ def CheckFeatureFlagPcd(self,Instance):
EdkLogger.error('build', FORMAT_INVALID,
"\nFeatureFlagPcd must be defined in a [PcdsFeatureFlag]
or [PcdsFixedAtBuild] section of Dsc or Dec file",
File=str(self), ExtraData=Instance)
+ for item in BuildOptionPcds:
+ if self.Pcds[PcdTuple].TokenCName in item:
+ if type(item) == type(()):
+ # ('gEfiCryptoPkgTokenSpaceGuid', 'PcdOpensslEcEnabled', '',
'1', ('build command options', 1)
+ Pcds[Instance] = list(item)[3]
+ elif type(item) == type(''):
+ # gEfiCryptoPkgTokenSpaceGuid.PcdOpensslEcEnabled=1
+ Pcds[Instance] = item.split('=')[1]
if not Instance in Pcds:
Pcds[Instance] = self.Pcds[PcdTuple].DefaultValue
else: #if PcdTuple not in self.Pcds:
@@ -1085,6 +1094,14 @@ def CheckFeatureFlagPcd(self,Instance):
for Name, Guid in self.Pcds:
if self.Pcds[(Name, Guid)].Type == 'FeatureFlag' or self.Pcds[(Name,
Guid)].Type == 'FixedAtBuild':
PcdFullName = '%s.%s' % (Guid, Name);
+ for item in BuildOptionPcds:
+ if Name in item:
+ if type(item) == type(()):
+ # ('gEfiCryptoPkgTokenSpaceGuid', 'PcdOpensslEcEnabled',
'', '1', ('build command options', 1)
+ Pcds[PcdFullName] = list(item)[3]
+ elif type(item) == type(''):
+ # gEfiCryptoPkgTokenSpaceGuid.PcdOpensslEcEnabled=1
+ Pcds[PcdFullName] = item.split('=')[1]
if not PcdFullName in Pcds:
Pcds[PcdFullName] = self.Pcds[(Name, Guid)].DefaultValue
try:
--
2.27.0.windows.1

-----Original Message-----
From: Xu, Min M <min.m.xu@...>
Sent: Friday, October 28, 2022 4:24 PM
To: devel@edk2.groups.io
Cc: Xu, Min M <min.m.xu@...>; Aktas, Erdem
<erdemaktas@...>; Gerd Hoffmann <kraxel@...>; James
Bottomley <jejb@...>; Yao, Jiewen <jiewen.yao@...>;
Tom Lendacky <thomas.lendacky@...>
Subject: [PATCH V2 1/1] OvmfPkg/VmgExitLig: HALT on #VE when access to
private memory

From: Min M Xu <min.m.xu@...>

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

EPT-violation #VE should be always on shared memory, which means the
shared bit of the GuestPA should be set. But in current #VE Handler
it is not checked. When it occurs, stop TD immediately and log out
the error.

Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
.../Library/VmgExitLib/VmTdExitVeHandler.c | 40 ++++++++++++++-----
1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
b/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
index b73e877c093b..c89268c5d8e8 100644
--- a/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmTdExitVeHandler.c
@@ -300,23 +300,41 @@ MmioExit (
IN TDCALL_VEINFO_RETURN_DATA *Veinfo
)
{
- UINT64 Status;
- UINT32 MmioSize;
- UINT32 RegSize;
- UINT8 OpCode;
- BOOLEAN SeenRex;
- UINT64 *Reg;
- UINT8 *Rip;
- UINT64 Val;
- UINT32 OpSize;
- MODRM ModRm;
- REX Rex;
+ UINT64 Status;
+ UINT32 MmioSize;
+ UINT32 RegSize;
+ UINT8 OpCode;
+ BOOLEAN SeenRex;
+ UINT64 *Reg;
+ UINT8 *Rip;
+ UINT64 Val;
+ UINT32 OpSize;
+ MODRM ModRm;
+ REX Rex;
+ TD_RETURN_DATA TdReturnData;
+ UINT8 Gpaw;
+ UINT64 TdSharedPageMask;

Rip = (UINT8 *)Regs->Rip;
Val = 0;
Rex.Val = 0;
SeenRex = FALSE;

+ Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ if (Status == TDX_EXIT_REASON_SUCCESS) {
+ Gpaw = (UINT8)(TdReturnData.TdInfo.Gpaw & 0x3f);
+ TdSharedPageMask = 1ULL << (Gpaw - 1);
+ } else {
+ DEBUG ((DEBUG_ERROR, "TDCALL failed with status=%llx\n", Status));
+ return Status;
+ }
+
+ if ((Veinfo->GuestPA & TdSharedPageMask) == 0) {
+ DEBUG ((DEBUG_ERROR, "EPT-violation #VE on private memory is not
allowed!"));
+ TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+ CpuDeadLoop ();
+ }
+
//
// Default to 32bit transfer
//
--
2.29.2.windows.2