Re: [edk2-devel] [Patch V2 1/3] UefiCpuPkg/MpInitLib:Repeat WakeUpAp of loading microcode,Syncing MTRR.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Xie, Yuanhao 
> Sent: Tuesday, November 21, 2023 3:40 PM
> To: devel@edk2.groups.io
> Cc: Xie, Yuanhao ; Ni, Ray ;
> Dong, Eric ; Kumar, Rahul R
> ; Tom Lendacky ;
> Laszlo Ersek 
> Subject: [Patch V2 1/3] UefiCpuPkg/MpInitLib:Repeat WakeUpAp of loading
> microcode,Syncing MTRR.
> 
> This patch replicates the WakeUpAp operation, encompassing both
> microcode loading and MTRR synchronization, regardless of whether
> MpHandOff is NULL.
> 
> The purpose of this patch is to enhance the review process.
> The separation in this patch is aimed at facilitating a more
> straightforward review, with the ultimate goal of eliminating the
> microcode loading functionality for the second time Mp initialization.
> 
> Cc: Ray Ni 
> Cc: Eric Dong 
> Cc: Rahul Kumar 
> Cc: Tom Lendacky 
> Cc: Laszlo Ersek 
> Signed-off-by: Yuanhao Xie 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 9a6ec5db5c..e8ffb99874 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -2256,9 +2256,11 @@ MpInitLibInitialize (
>// in DXE.
>//
>CpuMpData->InitFlag = ApInitReconfig;
> +  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData,
> TRUE);
> +} else {
> +  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData,
> TRUE);
>  }
> 
> -WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
>  //
>  // Wait for all APs finished initialization
>  //
> --
> 2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111531): https://edk2.groups.io/g/devel/message/111531
Mute This Topic: https://groups.io/mt/102724546/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 6/6] MdePkg: Use macro CR4_CET_BIT to replace hard code value.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 6/6] MdePkg: Use macro CR4_CET_BIT to replace hard
> code value.
> 
> The macro is used in file LongJump.nasm and SetJump.nasm.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  MdePkg/Library/BaseLib/Ia32/LongJump.nasm | 3 ++-
>  MdePkg/Library/BaseLib/Ia32/SetJump.nasm  | 3 ++-
>  MdePkg/Library/BaseLib/X64/LongJump.nasm  | 3 ++-
>  MdePkg/Library/BaseLib/X64/SetJump.nasm   | 3 ++-
>  4 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
> b/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
> index 6c13dfe307..df1bf9749e 100644
> --- a/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
> +++ b/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
> @@ -14,6 +14,7 @@
>  
> ;--
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  SECTION .text
> 
> 
> 
> @@ -34,7 +35,7 @@ ASM_PFX(InternalLongJump):
>  testeax, eax
> 
>  jz  CetDone
> 
>  mov eax, cr4
> 
> -bt  eax, 23; check if CET is enabled
> 
> +bt  eax, CR4_CET_BIT   ; check if CET is enabled
> 
>  jnc CetDone
> 
> 
> 
>  mov edx, [esp + 4] ; edx = JumpBuffer
> 
> diff --git a/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
> b/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
> index 2577373241..0c484f6852 100644
> --- a/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
> +++ b/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
> @@ -14,6 +14,7 @@
>  
> ;--
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  SECTION .text
> 
> 
> 
> @@ -42,7 +43,7 @@ ASM_PFX(SetJump):
>  testeax, eax
> 
>  jz  CetDone
> 
>  mov eax, cr4
> 
> -bt  eax, 23; check if CET is enabled
> 
> +bt  eax, CR4_CET_BIT   ; check if CET is enabled
> 
>  jnc CetDone
> 
> 
> 
>  mov eax, 1
> 
> diff --git a/MdePkg/Library/BaseLib/X64/LongJump.nasm
> b/MdePkg/Library/BaseLib/X64/LongJump.nasm
> index 2002f65cba..021b49e855 100644
> --- a/MdePkg/Library/BaseLib/X64/LongJump.nasm
> +++ b/MdePkg/Library/BaseLib/X64/LongJump.nasm
> @@ -14,6 +14,7 @@
>  
> ;--
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  DEFAULT REL
> 
>  SECTION .text
> 
> @@ -35,7 +36,7 @@ ASM_PFX(InternalLongJump):
>  testeax, eax
> 
>  jz  CetDone
> 
>  mov rax, cr4
> 
> -bt  eax, 23  ; check if CET is enabled
> 
> +bt  eax, CR4_CET_BIT ; check if CET is enabled
> 
>  jnc CetDone
> 
> 
> 
>  pushrdx  ; save rdx
> 
> diff --git a/MdePkg/Library/BaseLib/X64/SetJump.nasm
> b/MdePkg/Library/BaseLib/X64/SetJump.nasm
> index 5943a5ebe5..d2c0991e66 100644
> --- a/MdePkg/Library/BaseLib/X64/SetJump.nasm
> +++ b/MdePkg/Library/BaseLib/X64/SetJump.nasm
> @@ -14,6 +14,7 @@
>  
> ;--
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  DEFAULT REL
> 
>  SECTION .text
> 
> @@ -44,7 +45,7 @@ ASM_PFX(SetJump):
>  testeax, eax
> 
>  jz  CetDone
> 
>  mov rax, cr4
> 
> -bt  eax, 23  ; check if CET is enabled
> 
> +bt  eax, CR4_CET_BIT ; check if CET is enabled
> 
>  jnc CetDone
> 
> 
> 
>  mov rax, 1
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111530): https://edk2.groups.io/g/devel/message/111530
Mute This Topic: https://groups.io/mt/102724277/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 5/6] UefiCpuPkg: Backup and Restore MSR IA32_U_CET in SMI handler.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 5/6] UefiCpuPkg: Backup and Restore MSR IA32_U_CET in
> SMI handler.
> 
> OS may enable CET-IBT feature by set MSR IA32_U_CET.bit2.
> If IA32_U_CET.bit2 is set, CPU is in WAIT_FOR_ENDBRANCH state and
>  the next assemble code is not ENDBR, it will trigger #CP exception
>  when set CR4.CET bit.
> SMI handler needs to backup MSR IA32_U_CET and clear MSR IA32_U_CET
>  before set CR4.CET bit,
> And SMI handler needs to restore MSR IA32_U_CET when exit SMI handler.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 15
> +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 15
> +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> index 1da9afab97..9e1155dee6 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> @@ -202,11 +202,21 @@ ASM_PFX(mPatchCetSupported):
>  pushedx
> 
>  pusheax
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +rdmsr
> 
> +pushedx
> 
> +pusheax
> 
> +
> 
>  mov ecx, MSR_IA32_PL0_SSP
> 
>  rdmsr
> 
>  pushedx
> 
>  pusheax
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +xor eax, eax
> 
> +xor edx, edx
> 
> +wrmsr
> 
> +
> 
>  mov ecx, MSR_IA32_S_CET
> 
>  mov eax, MSR_IA32_CET_SH_STK_EN
> 
>  xor edx, edx
> 
> @@ -276,6 +286,11 @@ CetDone:
>  pop edx
> 
>  wrmsr
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +pop eax
> 
> +pop edx
> 
> +wrmsr
> 
> +
> 
>  mov ecx, MSR_IA32_S_CET
> 
>  pop eax
> 
>  pop edx
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index abf9f1a90a..881d3177f7 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -217,6 +217,11 @@ ASM_PFX(mPatchCetSupported):
>  pushrdx
> 
>  pushrax
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +rdmsr
> 
> +pushrdx
> 
> +pushrax
> 
> +
> 
>  mov ecx, MSR_IA32_PL0_SSP
> 
>  rdmsr
> 
>  pushrdx
> 
> @@ -227,6 +232,11 @@ ASM_PFX(mPatchCetSupported):
>  pushrdx
> 
>  pushrax
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +xor eax, eax
> 
> +xor edx, edx
> 
> +wrmsr
> 
> +
> 
>  mov ecx, MSR_IA32_S_CET
> 
>  mov eax, MSR_IA32_CET_SH_STK_EN
> 
>  xor edx, edx
> 
> @@ -325,6 +335,11 @@ mCetSupportedAbsAddr:
>  pop rdx
> 
>  wrmsr
> 
> 
> 
> +mov ecx, MSR_IA32_U_CET
> 
> +pop rax
> 
> +pop rdx
> 
> +wrmsr
> 
> +
> 
>  mov ecx, MSR_IA32_S_CET
> 
>  pop rax
> 
>  pop rdx
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111529): https://edk2.groups.io/g/devel/message/111529
Mute This Topic: https://groups.io/mt/102724276/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 4/6] UefiCpuPkg: Only change CR4.CET bit for enable and disable CET.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 4/6] UefiCpuPkg: Only change CR4.CET bit for enable and
> disable CET.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 10 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 10 +++---
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> index 6368982433..1da9afab97 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> @@ -237,7 +237,9 @@ CetInterruptDone:
>  bts ecx, 16 ; set WP
> 
>  mov cr0, ecx
> 
> 
> 
> -mov eax, 0x668 | CR4_CET
> 
> +; set CR4.CET bit for enable CET
> 
> +mov eax, cr4
> 
> +bts eax, CR4_CET_BIT
> 
>  mov cr4, eax
> 
> 
> 
>  setssbsy
> 
> @@ -264,8 +266,10 @@ CetDone:
>  cmp al, 0
> 
>  jz  CetDone2
> 
> 
> 
> -mov eax, 0x668
> 
> -mov cr4, eax   ; disable CET
> 
> +; clear CR4.CET bit for disable CET
> 
> +mov eax, cr4
> 
> +btr eax, CR4_CET_BIT
> 
> +mov cr4, eax
> 
> 
> 
>  mov ecx, MSR_IA32_PL0_SSP
> 
>  pop eax
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index 9a225bc3be..abf9f1a90a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -263,7 +263,9 @@ CetInterruptDone:
>  bts ecx, 16 ; set WP
> 
>  mov cr0, rcx
> 
> 
> 
> -mov eax, 0x668 | CR4_CET
> 
> +; set CR4.CET bit for enable CET
> 
> +mov rax, cr4
> 
> +bts rax, CR4_CET_BIT
> 
>  mov cr4, rax
> 
> 
> 
>  setssbsy
> 
> @@ -308,8 +310,10 @@ mCetSupportedAbsAddr:
>  cmp al, 0
> 
>  jz  CetDone2
> 
> 
> 
> -mov eax, 0x668
> 
> -mov cr4, rax   ; disable CET
> 
> +; clear CR4.CET bit for disable CET
> 
> +mov rax, cr4
> 
> +btr rax, CR4_CET_BIT
> 
> +mov cr4, rax
> 
> 
> 
>  mov ecx, MSR_IA32_INTERRUPT_SSP_TABLE_ADDR
> 
>  pop rax
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111528): https://edk2.groups.io/g/devel/message/111528
Mute This Topic: https://groups.io/mt/102724275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 3/6] UefiCpuPkg: Use CET macro definitions in Cet.inc for SmiEntry.nasm files.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 3/6] UefiCpuPkg: Use CET macro definitions in Cet.inc for
> SmiEntry.nasm files.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 14 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 15 +--
>  2 files changed, 2 insertions(+), 27 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> index 19de5f614e..6368982433 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> @@ -15,19 +15,7 @@
> 
> 
>  %include "StuffRsbNasm.inc"
> 
>  %include "Nasm.inc"
> 
> -
> 
> -%define MSR_IA32_S_CET 0x6A2
> 
> -%define   MSR_IA32_CET_SH_STK_EN 0x1
> 
> -%define   MSR_IA32_CET_WR_SHSTK_EN   0x2
> 
> -%define   MSR_IA32_CET_ENDBR_EN  0x4
> 
> -%define   MSR_IA32_CET_LEG_IW_EN 0x8
> 
> -%define   MSR_IA32_CET_NO_TRACK_EN   0x10
> 
> -%define   MSR_IA32_CET_SUPPRESS_DIS  0x20
> 
> -%define   MSR_IA32_CET_SUPPRESS  0x400
> 
> -%define   MSR_IA32_CET_TRACKER   0x800
> 
> -%define MSR_IA32_PL0_SSP   0x6A4
> 
> -
> 
> -%define CR4_CET0x80
> 
> +%include "Cet.inc"
> 
> 
> 
>  %define MSR_IA32_MISC_ENABLE 0x1A0
> 
>  %define MSR_EFER  0xc080
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index d302ca8d01..9a225bc3be 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -15,25 +15,12 @@
> 
> 
>  %include "StuffRsbNasm.inc"
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  ;
> 
>  ; Variables referenced by C code
> 
>  ;
> 
> 
> 
> -%define MSR_IA32_S_CET 0x6A2
> 
> -%define   MSR_IA32_CET_SH_STK_EN 0x1
> 
> -%define   MSR_IA32_CET_WR_SHSTK_EN   0x2
> 
> -%define   MSR_IA32_CET_ENDBR_EN  0x4
> 
> -%define   MSR_IA32_CET_LEG_IW_EN 0x8
> 
> -%define   MSR_IA32_CET_NO_TRACK_EN   0x10
> 
> -%define   MSR_IA32_CET_SUPPRESS_DIS  0x20
> 
> -%define   MSR_IA32_CET_SUPPRESS  0x400
> 
> -%define   MSR_IA32_CET_TRACKER   0x800
> 
> -%define MSR_IA32_PL0_SSP   0x6A4
> 
> -%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
> 
> -
> 
> -%define CR4_CET0x80
> 
> -
> 
>  %define MSR_IA32_MISC_ENABLE 0x1A0
> 
>  %define MSR_EFER  0xc080
> 
>  %define MSR_EFER_XD   0x800
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111527): https://edk2.groups.io/g/devel/message/111527
Mute This Topic: https://groups.io/mt/102724274/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 2/6] UefiCpuPkg: Use macro CR4_CET_BIT to replace hard code value in Cet.nasm.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 2/6] UefiCpuPkg: Use macro CR4_CET_BIT to replace
> hard code value in Cet.nasm.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm | 5 +++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm  | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
> index 9d66b9c5da..3d07da1cd4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
> @@ -5,6 +5,7 @@
>  
> ;---
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  SECTION .text
> 
> 
> 
> @@ -16,7 +17,7 @@ ASM_PFX(DisableCet):
>  incsspd eax
> 
> 
> 
>  mov eax, cr4
> 
> -btr eax, 23  ; clear CET
> 
> +btr eax, CR4_CET_BIT ; clear CET
> 
>  mov cr4, eax
> 
>  ret
> 
> 
> 
> @@ -24,7 +25,7 @@ global ASM_PFX(EnableCet)
>  ASM_PFX(EnableCet):
> 
> 
> 
>  mov eax, cr4
> 
> -bts eax, 23  ; set CET
> 
> +bts eax, CR4_CET_BIT ; set CET
> 
>  mov cr4, eax
> 
> 
> 
>  ; use jmp to skip the check for ret
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
> index 8bbdbb31cc..700aef4703 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
> @@ -5,6 +5,7 @@
>  
> ;---
> 
> 
> 
>  %include "Nasm.inc"
> 
> +%include "Cet.inc"
> 
> 
> 
>  DEFAULT REL
> 
>  SECTION .text
> 
> @@ -17,7 +18,7 @@ ASM_PFX(DisableCet):
>  incsspq rax
> 
> 
> 
>  mov rax, cr4
> 
> -btr eax, 23  ; clear CET
> 
> +btr eax, CR4_CET_BIT ; clear CET
> 
>  mov cr4, rax
> 
>  ret
> 
> 
> 
> @@ -25,7 +26,7 @@ global ASM_PFX(EnableCet)
>  ASM_PFX(EnableCet):
> 
> 
> 
>  mov rax, cr4
> 
> -bts eax, 23  ; set CET
> 
> +bts eax, CR4_CET_BIT ; set CET
> 
>  mov cr4, rax
> 
> 
> 
>  ; use jmp to skip the check for ret
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111526): https://edk2.groups.io/g/devel/message/111526
Mute This Topic: https://groups.io/mt/102724273/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 1/6] MdePkg: Add macro definitions for CET feature for NASM files.

2023-11-20 Thread Ni, Ray
You might need to drop Reviewed-by from Laszlo as the patch content is so 
different than the reviewed version.

Reviewed-by: Ray Ni 



Thanks,
Ray
> -Original Message-
> From: Sheng, W 
> Sent: Tuesday, November 21, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Wu, Jiaxin ; Tan, Dun
> 
> Subject: [PATCH v6 1/6] MdePkg: Add macro definitions for CET feature for
> NASM files.
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Wu Jiaxin 
> Cc: Tan Dun 
> Reviewed-by: Laszlo Ersek 
> ---
>  MdePkg/Include/Ia32/Cet.inc | 26 ++
>  MdePkg/Include/X64/Cet.inc  | 26 ++
>  2 files changed, 52 insertions(+)
>  create mode 100644 MdePkg/Include/Ia32/Cet.inc
>  create mode 100644 MdePkg/Include/X64/Cet.inc
> 
> diff --git a/MdePkg/Include/Ia32/Cet.inc b/MdePkg/Include/Ia32/Cet.inc
> new file mode 100644
> index 00..41c99988c9
> --- /dev/null
> +++ b/MdePkg/Include/Ia32/Cet.inc
> @@ -0,0 +1,26 @@
> +;--
> 
> +;
> 
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.
> 
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +;
> 
> +; Abstract:
> 
> +;
> 
> +;   This file provides macro definitions for CET feature for NASM files.
> 
> +;
> 
> +;--
> 
> +
> 
> +%define MSR_IA32_U_CET 0x6A0
> 
> +%define MSR_IA32_S_CET 0x6A2
> 
> +%define MSR_IA32_CET_SH_STK_EN (1<<0)
> 
> +%define MSR_IA32_CET_WR_SHSTK_EN   (1<<1)
> 
> +%define MSR_IA32_CET_ENDBR_EN  (1<<2)
> 
> +%define MSR_IA32_CET_LEG_IW_EN (1<<3)
> 
> +%define MSR_IA32_CET_NO_TRACK_EN   (1<<4)
> 
> +%define MSR_IA32_CET_SUPPRESS_DIS  (1<<5)
> 
> +%define MSR_IA32_CET_SUPPRESS  (1<<10)
> 
> +%define MSR_IA32_CET_TRACKER   (1<<11)
> 
> +%define MSR_IA32_PL0_SSP   0x6A4
> 
> +%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
> 
> +
> 
> +%define CR4_CET_BIT23
> 
> +%define CR4_CET(1< 
> diff --git a/MdePkg/Include/X64/Cet.inc b/MdePkg/Include/X64/Cet.inc
> new file mode 100644
> index 00..41c99988c9
> --- /dev/null
> +++ b/MdePkg/Include/X64/Cet.inc
> @@ -0,0 +1,26 @@
> +;--
> 
> +;
> 
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.
> 
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +;
> 
> +; Abstract:
> 
> +;
> 
> +;   This file provides macro definitions for CET feature for NASM files.
> 
> +;
> 
> +;--
> 
> +
> 
> +%define MSR_IA32_U_CET 0x6A0
> 
> +%define MSR_IA32_S_CET 0x6A2
> 
> +%define MSR_IA32_CET_SH_STK_EN (1<<0)
> 
> +%define MSR_IA32_CET_WR_SHSTK_EN   (1<<1)
> 
> +%define MSR_IA32_CET_ENDBR_EN  (1<<2)
> 
> +%define MSR_IA32_CET_LEG_IW_EN (1<<3)
> 
> +%define MSR_IA32_CET_NO_TRACK_EN   (1<<4)
> 
> +%define MSR_IA32_CET_SUPPRESS_DIS  (1<<5)
> 
> +%define MSR_IA32_CET_SUPPRESS  (1<<10)
> 
> +%define MSR_IA32_CET_TRACKER   (1<<11)
> 
> +%define MSR_IA32_PL0_SSP   0x6A4
> 
> +%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
> 
> +
> 
> +%define CR4_CET_BIT23
> 
> +%define CR4_CET(1< 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111525): https://edk2.groups.io/g/devel/message/111525
Mute This Topic: https://groups.io/mt/102724272/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V2 3/3] UefiCpuPkg/MpInitLib: Eliminate redundant microcode loading in DXE.

2023-11-20 Thread Yuanhao Xie
The DXE stage's Microcode loading process has been elimincated by:

1. Microcode HOB consumption in MP initialization within the DXE phase.
2. Restricting MicrocodeDetect to the PEI phase instead of DXE for BSP.
3. BSP now WakeUpAp only for synchronizing MTRR settings, with
Microcode loading no longer a part of this process.

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 54 
+++---
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index bb5d4188f0..0cf3520f9e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -434,7 +434,27 @@ ApFuncEnableX2Apic (
 }
 
 /**
-  Do sync on APs.
+  Sync BSP's MTRR table to AP during waking up APs.
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+ApMtrrSync (
+  IN OUT VOID  *Buffer
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+
+  CpuMpData = (CPU_MP_DATA *)Buffer;
+
+  //
+  // Sync BSP's MTRR table to AP
+  //
+  MtrrSetAllMtrrs (>MtrrTable);
+}
+
+/**
+  Do sync on APs, includes loading microcode, and sync MTRRs.
 
   @param[in, out] Buffer  Pointer to private data buffer.
 **/
@@ -2224,26 +2244,10 @@ MpInitLibInitialize (
 }
   }
 
-  if (!GetMicrocodePatchInfoFromHob (
- >MicrocodePatchAddress,
- >MicrocodePatchRegionSize
- ))
-  {
-//
-// The microcode patch information cache HOB does not exist, which means
-// the microcode patches data has not been loaded into memory yet
-//
-ShadowMicrocodeUpdatePatch (CpuMpData);
-  }
-
   //
-  // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
+  // Wakeup APs to do some AP initialize sync.
   //
   if (CpuMpData->CpuCount > 1) {
-//
-// Detect and apply Microcode on BSP
-//
-MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
 //
 // Store BSP's MTRR setting
 //
@@ -2256,8 +2260,20 @@ MpInitLibInitialize (
   // in DXE.
   //
   CpuMpData->InitFlag = ApInitReconfig;
-  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+  //
+  //  Wakeup APs to sync MTRR settings.
+  //  For the case the PEI and DXE are in different bit mode.
+  //  It is necessary to do the MTRRs syncing.
+  //
+  WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
 } else {
+  //
+  // Detect and apply Microcode on BSP
+  //
+  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
+  //
+  // Wakeup APs to do some AP initialize sync load microcode and Sync MTRRs
+  //
   WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 }
 
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111524): https://edk2.groups.io/g/devel/message/111524
Mute This Topic: https://groups.io/mt/102724548/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V2 1/3] UefiCpuPkg/MpInitLib:Repeat WakeUpAp of loading microcode,Syncing MTRR.

