subject:"\[edk2\-devel\]\[PATCH v2 2\/8\] IntelFsp2Pkg\: Add FSPx_ARCH2

Re: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

2022-04-06 Thread Chiu, Chasel


One more comments.

Thanks,
Chasel


> -Original Message-
> From: Chiu, Chasel
> Sent: Wednesday, April 6, 2022 10:40 PM
> To: Kuo, Ted ; devel@edk2.groups.io
> Cc: Desimone, Nathaniel L ; Zeng, Star
> ; S, Ashraf Ali 
> Subject: RE: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add
> FSPx_ARCH2_UPD support for X64
> 
> 
> Hi Ted,
> 
> Please see my comments inline below.
> 
> Thanks,
> Chasel
> 
> 
> > -Original Message-
> > From: Kuo, Ted 
> > Sent: Monday, April 4, 2022 2:23 PM
> > To: devel@edk2.groups.io
> > Cc: Chiu, Chasel ; Desimone, Nathaniel L
> > ; Zeng, Star ; S,
> > Ashraf Ali 
> > Subject: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD
> > support for X64
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3893
> > 1.Added FSPx_ARCH2_UPD structures which support both IA32 and X64.
> > 2.Added FSPx_UPD_COMMON_FSP24 structures.
> >
> > Cc: Chasel Chiu 
> > Cc: Nate DeSimone 
> > Cc: Star Zeng 
> > Cc: Ashraf Ali S 
> > Signed-off-by: Ted Kuo 
> > ---
> >  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm |  29 +
> > IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm |  93 +++--
> >  IntelFsp2Pkg/Include/FspEas/FspApi.h   | 172
> > -
> >  IntelFsp2Pkg/Tools/GenCfgOpt.py|   2 +-
> >  4 files changed, 278 insertions(+), 18 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> > b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> > index e7261b41cd..de7f916070 100644
> > --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> > +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> > @@ -32,6 +32,25 @@ struc FSPM_UPD_COMMON
> >  .size:
> >  endstruc
> >
> > +struc FSPM_UPD_COMMON_FSP24
> > +; FSP_UPD_HEADER {
> > +.FspUpdHeader:  resd  8
> > +; }
> > +; FSPM_ARCH2_UPD {
> > +.Revision:  resb  1
> > +.Reserved:  resb  3
> > +.Length resd  1
> > +.StackBase: resq  1
> > +.StackSize: resq  1
> > +.BootLoaderTolumSize:   resd  1
> > +.BootMode:  resd  1
> > +.FspEventHandlerresq  1
> > +.EnableMultiPhaseMemoryInit resb  1
> > +.Reserved1: resb 23
> 
> 
> 
> 
> Please remove EnableMultiPhaseMemoryInit as it is not used.
> Also the Reserved1 will  be 24 bytes instead of 23
> 
> 
> 
> 
> > +; }
> > +.size:
> > +endstruc
> > +
> >  ;
> >  ; Following functions will be provided in C  ; @@ -124,12 +143,22 @@
> > ASM_PFX(FspApiCommonContinue):
> >popeax
> >
> >  FspStackSetup:
> > +  movecx, [edx + FSPM_UPD_COMMON.Revision]
> > +  cmpecx, 3
> > +  jaeFspmUpdCommon2
> > +
> >;
> >; StackBase = temp memory base, StackSize = temp memory size
> >;
> >movedi, [edx + FSPM_UPD_COMMON.StackBase]
> >movecx, [edx + FSPM_UPD_COMMON.StackSize]
> > +  jmpChkFspHeapSize
> > +
> > +FspmUpdCommon2:
> > +  movedi, [edx + FSPM_UPD_COMMON_FSP24.StackBase]
> > +  movecx, [edx + FSPM_UPD_COMMON_FSP24.StackSize]
> >
> > +ChkFspHeapSize:
> >;
> >; Keep using bootloader stack if heap size % is 0
> >;
> > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> > b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> > index 7fd3d6d843..25ef99b798 100644
> > --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> > +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> > @@ -84,8 +84,10 @@ struc LoadMicrocodeParamsFsp22
> >  .FspUpdHeaderRevision:resb1
> >  .FspUpdHeaderReserved:resb   23
> >  ; }
> > -; FSPT_ARCH_UPD{
> > -.FsptArchUpd: resd8
> > +; FSPT_ARCH_UPD {
> > +.FsptArchRevision:resb1
> > +.FsptArchReserved:resb3
> > +.FsptArchUpd: resd7
> >  ; }
> >  ; FSPT_CORE_UPD {
> >  .MicrocodeCodeAddr:   resd1
> > @@ -96,6 +98,25 @@ struc LoadMicrocodeParamsFsp22
> >  .size:
> >  endstruc
> >
> > +struc LoadMicrocodeParamsFsp24
> > +; FSP_UPD_HEADER {
> > +.FspUpdHeaderSignature:   resd2
> > +.FspUpdHeaderRevision:resb1
> > +.FspUpdHeaderReserved:resb   23
> &

Re: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

2022-04-06 Thread Chiu, Chasel


Hi Ted,

Please see my comments inline below.

Thanks,
Chasel


> -Original Message-
> From: Kuo, Ted 
> Sent: Monday, April 4, 2022 2:23 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; S,
> Ashraf Ali 
> Subject: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD
> support for X64
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3893
> 1.Added FSPx_ARCH2_UPD structures which support both IA32 and X64.
> 2.Added FSPx_UPD_COMMON_FSP24 structures.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> Cc: Ashraf Ali S 
> Signed-off-by: Ted Kuo 
> ---
>  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm |  29 +
> IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm |  93 +++--
>  IntelFsp2Pkg/Include/FspEas/FspApi.h   | 172
> -
>  IntelFsp2Pkg/Tools/GenCfgOpt.py|   2 +-
>  4 files changed, 278 insertions(+), 18 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> index e7261b41cd..de7f916070 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> @@ -32,6 +32,25 @@ struc FSPM_UPD_COMMON
>  .size:
>  endstruc
> 
> +struc FSPM_UPD_COMMON_FSP24
> +; FSP_UPD_HEADER {
> +.FspUpdHeader:  resd  8
> +; }
> +; FSPM_ARCH2_UPD {
> +.Revision:  resb  1
> +.Reserved:  resb  3
> +.Length resd  1
> +.StackBase: resq  1
> +.StackSize: resq  1
> +.BootLoaderTolumSize:   resd  1
> +.BootMode:  resd  1
> +.FspEventHandlerresq  1
> +.EnableMultiPhaseMemoryInit resb  1
> +.Reserved1: resb 23




Please remove EnableMultiPhaseMemoryInit as it is not used.
Also the Reserved1 will  be 24 bytes instead of 23




> +; }
> +.size:
> +endstruc
> +
>  ;
>  ; Following functions will be provided in C  ; @@ -124,12 +143,22 @@
> ASM_PFX(FspApiCommonContinue):
>popeax
> 
>  FspStackSetup:
> +  movecx, [edx + FSPM_UPD_COMMON.Revision]
> +  cmpecx, 3
> +  jaeFspmUpdCommon2
> +
>;
>; StackBase = temp memory base, StackSize = temp memory size
>;
>movedi, [edx + FSPM_UPD_COMMON.StackBase]
>movecx, [edx + FSPM_UPD_COMMON.StackSize]
> +  jmpChkFspHeapSize
> +
> +FspmUpdCommon2:
> +  movedi, [edx + FSPM_UPD_COMMON_FSP24.StackBase]
> +  movecx, [edx + FSPM_UPD_COMMON_FSP24.StackSize]
> 
> +ChkFspHeapSize:
>;
>; Keep using bootloader stack if heap size % is 0
>;
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 7fd3d6d843..25ef99b798 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -84,8 +84,10 @@ struc LoadMicrocodeParamsFsp22
>  .FspUpdHeaderRevision:resb1
>  .FspUpdHeaderReserved:resb   23
>  ; }
> -; FSPT_ARCH_UPD{
> -.FsptArchUpd: resd8
> +; FSPT_ARCH_UPD {
> +.FsptArchRevision:resb1
> +.FsptArchReserved:resb3
> +.FsptArchUpd: resd7
>  ; }
>  ; FSPT_CORE_UPD {
>  .MicrocodeCodeAddr:   resd1
> @@ -96,6 +98,25 @@ struc LoadMicrocodeParamsFsp22
>  .size:
>  endstruc
> 
> +struc LoadMicrocodeParamsFsp24
> +; FSP_UPD_HEADER {
> +.FspUpdHeaderSignature:   resd2
> +.FspUpdHeaderRevision:resb1
> +.FspUpdHeaderReserved:resb   23
> +; }
> +; FSPT_ARCH2_UPD {
> +.FsptArchRevision:resb1
> +.FsptArchReserved:resb3
> +.FsptArchLength:  resd1
> +.FspDebugHandler  resq1
> +.MicrocodeCodeAddr:   resq1
> +.MicrocodeCodeSize:   resq1
> +.CodeRegionBase:  resq1
> +.CodeRegionSize:  resq1




Please remove 4 elements from the ARCH2 structure:
   .MicrocodeCodeAddr:   resq1
   .MicrocodeCodeSize:   resq1
   .CodeRegionBase:  resq1
   .CodeRegionSize:  resq1

Also the new CORE_UPD structure is still after ARCH2_UPD structure which having 
64bits elements.







> +; }
> +.size:
> +endstruc
> +
>  ;
>  ; Define SSE macros
>  ;
> @@ -172,9 +193,9 @@ ASM_PFX(LoadMicrocodeDefault):
> ;   Executed by SBSP and NBSP
> ;   Beginning of microcode update region starts on p

[edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

2022-04-04 Thread Kuo, Ted

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3893
1.Added FSPx_ARCH2_UPD structures which support both IA32 and X64.
2.Added FSPx_UPD_COMMON_FSP24 structures.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Ashraf Ali S 
Signed-off-by: Ted Kuo 
---
 IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm |  29 +
 IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm |  93 +++--
 IntelFsp2Pkg/Include/FspEas/FspApi.h   | 172 -
 IntelFsp2Pkg/Tools/GenCfgOpt.py|   2 +-
 4 files changed, 278 insertions(+), 18 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
index e7261b41cd..de7f916070 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
@@ -32,6 +32,25 @@ struc FSPM_UPD_COMMON
 .size:
 endstruc
 
+struc FSPM_UPD_COMMON_FSP24
+; FSP_UPD_HEADER {
+.FspUpdHeader:  resd  8
+; }
+; FSPM_ARCH2_UPD {
+.Revision:  resb  1
+.Reserved:  resb  3
+.Length resd  1
+.StackBase: resq  1
+.StackSize: resq  1
+.BootLoaderTolumSize:   resd  1
+.BootMode:  resd  1
+.FspEventHandlerresq  1
+.EnableMultiPhaseMemoryInit resb  1
+.Reserved1: resb 23
+; }
+.size:
+endstruc
+
 ;
 ; Following functions will be provided in C
 ;
@@ -124,12 +143,22 @@ ASM_PFX(FspApiCommonContinue):
   popeax
 
 FspStackSetup:
+  movecx, [edx + FSPM_UPD_COMMON.Revision]
+  cmpecx, 3
+  jaeFspmUpdCommon2
+
   ;
   ; StackBase = temp memory base, StackSize = temp memory size
   ;
   movedi, [edx + FSPM_UPD_COMMON.StackBase]
   movecx, [edx + FSPM_UPD_COMMON.StackSize]
+  jmpChkFspHeapSize
+
+FspmUpdCommon2:
+  movedi, [edx + FSPM_UPD_COMMON_FSP24.StackBase]
+  movecx, [edx + FSPM_UPD_COMMON_FSP24.StackSize]
 
+ChkFspHeapSize:
   ;
   ; Keep using bootloader stack if heap size % is 0
   ;
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
index 7fd3d6d843..25ef99b798 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
@@ -84,8 +84,10 @@ struc LoadMicrocodeParamsFsp22
 .FspUpdHeaderRevision:resb1
 .FspUpdHeaderReserved:resb   23
 ; }
-; FSPT_ARCH_UPD{
-.FsptArchUpd: resd8
+; FSPT_ARCH_UPD {
+.FsptArchRevision:resb1
+.FsptArchReserved:resb3
+.FsptArchUpd: resd7
 ; }
 ; FSPT_CORE_UPD {
 .MicrocodeCodeAddr:   resd1
@@ -96,6 +98,25 @@ struc LoadMicrocodeParamsFsp22
 .size:
 endstruc
 
+struc LoadMicrocodeParamsFsp24
+; FSP_UPD_HEADER {
+.FspUpdHeaderSignature:   resd2
+.FspUpdHeaderRevision:resb1
+.FspUpdHeaderReserved:resb   23
+; }
+; FSPT_ARCH2_UPD {
+.FsptArchRevision:resb1
+.FsptArchReserved:resb3
+.FsptArchLength:  resd1
+.FspDebugHandler  resq1
+.MicrocodeCodeAddr:   resq1
+.MicrocodeCodeSize:   resq1
+.CodeRegionBase:  resq1
+.CodeRegionSize:  resq1
+; }
+.size:
+endstruc
+
 ;
 ; Define SSE macros
 ;
@@ -172,9 +193,9 @@ ASM_PFX(LoadMicrocodeDefault):
;   Executed by SBSP and NBSP
;   Beginning of microcode update region starts on paragraph boundary
 
-   ;
;
; Save return address to EBP
+   ;
movd   ebp, mm7
 
cmpesp, 0
@@ -188,8 +209,12 @@ ASM_PFX(LoadMicrocodeDefault):
; and report error if size is less than 2k
; first check UPD header revision
cmpbyte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2
-   jaeFsp22UpdHeader
+   jb Fsp20UpdHeader
+   cmpbyte [esp + LoadMicrocodeParamsFsp22.FsptArchRevision], 2
+   je Fsp24UpdHeader
+   jmpFsp22UpdHeader
 
+Fsp20UpdHeader:
; UPD structure is compliant with FSP spec 2.0/2.1
moveax, dword [esp + LoadMicrocodeParams.MicrocodeCodeSize]
cmpeax, 0
@@ -213,6 +238,19 @@ Fsp22UpdHeader:
movesi, dword [esp + LoadMicrocodeParamsFsp22.MicrocodeCodeAddr]
cmpesi, 0
jnzCheckMainHeader
+   jmpParamError
+
+Fsp24UpdHeader:
+   ; UPD structure is compliant with FSP spec 2.4
+   moveax, dword [esp + LoadMicrocodeParamsFsp24.MicrocodeCodeSize]
+   cmpeax, 0
+   jz Exit2
+   cmpeax, 0800h
+   jl ParamError
+
+   movesi, dword [esp + LoadMicrocodeParamsFsp24.MicrocodeCodeAddr]
+   cmpesi, 0
+   jnzCheckMainHeader
 
 ParamError:
moveax, 08002h
@@ -308,9 +346,13 @@ AdvanceFixedSize:
 
 CheckAddress:
; Check UPD header revision
-   cmpbyte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2
-   jaeFsp22UpdHeader1

Re: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

Re: [edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

[edk2-devel][PATCH v2 2/8] IntelFsp2Pkg: Add FSPx_ARCH2_UPD support for X64

3 matches

Site Navigation

Mail list logo

Footer information