date:20230518

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI FRB Driver

2023-05-18 Thread Chang, Abner via groups.io

[AMD Official Use Only - General]

Thanks, typos are fixed.

Abner

> -Original Message-
> From: Oram, Isaac W 
> Sent: Friday, May 19, 2023 11:34 AM
> To: Chang, Abner ; devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) ; Nickle
> Wang ; Tinh Nguyen
> 
> Subject: RE: [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI
> FRB Driver
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Reviewed-by: Isaac Oram 
>
> Typo in "wtchdog" and "Failt".  Can be fixed prior to commit.
>
> -Original Message-
> From: abner.ch...@amd.com 
> Sent: Saturday, May 13, 2023 5:33 AM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W ; Abdul Lateef Attar
> ; Nickle Wang ; Tinh Nguyen
> 
> Subject: [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI FRB
> Driver
>
> From: Abner Chang 
>
> IpmiFrb is cloned from
> edk2-platforms/Features/Intel/OutOfBandManagement/
> IpmiFeaturePkg/Frb in order to consolidate
> edk2 system manageability support in one place.
> Uncustify is applied to C files and no functionalities are changed in this 
> patch.
>
> We will still keep the one under IpmiFeaturePkg/Frb until the reference to 
> this
> instance are removed from platforms.
>
> Signed-off-by: Abner Chang 
> Cc: Isaac Oram 
> Cc: Abdul Lateef Attar 
> Cc: Nickle Wang 
> Cc: Tinh Nguyen 
> ---
>  .../ManageabilityPkg/ManageabilityPkg.dec |   5 +
>  .../Universal/IpmiFrb/FrbDxe.inf  |  37 +++
>  .../Universal/IpmiFrb/FrbPei.inf  |  37 +++
>  .../Universal/IpmiFrb/FrbDxe.c| 212 ++
>  .../Universal/IpmiFrb/FrbPei.c|  87 +++
>  5 files changed, 378 insertions(+)
>  create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
>  create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
>  create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
>  create mode 100644 Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c
>
> diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec
> b/Features/ManageabilityPkg/ManageabilityPkg.dec
> index 38813c5f48..b0ca01094a 100644
> --- a/Features/ManageabilityPkg/ManageabilityPkg.dec
> +++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
> @@ -80,3 +80,8 @@
>
> gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|
> BOOLEAN|0x1005
>
> gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransfer
> Enable|FALSE|BOOLEAN|0x1006
>
> +[PcdsDynamic, PcdsDynamicEx]
> +
> +gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag|TRUE|BOOLEAN|0
> x2
> +001
> +  ## This is the timeout value in milliseconds, default set to 360
> +milliseconds
> +  # @Prompt IPMI Fault Resilient Booting timeout value in milliseconds.
> +
> +gManageabilityPkgTokenSpaceGuid.PcdFRBTimeoutValue|360|UINT16|0x2
> 00
> +2
> diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> new file mode 100644
> index 00..ae57fe7697
> --- /dev/null
> +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> @@ -0,0 +1,37 @@
> +### @file
> +# Component description file for IPMI FRB.
> +#
> +# Copyright (c) 2018 - 2019, Intel Corporation. All rights
> +reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
> +
> +[defines]
> +  INF_VERSION  = 0x00010005
> +  BASE_NAME= FrbDxe
> +  FILE_GUID= A142CEE5-99D5-4ECF-943E-D8F0DE3052AA
> +  MODULE_TYPE  = DXE_DRIVER
> +  VERSION_STRING   = 1.0
> +  ENTRY_POINT  = FrbDxeEntryPoint
> +
> +[Sources]
> +  FrbDxe.c
> +
> +[Packages]
> +  ManageabilityPkg/ManageabilityPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  UefiLib
> +  BaseMemoryLib
> +  DebugLib
> +  IpmiCommandLib
> +  MemoryAllocationLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Depex]
> +  TRUE
> diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> new file mode 100644
> index 00..89d633f32e
> --- /dev/null
> +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> @@ -0,0 +1,37 @@
> +### @file
> +# Component description file for IPMI FRB PEIM.
> +#
> +# Copyright (c) 2018 - 2019, Intel Corporation. All rights
> +reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
> +
> +[defines]
> +  INF_VERSION  = 0x00010005
> +  BASE_NAME= FrbPei
> +  FILE_GUID= 7CAAE1A7-0B37-4EEA-A432-2F203DA6F288
> +  MODULE_TYPE  = PEIM
> +  VERSION_STRING   = 1.0
> +  ENTRY_POINT  = InitializeFrbPei
> +
> +[Sources]
> +  FrbPei.c
> +
> +[Packages]
> +  ManageabilityPkg/ManageabilityPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  IpmiCommandLib
> +  PcdLib
> +

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiFrb: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Saturday, May 13, 2023 5:33 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiFrb: Add 
to ManageabilityPkg

From: Abner Chang 

Add IpmiFrb to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 2 ++
 Features/ManageabilityPkg/Include/Manageability.dsc | 8 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 3 +++
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 Features/ManageabilityPkg/Include/PreMemory.fdf | 4 
 5 files changed, 21 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index b0ca01094a..3980931424 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -79,6 +79,8 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
+ gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmFrb|FALSE|BOOLEA
+ N|0x100B  
+ gManageabilityPkgTokenSpaceGuid.PcdManageabilityPeiIpmFrb|FALSE|BOOLEA
+ N|0x100C
 
 [PcdsDynamic, PcdsDynamicEx]
   gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag|TRUE|BOOLEAN|0x2001
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..06fed828b2 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -30,6 +30,10 @@
   ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf
 !endif
 
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityPeiIpmFrb == TRUE
+  ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
+!endif
+
 [Components.X64, Components.AARCH64]
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiEnable == TRUE
   ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe.inf
@@ -51,3 +55,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmFrb == TRUE
+  ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..6fa3e3c6ae 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,6 +37,8 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmFrb  
|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityPeiIpmFrb  
|TRUE
 
 #
 # Include common libraries
@@ -53,5 +55,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..84e7ea978b 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmFrb == TRUE
+  INF ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
+!endif
diff --git a/Features/ManageabilityPkg/Include/PreMemory.fdf 
b/Features/ManageabilityPkg/Include/PreMemory.fdf
index 16e079f494..add36e2095 100644
--- a/Features/ManageabilityPkg/Include/PreMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PreMemory.fdf
@@ -10,3 +10,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityPeiIpmiEnable == TRUE
   INF  ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityPeiIpmFrb == TRUE
+  INF ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
+!endif
--
2.37.1.windows.1








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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI FRB Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

Typo in "wtchdog" and "Failt".  Can be fixed prior to commit.

-Original Message-
From: abner.ch...@amd.com  
Sent: Saturday, May 13, 2023 5:33 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI FRB Driver

From: Abner Chang 

IpmiFrb is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/Frb in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/Frb until the reference to this 
instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../ManageabilityPkg/ManageabilityPkg.dec |   5 +
 .../Universal/IpmiFrb/FrbDxe.inf  |  37 +++
 .../Universal/IpmiFrb/FrbPei.inf  |  37 +++
 .../Universal/IpmiFrb/FrbDxe.c| 212 ++
 .../Universal/IpmiFrb/FrbPei.c|  87 +++
 5 files changed, 378 insertions(+)
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..b0ca01094a 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -80,3 +80,8 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
 
+[PcdsDynamic, PcdsDynamicEx]
+  
+gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag|TRUE|BOOLEAN|0x2
+001
+  ## This is the timeout value in milliseconds, default set to 360 
+milliseconds
+  # @Prompt IPMI Fault Resilient Booting timeout value in milliseconds.
+  
+gManageabilityPkgTokenSpaceGuid.PcdFRBTimeoutValue|360|UINT16|0x200
+2
diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf 
b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
new file mode 100644
index 00..ae57fe7697
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
@@ -0,0 +1,37 @@
+### @file
+# Component description file for IPMI FRB.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[defines]
+  INF_VERSION  = 0x00010005
+  BASE_NAME= FrbDxe
+  FILE_GUID= A142CEE5-99D5-4ECF-943E-D8F0DE3052AA
+  MODULE_TYPE  = DXE_DRIVER
+  VERSION_STRING   = 1.0
+  ENTRY_POINT  = FrbDxeEntryPoint
+
+[Sources]
+  FrbDxe.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  UefiLib
+  BaseMemoryLib
+  DebugLib
+  IpmiCommandLib
+  MemoryAllocationLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf 
b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
new file mode 100644
index 00..89d633f32e
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
@@ -0,0 +1,37 @@
+### @file
+# Component description file for IPMI FRB PEIM.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[defines]
+  INF_VERSION  = 0x00010005
+  BASE_NAME= FrbPei
+  FILE_GUID= 7CAAE1A7-0B37-4EEA-A432-2F203DA6F288
+  MODULE_TYPE  = PEIM
+  VERSION_STRING   = 1.0
+  ENTRY_POINT  = InitializeFrbPei
+
+[Sources]
+  FrbPei.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  IpmiCommandLib
+  PcdLib
+  PeimEntryPoint
+
+[Pcd]
+  gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag
+  gManageabilityPkgTokenSpaceGuid.PcdFRBTimeoutValue
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c 
b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
new file mode 100644
index 00..46f741eed1
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
@@ -0,0 +1,212 @@
+/** @file
+IPMI Fault Resilient Booting Driver.
+
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include  #include  
+#include  #include 
+
+/**
+  This routine disables the specified FRB timer.
+
+  @retval EFI_STATUS  EFI_SUCCESS

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiBmcAcpi: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: abner.ch...@amd.com  
Sent: Saturday, May 13, 2023 8:49 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiBmcAcpi: Add to 
ManageabilityPkg

From: Abner Chang 

Add IpmiBmcAcpi to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 1 +
 Features/ManageabilityPkg/Include/Manageability.dsc | 4 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 3 ++-
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..ac3504cb41 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -79,4 +79,5 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcAcpi|FALSE|BOOLEAN|0x100D
 
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..1a4f719dd3 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -51,3 +51,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcAcpi == TRUE
+  ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..712483faeb 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,7 +37,7 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
-
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcAcpi 
|TRUE
 #
 # Include common libraries
 #
@@ -53,5 +53,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..a23144d5aa 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcAcpi == TRUE
+  INF ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
+!endif
-- 
2.37.1.windows.1



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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI BMC ACPI Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

You could consider changing "INTEL" and ('M', 'S', 'F', 'T') to more generic 
placeholders.  But that is also something that could be looked at separately 
and more widely. 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Saturday, May 13, 2023 8:49 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: 
IPMI BMC ACPI Driver

From: Abner Chang 

IpmiBmcAcpi is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/BmcAcpi in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/BmcAcpi until the reference to 
this instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../Universal/IpmiBmcAcpi/BmcAcpi.inf |  47 
 .../Universal/IpmiBmcAcpi/BmcAcpi.c   | 264 ++
 .../Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl |  28 ++
 .../IpmiBmcAcpi/BmcSsdt/IpmiOprRegions.asi|  58 
 4 files changed, 397 insertions(+)
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
 create mode 100644 
Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/BmcSsdt.asl
 create mode 100644 
Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcSsdt/IpmiOprRegions.asi

diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf 
b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
new file mode 100644
index 00..a21c5b9b35
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.inf
@@ -0,0 +1,47 @@
+### @file
+# Component description file for BMC ACPI.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[Defines]
+  INF_VERSION   = 0x00010005
+  BASE_NAME = BmcAcpi
+  FILE_GUID = 09E3B4BE-F731-4903-AAE6-BD5D573B6140
+  MODULE_TYPE   = DXE_DRIVER
+  VERSION_STRING= 1.0
+  ENTRY_POINT   = BmcAcpiEntryPoint
+
+[Sources]
+  BmcAcpi.c
+  BmcSsdt/BmcSsdt.asl
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiFirmwareVolume2ProtocolGuid
+  gEfiAcpiTableProtocolGuid
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsIoBaseAddress
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
+
+[Depex]
+  gEfiAcpiTableProtocolGuid
+
+[BuildOptions]
+  *_*_*_ASL_FLAGS = -oi
diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c 
b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
new file mode 100644
index 00..cf066dd095
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
@@ -0,0 +1,264 @@
+/** @file
+  IPMI BMC ACPI.
+
+  Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+ reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+//
+// Statements that include other header files // #include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include  #include 
+ #include 
+
+#ifndef EFI_ACPI_CREATOR_ID
+#define EFI_ACPI_CREATOR_ID  SIGNATURE_32 ('M', 'S', 'F', 'T') #endif 
+#ifndef EFI_ACPI_CREATOR_REVISION #define EFI_ACPI_CREATOR_REVISION  
+0x010D #endif
+
+/**
+
+  Locate the first instance of a protocol.  If the protocol requested 
+ is an  FV protocol, then it will return the first FV that contains the 
+ ACPI table  storage file.
+
+  @param [in] Protocol  The protocol to find.
+  @param [in] Instance  Return pointer to the first instance of the protocol.
+  @param [in] Type  The type of protocol to locate.
+
+  @retval EFI_SUCCESS   The function completed successfully.
+  @retval EFI_NOT_FOUND The protocol could not be located.
+  @retval EFI_OUT_OF_RESOURCES  There are not enough resources to find the 
protocol.
+
+**/
+EFI_STATUS
+LocateSupportProtocol (
+  IN   EFI_GUID  *Protocol,
+  OUT  VOID  **Instance,
+  IN   UINT32Type
+  )
+{
+  EFI_STATUS  Status;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN   NumberOfHandles;
+  EFI_FV_FILETYPE FileType;
+  UINT32  FvStatus = 0;
+  EFI_FV_FILE_ATTRIBUTES  Attributes;
+  UINTN   Size;
+  UINTN   Index;
+
+  Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL, 
+ , );  if (EFI_ERROR (Status))

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiBmcElog: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Friday, May 12, 2023 2:58 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiBmcElog: 
Add to ManageabilityPkg

From: Abner Chang 

Add IpmiBmcElog to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 1 +
 Features/ManageabilityPkg/Include/Manageability.dsc | 4 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 2 ++
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 4 files changed, 11 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..7c36ec93ff 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -79,4 +79,5 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcElog|FALSE|BOOLEAN|0x100A
 
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..6806093d98 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -51,3 +51,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcElog == TRUE
+  ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..2262362f97 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,6 +37,7 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcElog 
|TRUE
 
 #
 # Include common libraries
@@ -53,5 +54,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..e4eed660fd 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiBmcElog == TRUE
+  INF ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf
+!endif
-- 
2.37.1.windows.1








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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiBmcElog: IPMI BMC Elog Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Friday, May 12, 2023 2:58 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiBmcElog: 
IPMI BMC Elog Driver

From: Abner Chang 

IpmiBmcElog is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/BmcElog in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/BmcElog until the reference to 
this instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../Universal/IpmiBmcElog/BmcElog.inf |  33 +++
 .../Universal/IpmiBmcElog/BmcElog.c   | 192 ++
 2 files changed, 225 insertions(+)
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c

diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf 
b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf
new file mode 100644
index 00..4c28862fe5
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.inf
@@ -0,0 +1,33 @@
+### @file
+# Component description file for BMC ELOG.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[Defines]
+  INF_VERSION  = 0x00010005
+  BASE_NAME= BmcElog
+  FILE_GUID= A0FF2235-B652-45E3-B3D2-B20F3E714E6F
+  MODULE_TYPE  = DXE_DRIVER
+  PI_SPECIFICATION_VERSION = 0x0001000A
+  VERSION_STRING   = 1.0
+  ENTRY_POINT  = InitializeBmcElogLayer
+
+[Sources]
+  BmcElog.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  IpmiCommandLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c 
b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
new file mode 100644
index 00..ab179e9d49
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
@@ -0,0 +1,192 @@
+/** @file
+  BMC Event Log functions.
+
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include  #include 
+
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+CheckIfSelIsFull (
+  VOID
+  );
+
+/**
+  This function erases event logs and waits unti complete.
+
+  @param [in]  ResvId  - Reserved ID
+
+  @retval  EFI_STATUS   EFI_SUCCESS
+EFI_NO_RESPONSE
+
+**/
+EFI_STATUS
+WaitTillErased (
+  IN  UINT8  *ResvId
+  )
+{
+  INTN Counter;
+  IPMI_CLEAR_SEL_REQUEST   ClearSel;
+  IPMI_CLEAR_SEL_RESPONSE  ClearSelResponse;
+
+  Counter = 0x200;
+  ZeroMem (, sizeof (ClearSelResponse));
+
+  while (TRUE) {
+ZeroMem (, sizeof (ClearSel));
+ClearSel.Reserve[0] = ResvId[0];
+ClearSel.Reserve[1] = ResvId[1];
+ClearSel.AscC   = 0x43;
+ClearSel.AscL   = 0x4C;
+ClearSel.AscR   = 0x52;
+ClearSel.Erase  = 0x00;
+
+IpmiClearSel (
+  ,
+  
+  );
+
+if ((ClearSelResponse.ErasureProgress & 0xf) == 1) {
+  return EFI_SUCCESS;
+}
+
+//
+//  If there is not a response from the BMC controller we need to return 
and not hang.
+//
+--Counter;
+if (Counter == 0x0) {
+  return EFI_NO_RESPONSE;
+}
+  }
+}
+
+/**
+  This function activates BMC event log.
+
+  @param [in] EnableElog  Enable/Disable event log  @param [out] 
+ ElogStatus  return log status
+
+  @retval  EFI_STATUS
+
+**/
+EFI_STATUS
+EfiActivateBmcElog (
+  IN BOOLEAN   *EnableElog,
+  OUT BOOLEAN  *ElogStatus
+  )
+{
+  EFI_STATUSStatus;
+  UINT8 ElogStat;
+  IPMI_SET_BMC_GLOBAL_ENABLES_REQUEST   SetBmcGlobalEnables;
+  IPMI_GET_BMC_GLOBAL_ENABLES_RESPONSE  GetBmcGlobalEnables;
+  UINT8 CompletionCode;
+
+  Status   = EFI_SUCCESS;
+  ElogStat = 0;
+
+  Status = IpmiGetBmcGlobalEnables ();  if 
+ (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (EnableElog == NULL) {
+*ElogStatus = 
+ GetBmcGlobalEnables.GetEnables.Bits.SystemEventLogging;
+  } else {
+if (Status == EFI_SUCCESS) {
+  if (*EnableElog) {
+ElogStat = 1;
+  }
+
+  CopyMem (, (UINT8 *) + 1, sizeof 
(UINT8));
+  SetBmcGlobalEnables.SetEnables.Bits.SystemEventLogging = 
+ ElogStat;
+
+  Status = IpmiSetBmcGlobalEnables (, );
+}
+  }
+
+  return

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiSolStatus: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Friday, May 12, 2023 12:29 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-devel] [edk2-platforms][PATCH 2/2] 
ManageabilityPkg/IpmiSolStatus: Add to ManageabilityPkg

From: Abner Chang 

Add IpmiSolStatus to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 1 +
 Features/ManageabilityPkg/Include/Manageability.dsc | 4 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 2 ++
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 4 files changed, 11 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index c11bf5d0df..6bf65feda4 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -83,4 +83,5 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiSolStatus|FALSE|BOOLEAN|0x1009
 
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..5f014b5d8d 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -51,3 +51,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiSolStatus == TRUE
+  ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..6426f84f1f 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,6 +37,7 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiSolStatus   
|TRUE
 
 #
 # Include common libraries
@@ -53,5 +54,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..2d04e736c1 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiSolStatus == TRUE
+  INF ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf
+!endif
-- 
2.37.1.windows.1








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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiSolStatus: IPMI Serail over Lan Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Friday, May 12, 2023 12:29 AM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-devel] [edk2-platforms][PATCH 1/2] 
ManageabilityPkg/IpmiSolStatus: IPMI Serail over Lan Driver

From: Abner Chang 

IpmiSolStatus is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/SolStatus in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/SolStatus until the reference 
to this instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../ManageabilityPkg/ManageabilityPkg.dec |   4 +
 .../Universal/IpmiSolStatus/SolStatus.inf |  37 
 .../Universal/IpmiSolStatus/SolStatus.c   | 164 ++
 3 files changed, 205 insertions(+)
 create mode 100644 
Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf
 create mode 100644 
Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.c

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..c11bf5d0df 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -72,6 +72,10 @@
   # @Prompt MCTP KCS (Memory mapped) I/O base address
   gManageabilityPkgTokenSpaceGuid.PcdMctpKcsBaseAddress|0xca2|UINT32|0x0004
 
+  ## This is the value of SOL channels supported on platform.
+  # @Prompt SOL channel number
+  gManageabilityPkgTokenSpaceGuid.PcdMaxSolChannels|3|UINT8|0x0100
+
 [PcdsFeatureFlag]
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiEnable|FALSE|BOOLEAN|0x1001
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilitySmmIpmiEnable|FALSE|BOOLEAN|0x1002
diff --git a/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf 
b/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf
new file mode 100644
index 00..1d7cbf1a08
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.inf
@@ -0,0 +1,37 @@
+### @file
+# Component description file for IPMI Serial Over LAN (SOL) driver.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[Defines]
+  INF_VERSION  = 0x00010005
+  BASE_NAME= SolStatus
+  FILE_GUID= 1AF7E6C4-7678-4A6D-9240-E8BA17C3B772
+  MODULE_TYPE  = DXE_DRIVER
+  PI_SPECIFICATION_VERSION = 0x0001000A
+  VERSION_STRING   = 1.0
+  ENTRY_POINT  = SolStatusEntryPoint
+
+[Sources]
+  SolStatus.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+
+[Pcd]
+  gManageabilityPkgTokenSpaceGuid.PcdMaxSolChannels
+
+[LibraryClasses]
+  DebugLib
+  IpmiCommandLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.c 
b/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.c
new file mode 100644
index 00..996386372e
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiSolStatus/SolStatus.c
@@ -0,0 +1,164 @@
+/** @file
+  IPMI Serial Over Lan Driver.
+
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include  #include 
+
+#include 
+#include 
+#include 
+
+#define SOL_CMD_RETRY_COUNT  10
+
+/*++
+
+Routine Description:
+
+This routine gets the SOL payload status or settings for a specific 
channel.
+
+Arguments:
+Channel - LAN channel naumber.
+ParamSel- Configuration parameter selection.
+Data- Information returned from BMC.
+Returns:
+EFI_SUCCESS - SOL configuration parameters are successfully read from 
BMC.
+Others  - SOL configuration parameters could not be read from BMC.
+
+--*/
+EFI_STATUS
+GetSolStatus (
+  IN UINT8  Channel,
+  IN UINT8  ParamSel,
+  IN OUT UINT8  *Data
+  )
+{
+  EFI_STATUS  Status = EFI_SUCCESS;
+  IPMI_GET_SOL_CONFIGURATION_PARAMETERS_REQUEST   
GetConfigurationParametersRequest;
+  IPMI_GET_SOL_CONFIGURATION_PARAMETERS_RESPONSE  
GetConfigurationParametersResponse;
+  UINT32  DataSize;
+  UINT8   RetryCount;
+
+  for (RetryCount = 0; RetryCount < SOL_CMD_RETRY_COUNT; RetryCount++) {
+ZeroMem (, sizeof 
(GetConfigurationParametersRequest));
+GetConfigurationParametersRequest.ChannelNumber.Bits.ChannelNumber = 
Channel;
+

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiOsWdt: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: abner.ch...@amd.com  
Sent: Thursday, May 11, 2023 9:07 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiOsWdt: Add to 
ManageabilityPkg

From: Abner Chang 

Add IpmiOsWdt to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 1 +
 Features/ManageabilityPkg/Include/Manageability.dsc | 4 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 2 ++
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 4 files changed, 11 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..0a1c527107 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -79,4 +79,5 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiOsWdt|FALSE|BOOLEAN|0x1008
 
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..439f3be1ce 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -51,3 +51,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiOsWdt == TRUE
+  ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..177c900360 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,6 +37,7 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiOsWdt   
|TRUE
 
 #
 # Include common libraries
@@ -53,5 +54,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..1a2fed2253 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiOsWdt == TRUE
+  INF ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf
+!endif
-- 
2.37.1.windows.1



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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiOsWdt: IPMI OS Watchdog timer Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Thursday, May 11, 2023 9:07 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiOsWdt: 
IPMI OS Watchdog timer Driver

From: Abner Chang 

IpmiOsWdt is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/OsWdt in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/OsWdt until the reference to 
this instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../Universal/IpmiOsWdt/OsWdt.inf |  33 ++
 .../Universal/IpmiOsWdt/OsWdt.c   | 112 ++
 2 files changed, 145 insertions(+)
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.c

diff --git a/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf 
b/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf
new file mode 100644
index 00..b5af3b25e1
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.inf
@@ -0,0 +1,33 @@
+### @file
+# Component description file for IPMI OS watch dog timer driver.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = OsWdt
+  FILE_GUID  = BA4FD21F-8443-4017-8D13-70EC92F4BD4C
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= DriverInit
+
+[Sources]
+  OsWdt.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  IpmiCommandLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.c 
b/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.c
new file mode 100644
index 00..e2bbd95b83
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiOsWdt/OsWdt.c
@@ -0,0 +1,112 @@
+/** @file
+  IPMI Os watchdog timer Driver.
+
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+BOOLEAN  mOsWdtFlag = FALSE;
+
+EFI_EVENT  mExitBootServicesEvent;
+
+/*++
+
+Routine Description:
+  Enable the OS Boot Watchdog Timer.
+  Is called only on legacy or EFI OS boot.
+
+Arguments:
+  Event- Event type
+  *Context - Context for the event
+
+Returns:
+  None
+
+--*/
+VOID
+EFIAPI
+EnableEfiOsBootWdtHandler (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  EFI_STATUSStatus;
+  IPMI_SET_WATCHDOG_TIMER_REQUEST   SetWatchdogTimer;
+  UINT8 CompletionCode;
+  IPMI_GET_WATCHDOG_TIMER_RESPONSE  GetWatchdogTimer;
+  static BOOLEANOsWdtEventHandled = FALSE;
+
+  DEBUG ((DEBUG_ERROR, "!!! EnableEfiOsBootWdtHandler()!!!\n"));
+
+  //
+  // Make sure it processes once only. And proceess it only if 
+ OsWdtFlag==TRUE;  //  if (OsWdtEventHandled || !mOsWdtFlag) {
+return;
+  }
+
+  OsWdtEventHandled = TRUE;
+
+  Status = IpmiGetWatchdogTimer ();  if (EFI_ERROR 
+ (Status)) {
+return;
+  }
+
+  ZeroMem (, sizeof (SetWatchdogTimer));  //  // Just 
+ flip the Timer Use bit. This should release the timer.
+  //
+  SetWatchdogTimer.TimerUse.Bits.TimerRunning= 1;
+  SetWatchdogTimer.TimerUse.Bits.TimerUse= 
IPMI_WATCHDOG_TIMER_OS_LOADER;
+  SetWatchdogTimer.TimerActions.Uint8= 
IPMI_WATCHDOG_TIMER_ACTION_HARD_RESET;
+  SetWatchdogTimer.TimerUseExpirationFlagsClear &= ~BIT4;  
+ SetWatchdogTimer.TimerUseExpirationFlagsClear |= BIT1 | BIT2;
+  SetWatchdogTimer.InitialCountdownValue = 600; // 100ms / count
+
+  Status = IpmiSetWatchdogTimer (, );
+  return;
+}
+
+/*++
+
+Routine Description:
+  This is the standard EFI driver point. This function intitializes
+  the private data required for creating ASRR Driver.
+
+Arguments:
+  As required for DXE driver enrty routine.
+  ImageHandle - ImageHandle of the loaded driver
+  SystemTable - Pointer to the System Table
+
+Returns:
+  @retval EFI_SUCCESS   Protocol successfully started and installed.
+  @retval EFI_OUT_OF_RESOURCES  The event could not be allocated.
+
+--*/
+EFI_STATUS
+EFIAPI
+DriverInit (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = gBS->CreateEvent (
+

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiFru: Add to ManageabilityPkg

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Thursday, May 11, 2023 8:27 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang 
Subject: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg/IpmiFru: Add 
to ManageabilityPkg

From: Abner Chang 

Add IpmiFru to ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 Features/ManageabilityPkg/ManageabilityPkg.dec  | 1 +
 Features/ManageabilityPkg/Include/Manageability.dsc | 4 
 Features/ManageabilityPkg/ManageabilityPkg.dsc  | 2 ++
 Features/ManageabilityPkg/Include/PostMemory.fdf| 4 
 4 files changed, 11 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec 
b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 38813c5f48..8e8f30b122 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -79,4 +79,5 @@
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable|FALSE|BOOLEAN|0x1004
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE|BOOLEAN|0x1005
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|FALSE|BOOLEAN|0x1006
+  
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiFru|FALSE|BOOLEAN|0x1007
 
diff --git a/Features/ManageabilityPkg/Include/Manageability.dsc 
b/Features/ManageabilityPkg/Include/Manageability.dsc
index a432b0ff26..337c1fb9cc 100644
--- a/Features/ManageabilityPkg/Include/Manageability.dsc
+++ b/Features/ManageabilityPkg/Include/Manageability.dsc
@@ -51,3 +51,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiFru == TRUE
+  ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf
+!endif
diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dsc 
b/Features/ManageabilityPkg/ManageabilityPkg.dsc
index e3baf27f2a..6074e9d904 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dsc
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dsc
@@ -37,6 +37,7 @@
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmEnable  
|TRUE
   gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable  
|TRUE
   
gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransferEnable|TRUE
+  gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiFru 
|TRUE
 
 #
 # Include common libraries
@@ -53,5 +54,6 @@
 
 [LibraryClasses]
   
ManageabilityTransportLib|ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.inf
+  IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
 !include Include/Manageability.dsc
diff --git a/Features/ManageabilityPkg/Include/PostMemory.fdf 
b/Features/ManageabilityPkg/Include/PostMemory.fdf
index 9100cb2646..2d7be8cc31 100644
--- a/Features/ManageabilityPkg/Include/PostMemory.fdf
+++ b/Features/ManageabilityPkg/Include/PostMemory.fdf
@@ -26,3 +26,7 @@
 !if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable == TRUE
   INF ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf
 !endif
+
+!if gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeIpmiFru == TRUE
+  INF ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf
+!endif
-- 
2.37.1.windows.1








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

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFru: IPMI FRU Driver

2023-05-18 Thread Isaac Oram

Reviewed-by: Isaac Oram 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Chang, Abner via 
groups.io
Sent: Thursday, May 11, 2023 8:27 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W ; Abdul Lateef Attar 
; Nickle Wang ; Tinh Nguyen 

Subject: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFru: 
IPMI FRU Driver

From: Abner Chang 

IpmiFru is cloned from
edk2-platforms/Features/Intel/OutOfBandManagement/
IpmiFeaturePkg/IpmiFru in order to consolidate
edk2 system manageability support in one place.
Uncustify is applied to C files and no functionalities are changed in this 
patch.

We will still keep the one under IpmiFeaturePkg/IpmiFru until the reference to 
this instance are removed from platforms.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../Universal/IpmiFru/IpmiFru.inf | 34 ++
 .../Universal/IpmiFru/IpmiFru.c   | 67 +++
 2 files changed, 101 insertions(+)
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf
 create mode 100644 Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.c

diff --git a/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf 
b/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf
new file mode 100644
index 00..ddef310309
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.inf
@@ -0,0 +1,34 @@
+### @file
+# Component description file for IPMI FRU.
+#
+# Copyright (c) 2018 - 2019, Intel Corporation. All rights 
+reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[Defines]
+  INF_VERSION  = 0x00010005
+  BASE_NAME= IpmiFru
+  FILE_GUID= CD9B99D9-E86F-48CF-A8EB-20120AC22666
+  MODULE_TYPE  = DXE_DRIVER
+  PI_SPECIFICATION_VERSION = 0x0001000A
+  VERSION_STRING   = 1.0
+  ENTRY_POINT  = InitializeFru
+
+[Sources]
+  IpmiFru.c
+
+[Packages]
+  ManageabilityPkg/ManageabilityPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  IpmiCommandLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Depex]
+  TRUE
diff --git a/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.c 
b/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.c
new file mode 100644
index 00..2b489410db
--- /dev/null
+++ b/Features/ManageabilityPkg/Universal/IpmiFru/IpmiFru.c
@@ -0,0 +1,67 @@
+/** @file
+  IPMI FRU Driver.
+
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include  #include  
+#include  #include 
+
+/*++
+
+Routine Description:
+
+  Initialize SM Redirection Fru Layer
+
+Arguments:
+
+  ImageHandle - ImageHandle of the loaded driver  SystemTable - Pointer 
+ to the System Table
+
+Returns:
+
+  EFI_STATUS
+
+--*/
+EFI_STATUS
+EFIAPI
+InitializeFru (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS Status;
+  IPMI_GET_DEVICE_ID_RESPONSEControllerInfo;
+  IPMI_GET_FRU_INVENTORY_AREA_INFO_REQUEST   GetFruInventoryAreaInfoRequest;
+  IPMI_GET_FRU_INVENTORY_AREA_INFO_RESPONSE  
+GetFruInventoryAreaInfoResponse;
+
+  //
+  //  Get all the SDR Records from BMC and retrieve the Record ID from the 
structure for future use.
+  //
+  Status = IpmiGetDeviceId ();  if (EFI_ERROR (Status)) 
+ {
+DEBUG ((DEBUG_ERROR, "!!! IpmiFru  IpmiGetDeviceId Status=%x\n", Status));
+return Status;
+  }
+
+  DEBUG ((DEBUG_ERROR, "!!! IpmiFru  FruInventorySupport %x\n", 
+ ControllerInfo.DeviceSupport.Bits.FruInventorySupport));
+
+  if (ControllerInfo.DeviceSupport.Bits.FruInventorySupport) {
+GetFruInventoryAreaInfoRequest.DeviceId = 0;
+Status  = IpmiGetFruInventoryAreaInfo 
(, );
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "!!! IpmiFru  IpmiGetFruInventoryAreaInfo 
Status=%x\n", Status));
+  return Status;
+}
+
+DEBUG ((DEBUG_ERROR, "!!! IpmiFru  InventoryAreaSize=%x\n", 
+ GetFruInventoryAreaInfoResponse.InventoryAreaSize));
+  }
+
+  return EFI_SUCCESS;
+}
--
2.37.1.windows.1








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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Jeff Fan via groups.io

Ray,

If you chooses assembly solution, you'd better to consider stack adjust to 
follow calling convention. Otherwise, it may break some debugger tools to do 
call stack trace.

Jeff


fanjianf...@byosoft.com.cn
 
From: Ni, Ray
Date: 2023-05-19 10:53
To: Andrew (EFI) Fish; devel@edk2.groups.io; Kinney, Michael D
CC: Rebecca Cran
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
I think all the options we considered are workarounds. These might break again 
if compiler is “cleverer” in future. Unless some Cxx spec clearly guarantees 
that.
 
I like Mike’s idea to use assembly implementation for CpuDeadLoop. The assembly 
can simply “jmp $” then “ret”.
 
I didn’t find a dead-loop intrinsic function in MSVC.
Any better idea?
 
Thanks,
Ray
 
From: Andrew (EFI) Fish  
Sent: Friday, May 19, 2023 8:42 AM
To: devel@edk2.groups.io; Kinney, Michael D 
Cc: Ni, Ray ; Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
 
Mike,
 
Sorry static was just to scope the name to the file since it is a lib, not to 
make it work.
 
That is a cool site. I learned about it complaining about stuff to the compiler 
team on our internal clang Slack channel as they use it to answer my questions.
 
Thanks,
 
Andrew Fish


On May 18, 2023, at 2:42 PM, Michael D Kinney  
wrote:
 
Using that tool, the following fragment seems to generate the right code.  
Volatile is required.  Static is optional.
 
static volatile int  mDeadLoopCount = 0;
 
void
CpuDeadLoop(
  void
  )
{
  while (mDeadLoopCount == 0);
}
 
 
GCC
===
CpuDeadLoop():
.L2:
mov eax, DWORD PTR mDeadLoopCount[rip]
testeax, eax
je  .L2
ret
 
 
CLANG
=
CpuDeadLoop():   # @CpuDeadLoop()
.LBB0_1:# =>This Inner Loop Header: Depth=1
cmp dword ptr [rip + _ZL14mDeadLoopCount], 0
je  .LBB0_1
ret
 
 
Mike
 
 
From: Andrew (EFI) Fish  
Sent: Thursday, May 18, 2023 1:45 PM
To: edk2-devel-groups-io ; Andrew Fish 
Cc: Kinney, Michael D ; Ni, Ray ; 
Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
 
Whoops wrong compiler. Here is an update. I added the flags so this one 
reproduces the issue.
 
Compiler Explorer
godbolt.org

 
Thanks,
 
Andrew Fish



On May 18, 2023, at 11:45 AM, Andrew Fish via 
groups.io wrote:
 
Mike,
 
This is a good way to play around with fixes, and to report bugs. You can see 
the assembler for different compilers with different flag. 
 
Compiler Explorer
godbolt.org

 
Sorry I’m traveling and in Cupertino with lots of meetings so I did not have 
time to adjust the compiler flags….
 
Thanks,
 
Andrew Fish



On May 18, 2023, at 10:24 AM, Andrew (EFI) Fish  wrote:
 
Mike,
 
I guess my other question… If this turns out to be a compiler bug should we 
scope the change to the broken toolchain. I’m not sure what the right answer is 
for that, but I want to ask the question? 
 
Thanks,
 
Andrew Fish



On May 18, 2023, at 10:19 AM, Michael D Kinney  
wrote:
 
Andrew,
 
This might work for XIP.  Set non const global to initial value that is 
expected value to stay in dead loop.
 
UINTN  mDeadLoopCount = 0;
 
VOID
CpuDeadLoop(
  VOID
  ) 
{
  while (mDeadLoopCount == 0) {
  CpuPause();
  }
}
 
When deadloop is entered, developer can not change value of mDeadLoopCount, but 
they can use debugger to force exit loop and return from function.
 
Mike
 
 
From: Andrew (EFI) Fish  
Sent: Thursday, May 18, 2023 10:09 AM
To: Kinney, Michael D 
Cc: edk2-devel-groups-io ; Ni, Ray ; 
Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
 
Mike,
 
Good point, that is why we are using the stack ….
 
The only other thing I can think of is to pass the address of Index to some 
inline assembler, or an asm no op function, to give it a side effect the 
compiler can’t resolve. 
 
Thanks,
 
Andrew Fish




On May 18, 2023, at 10:05 AM, Kinney, Michael D  
wrote:
 
Static global will not work for XIP
 
Mike
 
From: Andrew (EFI) Fish  
Sent: Thursday, May 18, 2023 9:49 AM
To: edk2-devel-groups-io ; Kinney, Michael D 

Cc: Ni, Ray ; Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
 
Mike,
 
I pinged some compiler experts to see if our code is correct, or if the 
compiler has an issue. Seems to be trending compiler issue right now, but I’ve 
NOT gotten feedback from anyone on the spec committee yet. 
 
If we move Index to a static global that would likely work around the compiler 
issue.
 
Thanks,
 
Andrew Fish





On May 18, 2023, at 8:36 AM, Michael D Kinney  
wrote:
 
Hi Ray,
 
So the code generated does deadloop, but is just not easy to resume from as we 
have been able to do in the past.
 
We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
reason to ever continue.
 
The 2nd is a debug aide for developers to halt the system at a specific 
location and then continue from that point, usually with a debugger, to

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Ni, Ray

I think all the options we considered are workarounds. These might break again 
if compiler is “cleverer” in future. Unless some Cxx spec clearly guarantees 
that.

I like Mike’s idea to use assembly implementation for CpuDeadLoop. The assembly 
can simply “jmp $” then “ret”.

I didn’t find a dead-loop intrinsic function in MSVC.
Any better idea?

Thanks,
Ray

From: Andrew (EFI) Fish 
Sent: Friday, May 19, 2023 8:42 AM
To: devel@edk2.groups.io; Kinney, Michael D 
Cc: Ni, Ray ; Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Mike,

Sorry static was just to scope the name to the file since it is a lib, not to 
make it work.

That is a cool site. I learned about it complaining about stuff to the compiler 
team on our internal clang Slack channel as they use it to answer my questions.

Thanks,

Andrew Fish


On May 18, 2023, at 2:42 PM, Michael D Kinney 
mailto:michael.d.kin...@intel.com>> wrote:

Using that tool, the following fragment seems to generate the right code.  
Volatile is required.  Static is optional.

static volatile int  mDeadLoopCount = 0;

void
CpuDeadLoop(
  void
  )
{
  while (mDeadLoopCount == 0);
}


GCC
===
CpuDeadLoop():
.L2:
mov eax, DWORD PTR mDeadLoopCount[rip]
testeax, eax
je  .L2
ret


CLANG
=
CpuDeadLoop():   # @CpuDeadLoop()
.LBB0_1:# =>This Inner Loop Header: Depth=1
cmp dword ptr [rip + _ZL14mDeadLoopCount], 0
je  .LBB0_1
ret


Mike


From: Andrew (EFI) Fish mailto:af...@apple.com>>
Sent: Thursday, May 18, 2023 1:45 PM
To: edk2-devel-groups-io mailto:devel@edk2.groups.io>>; 
Andrew Fish mailto:af...@apple.com>>
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Ni, Ray 
mailto:ray...@intel.com>>; Rebecca Cran 
mailto:rebe...@bsdio.com>>
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Whoops wrong compiler. Here is an update. I added the flags so this one 
reproduces the issue.

Compiler 
Explorer
godbolt.org

Re: [edk2-devel] [PATCH] ShellPkg/SmbiosView: type 45 and type 46 support.

2023-05-18 Thread Gao, Zhichao

Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

-Original Message-
From: Simon Wang  
Sent: Thursday, May 4, 2023 10:34 AM
To: devel@edk2.groups.io
Cc: Ni, Ray ; Gao, Zhichao 
Subject: [PATCH] ShellPkg/SmbiosView: type 45 and type 46 support.

The initial version of Smbios Specification 3.6.0 type 45 and type 46 support.

Signed-off-by: Simon Wang 
Cc: Ray Ni 
Cc: Zhichao Gao 
---
 .../SmbiosView/PrintInfo.c| 111 +-
 .../SmbiosView/PrintInfo.h|  25 
 .../SmbiosView/QueryTable.c   |  81 +
 .../SmbiosView/SmbiosViewStrings.uni  |   9 +-
 4 files changed, 222 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
index 1811cf0c44..e6a110beee 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
@@ -5,6 +5,7 @@
   Copyright (c) 1985 - 2022, American Megatrends International LLC.
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
   (C) Copyright 2015-2019 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -305,9 +306,10 @@ SmbiosPrintStructure (
   IN  UINT8 Option
   )
 {
-  UINT8  Index;
-  UINT8  Index2;
-  UINT8  *Buffer;
+  UINT8   Index;
+  UINT8   Index2;
+  UINT8   *Buffer;
+  EFI_STRING  String;
 
   if (Struct == NULL) {
 return EFI_INVALID_PARAMETER;
@@ -1302,6 +1304,109 @@ SmbiosPrintStructure (
   break;
 
 //
+// Firmware Inventory (Type 45)
+//
+case 45:
+  PRINT_PENDING_STRING (Struct, Type45, FirmwareComponentName);
+  PRINT_PENDING_STRING (Struct, Type45, FirmwareVersion);
+  if (Struct->Type45->FirmwareVersionFormat == VersionFormatTypeFreeForm) {
+String = L"Free-form string";
+  } else if (Struct->Type45->FirmwareVersionFormat == 
VersionFormatTypeMajorMinor) {
+String = L"MAJOR.MINOR";
+  } else if (Struct->Type45->FirmwareVersionFormat == 
VersionFormatType32BitHex) {
+String = L"32-bit hexadecimal string";
+  } else if (Struct->Type45->FirmwareVersionFormat == 
VersionFormatTypeMajorMinor) {
+String = L"64-bit hexadecimal string";
+  } else if (Struct->Type45->FirmwareVersionFormat >= 0x80) {
+String = L"BIOS Vendor/OEM-specific";
+  } else {
+String = L"Reserved";
+  }
+
+  ShellPrintHiiEx (
+-1,
+-1,
+NULL,
+STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_FIRMWARE_VERSION_FORMAT),
+gShellDebug1HiiHandle,
+String
+);
+  PRINT_PENDING_STRING (Struct, Type45, FirmwareId);
+  if (Struct->Type45->FirmwareIdFormat == FirmwareIdFormatTypeFreeForm) {
+String = L"Free-form string";
+  } else if (Struct->Type45->FirmwareIdFormat == FirmwareIdFormatTypeUuid) 
{
+String = L"RFC4122 UUID string";
+  } else if (Struct->Type45->FirmwareIdFormat >= 0x80) {
+String = L"BIOS Vendor/OEM-specific";
+  } else {
+String = L"Reserved";
+  }
+
+  ShellPrintHiiEx (
+-1,
+-1,
+NULL,
+STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_FIRMWARE_ID_FORMAT),
+gShellDebug1HiiHandle,
+String
+);
+  PRINT_PENDING_STRING (Struct, Type45, ReleaseDate);
+  PRINT_PENDING_STRING (Struct, Type45, Manufacturer);
+  PRINT_PENDING_STRING (Struct, Type45, LowestSupportedVersion);
+  if (Struct->Type45->ImageSize != MAX_UINT64) {
+PRINT_STRUCT_VALUE_H (Struct, Type45, ImageSize);
+  } else {
+ShellPrintHiiEx (
+  -1,
+  -1,
+  NULL,
+  STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_IMAGE_SIZE_UNKNOWN),
+  gShellDebug1HiiHandle
+  );
+  }
+
+  DisplayFirmwareCharacteristics (ReadUnaligned16 ((UINT16 
*)(UINTN)&(Struct->Type45->Characteristics)), Option);
+  DisplayFirmwareState (*(UINT8 *)(UINTN)&(Struct->Type45->State), 
+ Option);
+
+  PRINT_STRUCT_VALUE_H (Struct, Type45, AssociatedComponentCount);
+  if (Struct->Hdr->Length > sizeof (*Struct->Type45)) {
+for (Index = 0; Index < Struct->Type45->AssociatedComponentCount; 
Index++) {
+  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_SMBIOSVIEW_PRINTINFO_FIRMWARE_INVENTORY_ASSOCIATED), 
gShellDebug1HiiHandle);
+  Print (L"0x%04X ", Buffer[sizeof (*Struct->Type45) + (Index * 
sizeof (SMBIOS_HANDLE))]);
+  Print (L"\n");
+}
+  }
+
+  break;
+
+//
+// String Property (Type 46)
+//
+case 46:
+  if (Struct->Type46->StringPropertyId == StringPropertyIdDevicePath) {
+String = L"UEFI device path";
+  } else if ((Struct->Type46->StringPropertyId >=

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Mike,

Sorry static was just to scope the name to the file since it is a lib, not to 
make it work.

That is a cool site. I learned about it complaining about stuff to the compiler 
team on our internal clang Slack channel as they use it to answer my questions.

Thanks,

Andrew Fish

> On May 18, 2023, at 2:42 PM, Michael D Kinney  
> wrote:
> 
> Using that tool, the following fragment seems to generate the right code.  
> Volatile is required.  Static is optional.
>  
> static volatile int  mDeadLoopCount = 0;
>  
> void
> CpuDeadLoop(
>   void
>   )
> {
>   while (mDeadLoopCount == 0);
> }
>  
>  
> GCC
> ===
> CpuDeadLoop():
> .L2:
> mov eax, DWORD PTR mDeadLoopCount[rip]
> testeax, eax
> je  .L2
> ret
>  
>  
> CLANG
> =
> CpuDeadLoop():   # @CpuDeadLoop()
> .LBB0_1:# =>This Inner Loop Header: Depth=1
> cmp dword ptr [rip + _ZL14mDeadLoopCount], 0
> je  .LBB0_1
> ret
>  
>  
> Mike
>  
>  
> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
> Sent: Thursday, May 18, 2023 1:45 PM
> To: edk2-devel-groups-io  >; Andrew Fish  >
> Cc: Kinney, Michael D  >; Ni, Ray  >; Rebecca Cran  >
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Whoops wrong compiler. Here is an update. I added the flags so this one 
> reproduces the issue.
>  
> Compiler Explorer 
> 
> godbolt.org 
> 
>  
>  
>

Re: [edk2-devel] [Patch V2] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread Ni, Ray

Mike,
No. The code is there for couple years. With that fact, I don't think it's a 
critical bug.

Yes. all memory allocation libs should be fixed. We start with the SMM one 
first to see feedbacks.

Thanks,
Ray

> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, May 18, 2023 11:38 PM
> To: Ni, Ray ; Tan, Dun ;
> devel@edk2.groups.io
> Cc: Gao, Liming ; Liu, Zhiguang
> ; Kinney, Michael D 
> Subject: RE: [Patch V2] MdePkg: Code optimization to SMM
> InternalAllocateAlignedPages
> 
> Is this considered a critical bug for stable tag release?
> 
> Is there are HSD and if it is critical, all memory allocation lobs should be 
> fixed.
> Right?
> 
> Mike
> 
> > -Original Message-
> > From: Ni, Ray 
> > Sent: Thursday, May 18, 2023 1:55 AM
> > To: Tan, Dun ; devel@edk2.groups.io
> > Cc: Kinney, Michael D ; Gao, Liming
> > ; Liu, Zhiguang 
> > Subject: RE: [Patch V2] MdePkg: Code optimization to SMM
> > InternalAllocateAlignedPages
> >
> > Reviewed-by: Ray Ni 
> >
> > > -Original Message-
> > > From: Tan, Dun 
> > > Sent: Thursday, May 18, 2023 3:31 PM
> > > To: devel@edk2.groups.io
> > > Cc: Ni, Ray ; Kinney, Michael D
> > > ; Gao, Liming ;
> > Liu,
> > > Zhiguang 
> > > Subject: [Patch V2] MdePkg: Code optimization to SMM
> > > InternalAllocateAlignedPages
> > >
> > > This commit is code optimization to InternalAllocateAlignedPages of
> > > SmmMemoryAllocationLib which can reduce free memory fragments. Also
> > > it can reduce one pre-allocation page.
> > >
> > > Let's take a simple example:
> > > The expected pages size is 8KB, Alignment value is 8KB.
> > >
> > > In original InternalAllocateAlignedPages(), the first step is to
> > > allocate 4 pages and then find the first 8KB-aligned address in
> > > allocated 4 pages. If the upper limit address of allocated 4 pages
> > > is already 8KB aligned, then the allocated 4 pages contains two
> > > 8KB-aligned 8KB ranges. The lower 2 pages will be selected and
> > > removed from free pages. Then the higher 2 pages will be free.
> > > Since the whole memory allocation is from high address to low
> > > address, then the higher 2 pages cann't be merged with other free
> > > pages, causing the free memory fragments.
> > >
> > > However, when only allocate 3(2+2-1) pages, we can avoid the free
> > > memory fragments in specific case. Also 3 pages must contain a
> > > 8KB-aligned 8KB range, which meets the requirement. If the upper
> > > limit address of allocated 3 pages is 8KB-aligned, then the higher
> > > 2 pages range of allocated 3 pages is 8KB-aligned and will be
> > > selected and removed from free pages. The remaining lower one page
> > > of allocated 3 pages will be free and merged with left lower free
> > > memory. This can reduce free memory fragments in smm.
> > >
> > > Signed-off-by: Dun Tan 
> > > Cc: Ray Ni 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Cc: Zhiguang Liu 
> > > ---
> > >  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git
> > a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > > b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > > index 3ab2c1ebfd..99a8259325 100644
> > > --- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > > +++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > > @@ -322,7 +322,7 @@ InternalAllocateAlignedPages (
> > >  // Calculate the total number of pages since alignment is larger than
> > page size.
> > >  //
> > >  AlignmentMask = Alignment - 1;
> > > -RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> > > +RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment) - 1;
> > >  //
> > >  // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> > > overflow.
> > >  //
> > > --
> > > 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105052): https://edk2.groups.io/g/devel/message/105052
Mute This Topic: https://groups.io/mt/98986911/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] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Michael D Kinney

Using that tool, the following fragment seems to generate the right code.  
Volatile is required.  Static is optional.

static volatile int  mDeadLoopCount = 0;

void
CpuDeadLoop(
  void
  )
{
  while (mDeadLoopCount == 0);
}


GCC
===
CpuDeadLoop():
.L2:
mov eax, DWORD PTR mDeadLoopCount[rip]
testeax, eax
je  .L2
ret


CLANG
=
CpuDeadLoop():   # @CpuDeadLoop()
.LBB0_1:# =>This Inner Loop Header: Depth=1
cmp dword ptr [rip + _ZL14mDeadLoopCount], 0
je  .LBB0_1
ret


Mike


From: Andrew (EFI) Fish 
Sent: Thursday, May 18, 2023 1:45 PM
To: edk2-devel-groups-io ; Andrew Fish 
Cc: Kinney, Michael D ; Ni, Ray ; 
Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Whoops wrong compiler. Here is an update. I added the flags so this one 
reproduces the issue.

Compiler 
Explorer
godbolt.org

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Whoops wrong compiler. Here is an update. I added the flags so this one 
reproduces the issue.

https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIM6SuADJ4DJgAcj4ARpjEINIADqgKhE4MHt6%2B/qRJKY4CIWGRLDFx0naYDmlCBEzEBBk%2BfgEVVQI1dQSFEdGx8ba19Y1ZLYNdoT0lfZIAlLaoXsTI7BwA9KsA1AAqAJ4JmBs7C8QbaFgbCLGYpBskG7SoTOgbhhuYqqwJ9AB0JhoAgqECBsFABHLx1TAQIEbBg%2BGYbEwAdisAI26I2xEwBEWDFhPg2ACp8SwTABmVH/ZEAEQ4c1onAArLw/BwtKRUJw3NZrCDjstEWYyTxSARNHS5gBrECMjT6TiSFnijmcXgKEBysVsumkOCwJBoFgJOixciUQ3G%2BhxYBcMxcPh0AixdUQKLKqKhOo7Tgiw1sQQAeQYtG92tIWBYhmA4jD%2BCxVQAbph1WH3pUvE6fbwgZgGWHaHgosQvR4sMqCMQ8Cws3MqAZgAoAGp4TAAdwD%2B1ZIv4ghEYnYUhkgkUKnUYd09oMRhQPMs%2BkL6sgc1QCXyDBTAFoA2SNhvI0sEOTqQpJTsDJLMGrc5U1y4GO5PE09MEJsVSnpcqkBMM/PbP2vujfPp7VaNcOiGR8shA682gYcDxiKXo4hAsYfz0BQxkApCJDmBR%2BQHUUsRWHh6SZJUw05DhVAADgANg3WjJA2YBkGQDZbW%2BLgNggblLGsG5cEIO5zGFG4PCNE0ThErgZl4LUtDmCADVQCSrTNCALUklApxtSQNDlGhaCdYgXTdMMPWYYhQ19FT/QIIMQ2VCMoxjdk4xvPAkxTdk02QDNiOzQRc2VAsixLDAVnZCsqxrPh6ybFt207LMh2EURxEHHt5CUNRlV0AIdJnPi51CxcIGXVc0k3bdd33ZBDzJY9T3PS9bBg28IFcND7RfRCpmQnJki/dJIN/Qa8jSLD%2BvQ9rqlQ0aZvsMDMNfbCUM6bqBk6Kb3xk%2BZFmWPQK0wALSI4ZlSFZdlKJo%2BjGNOHT2Mkb4NBe7jeKsOcNkEogpKFe0NnEy1YkFMkzFk0VxUUpB8CoKh1KyvsMukLKR1y/MEHVSdMdhqgCD2dg5WITHsmJhRcfx/YNTOi6rt4SjqTwOGNmbNsQduhimJYtiOK4hMFA2Dn7uQR6pBel65KhqUZTlPNFUu5VKLVbJ5IleUODMcjrtVSHtRmOYkxMtJ4iAA

Thanks,

Andrew Fish

> On May 18, 2023, at 11:45 AM, Andrew Fish via groups.io 
>  wrote:
> 
> Mike,
> 
> This is a good way to play around with fixes, and to report bugs. You can see 
> the assembler for different compilers with different flag. 
> 
> https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIM6SuADJ4DJgAcj4ARpjEINIADqgKhE4MHt6%2B/qRJKY4CIWGRLDFx0naYDmlCBEzEBBk%2BfgEVVQI1dQSFEdGx8ba19Y1ZLYNdoT0lfZIAlLaoXsTI7BwA9KsA1AAqAJ4JmBs7C8QbaFgbCLGYpBskG7SoTOgbhhuYqqwJ9AB0JhoAgqECBsFABHLx1TAQIEbBg%2BGYbEwAdisAI26I2xEwBEWDFhPg2ACp8SwTABmVH/ZEAEQ4c1onAArLw/BwtKRUJw3NZrCDjstEWYyTxSARNHS5gBrECMjT6TiSFnijmcXgKEBysVsumkOCwJBoFgJOixciUQ3G%2BhxYBcMxcPh0AixdUQKLKqKhOo7Tgiw1sQQAeQYtG92tIWBYhmA4jD%2BCxVQAbph1WH3pUvE6fbwgZgGWHaHgosQvR4sMqCMQ8Cws3MqAZgAoAGp4TAAdwD%2B1ZIv4ghEYnYUhkgkUKnUYd09oMRhQPMs%2BkL6sgc1QCXyDBTAFoA2SNhvI0sEOTqQpJTsDJLMGrc5U1y4GO5PE09MEJsVSnpcqkBMM/PbP2vujfPp7VaNcOiGR8shA682gYcDxiKXo4hAsYfz0BQxkApCJDmBR%2BQHUUsRWHh6SZJUw05DhVAADgANg3WjJA2YBkGQDZbW%2BLgNggblLGsG5cEIO5zGFG4PCNE0ThErgZl4LUtDmCADVQCSrTNCALUklApxtSQNDlGhaCdYgXTdMMPWYYhQ19FT/QIIMQ2VCMoxjdk4xvPAkxTdk02QDNiOzQRc2VAsixLDAVnZCsqxrPh6ybFt207LMh2EURxEHHt5CUNRlV0AIdJnPi51CxcIGXVc0k3bdd33ZBDzJY9T3PS9bBg28IFcND7RfRCpmQnJki/dJIN/Qa8jSLD%2BvQ9rqlQ0aZvsMDMNfbCUM6bqBk6Kb3xk%2BZFmWPQK0wALSI4ZlSFZdlKJo%2BjGNOHT2Mkb4NBe7jeKsOcNkEogpKFe0NnEy1YkFMkzFk0VxUUpB8CoKh1KyvsMukLKR1y/MEHVSdMdhqgCD2dg5WITHsmJhRcfx/YNTOi6rt4SjqTwOGNmbNsQduhimJYtiOK4hMFA2Dn7uQR6pBel65KhqUZTlPNFUu5VKLVbJ5IleUODMcjrtVSHtRmOYkxMtJ4iAA
> 
> Sorry I’m traveling and in Cupertino with lots of meetings so I did not have 
> time to adjust the compiler flags….
> 
> Thanks,
> 
> Andrew Fish
> 
>> On May 18, 2023, at 10:24 AM, Andrew (EFI) Fish  wrote:
>> 
>> Mike,
>> 
>> I guess my other question… If this turns out to be a compiler bug should we 
>> scope the change to the broken toolchain. I’m not sure what the right answer 
>> is for that, but I want to ask the question? 
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> On May 18, 2023, at 10:19 AM, Michael D Kinney  
>>> wrote:
>>> 
>>> Andrew,
>>>  
>>> This might work for XIP.  Set non const global to initial value that is 
>>> expected value to stay in dead loop.
>>>  
>>> UINTN  mDeadLoopCount = 0;
>>>  
>>> VOID
>>> CpuDeadLoop(
>>>   VOID
>>>   ) 
>>> {
>>>   while (mDeadLoopCount == 0) {
>>>   CpuPause();
>>>   }
>>> }
>>>  
>>> When deadloop is entered, developer can not change value of mDeadLoopCount, 
>>> but they can use debugger to force exit loop and return from function.
>>>  
>>> Mike
>>>  
>>>  
>>> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
>>> Sent: Thursday, May 18, 2023 10:09 AM
>>> To: Kinney, Michael D >> >
>>> Cc: edk2-devel-groups-io >> >; Ni, Ray >> >; Rebecca Cran >> >
>>> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>>>  
>>> Mike,
>>>  
>>> Good point, that is why we are using the stack ….
>>>  
>>> The only other thing I can think of is to pass the address of Index to some 
>>> inline assembler, or an asm no op function, to give it a side effect the 
>>> compiler can’t resolve. 
>>>  
>>> Thanks,
>>>  
>>> Andrew Fish
>>> 
>>> 
>>> On May 18, 2023, at 10:05 AM, Kinney, Michael D >> > wrote:
>>>  
>>> Static global will not work for XIP
>>>  
>>> Mike
>>>  
>>> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
>>> Sent:

Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN Coverity issue

2023-05-18 Thread Chiu, Chasel



This fix looks good to me! Thanks Ranbir!
Reviewed-by: Chasel Chiu 


> -Original Message-
> From: Ranbir Singh 
> Sent: Wednesday, May 17, 2023 11:29 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; Ranbir
> Singh 
> Subject: [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN
> Coverity issue
> 
> FspData->PerfIdx is getting increased for every call unconditionally
> in the function SetFspMeasurePoint and hence memory access can happen for
> out of bound FspData->PerfData[] array entries also.
> 
> Example -
>FspData->PerfData is an array of 32 UINT64 entries. Assume a call
>is made to SetFspMeasurePoint function when the FspData->PerfIdx
>last value is 31. It gets incremented to 32 at line 400.
>Any subsequent call to SetFspMeasurePoint functions leads to
>FspData->PerfData[32] getting accessed which is out of the PerfData
>array as well as the FSP_GLOBAL_DATA structure boundary.
> 
> Hence keep array access and index increment inside if block only and return
> invalid performance timestamp when PerfIdx is invalid.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4200
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
>  IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> index a22b0e7825ad..cda2a7b2478e 100644
> --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> @@ -377,7 +377,8 @@ GetFspSiliconInitUpdDataPointer (
> @param[in] Id   Measurement point ID. -  @return performance
> timestamp.+  @return performance timestamp if current PerfIdx is valid,+
> else return 0 as invalid performance timestamp **/ UINT64 EFIAPI@@ -395,9
> +396,10 @@ SetFspMeasurePoint (
>if (FspData->PerfIdx < sizeof (FspData->PerfData) / sizeof (FspData-
> >PerfData[0])) { FspData->PerfData[FspData->PerfIdx]  =
> AsmReadTsc (); ((UINT8 *)(>PerfData[FspData->PerfIdx]))[7] = Id;+
> return FspData->PerfData[(FspData->PerfIdx)++];   } -  return FspData-
> >PerfData[(FspData->PerfIdx)++];+  return (UINT64)0x; }
> /**--
> 2.34.1



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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Mike,

This is a good way to play around with fixes, and to report bugs. You can see 
the assembler for different compilers with different flag. 

https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIM6SuADJ4DJgAcj4ARpjEINIADqgKhE4MHt6%2B/qRJKY4CIWGRLDFx0naYDmlCBEzEBBk%2BfgEVVQI1dQSFEdGx8ba19Y1ZLYNdoT0lfZIAlLaoXsTI7BwA9KsA1AAqAJ4JmBs7C8QbaFgbCLGYpBskG7SoTOgbhhuYqqwJ9AB0JhoAgqECBsFABHLx1TAQIEbBg%2BGYbEwAdisAI26I2xEwBEWDFhPg2ACp8SwTABmVH/ZEAEQ4c1onAArLw/BwtKRUJw3NZrCDjstEWYyTxSARNHS5gBrECMjT6TiSFnijmcXgKEBysVsumkOCwJBoFgJOixciUQ3G%2BhxYBcMxcPh0AixdUQKLKqKhOo7Tgiw1sQQAeQYtG92tIWBYhmA4jD%2BCxVQAbph1WH3pUvE6fbwgZgGWHaHgosQvR4sMqCMQ8Cws3MqAZgAoAGp4TAAdwD%2B1ZIv4ghEYnYUhkgkUKnUYd09oMRhQPMs%2BkL6sgc1QCXyDBTAFoA2SNhvI0sEOTqQpJTsDJLMGrc5U1y4GO5PE09MEJsVSnpcqkBMM/PbP2vujfPp7VaNcOiGR8shA682gYcDxiKXo4hAsYfz0BQxkApCJDmBR%2BQHUUsRWHh6SZJUw05DhVAADgANg3WjJA2YBkGQDZbW%2BLgNggblLGsG5cEIO5zGFG4PCNE0ThErgZl4LUtDmCADVQCSrTNCALUklApxtSQNDlGhaCdYgXTdMMPWYYhQ19FT/QIIMQ2VCMoxjdk4xvPAkxTdk02QDNiOzQRc2VAsixLDAVnZCsqxrPh6ybFt207LMh2EURxEHHt5CUNRlV0AIdJnPi51CxcIGXVc0k3bdd33ZBDzJY9T3PS9bBg28IFcND7RfRCpmQnJki/dJIN/Qa8jSLD%2BvQ9rqlQ0aZvsMDMNfbCUM6bqBk6Kb3xk%2BZFmWPQK0wALSI4ZlSFZdlKJo%2BjGNOHT2Mkb4NBe7jeKsOcNkEogpKFe0NnEy1YkFMkzFk0VxUUpB8CoKh1KyvsMukLKR1y/MEHVSdMdhqgCD2dg5WITHsmJhRcfx/YNTOi6rt4SjqTwOGNmbNsQduhimJYtiOK4hMFA2Dn7uQR6pBel65KhqUZTlPNFUu5VKLVbJ5IleUODMcjrtVSHtRmOYkxMtJ4iAA

Sorry I’m traveling and in Cupertino with lots of meetings so I did not have 
time to adjust the compiler flags….

Thanks,

Andrew Fish

> On May 18, 2023, at 10:24 AM, Andrew (EFI) Fish  wrote:
> 
> Mike,
> 
> I guess my other question… If this turns out to be a compiler bug should we 
> scope the change to the broken toolchain. I’m not sure what the right answer 
> is for that, but I want to ask the question? 
> 
> Thanks,
> 
> Andrew Fish
> 
>> On May 18, 2023, at 10:19 AM, Michael D Kinney  
>> wrote:
>> 
>> Andrew,
>>  
>> This might work for XIP.  Set non const global to initial value that is 
>> expected value to stay in dead loop.
>>  
>> UINTN  mDeadLoopCount = 0;
>>  
>> VOID
>> CpuDeadLoop(
>>   VOID
>>   ) 
>> {
>>   while (mDeadLoopCount == 0) {
>>   CpuPause();
>>   }
>> }
>>  
>> When deadloop is entered, developer can not change value of mDeadLoopCount, 
>> but they can use debugger to force exit loop and return from function.
>>  
>> Mike
>>  
>>  
>> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
>> Sent: Thursday, May 18, 2023 10:09 AM
>> To: Kinney, Michael D > >
>> Cc: edk2-devel-groups-io > >; Ni, Ray > >; Rebecca Cran > >
>> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>>  
>> Mike,
>>  
>> Good point, that is why we are using the stack ….
>>  
>> The only other thing I can think of is to pass the address of Index to some 
>> inline assembler, or an asm no op function, to give it a side effect the 
>> compiler can’t resolve. 
>>  
>> Thanks,
>>  
>> Andrew Fish
>> 
>> 
>> On May 18, 2023, at 10:05 AM, Kinney, Michael D > > wrote:
>>  
>> Static global will not work for XIP
>>  
>> Mike
>>  
>> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
>> Sent: Thursday, May 18, 2023 9:49 AM
>> To: edk2-devel-groups-io > >; Kinney, Michael D 
>> mailto:michael.d.kin...@intel.com>>
>> Cc: Ni, Ray mailto:ray...@intel.com>>; Rebecca Cran 
>> mailto:rebe...@bsdio.com>>
>> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>>  
>> Mike,
>>  
>> I pinged some compiler experts to see if our code is correct, or if the 
>> compiler has an issue. Seems to be trending compiler issue right now, but 
>> I’ve NOT gotten feedback from anyone on the spec committee yet. 
>>  
>> If we move Index to a static global that would likely work around the 
>> compiler issue.
>>  
>> Thanks,
>>  
>> Andrew Fish
>> 
>> 
>> 
>> On May 18, 2023, at 8:36 AM, Michael D Kinney > > wrote:
>>  
>> Hi Ray,
>>  
>> So the code generated does deadloop, but is just not easy to resume from as 
>> we have been able to do in the past.
>>  
>> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
>> reason to ever continue.
>>  
>> The 2nd is a debug aide for developers to halt the system at a specific 
>> location and then continue from that point, usually with a debugger, to step 
>> through code to an area to evaluate unexpected behavior.
>>  
>> We may have to do a NASM implementation of CpuDeadloop() to make sure it 
>> meets both use cases.
>>  
>> Mike
>>  
>> From: Ni, Ray mailto:ray...@intel.com>> 
>> Sent: Thursday, May 18, 2023 3:00 AM
>> To: devel@edk2.groups.io 
>> Cc: Kinney, Michael D > >; Rebecca Cran >

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Rebecca,

It looks like VC++ is trying to honor the volatile by reading the variable, 
incase that has side effects. But the loop is not checking the value of the 
variable and it is just doing an unconditional jump. This is why I think it is 
likely a compiler bug. Since the compiler emitted a hard code jmp in a loop it 
optimized out the return instruction….

$LN10@CpuDeadLoo:
mov   rax, QWORD PTR Index$[rsp]
call  CpuPause
jmp   SHORT $LN10@CpuDeadLoo
….

So changing the variable does not break you out of the loop. If you pc += 2 
when you are at the jmp instruction that will not return you from CpuDeadLoop() 
that will just fall into the next function. That might work if CpuDeadLoop() 
was inlined, but if it was a call you would start running the next function in 
the binary. 

Thanks,

Andrew Fish


> On May 18, 2023, at 10:36 AM, Rebecca Cran  wrote:
> 
> When I use CpuDeadLoop for debugging on Aarch64 I have symbols loaded so I 
> can just do ‘set Index=1’ and resume, but it sounds like the issue is that 
> people want to sometimes debug without symbols/source, and the generated 
> assembly is making that difficult.
> 
> Rebecca
> 
> On Thu, May 18, 2023, at 9:36 AM, Michael D Kinney wrote:
>> Hi Ray,
>> 
>> So the code generated does deadloop, but is just not easy to resume 
>> from as we have been able to do in the past.
>> 
>> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with 
>> no reason to ever continue.
>> 
>> The 2nd is a debug aide for developers to halt the system at a specific 
>> location and then continue from that point, usually with a debugger, to 
>> step through code to an area to evaluate unexpected behavior.
>> 
>> We may have to do a NASM implementation of CpuDeadloop() to make sure 
>> it meets both use cases.
>> 
>> Mike
>> 
>> *From:* Ni, Ray  
>> *Sent:* Thursday, May 18, 2023 3:00 AM
>> *To:* devel@edk2.groups.io
>> *Cc:* Kinney, Michael D ; Rebecca Cran 
>> ; Ni, Ray 
>> *Subject:* CpuDeadLoop() is optimized by compiler
>> 
>> Hi,
>> Starting from certain version of Visual Studio C compiler (I don’t have 
>> the exact version. I am using VS2019), CpuDeadLoop is now optimized 
>> quite well by compiler.
>> 
>> The optimization is so “good” that it becomes harder for developers to 
>> break out of the deadloop.
>> 
>> I copied the assembly instructions as below for your reference.
>> The compiler does not generate instructions that jump out of the loop 
>> when the Index is not zero.
>> So in order to break out of the loop, developers need to:
>> 1. Manually adjust rsp by increasing 40
>> 2. Manually “ret”
>> 
>> I am not sure if anyone has interest to re-write this function so that 
>> compiler can be “fooled” again.
>> Thanks,
>> Ray
>> 
>> ===
>> ; Function compile flags: /Ogspy
>> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
>> ;  COMDAT CpuDeadLoop
>> _TEXTSEGMENT
>> Index$ = 48
>> CpuDeadLoop PROC
>>; COMDAT
>> 
>> ; 26   : {
>> 
>> $LN12:
>>  0  48 83 ec 28 subrsp, 40 
>>   ; 0028H
>> 
>> ; 27   :   volatile UINTN  Index;
>> ; 28   : 
>> ; 29   :   for (Index = 0; Index == 0;) {
>> 
>>  4  48 c7 44 24 30
>>   00 00 00 00mov  QWORD PTR Index$[rsp], 0
>> $LN10@CpuDeadLoo:
>> 
>> ; 30   : CpuPause ();
>> 
>>  d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>>  00012  e8 00 00 00 00   callCpuPause
>>  00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
>> CpuDeadLoop ENDP
>> _TEXTENDS
>> END
>> 
>> 
>> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105047): https://edk2.groups.io/g/devel/message/105047
Mute This Topic: https://groups.io/mt/98987896/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-stable202305 PATCH 1/1] Rename GCC5 toolchain references in docs and scripts to GCC

2023-05-18 Thread Rebecca Cran

I'm not sure I understand. Do you mean adding Cc lines to the commit 
message, or splitting the patch into a series with the package name in 
the subject? Or just moving the package owners from the To: line to Cc:?



--
Rebecca


On 5/17/23 18:10, Kinney, Michael D wrote:

Please add Cc lines for all package owners so it matches typical email filters.

I agree this is a good cleanup to use GCC instead of GCC5 consistently.

We should let each package owner provide feedback on if this change should go 
in before or after the stable tag.

It is not a critical bug fix, so it does not meet the standard criteria for a 
fix in hard freeze.

Mike


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Rebecca
Cran
Sent: Wednesday, May 17, 2023 11:59 AM
To: devel@edk2.groups.io; Sean Brogan ;
Michael Kubacki ; Gao, Liming
; Leif Lindholm ;
Ard Biesheuvel ; Sami Mujawar
; Gerd Hoffmann ; Feng, Bob
C ; Chen, Christine ; Sami
Mujawar ; Alexei Fedorov
; Pierre Gondois ;
Andrew Fish ; Ni, Ray ; Yao, Jiewen
; Justen, Jordan L ; Boeuf,
Sebastien ; Desimone, Nathaniel L

Cc: Rebecca Cran 
Subject: [edk2-devel] [edk2-stable202305 PATCH 1/1] Rename GCC5 toolchain
references in docs and scripts to GCC

The GCC5 toolchain has been deprecated, and GCC should be used going
forward.

Update references to GCC5 in filenames, scripts and documentation.

Signed-off-by: Rebecca Cran 
---
  .azurepipelines/{Ubuntu-GCC5.yml => Ubuntu-GCC.yml}|  
4 ++-
-
  .azurepipelines/templates/platform-build-run-steps.yml |  
2 +-
  .azurepipelines/templates/pr-gate-build-job.yml|  
2 +-
  .azurepipelines/templates/pr-gate-steps.yml|  
2 +-
  ArmPlatformPkg/Scripts/Makefile|  
2 +-
  ArmVirtPkg/PlatformCI/.azurepipelines/{Ubuntu-GCC5.yml => Ubuntu-
GCC.yml}  |  4 ++--
  ArmVirtPkg/PlatformCI/ReadMe.md|  
2 +-
  BaseTools/Edk2ToolsBuild.py|  
2 +-
  BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py
|  2 +-
  DynamicTablesPkg/Readme.md | 
10 -
  EmulatorPkg/PlatformCI/.azurepipelines/{Ubuntu-GCC5.yml => Ubuntu-
GCC.yml} |  4 ++--
  EmulatorPkg/PlatformCI/ReadMe.md   |  
2 +-
  EmulatorPkg/Readme.md  |  
8 +++
  EmulatorPkg/build.sh   | 
14 -
  OvmfPkg/CloudHv/README |  
2 +-
  OvmfPkg/IntelTdx/README|  
4 ++--
  OvmfPkg/PlatformCI/.azurepipelines/{Ubuntu-GCC5.yml => Ubuntu-GCC.yml}
|  4 ++--
  OvmfPkg/PlatformCI/ReadMe.md   |  
2 +-
  OvmfPkg/build.sh   | 
12 +++
  PrmPkg/Readme.md   |  
6 +++---
  ReadMe.rst | 
22 ++--
  UnitTestFrameworkPkg/ReadMe.md |  
8 +++
  22 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/.azurepipelines/Ubuntu-GCC5.yml b/.azurepipelines/Ubuntu-
GCC.yml
similarity index 84%
rename from .azurepipelines/Ubuntu-GCC5.yml
rename to .azurepipelines/Ubuntu-GCC.yml
index b9a3b851cf3c..c6ddcc7f9af6 100644
--- a/.azurepipelines/Ubuntu-GCC5.yml
+++ b/.azurepipelines/Ubuntu-GCC.yml
@@ -1,5 +1,5 @@
  ## @file
-# Azure Pipeline build file for a build using ubuntu and GCC5
+# Azure Pipeline build file for a build using ubuntu and GCC
  #
  # Copyright (c) Microsoft Corporation.
  # Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights
reserved.
@@ -19,7 +19,7 @@ variables:
  jobs:
  - template: templates/pr-gate-build-job.yml
parameters:
-tool_chain_tag: 'GCC5'
+tool_chain_tag: 'GCC'
  vm_image: 'ubuntu-22.04'
  container: ${{ variables.default_linux_image }}
  arch_list: "IA32,X64,ARM,AARCH64,RISCV64,LOONGARCH64"
diff --git a/.azurepipelines/templates/platform-build-run-steps.yml
b/.azurepipelines/templates/platform-build-run-steps.yml
index 8be46cda0e2d..3d7515d76a8e 100644
--- a/.azurepipelines/templates/platform-build-run-steps.yml
+++ b/.azurepipelines/templates/platform-build-run-steps.yml
@@ -43,7 +43,7 @@ steps:
  echo "##vso[task.prependpath]${HOME}/.local/bin"
  echo "new PATH=${PATH}"
displayName: Set PATH
-  condition: eq('${{ parameters.tool_chain_tag }}', 'GCC5')
+  condition: eq('${{ parameters.tool_chain_tag }}', 'GCC')

  - checkout: self
clean: true
diff --git a/.azurepipelines/templates/pr-gate-build-job.yml
b/.azurepipelines/templates/pr-gate-build-job.yml
index

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Rebecca Cran

When I use CpuDeadLoop for debugging on Aarch64 I have symbols loaded so I can 
just do ‘set Index=1’ and resume, but it sounds like the issue is that people 
want to sometimes debug without symbols/source, and the generated assembly is 
making that difficult.

Rebecca

On Thu, May 18, 2023, at 9:36 AM, Michael D Kinney wrote:
> Hi Ray,
> 
> So the code generated does deadloop, but is just not easy to resume 
> from as we have been able to do in the past.
> 
> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with 
> no reason to ever continue.
> 
> The 2nd is a debug aide for developers to halt the system at a specific 
> location and then continue from that point, usually with a debugger, to 
> step through code to an area to evaluate unexpected behavior.
> 
> We may have to do a NASM implementation of CpuDeadloop() to make sure 
> it meets both use cases.
> 
> Mike
> 
> *From:* Ni, Ray  
> *Sent:* Thursday, May 18, 2023 3:00 AM
> *To:* devel@edk2.groups.io
> *Cc:* Kinney, Michael D ; Rebecca Cran 
> ; Ni, Ray 
> *Subject:* CpuDeadLoop() is optimized by compiler
> 
> Hi,
> Starting from certain version of Visual Studio C compiler (I don’t have 
> the exact version. I am using VS2019), CpuDeadLoop is now optimized 
> quite well by compiler.
> 
> The optimization is so “good” that it becomes harder for developers to 
> break out of the deadloop.
> 
> I copied the assembly instructions as below for your reference.
> The compiler does not generate instructions that jump out of the loop 
> when the Index is not zero.
> So in order to break out of the loop, developers need to:
>  1. Manually adjust rsp by increasing 40
>  2. Manually “ret”
> 
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
> Thanks,
> Ray
> 
> ===
> ; Function compile flags: /Ogspy
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
> ;  COMDAT CpuDeadLoop
> _TEXTSEGMENT
> Index$ = 48
> CpuDeadLoop PROC
> ; COMDAT
> 
> ; 26   : {
> 
> $LN12:
>   0  48 83 ec 28 subrsp, 40 
>; 0028H
> 
> ; 27   :   volatile UINTN  Index;
> ; 28   : 
> ; 29   :   for (Index = 0; Index == 0;) {
> 
>   4  48 c7 44 24 30
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
> $LN10@CpuDeadLoo:
> 
> ; 30   : CpuPause ();
> 
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>   00012  e8 00 00 00 00   callCpuPause
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
> CpuDeadLoop ENDP
> _TEXTENDS
> END
> 
> 
> 


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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Mike,

I guess my other question… If this turns out to be a compiler bug should we 
scope the change to the broken toolchain. I’m not sure what the right answer is 
for that, but I want to ask the question? 

Thanks,

Andrew Fish

> On May 18, 2023, at 10:19 AM, Michael D Kinney  
> wrote:
> 
> Andrew,
>  
> This might work for XIP.  Set non const global to initial value that is 
> expected value to stay in dead loop.
>  
> UINTN  mDeadLoopCount = 0;
>  
> VOID
> CpuDeadLoop(
>   VOID
>   ) 
> {
>   while (mDeadLoopCount == 0) {
>   CpuPause();
>   }
> }
>  
> When deadloop is entered, developer can not change value of mDeadLoopCount, 
> but they can use debugger to force exit loop and return from function.
>  
> Mike
>  
>  
> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
> Sent: Thursday, May 18, 2023 10:09 AM
> To: Kinney, Michael D  >
> Cc: edk2-devel-groups-io  >; Ni, Ray  >; Rebecca Cran  >
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Mike,
>  
> Good point, that is why we are using the stack ….
>  
> The only other thing I can think of is to pass the address of Index to some 
> inline assembler, or an asm no op function, to give it a side effect the 
> compiler can’t resolve. 
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> On May 18, 2023, at 10:05 AM, Kinney, Michael D  > wrote:
>  
> Static global will not work for XIP
>  
> Mike
>  
> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
> Sent: Thursday, May 18, 2023 9:49 AM
> To: edk2-devel-groups-io  >; Kinney, Michael D  >
> Cc: Ni, Ray mailto:ray...@intel.com>>; Rebecca Cran 
> mailto:rebe...@bsdio.com>>
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Mike,
>  
> I pinged some compiler experts to see if our code is correct, or if the 
> compiler has an issue. Seems to be trending compiler issue right now, but 
> I’ve NOT gotten feedback from anyone on the spec committee yet. 
>  
> If we move Index to a static global that would likely work around the 
> compiler issue.
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> 
> On May 18, 2023, at 8:36 AM, Michael D Kinney  > wrote:
>  
> Hi Ray,
>  
> So the code generated does deadloop, but is just not easy to resume from as 
> we have been able to do in the past.
>  
> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
> reason to ever continue.
>  
> The 2nd is a debug aide for developers to halt the system at a specific 
> location and then continue from that point, usually with a debugger, to step 
> through code to an area to evaluate unexpected behavior.
>  
> We may have to do a NASM implementation of CpuDeadloop() to make sure it 
> meets both use cases.
>  
> Mike
>  
> From: Ni, Ray mailto:ray...@intel.com>> 
> Sent: Thursday, May 18, 2023 3:00 AM
> To: devel@edk2.groups.io 
> Cc: Kinney, Michael D  >; Rebecca Cran  >; Ni, Ray  >
> Subject: CpuDeadLoop() is optimized by compiler
>  
> Hi,
> Starting from certain version of Visual Studio C compiler (I don’t have the 
> exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
> compiler.
>  
> The optimization is so “good” that it becomes harder for developers to break 
> out of the deadloop.
>  
> I copied the assembly instructions as below for your reference.
> The compiler does not generate instructions that jump out of the loop when 
> the Index is not zero.
> So in order to break out of the loop, developers need to:
> Manually adjust rsp by increasing 40
> Manually “ret”
>  
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
> Thanks,
> Ray
>  
> ===
> ; Function compile flags: /Ogspy
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
> ;  COMDAT CpuDeadLoop
> _TEXTSEGMENT
> Index$ = 48
> CpuDeadLoop PROC  
>   ; COMDAT
>  
> ; 26   : {
>  
> $LN12:
>   0  48 83 ec 28 subrsp, 40   
>  ; 0028H
>  
> ; 27   :   volatile UINTN  Index;
> ; 28   : 
> ; 29   :   for (Index = 0; Index == 0;) {
>  
>   4  48 c7 44 24 30
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
> $LN10@CpuDeadLoo:
>  
> ; 30   : CpuPause ();
>  
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>   00012  e8 00 00 00 00   callCpuPause
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
> CpuDeadLoop ENDP
> _TEXTENDS
> END
>  
>  
>  
> 



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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Yea,  but I think you want static and volatile on the global. But good idea as 
for the non XIP case you can just modify the global. It might be a good idea to 
document the debugging flow in the header for CpuDeadLoop()...

Thanks,

Andrew Fish

> On May 18, 2023, at 10:19 AM, Kinney, Michael D  
> wrote:
> 
> Andrew,
>  
> This might work for XIP.  Set non const global to initial value that is 
> expected value to stay in dead loop.
>  
> UINTN  mDeadLoopCount = 0;
>  
> VOID
> CpuDeadLoop(
>   VOID
>   ) 
> {
>   while (mDeadLoopCount == 0) {
>   CpuPause();
>   }
> }
>  
> When deadloop is entered, developer can not change value of mDeadLoopCount, 
> but they can use debugger to force exit loop and return from function.
>  
> Mike
>  
>  
> From: Andrew (EFI) Fish  
> Sent: Thursday, May 18, 2023 10:09 AM
> To: Kinney, Michael D 
> Cc: edk2-devel-groups-io ; Ni, Ray ; 
> Rebecca Cran 
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Mike,
>  
> Good point, that is why we are using the stack ….
>  
> The only other thing I can think of is to pass the address of Index to some 
> inline assembler, or an asm no op function, to give it a side effect the 
> compiler can’t resolve. 
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> On May 18, 2023, at 10:05 AM, Kinney, Michael D  > wrote:
>  
> Static global will not work for XIP
>  
> Mike
>  
> From: Andrew (EFI) Fish mailto:af...@apple.com>> 
> Sent: Thursday, May 18, 2023 9:49 AM
> To: edk2-devel-groups-io  >; Kinney, Michael D  >
> Cc: Ni, Ray mailto:ray...@intel.com>>; Rebecca Cran 
> mailto:rebe...@bsdio.com>>
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Mike,
>  
> I pinged some compiler experts to see if our code is correct, or if the 
> compiler has an issue. Seems to be trending compiler issue right now, but 
> I’ve NOT gotten feedback from anyone on the spec committee yet. 
>  
> If we move Index to a static global that would likely work around the 
> compiler issue.
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> 
> On May 18, 2023, at 8:36 AM, Michael D Kinney  > wrote:
>  
> Hi Ray,
>  
> So the code generated does deadloop, but is just not easy to resume from as 
> we have been able to do in the past.
>  
> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
> reason to ever continue.
>  
> The 2nd is a debug aide for developers to halt the system at a specific 
> location and then continue from that point, usually with a debugger, to step 
> through code to an area to evaluate unexpected behavior.
>  
> We may have to do a NASM implementation of CpuDeadloop() to make sure it 
> meets both use cases.
>  
> Mike
>  
> From: Ni, Ray mailto:ray...@intel.com>> 
> Sent: Thursday, May 18, 2023 3:00 AM
> To: devel@edk2.groups.io 
> Cc: Kinney, Michael D  >; Rebecca Cran  >; Ni, Ray  >
> Subject: CpuDeadLoop() is optimized by compiler
>  
> Hi,
> Starting from certain version of Visual Studio C compiler (I don’t have the 
> exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
> compiler.
>  
> The optimization is so “good” that it becomes harder for developers to break 
> out of the deadloop.
>  
> I copied the assembly instructions as below for your reference.
> The compiler does not generate instructions that jump out of the loop when 
> the Index is not zero.
> So in order to break out of the loop, developers need to:
> Manually adjust rsp by increasing 40
> Manually “ret”
>  
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
> Thanks,
> Ray
>  
> ===
> ; Function compile flags: /Ogspy
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
> ;  COMDAT CpuDeadLoop
> _TEXTSEGMENT
> Index$ = 48
> CpuDeadLoop PROC  
>   ; COMDAT
>  
> ; 26   : {
>  
> $LN12:
>   0  48 83 ec 28 subrsp, 40   
>  ; 0028H
>  
> ; 27   :   volatile UINTN  Index;
> ; 28   : 
> ; 29   :   for (Index = 0; Index == 0;) {
>  
>   4  48 c7 44 24 30
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
> $LN10@CpuDeadLoo:
>  
> ; 30   : CpuPause ();
>  
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>   00012  e8 00 00 00 00   callCpuPause
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
> CpuDeadLoop ENDP
> _TEXTENDS
> END
>  
>  
> 



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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Michael D Kinney

Andrew,

This might work for XIP.  Set non const global to initial value that is 
expected value to stay in dead loop.

UINTN  mDeadLoopCount = 0;

VOID
CpuDeadLoop(
  VOID
  )
{
  while (mDeadLoopCount == 0) {
  CpuPause();
  }
}

When deadloop is entered, developer can not change value of mDeadLoopCount, but 
they can use debugger to force exit loop and return from function.

Mike


From: Andrew (EFI) Fish 
Sent: Thursday, May 18, 2023 10:09 AM
To: Kinney, Michael D 
Cc: edk2-devel-groups-io ; Ni, Ray ; 
Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Mike,

Good point, that is why we are using the stack ….

The only other thing I can think of is to pass the address of Index to some 
inline assembler, or an asm no op function, to give it a side effect the 
compiler can’t resolve.

Thanks,

Andrew Fish


On May 18, 2023, at 10:05 AM, Kinney, Michael D 
mailto:michael.d.kin...@intel.com>> wrote:

Static global will not work for XIP

Mike

From: Andrew (EFI) Fish mailto:af...@apple.com>>
Sent: Thursday, May 18, 2023 9:49 AM
To: edk2-devel-groups-io mailto:devel@edk2.groups.io>>; 
Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Ni, Ray mailto:ray...@intel.com>>; Rebecca Cran 
mailto:rebe...@bsdio.com>>
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Mike,

I pinged some compiler experts to see if our code is correct, or if the 
compiler has an issue. Seems to be trending compiler issue right now, but I’ve 
NOT gotten feedback from anyone on the spec committee yet.

If we move Index to a static global that would likely work around the compiler 
issue.

Thanks,

Andrew Fish



On May 18, 2023, at 8:36 AM, Michael D Kinney 
mailto:michael.d.kin...@intel.com>> wrote:

Hi Ray,

So the code generated does deadloop, but is just not easy to resume from as we 
have been able to do in the past.

We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
reason to ever continue.

The 2nd is a debug aide for developers to halt the system at a specific 
location and then continue from that point, usually with a debugger, to step 
through code to an area to evaluate unexpected behavior.

We may have to do a NASM implementation of CpuDeadloop() to make sure it meets 
both use cases.

Mike

From: Ni, Ray mailto:ray...@intel.com>>
Sent: Thursday, May 18, 2023 3:00 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Rebecca Cran 
mailto:rebe...@bsdio.com>>; Ni, Ray 
mailto:ray...@intel.com>>
Subject: CpuDeadLoop() is optimized by compiler

Hi,
Starting from certain version of Visual Studio C compiler (I don’t have the 
exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
compiler.

The optimization is so “good” that it becomes harder for developers to break 
out of the deadloop.

I copied the assembly instructions as below for your reference.
The compiler does not generate instructions that jump out of the loop when the 
Index is not zero.
So in order to break out of the loop, developers need to:

  1.  Manually adjust rsp by increasing 40
  2.  Manually “ret”

I am not sure if anyone has interest to re-write this function so that compiler 
can be “fooled” again.
Thanks,
Ray

===
; Function compile flags: /Ogspy
; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
;  COMDAT CpuDeadLoop
_TEXTSEGMENT
Index$ = 48
CpuDeadLoop PROC
; COMDAT

; 26   : {

$LN12:
  0  48 83 ec 28 subrsp, 40
; 0028H

; 27   :   volatile UINTN  Index;
; 28   :
; 29   :   for (Index = 0; Index == 0;) {

  4  48 c7 44 24 30
   00 00 00 00mov  QWORD PTR Index$[rsp], 0
$LN10@CpuDeadLoo:

; 30   : CpuPause ();

  d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
  00012  e8 00 00 00 00   callCpuPause
  00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
CpuDeadLoop ENDP
_TEXTENDS
END






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105042): https://edk2.groups.io/g/devel/message/105042
Mute This Topic: https://groups.io/mt/98987896/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] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Mike,

Good point, that is why we are using the stack ….

The only other thing I can think of is to pass the address of Index to some 
inline assembler, or an asm no op function, to give it a side effect the 
compiler can’t resolve. 

Thanks,

Andrew Fish

> On May 18, 2023, at 10:05 AM, Kinney, Michael D  
> wrote:
> 
> Static global will not work for XIP
>  
> Mike
>  
> From: Andrew (EFI) Fish  
> Sent: Thursday, May 18, 2023 9:49 AM
> To: edk2-devel-groups-io ; Kinney, Michael D 
> 
> Cc: Ni, Ray ; Rebecca Cran 
> Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler
>  
> Mike,
>  
> I pinged some compiler experts to see if our code is correct, or if the 
> compiler has an issue. Seems to be trending compiler issue right now, but 
> I’ve NOT gotten feedback from anyone on the spec committee yet. 
>  
> If we move Index to a static global that would likely work around the 
> compiler issue.
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> On May 18, 2023, at 8:36 AM, Michael D Kinney  > wrote:
>  
> Hi Ray,
>  
> So the code generated does deadloop, but is just not easy to resume from as 
> we have been able to do in the past.
>  
> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
> reason to ever continue.
>  
> The 2nd is a debug aide for developers to halt the system at a specific 
> location and then continue from that point, usually with a debugger, to step 
> through code to an area to evaluate unexpected behavior.
>  
> We may have to do a NASM implementation of CpuDeadloop() to make sure it 
> meets both use cases.
>  
> Mike
>  
> From: Ni, Ray mailto:ray...@intel.com>> 
> Sent: Thursday, May 18, 2023 3:00 AM
> To: devel@edk2.groups.io 
> Cc: Kinney, Michael D  >; Rebecca Cran  >; Ni, Ray  >
> Subject: CpuDeadLoop() is optimized by compiler
>  
> Hi,
> Starting from certain version of Visual Studio C compiler (I don’t have the 
> exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
> compiler.
>  
> The optimization is so “good” that it becomes harder for developers to break 
> out of the deadloop.
>  
> I copied the assembly instructions as below for your reference.
> The compiler does not generate instructions that jump out of the loop when 
> the Index is not zero.
> So in order to break out of the loop, developers need to:
> Manually adjust rsp by increasing 40
> Manually “ret”
>  
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
> Thanks,
> Ray
>  
> ===
> ; Function compile flags: /Ogspy
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
> ;  COMDAT CpuDeadLoop
> _TEXTSEGMENT
> Index$ = 48
> CpuDeadLoop PROC  
>   ; COMDAT
>  
> ; 26   : {
>  
> $LN12:
>   0  48 83 ec 28 subrsp, 40   
>  ; 0028H
>  
> ; 27   :   volatile UINTN  Index;
> ; 28   : 
> ; 29   :   for (Index = 0; Index == 0;) {
>  
>   4  48 c7 44 24 30
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
> $LN10@CpuDeadLoo:
>  
> ; 30   : CpuPause ();
>  
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>   00012  e8 00 00 00 00   callCpuPause
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
> CpuDeadLoop ENDP
> _TEXTENDS
> END
>  
>  
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105041): https://edk2.groups.io/g/devel/message/105041
Mute This Topic: https://groups.io/mt/98987896/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] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Michael D Kinney

Static global will not work for XIP

Mike

From: Andrew (EFI) Fish 
Sent: Thursday, May 18, 2023 9:49 AM
To: edk2-devel-groups-io ; Kinney, Michael D 

Cc: Ni, Ray ; Rebecca Cran 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

Mike,

I pinged some compiler experts to see if our code is correct, or if the 
compiler has an issue. Seems to be trending compiler issue right now, but I’ve 
NOT gotten feedback from anyone on the spec committee yet.

If we move Index to a static global that would likely work around the compiler 
issue.

Thanks,

Andrew Fish

On May 18, 2023, at 8:36 AM, Michael D Kinney 
mailto:michael.d.kin...@intel.com>> wrote:

Hi Ray,

So the code generated does deadloop, but is just not easy to resume from as we 
have been able to do in the past.

We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
reason to ever continue.

The 2nd is a debug aide for developers to halt the system at a specific 
location and then continue from that point, usually with a debugger, to step 
through code to an area to evaluate unexpected behavior.

We may have to do a NASM implementation of CpuDeadloop() to make sure it meets 
both use cases.

Mike

From: Ni, Ray mailto:ray...@intel.com>>
Sent: Thursday, May 18, 2023 3:00 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Rebecca Cran 
mailto:rebe...@bsdio.com>>; Ni, Ray 
mailto:ray...@intel.com>>
Subject: CpuDeadLoop() is optimized by compiler

Hi,
Starting from certain version of Visual Studio C compiler (I don’t have the 
exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
compiler.

The optimization is so “good” that it becomes harder for developers to break 
out of the deadloop.

I copied the assembly instructions as below for your reference.
The compiler does not generate instructions that jump out of the loop when the 
Index is not zero.
So in order to break out of the loop, developers need to:

  1.  Manually adjust rsp by increasing 40
  2.  Manually “ret”

I am not sure if anyone has interest to re-write this function so that compiler 
can be “fooled” again.
Thanks,
Ray

===
; Function compile flags: /Ogspy
; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
;  COMDAT CpuDeadLoop
_TEXTSEGMENT
Index$ = 48
CpuDeadLoop PROC
; COMDAT

; 26   : {

$LN12:
  0  48 83 ec 28 subrsp, 40
; 0028H

; 27   :   volatile UINTN  Index;
; 28   :
; 29   :   for (Index = 0; Index == 0;) {

  4  48 c7 44 24 30
   00 00 00 00mov  QWORD PTR Index$[rsp], 0
$LN10@CpuDeadLoo:

; 30   : CpuPause ();

  d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
  00012  e8 00 00 00 00   callCpuPause
  00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
CpuDeadLoop ENDP
_TEXTENDS
END

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105040): https://edk2.groups.io/g/devel/message/105040
Mute This Topic: https://groups.io/mt/98987896/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] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Andrew Fish via groups.io

Mike,

I pinged some compiler experts to see if our code is correct, or if the 
compiler has an issue. Seems to be trending compiler issue right now, but I’ve 
NOT gotten feedback from anyone on the spec committee yet. 

If we move Index to a static global that would likely work around the compiler 
issue.

Thanks,

Andrew Fish

> On May 18, 2023, at 8:36 AM, Michael D Kinney  
> wrote:
> 
> Hi Ray,
>  
> So the code generated does deadloop, but is just not easy to resume from as 
> we have been able to do in the past.
>  
> We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
> reason to ever continue.
>  
> The 2nd is a debug aide for developers to halt the system at a specific 
> location and then continue from that point, usually with a debugger, to step 
> through code to an area to evaluate unexpected behavior.
>  
> We may have to do a NASM implementation of CpuDeadloop() to make sure it 
> meets both use cases.
>  
> Mike
>  
> From: Ni, Ray mailto:ray...@intel.com>> 
> Sent: Thursday, May 18, 2023 3:00 AM
> To: devel@edk2.groups.io 
> Cc: Kinney, Michael D  >; Rebecca Cran  >; Ni, Ray  >
> Subject: CpuDeadLoop() is optimized by compiler
>  
> Hi,
> Starting from certain version of Visual Studio C compiler (I don’t have the 
> exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
> compiler.
>  
> The optimization is so “good” that it becomes harder for developers to break 
> out of the deadloop.
>  
> I copied the assembly instructions as below for your reference.
> The compiler does not generate instructions that jump out of the loop when 
> the Index is not zero.
> So in order to break out of the loop, developers need to:
> Manually adjust rsp by increasing 40
> Manually “ret”
>  
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
> Thanks,
> Ray
>  
> ===
> ; Function compile flags: /Ogspy
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
> ;  COMDAT CpuDeadLoop
> _TEXTSEGMENT
> Index$ = 48
> CpuDeadLoop PROC  
>   ; COMDAT
>  
> ; 26   : {
>  
> $LN12:
>   0  48 83 ec 28 subrsp, 40   
>  ; 0028H
>  
> ; 27   :   volatile UINTN  Index;
> ; 28   : 
> ; 29   :   for (Index = 0; Index == 0;) {
>  
>   4  48 c7 44 24 30
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
> $LN10@CpuDeadLoo:
>  
> ; 30   : CpuPause ();
>  
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>   00012  e8 00 00 00 00   callCpuPause
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
> CpuDeadLoop ENDP
> _TEXTENDS
> END
>  
>  
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105039): https://edk2.groups.io/g/devel/message/105039
Mute This Topic: https://groups.io/mt/98987896/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 v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

2023-05-18 Thread Sami Mujawar

Hi Ard,

On Thu, May 18, 2023 at 08:17 AM, Ard Biesheuvel wrote:

> 
> Given the wider discussion we had the other day about tightening
> memory protections, I think it is important that we get this fixed but
> I don't think there is any urgency to it. I sent some patches a couple
> of months ago to map DxeCore code and data with tightened permissions
> as well, and I think we can revisit this in that context the next time
> around.
> 
> So for now, just passing the stack size as you suggested above is
> sufficient IMO.

[SAMI] Thanks, I will submit a v2 series with the changes.

I also observed that a similar change would be needed for ArmPlatformPkg/PrePi 
at
https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/PrePi/PrePi.c#L164
As of now we do not enable PcdCpuStackGuard in
edk2-platforms\Platform\ARM\VExpressPkg\ArmVExpress.dsc.inc
But if this is done the same stack overflow issue is seen on the FVP model.

Considering that, should I send out a patch for ArmPlatformPkg/PrePi as well?

Also, I think it be good to enable the stack guard check in
edk2-platforms\Platform\ARM\VExpressPkg\ArmVExpress.dsc.inc.
Please let me know your thoughts about this.

[/SAMI]

Regards,

Sami Mujawar

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105038): https://edk2.groups.io/g/devel/message/105038
Mute This Topic: https://groups.io/mt/98987538/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] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread Michael D Kinney

Is this considered a critical bug for stable tag release?

Is there are HSD and if it is critical, all memory allocation lobs should be 
fixed.  Right?

Mike

> -Original Message-
> From: Ni, Ray 
> Sent: Thursday, May 18, 2023 1:55 AM
> To: Tan, Dun ; devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: RE: [Patch V2] MdePkg: Code optimization to SMM
> InternalAllocateAlignedPages
> 
> Reviewed-by: Ray Ni 
> 
> > -Original Message-
> > From: Tan, Dun 
> > Sent: Thursday, May 18, 2023 3:31 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray ; Kinney, Michael D
> > ; Gao, Liming ;
> Liu,
> > Zhiguang 
> > Subject: [Patch V2] MdePkg: Code optimization to SMM
> > InternalAllocateAlignedPages
> >
> > This commit is code optimization to InternalAllocateAlignedPages of
> > SmmMemoryAllocationLib which can reduce free memory fragments. Also
> > it can reduce one pre-allocation page.
> >
> > Let's take a simple example:
> > The expected pages size is 8KB, Alignment value is 8KB.
> >
> > In original InternalAllocateAlignedPages(), the first step is to
> > allocate 4 pages and then find the first 8KB-aligned address in
> > allocated 4 pages. If the upper limit address of allocated 4 pages
> > is already 8KB aligned, then the allocated 4 pages contains two
> > 8KB-aligned 8KB ranges. The lower 2 pages will be selected and
> > removed from free pages. Then the higher 2 pages will be free.
> > Since the whole memory allocation is from high address to low
> > address, then the higher 2 pages cann't be merged with other free
> > pages, causing the free memory fragments.
> >
> > However, when only allocate 3(2+2-1) pages, we can avoid the free
> > memory fragments in specific case. Also 3 pages must contain a
> > 8KB-aligned 8KB range, which meets the requirement. If the upper
> > limit address of allocated 3 pages is 8KB-aligned, then the higher
> > 2 pages range of allocated 3 pages is 8KB-aligned and will be
> > selected and removed from free pages. The remaining lower one page
> > of allocated 3 pages will be free and merged with left lower free
> > memory. This can reduce free memory fragments in smm.
> >
> > Signed-off-by: Dun Tan 
> > Cc: Ray Ni 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > ---
> >  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > index 3ab2c1ebfd..99a8259325 100644
> > --- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > +++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> > @@ -322,7 +322,7 @@ InternalAllocateAlignedPages (
> >  // Calculate the total number of pages since alignment is larger than
> page size.
> >  //
> >  AlignmentMask = Alignment - 1;
> > -RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> > +RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment) - 1;
> >  //
> >  // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> > overflow.
> >  //
> > --
> > 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105037): https://edk2.groups.io/g/devel/message/105037
Mute This Topic: https://groups.io/mt/98986911/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] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Michael D Kinney

Hi Ray,

So the code generated does deadloop, but is just not easy to resume from as we 
have been able to do in the past.

We use CpuDeadloop() for 2 purposes.  One is a terminal condition with no 
reason to ever continue.

The 2nd is a debug aide for developers to halt the system at a specific 
location and then continue from that point, usually with a debugger, to step 
through code to an area to evaluate unexpected behavior.

We may have to do a NASM implementation of CpuDeadloop() to make sure it meets 
both use cases.

Mike

From: Ni, Ray 
Sent: Thursday, May 18, 2023 3:00 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D ; Rebecca Cran 
; Ni, Ray 
Subject: CpuDeadLoop() is optimized by compiler

Hi,
Starting from certain version of Visual Studio C compiler (I don't have the 
exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
compiler.

The optimization is so "good" that it becomes harder for developers to break 
out of the deadloop.

I copied the assembly instructions as below for your reference.
The compiler does not generate instructions that jump out of the loop when the 
Index is not zero.
So in order to break out of the loop, developers need to:

  1.  Manually adjust rsp by increasing 40
  2.  Manually "ret"

I am not sure if anyone has interest to re-write this function so that compiler 
can be "fooled" again.
Thanks,
Ray

===
; Function compile flags: /Ogspy
; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
;  COMDAT CpuDeadLoop
_TEXTSEGMENT
Index$ = 48
CpuDeadLoop PROC
; COMDAT

; 26   : {

$LN12:
  0  48 83 ec 28 subrsp, 40
; 0028H

; 27   :   volatile UINTN  Index;
; 28   :
; 29   :   for (Index = 0; Index == 0;) {

  4  48 c7 44 24 30
   00 00 00 00mov  QWORD PTR Index$[rsp], 0
$LN10@CpuDeadLoo:

; 30   : CpuPause ();

  d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
  00012  e8 00 00 00 00   callCpuPause
  00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
CpuDeadLoop ENDP
_TEXTENDS
END




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105036): https://edk2.groups.io/g/devel/message/105036
Mute This Topic: https://groups.io/mt/98987896/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] Building error

2023-05-18 Thread Andrew Fish via groups.io

In the WmiAwcc.asl file it looks like the 1109th byte has some invalid UTF-8 
encoding. So it looks like the file got edited with an editor that us using an 
encoding other than ASCII or UTF-8?

Thanks,

Andrew Fish

> On May 18, 2023, at 1:43 AM, Eva.Ss.Feng (ńTÉșÉș)  
> wrote:
> 
> HI Devel
> I have a little question that I need to ask for you. When I was building the 
> code, the following error occurred. The error message tells me to email you 
> for help. The branch I build can be compiled on someone else's computer, but 
> it cannot be compiled successfully on my computer. I tried checking out to 
> another branch and it was successful. I don't understand the reason for the 
> moment, so I sent you an email with the error message attached to the error 
> message.
>  
> 
>  
> Eva 馮珊珊
> Foxconn Technology Group-
> PCEBG-DTSA/R/DPD/FWOD
> Tel：63150/13307169381
> Teams: eva.ss.f...@foxconn.com 
> mail:eva.ss.f...@foxconn.com 
> UC: shanshan.f...@foxconn.net 
> 
>  
> ⌘---
> 
> 本電子郵件及附件所載之信息為保密信息，其内容僅供指定收件人使用。上述信息為發表人之個人意見，並不代表本公司之立場。若您因為誤傳而收到本郵件，請立即以郵件通知寄件人，並將本郵件删除。禁止將本郵件及附件複製、散佈或洩露給任何第三者。本郵件經由網路傳遞，有可能遭竄改或病毒之入侵。為確保資訊之安全，收件人請就本郵件及附件之重要内容，與寄件人進行確認。
> 
> This email and any attachments to it may be confidential and are intended 
> solely for the use of the individual to whom it is addressed. Any views or 
> opinions expressed are solely those of the author and do not necessarily 
> represent those of Hon Hai/Foxconn. If you have received it by mistake, 
> please inform the sender by an email reply and then delete the message. It is 
> forbidden to copy, forward, or in any way reveal the contents of this message 
> to anyone. The integrity and security of this email cannot be guaranteed over 
> the Internet. Therefore, it's advised for the receiver to verify the validity 
> of this message with the sender.
> 
> ---⌘
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105035): https://edk2.groups.io/g/devel/message/105035
Mute This Topic: https://groups.io/mt/98993280/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 v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

2023-05-18 Thread Ard Biesheuvel

On Thu, 18 May 2023 at 17:12, Sami Mujawar  wrote:
>
> Hi Ard,
>
> Thank you for the feedback and for the pointers.
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 18/05/2023 12:01 pm, Ard Biesheuvel via groups.io wrote:
> > On Thu, 18 May 2023 at 11:10, Sami Mujawar  wrote:
> >> The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
> >> enabled stack overflow detection for ArmVirtPkg. Following
> >> this patch, running UEFI shell command 'dmpstore' resulted
> >> in a crash indicating a stack overflow. Invoking 'dmpstore'
> >> results in recursive calls to CascadeProcessVariables ()
> >> which apparently consumes the available stack space and
> >> overflows.
> >>
> >> Therefore, increase the primary core stack size.
> >>
> > Thanks for the fix. I imagine diagnosing this may not have been trivial.
> >
> > However, I don't think this is the right fix tbh. Normally, SEC and
> > PEI run off this initial stack, and the DxeIpl PEIM is in charging of
> > launching the DxeCore with a full sized stack, and remapping it
> > non-executable as well.
> >
> > These PrePi platforms take some shortcuts and apparently, one of the
> > consequences is that DXE and BDS run off the initial stack, which
> > points into the firmware image IIRC.
> >
> > IOW, it would be better to explicitly allocate 128 KiB worth of
> > bootservices data memory and let the DxeCore run off of that.
>
> [SAMI] If the stack size is passed in LoadDxeCoreFromFv() at
>
> https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/PrePi/PrePi.c#L104,
> the code at
>
> https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Library/PrePiLib/PrePiLib.c#L158-L182
>
> allocates the stack and switches it. I have set the stack size to 128KB
> in the call to
>
> LoadDxeCoreFromFv (NULL, SIZE_128KB) and it fixes the issue.
>

Fantastic, so all the code we need is already there.

> However, the PrePiLib implementation lacks the code to remap the stack
> as NonExecutable as
>
> done by  the DxeIplPeim code at
>
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c#L42-L45).
>
> I have added a call to ArmSetMemoryRegionNoExec () in PrePiLib and it
> works. However, this code would
>
> need to go in a separate Arch specific file. I am not sure what would be
> required for other architectures but
>
> I can submit a patch that adds an arch hook function 'ArchSetStackNx
> (UINTN StackBase, UINTN StackSize)' to
>
> set the stack as NonExecutable and provide an implementation for Arm.
> Other architectures can similarly
>
> implement this function.
>
> Please let me know if this approach is ok.
>

Given the wider discussion we had the other day about tightening
memory protections, I think it is important that we get this fixed but
I don't think there is any urgency to it. I sent some patches a couple
of months ago to map DxeCore code and data with tightened permissions
as well, and I think we can revisit this in that context the next time
around.

So for now, just passing the stack size as you suggested above is
sufficient IMO.

Thanks,
Ard.


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

[edk2-devel] Status of OpenSSL 3.0 patch

2023-05-18 Thread Luse, Paul E

Hello Tianocore Community!

I’m a software developer working on a storage product that uses just a few EDK 
II / OpenSSL functions related to key derivation in support of self-encrypting 
drives.  We were recently made aware of the pending EOL of the current OpenSSL 
in EDK II and have a release of our product coming up soon so are trying to get 
some insight into the following patch 
https://github.com/tianocore/edk2-staging/tree/OpenSSL11_EOL.

Both myself and several of my team members have a decent amount of open source 
experience but not with Tianocore and nothing with OpenSSL either.   So mostly 
what I’m looking for is some information on:


  *   Status/next steps on the patch above.  Who is driving, do they have a 
specific date range/goal for landing?
  *   Is anyone else out there a consumer of this and also in need of timely 
resolution, maybe we can brainstorm some thoughts?

I would *love* to be able to jump in and help with the patch but with our lack 
of experience here, I’m afraid we might just slow things down.  But, I’m open 
to any and all suggestions :)

Thanks!
Paul

PS: FYI I’ll be attending the next community meeting as well so look forward to 
talking some more about it there as well.





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105033): https://edk2.groups.io/g/devel/message/105033
Mute This Topic: https://groups.io/mt/98993293/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] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN Coverity issue

2023-05-18 Thread Ranbir Singh

FspData->PerfIdx is getting increased for every call unconditionally
in the function SetFspMeasurePoint and hence memory access can happen
for out of bound FspData->PerfData[] array entries also.

Example -
   FspData->PerfData is an array of 32 UINT64 entries. Assume a call
   is made to SetFspMeasurePoint function when the FspData->PerfIdx
   last value is 31. It gets incremented to 32 at line 400.
   Any subsequent call to SetFspMeasurePoint functions leads to
   FspData->PerfData[32] getting accessed which is out of the PerfData
   array as well as the FSP_GLOBAL_DATA structure boundary.

Hence keep array access and index increment inside if block only and
return invalid performance timestamp when PerfIdx is invalid.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Star Zeng 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4200
Signed-off-by: Ranbir Singh 
Signed-off-by: Ranbir Singh 
---
 IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c 
b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
index a22b0e7825ad..cda2a7b2478e 100644
--- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
+++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
@@ -377,7 +377,8 @@ GetFspSiliconInitUpdDataPointer (
 
   @param[in] Id   Measurement point ID.
 
-  @return performance timestamp.
+  @return performance timestamp if current PerfIdx is valid,
+  else return 0 as invalid performance timestamp
 **/
 UINT64
 EFIAPI
@@ -395,9 +396,10 @@ SetFspMeasurePoint (
   if (FspData->PerfIdx < sizeof (FspData->PerfData) / sizeof 
(FspData->PerfData[0])) {
 FspData->PerfData[FspData->PerfIdx]  = AsmReadTsc ();
 ((UINT8 *)(>PerfData[FspData->PerfIdx]))[7] = Id;
+return FspData->PerfData[(FspData->PerfIdx)++];
   }
 
-  return FspData->PerfData[(FspData->PerfIdx)++];
+  return (UINT64)0x;
 }
 
 /**
-- 
2.34.1



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

[edk2-devel] Building error

2023-05-18 Thread �T珊珊

HI Devel
I have a little question that I need to ask for you. When I was building the 
code, the following error occurred. The error message tells me to email you for 
help. The branch I build can be compiled on someone else's computer, but it 
cannot be compiled successfully on my computer. I tried checking out to another 
branch and it was successful. I don't understand the reason for the moment, so 
I sent you an email with the error message attached to the error message.

[cid:image002.png@01D98973.0418AA70]

Eva �T珊珊
Foxconn Technology Group-
PCEBG-DTSA/R/DPD/FWOD
Tel：63150/13307169381
Teams: eva.ss.f...@foxconn.com
mail:eva.ss.f...@foxconn.com
UC: shanshan.f...@foxconn.net
[查看源图像]


?---

本��子�]件及附件所�d之信息�楸Ｃ苄畔�，其内容�H供指定收件人使用。上述信息�榘l表人之��人意��，�K不代表本公司之立�觥Ｈ裟�因�檎`�鞫�收到本�]件，��立即以�]件通知寄件人，�K�⒈距]件删除。禁止�⒈距]件及附件�}�u、散�鸦���露�o任何第三者。本�]件��由�W路�鬟f，有可能遭�Z改或病毒之入侵。�榇_保�Y��之安全，收件人��就本�]件及附件之重要内容，�c寄件人�M行�_�J。

This email and any attachments to it may be confidential and are intended 
solely for the use of the individual to whom it is addressed. Any views or 
opinions expressed are solely those of the author and do not necessarily 
represent those of Hon Hai/Foxconn. If you have received it by mistake, please 
inform the sender by an email reply and then delete the message. It is 
forbidden to copy, forward, or in any way reveal the contents of this message 
to anyone. The integrity and security of this email cannot be guaranteed over 
the Internet. Therefore, it's advised for the receiver to verify the validity 
of this message with the sender.

---?


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

Re: [edk2-devel] [PATCH v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

2023-05-18 Thread Sami Mujawar


Hi Ard,

Thank you for the feedback and for the pointers.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/05/2023 12:01 pm, Ard Biesheuvel via groups.io wrote:

On Thu, 18 May 2023 at 11:10, Sami Mujawar  wrote:

The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
enabled stack overflow detection for ArmVirtPkg. Following
this patch, running UEFI shell command 'dmpstore' resulted
in a crash indicating a stack overflow. Invoking 'dmpstore'
results in recursive calls to CascadeProcessVariables ()
which apparently consumes the available stack space and
overflows.

Therefore, increase the primary core stack size.


Thanks for the fix. I imagine diagnosing this may not have been trivial.

However, I don't think this is the right fix tbh. Normally, SEC and
PEI run off this initial stack, and the DxeIpl PEIM is in charging of
launching the DxeCore with a full sized stack, and remapping it
non-executable as well.

These PrePi platforms take some shortcuts and apparently, one of the
consequences is that DXE and BDS run off the initial stack, which
points into the firmware image IIRC.

IOW, it would be better to explicitly allocate 128 KiB worth of
bootservices data memory and let the DxeCore run off of that.


[SAMI] If the stack size is passed in LoadDxeCoreFromFv() at

https://github.com/tianocore/edk2/blob/master/ArmVirtPkg/PrePi/PrePi.c#L104, 
the code at


https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Library/PrePiLib/PrePiLib.c#L158-L182

allocates the stack and switches it. I have set the stack size to 128KB 
in the call to


LoadDxeCoreFromFv (NULL, SIZE_128KB) and it fixes the issue.

However, the PrePiLib implementation lacks the code to remap the stack 
as NonExecutable as


done by  the DxeIplPeim code at

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c#L42-L45).

I have added a call to ArmSetMemoryRegionNoExec () in PrePiLib and it 
works. However, this code would


need to go in a separate Arch specific file. I am not sure what would be 
required for other architectures but


I can submit a patch that adds an arch hook function 'ArchSetStackNx 
(UINTN StackBase, UINTN StackSize)' to


set the stack as NonExecutable and provide an implementation for Arm. 
Other architectures can similarly


implement this function.

Please let me know if this approach is ok.

[/SAMI]





Signed-off-by: Sami Mujawar 
---
  ArmVirtPkg/ArmVirtKvmTool.dsc | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 
4541d03d23e0d98915b3d3ada688c48d979b75d2..664a624fd2a30bb466a3df2103482e3e6c1f303a
 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -126,7 +126,7 @@ [PcdsFixedAtBuild.common]
gArmTokenSpaceGuid.PcdVFPEnabled|1
  !endif

-  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'









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

[edk2-devel] [PATCH V2 3/3] ShellPkg/AcpiView: Add MPAM Parser

2023-05-18 Thread Rohit Mathew

Add a parser for the MPAM (Memory system resource partitioning and
monitoring) ACPI table. This parser would parse all MPAM related
structures embedded as part of the ACPI table. Necessary validations are
also performed where and when required.

Signed-off-by: Rohit Mathew 
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h| 
  21 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c   | 
1331 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.h   | 
  25 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c   | 
   3 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf | 
   4 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni | 
   3 +-
 6 files changed, 1384 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index c9f41650d9..fef08e714d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -825,6 +825,27 @@ ParseAcpiMcfg (
   IN UINT8AcpiTableRevision
   );
 
+/**
+  This function parses the ACPI MPAM table.
+  When trace is enabled this function parses the MCFG table and
+  traces the ACPI table fields.
+
+  This function also performs validation of the ACPI table fields.
+
+  @param [in] Trace  If TRUE, trace the ACPI fields.
+  @param [in] PtrPointer to the start of the buffer.
+  @param [in] AcpiTableLengthLength of the ACPI table.
+  @param [in] AcpiTableRevision  Revision of the ACPI table.
+**/
+VOID
+EFIAPI
+ParseAcpiMpam (
+  IN BOOLEAN  Trace,
+  IN UINT8*Ptr,
+  IN UINT32   AcpiTableLength,
+  IN UINT8AcpiTableRevision
+  );
+
 /**
   This function parses the ACPI PCCT table including its sub-structures
   of type 0 through 4.
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c
new file mode 100644
index 00..9352357318
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c
@@ -0,0 +1,1331 @@
+/** @file
+  MPAM table parser
+
+  Copyright (c) 2023, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Specification Reference:
+   - [1] ACPI for Memory System Resource Partitioning and Monitoring 2.0
+ (https://developer.arm.com/documentation/den0065/latest)
+
+  @par Glossary:
+- MPAM - Memory System Resource Partitioning And Monitoring
+- MSC  - Memory System Component
+- PCC  - Platform Communication Channel
+- RIS  - Resource Instance Selection
+- SMMU - Arm System Memory Management Unit
+ **/
+
+#include 
+#include 
+#include 
+#include "AcpiParser.h"
+#include "AcpiView.h"
+#include "AcpiViewConfig.h"
+#include "MpamParser.h"
+
+// Local variables
+STATIC UINT8 *Locator;
+STATIC CONST UINT8   *LocatorType;
+STATIC CONST UINT16  *MpamMscNodeLength;
+STATIC UINT32MpamMscNodeLengthCumulative;
+STATIC UINT32HeaderSize;
+STATIC CONST UINT32  *ErrorInterrupt;
+STATIC CONST UINT32  *InterfaceType;
+STATIC CONST UINT32  *NumberOfMscResources;
+STATIC CONST UINT32  *NumberOfFunctionalDependencies;
+STATIC CONST UINT32  *NumberOfInterconnectDescriptors;
+STATIC CONST UINT32  *OverflowInterrupt;
+STATIC ACPI_DESCRIPTION_HEADER_INFO  AcpiHdrInfo;
+
+/**
+  When the length of the table is insufficient to be parsed, this function 
could
+  be used to display an appropriate error message.
+
+  @param [in] ErrorMsg  Error message string that has to be appended to the
+  main error log. This string could explain the reason
+  why a insufficient length error was encountered in
+  the first place.
+**/
+STATIC
+VOID
+EFIAPI
+MpamLengthError (
+  IN CONST CHAR16  *ErrorMsg
+  )
+{
+  IncrementErrorCount ();
+  Print (L"\nERROR : ");
+  Print (ErrorMsg);
+  Print (
+L"\nError : Insufficient MPAM MSC Node length. Table length : %u.\n",
+*(AcpiHdrInfo.Length)
+);
+}
+
+/**
+  This function validates the passed reserved parameter. Any reserved field
+  within the MPAM specification must be 0.
+
+  @param [in] Reserved Reserved param that has to be validated.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateReserved (
+  IN CONST UINT64  Reserved
+  )
+{
+  if (Reserved != 0) {
+IncrementErrorCount ();
+Print (L"\nERROR : Reserved field should be 0\n");
+  }
+}
+
+/**
+  This function validates reserved fields. Any reserved field within the MPAM
+  specification must be 0.
+
+  @param [in] Ptr   Pointer to the start of the field data.
+  @param [in]

[edk2-devel] [PATCH V2 2/3] ShellPkg: acpiview: Add routine to print 16 chars

2023-05-18 Thread Rohit Mathew

Certain ACPI tables like MPAM has fields which are 16 bytes long.
Routines similar to Dump12Chars but for 16 characters are required to
print such fields. Add Dump16Chars routine to satisfy this requirement.

Signed-off-by: Rohit Mathew 
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 39 
+++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 18 -
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index eac9286176..87f55098b8 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI parser
 
-  Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2016 - 2023, Arm Limited. All rights reserved.
   Copyright (c) 2022, AMD Incorporated. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -449,6 +449,43 @@ Dump12Chars (
 );
 }
 
+/**
+  This function traces 16 characters which can be optionally
+  formated using the format string if specified.
+
+  If no format string is specified the Format must be NULL.
+
+  @param [in] Format  Optional format string for tracing the data.
+  @param [in] Ptr Pointer to the start of the buffer.
+**/
+VOID
+EFIAPI
+Dump16Chars (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  Print (
+(Format != NULL) ? Format : L"%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c",
+Ptr[0],
+Ptr[1],
+Ptr[2],
+Ptr[3],
+Ptr[4],
+Ptr[5],
+Ptr[6],
+Ptr[7],
+Ptr[8],
+Ptr[9],
+Ptr[10],
+Ptr[11],
+Ptr[12],
+Ptr[13],
+Ptr[14],
+Ptr[15]
+);
+}
+
 /**
   This function indents and prints the ACPI table Field Name.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 4b4397961b..c9f41650d9 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -2,7 +2,7 @@
   Header file for ACPI parser
 
   Copyright (c) 2022, NVIDIA CORPORATION. All rights reserved.
-  Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
+  Copyright (c) 2016 - 2023, Arm Limited. All rights reserved.
   Copyright (c) 2022, AMD Incorporated. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -202,6 +202,22 @@ Dump12Chars (
   IN   UINT8   *Ptr
   );
 
+/**
+  This function traces 16 characters which can be optionally
+  formated using the format string if specified.
+
+  If no format string is specified the Format must be NULL.
+
+  @param [in] Format  Optional format string for tracing the data.
+  @param [in] Ptr Pointer to the start of the buffer.
+**/
+VOID
+EFIAPI
+Dump16Chars (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  );
+
 /**
   This function indents and prints the ACPI table Field Name.
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105028): https://edk2.groups.io/g/devel/message/105028
Mute This Topic: https://groups.io/mt/98993071/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] MdePkg/IndustryStandard: add definitions for MPAM ACPI specification

2023-05-18 Thread Rohit Mathew

From: Rohit Mathew 

add definitions, macros and types for elements associated with MPAM
ACPI 2.0 specification.

Signed-off-by: Rohit Mathew 
---
 MdePkg/Include/IndustryStandard/Acpi65.h |   7 +-
 MdePkg/Include/IndustryStandard/Mpam.h   | 258 
 2 files changed, 264 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/Acpi65.h 
b/MdePkg/Include/IndustryStandard/Acpi65.h
index 1e41ae9a27..8a1d3d125a 100644
--- a/MdePkg/Include/IndustryStandard/Acpi65.h
+++ b/MdePkg/Include/IndustryStandard/Acpi65.h
@@ -2,7 +2,7 @@
   ACPI 6.5 definitions from the ACPI Specification Revision 6.5 Aug, 2022.
 
   Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
-  Copyright (c) 2019 - 2021, ARM Ltd. All rights reserved.
+  Copyright (c) 2019 - 2023, ARM Ltd. All rights reserved.
   Copyright (c) 2023, Loongson Technology Corporation Limited. All rights 
reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -3251,6 +3251,11 @@ typedef struct {
 ///
 #define EFI_ACPI_6_5_XEN_PROJECT_TABLE_SIGNATURE  SIGNATURE_32('X', 'E', 'N', 
'V')
 
+///
+/// "MPAM" Memory System Resource Partitioning and Monitoring Table
+///
+#define 
EFI_ACPI_MEMORY_SYSTEM_RESOURCE_PARTITIONING_AND_MONITORING_TABLE_SIGNATURE  
SIGNATURE_32('M', 'P', 'A', 'M')
+
 #pragma pack()
 
 #endif
diff --git a/MdePkg/Include/IndustryStandard/Mpam.h 
b/MdePkg/Include/IndustryStandard/Mpam.h
new file mode 100644
index 00..5d4c466abb
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Mpam.h
@@ -0,0 +1,258 @@
+/** @file
+  ACPI for Memory System Resource Partitioning and Monitoring 2.0 (MPAM) as
+  specified in ARM spec DEN0065
+
+  Copyright (c) 2023, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Specification Reference:
+   - [1] ACPI for Memory System Resource Partitioning and Monitoring 2.0
+ (https://developer.arm.com/documentation/den0065/latest)
+
+  @par Glossary:
+- MPAM - Memory System Resource Partitioning And Monitoring
+- MSC  - Memory System Component
+- PCC  - Platform Communication Channel
+- RIS  - Resource Instance Selection
+- SMMU - Arm System Memory Management Unit
+ **/
+
+#ifndef MPAM_H_
+#define MPAM_H_
+
+#include 
+
+///
+/// MPAM Revision
+///
+#define 
EFI_ACPI_MEMORY_SYSTEM_RESOURCE_PARTITIONING_AND_MONITORING_TABLE_REVISION  
(0x01)
+
+///
+/// MPAM Interrupt mode
+///
+#define EFI_ACPI_MPAM_INTERRUPT_LEVEL_TRIGGERED  (0x0)
+#define EFI_ACPI_MPAM_INTERRUPT_EDGE_TRIGGERED   (0x1)
+
+///
+/// MPAM Interrupt type
+///
+#define EFI_ACPI_MPAM_INTERRUPT_WIRED  (0x0)
+
+///
+/// MPAM Interrupt affinity type
+///
+#define EFI_ACPI_MPAM_INTERRUPT_PROCESSOR_AFFINITY(0x0)
+#define EFI_ACPI_MPAM_INTERRUPT_PROCESSOR_CONTAINER_AFFINITY  (0x1)
+
+///
+/// MPAM MSC affinity valid
+///
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_NOT_VALID  (0x0)
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID  (0x1)
+
+///
+/// MPAM Interrupt flag - bit positions
+///
+#define EFI_ACPI_MPAM_INTERRUPT_MODE_SHIFT(0)
+#define EFI_ACPI_MPAM_INTERRUPT_TYPE_SHIFT(1)
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_TYPE_SHIFT   (3)
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID_SHIFT  (4)
+#define EFI_ACPI_MPAM_INTERRUPT_RESERVED_SHIFT(5)
+
+///
+/// MPAM Interrupt flag - bit masks
+///
+#define EFI_ACPI_MPAM_INTERRUPT_MODE_MASK(0x1)
+#define EFI_ACPI_MPAM_INTERRUPT_TYPE_MASK(0x3)
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_TYPE_MASK   (0x8)
+#define EFI_ACPI_MPAM_INTERRUPT_AFFINITY_VALID_MASK  (0x10)
+#define EFI_ACPI_MPAM_INTERRUPT_RESERVED_MASK(0xFFE0)
+
+///
+/// MPAM_MEMORY_LOCATOR_RESERVED_MASK should be used along with an 8 byte 
object
+/// starting at the base of the locator field.
+///
+#define EFI_ACPI_MPAM_MEM_LOCATOR_RESERVED_FIELD_MASK   (0x00FFULL)
+#define EFI_ACPI_MPAM_MEM_LOCATOR_RESERVED_FIELD_SHIFT  (0)
+
+///
+/// MPAM_MEMORY_LOCATOR_LEVEL_MASK should be used along with an 8 byte object
+///  starting at the base of the locator field.
+///
+#define EFI_ACPI_MPAM_MEM_LOCATOR_LEVEL_FIELD_MASK   (0xFF00ULL)
+#define EFI_ACPI_MPAM_MEM_LOCATOR_LEVEL_FIELD_SHIFT  (56)
+
+///
+/// MPAM Location types
+/// as described in document [1], table 11
+///
+#define EFI_ACPI_MPAM_LOCATION_PROCESSOR_CACHE  (0x0)
+#define EFI_ACPI_MPAM_LOCATION_MEMORY   (0x1)
+#define EFI_ACPI_MPAM_LOCATION_SMMU (0x2)
+#define EFI_ACPI_MPAM_LOCATION_MEMORY_CACHE (0x3)
+#define EFI_ACPI_MPAM_LOCATION_ACPI_DEVICE  (0x4)
+#define EFI_ACPI_MPAM_LOCATION_INTERCONNECT (0x5)
+#define EFI_ACPI_MPAM_LOCATION_UNKNOWN  (0xFF)
+
+///
+/// MPAM Interface types
+///
+#define EFI_ACPI_MPAM_INTERFACE_MMIO  (0x0)
+#define EFI_ACPI_MPAM_INTERFACE_PCC   (0x1)
+
+///
+/// MPAM Link types
+///
+#define EFI_ACPI_MPAM_LINK_TYPE_NUMA  (0x0)
+#define EFI_ACPI_MPAM_LINK_TYPE_PROC  (0x0A)
+
+#pragma pack(1)
+
+///
+/// MPAM MSC generic

[edk2-devel] [PATCH V2 0/3] MPAM ACPI definitions and parser

2023-05-18 Thread Rohit Mathew

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.

Link to github:

https://github.com/rohit-arm/edk2/tree/mpam_acpi

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/log/?h=mpam/snapshot/v6.2

Rohit Mathew (3):
  MdePkg/IndustryStandard: add definitions for MPAM ACPI specification
  ShellPkg: acpiview: Add routine to print 16 chars
  ShellPkg/AcpiView: Add MPAM Parser

 MdePkg/Include/IndustryStandard/Acpi65.h | 
   7 +-
 MdePkg/Include/IndustryStandard/Mpam.h   | 
 258 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c| 
  39 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h| 
  39 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c   | 
1331 
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.h   | 
  25 +
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c   | 
   3 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf | 
   4 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni | 
   3 +-
 9 files changed, 1703 insertions(+), 6 deletions(-)
 create mode 100644 MdePkg/Include/IndustryStandard/Mpam.h
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.c
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamParser.h

-- 
2.34.1



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

Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Pedro Falcato

On Thu, May 18, 2023 at 10:59 AM Ni, Ray  wrote:
>
> Hi,
>
> Starting from certain version of Visual Studio C compiler (I don’t have the 
> exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
> compiler.
>
>
>
> The optimization is so “good” that it becomes harder for developers to break 
> out of the deadloop.
>
>
>
> I copied the assembly instructions as below for your reference.
>
> The compiler does not generate instructions that jump out of the loop when 
> the Index is not zero.
>
> So in order to break out of the loop, developers need to:
>
> Manually adjust rsp by increasing 40
> Manually “ret”
>
>
>
> I am not sure if anyone has interest to re-write this function so that 
> compiler can be “fooled” again.
>
> Thanks,
> Ray
>
>
>
> ===
>
> ; Function compile flags: /Ogspy
>
> ; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
>
> ;  COMDAT CpuDeadLoop
>
> _TEXTSEGMENT
>
> Index$ = 48
>
> CpuDeadLoop PROC  
>   ; COMDAT
>
>
>
> ; 26   : {
>
>
>
> $LN12:
>
>   0  48 83 ec 28 subrsp, 40   
>  ; 0028H
>
>
>
> ; 27   :   volatile UINTN  Index;
>
> ; 28   :
>
> ; 29   :   for (Index = 0; Index == 0;) {
>
>
>
>   4  48 c7 44 24 30
>
>00 00 00 00mov  QWORD PTR Index$[rsp], 0
>
> $LN10@CpuDeadLoo:
>
>
>
> ; 30   : CpuPause ();
>
>
>
>   d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
>
>   00012  e8 00 00 00 00   callCpuPause
>
>   00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
>
> CpuDeadLoop ENDP
>
> _TEXTENDS
>
> END

Hi Ray,

Can you try something like this? https://godbolt.org/z/x7P1PqY59

Seems to work, but godbolt does not support MSVC LTO :/

-- 
Pedro


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

Re: [edk2-devel] [PATCH v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

2023-05-18 Thread Ard Biesheuvel

On Thu, 18 May 2023 at 11:10, Sami Mujawar  wrote:
>
> The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
> enabled stack overflow detection for ArmVirtPkg. Following
> this patch, running UEFI shell command 'dmpstore' resulted
> in a crash indicating a stack overflow. Invoking 'dmpstore'
> results in recursive calls to CascadeProcessVariables ()
> which apparently consumes the available stack space and
> overflows.
>
> Therefore, increase the primary core stack size.
>

Thanks for the fix. I imagine diagnosing this may not have been trivial.

However, I don't think this is the right fix tbh. Normally, SEC and
PEI run off this initial stack, and the DxeIpl PEIM is in charging of
launching the DxeCore with a full sized stack, and remapping it
non-executable as well.

These PrePi platforms take some shortcuts and apparently, one of the
consequences is that DXE and BDS run off the initial stack, which
points into the firmware image IIRC.

IOW, it would be better to explicitly allocate 128 KiB worth of
bootservices data memory and let the DxeCore run off of that.


> Signed-off-by: Sami Mujawar 
> ---
>  ArmVirtPkg/ArmVirtKvmTool.dsc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index 
> 4541d03d23e0d98915b3d3ada688c48d979b75d2..664a624fd2a30bb466a3df2103482e3e6c1f303a
>  100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -126,7 +126,7 @@ [PcdsFixedAtBuild.common]
>gArmTokenSpaceGuid.PcdVFPEnabled|1
>  !endif
>
> -  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>


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

[edk2-devel] CpuDeadLoop() is optimized by compiler

2023-05-18 Thread Ni, Ray

Hi,
Starting from certain version of Visual Studio C compiler (I don't have the 
exact version. I am using VS2019), CpuDeadLoop is now optimized quite well by 
compiler.

The optimization is so "good" that it becomes harder for developers to break 
out of the deadloop.

I copied the assembly instructions as below for your reference.
The compiler does not generate instructions that jump out of the loop when the 
Index is not zero.
So in order to break out of the loop, developers need to:

  1.  Manually adjust rsp by increasing 40
  2.  Manually "ret"

I am not sure if anyone has interest to re-write this function so that compiler 
can be "fooled" again.
Thanks,
Ray

===
; Function compile flags: /Ogspy
; File e:\work\edk2\MdePkg\Library\BaseLib\CpuDeadLoop.c
;  COMDAT CpuDeadLoop
_TEXTSEGMENT
Index$ = 48
CpuDeadLoop PROC
; COMDAT

; 26   : {

$LN12:
  0  48 83 ec 28 subrsp, 40
; 0028H

; 27   :   volatile UINTN  Index;
; 28   :
; 29   :   for (Index = 0; Index == 0;) {

  4  48 c7 44 24 30
   00 00 00 00mov  QWORD PTR Index$[rsp], 0
$LN10@CpuDeadLoo:

; 30   : CpuPause ();

  d  48 8b 44 24 30   mov  rax, QWORD PTR Index$[rsp]
  00012  e8 00 00 00 00   callCpuPause
  00017  eb f4 jmp   SHORT $LN10@CpuDeadLoo
CpuDeadLoop ENDP
_TEXTENDS
END




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105023): https://edk2.groups.io/g/devel/message/105023
Mute This Topic: https://groups.io/mt/98987896/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 v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.

2023-05-18 Thread gaoliming via groups.io

Zhihao:
  Have you root cause this issue that SmmVariableSetVariable may return
EFI_ACCESS_DENIED?

  I am not sure whether this fix is proper. I also add UefiCpuPkg
maintainers Ray and Gerd in the mail loop for this discussion. 

Thanks
Liming
> -邮件原件-
> 发件人: Zhihao Li 
> 发送时间: 2023年5月10日 18:57
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang ; Liming Gao
> 
> 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> check before SmmSetVariable.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> 
> For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> arrive to smm before it set the variable. If not, it would return
> EFI_ACCESS_DENIED.
> 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> 
> Signed-off-by: Zhihao Li 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 10 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> |  3 ++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |  3 ++-
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 5253c328dcd9..4944903e64d4 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -14,7 +14,7 @@
>VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> ReclaimForOS(),
> 
>SmmVariableGetStatistics() should also do validation based on its own
> knowledge.
> 
> 
> 
> -Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> 
> +Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
> 
>  Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
> @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  #include 
> 
>  #include 
> 
> +#include 
> 
> 
> 
>  #include 
> 
>  #include "Variable.h"
> 
> @@ -87,6 +88,13 @@ SmmVariableSetVariable (
>  {
> 
>EFI_STATUS  Status;
> 
> 
> 
> +  //
> 
> +  // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> 
> +  //
> 
> +  if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> 
> +DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check in
> SMM!\n"));
> 
> +  }
> 
> +
> 
>//
> 
>// Disable write protection when the calling SetVariable() through
> EFI_SMM_VARIABLE_PROTOCOL.
> 
>//
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 8c552b87e080..1cf0d051e6c9 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -18,7 +18,7 @@
>  #  may not be modified without authorization. If platform fails to
protect
> these resources,
> 
>  #  the authentication service provided in this driver will be broken, and
the
> behavior is undefined.
> 
>  #
> 
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> 
> +# Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
> 
>  # Copyright (c) Microsoft Corporation.
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
> @@ -84,6 +84,7 @@
>VariablePolicyLib
> 
>VariablePolicyHelperLib
> 
>SafeIntLib
> 
> +  SmmCpuRendezvousLib
> 
> 
> 
>  [Protocols]
> 
>gEfiSmmFirmwareVolumeBlockProtocolGuid## CONSUMES
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> index f09bed40cf51..89187456ca25 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> @@ -18,7 +18,7 @@
>  #  may not be modified without authorization. If platform fails to
protect
> these resources,
> 
>  #  the authentication service provided in this driver will be broken, and
the
> behavior is undefined.
> 
>  #
> 
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> 
> +# Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
> 
>  # Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> 
>  # Copyright (c) Microsoft Corporation.
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -80,6 +80,7 @@
>VariableFlashInfoLib
> 
>VariablePolicyLib
> 
>VariablePolicyHelperLib
> 
> +  SmmCpuRendezvousLib
> 
> 
> 
>  [Protocols]
> 
>gEfiSmmFirmwareVolumeBlockProtocolGuid## CONSUMES
> 
> --
> 2.26.2.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105022): https://edk2.groups.io/g/devel/message/105022
Mute This Topic: https://groups.io/mt/98987751/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/6] ArmPkg: Configure PcdEmuVariableNvModeEnable as a dynamic PCD

2023-05-18 Thread Sami Mujawar

The PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
indicates if a variable driver will emulate the variable NV mode.
This PCD is defined as [PcdsFixedAtBuild, PcdsPatchableInModule,
PcdsDynamic, PcdsDynamicEx].

Some firmware builds may define this PCD as a dynamic PCD and
initialise the value at runtime. Therefore, move the PCD declaration
from the [FixedPcd] section to the [Pcd] section in the platform
boot manager library file PlatformBootManagerLib.inf. Without this
change the build would not succeed.

Signed-off-by: Sami Mujawar 
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 
05ed46456cc482d735490ad4418aa75a1b331aa7..bc029be635f26fa29731a4139109d0f5eb177054
 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -57,7 +57,6 @@ [FeaturePcd]
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable
-  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
@@ -68,6 +67,7 @@ [FixedPcd]
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
 
 [Guids]
   gBootDiscoveryPolicyMgrFormsetGuid
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 6/6] ArmVirtPkg: ArmVirtQemuKernel: Increase primary core stack size

2023-05-18 Thread Sami Mujawar

The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
enabled stack overflow detection for ArmVirtPkg. Following
this patch, running UEFI shell command 'dmpstore' resulted
in a crash indicating a stack overflow. Invoking 'dmpstore'
results in recursive calls to CascadeProcessVariables ()
which apparently consumes the available stack space and
overflows.

Therefore, increase the primary core stack size.

Signed-off-by: Sami Mujawar 
---
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 
3cb9120e4e10551a2dc283f29b7d75fa8926a273..23b0296d8c7bc01b86a592e561b62c306d2fe018
 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -119,7 +119,7 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
 
-  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 !if $(NETWORK_TLS_ENABLE) == TRUE
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 3/6] ArmVirtPkg: Fallback to variable emulation if no CFI is found

2023-05-18 Thread Sami Mujawar

The kvmtool option '--flash ' is used to launch
a guests VM with a CFI flash device that maps the flash file
specified at the command line.
However, kvmtool allows guest VMs to be launched without a CFI
flash device. In such scenarios the firmware can utilize the
emulated variable storage for UEFI variables. To support this
the PCD gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
must be set to TRUE.

Therefore, update the NorFlashKvmtoolLib to fallback to variable
emulation if a CFI device is not detected. Also improve the error
logging.

Signed-off-by: Sami Mujawar 
---
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c  | 38 
+---
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf |  3 +-
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c 
b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
index 
43f5858644b1f47ada17e00fba55a670ab5862bd..2beeefdd272d6f8841f7d0b972394739b745982e
 100644
--- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c
@@ -1,7 +1,7 @@
 /** @file
An instance of the NorFlashPlatformLib for Kvmtool platform.
 
- Copyright (c) 2020, ARM Ltd. All rights reserved.
+ Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.
 
  SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -228,7 +228,7 @@ NorFlashPlatformLibConstructor (
   CONST CHAR8   *Label;
   UINT32LabelLen;
 
-  if (mNorFlashDeviceCount != 0) {
+  if ((mNorFlashDeviceCount != 0) || PcdGetBool (PcdEmuVariableNvModeEnable)) {
 return EFI_SUCCESS;
   }
 
@@ -337,9 +337,39 @@ NorFlashPlatformLibConstructor (
 }
 
 if (mNorFlashDevices[UefiVarStoreIndex].DeviceBaseAddress != 0) {
-  return SetupVariableStore ([UefiVarStoreIndex]);
+  Status = SetupVariableStore ([UefiVarStoreIndex]);
+  if (EFI_ERROR (Status)) {
+DEBUG ((
+  DEBUG_ERROR,
+  "ERROR: Failed to setup variable store, Status = %r\n",
+  Status
+  ));
+ASSERT (0);
+  }
+} else {
+  DEBUG ((
+DEBUG_ERROR,
+"ERROR: Invalid Flash device Base address\n"
+));
+  ASSERT (0);
+  Status = EFI_NOT_FOUND;
+}
+  } else {
+// No Flash device found fallback to Runtime Variable Emulation.
+DEBUG ((
+  DEBUG_INFO,
+  "INFO: No Flash device found fallback to Runtime Variable Emulation.\n"
+  ));
+Status = PcdSetBoolS (PcdEmuVariableNvModeEnable, TRUE);
+if (EFI_ERROR (Status)) {
+  DEBUG ((
+DEBUG_ERROR,
+"ERROR: Failed to set PcdEmuVariableNvModeEnable, Status = %r\n",
+Status
+));
+  ASSERT (0);
 }
   }
 
-  return EFI_NOT_FOUND;
+  return Status;
 }
diff --git a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf 
b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
index 
b5f35d4782896761e7975a6e5c196ff0fab0d6db..fba1245e41ec4b146db79a821b8343247377af41
 100644
--- a/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
+++ b/ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Nor Flash library for Kvmtool.
 #
-#  Copyright (c) 2020, ARM Ltd. All rights reserved.
+#  Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -39,6 +39,7 @@ [Pcd]
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdFvSize
 
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 2/6] ArmVirtPkg: Define variables for emulating runtime variables

2023-05-18 Thread Sami Mujawar

Kvmtool allows guest VMs to be launched with or without a
CFI flash device.

When the kvmtool option '--flash ' is used to
launch a guest VM a CFI flash device maps the flash file that
was specified at the command line. The NorFlash driver uses
this flash as the variable storage backend.

However, when the above option is not specified, a CFI flash
device is not present. In such cases, the firmware can fallback
to use emulated runtime variables (which uses the VMs DRAM as
the storage backend).

Therefore, define the PCD PcdEmuVariableNvModeEnable required
to enable the emulated runtime variable support, but do not
enable it by default.

The firmware is expected to dynamically discover if the CFI
flash is present and subsequently enable NorFlash or emulate
the runtime variables.

Signed-off-by: Sami Mujawar 
---
 ArmVirtPkg/ArmVirtKvmTool.dsc | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 
d0afe1b49e250c554313c2077b89650d6f6d67cb..25920ab4ae3cce20fdbe8e9ff7e25b8696d2c851
 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -1,7 +1,7 @@
 #  @file
 #  Workspace file for KVMTool virtual platform.
 #
-#  Copyright (c) 2018 - 2022, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -219,6 +219,10 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x4
 
+  # Define PCD for emulating Runtime Variable storage when
+  # CFI flash is absent.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|FALSE
+
   ## RTC Register address in MMIO space.
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 0/6] ArmVirtPkg: Add dynamic CFI flash detection for Kvmtool guests

2023-05-18 Thread Sami Mujawar

Kvmtool allows guest VMs to be launched with or without
a CFI flash device. The guest hardware configuration can
be seen in the device tree that Kvmtool hands off to the
guest firmware.

Therefore, add support to dynamically detect if a CFI
flash device is present. If CFI is present use the
NorFlashDxe driver as the backend for variable services;
otherwise use emulated runtime variables.

The last 2 patches in this series fix a crash due to
stack overflow which is observed when running the UEFI
shell command 'dmpstore'.

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/2646_dynamic_cfi_detection_v1

Sami Mujawar (6):
  ArmPkg: Configure PcdEmuVariableNvModeEnable as a dynamic PCD
  ArmVirtPkg: Define variables for emulating runtime variables
  ArmVirtPkg: Fallback to variable emulation if no CFI is found
  ArmVirtPkg: Dispatch variable service if variable emulation is enabled
  ArmVirtPkg: Kvmtool: Increase primary core stack size
  ArmVirtPkg: ArmVirtQemuKernel: Increase primary core stack size

 ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  2 +-
 ArmVirtPkg/ArmVirtKvmTool.dsc| 13 +--
 ArmVirtPkg/ArmVirtQemuKernel.dsc |  2 +-
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c   | 13 ++-
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf |  4 ++-
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtool.c  | 38 
+---
 ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf |  3 +-
 7 files changed, 63 insertions(+), 12 deletions(-)

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 4/6] ArmVirtPkg: Dispatch variable service if variable emulation is enabled

2023-05-18 Thread Sami Mujawar

The VariableRuntimeDxe links with NvVarStoreFormattedLib which is
required to establish the dependency on OvmfPkg\VirtNorFlashDxe.
The VirtNorFlashDxe installs the gEdkiiNvVarStoreFormattedGuid to
indicate it has finished initialising the flash variable storage
and that the variable service can be dispatched.

However, the kvmtool guest firmware dynamically detects if CFI
flash is absent and sets PcdEmuVariableNvModeEnable to TRUE
indicating emulated runtime variable must be used. Therefore,
in this scenario install the gEdkiiNvVarStoreFormattedGuid so
that the variable service can be dispatched.

Also link the NorFlashKvmtoolLib as a NULL library so that
it can discover if the CFI flash is absent and setup the PCD
PcdEmuVariableNvModeEnable. This is required in case the
NorFlashDxe is not yet dispatched.

Signed-off-by: Sami Mujawar 
---
 ArmVirtPkg/ArmVirtKvmTool.dsc|  5 -
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c   | 13 -
 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf |  4 +++-
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 
25920ab4ae3cce20fdbe8e9ff7e25b8696d2c851..4541d03d23e0d98915b3d3ada688c48d979b75d2
 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -311,7 +311,10 @@ [Components.common]
   #
   # Platform Driver
   #
-  ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+  ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf {
+
+NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
+  }
   OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf
   EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
   OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c 
b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
index 
3f5027fac4d65c4ae3f370c5349c6f410aae5b43..bf6fc1f1f070f32e3ce351f57da955c5cc849409
 100644
--- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
@@ -4,7 +4,7 @@
   - It decides if the firmware should expose ACPI or Device Tree-based
 hardware description to the operating system.
 
-  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -75,6 +75,17 @@ KvmtoolPlatformDxeEntryPoint (
 {
   EFI_STATUS  Status;
 
+  if (PcdGetBool (PcdEmuVariableNvModeEnable)) {
+// The driver implementing the variable service can now be dispatched.
+Status = gBS->InstallProtocolInterface (
+,
+,
+EFI_NATIVE_INTERFACE,
+NULL
+);
+ASSERT_EFI_ERROR (Status);
+  }
+
   Status = PlatformHasAcpiDt (ImageHandle);
   ASSERT_EFI_ERROR (Status);
 
diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf 
b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
index 
c5bf798c3b2b7bf1f77e0c5ada9000f536123d6a..b0583d52058805aaeece31d7e3776ac498f101ad
 100644
--- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
@@ -3,7 +3,7 @@
 #  - It decides if the firmware should expose ACPI or Device Tree-based
 #hardware description to the operating system.
 #
-#  Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -33,10 +33,12 @@ [LibraryClasses]
   UefiDriverEntryPoint
 
 [Guids]
+  gEdkiiNvVarStoreFormattedGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
   gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
   gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
 
 [Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
   gUefiOvmfPkgTokenSpaceGuid.PcdForceNoAcpi
 
 [Depex]
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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

[edk2-devel] [PATCH v1 5/6] ArmVirtPkg: Kvmtool: Increase primary core stack size

2023-05-18 Thread Sami Mujawar

The patch "f07a9df9af60 ArmVirtPkg: Enable stack guard"
enabled stack overflow detection for ArmVirtPkg. Following
this patch, running UEFI shell command 'dmpstore' resulted
in a crash indicating a stack overflow. Invoking 'dmpstore'
results in recursive calls to CascadeProcessVariables ()
which apparently consumes the available stack space and
overflows.

Therefore, increase the primary core stack size.

Signed-off-by: Sami Mujawar 
---
 ArmVirtPkg/ArmVirtKvmTool.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index 
4541d03d23e0d98915b3d3ada688c48d979b75d2..664a624fd2a30bb466a3df2103482e3e6c1f303a
 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -126,7 +126,7 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdVFPEnabled|1
 !endif
 
-  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x8000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105015): https://edk2.groups.io/g/devel/message/105015
Mute This Topic: https://groups.io/mt/98987538/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] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread Ni, Ray

Reviewed-by: Ray Ni 

> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, May 18, 2023 3:31 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Kinney, Michael D
> ; Gao, Liming ; Liu,
> Zhiguang 
> Subject: [Patch V2] MdePkg: Code optimization to SMM
> InternalAllocateAlignedPages
> 
> This commit is code optimization to InternalAllocateAlignedPages of
> SmmMemoryAllocationLib which can reduce free memory fragments. Also
> it can reduce one pre-allocation page.
> 
> Let's take a simple example:
> The expected pages size is 8KB, Alignment value is 8KB.
> 
> In original InternalAllocateAlignedPages(), the first step is to
> allocate 4 pages and then find the first 8KB-aligned address in
> allocated 4 pages. If the upper limit address of allocated 4 pages
> is already 8KB aligned, then the allocated 4 pages contains two
> 8KB-aligned 8KB ranges. The lower 2 pages will be selected and
> removed from free pages. Then the higher 2 pages will be free.
> Since the whole memory allocation is from high address to low
> address, then the higher 2 pages cann't be merged with other free
> pages, causing the free memory fragments.
> 
> However, when only allocate 3(2+2-1) pages, we can avoid the free
> memory fragments in specific case. Also 3 pages must contain a
> 8KB-aligned 8KB range, which meets the requirement. If the upper
> limit address of allocated 3 pages is 8KB-aligned, then the higher
> 2 pages range of allocated 3 pages is 8KB-aligned and will be
> selected and removed from free pages. The remaining lower one page
> of allocated 3 pages will be free and merged with left lower free
> memory. This can reduce free memory fragments in smm.
> 
> Signed-off-by: Dun Tan 
> Cc: Ray Ni 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> ---
>  MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> index 3ab2c1ebfd..99a8259325 100644
> --- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> +++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
> @@ -322,7 +322,7 @@ InternalAllocateAlignedPages (
>  // Calculate the total number of pages since alignment is larger than 
> page size.
>  //
>  AlignmentMask = Alignment - 1;
> -RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> +RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment) - 1;
>  //
>  // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> overflow.
>  //
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105014): https://edk2.groups.io/g/devel/message/105014
Mute This Topic: https://groups.io/mt/98986911/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/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous check in PerformRemainingTasks.

2023-05-18 Thread Ni, Ray

Reviewed-by: Ray Ni 

> -Original Message-
> From: Li, Zhihao 
> Sent: Thursday, May 4, 2023 3:07 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray 
> Subject: [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous
> check in PerformRemainingTasks.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4424
> 
> In Relaxed-AP Sync Mode, BSP will not wait for all Aps arrive. However,
> PerformRemainingTasks() needs to wait all Aps arrive before calling
> SetMemMapAttributes and ConfigSmmCodeAccessCheck() when
> mSmmReadyToLock
> is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will call
> FlushTlbForAll() that need to start up the aps. So it need to let all
> aps arrive. Same as SetMemMapAttributes(), ConfigSmmCodeAccessCheck()
> also will start up the aps.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> 
> Signed-off-by: Zhihao Li 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 655175a2c6db..1e210beb0e06 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>  Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
> 
> 
> 
> -Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
> 
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
> 
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
> 
> 
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -1441,6 +1441,19 @@ PerformRemainingTasks (
>)
> 
>  {
> 
>if (mSmmReadyToLock) {
> 
> +//
> 
> +// Check if all Aps enter SMM. In Relaxed-AP Sync Mode, BSP will not 
> wait for
> 
> +// all Aps arrive. However,PerformRemainingTasks() needs to wait all Aps
> arrive before calling
> 
> +// SetMemMapAttributes() and ConfigSmmCodeAccessCheck() when
> mSmmReadyToLock
> 
> +// is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will 
> call
> 
> +// FlushTlbForAll() that need to start up the aps. So it need to let all
> 
> +// aps arrive. Same as SetMemMapAttributes(),
> ConfigSmmCodeAccessCheck()
> 
> +// also will start up the aps.
> 
> +//
> 
> +if (EFI_ERROR (SmmCpuRendezvous (NULL, TRUE))) {
> 
> +  DEBUG ((DEBUG_ERROR, "PerformRemainingTasks: fail to wait for all AP
> check in SMM!\n"));
> 
> +}
> 
> +
> 
>  //
> 
>  // Start SMM Profile feature
> 
>  //
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105013): https://edk2.groups.io/g/devel/message/105013
Mute This Topic: https://groups.io/mt/98679265/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] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread duntan

This commit is code optimization to InternalAllocateAlignedPages of
SmmMemoryAllocationLib which can reduce free memory fragments. Also
it can reduce one pre-allocation page.

Let's take a simple example:
The expected pages size is 8KB, Alignment value is 8KB.

In original InternalAllocateAlignedPages(), the first step is to
allocate 4 pages and then find the first 8KB-aligned address in
allocated 4 pages. If the upper limit address of allocated 4 pages
is already 8KB aligned, then the allocated 4 pages contains two
8KB-aligned 8KB ranges. The lower 2 pages will be selected and
removed from free pages. Then the higher 2 pages will be free.
Since the whole memory allocation is from high address to low
address, then the higher 2 pages cann't be merged with other free
pages, causing the free memory fragments.

However, when only allocate 3(2+2-1) pages, we can avoid the free
memory fragments in specific case. Also 3 pages must contain a
8KB-aligned 8KB range, which meets the requirement. If the upper
limit address of allocated 3 pages is 8KB-aligned, then the higher
2 pages range of allocated 3 pages is 8KB-aligned and will be
selected and removed from free pages. The remaining lower one page
of allocated 3 pages will be free and merged with left lower free
memory. This can reduce free memory fragments in smm.

Signed-off-by: Dun Tan 
Cc: Ray Ni 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
---
 MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c 
b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
index 3ab2c1ebfd..99a8259325 100644
--- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
+++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
@@ -322,7 +322,7 @@ InternalAllocateAlignedPages (
 // Calculate the total number of pages since alignment is larger than page 
size.
 //
 AlignmentMask = Alignment - 1;
-RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
+RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment) - 1;
 //
 // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not 
overflow.
 //
-- 
2.31.1.windows.1



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

Re: [edk2-devel] [PATCH] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread duntan

Ray,
Thanks for the comments. I'll refine the commit message according to your 
suggestions in V2 version patch.

Thanks,
Dun

-Original Message-
From: Ni, Ray  
Sent: Thursday, May 18, 2023 2:59 PM
To: Tan, Dun ; devel@edk2.groups.io
Cc: Kinney, Michael D ; Gao, Liming 
; Liu, Zhiguang 
Subject: RE: [PATCH] MdePkg: Code optimization to SMM 
InternalAllocateAlignedPages

Dun,
I can understand what you are trying to describe in the commit message.
I have some suggestions/questions that might help you refine the commit message 
a bit better.

Code changes look good to me.

Thanks,
Ray
> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, May 18, 2023 2:37 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Kinney, Michael D 
> ; Gao, Liming ; 
> Liu, Zhiguang 
> Subject: [PATCH] MdePkg: Code optimization to SMM 
> InternalAllocateAlignedPages
> 
> This commit is code optimization to InternalAllocateAlignedPages of 
> SmmMemoryAllocationLib which can reduce free memory fragments. Also it 
> can reduce one pre-allocation page.
> 
> Let's take a simple example:
> The expected pages size is 8KB, Alignment value is 8KB.
> 
> In original InternalAllocateAlignedPages(), the first step is to 
> allocate 4 pages and then find the first 8KB-aligned address in the 
> allocated 4 pages. If the limit address is already 8KB aligned, then
1. What's "limit address"?

> the allocated 4 pages contains two 8KB-aligned 8KB ranges. The lower
> 2 pages will be selected and removed from free pages. Then the higher
> 2 pages will be free. Since the whole memory allocation is from high 
> address to lower address, then the higher 2 pages cann't be merged 
> other free pages, causing the free memory fragments.
> 
> However, when only allocate 3(2+2-1) pages, we can avoid the free 
> memory fragments in specific case. Also 3 pages must contain a 
> 8kb-aligned 8kb range, which meets the requirement. If the limit of 
> allocated 3 pages is 8KB-aligned, then the last 8K range will
2. What's "last 8k"?


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

Re: [edk2-devel] [PATCH] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread Ni, Ray

Dun,
I can understand what you are trying to describe in the commit message.
I have some suggestions/questions that might help you refine the commit message 
a bit better.

Code changes look good to me.

Thanks,
Ray
> -Original Message-
> From: Tan, Dun 
> Sent: Thursday, May 18, 2023 2:37 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Kinney, Michael D
> ; Gao, Liming ; Liu,
> Zhiguang 
> Subject: [PATCH] MdePkg: Code optimization to SMM
> InternalAllocateAlignedPages
> 
> This commit is code optimization to InternalAllocateAlignedPages of
> SmmMemoryAllocationLib which can reduce free memory fragments. Also
> it can reduce one pre-allocation page.
> 
> Let's take a simple example:
> The expected pages size is 8KB, Alignment value is 8KB.
> 
> In original InternalAllocateAlignedPages(), the first step is to
> allocate 4 pages and then find the first 8KB-aligned address in the
> allocated 4 pages. If the limit address is already 8KB aligned, then
1. What's "limit address"?

> the allocated 4 pages contains two 8KB-aligned 8KB ranges. The lower
> 2 pages will be selected and removed from free pages. Then the higher
> 2 pages will be free. Since the whole memory allocation is from high
> address to lower address, then the higher 2 pages cann't be merged
> other free pages, causing the free memory fragments.
> 
> However, when only allocate 3(2+2-1) pages, we can avoid the free
> memory fragments in specific case. Also 3 pages must contain a
> 8kb-aligned 8kb range, which meets the requirement. If the limit
> of allocated 3 pages is 8KB-aligned, then the last 8K range will
2. What's "last 8k"?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105010): https://edk2.groups.io/g/devel/message/105010
Mute This Topic: https://groups.io/mt/98986400/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] MdePkg: Code optimization to SMM InternalAllocateAlignedPages

2023-05-18 Thread duntan

This commit is code optimization to InternalAllocateAlignedPages of
SmmMemoryAllocationLib which can reduce free memory fragments. Also
it can reduce one pre-allocation page.

Let's take a simple example:
The expected pages size is 8KB, Alignment value is 8KB.

In original InternalAllocateAlignedPages(), the first step is to
allocate 4 pages and then find the first 8KB-aligned address in the
allocated 4 pages. If the limit address is already 8KB aligned, then
the allocated 4 pages contains two 8KB-aligned 8KB ranges. The lower
2 pages will be selected and removed from free pages. Then the higher
2 pages will be free. Since the whole memory allocation is from high
address to lower address, then the higher 2 pages cann't be merged
other free pages, causing the free memory fragments.

However, when only allocate 3(2+2-1) pages, we can avoid the free
memory fragments in specific case. Also 3 pages must contain a
8kb-aligned 8kb range, which meets the requirement. If the limit
of allocated 3 pages is 8KB-aligned, then the last 8K range will
be selected and removed from free pages. The lower 4Kb will be
free and merged with left lower free memory. This can reduce free
memory fragments in smm.

Signed-off-by: Dun Tan 
Cc: Ray Ni 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
---
 MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c 
b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
index 3ab2c1ebfd..99a8259325 100644
--- a/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
+++ b/MdePkg/Library/SmmMemoryAllocationLib/MemoryAllocationLib.c
@@ -322,7 +322,7 @@ InternalAllocateAlignedPages (
 // Calculate the total number of pages since alignment is larger than page 
size.
 //
 AlignmentMask = Alignment - 1;
-RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
+RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment) - 1;
 //
 // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not 
overflow.
 //
-- 
2.31.1.windows.1



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

61 matches

Mail list logo