2023-11-20 Thread Yuanhao Xie
This patch replicates the WakeUpAp operation, encompassing both
microcode loading and MTRR synchronization, regardless of whether
MpHandOff is NULL.

The purpose of this patch is to enhance the review process.
The separation in this patch is aimed at facilitating a more
straightforward review, with the ultimate goal of eliminating the
microcode loading functionality for the second time Mp initialization.

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..e8ffb99874 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2256,9 +2256,11 @@ MpInitLibInitialize (
   // in DXE.
   //
   CpuMpData->InitFlag = ApInitReconfig;
+  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+} else {
+  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 }
 
-WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 //
 // Wait for all APs finished initialization
 //
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111522): https://edk2.groups.io/g/devel/message/111522
Mute This Topic: https://groups.io/mt/102724546/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V2 2/3] UefiCpuPkg/MpInitLib: Detect microcode and store MTRR when CpuCount > 1

2023-11-20 Thread Yuanhao Xie
Detect and apply Microcode on BSP, store BSP's MTRR setting only when
CpuCount > 1.

The purpose of this patch is to enhance the review process.
The separation in this patch is aimed at facilitating a more
straightforward review, with the ultimate goal of eliminating the
microcode loading functionality for the second time Mp initialization

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e8ffb99874..bb5d4188f0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2236,19 +2236,19 @@ MpInitLibInitialize (
 ShadowMicrocodeUpdatePatch (CpuMpData);
   }
 
-  //
-  // Detect and apply Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
-  //
-  // Store BSP's MTRR setting
-  //
-  MtrrGetAllMtrrs (>MtrrTable);
-
   //
   // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
   //
   if (CpuMpData->CpuCount > 1) {
+//
+// Detect and apply Microcode on BSP
+//
+MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
+//
+// Store BSP's MTRR setting
+//
+MtrrGetAllMtrrs (>MtrrTable);
+
 if (MpHandOff != NULL) {
   //
   // Only needs to use this flag for DXE phase to update the wake up
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111523): https://edk2.groups.io/g/devel/message/111523
Mute This Topic: https://groups.io/mt/102724547/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V2 0/3] Eliminate redundant microcode loading in DXE.

2023-11-20 Thread Yuanhao Xie
The DXE stage's Microcode loading process has been elimincated.
Compare to V1, V2 seperates the patches and adds more comments

xieyuanh (3):
  UefiCpuPkg/MpInitLib:Repeat WakeUpAp of loading microcode,Syncing
MTRR.
  UefiCpuPkg/MpInitLib: Detect microcode and store MTRR when CpuCount >
1
  UefiCpuPkg/MpInitLib: Eliminate redundant microcode loading in DXE.

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 66 
++
 1 file changed, 42 insertions(+), 24 deletions(-)

--
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111521): https://edk2.groups.io/g/devel/message/111521
Mute This Topic: https://groups.io/mt/102724544/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 6/6] MdePkg: Use macro CR4_CET_BIT to replace hard code value.

2023-11-20 Thread Sheng Wei
The macro is used in file LongJump.nasm and SetJump.nasm.

Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 MdePkg/Library/BaseLib/Ia32/LongJump.nasm | 3 ++-
 MdePkg/Library/BaseLib/Ia32/SetJump.nasm  | 3 ++-
 MdePkg/Library/BaseLib/X64/LongJump.nasm  | 3 ++-
 MdePkg/Library/BaseLib/X64/SetJump.nasm   | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseLib/Ia32/LongJump.nasm 
b/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
index 6c13dfe307..df1bf9749e 100644
--- a/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/LongJump.nasm
@@ -14,6 +14,7 @@
 ;--
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 SECTION .text
 
@@ -34,7 +35,7 @@ ASM_PFX(InternalLongJump):
 testeax, eax
 jz  CetDone
 mov eax, cr4
-bt  eax, 23; check if CET is enabled
+bt  eax, CR4_CET_BIT   ; check if CET is enabled
 jnc CetDone
 
 mov edx, [esp + 4] ; edx = JumpBuffer
diff --git a/MdePkg/Library/BaseLib/Ia32/SetJump.nasm 
b/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
index 2577373241..0c484f6852 100644
--- a/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
+++ b/MdePkg/Library/BaseLib/Ia32/SetJump.nasm
@@ -14,6 +14,7 @@
 ;--
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 SECTION .text
 
@@ -42,7 +43,7 @@ ASM_PFX(SetJump):
 testeax, eax
 jz  CetDone
 mov eax, cr4
-bt  eax, 23; check if CET is enabled
+bt  eax, CR4_CET_BIT   ; check if CET is enabled
 jnc CetDone
 
 mov eax, 1
diff --git a/MdePkg/Library/BaseLib/X64/LongJump.nasm 
b/MdePkg/Library/BaseLib/X64/LongJump.nasm
index 2002f65cba..021b49e855 100644
--- a/MdePkg/Library/BaseLib/X64/LongJump.nasm
+++ b/MdePkg/Library/BaseLib/X64/LongJump.nasm
@@ -14,6 +14,7 @@
 ;--
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 DEFAULT REL
 SECTION .text
@@ -35,7 +36,7 @@ ASM_PFX(InternalLongJump):
 testeax, eax
 jz  CetDone
 mov rax, cr4
-bt  eax, 23  ; check if CET is enabled
+bt  eax, CR4_CET_BIT ; check if CET is enabled
 jnc CetDone
 
 pushrdx  ; save rdx
diff --git a/MdePkg/Library/BaseLib/X64/SetJump.nasm 
b/MdePkg/Library/BaseLib/X64/SetJump.nasm
index 5943a5ebe5..d2c0991e66 100644
--- a/MdePkg/Library/BaseLib/X64/SetJump.nasm
+++ b/MdePkg/Library/BaseLib/X64/SetJump.nasm
@@ -14,6 +14,7 @@
 ;--
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 DEFAULT REL
 SECTION .text
@@ -44,7 +45,7 @@ ASM_PFX(SetJump):
 testeax, eax
 jz  CetDone
 mov rax, cr4
-bt  eax, 23  ; check if CET is enabled
+bt  eax, CR4_CET_BIT ; check if CET is enabled
 jnc CetDone
 
 mov rax, 1
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111520): https://edk2.groups.io/g/devel/message/111520
Mute This Topic: https://groups.io/mt/102724277/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 5/6] UefiCpuPkg: Backup and Restore MSR IA32_U_CET in SMI handler.

2023-11-20 Thread Sheng Wei
OS may enable CET-IBT feature by set MSR IA32_U_CET.bit2.
If IA32_U_CET.bit2 is set, CPU is in WAIT_FOR_ENDBRANCH state and
 the next assemble code is not ENDBR, it will trigger #CP exception
 when set CR4.CET bit.
SMI handler needs to backup MSR IA32_U_CET and clear MSR IA32_U_CET
 before set CR4.CET bit,
And SMI handler needs to restore MSR IA32_U_CET when exit SMI handler.

Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 15 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 15 +++
 2 files changed, 30 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 1da9afab97..9e1155dee6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -202,11 +202,21 @@ ASM_PFX(mPatchCetSupported):
 pushedx
 pusheax
 
+mov ecx, MSR_IA32_U_CET
+rdmsr
+pushedx
+pusheax
+
 mov ecx, MSR_IA32_PL0_SSP
 rdmsr
 pushedx
 pusheax
 
+mov ecx, MSR_IA32_U_CET
+xor eax, eax
+xor edx, edx
+wrmsr
+
 mov ecx, MSR_IA32_S_CET
 mov eax, MSR_IA32_CET_SH_STK_EN
 xor edx, edx
@@ -276,6 +286,11 @@ CetDone:
 pop edx
 wrmsr
 
+mov ecx, MSR_IA32_U_CET
+pop eax
+pop edx
+wrmsr
+
 mov ecx, MSR_IA32_S_CET
 pop eax
 pop edx
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index abf9f1a90a..881d3177f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -217,6 +217,11 @@ ASM_PFX(mPatchCetSupported):
 pushrdx
 pushrax
 
+mov ecx, MSR_IA32_U_CET
+rdmsr
+pushrdx
+pushrax
+
 mov ecx, MSR_IA32_PL0_SSP
 rdmsr
 pushrdx
@@ -227,6 +232,11 @@ ASM_PFX(mPatchCetSupported):
 pushrdx
 pushrax
 
+mov ecx, MSR_IA32_U_CET
+xor eax, eax
+xor edx, edx
+wrmsr
+
 mov ecx, MSR_IA32_S_CET
 mov eax, MSR_IA32_CET_SH_STK_EN
 xor edx, edx
@@ -325,6 +335,11 @@ mCetSupportedAbsAddr:
 pop rdx
 wrmsr
 
+mov ecx, MSR_IA32_U_CET
+pop rax
+pop rdx
+wrmsr
+
 mov ecx, MSR_IA32_S_CET
 pop rax
 pop rdx
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111519): https://edk2.groups.io/g/devel/message/111519
Mute This Topic: https://groups.io/mt/102724276/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 4/6] UefiCpuPkg: Only change CR4.CET bit for enable and disable CET.

2023-11-20 Thread Sheng Wei
Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 10 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 10 +++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 6368982433..1da9afab97 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -237,7 +237,9 @@ CetInterruptDone:
 bts ecx, 16 ; set WP
 mov cr0, ecx
 
-mov eax, 0x668 | CR4_CET
+; set CR4.CET bit for enable CET
+mov eax, cr4
+bts eax, CR4_CET_BIT
 mov cr4, eax
 
 setssbsy
@@ -264,8 +266,10 @@ CetDone:
 cmp al, 0
 jz  CetDone2
 
-mov eax, 0x668
-mov cr4, eax   ; disable CET
+; clear CR4.CET bit for disable CET
+mov eax, cr4
+btr eax, CR4_CET_BIT
+mov cr4, eax
 
 mov ecx, MSR_IA32_PL0_SSP
 pop eax
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index 9a225bc3be..abf9f1a90a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -263,7 +263,9 @@ CetInterruptDone:
 bts ecx, 16 ; set WP
 mov cr0, rcx
 
-mov eax, 0x668 | CR4_CET
+; set CR4.CET bit for enable CET
+mov rax, cr4
+bts rax, CR4_CET_BIT
 mov cr4, rax
 
 setssbsy
@@ -308,8 +310,10 @@ mCetSupportedAbsAddr:
 cmp al, 0
 jz  CetDone2
 
-mov eax, 0x668
-mov cr4, rax   ; disable CET
+; clear CR4.CET bit for disable CET
+mov rax, cr4
+btr rax, CR4_CET_BIT
+mov cr4, rax
 
 mov ecx, MSR_IA32_INTERRUPT_SSP_TABLE_ADDR
 pop rax
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111518): https://edk2.groups.io/g/devel/message/111518
Mute This Topic: https://groups.io/mt/102724275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 3/6] UefiCpuPkg: Use CET macro definitions in Cet.inc for SmiEntry.nasm files.

2023-11-20 Thread Sheng Wei
Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 14 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 15 +--
 2 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 19de5f614e..6368982433 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -15,19 +15,7 @@
 
 %include "StuffRsbNasm.inc"
 %include "Nasm.inc"
-
-%define MSR_IA32_S_CET 0x6A2
-%define   MSR_IA32_CET_SH_STK_EN 0x1
-%define   MSR_IA32_CET_WR_SHSTK_EN   0x2
-%define   MSR_IA32_CET_ENDBR_EN  0x4
-%define   MSR_IA32_CET_LEG_IW_EN 0x8
-%define   MSR_IA32_CET_NO_TRACK_EN   0x10
-%define   MSR_IA32_CET_SUPPRESS_DIS  0x20
-%define   MSR_IA32_CET_SUPPRESS  0x400
-%define   MSR_IA32_CET_TRACKER   0x800
-%define MSR_IA32_PL0_SSP   0x6A4
-
-%define CR4_CET0x80
+%include "Cet.inc"
 
 %define MSR_IA32_MISC_ENABLE 0x1A0
 %define MSR_EFER  0xc080
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index d302ca8d01..9a225bc3be 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -15,25 +15,12 @@
 
 %include "StuffRsbNasm.inc"
 %include "Nasm.inc"
+%include "Cet.inc"
 
 ;
 ; Variables referenced by C code
 ;
 
-%define MSR_IA32_S_CET 0x6A2
-%define   MSR_IA32_CET_SH_STK_EN 0x1
-%define   MSR_IA32_CET_WR_SHSTK_EN   0x2
-%define   MSR_IA32_CET_ENDBR_EN  0x4
-%define   MSR_IA32_CET_LEG_IW_EN 0x8
-%define   MSR_IA32_CET_NO_TRACK_EN   0x10
-%define   MSR_IA32_CET_SUPPRESS_DIS  0x20
-%define   MSR_IA32_CET_SUPPRESS  0x400
-%define   MSR_IA32_CET_TRACKER   0x800
-%define MSR_IA32_PL0_SSP   0x6A4
-%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
-
-%define CR4_CET0x80
-
 %define MSR_IA32_MISC_ENABLE 0x1A0
 %define MSR_EFER  0xc080
 %define MSR_EFER_XD   0x800
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111517): https://edk2.groups.io/g/devel/message/111517
Mute This Topic: https://groups.io/mt/102724274/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 2/6] UefiCpuPkg: Use macro CR4_CET_BIT to replace hard code value in Cet.nasm.

2023-11-20 Thread Sheng Wei
Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm | 5 +++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm  | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
index 9d66b9c5da..3d07da1cd4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm
@@ -5,6 +5,7 @@
 
;---
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 SECTION .text
 
@@ -16,7 +17,7 @@ ASM_PFX(DisableCet):
 incsspd eax
 
 mov eax, cr4
-btr eax, 23  ; clear CET
+btr eax, CR4_CET_BIT ; clear CET
 mov cr4, eax
 ret
 
@@ -24,7 +25,7 @@ global ASM_PFX(EnableCet)
 ASM_PFX(EnableCet):
 
 mov eax, cr4
-bts eax, 23  ; set CET
+bts eax, CR4_CET_BIT ; set CET
 mov cr4, eax
 
 ; use jmp to skip the check for ret
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
index 8bbdbb31cc..700aef4703 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm
@@ -5,6 +5,7 @@
 
;---
 
 %include "Nasm.inc"
+%include "Cet.inc"
 
 DEFAULT REL
 SECTION .text
@@ -17,7 +18,7 @@ ASM_PFX(DisableCet):
 incsspq rax
 
 mov rax, cr4
-btr eax, 23  ; clear CET
+btr eax, CR4_CET_BIT ; clear CET
 mov cr4, rax
 ret
 
@@ -25,7 +26,7 @@ global ASM_PFX(EnableCet)
 ASM_PFX(EnableCet):
 
 mov rax, cr4
-bts eax, 23  ; set CET
+bts eax, CR4_CET_BIT ; set CET
 mov cr4, rax
 
 ; use jmp to skip the check for ret
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111516): https://edk2.groups.io/g/devel/message/111516
Mute This Topic: https://groups.io/mt/102724273/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v6 1/6] MdePkg: Add macro definitions for CET feature for NASM files.

2023-11-20 Thread Sheng Wei
Signed-off-by: Sheng Wei 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Wu Jiaxin 
Cc: Tan Dun 
Reviewed-by: Laszlo Ersek 
---
 MdePkg/Include/Ia32/Cet.inc | 26 ++
 MdePkg/Include/X64/Cet.inc  | 26 ++
 2 files changed, 52 insertions(+)
 create mode 100644 MdePkg/Include/Ia32/Cet.inc
 create mode 100644 MdePkg/Include/X64/Cet.inc

diff --git a/MdePkg/Include/Ia32/Cet.inc b/MdePkg/Include/Ia32/Cet.inc
new file mode 100644
index 00..41c99988c9
--- /dev/null
+++ b/MdePkg/Include/Ia32/Cet.inc
@@ -0,0 +1,26 @@
+;--
+;
+; Copyright (c) 2023, Intel Corporation. All rights reserved.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Abstract:
+;
+;   This file provides macro definitions for CET feature for NASM files.
+;
+;--
+
+%define MSR_IA32_U_CET 0x6A0
+%define MSR_IA32_S_CET 0x6A2
+%define MSR_IA32_CET_SH_STK_EN (1<<0)
+%define MSR_IA32_CET_WR_SHSTK_EN   (1<<1)
+%define MSR_IA32_CET_ENDBR_EN  (1<<2)
+%define MSR_IA32_CET_LEG_IW_EN (1<<3)
+%define MSR_IA32_CET_NO_TRACK_EN   (1<<4)
+%define MSR_IA32_CET_SUPPRESS_DIS  (1<<5)
+%define MSR_IA32_CET_SUPPRESS  (1<<10)
+%define MSR_IA32_CET_TRACKER   (1<<11)
+%define MSR_IA32_PL0_SSP   0x6A4
+%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
+
+%define CR4_CET_BIT23
+%define CR4_CET(1<
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Abstract:
+;
+;   This file provides macro definitions for CET feature for NASM files.
+;
+;--
+
+%define MSR_IA32_U_CET 0x6A0
+%define MSR_IA32_S_CET 0x6A2
+%define MSR_IA32_CET_SH_STK_EN (1<<0)
+%define MSR_IA32_CET_WR_SHSTK_EN   (1<<1)
+%define MSR_IA32_CET_ENDBR_EN  (1<<2)
+%define MSR_IA32_CET_LEG_IW_EN (1<<3)
+%define MSR_IA32_CET_NO_TRACK_EN   (1<<4)
+%define MSR_IA32_CET_SUPPRESS_DIS  (1<<5)
+%define MSR_IA32_CET_SUPPRESS  (1<<10)
+%define MSR_IA32_CET_TRACKER   (1<<11)
+%define MSR_IA32_PL0_SSP   0x6A4
+%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
+
+%define CR4_CET_BIT23
+%define CR4_CET(1

[edk2-devel] [PATCH v6 0/6] MdePkg: Add macro definitions for CET feature for NASM files.

2023-11-20 Thread Sheng Wei
Patch V6:
  Cet.inc only contains definitions for x86 CPU.
  Move the file to \Ia32 and \X64 folder.
  Refine code for cet.inc.

Patch V5:
  File cet.inc will be used in both MdePkg UefiCpuPkg.
  Move cet.inc file from UefiCpuPkg to MdePkg.
  Use macro CR4_CET_BIT to replace hard code value for
   both LongJump.nasm and SetJump.nasm.

Patch V4:
  Separate the changes to 5 patches.
1) Add macro definitions for CET feature for NASM files.
2) Use macro CR4_CET_BIT to replace hard code value in Cet.nasm.
3) Use CET macro definitions in Cet.inc for SmiEntry.nasm files.
4) Only change CR4.CET bit for enable/disable CET.
5) Backup and Restore MSR IA32_U_CET in SMI handler.
  Remove some unused code.
It is no need to clear MSR IA32_S_CET,
 because clear CR4.CET bit will disable all CET functions.
Since CET is disabled between clear CR4.CET and run 'rsm',
 it is no need to delay MSR IA32_S_CET restoration.

Patch V3:
  Remove the 3rd patch. mSmmInterruptSspTables is a global variable.
  It is unnecessary to initializ it to zero manually.

Patch V2:
  No function change with Patch V1.
  Split the patch to into 3 separate patches.


Sheng Wei (6):
  MdePkg: Add macro definitions for CET feature for NASM files.
  UefiCpuPkg: Use macro CR4_CET_BIT to replace hard code value in
Cet.nasm.
  UefiCpuPkg: Use CET macro definitions in Cet.inc for SmiEntry.nasm
files.
  UefiCpuPkg: Only change CR4.CET bit for enable and disable CET.
  UefiCpuPkg: Backup and Restore MSR IA32_U_CET in SMI handler.
  MdePkg: Use macro CR4_CET_BIT to replace hard code value.

 MdePkg/Include/Ia32/Cet.inc  | 26 +
 MdePkg/Include/X64/Cet.inc   | 26 +
 MdePkg/Library/BaseLib/Ia32/LongJump.nasm|  3 +-
 MdePkg/Library/BaseLib/Ia32/SetJump.nasm |  3 +-
 MdePkg/Library/BaseLib/X64/LongJump.nasm |  3 +-
 MdePkg/Library/BaseLib/X64/SetJump.nasm  |  3 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm  |  5 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 39 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Cet.nasm   |  5 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 40 +++-
 10 files changed, 112 insertions(+), 41 deletions(-)
 create mode 100644 MdePkg/Include/Ia32/Cet.inc
 create mode 100644 MdePkg/Include/X64/Cet.inc

-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111514): https://edk2.groups.io/g/devel/message/111514
Mute This Topic: https://groups.io/mt/102724271/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v3 27/39] ArmVirtPkg: Move PlatformBootManagerLib to OvmfPkg

2023-11-20 Thread Chao Li

This patch accidentally breaks some platforms, I will fix them in V4.


Thanks,
Chao
On 2023/11/17 18:02, Chao Li wrote:

Moved the PlatformBootManagerLib to OvmfPkg and renamed to
PlatformBootManagerLibLight for easy use by other ARCH.

Build-tested only (with "ArmVirtQemu.dsc").

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

