Re: [edk2-devel][edk2-platforms][PATCH v3] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Hi Sai,
Thanks for the feedback. I'll change SecFspWrapperPlatformSecLibFspO to
SecFspWrapperPlatformSecLibResetVectorInFsp in patch V4. Regarding the
timestamp, I'm planning to save the TSC value near the entry of FspSecCoreO in
FSP. In patch V4, FSP wrapper/platform code reads the timestamp from mm5
instead of ymm6 and then push it to stack.
Thanks,
Ted
-Original Message-
From: Chaganty, Rangasai V
Sent: Saturday, December 30, 2023 4:34 AM
To: Kuo, Ted ; devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Dong, Eric ; S, Ashraf
Ali ; Duggapu, Chinni B ;
Gao, Liming
Subject: RE: [edk2-devel][edk2-platforms][PATCH v3] MinPlatformPkg: Support
SecFspWrapperPlatformSecLib in X64
Hi Ted,
I see good improvements on V3. Thanks!
Few comments/feedback:
1. Minor feedback - The module name choice SecFspWrapperPlatformSecLibFspO.inf
is assuming FSP owning reset vector will be part of FSP-O. it's an
implementation choice. Perhaps consider the name to be
SecFspWrapperPlatformSecLibResetVectorInFsp or
SecFspWrapperPlatformSecLibFor64Bit (to be more relevant to the purpose of the
file).
2. I see TSC values are pushed into stack. However, I am not seeing a rdtsc
instruction to read the TSC value. Can you clarify how are we getting the TSC
values?
Thanks,
Sai
-Original Message-
From: Kuo, Ted
Sent: Friday, December 22, 2023 2:14 AM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V ; Chiu, Chasel
; Desimone, Nathaniel L
; Dong, Eric ; S, Ashraf
Ali ; Duggapu, Chinni B ;
Gao, Liming
Subject: [edk2-devel][edk2-platforms][PATCH v3] MinPlatformPkg: Support
SecFspWrapperPlatformSecLib in X64
https://bugzilla.tianocore.org/show_bug.cgi?id=4623
1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
2.Made changes in common file to support both IA32 and X64.
3.Added the PCDs below for FSP-T UPD revisions and reset vector in FSP.
- PcdFspWrapperBfvforResetVectorInFsp
- PcdFsptUpdHeaderRevision
- PcdFsptArchUpdRevision
Cc: Sai Chaganty
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Eric Dong
Cc: Ashraf Ali S
Cc: Chinni B Duggapu
Cc: Liming Gao
Signed-off-by: Ted Kuo
---
.../SecFspWrapperPlatformSecLib/FsptCoreUpd.h | 25 +-
.../Ia32/SecEntry.nasm| 4 +-
.../SecFspWrapperPlatformSecLib.inf | 9 +-
.../SecFspWrapperPlatformSecLibFspO.inf | 101
.../SecGetPerformance.c | 11 +-
.../SecPlatformInformation.c | 8 +-
.../SecRamInitData.c | 73 --
.../X64/PeiCoreEntry.nasm | 218 ++
.../X64/SecEntry.nasm | 71 ++
.../X64/Stack.nasm| 72 ++
.../Ia32 => Include}/Fsp.h| 4 +-
.../Intel/MinPlatformPkg/MinPlatformPkg.dec | 15 ++
12 files changed, 579 insertions(+), 32 deletions(-) create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLibFspO.inf
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
rename
Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32
=> Include}/Fsp.h (86%)
diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
index 7c0f605b92..cc36334227 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/FsptCoreUpd.h
@@ -1,6 +1,6 @@
/** @file -Copyright (c) 2017, Intel Corporation. All rights
reserved.+Copyright (c) 2017 - 2023, Intel Corporation. All rights
reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -10,6 +10,7
@@ SPDX-License-Identifier: BSD-2-Clause-Patent
#pragma pack(1) +#if FixedPcdGet8 (PcdFsptArchUpdRevision) <= 1 /** Fsp T
Core UPD **/ typedef struct {@@ -34,6 +35,28 @@ typedef struct {
**/ UINT8 Reserved[16]; } FSPT_CORE_UPD;+#else+/** Fsp
T Core UPD+**/+typedef struct {++/** Offset 0x0040+**/+ EFI_PHYSICAL_ADDRESS
MicrocodeRegionBase;++/** Offset 0x0048+**/+ UINT64
MicrocodeRegionSize;++/** Offset 0x0050+**/+ EFI_PHYSICAL_ADDRESS
CodeRegionBase;++/** Offset 0x0058+**/+ UINT64
CodeRegionSize;+} FSPT_CORE_UPD;+#endif #pragma pack() diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
Re: [edk2-devel][edk2-platforms][PATCH v3] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Hi Ted,
I see good improvements on V3. Thanks!
Few comments/feedback:
1. Minor feedback - The module name choice SecFspWrapperPlatformSecLibFspO.inf
is assuming FSP owning reset vector will be part of FSP-O. it's an
implementation choice. Perhaps consider the name to be
SecFspWrapperPlatformSecLibResetVectorInFsp or
SecFspWrapperPlatformSecLibFor64Bit (to be more relevant to the purpose of the
file).
2. I see TSC values are pushed into stack. However, I am not seeing a rdtsc
instruction to read the TSC value. Can you clarify how are we getting the TSC
values?
Thanks,
Sai
-Original Message-
From: Kuo, Ted
Sent: Friday, December 22, 2023 2:14 AM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V ; Chiu, Chasel
; Desimone, Nathaniel L
; Dong, Eric ; S, Ashraf
Ali ; Duggapu, Chinni B ;
Gao, Liming
Subject: [edk2-devel][edk2-platforms][PATCH v3] MinPlatformPkg: Support
SecFspWrapperPlatformSecLib in X64
https://bugzilla.tianocore.org/show_bug.cgi?id=4623
1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
2.Made changes in common file to support both IA32 and X64.
3.Added the PCDs below for FSP-T UPD revisions and reset vector in FSP.
- PcdFspWrapperBfvforResetVectorInFsp
- PcdFsptUpdHeaderRevision
- PcdFsptArchUpdRevision
Cc: Sai Chaganty
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Eric Dong
Cc: Ashraf Ali S
Cc: Chinni B Duggapu
Cc: Liming Gao
Signed-off-by: Ted Kuo
---
.../SecFspWrapperPlatformSecLib/FsptCoreUpd.h | 25 +-
.../Ia32/SecEntry.nasm| 4 +-
.../SecFspWrapperPlatformSecLib.inf | 9 +-
.../SecFspWrapperPlatformSecLibFspO.inf | 101
.../SecGetPerformance.c | 11 +-
.../SecPlatformInformation.c | 8 +-
.../SecRamInitData.c | 73 --
.../X64/PeiCoreEntry.nasm | 218 ++
.../X64/SecEntry.nasm | 71 ++
.../X64/Stack.nasm| 72 ++
.../Ia32 => Include}/Fsp.h| 4 +-
.../Intel/MinPlatformPkg/MinPlatformPkg.dec | 15 ++
12 files changed, 579 insertions(+), 32 deletions(-) create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLibFspO.inf
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
create mode 100644
Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
rename
Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32
=> Include}/Fsp.h (86%)
diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
index 7c0f605b92..cc36334227 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/FsptCoreUpd.h
@@ -1,6 +1,6 @@
/** @file -Copyright (c) 2017, Intel Corporation. All rights
reserved.+Copyright (c) 2017 - 2023, Intel Corporation. All rights
reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -10,6 +10,7
@@ SPDX-License-Identifier: BSD-2-Clause-Patent
#pragma pack(1) +#if FixedPcdGet8 (PcdFsptArchUpdRevision) <= 1 /** Fsp T
Core UPD **/ typedef struct {@@ -34,6 +35,28 @@ typedef struct {
**/ UINT8 Reserved[16]; } FSPT_CORE_UPD;+#else+/** Fsp
T Core UPD+**/+typedef struct {++/** Offset 0x0040+**/+ EFI_PHYSICAL_ADDRESS
MicrocodeRegionBase;++/** Offset 0x0048+**/+ UINT64
MicrocodeRegionSize;++/** Offset 0x0050+**/+ EFI_PHYSICAL_ADDRESS
CodeRegionBase;++/** Offset 0x0058+**/+ UINT64
CodeRegionSize;+} FSPT_CORE_UPD;+#endif #pragma pack() diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
index 7f6d771e41..de44066a20 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/Ia32/SecEntry.nasm
@@ -1,6 +1,6 @@
;--
;-; Copyright (c) 2019, Intel Corporation. All rights reserved.+; Copyright
(c) 2019 - 2023, Intel Corporation. All rights reserved. ;
SPDX-License-Identifier: BSD-2-Clause-Patent ; Module Name: ;@@ -13,7 +13,7 @@
;
;--
-#include "Fsp.h"+#include SECTION .text diff --git