Cc: Ard Biesheuvel
Cc: Leif Lindholm
Cc: Sami Mujawar
Cc: Gerd Hoffmann
Cc: Jiewen Yao
Cc: Lazlo Ersek
Signed-off-by: Chao Li
---
  ArmPkg/ArmPkg.dsc   | 1 -
  ArmVirtPkg/ArmVirtCloudHv.dsc   | 2 +-
  ArmVirtPkg/ArmVirtKvmTool.dsc   | 2 +-
  ArmVirtPkg/ArmVirtQemu.dsc  | 2 +-
  ArmVirtPkg/ArmVirtQemuKernel.dsc| 2 +-
  ArmVirtPkg/ArmVirtXen.dsc   | 2 +-
  .../Library/PlatformBootManagerLibLight}/PlatformBm.c   | 0
  .../Library/PlatformBootManagerLibLight}/PlatformBm.h   | 0
  .../PlatformBootManagerLibLight}/PlatformBootManagerLib.inf | 2 +-
  .../Library/PlatformBootManagerLibLight}/QemuKernel.c   | 0
  10 files changed, 6 insertions(+), 7 deletions(-)
  rename {ArmVirtPkg/Library/PlatformBootManagerLib => 
OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBm.c (100%)
  rename {ArmVirtPkg/Library/PlatformBootManagerLib => 
OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBm.h (100%)
  rename {ArmVirtPkg/Library/PlatformBootManagerLib => 
OvmfPkg/Library/PlatformBootManagerLibLight}/PlatformBootManagerLib.inf (93%)
  rename {ArmVirtPkg/Library/PlatformBootManagerLib => 
OvmfPkg/Library/PlatformBootManagerLibLight}/QemuKernel.c (100%)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 7af25a91a1..f0667c72f8 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -151,7 +151,6 @@
ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
-  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
ArmPkg/Library/LinuxBootBootManagerLib/LinuxBootBootManagerLib.inf
  
ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index 0f80fb34cc..aeed77ffcb 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -43,7 +43,7 @@
TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  
PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf

PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 31d5bc13cf..e2b90e000f 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -56,7 +56,7 @@
  
# BDS Libraries


UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
-  
PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf
BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
  
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 6b2b4d1086..ac980a2736 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -70,7 +70,7 @@
  
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  
PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  
PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibLight/PlatformBootManagerLib.inf

PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf

CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 34fdf5d5a9..b8867ea6fe 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -69,7 +69,7 @@
  
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  

Re: [edk2-devel] [PATCH v5 1/6] MdePkg: Add macro definitions for CET feature for NASM files.

2023-11-20 Thread Ni, Ray


> ---
>  MdePkg/Include/Cet.inc | 26 ++

1. I am not sure adding "Cet.inc" in MdePkg/Include is ok as Cet.inc only 
contains definitions for x86 CPU.
  Is it better to add the macros to existing Ia32/Nasm.inc, X64/Nasm.inc?
  But that would cause duplicated macro definitions in the two INC files.
  So I am not sure which way is better.
  @Kinney, Michael D, comments?


> 
> +%define MSR_IA32_U_CET 0x6A0
> 
> +%define MSR_IA32_S_CET 0x6A2
> 
> +%define MSR_IA32_CET_SH_STK_EN 0x1

2. can you use "<<" such as:
%define MSR_IA32_CET_SH_STK_EN (1<<0)

> 
> +%define MSR_IA32_CET_WR_SHSTK_EN   0x2
> 
> +%define MSR_IA32_CET_ENDBR_EN  0x4
> 
> +%define MSR_IA32_CET_LEG_IW_EN 0x8
> 
> +%define MSR_IA32_CET_NO_TRACK_EN   0x10
> 
> +%define MSR_IA32_CET_SUPPRESS_DIS  0x20
> 
> +%define MSR_IA32_CET_SUPPRESS  0x400
> 
> +%define MSR_IA32_CET_TRACKER   0x800
> 
> +%define MSR_IA32_PL0_SSP   0x6A4
> 
> +%define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8
> 
> +
> 
> +%define CR4_CET_BIT23
> 
> +%define CR4_CET0x80

3. %define CR4_CET   (1< 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111512): https://edk2.groups.io/g/devel/message/111512
Mute This Topic: https://groups.io/mt/102599349/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4 installed after RestEx

2023-11-20 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

Hi Igor,
You can update the commit message with this BZ# information in this commit and 
resend the PR.
https://github.com/tianocore/edk2/pull/5052/commits/45fbed1e540f902ee21eebeb0752ba24911e445e
Also, let Liming knows you have this update.

Thanks
Abner

From: Igor Kulchytskyy 
Sent: Tuesday, November 21, 2023 11:04 AM
To: Chang, Abner ; Laszlo Ersek ; 
devel@edk2.groups.io
Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the 
issue if IPv4 installed after RestEx


[AMD Official Use Only - General]

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

Hello,
I created a ticket.
https://bugzilla.tianocore.org/show_bug.cgi?id=4602
Since it is my first ticket, could you please check it and tell if it is what 
we need?
Thank you,
Igor

From: Igor Kulchytskyy mailto:ig...@ami.com>>
Sent: Monday, November 20, 2023 8:50 AM
To: Chang, Abner mailto:abner.ch...@amd.com>>; Laszlo 
Ersek mailto:ler...@redhat.com>>; 
devel@edk2.groups.io
Subject: Re: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the 
issue if IPv4 installed after RestEx

Hi Abner,
I have an account.
I will create a ticket.
Thank you,
Igor

Get Outlook for Android

From: Chang, Abner mailto:abner.ch...@amd.com>>
Sent: Sunday, November 19, 2023 11:31:37 PM
To: Laszlo Ersek mailto:ler...@redhat.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Igor Kulchytskyy 
mailto:ig...@ami.com>>
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue 
if IPv4 installed after RestEx


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

[AMD Official Use Only - General]

Yes Laszlo, we need one for this.
Hi Igor, do you have an account on Bugzilla? We need a ticket for this issue.

Thnaks
Abner

> -Original Message-
> From: Laszlo Ersek mailto:ler...@redhat.com>>
> Sent: Friday, November 17, 2023 5:11 PM
> To: devel@edk2.groups.io; 
> ig...@ami.com; Chang, Abner
> mailto:abner.ch...@amd.com>>
> Subject: Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4
> installed after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 11/15/23 23:12, Igor Kulchytskyy via groups.io wrote:
> > Igor Kulchytskyy (2):
> >   RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after
> > RestEx
> >   RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow
> >
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 225 --
> >  .../RedfishDiscoverInternal.h |   4 +
> >  2 files changed, 158 insertions(+), 71 deletions(-)
>
> Should this patch series be highlighted in the release plan? Is there a BZ?
>
> Laszlo
-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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111511): https://edk2.groups.io/g/devel/message/111511
Mute This Topic: https://groups.io/mt/102615650/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4 installed after RestEx

2023-11-20 Thread Igor Kulchytskyy via groups.io
Hello,
I created a ticket.
https://bugzilla.tianocore.org/show_bug.cgi?id=4602
Since it is my first ticket, could you please check it and tell if it is what 
we need?
Thank you,
Igor

From: Igor Kulchytskyy 
Sent: Monday, November 20, 2023 8:50 AM
To: Chang, Abner ; Laszlo Ersek ; 
devel@edk2.groups.io
Subject: Re: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the 
issue if IPv4 installed after RestEx

Hi Abner,
I have an account.
I will create a ticket.
Thank you,
Igor

Get Outlook for Android

From: Chang, Abner mailto:abner.ch...@amd.com>>
Sent: Sunday, November 19, 2023 11:31:37 PM
To: Laszlo Ersek mailto:ler...@redhat.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Igor Kulchytskyy 
mailto:ig...@ami.com>>
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue 
if IPv4 installed after RestEx


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

[AMD Official Use Only - General]

Yes Laszlo, we need one for this.
Hi Igor, do you have an account on Bugzilla? We need a ticket for this issue.

Thnaks
Abner

> -Original Message-
> From: Laszlo Ersek mailto:ler...@redhat.com>>
> Sent: Friday, November 17, 2023 5:11 PM
> To: devel@edk2.groups.io; 
> ig...@ami.com; Chang, Abner
> mailto:abner.ch...@amd.com>>
> Subject: Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4
> installed after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 11/15/23 23:12, Igor Kulchytskyy via groups.io wrote:
> > Igor Kulchytskyy (2):
> >   RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after
> > RestEx
> >   RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow
> >
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 225 --
> >  .../RedfishDiscoverInternal.h |   4 +
> >  2 files changed, 158 insertions(+), 71 deletions(-)
>
> Should this patch series be highlighted in the release plan? Is there a BZ?
>
> Laszlo
-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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111510): https://edk2.groups.io/g/devel/message/111510
Mute This Topic: https://groups.io/mt/102615650/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 2/2] UefiCpuPkg/MpInitLib: Update the comments of _CPU_MP_DATA.

2023-11-20 Thread Yuanhao Xie
No functional changes in this patch.

Updates the comments of _CPU_MP_DATA to delcared that duplications in
CpuMpData are present to avoid to be direct accessed and comprehended
 in assembly code. CpuMpData: Intended for use in C code while
 ExchangeInfo are used in assembly code in this module.

This patch deletes the unnecessary comments in CpuMpData, since
CpuMpData is no longer responsible for passing information from PEI to
DXE.

Signed-off-by: Yuanhao Xie 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc |  2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.h   | 13 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc 
b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 72af196513..317e627b58 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -67,6 +67,8 @@ endstruc
 
 ;
 ; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
+; Assembly routines should refrain from directly interacting with
+; the internal details of CPU_MP_DATA.
 ;
 struc MP_CPU_EXCHANGE_INFO
   .StackStart:   CTYPE_UINTN 1
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index af296f6ac0..a96a6389c1 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -203,6 +203,8 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA;
 // MP CPU exchange information for AP reset code
 // This structure is required to be packed because fixed field offsets
 // into this structure are used in assembly code in this module
+// Assembly routines should refrain from directly interacting with
+// the internal details of CPU_MP_DATA.
 //
 typedef struct {
   UINTN  StackStart;
@@ -239,17 +241,16 @@ typedef struct {
 #pragma pack()
 
 //
-// CPU MP Data save in memory
+// CPU MP Data save in memory, and intended for use in C code.
+// There are some duplicated fields, such as XD status, between
+// CpuMpData and ExchangeInfo. These duplications in CpuMpData
+// are present to avoid to be direct accessed and comprehended
+// in assembly code.
 //
 struct _CPU_MP_DATA {
   UINT64   CpuInfoInHob;
   UINT32   CpuCount;
   UINT32   BspNumber;
-  //
-  // The above fields data will be passed from PEI to DXE
-  // Please make sure the fields offset same in the different
-  // architecture.
-  //
   SPIN_LOCKMpLock;
   UINTNBuffer;
   UINTNCpuApStackSize;
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111509): https://edk2.groups.io/g/devel/message/111509
Mute This Topic: https://groups.io/mt/102721681/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 1/2] UefiCpuPkg/MpInitLib: Enable execute disable bit.

2023-11-20 Thread Yuanhao Xie
From: Yuanhao Xie 

This patch synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.

The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
calling the SwitchApContext function to initiate a specialized start-up
signal, waking up APs in the DXE instead of using INIT-SIPI-SIPI.

Due to this change, the logic for "Enable execute disable bit" in
MpFuncs.nasm is no longer executed. However, to ensure the proper setup
of the page table, it is necessary to synchronize the IA32_EFER.NXE for
APs before executing RestoreVolatileRegisters .

Based on SDM:
If IA32_EFER.NXE is set to 1, it signifies execute-disable, meaning
instruction fetches are not allowed from the 4-KByte page controlled by
this entry. Conversely, if it is set to 0, it is reserved.

Signed-off-by: Yuanhao Xie 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Ray Ni 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..f29e66a14f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -910,9 +910,16 @@ DxeApEntryPoint (
   CPU_MP_DATA  *CpuMpData
   )
 {
-  UINTN  ProcessorNumber;
+  UINTN   ProcessorNumber;
+  MSR_IA32_EFER_REGISTER  EferMsr;
 
   GetProcessorNumber (CpuMpData, );
+  if (CpuMpData->EnableExecuteDisableForSwitchContext) {
+EferMsr.Uint64   = AsmReadMsr64 (MSR_IA32_EFER);
+EferMsr.Bits.NXE = 1;
+AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
+  }
+
   RestoreVolatileRegisters (>CpuData[0].VolatileRegisters, FALSE);
   InterlockedIncrement ((UINT32 *)>FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
@@ -2188,8 +2195,9 @@ MpInitLibInitialize (
 if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
   ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
 
-  CpuMpData->FinishedCount = 0;
-  CpuMpData->InitFlag  = ApInitDone;
+  CpuMpData->FinishedCount= 0;
+  CpuMpData->InitFlag = ApInitDone;
+  CpuMpData->EnableExecuteDisableForSwitchContext = 
IsBspExecuteDisableEnabled ();
   SaveCpuMpData (CpuMpData);
   //
   // In scenarios where both the PEI and DXE phases run in the same
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 763db4963d..af296f6ac0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -270,6 +270,7 @@ struct _CPU_MP_DATA {
   UINT64   TotalTime;
   EFI_EVENTWaitEvent;
   UINTN**FailedCpuList;
+  BOOLEAN  EnableExecuteDisableForSwitchContext;
 
   AP_INIT_STATEInitFlag;
   BOOLEAN  SwitchBspFlag;
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111508): https://edk2.groups.io/g/devel/message/111508
Mute This Topic: https://groups.io/mt/102721680/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 0/2]UefiCpuPkg/MpInitLib: Enable execute disable bit.

2023-11-20 Thread Yuanhao Xie
This patch series synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.
It also updated the comments of _CPU_MP_DATA to delcared that
duplications in CpuMpData are present to avoid to be direct accessed
and comprehended in assembly code.

It updated the structure comments compared with V6.

Yuanhao Xie (1):
  UefiCpuPkg/MpInitLib: Enable execute disable bit.

xieyuanh (1):
  UefiCpuPkg/MpInitLib: Update the comments of _CPU_MP_DATA.

 UefiCpuPkg/Library/MpInitLib/MpEqu.inc |  2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c   | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h   | 14 --
 3 files changed, 21 insertions(+), 9 deletions(-)

--
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111507): https://edk2.groups.io/g/devel/message/111507
Mute This Topic: https://groups.io/mt/102721660/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 2/2] UefiCpuPkg/MpInitLib: Update the comments of _CPU_MP_DATA.

2023-11-20 Thread Yuanhao Xie
No functional changes in this patch.

Updates the comments of _CPU_MP_DATA to delcared that duplications in
CpuMpData are present to avoid to be direct accessed and comprehended
 in assembly code. CpuMpData: Intended for use in C code while
 ExchangeInfo are used in assembly code in this module.

This patch deletes the unnecessary comments in CpuMpData, since
CpuMpData is no longer responsible for passing information from PEI to
DXE.

Signed-off-by: Yuanhao Xie 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc |  2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.h   | 13 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc 
b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 72af196513..317e627b58 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -67,6 +67,8 @@ endstruc
 
 ;
 ; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
+; Assembly routines should refrain from directly interacting with
+; the internal details of CPU_MP_DATA.
 ;
 struc MP_CPU_EXCHANGE_INFO
   .StackStart:   CTYPE_UINTN 1
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index af296f6ac0..a96a6389c1 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -203,6 +203,8 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA;
 // MP CPU exchange information for AP reset code
 // This structure is required to be packed because fixed field offsets
 // into this structure are used in assembly code in this module
+// Assembly routines should refrain from directly interacting with
+// the internal details of CPU_MP_DATA.
 //
 typedef struct {
   UINTN  StackStart;
@@ -239,17 +241,16 @@ typedef struct {
 #pragma pack()
 
 //
-// CPU MP Data save in memory
+// CPU MP Data save in memory, and intended for use in C code.
+// There are some duplicated fields, such as XD status, between
+// CpuMpData and ExchangeInfo. These duplications in CpuMpData
+// are present to avoid to be direct accessed and comprehended
+// in assembly code.
 //
 struct _CPU_MP_DATA {
   UINT64   CpuInfoInHob;
   UINT32   CpuCount;
   UINT32   BspNumber;
-  //
-  // The above fields data will be passed from PEI to DXE
-  // Please make sure the fields offset same in the different
-  // architecture.
-  //
   SPIN_LOCKMpLock;
   UINTNBuffer;
   UINTNCpuApStackSize;
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111506): https://edk2.groups.io/g/devel/message/111506
Mute This Topic: https://groups.io/mt/102721665/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 0/2]UefiCpuPkg/MpInitLib: Enable execute disable bit.

2023-11-20 Thread Yuanhao Xie
This patch series synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.
It also updated the comments of _CPU_MP_DATA to delcared that
duplications in CpuMpData are present to avoid to be direct accessed
and comprehended in assembly code.

It updated the structure comments compared with V6.

Yuanhao Xie (1):
  UefiCpuPkg/MpInitLib: Enable execute disable bit.

xieyuanh (1):
  UefiCpuPkg/MpInitLib: Update the comments of _CPU_MP_DATA.

 UefiCpuPkg/Library/MpInitLib/MpEqu.inc |  2 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c   | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h   | 14 --
 3 files changed, 21 insertions(+), 9 deletions(-)

--
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111504): https://edk2.groups.io/g/devel/message/111504
Mute This Topic: https://groups.io/mt/102721660/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [Patch V7 1/2] UefiCpuPkg/MpInitLib: Enable execute disable bit.

2023-11-20 Thread Yuanhao Xie
From: Yuanhao Xie 

This patch synchronizes the No-Execute bit in the IA32_EFER
register for the APs before the RestoreVolatileRegisters operation.

The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
calling the SwitchApContext function to initiate a specialized start-up
signal, waking up APs in the DXE instead of using INIT-SIPI-SIPI.

Due to this change, the logic for "Enable execute disable bit" in
MpFuncs.nasm is no longer executed. However, to ensure the proper setup
of the page table, it is necessary to synchronize the IA32_EFER.NXE for
APs before executing RestoreVolatileRegisters .

Based on SDM:
If IA32_EFER.NXE is set to 1, it signifies execute-disable, meaning
instruction fetches are not allowed from the 4-KByte page controlled by
this entry. Conversely, if it is set to 0, it is reserved.

Signed-off-by: Yuanhao Xie 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Ray Ni 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..f29e66a14f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -910,9 +910,16 @@ DxeApEntryPoint (
   CPU_MP_DATA  *CpuMpData
   )
 {
-  UINTN  ProcessorNumber;
+  UINTN   ProcessorNumber;
+  MSR_IA32_EFER_REGISTER  EferMsr;
 
   GetProcessorNumber (CpuMpData, );
+  if (CpuMpData->EnableExecuteDisableForSwitchContext) {
+EferMsr.Uint64   = AsmReadMsr64 (MSR_IA32_EFER);
+EferMsr.Bits.NXE = 1;
+AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
+  }
+
   RestoreVolatileRegisters (>CpuData[0].VolatileRegisters, FALSE);
   InterlockedIncrement ((UINT32 *)>FinishedCount);
   PlaceAPInMwaitLoopOrRunLoop (
@@ -2188,8 +2195,9 @@ MpInitLibInitialize (
 if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
   ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
 
-  CpuMpData->FinishedCount = 0;
-  CpuMpData->InitFlag  = ApInitDone;
+  CpuMpData->FinishedCount= 0;
+  CpuMpData->InitFlag = ApInitDone;
+  CpuMpData->EnableExecuteDisableForSwitchContext = 
IsBspExecuteDisableEnabled ();
   SaveCpuMpData (CpuMpData);
   //
   // In scenarios where both the PEI and DXE phases run in the same
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 763db4963d..af296f6ac0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -270,6 +270,7 @@ struct _CPU_MP_DATA {
   UINT64   TotalTime;
   EFI_EVENTWaitEvent;
   UINTN**FailedCpuList;
+  BOOLEAN  EnableExecuteDisableForSwitchContext;
 
   AP_INIT_STATEInitFlag;
   BOOLEAN  SwitchBspFlag;
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111505): https://edk2.groups.io/g/devel/message/111505
Mute This Topic: https://groups.io/mt/102721661/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v1 1/1] BoardModulePkg\Library\BiosIdLib: Support Standalone MM

2023-11-20 Thread Huang, Li-Xia
Add Standalone Mm BiosIdLib.
Also fix some EDKII Coding Style issue with uncrustify.

Cc: Eric Dong 
Cc: Nate DeSimone 

Signed-off-by: Lixia Huang 
---
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/BiosIdCommon.c|  
96 
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.c| 
111 +++---
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/PeiBiosIdLib.c| 
118 +++-
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/StandaloneMmBiosIdLib.c   |  
65 +++
 Platform/Intel/BoardModulePkg/BoardModulePkg.dsc  |   
1 +
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.inf  |   
6 +-
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/PeiBiosIdLib.inf  |   
6 +-
 Platform/Intel/BoardModulePkg/Library/BiosIdLib/StandaloneMmBiosIdLib.inf |  
42 +++
 8 files changed, 244 insertions(+), 201 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BiosIdLib/BiosIdCommon.c 
b/Platform/Intel/BoardModulePkg/Library/BiosIdLib/BiosIdCommon.c
new file mode 100644
index ..5735566bfe3a
--- /dev/null
+++ b/Platform/Intel/BoardModulePkg/Library/BiosIdLib/BiosIdCommon.c
@@ -0,0 +1,96 @@
+/** @file
+  Boot service common BIOS ID library implementation.
+
+  These functions in this file can be called during DXE and cannot be called 
during runtime
+  or in SMM which should use a RT or SMM library.
+
+
+Copyright (c) 2023, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+/**
+  This function returns the BIOS Version & Release Date and Time by getting 
and converting BIOS ID.
+
+  @param[out] BiosVersion   The Bios Version out of the conversion.
+  @param[out] BiosReleaseDate   The Bios Release Date out of the conversion.
+  @param[out] BiosReleaseTime   The Bios Release Time out of the conversion.
+
+  @retval EFI_SUCCESS   BIOS Version & Release Date and Time have 
been got successfully.
+  @retval EFI_NOT_FOUND BIOS ID image is not found, and no 
parameter will be modified.
+  @retval EFI_INVALID_PARAMETER All the parameters are NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+GetBiosVersionDateTime (
+  OUT CHAR16  *BiosVersion OPTIONAL,
+  OUT CHAR16  *BiosReleaseDate OPTIONAL,
+  OUT CHAR16  *BiosReleaseTime OPTIONAL
+  )
+{
+  EFI_STATUS Status;
+  BIOS_ID_IMAGE  BiosIdImage;
+
+  if ((BiosVersion == NULL) && (BiosReleaseDate == NULL) && (BiosReleaseTime 
== NULL)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = GetBiosId ();
+  if (EFI_ERROR (Status)) {
+return EFI_NOT_FOUND;
+  }
+
+  if (BiosVersion != NULL) {
+//
+// Fill the BiosVersion data from the BIOS ID.
+//
+CopyMem (BiosVersion, &(BiosIdImage.BiosIdString), sizeof 
(BIOS_ID_STRING));
+  }
+
+  if (BiosReleaseDate != NULL) {
+//
+// Fill the build timestamp date from the BIOS ID in the "MM/DD/YY" format.
+//
+BiosReleaseDate[0] = BiosIdImage.BiosIdString.TimeStamp[2];
+BiosReleaseDate[1] = BiosIdImage.BiosIdString.TimeStamp[3];
+BiosReleaseDate[2] = (CHAR16)((UINT8)('/'));
+
+BiosReleaseDate[3] = BiosIdImage.BiosIdString.TimeStamp[4];
+BiosReleaseDate[4] = BiosIdImage.BiosIdString.TimeStamp[5];
+BiosReleaseDate[5] = (CHAR16)((UINT8)('/'));
+
+//
+// Add 20 for SMBIOS table
+// Current Linux kernel will misjudge 09 as year 0, so using 2009 for 
SMBIOS table
+//
+BiosReleaseDate[6] = '2';
+BiosReleaseDate[7] = '0';
+BiosReleaseDate[8] = BiosIdImage.BiosIdString.TimeStamp[0];
+BiosReleaseDate[9] = BiosIdImage.BiosIdString.TimeStamp[1];
+
+BiosReleaseDate[10] = (CHAR16)((UINT8)('\0'));
+  }
+
+  if (BiosReleaseTime != NULL) {
+//
+// Fill the build timestamp time from the BIOS ID in the "HH:MM" format.
+//
+BiosReleaseTime[0] = BiosIdImage.BiosIdString.TimeStamp[6];
+BiosReleaseTime[1] = BiosIdImage.BiosIdString.TimeStamp[7];
+BiosReleaseTime[2] = (CHAR16)((UINT8)(':'));
+
+BiosReleaseTime[3] = BiosIdImage.BiosIdString.TimeStamp[8];
+BiosReleaseTime[4] = BiosIdImage.BiosIdString.TimeStamp[9];
+
+BiosReleaseTime[5] = (CHAR16)((UINT8)('\0'));
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.c 
b/Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.c
index 3e614d9efc3e..6535bb36f6c9 100644
--- a/Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BiosIdLib/DxeBiosIdLib.c
@@ -5,7 +5,7 @@
   or in SMM which should use a RT or SMM library.
 
 
-Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -15,7 +15,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
-#include 
 #include 
 

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, November 21, 2023 #cal-reminder

2023-11-20 Thread Group Notification
*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, November 21, 2023
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Liming Gao gaolim...@byosoft.com.cn ( 
gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=2101038 )

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions ( 
https://conf.intel.com/teams/?conf=1160620940=teams=conf.intel.com=test_call
 )

*Or call in (audio only)*

+1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, 
Sacramento

Phone Conference ID: 774 638 21#

Find a local number ( 
https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821
 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2=0=en-US
 )


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111502): https://edk2.groups.io/g/devel/message/111502
Mute This Topic: https://groups.io/mt/102721567/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Get processor extended information

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Wu, Jiaxin 
> Sent: Wednesday, November 15, 2023 7:16 PM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek ; Dong, Eric ; Ni,
> Ray ; Kumar, Rahul R ; Gerd
> Hoffmann ; Zeng, Star 
> Subject: [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Get processor
> extended information
> 
> This patch is to extend SmmAddProcessor function to get processor
> extended information. It's to complete commit 1fadd18d.
> 
> Cc: Laszlo Ersek 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Star Zeng 
> Signed-off-by: Jiaxin Wu 
> Reviewed-by: Laszlo Ersek 
> Regression-tested-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> index 391b64e9f2..c0485b0519 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c
> @@ -169,10 +169,20 @@ SmmAddProcessor (
>  >ProcessorInfo[Index].Location.Package,
>  >ProcessorInfo[Index].Location.Core,
>  >ProcessorInfo[Index].Location.Thread
>  );
> 
> +  GetProcessorLocation2ByApicId (
> +(UINT32)ProcessorId,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Pac
> kage,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Die
> ,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Tile
> ,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Mo
> dule,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Cor
> e,
> +
> >ProcessorInfo[Index].ExtendedInformation.Location2.Thr
> ead
> +);
> +
>*ProcessorNumber = Index;
>gSmmCpuPrivate->Operation[Index] = SmmCpuAdd;
>return EFI_SUCCESS;
>  }
>}
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111501): https://edk2.groups.io/g/devel/message/111501
Mute This Topic: https://groups.io/mt/102602852/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/BaseXApicLib: Fix CPUID_V2_EXTENDED_TOPOLOGY detection

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Wu, Jiaxin 
> Sent: Wednesday, November 15, 2023 7:16 PM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek ; Dong, Eric ; Ni,
> Ray ; Kumar, Rahul R ; Gerd
> Hoffmann ; Zeng, Star 
> Subject: [PATCH v2 1/3] UefiCpuPkg/BaseXApicLib: Fix
> CPUID_V2_EXTENDED_TOPOLOGY detection
> 
> This patch is to complete 170d4ce8, sync the change to BaseXApicLib.
> 
> Checking the max cpuid leaf is not enough to figure whenever
> CPUID_V2_EXTENDED_TOPOLOGY is supported.  Intel SDM says:
> 
> Software must detect the presence of CPUID leaf 1FH by verifying
> (a) the highest leaf index supported by CPUID is >= 1FH, and
> (b) CPUID.1FH:EBX[15:0] reports a non-zero value.
> 
> The same is true for CPUID leaf 0BH.
> 
> This patch adds the EBX check to GetProcessorLocation2ByApicId().  The
> patch also fixes the existing check in GetProcessorLocationByApicId() to
> be in line with the spec by looking at bits 15:0.  The comments are
> updated with a quote from the Intel SDM.
> 
> Cc: Laszlo Ersek 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Star Zeng 
> Signed-off-by: Gerd Hoffmann 
> Signed-off-by: Jiaxin Wu 
> ---
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 21
> -
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index d56c6275cc..efb9d71ca1 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -1053,15 +1053,16 @@ GetProcessorLocationByApicId (
>,
>,
>NULL
>);
>  //
> -// If CPUID.(EAX=0BH, ECX=0H):EBX returns zero and maximum input
> value for
> -// basic CPUID information is greater than 0BH, then CPUID.0BH leaf is
> not
> -// supported on that processor.
> +// Quoting Intel SDM:
> +// Software must detect the presence of CPUID leaf 0BH by
> +// verifying (a) the highest leaf index supported by CPUID is >=
> +// 0BH, and (b) CPUID.0BH:EBX[15:0] reports a non-zero value.
>  //
> -if (ExtendedTopologyEbx.Uint32 != 0) {
> +if (ExtendedTopologyEbx.Bits.LogicalProcessors != 0) {
>TopologyLeafSupported = TRUE;
> 
>//
>// Sub-leaf index 0 (ECX= 0 as input) provides enumeration
> parameters to extract
>// the SMT sub-field of x2APIC ID.
> @@ -1183,10 +1184,11 @@ GetProcessorLocation2ByApicId (
>OUT UINT32  *Core OPTIONAL,
>OUT UINT32  *Thread   OPTIONAL
>)
>  {
>CPUID_EXTENDED_TOPOLOGY_EAX  ExtendedTopologyEax;
> +  CPUID_EXTENDED_TOPOLOGY_EBX  ExtendedTopologyEbx;
>CPUID_EXTENDED_TOPOLOGY_ECX  ExtendedTopologyEcx;
>UINT32   MaxStandardCpuIdIndex;
>UINT32   Index;
>UINTNLevelType;
>UINT32
> Bits[CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_DIE + 2];
> @@ -1195,14 +1197,23 @@ GetProcessorLocation2ByApicId (
>for (LevelType = 0; LevelType < ARRAY_SIZE (Bits); LevelType++) {
>  Bits[LevelType] = 0;
>}
> 
>//
> -  // Get max index of CPUID
> +  // Quoting Intel SDM:
> +  // Software must detect the presence of CPUID leaf 1FH by verifying
> +  // (a) the highest leaf index supported by CPUID is >= 1FH, and (b)
> +  // CPUID.1FH:EBX[15:0] reports a non-zero value.
>//
>AsmCpuid (CPUID_SIGNATURE, , NULL, NULL,
> NULL);
>if (MaxStandardCpuIdIndex < CPUID_V2_EXTENDED_TOPOLOGY) {
> +ExtendedTopologyEbx.Bits.LogicalProcessors = 0;
> +  } else {
> +AsmCpuidEx (CPUID_V2_EXTENDED_TOPOLOGY, 0, NULL,
> , NULL, NULL);
> +  }
> +
> +  if (ExtendedTopologyEbx.Bits.LogicalProcessors == 0) {
>  if (Die != NULL) {
>*Die = 0;
>  }
> 
>  if (Tile != NULL) {
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111500): https://edk2.groups.io/g/devel/message/111500
Mute This Topic: https://groups.io/mt/102602851/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [edk2-platforms PATCH 1/2] WhitleyOpenBoardPkg: remove references

2023-11-20 Thread Chiu, Chasel

Hi Laszlo,

I think you have to clone all the required repos.
edk2 repository
git clone https://github.com/tianocore/edk2.git

edk2-platforms repository
git clone https://github.com/tianocore/edk2-platforms.git

edk2-non-osi repository
git clone https://github.com/tianocore/edk2-non-osi.git

FSP repository
git clone https://github.com/IntelFsp/FSP.git

check if you got all repos like below:
edk2
edk2-non-osi
edk2-platforms
FSP

Then switch to edk2-platforms/Platform/Intel/build_bios.py -p WilsonCityRvp to 
start the build

Detail information is in readme: 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/Readme.md

Please let me know if any question.

Thanks,
Chasel





> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, November 15, 2023 3:51 AM
> To: devel@edk2.groups.io; Chiu, Chasel 
> Cc: Chaganty, Rangasai V ; Desimone, Nathaniel
> L 
> Subject: Re: [edk2-devel] [edk2-platforms PATCH 1/2] WhitleyOpenBoardPkg:
> remove  references
> 
> Hi Chasel,
> 
> On 11/10/23 02:13, Chiu, Chasel wrote:
> >
> > Hi Laszlo,
> >
> > I verified and encountered build failure as some files still consuming 
> > definitions
> from LegacyBiosMpTable.h, for example:
> > https://github.com/tianocore/edk2-platforms/blob/899a9dc97cd5469051338
> >
> 0ad01ee8b2609dbefd5/Platform/Intel/WhitleyOpenBoardPkg/Uba/UbaMain/Co
> m
> > mon/Dxe/SystemBoardInfoDxe/SystemBoardInfoDxe.c#L22
> >
> > Any suggestion that we can reduce impact to existing platforms?
> 
> I've been hoping to get comments from Nate on the expected contents of
> DEVICE_DATA_HW_LOCAL_INT. Barring such comments, here is my proposal:
> I'd like to just replace the enum constants in question with their numerical 
> values.
> 
> However, even for that, I'd really like to be able to build-test the platform 
> locally.
> Do you have build instructions? How can I satisfy the FSP blob requirement?
> 
> Thanks!
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111499): https://edk2.groups.io/g/devel/message/111499
Mute This Topic: https://groups.io/mt/102483850/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Remove the duplicate loading of microcode in DXE.

2023-11-20 Thread Ni, Ray
> +
> +if (!GetMicrocodePatchInfoFromHob (
> +   >MicrocodePatchAddress,
> +   >MicrocodePatchRegionSize
> +   ))
> +{
> +  //
> +  // The microcode patch information cache HOB does not exist, which
> means
> +  // the microcode patches data has not been loaded into memory yet
> +  //
> +  ShadowMicrocodeUpdatePatch (CpuMpData);
> +}

1. The microcode HOB is produced by MpInitLib PEI instance.
So the above code running in PEI phase cannot get the HOB.
We could simply delete it.

The microcode HOB has multiple consumers, such as some code that
measures the microcode binaries.
So, even MpInitLib DXE instance doesn't consume this HOB any more,
the producing is still needed.



> +
> +//
> +// Detect and apply Microcode on BSP
> +//
> +MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);

2. Loading microcode in BSP is good. Can you move this function call to 
together with WakeUpAp (ApInitializeSync)?
I think it could make the code more readable.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111498): https://edk2.groups.io/g/devel/message/111498
Mute This Topic: https://groups.io/mt/102701619/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/1] MdeModulePkg: Fix typos in Bus and Core

2023-11-20 Thread Page Chen via groups.io
begining -> beginning
Cabability -> Capability
CHNAGED -> CHANGED
compatability -> compatibility
concident -> coincident
correspoding -> corresponding
defered -> deferred
Dispacher -> Dispatcher
execuing -> executing
exhausive -> exhaustive
functio -> function
funtion -> function
funtions -> functions
notificaiton -> notification
Regsitered -> Registered
SUCESS -> SUCCESS
sucessfully -> successfully
writting -> writing

Signed-off-by: Page Chen 
Cc: Liming Gao 
Cc: Ray Ni 
Cc: Andrew Fish 
---
 .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.c   |  2 +-
 .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.h   |  2 +-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c  | 18 +-
 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 ++--
 .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c  | 10 +-
 .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.h  |  8 
 .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c   |  4 ++--
 .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.h   |  4 ++--
 .../NvmExpressPeiStorageSecurity.c |  2 +-
 .../NvmExpressPeiStorageSecurity.h |  2 +-
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c|  4 ++--
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.h|  2 +-
 MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h |  2 +-
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c   |  2 +-
 MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c   |  2 +-
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 18 +-
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h |  4 ++--
 MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c  |  2 +-
 MdeModulePkg/Core/Dxe/DxeMain.h|  8 
 MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c|  2 +-
 MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
 MdeModulePkg/Core/Dxe/Image/Image.c|  2 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c   |  6 +++---
 .../SectionExtraction/CoreSectionExtraction.c  |  6 +++---
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c |  4 ++--
 MdeModulePkg/Core/Pei/PeiMain.h|  2 +-
 26 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c 
b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
index 40e042082cde..6381c371698f 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
@@ -302,7 +302,7 @@ AhciStorageSecurityReceiveData (
 
   If the security protocol command completes without an error, the function 
shall
   return EFI_SUCCESS. If the security protocol command completes with an error,
-  the functio shall return EFI_DEVICE_ERROR.
+  the function shall return EFI_DEVICE_ERROR.
 
   @param[in] This  The PPI instance pointer.
   @param[in] DeviceIndex   The ID of the device.
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h 
b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
index a13533ea57ea..8bc367da21d8 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
@@ -190,7 +190,7 @@ AhciStorageSecurityReceiveData (
 
   If the security protocol command completes without an error, the function 
shall
   return EFI_SUCCESS. If the security protocol command completes with an error,
-  the functio shall return EFI_DEVICE_ERROR.
+  the function shall return EFI_DEVICE_ERROR.
 
   @param[in] This  The PPI instance pointer.
   @param[in] DeviceIndex   The ID of the device.
diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c 
b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
index b1ab17af3788..535f234916c0 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
@@ -81,7 +81,7 @@ PushEfikeyBufTail (
 /**
   Judge whether is a registered key
 
-  @param RegsiteredData   A pointer to a buffer that is filled in with the 
keystroke
+  @param RegisteredData   A pointer to a buffer that is filled in with the 
keystroke
   state data for the key that was registered.
   @param InputDataA pointer to a buffer that is filled in with the 
keystroke
   state data for the key that was pressed.
@@ -92,15 +92,15 @@ PushEfikeyBufTail (
 **/
 BOOLEAN
 IsKeyRegistered (
-  IN EFI_KEY_DATA  *RegsiteredData,
+  IN EFI_KEY_DATA  *RegisteredData,
   IN EFI_KEY_DATA  *InputData
   )
 
 {
-  ASSERT (RegsiteredData != NULL && InputData != NULL);
+  ASSERT (RegisteredData != NULL && InputData != NULL);
 
-  if ((RegsiteredData->Key.ScanCode!= InputData->Key.ScanCode) ||
-  (RegsiteredData->Key.UnicodeChar != InputData->Key.UnicodeChar))
+  if ((RegisteredData->Key.ScanCode!= InputData->Key.ScanCode) ||
+  (RegisteredData->Key.UnicodeChar != InputData->Key.UnicodeChar))
   {
 return FALSE;
   }
@@ -108,14 +108,14 @@ IsKeyRegistered (
   //
   // Assume KeyShiftState/KeyToggleState = 0 in 

Re: [edk2-devel] [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

2023-11-20 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

>I did not add this intentionally and even sent out an email asking the same on 
>edk2-rfc if this should be done by maintainers unless you want me to do it as 
>a ??separate patch set.
>Link to the message is https://edk2.groups.io/g/rfc/message/827
That's fine.

> String comes in from the 
> https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Include/Library/AndroidBootImgLib.h#L43
>  header so it should ideally be self-contained. If you still think we should 
> check for it, I can add a separate patch while keeping this one unaffected.

Reviewed-by: Abner Chang 

Abner


From: Ashish Singhal 
Sent: Tuesday, November 21, 2023 3:34 AM
To: Chang, Abner ; devel@edk2.groups.io; 
quic_llind...@quicinc.com; ardb+tianoc...@kernel.org; g...@danielschaefer.me; 
Jeff Brasen 
Subject: Re: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length 
Bug

[AMD Official Use Only - General]


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




From: Chang, Abner 
Sent: Monday, November 20, 2023 5:13 AM
To: Ashish Singhal ; 
mailto:devel@edk2.groups.io ; 
mailto:quic_llind...@quicinc.com ; 
mailto:ardb+tianoc...@kernel.org ; 
mailto:g...@danielschaefer.me ; Jeff Brasen 

Subject: RE: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length 
Bug

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

Thanks for this update and two more comments,

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: mailto:devel@edk2.groups.io; mailto:quic_llind...@quicinc.com;
> mailto:ardb+tianoc...@kernel.org; Chang, Abner ;
> mailto:g...@danielschaefer.me; mailto:jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line
> Length Bug
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Curently, AndroidBootImgLib expects input kernel command line
> to never exceed 256 unicode characters where the image header
> allows for 512 ascii characters. If image header allows 512
> ascii characters, similar number of unicode characters should be
> allowed at the minimum.
>
> Signed-off-by: Ashish Singhal 
> ---
>  .../AndroidBootImgLib/AndroidBootImgLib.c | 31 +++
>  1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index 1359a66db2..f63648e60d 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -322,11 +322,12 @@ AndroidBootImgGetFdt (
>  EFI_STATUS
>  AndroidBootImgUpdateArgs (
>IN  VOID  *BootImg,
> -  OUT VOID  *KernelArgs
> +  OUT VOID  **KernelArgs
>)
I know the original code doesn't have the function header. Would you mind to 
add it as we change the prototype of this function?
Or you probably would like to keep it as it for the consistence, as no function 
header for any of functions in this file.

I did not add this intentionally and even sent out an email asking the same on 
edk2-rfc if this should be done by maintainers unless you want me to do it as a 
separate patch set.

Link to the message is https://edk2.groups.io/g/rfc/message/827


>  {
>CHAR8   ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
>EFI_STATUS  Status;
> +  UINT32  NewKernelArgSize;
>
>// Get kernel arguments from Android boot image
>Status = AndroidBootImgGetKernelArgs (BootImg, ImageKernelArgs);
> @@ -334,16 +335,23 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
Do you think we have to check if the ASCII string size in ImageKernelArgs 
greater than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE?

Thanks
Abner

String comes in from the 
https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Include/Library/AndroidBootImgLib.h#L43
 header so it should ideally be self-contained. If you still think we should 
check for it, I can add a separate patch while keeping this one unaffected.

Thanks
Ashish


> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  *KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
> +  if (*KernelArgs == NULL) {
> +DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
>AsciiStrToUnicodeStrS (
>  ImageKernelArgs,
> -KernelArgs,
> 

Re: [edk2-devel] [Patch V6 1/2] UefiCpuPkg/MpInitLib: Enable execute disable bit.

2023-11-20 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Xie, Yuanhao 
> Sent: Monday, November 20, 2023 12:55 PM
> To: devel@edk2.groups.io
> Cc: Xie, Yuanhao ; Laszlo Ersek
> ; Dong, Eric ; Ni, Ray
> ; Kumar, Rahul R ; Gerd
> Hoffmann 
> Subject: [Patch V6 1/2] UefiCpuPkg/MpInitLib: Enable execute disable bit.
> 
> From: Yuanhao Xie 
> 
> This patch synchronizes the No-Execute bit in the IA32_EFER
> register for the APs before the RestoreVolatileRegisters operation.
> 
> The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
> sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
> calling the SwitchApContext function to initiate a specialized start-up
> signal, waking up APs in the DXE instead of using INIT-SIPI-SIPI.
> 
> Due to this change, the logic for "Enable execute disable bit" in
> MpFuncs.nasm is no longer executed. However, to ensure the proper setup
> of the page table, it is necessary to synchronize the IA32_EFER.NXE for
> APs before executing RestoreVolatileRegisters .
> 
> Based on SDM:
> If IA32_EFER.NXE is set to 1, it signifies execute-disable, meaning
> instruction fetches are not allowed from the 4-KByte page controlled by
> this entry. Conversely, if it is set to 0, it is reserved.
> 
> Signed-off-by: Yuanhao Xie 
> Reviewed-by: Laszlo Ersek 
> Cc: Laszlo Ersek ler...@redhat.com
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  1 +
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 9a6ec5db5c..f29e66a14f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -910,9 +910,16 @@ DxeApEntryPoint (
>CPU_MP_DATA  *CpuMpData
>)
>  {
> -  UINTN  ProcessorNumber;
> +  UINTN   ProcessorNumber;
> +  MSR_IA32_EFER_REGISTER  EferMsr;
> 
>GetProcessorNumber (CpuMpData, );
> +  if (CpuMpData->EnableExecuteDisableForSwitchContext) {
> +EferMsr.Uint64   = AsmReadMsr64 (MSR_IA32_EFER);
> +EferMsr.Bits.NXE = 1;
> +AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
> +  }
> +
>RestoreVolatileRegisters (>CpuData[0].VolatileRegisters,
> FALSE);
>InterlockedIncrement ((UINT32 *)>FinishedCount);
>PlaceAPInMwaitLoopOrRunLoop (
> @@ -2188,8 +2195,9 @@ MpInitLibInitialize (
>  if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
>ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
> 
> -  CpuMpData->FinishedCount = 0;
> -  CpuMpData->InitFlag  = ApInitDone;
> +  CpuMpData->FinishedCount= 0;
> +  CpuMpData->InitFlag =
> ApInitDone;
> +  CpuMpData->EnableExecuteDisableForSwitchContext =
> IsBspExecuteDisableEnabled ();
>SaveCpuMpData (CpuMpData);
>//
>// In scenarios where both the PEI and DXE phases run in the same
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 763db4963d..af296f6ac0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -270,6 +270,7 @@ struct _CPU_MP_DATA {
>UINT64   TotalTime;
>EFI_EVENTWaitEvent;
>UINTN**FailedCpuList;
> +  BOOLEAN
> EnableExecuteDisableForSwitchContext;
> 
>AP_INIT_STATEInitFlag;
>BOOLEAN  SwitchBspFlag;
> --
> 2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111495): https://edk2.groups.io/g/devel/message/111495
Mute This Topic: https://groups.io/mt/102702384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command line

2023-11-20 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

Ok, I got it. Thanks.

Reviewed-by: Abner Chang 

Abner


From: Ashish Singhal 
Sent: Tuesday, November 21, 2023 1:18 AM
To: Chang, Abner ; devel@edk2.groups.io; 
quic_llind...@quicinc.com; ardb+tianoc...@kernel.org; g...@danielschaefer.me; 
Jeff Brasen 
Subject: Re: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command 
line

[AMD Official Use Only - General]


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




From: Chang, Abner 
Sent: Monday, November 20, 2023 5:30 AM
To: Ashish Singhal ; 
mailto:devel@edk2.groups.io ; 
mailto:quic_llind...@quicinc.com ; 
mailto:ardb+tianoc...@kernel.org ; 
mailto:g...@danielschaefer.me ; Jeff Brasen 

Subject: RE: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command 
line

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: mailto:devel@edk2.groups.io; mailto:quic_llind...@quicinc.com;
> mailto:ardb+tianoc...@kernel.org; Chang, Abner ;
> mailto:g...@danielschaefer.me; mailto:jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command
> line
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> AndroidBootImgLib allows for platforms to append to kernel command
> line but does not allow for the overall kernel command line to go
> beyond the limit set by the image header. Address this limitation
> by adding a pcd where platform can tell how many extra characters
> they expect on their platform in addition to what the image header
> specifies.
>
> Signed-off-by: Ashish Singhal 
> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 5 +
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c   | 2 +-
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec
> b/EmbeddedPkg/EmbeddedPkg.dec
> index 341ef5e6a6..94dc3c9b76 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -183,3 +183,8 @@
># Selection between DT and ACPI as a default
>#
>gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x059
> +
> +  #
> +  # Expected Overflow Android Kernel Command Line Characters
> +  #
> +
> gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UI
> NT32|0x05C
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index f63648e60d..d16929f2bb 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -335,7 +335,7 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
> -  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32
> (PcdAndroidKernelCommandLineOverflow);

I don't know the history of ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, however, I am a 
little bit confused.
Is this PCD introduced for the case the string size may greater than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE? If yes, then why don't we just use StrSize 
(ImageKernelArgs) * sizeof(UINT16) as the buffer size?

Regards,
Abner
Android style image header can never have more than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE bytes. However, a platform may need to add new 
kernel command line arguments at boot time that may need more buffer space than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE bytes. To allow for that, we can have a PCD 
that by default is set to 0 but a user can override that in their platform dsc 
if they expect more than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE characters. This is 
not a bugfix but a limitation that is being addressed in this patch.

Thanks
Ashish

>*KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
>if (*KernelArgs == NULL) {
>  DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index 8eefeef4f9..9754664df5 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -45,5 +45,6 @@
>gEfiAcpiTableGuid
>gFdtTableGuid
>
> 

Re: [edk2-devel] [PATCH v3 22/39] ArmPkg: Remove ArmPciCpuIo2Dxe from ArmPkg

2023-11-20 Thread Chao Li

OK, I will remove this patch in V4.


Thanks,
Chao
On 2023/11/21 02:47, Leif Lindholm wrote:

Hi Chao,

Yes, correct.

So we can update the existing users of this driver in edk2-platforms
before deleting it.

Regards,

Leif

On Mon, Nov 20, 2023 at 11:24:03 +0800, Chao Li wrote:

Hi Leif,

Do you mean that CpuIo2Dxe adds MMIO method first, then waits for this patch
series to be merged, and finally makes a new BZ and remove the ARM version?


Thanks,
Chao
On 2023/11/17 21:13, Leif Lindholm wrote:

On Fri, Nov 17, 2023 at 18:01:39 +0800, Chao Li wrote:

ArmPciCpuIo2Dxe has been merged into CpuIo2Dxe, and CpuIo2Dxe is already
used by all ARM virtual platforms, so remove it.

This does affect 15 platforms in edk2-platforms.
You should ping the maintainers of the affected platforms, or even
better write a patch yourself, so we don't end up with sudden
mass-breakage.

It might be worth splitting this patch out of the rest of the set in
order to permit a more graceful switchover.

/
  Leif


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

Cc: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
---
   ArmPkg/ArmPkg.dsc |   1 -
   .../Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c | 556 --
   .../ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf   |  47 --
   3 files changed, 604 deletions(-)
   delete mode 100644 ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
   delete mode 100644 ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 6dd91e6941..7af25a91a1 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -143,7 +143,6 @@
 ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
-  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
 ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
 ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
 ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
diff --git a/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c 
b/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
deleted file mode 100644
index 5a2866ccd8..00
--- a/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
+++ /dev/null
@@ -1,556 +0,0 @@
-/** @file
-  Produces the CPU I/O 2 Protocol.
-
-Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
-Copyright (c) 2016, Linaro Ltd. All rights reserved.
-
-SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include 
-
-#include 
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define MAX_IO_PORT_ADDRESS  0x
-
-//
-// Handle for the CPU I/O 2 Protocol
-//
-STATIC EFI_HANDLE  mHandle = NULL;
-
-//
-// Lookup table for increment values based on transfer widths
-//
-STATIC CONST UINT8  mInStride[] = {
-  1, // EfiCpuIoWidthUint8
-  2, // EfiCpuIoWidthUint16
-  4, // EfiCpuIoWidthUint32
-  8, // EfiCpuIoWidthUint64
-  0, // EfiCpuIoWidthFifoUint8
-  0, // EfiCpuIoWidthFifoUint16
-  0, // EfiCpuIoWidthFifoUint32
-  0, // EfiCpuIoWidthFifoUint64
-  1, // EfiCpuIoWidthFillUint8
-  2, // EfiCpuIoWidthFillUint16
-  4, // EfiCpuIoWidthFillUint32
-  8  // EfiCpuIoWidthFillUint64
-};
-
-//
-// Lookup table for increment values based on transfer widths
-//
-STATIC CONST UINT8  mOutStride[] = {
-  1, // EfiCpuIoWidthUint8
-  2, // EfiCpuIoWidthUint16
-  4, // EfiCpuIoWidthUint32
-  8, // EfiCpuIoWidthUint64
-  1, // EfiCpuIoWidthFifoUint8
-  2, // EfiCpuIoWidthFifoUint16
-  4, // EfiCpuIoWidthFifoUint32
-  8, // EfiCpuIoWidthFifoUint64
-  0, // EfiCpuIoWidthFillUint8
-  0, // EfiCpuIoWidthFillUint16
-  0, // EfiCpuIoWidthFillUint32
-  0  // EfiCpuIoWidthFillUint64
-};
-
-/**
-  Check parameters to a CPU I/O 2 Protocol service request.
-
-  The I/O operations are carried out exactly as requested. The caller is 
responsible
-  for satisfying any alignment and I/O width restrictions that a PI System on a
-  platform might require. For example on some platforms, width requests of
-  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other hand, will
-  be handled by the driver.
-
-  @param[in] MmioOperation  TRUE for an MMIO operation, FALSE for I/O Port 
operation.
-  @param[in] Width  Signifies the width of the I/O or Memory operation.
-  @param[in] AddressThe base address of the I/O operation.
-  @param[in] Count  The number of I/O operations to perform. The 
number of
-bytes moved is Width size * Count, starting at 
Address.
-  @param[in] Buffer For read operations, the destination buffer to 
store the results.
-For write operations, the source buffer from which 
to write data.
-
-  @retval EFI_SUCCESSThe parameters for this request pass the 
checks.
-  @retval EFI_INVALID_PARAMETER  Width is invalid for this PI system.
-  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
-  @retval EFI_UNSUPPORTEDThe Buffer is not aligned for the given Width.
-  @retval EFI_UNSUPPORTEDThe address range specified by Address, Width,
- 

回复: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

2023-11-20 Thread gaoliming via groups.io
Sami:
 I am OK to merge this patch for this stable bug. I will add push label for 
https://github.com/tianocore/edk2/pull/5061 

Thanks
Liming
> -邮件原件-
> 发件人: Sami Mujawar 
> 发送时间: 2023年11月21日 0:07
> 收件人: Laszlo Ersek ; Liming Gao (Byosoft address)
> ; Michael Kinney ;
> devel@edk2.groups.io; ashishsin...@nvidia.com; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; jbra...@nvidia.com
> 抄送: n...@arm.com
> 主题: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix
> ETE _UID Creation
> 
> Hi Laszlo, Liming, Mike,
> 
> Please see my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> On 17/11/2023 09:20 am, Laszlo Ersek wrote:
> > On 11/15/23 04:19, Ashish Singhal via groups.io wrote:
> >> Just like CPU _UID, ETE UID also needs to be unique so
> >> use AcpiProcessorUid instead of CpuName
> >>
> >> Signed-off-by: Ashish Singhal 
> >> ---
> >>   .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5
> -
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> > Is this a fixup for the recent feature
> >
> > [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
> > https://edk2.groups.io/g/devel/message/108996
> >
> > ?
> >
> > If so, then I *think* this qualifies to be merged during the hard
> > feature freeze (+Liming +Mike), but:
> 
> [SAMI] I raised a bugzilla for this issue at
> https://bugzilla.tianocore.org/show_bug.cgi?id=4600
> 
> and have also created a pull request at
> https://github.com/tianocore/edk2/pull/5061.
> 
> This patch has also passed the CI checks when I did a draft pull request.
> 
> I am not sure if I can apply the push label as we are in the code freeze
> stage.
> 
> Can you advise on how to proceed, please?
> 
> [/SAMI]
> 
> > - I think we should have a "Fixes:" tag in the commit message (for
> > pointing out the commit that should have contained the code being
> > added/updated now)
> >
> > - I think we should have a BZ too (also linked into the commit message).
> >
> > Laszlo
> >




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111492): https://edk2.groups.io/g/devel/message/111492
Mute This Topic: https://groups.io/mt/102720225/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, November 20, 2023 #cal-notice

2023-11-20 Thread Group Notification
*Tools, CI, Code base construction meeting series*

*When:*
Monday, November 20, 2023
4:30pm to 5:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=2100129 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111491): https://edk2.groups.io/g/devel/message/111491
Mute This Topic: https://groups.io/mt/102719616/21656
Mute #cal-notice:https://edk2.groups.io/g/devel/mutehashtag/cal-notice
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, November 20, 2023 #cal-reminder

2023-11-20 Thread Group Notification
*Reminder: Tools, CI, Code base construction meeting series*

*When:*
Monday, November 20, 2023
4:30pm to 5:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=2100129 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111490): https://edk2.groups.io/g/devel/message/111490
Mute This Topic: https://groups.io/mt/102698738/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v4 11/14] UefiCpuPkg: Use Attribute From SMM MemoryAttributesTable if Nonzero

2023-11-20 Thread Taylor Beebe

I missed this message before heading out on vacation, sorry :)

Let me know if you have more questions. I've also split the MAT fixes into

separate patches for the v5 version and will wait to hear back from you

before sending to ensure the updated notes for this patch

answer all your questions.

On 11/8/2023 1:57 PM, Laszlo Ersek wrote:

On 11/3/23 18:17, Taylor Beebe wrote:

The function EnforceMemoryMapAttribute() in the SMM MAT logic will
ensure that the CODE and DATA memory types have the desired attributes.

EnforceMemoryMapAttribute() leaves those descriptors alone where
Attribute is already nonzero ("PE image") [1].

For all other descriptors (i.e., where Attribute is zero), it:

- either sets Attribute to EFI_MEMORY_RO [2],

- or sets the EFI_MEMORY_XP *bit* in the Attribute [3] -- which is
identical to setting Attribute to EFI_MEMORY_XP altogether, given that
Attribute is zero to begin with. (So this |= operator looks like a
thinko in the function! But that's a separate topic.)


The consumer of the SMM MAT

So this seems to imply that the SMM MAT is produced based on
EnforceMemoryMapAttribute(), and then SetMemMapAttributes(), modified in
this patch, is what consumes the SMM MAT. Is that right?


This is correct. Note that the MAT is installed as a config table with the

gEdkiiPiSmmMemoryAttributesTableGuid but I could not find any info

in the PI spec on this table, just in the header file. Not sure if 
that's usual,


I just assumed that EdkiiPi in the guid name meant it was documented

in the spec.


In other words, SetMemMapAttributes() relies on
EnforceMemoryMapAttribute(); is that your point?


EnforceMemoryMapAttribute() is a final filter function run after 
creating the


MAT from the EFI memory map and loaded image list. More info below.


should only override the Attributes field
in the MAT if it is nonzero.

I don't understand -- do you mean "override the Attributes field if it
is *zero*"? (Because, if it is nonzero, then it has been pre-populated
by EnforceMemoryMapAttribute(), and then we should *use* it, as the
subject line of the patch says.)


PiSmmCore fetches the EFI memory map and calls SplitTable() to split each

loaded image section into its own descriptor with EFI_MEMORY_XP marking

data sections and EFI_MEMORY_RO marking code sections.


[4] The SMM MAT logic is almost identical to the DXE MAT logic but goes 
a step


further and also updates the memory map descriptors which describe

image code and data sections to be of type EfiRuntimeServicesCodeand

EfiRuntimeServicesData respectively.The consolidated MAT logic more

closely follows the DXE MAT logic which identifies image code sections 
by the


presence of the attribute EFI_MEMORY_RO in the descriptor and image

data sections by the presence of the attribute EFI_MEMORY_XP.


Further question: given that EnforceMemoryMapAttribute() exits with
*all* descriptors having a nonzero Attribute field, how is it possible
for SetMemMapAttributes() to see any descriptor with zero Attribute field?


It is not possible, but SetMemMapAttributes() would apply paging

attributes based on the memory type regardless of what was already

in the Attribute fieldof the SMM MAT. This worked because of [4],

but because this patch series is consolidating the DXE and SMM MAT

logic into a library this no longer works.


I see two possibilities:

- something introduces a new descriptor between
EnforceMemoryMapAttribute() and SetMemMapAttributes()

- and/or, SetMemMapAttributes() doesn't actually check the entire
Attribute field for nullity, but only the EFI_MEMORY_ACCESS_MASK
bitfield of it. Cases [2] and [3] above ensure the
EFI_MEMORY_ACCESS_MASK bitmask is nonzero, but case [1] ("PE image")
allows for an Attribute field in the descriptor that is nonzero, but
whose EFI_MEMORY_ACCESS_MASK bitfield is zero.


There won't be a descriptor with nonzero attributes after 
EnforceMemoryMapAttribute().


However, SetMemMapAttributes() should use the attributes present in the 
SMM MAT


to apply paging protections. When I wrote this patch comment, I assumed 
the DXE and


SMM MAT logic was more widely understood so describing its flow was not 
necessary. I'm


realizing that the MAT logic is basically a black box nowadays :D


This also allows the UEFI and SMM MAT
logic to use ImagePropertiesRecordLib instead of carrying two copies
of the image properties record manipulation logic.

Cc: Eric Dong
Cc: Ray Ni
Cc: Rahul Kumar
Cc: Gerd Hoffmann
Signed-off-by: Taylor Beebe
---
  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 19 +++
  1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 6f498666157e..d302a9b0cbcf 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1062,14 +1062,17 @@ SetMemMapAttributes (
MemoryMap = MemoryMapStart;

Re: [edk2-devel] [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

2023-11-20 Thread Ashish Singhal via groups.io



From: Chang, Abner 
Sent: Monday, November 20, 2023 5:13 AM
To: Ashish Singhal ; devel@edk2.groups.io 
; quic_llind...@quicinc.com ; 
ardb+tianoc...@kernel.org ; g...@danielschaefer.me 
; Jeff Brasen 
Subject: RE: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length 
Bug

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

Thanks for this update and two more comments,

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Chang, Abner ;
> g...@danielschaefer.me; jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line
> Length Bug
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Curently, AndroidBootImgLib expects input kernel command line
> to never exceed 256 unicode characters where the image header
> allows for 512 ascii characters. If image header allows 512
> ascii characters, similar number of unicode characters should be
> allowed at the minimum.
>
> Signed-off-by: Ashish Singhal 
> ---
>  .../AndroidBootImgLib/AndroidBootImgLib.c | 31 +++
>  1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index 1359a66db2..f63648e60d 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -322,11 +322,12 @@ AndroidBootImgGetFdt (
>  EFI_STATUS
>  AndroidBootImgUpdateArgs (
>IN  VOID  *BootImg,
> -  OUT VOID  *KernelArgs
> +  OUT VOID  **KernelArgs
>)
I know the original code doesn't have the function header. Would you mind to 
add it as we change the prototype of this function?
Or you probably would like to keep it as it for the consistence, as no function 
header for any of functions in this file.

I did not add this intentionally and even sent out an email asking the same on 
edk2-rfc if this should be done by maintainers unless you want me to do it as a 
separate patch set.

Link to the message is https://edk2.groups.io/g/rfc/message/827


>  {
>CHAR8   ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
>EFI_STATUS  Status;
> +  UINT32  NewKernelArgSize;
>
>// Get kernel arguments from Android boot image
>Status = AndroidBootImgGetKernelArgs (BootImg, ImageKernelArgs);
> @@ -334,16 +335,23 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
Do you think we have to check if the ASCII string size in ImageKernelArgs 
greater than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE?

Thanks
Abner

String comes in from the 
https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Include/Library/AndroidBootImgLib.h#L43
 header so it should ideally be self-contained. If you still think we should 
check for it, I can add a separate patch while keeping this one unaffected.

Thanks
Ashish


> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  *KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
> +  if (*KernelArgs == NULL) {
> +DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
>AsciiStrToUnicodeStrS (
>  ImageKernelArgs,
> -KernelArgs,
> -ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
> +*KernelArgs,
> +NewKernelArgSize
>  );
>// Append platform kernel arguments
>if (mAndroidBootImg->AppendArgs) {
>  Status = mAndroidBootImg->AppendArgs (
> -KernelArgs,
> -ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
> +*KernelArgs,
> +NewKernelArgSize
>  );
>}
>
> @@ -616,6 +624,10 @@ AndroidBootImgBoot (
>UINTN  RamdiskSize;
>IN  VOID   *FdtBase;
>
> +  if ((Buffer == NULL) || (BufferSize == 0)) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>NewKernelArg = NULL;
>ImageHandle  = NULL;
>
> @@ -637,14 +649,7 @@ AndroidBootImgBoot (
>  goto Exit;
>}
>
> -  NewKernelArg = AllocateZeroPool
> (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
> -  if (NewKernelArg == NULL) {
> -DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> -Status = EFI_OUT_OF_RESOURCES;
> -goto Exit;
> -  }
> -
> -  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
> +  Status = AndroidBootImgUpdateArgs (Buffer, );
>if (EFI_ERROR (Status)) {
>  goto Exit;
>}
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111488): https://edk2.groups.io/g/devel/message/111488
Mute This Topic: https://groups.io/mt/102598724/21656
Group 

Re: [edk2-devel] [PATCH v3 22/39] ArmPkg: Remove ArmPciCpuIo2Dxe from ArmPkg

2023-11-20 Thread Leif Lindholm
Hi Chao,

Yes, correct.

So we can update the existing users of this driver in edk2-platforms
before deleting it.

Regards,

Leif

On Mon, Nov 20, 2023 at 11:24:03 +0800, Chao Li wrote:
> Hi Leif,
> 
> Do you mean that CpuIo2Dxe adds MMIO method first, then waits for this patch
> series to be merged, and finally makes a new BZ and remove the ARM version?
> 
> 
> Thanks,
> Chao
> On 2023/11/17 21:13, Leif Lindholm wrote:
> > On Fri, Nov 17, 2023 at 18:01:39 +0800, Chao Li wrote:
> > > ArmPciCpuIo2Dxe has been merged into CpuIo2Dxe, and CpuIo2Dxe is already
> > > used by all ARM virtual platforms, so remove it.
> > This does affect 15 platforms in edk2-platforms.
> > You should ping the maintainers of the affected platforms, or even
> > better write a patch yourself, so we don't end up with sudden
> > mass-breakage.
> > 
> > It might be worth splitting this patch out of the rest of the set in
> > order to permit a more graceful switchover.
> > 
> > /
> >  Leif
> > 
> > > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4584
> > > 
> > > Cc: Leif Lindholm
> > > Cc: Ard Biesheuvel
> > > Cc: Sami Mujawar
> > > ---
> > >   ArmPkg/ArmPkg.dsc |   1 -
> > >   .../Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c | 556 --
> > >   .../ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf   |  47 --
> > >   3 files changed, 604 deletions(-)
> > >   delete mode 100644 ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
> > >   delete mode 100644 ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
> > > 
> > > diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> > > index 6dd91e6941..7af25a91a1 100644
> > > --- a/ArmPkg/ArmPkg.dsc
> > > +++ b/ArmPkg/ArmPkg.dsc
> > > @@ -143,7 +143,6 @@
> > > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > > -  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
> > > ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> > > ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> > > ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> > > diff --git a/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c 
> > > b/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
> > > deleted file mode 100644
> > > index 5a2866ccd8..00
> > > --- a/ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.c
> > > +++ /dev/null
> > > @@ -1,556 +0,0 @@
> > > -/** @file
> > > -  Produces the CPU I/O 2 Protocol.
> > > -
> > > -Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
> > > -Copyright (c) 2016, Linaro Ltd. All rights reserved.
> > > -
> > > -SPDX-License-Identifier: BSD-2-Clause-Patent
> > > -
> > > -**/
> > > -
> > > -#include 
> > > -
> > > -#include 
> > > -
> > > -#include 
> > > -#include 
> > > -#include 
> > > -#include 
> > > -#include 
> > > -
> > > -#define MAX_IO_PORT_ADDRESS  0x
> > > -
> > > -//
> > > -// Handle for the CPU I/O 2 Protocol
> > > -//
> > > -STATIC EFI_HANDLE  mHandle = NULL;
> > > -
> > > -//
> > > -// Lookup table for increment values based on transfer widths
> > > -//
> > > -STATIC CONST UINT8  mInStride[] = {
> > > -  1, // EfiCpuIoWidthUint8
> > > -  2, // EfiCpuIoWidthUint16
> > > -  4, // EfiCpuIoWidthUint32
> > > -  8, // EfiCpuIoWidthUint64
> > > -  0, // EfiCpuIoWidthFifoUint8
> > > -  0, // EfiCpuIoWidthFifoUint16
> > > -  0, // EfiCpuIoWidthFifoUint32
> > > -  0, // EfiCpuIoWidthFifoUint64
> > > -  1, // EfiCpuIoWidthFillUint8
> > > -  2, // EfiCpuIoWidthFillUint16
> > > -  4, // EfiCpuIoWidthFillUint32
> > > -  8  // EfiCpuIoWidthFillUint64
> > > -};
> > > -
> > > -//
> > > -// Lookup table for increment values based on transfer widths
> > > -//
> > > -STATIC CONST UINT8  mOutStride[] = {
> > > -  1, // EfiCpuIoWidthUint8
> > > -  2, // EfiCpuIoWidthUint16
> > > -  4, // EfiCpuIoWidthUint32
> > > -  8, // EfiCpuIoWidthUint64
> > > -  1, // EfiCpuIoWidthFifoUint8
> > > -  2, // EfiCpuIoWidthFifoUint16
> > > -  4, // EfiCpuIoWidthFifoUint32
> > > -  8, // EfiCpuIoWidthFifoUint64
> > > -  0, // EfiCpuIoWidthFillUint8
> > > -  0, // EfiCpuIoWidthFillUint16
> > > -  0, // EfiCpuIoWidthFillUint32
> > > -  0  // EfiCpuIoWidthFillUint64
> > > -};
> > > -
> > > -/**
> > > -  Check parameters to a CPU I/O 2 Protocol service request.
> > > -
> > > -  The I/O operations are carried out exactly as requested. The caller is 
> > > responsible
> > > -  for satisfying any alignment and I/O width restrictions that a PI 
> > > System on a
> > > -  platform might require. For example on some platforms, width requests 
> > > of
> > > -  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other 
> > > hand, will
> > > -  be handled by the driver.
> > > -
> > > -  @param[in] MmioOperation  TRUE for an MMIO operation, FALSE for I/O 
> > > Port operation.
> > > -  @param[in] Width  Signifies the width of the I/O or Memory 
> > > operation.
> > > -  @param[in] AddressThe base address of the I/O operation.
> > > -  @param[in] Count  The number of I/O operations to perform. The 
> > > number of
> > > - 

Re: [edk2-devel] [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks

2023-11-20 Thread Leif Lindholm
On Mon, Nov 20, 2023 at 14:48:25 +0100, Pierre Gondois wrote:
> Add AARCH64 specific implementations of:
> - OPENSSL_cpuid_setup(), probing hardware capabilitie
>   (presence of FEAT_AES, etc.)
> - OPENSSL_rdtsc(), returning non-trusted entropy by accessing
>   system counter.
> 
> Acked-by: Gerd Hoffmann 
> Signed-off-by: Pierre Gondois 
> ---
>  .../Library/OpensslLib/OpensslLibAccel.inf|  7 ++
>  .../OpensslLib/OpensslLibFullAccel.inf|  7 ++
>  .../OpensslLib/OpensslStub/AArch64Cap.c   | 84 +++
>  3 files changed, 98 insertions(+)
>  create mode 100644 CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
> b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
> index 3d1a9638b1c1..08e8be6ea9e1 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
> @@ -1329,6 +1329,7 @@ [Sources.X64]
>  # Autogenerated files list ends here
>  
>  [Sources.AARCH64]
> +  OpensslStub/AArch64Cap.c
>  # Autogenerated files list starts here
>$(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>$(OPENSSL_PATH)/crypto/aes/aes_cfb.c
> @@ -1955,11 +1956,17 @@ [Packages]
>MdePkg/MdePkg.dec
>CryptoPkg/CryptoPkg.dec
>  
> +[Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +
>  [LibraryClasses]
>BaseLib
>DebugLib
>RngLib
>  
> +[LibraryClasses.AARCH64]
> +  ArmLib
> +
>  [BuildOptions]
>#
># Disables the following Visual Studio compiler warnings brought by 
> openssl source,
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
> b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
> index e7e83d419f4b..2a01ffe06bd7 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
> @@ -1432,6 +1432,7 @@ [Sources.X64]
>  # Autogenerated files list ends here
>  
>  [Sources.AARCH64]
> +  OpensslStub/AArch64Cap.c
>  # Autogenerated files list starts here
>$(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>$(OPENSSL_PATH)/crypto/aes/aes_cfb.c
> @@ -2107,11 +2108,17 @@ [Packages]
>MdePkg/MdePkg.dec
>CryptoPkg/CryptoPkg.dec
>  
> +[Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +
>  [LibraryClasses]
>BaseLib
>DebugLib
>RngLib
>  
> +[LibraryClasses.AARCH64]
> +  ArmLib
> +
>  [BuildOptions]
>#
># Disables the following Visual Studio compiler warnings brought by 
> openssl source,
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c 
> b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
> new file mode 100644
> index ..7468ef3ab54e
> --- /dev/null
> +++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
> @@ -0,0 +1,84 @@
> +/** @file
> +  Arm capabilities probing.
> +
> +  Copyright (c) 2023, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include 
> +#include "crypto/arm_arch.h"
> +
> +#include 
> +
> +/** Get bits from a value.
> +
> +  Shift the input value from 'shift' bits and apply 'mask'.
> +
> +  @param   valueThe value to get the bits from.
> +  @param   shiftIndex of the bits to read.
> +  @param   mask Mask to apply to the value once shifted.
> +
> +  @return  The desired bitfield from the value.
> +**/
> +#define GET_BITFIELD(value, shift, mask)\
> +  ((value >> shift) & mask)

(This macro appears unused here now.)

> +
> +UINT32  OPENSSL_armcap_P = 0;
> +
> +void
> +OPENSSL_cpuid_setup (
> +  void
> +  )
> +{
> +  OPENSSL_armcap_P = 0;
> +
> +  /* Access to EL0 registers is possible from higher ELx. */
> +  OPENSSL_armcap_P |= ARMV8_CPUID;
> +  /* Access to Physical timer is possible. */
> +  OPENSSL_armcap_P |= ARMV7_TICK;
> +
> +  /* Neon support is not guaranteed, but it is assumed to be present.
> + Arm ARM for Armv8, sA1.5 Advanced SIMD and floating-point support
> +  */
> +  OPENSSL_armcap_P |= ARMV7_NEON;
> +
> +  if (ArmHasAes ())
> +  {

And I think that curly bracket is supposed to be on the previous line
(and similarly below), but this may be intional to align with nearby
code?

Anyway, this is a big readability improvement, thank you very much!
Acked-by: Leif Lindholm 

/
Leif

> +OPENSSL_armcap_P |= ARMV8_AES;
> +  }
> +
> +  if (ArmHasSha1 ())
> +  {
> +OPENSSL_armcap_P |= ARMV8_SHA1;
> +  }
> +
> +  if (ArmHasSha256 ())
> +  {
> +OPENSSL_armcap_P |= ARMV8_SHA256;
> +  }
> +
> +  if (ArmHasPmull ())
> +  {
> +OPENSSL_armcap_P |= ARMV8_PMULL;
> +  }
> +
> +  if (ArmHasSha512 ())
> +  {
> +OPENSSL_armcap_P |= ARMV8_SHA512;
> +  }
> +}
> +
> +/** Read system counter value.
> +
> +  Used to get some non-trusted entropy.
> +
> +  @return Lower bits of the physical counter.
> +**/
> +uint32_t
> +OPENSSL_rdtsc (
> +  void
> +  )
> +{
> +  return (UINT32)ArmReadCntPct ();
> +}
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111486): 

Re: [edk2-devel] [PATCH v4 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 register

2023-11-20 Thread Leif Lindholm
On Mon, Nov 20, 2023 at 14:48:21 +0100, Pierre Gondois wrote:
> Add macro definitions and helper functions to access AArch64
> capabilities described in the AA64Isar0 register.
> 
> Signed-off-by: Pierre Gondois 
> ---
>  ArmPkg/Include/Chipset/AArch64.h   |  60 +++-
>  ArmPkg/Include/Library/ArmLib.h| 228 -
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 367 +
>  3 files changed, 653 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Include/Chipset/AArch64.h 
> b/ArmPkg/Include/Chipset/AArch64.h
> index 5390bf0a2774..97c20e71a811 100644
> --- a/ArmPkg/Include/Chipset/AArch64.h
> +++ b/ArmPkg/Include/Chipset/AArch64.h
> @@ -1,7 +1,7 @@
>  /** @file
>  
>Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> -  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.
> +  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -127,6 +127,64 @@
>  // build for ARMv8.0, we need to define the register here.
>  #define ID_AA64MMFR2_EL1  S3_0_C0_C7_2
>  
> +//
> +// Bit shifts for the ID_AA64ISAR0_EL1 register.
> +//
> +#define ARM_ID_AA64ISAR0_EL1_AES_SHIFT (4U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA1_SHIFT(8U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA2_SHIFT(12U)
> +#define ARM_ID_AA64ISAR0_EL1_CRC32_SHIFT   (16U)
> +#define ARM_ID_AA64ISAR0_EL1_ATOMIC_SHIFT  (20U)
> +#define ARM_ID_AA64ISAR0_EL1_RDM_SHIFT (28U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA3_SHIFT(32U)
> +#define ARM_ID_AA64ISAR0_EL1_SM3_SHIFT (36U)
> +#define ARM_ID_AA64ISAR0_EL1_SM4_SHIFT (40U)
> +#define ARM_ID_AA64ISAR0_EL1_DP_SHIFT  (44U)
> +#define ARM_ID_AA64ISAR0_EL1_FHM_SHIFT (48U)
> +#define ARM_ID_AA64ISAR0_EL1_TS_SHIFT  (52U)
> +#define ARM_ID_AA64ISAR0_EL1_TLB_SHIFT (56U)
> +#define ARM_ID_AA64ISAR0_EL1_RNDR_SHIFT(60U)
> +
> +//
> +// Bit masks for the ID_AA64ISAR0_EL1 fields.
> +//
> +#define ARM_ID_AA64ISAR0_EL1_AES_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_SHA1_MASK(0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_SHA2_MASK(0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_CRC32_MASK   (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_ATOMIC_MASK  (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_RDM_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_SHA3_MASK(0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_SM3_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_SM4_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_DP_MASK  (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_FHM_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_TS_MASK  (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_TLB_MASK (0xFU)
> +#define ARM_ID_AA64ISAR0_EL1_RNDR_MASK(0xFU)
> +
> +//
> +// Bit masks for the ID_AA64ISAR0_EL1 field values.
> +//
> +#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_AES_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_PMULL_MASK  (0x2U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA1_FEAT_SHA1_MASK  (0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA256_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA512_MASK(0x2U)
> +#define ARM_ID_AA64ISAR0_EL1_CRC32_HAVE_CRC32_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_ATOMIC_FEAT_LSE_MASK (0x2U)
> +#define ARM_ID_AA64ISAR0_EL1_RDM_FEAT_RDM_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_SHA3_FEAT_SHA3_MASK  (0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_SM3_FEAT_SM3_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_SM4_FEAT_SM4_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_DP_FEAT_DOTPROD_MASK (0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_FHM_FEAT_FHM_MASK(0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM_MASK   (0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM2_MASK  (0x2U)
> +#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIOS_MASK (0x1U)
> +#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIRANGE_MASK  (0x2U)
> +#define ARM_ID_AA64ISAR0_EL1_RNDR_FEAT_RNG_MASK   (0x1U)
> +
>  #define VECTOR_BASE(tbl)  \
>.section .text.##tbl##,"ax";\
>.align 11;  \
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 6aa8a48f07f3..a176fcd7bf0a 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>  
>Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> -  Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.
> +  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
>Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -805,6 +805,232 @@ ArmHasEte (
>VOID
>);
>  
> +/** Read AA64Isar0 register.
> +
> +   @return AA64Isar0's register value.
> +**/
> +UINTN
> +EFIAPI
> +ArmReadIdAA64Isar0 (
> +  VOID
> +  );

This declaration already exists in
Library/ArmLib/AArch64/AArch64Lib.h, so can be dropped.

With that:
Reviewed-by: Leif Lindholm 

Thanks!

/
Leif

> +
> +/**
> +  Checks whether the CPU implements FEAT_AES.
> 

Re: [edk2-devel] [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command line

2023-11-20 Thread Ashish Singhal via groups.io



From: Chang, Abner 
Sent: Monday, November 20, 2023 5:30 AM
To: Ashish Singhal ; devel@edk2.groups.io 
; quic_llind...@quicinc.com ; 
ardb+tianoc...@kernel.org ; g...@danielschaefer.me 
; Jeff Brasen 
Subject: RE: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command 
line

External email: Use caution opening links or attachments


[AMD Official Use Only - General]

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Chang, Abner ;
> g...@danielschaefer.me; jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command
> line
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> AndroidBootImgLib allows for platforms to append to kernel command
> line but does not allow for the overall kernel command line to go
> beyond the limit set by the image header. Address this limitation
> by adding a pcd where platform can tell how many extra characters
> they expect on their platform in addition to what the image header
> specifies.
>
> Signed-off-by: Ashish Singhal 
> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 5 +
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c   | 2 +-
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec
> b/EmbeddedPkg/EmbeddedPkg.dec
> index 341ef5e6a6..94dc3c9b76 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -183,3 +183,8 @@
># Selection between DT and ACPI as a default
>#
>gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x059
> +
> +  #
> +  # Expected Overflow Android Kernel Command Line Characters
> +  #
> +
> gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UI
> NT32|0x05C
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index f63648e60d..d16929f2bb 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -335,7 +335,7 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
> -  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32
> (PcdAndroidKernelCommandLineOverflow);

I don't know the history of ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, however, I am a 
little bit confused.
Is this PCD introduced for the case the string size may greater than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE? If yes, then why don't we just use StrSize 
(ImageKernelArgs) * sizeof(UINT16) as the buffer size?

Regards,
Abner

Android style image header can never have more than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE bytes. However, a platform may need to add new 
kernel command line arguments at boot time that may need more buffer space than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE bytes. To allow for that, we can have a PCD 
that by default is set to 0 but a user can override that in their platform dsc 
if they expect more than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE characters. This is 
not a bugfix but a limitation that is being addressed in this patch.

Thanks
Ashish

>*KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
>if (*KernelArgs == NULL) {
>  DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index 8eefeef4f9..9754664df5 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -45,5 +45,6 @@
>gEfiAcpiTableGuid
>gFdtTableGuid
>
> -[FeaturePcd]
> +[Pcd]
>gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
> +  gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111484): https://edk2.groups.io/g/devel/message/111484
Mute This Topic: https://groups.io/mt/102598723/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

2023-11-20 Thread Sami Mujawar

Hi Laszlo, Liming, Mike,

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 17/11/2023 09:20 am, Laszlo Ersek wrote:

On 11/15/23 04:19, Ashish Singhal via groups.io wrote:

Just like CPU _UID, ETE UID also needs to be unique so
use AcpiProcessorUid instead of CpuName

Signed-off-by: Ashish Singhal 
---
  .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

Is this a fixup for the recent feature

[PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
https://edk2.groups.io/g/devel/message/108996

?

If so, then I *think* this qualifies to be merged during the hard
feature freeze (+Liming +Mike), but:


[SAMI] I raised a bugzilla for this issue at 
https://bugzilla.tianocore.org/show_bug.cgi?id=4600


and have also created a pull request at 
https://github.com/tianocore/edk2/pull/5061.


This patch has also passed the CI checks when I did a draft pull request.

I am not sure if I can apply the push label as we are in the code freeze 
stage.


Can you advise on how to proceed, please?

[/SAMI]


- I think we should have a "Fixes:" tag in the commit message (for
pointing out the commit that should have contained the code being
added/updated now)

- I think we should have a BZ too (also linked into the commit message).

Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111483): https://edk2.groups.io/g/devel/message/111483
Mute This Topic: https://groups.io/mt/102709920/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 4/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix NULL_RETURNS Coverity issue

2023-11-20 Thread Michael Kubacki

On 11/15/2023 5:10 AM, Laszlo Ersek wrote:

On 11/14/23 20:37, Michael Kubacki wrote:

On 11/14/2023 11:21 AM, Michael D Kinney wrote:

Hi Ranbir,

First I want to recognize your efforts to collect Coverity issues and
propose changes to address

them.

I still disagree with adding CpuDealLoop() for any static analysis
issues.

There have been previous discussions about adding a PANIC() or FATAL()
macro that would

perform platform specific actions if a condition is detected where the
boot of the platform

can not continue.  A platform get to make the choice to log or reboot
or hang, etc.  Not the

code that detected the condition.

https://edk2.groups.io/g/devel/message/86597



After going through hundreds of edk2 static analysis findings, we found
a small number of cases where an interface such as PanicLib was useful
and recently added an implementation
https://github.com/microsoft/mu_basecore/blob/release/202302/MdePkg/Include/Library/PanicLib.h.

For example, the return value from calls to MpInitLibWhoAmI() in
exception related code often goes unchecked and it's been used there.
Being able to separate the library instance implementation linked to a
given module from a more broad library class like DebugLib for these
cases has also been helpful.


Ah, great reminder that we have ANALYZER_UNREACHABLE. I've totally
forgotten about that; my apologies.

... I initially thought that a plain "CONST CHAR8  *Description" was not
too flexible, but on second thought, it should be exactly right. Reason
being, it's very easy to print. Format specifiers and variable arguments
(PrintLib style) may be too complex to implement safely within
PanicReport(). Arguably, no PanicReport() implementation should be
obligated (by the interface) to depend on, or to reimplement, PrintLib.
If the calling context permits, the caller can just use PrintLib to
format the message to a local buffer (on the stack), and pass that to
PanicReport.

So this looks very useful to me; can you upstream it?

Thanks for taking a look. We definitely want to align with edk2 on a 
consistent way to handle these scenarios. We'll send a patch for further 
discussion and review.


We've waited to do so to allow the original author (Ken Lautner) to 
return from his time out of office to send that patch.


- Michael


Laszlo





Unfortunately, in order to fix some of these static analysis issues
correctly, we are going

to have to identify the use of ASSERT() that really is a fatal
condition that can not continue

and introduce an implementation approach that provides a platform
handler and

satisfies the static analysis tools.

We also have to evaluate if a return error status with a DEBUG_ERROR
message would be a better

choice than an ASSERT() that can be filtered out by build options.

Best regards,

Mike

*From:* devel@edk2.groups.io  *On Behalf Of
*Ranbir Singh
*Sent:* Tuesday, November 14, 2023 7:08 AM
*To:* Laszlo Ersek 
*Cc:* devel@edk2.groups.io; Ni, Ray ; Veeresh
Sangolli 
*Subject:* Re: [edk2-devel] [PATCH v2 4/5]
MdeModulePkg/Bus/Pci/PciBusDxe: Fix NULL_RETURNS Coverity issue

On Mon, Nov 13, 2023 at 4:58 PM Laszlo Ersek mailto:ler...@redhat.com>> wrote:

     On 11/10/23 07:11, Ranbir Singh wrote:
  > EFI_NOT_READY was listed as one of the error return values in the
  > function header of StartPciDevices(). So, I considered it here.
  >
  > Maybe we can go by a dual approach, including CpuDeadLoop() in
  > StartPciDevices() as well as add return value check at the call
     site in
  > PciBusDriverBindingStart().

     I don't think this makes much sense, given that execution is not
     supposed to continue past CpuDeadLoop(), so the new error check would
     not be reached.

     I think two options are realistic:

     - replace the assert() with a CpuDeadLoop() -- this is my preference

     - keep the assert, add the "if", and in the caller, handle the
     EFI_NOT_READY error. This is workable too. (Keeping the assert is
goog
     because it shows that we don't expect the "if" to fire.)

     Anyway, now that we have no way to verify the change against
Coverity, I
     don't know if this patch is worth the churn. :( As I said, this
ASSERT()
     is one of those few justified ones in edk2 core that can indeed never
     fail, IMO.

     Laszlo

See, Does the following change look acceptable to you ?

      ASSERT (RootBridge != NULL);
+  if (RootBridge == NULL) {

+    CpuDeadLoop ();
+    return EFI_NOT_READY;
+  }

+

which retains the existing assert, adds the NULL pointer check and
includes CpuDeadLoop () within the if block. As a result of
CpuDeadLoop (), the return statement afterwards will never reach
execution (=> no need to handle this return value at the call sites),
but will satisfy static analysis tools as the "RootBridge" dereference
scenario doesn't arise thereafter.


  >
  > On Tue, Nov 7, 2023 at 10:18 PM Laszlo Ersek 

Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4 installed after RestEx

2023-11-20 Thread Igor Kulchytskyy via groups.io
Hi Abner,
I have an account.
I will create a ticket.
Thank you,
Igor

Get Outlook for Android

From: Chang, Abner 
Sent: Sunday, November 19, 2023 11:31:37 PM
To: Laszlo Ersek ; devel@edk2.groups.io 
; Igor Kulchytskyy 
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue 
if IPv4 installed after RestEx


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

[AMD Official Use Only - General]

Yes Laszlo, we need one for this.
Hi Igor, do you have an account on Bugzilla? We need a ticket for this issue.

Thnaks
Abner

> -Original Message-
> From: Laszlo Ersek 
> Sent: Friday, November 17, 2023 5:11 PM
> To: devel@edk2.groups.io; ig...@ami.com; Chang, Abner
> 
> Subject: Re: [edk2-devel] [PATCH v6 0/2] Fix and optimize the issue if IPv4
> installed after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 11/15/23 23:12, Igor Kulchytskyy via groups.io wrote:
> > Igor Kulchytskyy (2):
> >   RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after
> > RestEx
> >   RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow
> >
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 225 --
> >  .../RedfishDiscoverInternal.h |   4 +
> >  2 files changed, 158 insertions(+), 71 deletions(-)
>
> Should this patch series be highlighted in the release plan? Is there a BZ?
>
> Laszlo

-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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111480): https://edk2.groups.io/g/devel/message/111480
Mute This Topic: https://groups.io/mt/102615650/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 6/6] CryptoPkg: Enable Openssl Accel builds for AARCH64

2023-11-20 Thread PierreGondois
Enable the following modules builds for AARCH64:
- OpensslLibAccel.inf
- OpensslLibFullAccel.inf

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
 CryptoPkg/CryptoPkg.dsc   | 23 ++-
 .../OpensslLib/OpensslStub/AArch64Cap.c   | 15 
 CryptoPkg/Readme.md   | 14 +--
 3 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index 6a0104a3bb68..f08808d5d18e 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -5,6 +5,7 @@
 #  Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.
 #  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
reserved.
 #  Copyright (c) 2022, Loongson Technology Corporation Limited. All rights 
reserved.
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -327,7 +328,7 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   MSFT:NOOPT_*_*_DLINK_FLAGS = 
/EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x1
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf {
 
   FILE_GUID = B91B9A95-4D52-4501-A98F-A1711C14ED93
@@ -396,9 +397,9 @@ [Components]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # Build verification of IA32/X64 specific libraries
+  # Build verification of IA32/X64/AARCH64 specific libraries
   #
   CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
   CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -439,9 +440,9 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # CryptoPei with IA32/X64 performance optimized OpensslLib instance without 
EC services
+  # CryptoPei with IA32/X64/AARCH64 performance optimized OpensslLib instance 
without EC services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoPei.inf {
@@ -455,7 +456,7 @@ [Components.IA32, Components.X64]
   }
 
   #
-  # CryptoPei with IA32/X64 performance optimized OpensslLib instance all 
services
+  # CryptoPei with IA32/X64/AARCH64 performance optimized OpensslLib instance 
all services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoPei.inf {
@@ -505,9 +506,9 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # CryptoDxe with IA32/X64 performance optimized OpensslLib instance with no 
EC services
+  # CryptoDxe with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with no EC services
   # with TLS feature enabled.
   # IA32/X64 assembly optimizations required larger alignments
   #
@@ -521,7 +522,7 @@ [Components.IA32, Components.X64]
   MSFT:*_*_X64_DLINK_FLAGS  = /ALIGN:256
   }
   #
-  # CryptoDxe with IA32/X64 performance optimized OpensslLib instance with all 
services.
+  # CryptoDxe with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with all services.
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoDxe.inf {
@@ -561,7 +562,7 @@ [Components.IA32, Components.X64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
   #
-  # CryptoSmm with IA32/X64 performance optimized OpensslLib instance with no 
EC services
+  # CryptoSmm with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with no EC services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoSmm.inf {
@@ -574,7 +575,7 @@ [Components.IA32, Components.X64]
   MSFT:*_*_X64_DLINK_FLAGS  = /ALIGN:256
   }
   #
-  # CryptoSmm with IA32/X64 performance optimized OpensslLib instance with all 
services
+  # CryptoSmm with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with all services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoSmm.inf {
diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c 
b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
index 7468ef3ab54e..faee753b0fdf 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
+++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
@@ -43,28 +43,23 @@ OPENSSL_cpuid_setup (
   */
   OPENSSL_armcap_P |= ARMV7_NEON;
 
-  if (ArmHasAes ())
-  {
+  if (ArmHasAes ()) {
 OPENSSL_armcap_P |= ARMV8_AES;
   }
 
-  if (ArmHasSha1 ())
-  {
+  if (ArmHasSha1 ()) {
 

[edk2-devel] [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks

2023-11-20 Thread PierreGondois
Add AARCH64 specific implementations of:
- OPENSSL_cpuid_setup(), probing hardware capabilitie
  (presence of FEAT_AES, etc.)
- OPENSSL_rdtsc(), returning non-trusted entropy by accessing
  system counter.

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
 .../Library/OpensslLib/OpensslLibAccel.inf|  7 ++
 .../OpensslLib/OpensslLibFullAccel.inf|  7 ++
 .../OpensslLib/OpensslStub/AArch64Cap.c   | 84 +++
 3 files changed, 98 insertions(+)
 create mode 100644 CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
index 3d1a9638b1c1..08e8be6ea9e1 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
@@ -1329,6 +1329,7 @@ [Sources.X64]
 # Autogenerated files list ends here
 
 [Sources.AARCH64]
+  OpensslStub/AArch64Cap.c
 # Autogenerated files list starts here
   $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
   $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -1955,11 +1956,17 @@ [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
 
+[Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
 [LibraryClasses]
   BaseLib
   DebugLib
   RngLib
 
+[LibraryClasses.AARCH64]
+  ArmLib
+
 [BuildOptions]
   #
   # Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
index e7e83d419f4b..2a01ffe06bd7 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -1432,6 +1432,7 @@ [Sources.X64]
 # Autogenerated files list ends here
 
 [Sources.AARCH64]
+  OpensslStub/AArch64Cap.c
 # Autogenerated files list starts here
   $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
   $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -2107,11 +2108,17 @@ [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
 
+[Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
 [LibraryClasses]
   BaseLib
   DebugLib
   RngLib
 
+[LibraryClasses.AARCH64]
+  ArmLib
+
 [BuildOptions]
   #
   # Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c 
b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
new file mode 100644
index ..7468ef3ab54e
--- /dev/null
+++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
@@ -0,0 +1,84 @@
+/** @file
+  Arm capabilities probing.
+
+  Copyright (c) 2023, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include "crypto/arm_arch.h"
+
+#include 
+
+/** Get bits from a value.
+
+  Shift the input value from 'shift' bits and apply 'mask'.
+
+  @param   valueThe value to get the bits from.
+  @param   shiftIndex of the bits to read.
+  @param   mask Mask to apply to the value once shifted.
+
+  @return  The desired bitfield from the value.
+**/
+#define GET_BITFIELD(value, shift, mask)\
+  ((value >> shift) & mask)
+
+UINT32  OPENSSL_armcap_P = 0;
+
+void
+OPENSSL_cpuid_setup (
+  void
+  )
+{
+  OPENSSL_armcap_P = 0;
+
+  /* Access to EL0 registers is possible from higher ELx. */
+  OPENSSL_armcap_P |= ARMV8_CPUID;
+  /* Access to Physical timer is possible. */
+  OPENSSL_armcap_P |= ARMV7_TICK;
+
+  /* Neon support is not guaranteed, but it is assumed to be present.
+ Arm ARM for Armv8, sA1.5 Advanced SIMD and floating-point support
+  */
+  OPENSSL_armcap_P |= ARMV7_NEON;
+
+  if (ArmHasAes ())
+  {
+OPENSSL_armcap_P |= ARMV8_AES;
+  }
+
+  if (ArmHasSha1 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA1;
+  }
+
+  if (ArmHasSha256 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA256;
+  }
+
+  if (ArmHasPmull ())
+  {
+OPENSSL_armcap_P |= ARMV8_PMULL;
+  }
+
+  if (ArmHasSha512 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA512;
+  }
+}
+
+/** Read system counter value.
+
+  Used to get some non-trusted entropy.
+
+  @return Lower bits of the physical counter.
+**/
+uint32_t
+OPENSSL_rdtsc (
+  void
+  )
+{
+  return (UINT32)ArmReadCntPct ();
+}
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111478): https://edk2.groups.io/g/devel/message/111478
Mute This Topic: https://groups.io/mt/102707267/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 3/6] CryptoPkg/OpensslLib: Add native instruction support for AARCH64

2023-11-20 Thread PierreGondois
Add native instruction support for AARCH64.

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
 .../Library/OpensslLib/OpensslLibAccel.inf| 21 ++-
 .../OpensslLib/OpensslLibFullAccel.inf| 21 ++-
 CryptoPkg/Library/OpensslLib/UefiAsm.conf |  6 ++
 CryptoPkg/Library/OpensslLib/configure.py |  6 +-
 4 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
index a37347fbbfd6..a47e9087133b 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
@@ -5,6 +5,7 @@
 #
 #  Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.
 #  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -24,9 +25,10 @@ [Defines]
   DEFINE OPENSSL_FLAGS   = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT 
-D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DEDK2_OPENSSL_NOEC=1
   DEFINE OPENSSL_FLAGS_IA32  = -DAES_ASM -DGHASH_ASM -DMD5_ASM 
-DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM
   DEFINE OPENSSL_FLAGS_X64   = -DAES_ASM -DBSAES_ASM -DGHASH_ASM 
-DKECCAK1600_ASM -DMD5_ASM -DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM 
-DSHA512_ASM -DVPAES_ASM
+  DEFINE OPENSSL_FLAGS_AARCH64   =
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64
+#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
 #
 
 [Sources]
@@ -1326,6 +1328,10 @@ [Sources.X64]
   $(OPENSSL_GEN_PATH)/X64-GCC/crypto/sha/sha512-x86_64.s | GCC
 # Autogenerated files list ends here
 
+[Sources.AARCH64]
+# Autogenerated files list starts here
+# Autogenerated files list ends here
+
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -1403,3 +1409,16 @@ [BuildOptions]
   #   commit d9b8b89bec4480de3a10bdaf9425db371c19145b, and can be dropped 
then.)
   XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_IA32) -w -std=c99 -Wno-error=uninitialized
   XCODE:*_*_X64_CC_FLAGS= -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_X64) -w -std=c99 -Wno-error=uninitialized
+
+  GCC:*_*_AARCH64_CC_FLAGS= $(OPENSSL_FLAGS) $(OPENSSL_FLAGS_AARCH64) 
-Wno-error=format -Wno-format -D_BITS_STDINT_UINTN_H -D_BITS_STDINT_INTN_H
+
+  #
+  # AARCH64 uses strict alignment and avoids SIMD registers for code that may 
execute
+  # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as 
BASE
+  # libraries, given that they may be included into such modules.
+  # This library, even though of the BASE type, is never used in such cases, 
and
+  # avoiding the SIMD register file (which is shared with the FPU) prevents the
+  # compiler from successfully building some of the OpenSSL source files that
+  # use floating point types, so clear the flags here.
+  #
+  GCC:*_*_AARCH64_CC_XIPFLAGS ==
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
index 780d5febd72e..45a58c7d382e 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -10,6 +10,7 @@
 #
 #  Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.
 #  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -29,9 +30,10 @@ [Defines]
   DEFINE OPENSSL_FLAGS   = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT 
-D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE
   DEFINE OPENSSL_FLAGS_IA32  = -DAES_ASM -DGHASH_ASM -DMD5_ASM 
-DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM
   DEFINE OPENSSL_FLAGS_X64   = -DAES_ASM -DBSAES_ASM -DGHASH_ASM 
-DKECCAK1600_ASM -DMD5_ASM -DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM 
-DSHA512_ASM -DVPAES_ASM
+  DEFINE OPENSSL_FLAGS_AARCH64   =
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64
+#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
 #
 
 [Sources]
@@ -1429,6 +1431,10 @@ [Sources.X64]
   $(OPENSSL_GEN_PATH)/X64-GCC/crypto/sha/sha512-x86_64.s | GCC
 # Autogenerated files list ends here
 
+[Sources.AARCH64]
+# Autogenerated files list starts here
+# Autogenerated files list ends here
+
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -1506,3 +1512,16 @@ [BuildOptions]
   #   commit d9b8b89bec4480de3a10bdaf9425db371c19145b, and can be dropped 
then.)
   XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_IA32) -w -std=c99 -Wno-error=uninitialized
   XCODE:*_*_X64_CC_FLAGS= -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_X64) -w -std=c99 -Wno-error=uninitialized
+
+  GCC:*_*_AARCH64_CC_FLAGS= $(OPENSSL_FLAGS) $(OPENSSL_FLAGS_AARCH64) 
-Wno-error=format 

[edk2-devel] [PATCH v4 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

2023-11-20 Thread PierreGondois
Allow dependency upon ArmPkg to pass the dependency Check.

Signed-off-by: Pierre Gondois 
---
 CryptoPkg/CryptoPkg.ci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
index f961d85927c0..3bbb220d3224 100644
--- a/CryptoPkg/CryptoPkg.ci.yaml
+++ b/CryptoPkg/CryptoPkg.ci.yaml
@@ -69,6 +69,7 @@
 },
 "DependencyCheck": {
 "AcceptableDependencies": [
+"ArmPkg/ArmPkg.dec",
 "MdePkg/MdePkg.dec",
 "MdeModulePkg/MdeModulePkg.dec",
 "CryptoPkg/CryptoPkg.dec",
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111475): https://edk2.groups.io/g/devel/message/111475
Mute This Topic: https://groups.io/mt/102707264/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v4 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 register

2023-11-20 Thread PierreGondois
Add macro definitions and helper functions to access AArch64
capabilities described in the AA64Isar0 register.

Signed-off-by: Pierre Gondois 
---
 ArmPkg/Include/Chipset/AArch64.h   |  60 +++-
 ArmPkg/Include/Library/ArmLib.h| 228 -
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 367 +
 3 files changed, 653 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
index 5390bf0a2774..97c20e71a811 100644
--- a/ArmPkg/Include/Chipset/AArch64.h
+++ b/ArmPkg/Include/Chipset/AArch64.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -127,6 +127,64 @@
 // build for ARMv8.0, we need to define the register here.
 #define ID_AA64MMFR2_EL1  S3_0_C0_C7_2
 
+//
+// Bit shifts for the ID_AA64ISAR0_EL1 register.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_SHIFT (4U)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_SHIFT(8U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_SHIFT(12U)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_SHIFT   (16U)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_SHIFT  (20U)
+#define ARM_ID_AA64ISAR0_EL1_RDM_SHIFT (28U)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_SHIFT(32U)
+#define ARM_ID_AA64ISAR0_EL1_SM3_SHIFT (36U)
+#define ARM_ID_AA64ISAR0_EL1_SM4_SHIFT (40U)
+#define ARM_ID_AA64ISAR0_EL1_DP_SHIFT  (44U)
+#define ARM_ID_AA64ISAR0_EL1_FHM_SHIFT (48U)
+#define ARM_ID_AA64ISAR0_EL1_TS_SHIFT  (52U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_SHIFT (56U)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_SHIFT(60U)
+
+//
+// Bit masks for the ID_AA64ISAR0_EL1 fields.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_MASK   (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_RDM_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SM3_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SM4_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_DP_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_FHM_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_TS_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_TLB_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_MASK(0xFU)
+
+//
+// Bit masks for the ID_AA64ISAR0_EL1 field values.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_AES_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_PMULL_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_FEAT_SHA1_MASK  (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA256_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA512_MASK(0x2U)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_HAVE_CRC32_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_FEAT_LSE_MASK (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_RDM_FEAT_RDM_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_FEAT_SHA3_MASK  (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SM3_FEAT_SM3_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SM4_FEAT_SM4_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_DP_FEAT_DOTPROD_MASK (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_FHM_FEAT_FHM_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM_MASK   (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM2_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIOS_MASK (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIRANGE_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_FEAT_RNG_MASK   (0x1U)
+
 #define VECTOR_BASE(tbl)  \
   .section .text.##tbl##,"ax";\
   .align 11;  \
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 6aa8a48f07f3..a176fcd7bf0a 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-  Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.
+  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
   Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -805,6 +805,232 @@ ArmHasEte (
   VOID
   );
 
+/** Read AA64Isar0 register.
+
+   @return AA64Isar0's register value.
+**/
+UINTN
+EFIAPI
+ArmReadIdAA64Isar0 (
+  VOID
+  );
+
+/**
+  Checks whether the CPU implements FEAT_AES.
+
+  @retval TRUE  FEAT_AES is implemented.
+  @retval FALSE FEAT_AES is not implemented.
+**/
+BOOLEAN
+EFIAPI
+ArmHasAes (
+  VOID
+  );
+
+/**
+  Checks whether the CPU implements FEAT_PMULL.
+
+  @retval TRUE  FEAT_PMULL is implemented.
+  @retval FALSE FEAT_PMULL is not implemented.
+**/
+BOOLEAN
+EFIAPI
+ArmHasPmull (
+  VOID
+  );
+
+/**
+  Checks whether the CPU implements FEAT_SHA1.
+
+  @retval TRUE  FEAT_SHA1 is implemented.
+  @retval 

[edk2-devel] [PATCH v4 0/6] CryptoPkg: Enable Openssl native instruction support for AARCH64

2023-11-20 Thread PierreGondois
v4:
- Added Acked-by from Gerd
- Created bugzilla at:
  https://bugzilla.tianocore.org/show_bug.cgi?id=4601
- Replaced:
  - [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
  by:
  - [PATCH v4 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 
register
  and:
  - Moved ARM_ID_AA64ISAR0_EL1_* macros to ArmPkg/Include/Chipset/AArch64.h
  - Added macros to get capabilities described in AA64ISAR0

v3:
- Use ArmPkg's function to access register:
  - Dropped:
- [PATCH v2 1/7] MdePkg/BaseLib: AARCH64: Add ArmReadCntPctReg()
- [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
- [PATCH v2 3/7] MdePkg/BaseRngLib: Prefer ArmReadIdAA64Isar0Reg() over 
ArmReadIdIsar0()
  - Added:
- [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
- Allow dependency of CryptoPkg over ArmPkg in CI:
  - Added:
- [PATCH v3 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon

v2:
- [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
  - Correct bad mask values in MdePkg/Include/Library/BaseLib.h
- [PATCH v2 4/7] CryptoPkg/OpensslLib: Add native instruction support:
  - Add armcap.c to configure.py:sources_filter_fn() instead of
manually commenting the file in .inf files

Various OpensslLib implementations are available in edk2. The
OpensslLibAccel.inf and OpensslLibFullAccel.inf ones use
architecture specific instructions, e.g. AESE, PMULL, SHA256H, ...,
allowing to improve speed.

Enable support for Aarch64's native instructions:
- Add ArmReadCntPctReg() and ArmReadIdAA64Isar0Reg() to
  Aarch64's BaseLib.
- Generate Aarch64's specific Openssl functions.
- Add a OpensslStub/AArch64Cap.c file to allow Openssl
  to probe Aarch64 native instruction support.

This patch-set only enable support for GCC for now (MSFT support
not added).

 Testing 

The tests run are based on the TestBaseCryptLibShell module.
Each test is run 100 times, then the first 5 values
(considered as warmup) are removed.

The NoAccel column relies on the OpensslLibFull implementation,
the Accel column relies on the OpensslLibFullAccel implementation.
The 'Improvement' column is computed as:
   100 * ('Accel (ns)' - 'NoAccel (ns)') / 'NoAccel (ns)'

The std deviation of the TestVerifyDhGenerateKey is big.
It is due to [1] being called with the 'safe' parameter set, leading
to the prime number taking more time to generate. It requires ~10
iterations when safe=false, ~1000 iterations when safe=true.

The test was run on a Juno-r2. The native Openssl implementation makes
use of the following features (cf. [2]):
- ARMV7_NEON
- ARMV8_AES
- ARMV8_SHA1
- ARMV8_PMULL
- ARMV8_SHA256
and misses:
- ARMV8_SHA512

| TestName |   NoAccel (ns) |   NoAccel std |   Accel 
(ns) |   Accel std |   Improvement |
|:-|---:|--:|-:|:|--:|
| mPkcs7EkuTest|   14757511 | 14370 | 
14947276 |   35677 |   1.28589 |
| mAeadAesGcmTest  | 129667 |  2012 |   
113897 |1366 | -12.1619  |
| mBlockCipherTest |   7325 |   102 | 
6487 |  81 | -11.4403  |
| mAuthenticodeTest|   72852444 |   3097832 | 
67593102 | 3123627 |  -7.21917 |
| mBnTest  | 771921 | 57966 |   
737656 |   61354 |  -4.43893 |
| mDhTest  | 4082083501 |3340300622 |   
3502629757 |  3444890110 | -14.195   |
| mEcTest  |   24666075 |191971 | 
23250301 |  178985 |  -5.73976 |
| mHkdfTest| 848440 |  4295 |   
797966 |4320 |  -5.94904 |
| mHmacTest| 235527 | 36284 |   
204823 |   37936 | -13.0363  |
| mImageTimestampTest  |   12801070 | 18327 | 
12190046 |   23138 |  -4.77323 |
| mOaepTest|   20032245 | 46525 | 
18671388 |   36399 |  -6.79333 |
| mPkcs5Test   | 178624 |  1962 |   
114852 |1376 | -35.7018  |
| mPkcs7Test   |   28464572 | 70683 | 
25282753 |   82616 | -11.1782  |
| mPrngTest| 727013 |  3637 |   
460076 |2668 | -36.717   |
| mRsaCertTest |   39109865 | 90380 | 
36452412 |  220712 |  -6.79484 |
| mRsaTest |   22451367 | 60643 | 
16672060 |   53643 | -25.7414  |
| mRsaPssTest  |  142051533 |122172 | 
98638975 |   99131 | -30.5611  |
| mHashTest|  22033 |  6308 |
17650 |

Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use processor extended information

2023-11-20 Thread Wu, Jiaxin
For core id in cpu features library, I agree it should be not easy or simple 
change to 0x1f.

But in SMM CPU, there is no usage case depends on the number of cores retrieved 
from cupid 0x0b return value, only PackageId will be used. So, this patch 
doesn’t do bad things, should no regression issue. I agree with Ray’s 
explanation that  “CPUID.0B.PackageId == CPUID.1F.PackageId”, thus no need 
update for the PackageId update.

I checked the latest SDM:

“The sub-leaves of CPUID leaf 0BH describe an ordered hierarchy of logical 
processors starting from the smallest-scoped domain of a Logical Processor 
(sub-leaf index 0) to the Core domain (sub-leaf index 1) to the largest-scoped 
domain (the last valid sub-leaf index) *that is implicitly subordinate to the 
unenumerated highest-scoped domain of the processor package (socket)*”

Looks it already updated to indicate the largest-scoped domain is package.

With all above, I agree to drop this path, but other 2 patches in this set 
should be ok. Thanks Ray help clarify this.

Thanks,
Jiaxin

From: Ni, Ray 
Sent: Monday, November 20, 2023 9:45 AM
To: Laszlo Ersek ; devel@edk2.groups.io; Wu, Jiaxin 

Cc: Dong, Eric ; Kumar, Rahul R ; 
Gerd Hoffmann ; Zeng, Star 
Subject: Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use 
processor extended information

let me add more to explain:

1. CPUID.0B.PackageId == CPUID.1F.PackageId

SDM clearly states the scope of every MSR (public): package, core, or thread.
But SDM doesn't emphasize that if a MSR is package scope, it's within the 
package defined by CPUID.0B or CPUID.1F.
That implies, CPUID.0B and CPUID.1F should return the same value for package ID.

Also, SDM has following statement to explain result of EAX for CPUID.0B and 
CPUID.1F:
Bits 04-00: The number of bits that the x2APIC ID must be shifted to the 
right to address instances of the "next higher-scoped"​ domain.

That means when CPUID.0B returns the EAX[04:00], it returns the total bits of 
"thread", "core", "module", "tie", "die" because "package" is
the next higher-scoped domain.

That also supports the idea: CPUID.0B.PackageId == CPUID.1F.PackageId.

2. CPU Feature Initialization

In UefiCpuPkg/Include/RegisterCpuFeaturesLib.h, the following macros were added 
to support consumers of RegisterCpuFeaturesLib specify
dependencies among different features.
For example, when feature #a PACKAGE_BEFORE feature #b, #b is performed in one 
thread of a package and after all threads have performed #a.
That means internally multi-thread-sync is used to guarantee the dependencies.
#define CPU_FEATURE_THREAD_BEFORE   BIT25
#define CPU_FEATURE_THREAD_AFTERBIT26
#define CPU_FEATURE_CORE_BEFORE BIT27
#define CPU_FEATURE_CORE_AFTER  BIT28
#define CPU_FEATURE_PACKAGE_BEFORE  BIT29
#define CPU_FEATURE_PACKAGE_AFTER   BIT30

But above 3 sets of macro only define the dependencies between 3 scopes: 
thread, core and package.
Other scopes were not supported as there is no MSR which belongs to other 
scopes at that moment, even today.
So, the cpu features library implementation also only depends on CPUID.0B.
If we update the code to get package id from CPUID.1F, to be consistent, we 
should also get the core id from CPUID.1F.
But if we do that, the number of cores which belong to the same domain could be 
less in CPUID.1F. As CPUID.1F returns
the number of cores per module, instead of per package.
That will break the MP sync logic which depends on the number of cores per each 
domain.

Conclusion: we should not update code to use CPUID.1F as it will break the 
MP-sync logic in RegisterCpuFeaturesLib which is not aware of more than 3 
layers of scopes.

Thanks,
Ray


From: Laszlo Ersek mailto:ler...@redhat.com>>
Sent: Saturday, November 18, 2023 5:05 AM
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Ni, Ray 
mailto:ray...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Cc: Dong, Eric mailto:eric.d...@intel.com>>; Kumar, Rahul 
R mailto:rahul.r.ku...@intel.com>>; Gerd Hoffmann 
mailto:kra...@redhat.com>>; Zeng, Star 
mailto:star.z...@intel.com>>
Subject: Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use 
processor extended information

On 11/16/23 02:30, Ni, Ray wrote:
> I cannot remember if CPUID.0B and CPUID.1F return the same value for
> package ID.
>
> And I am not sure about the benefit if we get the package id from location2.

Isn't the benefit that Location2 / CPUID leaf 1F is fully specified,
while leaf 0B isn't? From the commit message it seems we should always
prefer leaf 1F and Location2, even if we're not aware of concrete
problems with leaf 0B.

Do you think we should only merge patches #1 and #2?

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111472): https://edk2.groups.io/g/devel/message/111472
Mute This Topic: https://groups.io/mt/102602853/21656
Group Owner: 

Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

2023-11-20 Thread Sami Mujawar
Hi Ashish,

I have created a bugzilla ticket for this at 
https://bugzilla.tianocore.org/show_bug.cgi?id=4600.

Regards,

Sami Mujawar


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111471): https://edk2.groups.io/g/devel/message/111471
Mute This Topic: https://groups.io/mt/102598848/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command line

2023-11-20 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Chang, Abner ;
> g...@danielschaefer.me; jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 2/2] EmbeddedPkg: Allow longer android kernel command
> line
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> AndroidBootImgLib allows for platforms to append to kernel command
> line but does not allow for the overall kernel command line to go
> beyond the limit set by the image header. Address this limitation
> by adding a pcd where platform can tell how many extra characters
> they expect on their platform in addition to what the image header
> specifies.
>
> Signed-off-by: Ashish Singhal 
> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 5 +
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c   | 2 +-
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec
> b/EmbeddedPkg/EmbeddedPkg.dec
> index 341ef5e6a6..94dc3c9b76 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -183,3 +183,8 @@
># Selection between DT and ACPI as a default
>#
>gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x059
> +
> +  #
> +  # Expected Overflow Android Kernel Command Line Characters
> +  #
> +
> gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UI
> NT32|0x05C
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index f63648e60d..d16929f2bb 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -335,7 +335,7 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
> -  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32
> (PcdAndroidKernelCommandLineOverflow);

I don't know the history of ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, however, I am a 
little bit confused.
Is this PCD introduced for the case the string size may greater than 
ANDROID_BOOTIMG_KERNEL_ARGS_SIZE? If yes, then why don't we just use StrSize 
(ImageKernelArgs) * sizeof(UINT16) as the buffer size?

Regards,
Abner

>*KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
>if (*KernelArgs == NULL) {
>  DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> index 8eefeef4f9..9754664df5 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -45,5 +45,6 @@
>gEfiAcpiTableGuid
>gFdtTableGuid
>
> -[FeaturePcd]
> +[Pcd]
>gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
> +  gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111470): https://edk2.groups.io/g/devel/message/111470
Mute This Topic: https://groups.io/mt/102598723/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

2023-11-20 Thread Sami Mujawar

Hi Ashish,

Thank you for this fix.

These changes look good to me.

Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar

On 15/11/2023 03:19 am, Ashish Singhal wrote:

Just like CPU _UID, ETE UID also needs to be unique so
use AcpiProcessorUid instead of CpuName

Signed-off-by: Ashish Singhal 
---
  .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git 
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
 
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 8228c7845a..724f33c660 100644
--- 
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ 
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -359,6 +359,7 @@ CreateAmlCpcNode (

@param [in]  GeneratorThe SSDT Cpu Topology generator.
@param [in]  ParentNode   Parent node to attach the Cpu node to.
+  @param [in]  GicCInfo CM_ARM_GICC_INFO object used to create the node.
@param [in]  CpuName  Value used to generate the node name.
@param [out] EtNodePtr   If not NULL, return the created Cpu node.

@@ -372,6 +373,7 @@ EFIAPI
  CreateAmlEtd (
IN   ACPI_CPU_TOPOLOGY_GENERATOR  *Generator,
IN   AML_NODE_HANDLE  ParentNode,
+  IN   CM_ARM_GICC_INFO *GicCInfo,
IN   UINT32   CpuName,
OUT  AML_OBJECT_NODE_HANDLE   *EtNodePtr OPTIONAL
)
@@ -397,7 +399,7 @@ CreateAmlEtd (

Status = AmlCodeGenNameInteger (
   "_UID",
- CpuName,
+ GicCInfo->AcpiProcessorUid,
   EtNode,
   NULL
   );
@@ -474,6 +476,7 @@ CreateAmlEtNode (
Status = CreateAmlEtd (
   Generator,
   Node,
+ GicCInfo,
   CpuName,
   NULL
   );

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111469): https://edk2.groups.io/g/devel/message/111469
Mute This Topic: https://groups.io/mt/102598848/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v5 0/6] MPAM ACPI definitions and parser

2023-11-20 Thread Rohit Mathew
Hi,

Gentle reminder on the series.

Regards,
Rohit

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Rohit
> Mathew via groups.io
> Sent: Monday, October 2, 2023 6:14 PM
> To: devel@edk2.groups.io
> Cc: Michael D Kinney ; Liming Gao
> ; Thomas Abraham
> ; Sami Mujawar ;
> James Morse ; Ray Ni ; Zhichao
> Gao 
> Subject: [edk2-devel] [PATCH v5 0/6] MPAM ACPI definitions and parser
> 
> This series adds the following
>  -  definitions corresponding to MPAM ACPI 2.0 specification.
>  -  MPAM parser
> 
> An MPAM ACPI table formulated using the newly added MPAM ACPI
> definitions were validated on the linux kernel tree at [1]. The same table was
> parsed via acpiview using the newly added parser. Certain aspects of the
> MPAM ACPI specification are still not implemented by the kernel tree. These
> aspects were verified only using acpiview.
> 
> Changes since V1:
>  -  Addressed comments on MPAM ACPI definitions from Sami.
>  -  V1 did not incorporate the parser. V2 has this implemented.
> 
> Changes since V2:
>  -  Addressed comments related to Interface/Link type defintions from Sami.
> 
> Changes since v3:
>  -  Addressed comments from Pierre
> 
> Changes since v4:
>  -  Addressed comments from Sami regarding Parser.
> 
> Changes can be found at:
> https://github.com/rohit-arm/edk2/tree/mpam_acpi
> 
> CI results can be found at:
> https://github.com/tianocore/edk2/pull/4748
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/log/?h=mpa
> m/snapshot/v6.5-rc1
> 
> 
> Rohit Mathew (6):
>   MdePkg/IndustryStandard: Add definitions for MPAM ACPI specification
>   ShellPkg/AcpiView: Update field-validator prototype
>   ShellPkg/AcpiView: Update print-formatter prototype
>   ShellPkg: acpiview: Add routine to print 16 chars
>   ShellPkg: acpiview: Add routines to print reserved fields
>   ShellPkg/AcpiView: Add MPAM Parser
> 
>  MdePkg/Include/IndustryStandard/Acpi65.h 
> |7 +-
>  MdePkg/Include/IndustryStandard/Mpam.h   
> |  246 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> |  199
> ++-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> |
> 109 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c
> |   36 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> |8 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> |   28 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> |   24 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> |   14 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
> |   12 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> |   32 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
> |   18 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser
> .c   | 1276 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
> |   56 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> |   38 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> |   14 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> |   14 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> |   26 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c   |3 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.inf |3 +-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.uni |3 +-
>  21 files changed, 2033 insertions(+), 133 deletions(-)  create mode 100644
> MdePkg/Include/IndustryStandard/Mpam.h
>  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser
> .c
> 
> --
> 2.34.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#109258):
> https://edk2.groups.io/g/devel/message/109258
> Mute This Topic: https://groups.io/mt/101716889/6795330
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [rohit.mat...@arm.com]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111468): https://edk2.groups.io/g/devel/message/111468
Mute This Topic: https://groups.io/mt/102705915/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line Length Bug

2023-11-20 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

Thanks for this update and two more comments,

> -Original Message-
> From: Ashish Singhal 
> Sent: Wednesday, November 15, 2023 11:12 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Chang, Abner ;
> g...@danielschaefer.me; jbra...@nvidia.com
> Cc: Ashish Singhal 
> Subject: [PATCH v2 1/2] EmbeddedPkg: Fix Android Boot Command Line
> Length Bug
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Curently, AndroidBootImgLib expects input kernel command line
> to never exceed 256 unicode characters where the image header
> allows for 512 ascii characters. If image header allows 512
> ascii characters, similar number of unicode characters should be
> allowed at the minimum.
>
> Signed-off-by: Ashish Singhal 
> ---
>  .../AndroidBootImgLib/AndroidBootImgLib.c | 31 +++
>  1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index 1359a66db2..f63648e60d 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -322,11 +322,12 @@ AndroidBootImgGetFdt (
>  EFI_STATUS
>  AndroidBootImgUpdateArgs (
>IN  VOID  *BootImg,
> -  OUT VOID  *KernelArgs
> +  OUT VOID  **KernelArgs
>)
I know the original code doesn't have the function header. Would you mind to 
add it as we change the prototype of this function?
Or you probably would like to keep it as it for the consistence, as no function 
header for any of functions in this file.

>  {
>CHAR8   ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
>EFI_STATUS  Status;
> +  UINT32  NewKernelArgSize;
>
>// Get kernel arguments from Android boot image
>Status = AndroidBootImgGetKernelArgs (BootImg, ImageKernelArgs);
> @@ -334,16 +335,23 @@ AndroidBootImgUpdateArgs (
>  return Status;
>}
>
Do you think we have to check if the ASCII string size in ImageKernelArgs 
greater than ANDROID_BOOTIMG_KERNEL_ARGS_SIZE?

Thanks
Abner

> +  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
> +  *KernelArgs  = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
> +  if (*KernelArgs == NULL) {
> +DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
>AsciiStrToUnicodeStrS (
>  ImageKernelArgs,
> -KernelArgs,
> -ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
> +*KernelArgs,
> +NewKernelArgSize
>  );
>// Append platform kernel arguments
>if (mAndroidBootImg->AppendArgs) {
>  Status = mAndroidBootImg->AppendArgs (
> -KernelArgs,
> -ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
> +*KernelArgs,
> +NewKernelArgSize
>  );
>}
>
> @@ -616,6 +624,10 @@ AndroidBootImgBoot (
>UINTN  RamdiskSize;
>IN  VOID   *FdtBase;
>
> +  if ((Buffer == NULL) || (BufferSize == 0)) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>NewKernelArg = NULL;
>ImageHandle  = NULL;
>
> @@ -637,14 +649,7 @@ AndroidBootImgBoot (
>  goto Exit;
>}
>
> -  NewKernelArg = AllocateZeroPool
> (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
> -  if (NewKernelArg == NULL) {
> -DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> -Status = EFI_OUT_OF_RESOURCES;
> -goto Exit;
> -  }
> -
> -  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
> +  Status = AndroidBootImgUpdateArgs (Buffer, );
>if (EFI_ERROR (Status)) {
>  goto Exit;
>}
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111467): https://edk2.groups.io/g/devel/message/111467
Mute This Topic: https://groups.io/mt/102598724/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [Bug] Building NetworkPkg fails due to missing SynchronizationLib dependency of BaseCryptLib

2023-11-20 Thread CrossedCarpet
Good morning Laszlo,
Thank you for your reply. I feared this was intentional due to believing that 
it would have been caught with automated testing.
Doesn't the Azure Pipeline try to build all packages to make sure no dependency 
broke?

Also Liming, would you do me the honour of creating a Bugzilla account for this 
email?
Best regards,
CC

From: Laszlo Ersek 
Sent: 17 November 2023 16:06
To: devel@edk2.groups.io ; crossedcar...@hotmail.com 

Cc: Liming Gao (Byosoft address) 
Subject: Re: [edk2-devel] [Bug] Building NetworkPkg fails due to missing 
SynchronizationLib dependency of BaseCryptLib

On 11/17/23 13:49, CrossedCarpet wrote:
> Steps to reproduce:
> - download and setup edk2
> - run:
> build -a X64 -b DEBUG -t GCC -p NetworkPkg/NetworkPkg.dsc
>
> Get the error:
> build.py...
> /.../edk2/NetworkPkg/NetworkPkg.dsc(...): error 4000: Instance of
> library class [SynchronizationLib] is not found
> in [/.../edk2/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf] [X64]
> consumed by module [/.../edk2/NetworkPkg/TlsDxe/TlsDxe.inf]
>
> Adding this LibClass to NetworkPkg.dsc solves it:
>
> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf

This is a regression from commit 5d5be45bd111 ("CryptPkg: Enable CryptoPkg 
BaseCryptLib ParallelHash for PEI and DXE", 2022-12-02), which was made for 
.

It added a new lib class dependency to "BaseCryptLib.inf", but it didn't ensure 
that all DSC files in the tree that employed the "BaseCryptLib.inf" instance 
had a resolution for the new lib class.

Indeed it is not just NetworkPkg.dsc but also FmpDevicePkg.dsc that's affected:

$ git grep -l -F CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf -- '**dsc*' \
  | xargs -- grep --files-without-match -w SynchronizationLib --

FmpDevicePkg/FmpDevicePkg.dsc
NetworkPkg/NetworkPkg.dsc

It also *seems* to affect at least one platform in edk2-platforms:

Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc

(although I realize this last DSC file is an "include" DSC, so the missing 
dependency could be resolved in the DSC file that includes this one.)

Either way, thanks for catching this; the edk2 issue should be fixed preferably 
during the current hard feature freeze (for NetworkPkg and FmpDevicePkg).

> I tried to open a bug in bugzilla but I wasn't able to log in or create
> an account. How should I do it next time?

I think the bugzilla account creation was disabled due to spammer accounts.

The way to request an account is described here (linked from the 
bugzilla.tianocore.org landing page under link "Reporting issues"):

  https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues

(I've added Liming to the CC list of this email.)

Laszlo


> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111466): https://edk2.groups.io/g/devel/message/111466
Mute This Topic: https://groups.io/mt/102646401/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/1] StandaloneMmPkg/Core: Restart dispatcher once MmEntryPoint is registered

2023-11-20 Thread Xu, Wei6
Defer the dispatch of the remaining MM drivers once the CPU driver has
been dispatched.

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

In MmDispatcher, return immediately if the MM Entry Point was registered.
Then the MM IPL will reinvoke the MM Core Dispatcher. This is required
so MM Mode may be enabled as soon as all the dependent MM Drivers for MM
Mode have been dispatched.

Introduce a FeatureFlag PCD to control if MmDispatcher returns or not
when MmEntryPointPoint is registered. Default value is FALSE.

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Wei6 Xu 
---
 StandaloneMmPkg/Core/Dispatcher.c | 76 +++
 StandaloneMmPkg/Core/StandaloneMmCore.c   |  1 +
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 +
 StandaloneMmPkg/StandaloneMmPkg.dec   |  6 ++
 4 files changed, 86 insertions(+)

diff --git a/StandaloneMmPkg/Core/Dispatcher.c 
b/StandaloneMmPkg/Core/Dispatcher.c
index b1ccba15b060..a3983785070b 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -586,6 +586,7 @@ MmDispatcher (
   LIST_ENTRY   *Link;
   EFI_MM_DRIVER_ENTRY  *DriverEntry;
   BOOLEAN  ReadyToRun;
+  BOOLEAN  PreviousMmEntryPointRegistered;
 
   DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
 
@@ -649,6 +650,11 @@ MmDispatcher (
   DriverEntry->Initialized = TRUE;
   RemoveEntryList (>ScheduledLink);
 
+  //
+  // Cache state of MmEntryPointRegistered before calling entry point
+  //
+  PreviousMmEntryPointRegistered = gMmCorePrivate->MmEntryPointRegistered;
+
   //
   // For each MM driver, pass NULL as ImageHandle
   //
@@ -667,6 +673,22 @@ MmDispatcher (
 DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
 MmFreePages (DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
   }
+
+  if (!PreviousMmEntryPointRegistered && 
gMmCorePrivate->MmEntryPointRegistered) {
+if (FeaturePcdGet (PcdRestartMmDispatcherOnceMmEntryRegistered)) {
+  //
+  // Return immediately if the MM Entry Point was registered by the MM
+  // Driver that was just dispatched. The MM IPL will reinvoke the MM
+  // Core Dispatcher. This is required so MM Mode may be enabled as 
soon
+  // as all the dependent MM Drivers for MM Mode have been dispatched.
+  // Once the MM Entry Point has been registered, then MM Mode will be
+  // used.
+  //
+  gRequestDispatch   = TRUE;
+  gDispatcherRunning = FALSE;
+  return EFI_NOT_READY;
+}
+  }
 }
 
 //
@@ -897,6 +919,60 @@ MmAddToDriverList (
   return EFI_SUCCESS;
 }
 
+/**
+  Event notification that is fired MM IPL to dispatch the previously 
discovered MM drivers.
+
+  @param[in]   DispatchHandle  The unique handle assigned to this handler 
by MmiHandlerRegister().
+  @param[in]   Context Points to an optional handler context which 
was specified when the
+   handler was registered.
+  @param[in, out]  CommBuffer  A pointer to a collection of data in memory 
that will
+   be conveyed from a non-MM environment into 
an MM environment.
+  @param[in, out]  CommBufferSize  The size of the CommBuffer.
+
+  @return EFI_SUCCESS  Dispatcher is executed.
+
+**/
+EFI_STATUS
+EFIAPI
+MmDriverDispatchHandler (
+  IN EFI_HANDLE  DispatchHandle,
+  IN CONST VOID  *Context OPTIONAL,
+  IN OUT VOID*CommBuffer  OPTIONAL,
+  IN OUT UINTN   *CommBufferSize  OPTIONAL
+  )
+{
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
+
+  //
+  // Execute the MM Dispatcher on MM drivers that have been discovered
+  // previously but not dispatched.
+  //
+  Status = MmDispatcher ();
+
+  //
+  // Check to see if CommBuffer and CommBufferSize are valid
+  //
+  if ((CommBuffer != NULL) && (CommBufferSize != NULL)) {
+if (*CommBufferSize > 0) {
+  if (!EFI_ERROR (Status)) {
+//
+// Set the flag to show that the MM Dispatcher executed without errors
+//
+*(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_SUCCESS;
+  } else {
+//
+// Set the flag to show that the MM Dispatcher encountered an error
+//
+*(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_ERROR;
+  }
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Traverse the discovered list for any drivers that were discovered but not 
loaded
   because the dependency expressions evaluated to false.
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c 
b/StandaloneMmPkg/Core/StandaloneMmCore.c
index d221f1d1115d..e65edee6d8c2 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -84,6 +84,7 @@ EFI_MM_SYSTEM_TABLE  gMmCoreMmst = {
 // Table of MMI Handlers that are registered by the MM Core when it is 
initialized
 //
 

[edk2-devel] [PATCH 0/1] StandaloneMmPkg/Core: Restart dispatcher once MmEntryPoint is registered

2023-11-20 Thread Xu, Wei6
This patch is to add support to StandaloneMmCore for restarting MmDispatcher 
once MmEntryPoint is registered.
Please check in detail in Bugzilla and PullRequest:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4599
PR: https://github.com/tianocore/edk2/pull/5056

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 

Wei6 Xu (1):
  StandaloneMmPkg/Core: Restart dispatcher once MmEntryPoint is
registered

 StandaloneMmPkg/Core/Dispatcher.c | 76 +++
 StandaloneMmPkg/Core/StandaloneMmCore.c   |  1 +
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 +
 StandaloneMmPkg/StandaloneMmPkg.dec   |  6 ++
 4 files changed, 86 insertions(+)

-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111463): https://edk2.groups.io/g/devel/message/111463
Mute This Topic: https://groups.io/mt/102703850/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-