[edk2-devel] [PATCH] OvmfPkg: RiscVVirt: Fix wrong checking Pci IO access

2023-06-07 Thread Tuan Phan
RiscV uses memory access for IO and MMIO resources, the address limit
is MAX_ADDRESS for both of them.

Signed-off-by: Tuan Phan 
---
 OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.c 
b/OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.c
index f3bf07e63141..75389235d897 100644
--- a/OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.c
+++ b/OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.c
@@ -19,8 +19,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
-#define MAX_IO_PORT_ADDRESS  0x
-
 //
 // Handle for the CPU I/O 2 Protocol
 //
@@ -143,16 +141,16 @@ CpuIoCheckParameter (
   // Address + Size * Count.  If the following condition is met, then the 
transfer
   // is not supported.
   //
-  //Address + Size * Count > (MmioOperation ? MAX_ADDRESS : 
MAX_IO_PORT_ADDRESS) + 1
+  //Address + Size * Count > MAX_ADDRESS + 1
   //
   // Since MAX_ADDRESS can be the maximum integer value supported by the CPU 
and Count
   // can also be the maximum integer value supported by the CPU, this range
   // check must be adjusted to avoid all overflow conditions.
   //
   // The following form of the range check is equivalent but assumes that
-  // MAX_ADDRESS and MAX_IO_PORT_ADDRESS are of the form (2^n - 1).
+  // MAX_ADDRESS is of the form (2^n - 1).
   //
-  Limit = (MmioOperation ? MAX_ADDRESS : MAX_IO_PORT_ADDRESS);
+  Limit = MAX_ADDRESS;
   if (Count == 0) {
 if (Address > Limit) {
   return EFI_UNSUPPORTED;
-- 
2.25.1



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




Re: [edk2-devel] [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Introduce Hii2RedfishBootDxe driver

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

Same as the comment given to 1/3, let's rename Hii2 to HiiTo. The same comment 
applied to 3/3.

Thanks
Abner

> -Original Message-
> From: Nickle Wang 
> Sent: Tuesday, June 6, 2023 9:38 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> 
> Subject: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Introduce
> Hii2RedfishBootDxe driver
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Add Hii2RedfishBootDxe driver with configure language defined in UNI
> file in order to demonstrate the use of Redfish Platform Config
> Protocol. Feature drivers under RedfishClientPkg will work with this
> driver and provide the REST data to Redfish service.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> ---
>  RedfishClientPkg/RedfishClientPkg.dec |   1 +
>  .../RedfishClientComponents.dsc.inc   |   1 +
>  RedfishClientPkg/RedfishClientPkg.dsc |   9 +
>  .../Hii2RedfishBootDxe/Hii2RedfishBootDxe.inf |  59 ++
>  .../Hii2RedfishBootDxe/Hii2RedfishBootData.h  |  60 ++
>  .../Hii2RedfishBootDxe/Hii2RedfishBootDxe.h   |  53 ++
>  .../Hii2RedfishBootDxe/Hii2RedfishBootVfr.vfr |  83 +++
>  .../Hii2RedfishBootDxe/Hii2RedfishBootDxe.c   | 698 ++
>  .../Hii2RedfishBootDxeMap.uni |  28 +
>  .../Hii2RedfishBootDxeStrings.uni |  41 +
>  RedfishClientPkg/RedfishClient.fdf.inc|   1 +
>  11 files changed, 1034 insertions(+)
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.inf
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootData.h
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.h
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootVfr.vfr
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.c
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxeMap.uni
>  create mode 100644
> RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxeStrings.uni
>
> diff --git a/RedfishClientPkg/RedfishClientPkg.dec
> b/RedfishClientPkg/RedfishClientPkg.dec
> index af241cf6..055c1924 100644
> --- a/RedfishClientPkg/RedfishClientPkg.dec
> +++ b/RedfishClientPkg/RedfishClientPkg.dec
> @@ -58,6 +58,7 @@
>gEfiRedfishClientVariableGuid   = { 0x91c46a3d, 0xed1a, 
> 0x477b,
> { 0xa5, 0x33, 0x87, 0x2d, 0xcd, 0xb0, 0xfc, 0xc1 } }
>
>gHii2RedfishMemoryFormsetGuid   = { 0XC2BE579E, 0X3C57,
> 0X499C, { 0XA9, 0XDF, 0XE6, 0X23, 0X8A, 0X49, 0X64, 0XF8 }}
> +  gHii2RedfishBootFormsetGuid = { 0x8399a787, 0x108e, 
> 0x4e53,
> { 0x9e, 0xde, 0x4b, 0x18, 0xcc, 0x9e, 0xab, 0x3b }}
>
>  [PcdsFixedAtBuild]
>
> gEfiRedfishClientPkgTokenSpaceGuid.PcdMaxRedfishSchemaStringSize|32|UI
> NT32|0x1001
> diff --git a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> index 3451c185..4633e962 100644
> --- a/RedfishClientPkg/RedfishClientComponents.dsc.inc
> +++ b/RedfishClientPkg/RedfishClientComponents.dsc.inc
> @@ -18,6 +18,7 @@
>RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.inf
>RedfishClientPkg/RedfishConfigLangMapDxe/RedfishConfigLangMapDxe.inf
>RedfishClientPkg/Hii2RedfishMemoryDxe/Hii2RedfishMemoryDxe.inf
> +  RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.inf
>  !endif
>#
># Below two modules should be pulled in by build tool.
> diff --git a/RedfishClientPkg/RedfishClientPkg.dsc
> b/RedfishClientPkg/RedfishClientPkg.dsc
> index ac9f8e9d..edc387ac 100644
> --- a/RedfishClientPkg/RedfishClientPkg.dsc
> +++ b/RedfishClientPkg/RedfishClientPkg.dsc
> @@ -39,6 +39,15 @@
>BaseSortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf
>HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
>
> UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesL
> ib.inf
> +
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBoot
> ManagerLib.inf
> +  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> +
> PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLi
> bNull.inf
> +
> PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePe
> CoffGetEntryPointLib.inf
> +
> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLi
> b.inf
> +  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeR
> eportStatusCodeLib.inf
> +  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Varia
> blePolicyHelperLib.inf
>
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>#
> diff --git a/RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.inf
> b/RedfishClientPkg/Hii2RedfishBootDxe/Hii2RedfishBootDxe.inf
> new file mo

Re: [edk2-devel] [PATCH] Maintainers.txt: Update maintainers for StandaloneMmPkg

2023-06-07 Thread Sami Mujawar
Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar

On 07/06/2023, 04:20, "Ray Ni" mailto:ray...@intel.com>> 
wrote:


Signed-off-by: Ray Ni mailto:ray...@intel.com>>
Cc: Ard Biesheuvel mailto:ardb+tianoc...@kernel.org>>
Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Supreeth Venkatesh mailto:supreeth.venkat...@arm.com>>
---
Maintainers.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/Maintainers.txt b/Maintainers.txt
index 09d04af27a..9c54800f2f 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -617,7 +617,7 @@ StandaloneMmPkg
F: StandaloneMmPkg/


M: Ard Biesheuvel mailto:ardb+tianoc...@kernel.org>> [ardbiesheuvel]


M: Sami Mujawar mailto:sami.muja...@arm.com>> 
[samimujawar]


-M: Jiewen Yao mailto:jiewen@intel.com>> [jyao1]


+M: Ray Ni mailto:ray...@intel.com>> [niruiyu]


R: Supreeth Venkatesh mailto:supreeth.venkat...@arm.com>> [supven01]






UefiCpuPkg


-- 
2.37.2.windows.2







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




Re: [edk2-devel] [PATCH] Maintainers.txt: Update maintainers for StandaloneMmPkg

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 09:40, Sami Mujawar  wrote:
>
> Reviewed-by: Sami Mujawar 
>

Thanks, I'll pick this up.

While we're at it, mind sending a patch to remove Supreeth?


>
> On 07/06/2023, 04:20, "Ray Ni" mailto:ray...@intel.com>> 
> wrote:
>
>
> Signed-off-by: Ray Ni mailto:ray...@intel.com>>
> Cc: Ard Biesheuvel  >
> Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
> Cc: Jiewen Yao mailto:jiewen@intel.com>>
> Cc: Supreeth Venkatesh  >
> ---
> Maintainers.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 09d04af27a..9c54800f2f 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -617,7 +617,7 @@ StandaloneMmPkg
> F: StandaloneMmPkg/
>
>
> M: Ard Biesheuvel  > [ardbiesheuvel]
>
>
> M: Sami Mujawar mailto:sami.muja...@arm.com>> 
> [samimujawar]
>
>
> -M: Jiewen Yao mailto:jiewen@intel.com>> [jyao1]
>
>
> +M: Ray Ni mailto:ray...@intel.com>> [niruiyu]
>
>
> R: Supreeth Venkatesh  > [supven01]
>
>
>
>
>
>
> UefiCpuPkg
>
>
> --
> 2.37.2.windows.2
>
>
>
>
>
>
>
> 
>
>


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




[edk2-devel] [PATCH] UefiCpuPkg/SmmCpu: Add PcdSmmApPerfLogEnable control AP perf-logging

2023-06-07 Thread Ni, Ray
When a platform has lots of CPU cores/threads, perf-logging on every
AP produces lots of records. When this multiplies with number of SMIs
during post, the records are even more.

So, this patch adds a new PCD PcdSmmApPerfLogEnable (default TRUE)
to allow platform to turn off perf-logging on APs.

Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c|  2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c| 11 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h|  4 +++-
 UefiCpuPkg/UefiCpuPkg.dec|  6 ++
 5 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index bcd90f0671..8364b73242 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -784,7 +784,7 @@ BSPHandler (
   // Any SMM MP performance logging after this point will be migrated in next 
SMI.
   //
   PERF_CODE (
-MigrateMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
+MigrateMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus, CpuIndex);
 );
 
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 4864532c35..d864ae9101 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -128,6 +128,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmApPerfLogEnable  ## CONSUMES
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
index c13556af46..92c67f31bf 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
@@ -39,16 +39,25 @@ InitializeMpPerf (
   Migrate MP performance data to standardized performance database.
 
   @param NumberofCpusNumber of processors in the platform.
+  @param BspIndexThe index of the BSP.
 **/
 VOID
 MigrateMpPerf (
-  UINTN  NumberofCpus
+  UINTN  NumberofCpus,
+  UINTN  BspIndex
   )
 {
   UINTN  CpuIndex;
   UINTN  MpProcecureId;
 
   for (CpuIndex = 0; CpuIndex < NumberofCpus; CpuIndex++) {
+if ((CpuIndex != BspIndex) && !FeaturePcdGet (PcdSmmApPerfLogEnable)) {
+  //
+  // Skip migrating AP performance data if AP perf-logging is disabled.
+  //
+  continue;
+}
+
 for (MpProcecureId = 0; MpProcecureId < SMM_MP_PERF_PROCEDURE_ID 
(SmmMpProcedureMax); MpProcecureId++) {
   if (mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId] != 0) {
 PERF_START (NULL, gSmmMpPerfProcedureName[MpProcecureId], NULL, 
mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId]);
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
index b148a99e86..5ad1734cc8 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
@@ -44,10 +44,12 @@ InitializeMpPerf (
   Migrate MP performance data to standardized performance database.
 
   @param NumberofCpusNumber of processors in the platform.
+  @param BspIndexThe index of the BSP.
 **/
 VOID
 MigrateMpPerf (
-  UINTN  NumberofCpus
+  UINTN  NumberofCpus,
+  UINTN  BspIndex
   );
 
 /**
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index d31c3b127c..5b0ac64e33 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -175,6 +175,12 @@
   # @Prompt Support SmmFeatureControl.
   gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable|TRUE|BOOLEAN|0x32132110
 
+  ## Indicates if SMM perf logging in APs will be enabled.
+  #   TRUE  - SMM perf logging in APs will be enabled.
+  #   FALSE - SMM perf logging in APs will not be enabled.
+  # @Prompt Enable SMM perf logging in APs.
+  gUefiCpuPkgTokenSpaceGuid.PcdSmmApPerfLogEnable|TRUE|BOOLEAN|0x32132114
+
 [PcdsFixedAtBuild]
   ## List of exception vectors which need switching stack.
   #  This PCD will only take into effect if PcdCpuStackGuard is enabled.
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105852): https://edk2.groups.io/g/devel/message/105852
Mute This Topic: https://groups.io/mt/99380509/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] UefiCpuPkg/SmmCpu: Add PcdSmmApPerfLogEnable control AP perf-logging

2023-06-07 Thread Ni, Ray
This patch is based on patch series: 
https://edk2.groups.io/g/devel/message/105492.
I don't want to send a V3 for just including the additional patch.

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Wednesday, June 7, 2023 4:38 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Kumar, Rahul R
> ; Gerd Hoffmann 
> Subject: [edk2-devel] [PATCH] UefiCpuPkg/SmmCpu: Add
> PcdSmmApPerfLogEnable control AP perf-logging
> 
> When a platform has lots of CPU cores/threads, perf-logging on every
> AP produces lots of records. When this multiplies with number of SMIs
> during post, the records are even more.
> 
> So, this patch adds a new PCD PcdSmmApPerfLogEnable (default TRUE)
> to allow platform to turn off perf-logging on APs.
> 
> Cc: Eric Dong 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c|  2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c| 11 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h|  4 +++-
>  UefiCpuPkg/UefiCpuPkg.dec|  6 ++
>  5 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index bcd90f0671..8364b73242 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -784,7 +784,7 @@ BSPHandler (
>// Any SMM MP performance logging after this point will be migrated in next
> SMI.
> 
>//
> 
>PERF_CODE (
> 
> -MigrateMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
> 
> +MigrateMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus,
> CpuIndex);
> 
>  );
> 
> 
> 
>//
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index 4864532c35..d864ae9101 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -128,6 +128,7 @@
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ##
> CONSUMES
> 
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ##
> CONSUMES
> 
>gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode ##
> CONSUMES
> 
> +  gUefiCpuPkgTokenSpaceGuid.PcdSmmApPerfLogEnable  ##
> CONSUMES
> 
> 
> 
>  [Pcd]
> 
>gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber##
> SOMETIMES_CONSUMES
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
> index c13556af46..92c67f31bf 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
> @@ -39,16 +39,25 @@ InitializeMpPerf (
>Migrate MP performance data to standardized performance database.
> 
> 
> 
>@param NumberofCpusNumber of processors in the platform.
> 
> +  @param BspIndexThe index of the BSP.
> 
>  **/
> 
>  VOID
> 
>  MigrateMpPerf (
> 
> -  UINTN  NumberofCpus
> 
> +  UINTN  NumberofCpus,
> 
> +  UINTN  BspIndex
> 
>)
> 
>  {
> 
>UINTN  CpuIndex;
> 
>UINTN  MpProcecureId;
> 
> 
> 
>for (CpuIndex = 0; CpuIndex < NumberofCpus; CpuIndex++) {
> 
> +if ((CpuIndex != BspIndex) && !FeaturePcdGet (PcdSmmApPerfLogEnable)) {
> 
> +  //
> 
> +  // Skip migrating AP performance data if AP perf-logging is disabled.
> 
> +  //
> 
> +  continue;
> 
> +}
> 
> +
> 
>  for (MpProcecureId = 0; MpProcecureId < SMM_MP_PERF_PROCEDURE_ID
> (SmmMpProcedureMax); MpProcecureId++) {
> 
>if (mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId] != 0) {
> 
>  PERF_START (NULL, gSmmMpPerfProcedureName[MpProcecureId], NULL,
> mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId]);
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
> index b148a99e86..5ad1734cc8 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
> @@ -44,10 +44,12 @@ InitializeMpPerf (
>Migrate MP performance data to standardized performance database.
> 
> 
> 
>@param NumberofCpusNumber of processors in the platform.
> 
> +  @param BspIndexThe index of the BSP.
> 
>  **/
> 
>  VOID
> 
>  MigrateMpPerf (
> 
> -  UINTN  NumberofCpus
> 
> +  UINTN  NumberofCpus,
> 
> +  UINTN  BspIndex
> 
>);
> 
> 
> 
>  /**
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index d31c3b127c..5b0ac64e33 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -175,6 +175,12 @@
># @Prompt Support SmmFeatureControl.
> 
> 
> gUefiCpuPkgTokenSpaceGuid.PcdSmmFeatureControlEnable|TRUE|BOOLEAN|0x
> 32132110
> 
> 
> 
> +  ## Indicates if SMM perf logging in APs will be enabled.
> 
> +  #   TRUE  - SMM perf logging in APs will be enabled.
> 
> +  #   FALSE - SMM perf logging in APs will not be enabled.
> 
> +  # @Prompt Enable SMM perf logging in 

Re: [edk2-devel] [PATCH] Maintainers.txt: Remove UEFI Shell Binaries section

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 05:26, Ni, Ray  wrote:
>
> The Shell binaries are not generated anymore in each
> stable tag release.
> So, remove the section.
>
> Cc: Zhichao Gao 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Signed-off-by: Ray Ni 

Reviewed-by: Ard Biesheuvel 

> ---
>  Maintainers.txt | 8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 9c54800f2f..5209e21449 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -81,14 +81,6 @@ EDK II Releases:
>  W: 
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
>  M: Liming Gao  [lgao4]
>
> -UEFI Shell Binaries (ShellBinPkg.zip) from EDK II Releases:
> 
> -W: https://github.com/tianocore/edk2/releases/
> -M: Ray Ni  [niruiyu](Ia32/X64)
> -M: Zhichao Gao  [ZhichaoGao]   (Ia32/X64)
> -M: Leif Lindholm  [leiflindholm]   
> (ARM/AArch64)
> -M: Ard Biesheuvel  [ardbiesheuvel] (ARM/AArch64)
> -
>  EDK II Architectures:
>  -
>  ARM, AARCH64
> --
> 2.37.2.windows.2
>
>
>
> 
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#105842): https://edk2.groups.io/g/devel/message/105842
> Mute This Topic: https://groups.io/mt/99378199/1131722
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: 
> https://edk2.groups.io/g/devel/leave/9927449/1131722/1583048064/xyzzy 
> [a...@kernel.org]
> 
>
>


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




[edk2-devel] [PATCH v1 1/1] Maintainers.txt: Remove reviewer for StandaloneMmPkg

2023-06-07 Thread Sami Mujawar
Supreeth is no longer supreeth.venkat...@arm.com. Therefore,
remove the reviewer entry from StandaloneMmPkg.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Ray Ni 

Signed-off-by: Sami Mujawar 
---
 Maintainers.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 
97886accd5c440069f4fe3763138e38a5b30b5ef..7311afbfa20d7272f093162d25955457e1eaf376
 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -644,7 +644,6 @@ F: StandaloneMmPkg/
 M: Ard Biesheuvel  [ardbiesheuvel]
 M: Sami Mujawar  [samimujawar]
 M: Jiewen Yao  [jyao1]
-R: Supreeth Venkatesh  [supven01]
 
 UefiCpuPkg
 F: UefiCpuPkg/
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



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




Re: [edk2-devel] [PATCH] Maintainers.txt: Update maintainers for StandaloneMmPkg

2023-06-07 Thread Sami Mujawar
Hi Ard,

On Wed, Jun 7, 2023 at 01:29 AM, Ard Biesheuvel wrote:

> 
> On Wed, 7 Jun 2023 at 09:40, Sami Mujawar  wrote:
> 
>> Reviewed-by: Sami Mujawar 
> 
> Thanks, I'll pick this up.
> 
> While we're at it, mind sending a patch to remove Supreeth?

I have submitted a patch to update the reviewer list for StandaloneMmPkg at 
https://edk2.groups.io/g/devel/message/105855

Regards,

Sami Mujawar


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105856): https://edk2.groups.io/g/devel/message/105856
Mute This Topic: https://groups.io/mt/99378152/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 1/1] Maintainers.txt: Remove reviewer for StandaloneMmPkg

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 12:32, Sami Mujawar  wrote:
>
> Supreeth is no longer supreeth.venkat...@arm.com. Therefore,
> remove the reviewer entry from StandaloneMmPkg.
>
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Ray Ni 
>
> Signed-off-by: Sami Mujawar 

Thanks, I'll merge this along with the other one.

> ---
>  Maintainers.txt | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 
> 97886accd5c440069f4fe3763138e38a5b30b5ef..7311afbfa20d7272f093162d25955457e1eaf376
>  100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -644,7 +644,6 @@ F: StandaloneMmPkg/
>  M: Ard Biesheuvel  [ardbiesheuvel]
>  M: Sami Mujawar  [samimujawar]
>  M: Jiewen Yao  [jyao1]
> -R: Supreeth Venkatesh  [supven01]
>
>  UefiCpuPkg
>  F: UefiCpuPkg/
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
> 
>
>


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

2023-06-07 Thread Zhang, Hongbin1
Thanks Ray.
Replied your code comments with [Hongbin].
And I will split to several patches, it will be better for reviewing.

-Original Message-
From: Ni, Ray  
Sent: Wednesday, June 7, 2023 10:04 AM
To: Zhang, Hongbin1 ; devel@edk2.groups.io; Yao, 
Jiewen 
Cc: Zeng, Star ; Wu, Jiaxin ; Sami 
Mujawar ; Ard Biesheuvel ; 
Supreeth Venkatesh 
Subject: RE: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.

Hongbin,
The patch is so big. Can you please split the patch to small patches? For 
example:
1st patch only implements the Ipl entrypoint to find the correct SMRAM range 
and dump it
2nd patch loads SMM CORE without mode switch if sizeof (UINTN) matches SMM core 
module type.
3rd patch loads SMM CORE with mode switch if sizeof (UINTN) == 4 but SMM core 
module type is 64bit.
4th patch Deadloop() when sizeof (UINTN) == 8 but SMM core module type is 32bit.
5th patch register ReadyToBoot callback. But is it too late from security 
perspective? Why not lock after SMM core exit? @Yao, Jiewen

Back to the code content, 3 comments:
1. Have you tested the flow when LoadImageAtFixedAddress is enabled? If no, I 
prefer you do not add the logic at all in the first version. We can enable that 
later.
 -- Yes, currently PcdLoadModuleAtFixAddressEnable was not used and 
verified, I will add later [Hongbin].
2. "EFIAPI" for GetSectionFromAnyFvByFileType should be removed. And the 
function prototype could be refined as below:
EFI_STATUS
LocateMmFvAndCore
  OUT EFI_PHYSICAL_ADDRESS  *MmFvBaseAddress,
  OUT VOID  **MmCoreImageAddress
Please avoid directly modifying global variables inside this function.
 -- Ok, will change that [Hongbin].
3. Can the StandaloneMmCore in open source be directly used with this IPL? 
 -- I will think about this later [Hongbin].

Thanks,
Ray


> -Original Message-
> From: Zhang, Hongbin1 
> Sent: Monday, May 22, 2023 1:21 PM
> To: devel@edk2.groups.io
> Cc: Zhang, Hongbin1 ; Yao, Jiewen
> ; Ni, Ray ; Zeng, Star
> ; Wu, Jiaxin ; Sami Mujawar
> ; Ard Biesheuvel ;
> Supreeth Venkatesh 
> Subject: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.
> 
> Add StandaloneMmIplPei IA32/X64 driver at PEI stage.
> FSP will use this driver to load Standalone MM code
> to dispatch other Standalone MM drivers.
> 
> Signed-off-by: Hongbin1 Zhang 
> Cc: Jiewen Yao 
> Cc: Ray Ni 
> Cc: Star Zeng 
> Cc: Jiaxin Wu 
> Cc: Sami Mujawar 
> Cc: Ard Biesheuvel 
> Cc: Supreeth Venkatesh 
> ---
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c | 456
> 
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c   | 787
> 
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/X64/LoadSmmCore.c  |  32 +
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/Thunk32To64.nasm  | 148
> 
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.h   |  66
> ++
>  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.inf |  75
> ++
>  StandaloneMmPkg/StandaloneMmPkg.ci.yaml   |   4 +-
>  StandaloneMmPkg/StandaloneMmPkg.dsc   |  15 +-
>  UefiPayloadPkg/UniversalPayloadBuild.sh   |  34 +-
>  edksetup.sh   | 294 
> 
>  10 files changed, 1744 insertions(+), 167 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> new file mode 100644
> index 00..d6174d73a3
> --- /dev/null
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> @@ -0,0 +1,456 @@
> +/** @file
> +  SMM IPL that load the SMM Core into SMRAM
> +
> +  Copyright (c) 2023, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#pragma pack(1)
> +
> +//
> +// Page-Map Level-4 Offset (PML4) and
> +// Page-Directory-Pointer Offset (PDPE) entries 4K & 2MB
> +//
> +
> +typedef union {
> +  struct {
> +UINT64Present  : 1;   // 0 = Not present in memory, 1 = 
> Present in
> memory
> +UINT64ReadWrite: 1;   // 0 = Read-Only, 1= Read/Write
> +UINT64UserSupervisor   : 1;   // 0 = Supervisor, 1=User
> +UINT64WriteThrough : 1;   // 0 = Write-Back caching, 
> 1=Write-Through
> caching
> +UINT64CacheDisabled: 1;   // 0 = Cached, 1=Non-Cached
> +UINT64Accessed : 1;   // 0 = Not accessed, 1 = Accessed 
> (set by CPU)
> +UINT64Reserved : 1;   // Reserved
> +UINT64MustBeZero   : 2;   // Must Be Zero
> +UINT64Available: 3;   // Available for use by system 
> software
> +UINT64PageTableBaseAddress : 40;  // Page Table Base Address
> +UINT64AvailableHigh 

Re: [edk2-devel] [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.

2023-06-07 Thread Zhang, Hongbin1
Thanks Jiewen.
I will change this.

-Original Message-
From: Yao, Jiewen  
Sent: Wednesday, June 7, 2023 10:31 AM
To: Zhang, Hongbin1 ; Ni, Ray ; 
devel@edk2.groups.io
Cc: Zeng, Star ; Wu, Jiaxin ; Sami 
Mujawar ; Ard Biesheuvel ; 
Supreeth Venkatesh ; Bu, Daocheng 
; Chen, Bo 
Subject: RE: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.

> 5th patch register ReadyToBoot callback. But is it too late from security 
> perspective? Why not lock after SMM core exit? @Yao, Jiewen

Agree with Ray. ReadyToBoot is too late.



> -Original Message-
> From: Zhang, Hongbin1 
> Sent: Wednesday, June 7, 2023 10:29 AM
> To: Ni, Ray ; devel@edk2.groups.io; Yao, Jiewen
> 
> Cc: Zeng, Star ; Wu, Jiaxin ; Sami
> Mujawar ; Ard Biesheuvel
> ; Supreeth Venkatesh
> ; Bu, Daocheng ;
> Chen, Bo 
> Subject: RE: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.
> 
> Thanks Ray.
> Replied your code comments with [Hongbin].
> And I will split to several patches, it will be better for reviewing.
> 
> -Original Message-
> From: Ni, Ray 
> Sent: Wednesday, June 7, 2023 10:04 AM
> To: Zhang, Hongbin1 ; devel@edk2.groups.io; Yao,
> Jiewen 
> Cc: Zeng, Star ; Wu, Jiaxin ; Sami
> Mujawar ; Ard Biesheuvel
> ; Supreeth Venkatesh
> 
> Subject: RE: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.
> 
> Hongbin,
> The patch is so big. Can you please split the patch to small patches? For 
> example:
> 1st patch only implements the Ipl entrypoint to find the correct SMRAM range
> and dump it
> 2nd patch loads SMM CORE without mode switch if sizeof (UINTN) matches
> SMM core module type.
> 3rd patch loads SMM CORE with mode switch if sizeof (UINTN) == 4 but SMM
> core module type is 64bit.
> 4th patch Deadloop() when sizeof (UINTN) == 8 but SMM core module type is
> 32bit.
> 5th patch register ReadyToBoot callback. But is it too late from security
> perspective? Why not lock after SMM core exit? @Yao, Jiewen
> 
> Back to the code content, 3 comments:
> 1. Have you tested the flow when LoadImageAtFixedAddress is enabled? If no, I
> prefer you do not add the logic at all in the first version. We can enable 
> that
> later.
>  -- Yes, currently PcdLoadModuleAtFixAddressEnable was not used and
> verified, I will add later [Hongbin].
> 2. "EFIAPI" for GetSectionFromAnyFvByFileType should be removed. And the
> function prototype could be refined as below:
> EFI_STATUS
> LocateMmFvAndCore
>   OUT EFI_PHYSICAL_ADDRESS  *MmFvBaseAddress,
>   OUT VOID  **MmCoreImageAddress
> Please avoid directly modifying global variables inside this function.
>  -- Ok, will change that [Hongbin].
> 3. Can the StandaloneMmCore in open source be directly used with this IPL?
>  -- I will think about this later [Hongbin].
> 
> Thanks,
> Ray
> 
> 
> > -Original Message-
> > From: Zhang, Hongbin1 
> > Sent: Monday, May 22, 2023 1:21 PM
> > To: devel@edk2.groups.io
> > Cc: Zhang, Hongbin1 ; Yao, Jiewen
> > ; Ni, Ray ; Zeng, Star
> > ; Wu, Jiaxin ; Sami Mujawar
> > ; Ard Biesheuvel ;
> > Supreeth Venkatesh 
> > Subject: [PATCH v1] StandaloneMmPkg: Add StandaloneMmIplPei driver.
> >
> > Add StandaloneMmIplPei IA32/X64 driver at PEI stage.
> > FSP will use this driver to load Standalone MM code
> > to dispatch other Standalone MM drivers.
> >
> > Signed-off-by: Hongbin1 Zhang 
> > Cc: Jiewen Yao 
> > Cc: Ray Ni 
> > Cc: Star Zeng 
> > Cc: Jiaxin Wu 
> > Cc: Sami Mujawar 
> > Cc: Ard Biesheuvel 
> > Cc: Supreeth Venkatesh 
> > ---
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c |
> 456
> > 
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.c   |
> 787
> > 
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/X64/LoadSmmCore.c  |
> 32 +
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/Thunk32To64.nasm  |
> 148
> > 
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.h   |  66
> > ++
> >  StandaloneMmPkg/Drivers/StandaloneMmIplPei/StandaloneMmIplPei.inf |
> 75
> > ++
> >  StandaloneMmPkg/StandaloneMmPkg.ci.yaml   |   4 +-
> >  StandaloneMmPkg/StandaloneMmPkg.dsc   |  15 +-
> >  UefiPayloadPkg/UniversalPayloadBuild.sh   |  34 +-
> >  edksetup.sh   | 294 
> > 
> >  10 files changed, 1744 insertions(+), 167 deletions(-)
> >
> > diff --git
> > a/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> > b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> > new file mode 100644
> > index 00..d6174d73a3
> > --- /dev/null
> > +++ b/StandaloneMmPkg/Drivers/StandaloneMmIplPei/Ia32/LoadSmmCore.c
> > @@ -0,0 +1,456 @@
> > +/** @file
> > +  SMM IPL that load the SMM Core into SMRAM
> > +
> > +  Copyright (c) 2023, Intel Corporation. All rights reserved.
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#inclu

[edk2-devel] [PATCH v2 0/1] bhyve: tpm support

2023-06-07 Thread Corvin Köhne
Hi,

I'm working on TPM emulation for bhyve and would like to merge this
patchset to edk2.

As already send this patch to edk2 by mistake, I'm now resending this
patch as v2.

I've already created a PR to run all CI checks at:
https://github.com/tianocore/edk2/pull/4531

Thanks,
Corvin

Corvin Köhne (1):
  OvmfPkg/Bhyve: include TPM driver

 OvmfPkg/Bhyve/BhyveX64.dsc | 17 +++--
 OvmfPkg/Bhyve/BhyveX64.fdf |  7 +++
 2 files changed, 22 insertions(+), 2 deletions(-)

-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105860): https://edk2.groups.io/g/devel/message/105860
Mute This Topic: https://groups.io/mt/99384217/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/1] OvmfPkg/Bhyve: include TPM driver

2023-06-07 Thread Corvin Köhne
From: Corvin Köhne 

Bhyve will gain support for TPM emulation in the near future. Therefore,
prepare OVMF by copying all TPM driver used by qemu's OVMF DSC into the
bhyve OVMF DSC.

Signed-off-by: Corvin Köhne 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
Cc: Peter Grehan 
---
 OvmfPkg/Bhyve/BhyveX64.dsc | 17 +++--
 OvmfPkg/Bhyve/BhyveX64.fdf |  7 +++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 7b974706f958..7fa40998ae80 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -32,6 +32,8 @@ [Defines]
   DEFINE SMM_REQUIRE = FALSE
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
+!include OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc
+
   #
   # Network definition
   #
@@ -226,8 +228,7 @@ [LibraryClasses]
   
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
   XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
 
-  
Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
-  
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+!include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -564,12 +565,17 @@ [PcdsDynamicDefault]
 
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
 
+!include OvmfPkg/Include/Dsc/OvmfTpmPcds.dsc.inc
+
   # MdeModulePkg resolution sets up the system display resolution
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|0
 
+[PcdsDynamicHii]
+!include OvmfPkg/Include/Dsc/OvmfTpmPcdsHii.dsc.inc
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform.
@@ -609,6 +615,8 @@ [Components]
 
   }
 
+!include OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc
+
   #
   # DXE Phase modules
   #
@@ -632,6 +640,7 @@ [Components]
 !if $(SECURE_BOOT_ENABLE) == TRUE
   
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
 !endif
+!include OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
@@ -826,3 +835,7 @@ [Components]
   NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
   }
 
+  #
+  # TPM support
+  #
+!include OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc
diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf
index 3f6270c048cc..c62d5757092e 100644
--- a/OvmfPkg/Bhyve/BhyveX64.fdf
+++ b/OvmfPkg/Bhyve/BhyveX64.fdf
@@ -158,6 +158,8 @@ [FV.PEIFV]
 INF  OvmfPkg/Bhyve/SmmAccess/SmmAccessPei.inf
 !endif
 
+!include OvmfPkg/Include/Fdf/OvmfTpmPei.fdf.inc
+
 

 
 [FV.DXEFV]
@@ -335,6 +337,11 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 !endif
 
+#
+# TPM support
+#
+!include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
+
 

 
 [FV.FVMAIN_COMPACT]
-- 
2.40.1



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




[edk2-devel] Event: UEFI Memory Map, GCD, Page Table discussion - ARM/X86 (2nd session) - Wednesday, June 7, 2023 #cal-reminder

2023-06-07 Thread Group Notification
*Reminder: UEFI Memory Map, GCD, Page Table discussion - ARM/X86 (2nd session)*

*When:*
Wednesday, June 7, 2023
10:00pm to 11:00pm
(UTC+08:00) Asia/Shanghai

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_M2RjMDI1NjEtMmQ3MS00ZWM0LTlkMzEtOWU1NGE2MGUwYmYx%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%2255d36a50-78be-4ced-bc27-3d06c576cc19%22%7d

*Organizer:* Ray Ni ray...@intel.com ( 
ray...@intel.com?subject=Re:%20Event:%20UEFI%20Memory%20Map%2C%20GCD%2C%20Page%20Table%20discussion%20-%20ARM%2FX86%20%282nd%20session%29
 )

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

*Description:*


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




[edk2-devel] Now: UEFI Memory Map, GCD, Page Table discussion - ARM/X86 (2nd session) - Wednesday, June 7, 2023 #cal-notice

2023-06-07 Thread Group Notification
*UEFI Memory Map, GCD, Page Table discussion - ARM/X86 (2nd session)*

*When:*
Wednesday, June 7, 2023
10:00pm to 11:00pm
(UTC+08:00) Asia/Shanghai

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_M2RjMDI1NjEtMmQ3MS00ZWM0LTlkMzEtOWU1NGE2MGUwYmYx%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%2255d36a50-78be-4ced-bc27-3d06c576cc19%22%7d

*Organizer:* Ray Ni ray...@intel.com ( 
ray...@intel.com?subject=Re:%20Event:%20UEFI%20Memory%20Map%2C%20GCD%2C%20Page%20Table%20discussion%20-%20ARM%2FX86%20%282nd%20session%29
 )

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

*Description:*


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




Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/Bhyve: include TPM driver

2023-06-07 Thread Rebecca Cran

Reviewed-by: Rebecca Cran 


--

Rebecca Cran


On 6/7/23 07:17, Corvin Köhne wrote:

From: Corvin Köhne 

Bhyve will gain support for TPM emulation in the near future. Therefore,
prepare OVMF by copying all TPM driver used by qemu's OVMF DSC into the
bhyve OVMF DSC.

Signed-off-by: Corvin Köhne 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
Cc: Peter Grehan 
---
  OvmfPkg/Bhyve/BhyveX64.dsc | 17 +++--
  OvmfPkg/Bhyve/BhyveX64.fdf |  7 +++
  2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 7b974706f958..7fa40998ae80 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -32,6 +32,8 @@ [Defines]
DEFINE SMM_REQUIRE = FALSE
DEFINE SOURCE_DEBUG_ENABLE = FALSE
  
+!include OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc

+
#
# Network definition
#
@@ -226,8 +228,7 @@ [LibraryClasses]

OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
  
-  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf

-  
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+!include OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
  
  [LibraryClasses.common]

BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -564,12 +565,17 @@ [PcdsDynamicDefault]
  
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
  
+!include OvmfPkg/Include/Dsc/OvmfTpmPcds.dsc.inc

+
# MdeModulePkg resolution sets up the system display resolution
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|0
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|0
  
+[PcdsDynamicHii]

+!include OvmfPkg/Include/Dsc/OvmfTpmPcdsHii.dsc.inc
+
  

  #
  # Components Section - list of all EDK II Modules needed by this Platform.
@@ -609,6 +615,8 @@ [Components]
  
}
  
+!include OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc

+
#
# DXE Phase modules
#
@@ -632,6 +640,7 @@ [Components]
  !if $(SECURE_BOOT_ENABLE) == TRUE

NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
  !endif
+!include OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc
}
  
MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

@@ -826,3 +835,7 @@ [Components]
NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
}
  
+  #

+  # TPM support
+  #
+!include OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc
diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf
index 3f6270c048cc..c62d5757092e 100644
--- a/OvmfPkg/Bhyve/BhyveX64.fdf
+++ b/OvmfPkg/Bhyve/BhyveX64.fdf
@@ -158,6 +158,8 @@ [FV.PEIFV]
  INF  OvmfPkg/Bhyve/SmmAccess/SmmAccessPei.inf
  !endif
  
+!include OvmfPkg/Include/Fdf/OvmfTpmPei.fdf.inc

+
  

  
  [FV.DXEFV]

@@ -335,6 +337,11 @@ [FV.DXEFV]
  INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
  !endif
  
+#

+# TPM support
+#
+!include OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
+
  

  
  [FV.FVMAIN_COMPACT]



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




[edk2-devel] enable MemoryProfile for uefi shell app

2023-06-07 Thread M.T.
Hello group

I'm looking for some help with MemoryProfile to catch some memory leaks in
a custom uefi application.

I followed the instructions found on:
https://github.com/tianocore/tianocore.github.io/wiki/Memory-leak-detection-with-memory-profile-feature

However I can't seem to get this to work, let me paint a more complete
picture.

My uefi app is standalone, it is not a part of any other package and has
its own .dsc file and this is where I make all the changes to enable Memory
Profiler.
The memory I want to watch for is UEFI_APPLICATION, any calls to Allocate*
functions to make sure everything has been freed accordingly, I suspect it
has not hence the leak.

So my debug build has the following Libs:
MemoryAllocationLib|MdeModulePkg/Library/UefiMemoryAllocationProfileLib/UefiMemoryAllocationProfileLib.inf
MemoryProfileLib|MdeModulePkg/Library/UefiMemoryAllocationProfileLib/UefiMemoryAllocationProfileLib.inf

PCDs are set like this:
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask|0x1
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileMemoryType|0x60
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath|{0x04, 0x06,
0x14, 0x00,  0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65,
0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1, 0x7F, 0xFF, 0x04, 0x00}

In Components I added:
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf

This also required that I add DxeServiceLib

Everything builds without issues, my app runs as expected, however when I
try to run
MemoryProfileInfo.efi, I only get the following:
UefiMemoryProfile: Locate MemoryProfile protocol - Not Found
GetUefiMemoryProfileData - Not Found
SmramProfile: Locate SmmCommunication protocol - Not Found
GetSmramProfileData - Not Found

Couple of questions about this:
Is MemoryProfileInfo.efi supposed to be run after my app exits?
Or is it more like a wrapper for my app (ie. valgrind)?

The errors seem to indicate that the libs are still missing, are they
supposed
to be linked into the OVMF image as well perhaps, or anywhere else aside
for my app?

Appreciate any help, debugging third party libs has become a major headache
and I hope
memoryProfiler can help with this.

Cheers
mt


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




[edk2-devel] Event: TianoCore Community Meeting EMEA/NAMO - Thursday, June 8, 2023 #cal-reminder

2023-06-07 Thread Group Notification
*Reminder: TianoCore Community Meeting EMEA/NAMO*

*When:*
Thursday, June 8, 2023
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

*Where:*
Microsoft Teams meeting Join on your computer or mobile app Click here to join 
the meeting Meeting ID: 226 323 011 029 Passcode: hMRCj6 Download Teams | Join 
on the web Join with a video conferencing device te...@conf.intel.com Video 
Conference ID: 112 716 814 3 Alternate VTC instructions Learn More | Meeting 
options

*Organizer:* Miki Demeter

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

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

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

Meeting ID: 226 323 011 029
Passcode: hMRCj6

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 112 716 814 3

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1127168143&ivr=teams&d=conf.intel.com&test=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_MTAyZGJhNjMtYzQ4Mi00MTUxLWFlMWMtOGU0MWNlZDk4NjY5@thread.v2&messageId=0&language=en-US
 )




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




Re: [edk2-devel] failed pr

2023-06-07 Thread Ard Biesheuvel
I found the button too, and it works as expected.

While the ongoing CI issues are investigated, could someone please
push the button on this one? Thanks.

https://github.com/tianocore/edk2/pull/4529



On Wed, 7 Jun 2023 at 05:16, Wu, Hao A  wrote:
>
> Thanks. Below approach works for me:
>
> > From Conversation tab, scroll down to the "Some checks were not
> > successful"
> > area and scroll down to a check that failed.  The select "Details".
> > that will take you to the specific area of the Checks tab that contains
> > a failure and the drop down for "Re-run failed checks" is available.
>
> Best Regards,
> Hao Wu
>
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Wednesday, June 7, 2023 11:08 AM
> > To: Wu, Hao A ; devel@edk2.groups.io; Ard
> > Biesheuvel 
> > Cc: Michael Kubacki ; Sean Brogan
> > ; Kinney, Michael D
> > 
> > Subject: RE: [edk2-devel] failed pr
> >
> > There seems to be multiple views in the checks tab.
> >
> > From Conversation tab, scroll down to the "Some checks were not
> > successful"
> > area and scroll down to a check that failed.  The select "Details".
> > that will take you to the specific area of the Checks tab that contains
> > a failure and the drop down for "Re-run failed checks" is available.
> >
> > Since we 4 groups of check jobs, you have to be in the right scope.
> >
> > From the checks tab, you can also select "Azure Pipelines" and scroll
> > down to a failed check.  Click on it, and "Re-run failed jobs" will be
> > available.  Most of the failures are in Azure Pipelines.
> >
> > Mike
> >
> > > -Original Message-
> > > From: Wu, Hao A 
> > > Sent: Tuesday, June 6, 2023 7:02 PM
> > > To: devel@edk2.groups.io; Kinney, Michael D
> > ;
> > > Ard Biesheuvel 
> > > Cc: Michael Kubacki ; Sean Brogan
> > > 
> > > Subject: RE: [edk2-devel] failed pr
> > >
> > > Hello Mike,
> > >
> > > I do not see the "re-run failed jobs" under "Checks" tab for cancelled CI
> > > tests:
> > > https://github.com/tianocore/edk2/pull/4478
> > > (Only "Re-run all checks" is found)
> > >
> > > What should be done to handle such case?
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > Michael
> > > > D Kinney
> > > > Sent: Wednesday, June 7, 2023 3:55 AM
> > > > To: Ard Biesheuvel ; devel@edk2.groups.io
> > > > Cc: Michael Kubacki ; Sean Brogan
> > > > ; Kinney, Michael D
> > > > 
> > > > Subject: Re: [edk2-devel] failed pr
> > > >
> > > > All EDK II Maintainers should now have permissions to "re-run failed
> > > jobs".
> > > >
> > > > If you see a PR with failed jobs that are not related to the code 
> > > > change,
> > > then
> > > > try rerunning the specific failed job or select "re-run failed jobs"
> > > > from the drop down in top right of "Checks" tab.
> > > >
> > > > Please let me know if you do not see this as an option.
> > > >
> > > > Thanks,
> > > >
> > > > Mike
> > > >
> > > > > -Original Message-
> > > > > From: Kinney, Michael D 
> > > > > Sent: Friday, June 2, 2023 5:07 PM
> > > > > To: Ard Biesheuvel ; devel@edk2.groups.io
> > > > > Cc: Michael Kubacki ; Sean Brogan
> > > > > ; Kinney, Michael D
> > > > > 
> > > > > Subject: RE: [edk2-devel] failed pr
> > > > >
> > > > > Merged
> > > > >
> > > > > Mike
> > > > >
> > > > > > -Original Message-
> > > > > > From: Ard Biesheuvel 
> > > > > > Sent: Friday, June 2, 2023 4:39 PM
> > > > > > To: devel@edk2.groups.io; Kinney, Michael D
> > > > > > 
> > > > > > Cc: Michael Kubacki ; Sean Brogan
> > > > > > 
> > > > > > Subject: Re: [edk2-devel] failed pr
> > > > > >
> > > > > > Another one
> > > > > >
> > > > > > https://github.com/tianocore/edk2/pull/4473
> > > > > >
> > > > > > On Sat, 3 Jun 2023 at 00:32, Ard Biesheuvel  wrote:
> > > > > > >
> > > > > > > Thanks!
> > > > > > >
> > > > > > > On Fri, 2 Jun 2023 at 18:38, Michael D Kinney
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Done.
> > > > > > > >
> > > > > > > > Mike
> > > > > > > >
> > > > > > > > > -Original Message-
> > > > > > > > > From: Kinney, Michael D 
> > > > > > > > > Sent: Friday, June 2, 2023 9:37 AM
> > > > > > > > > To: devel@edk2.groups.io; a...@kernel.org; Michael Kubacki
> > > > > > > > > ; Sean Brogan
> > > > > > 
> > > > > > > > > Cc: Kinney, Michael D 
> > > > > > > > > Subject: RE: [edk2-devel] failed pr
> > > > > > > > >
> > > > > > > > > I am working on it.
> > > > > > > > >
> > > > > > > > > Mike
> > > > > > > > >
> > > > > > > > > > -Original Message-
> > > > > > > > > > From: devel@edk2.groups.io  On
> > Behalf
> > > > > > > > > > Of
> > > > > Ard
> > > > > > > > > > Biesheuvel
> > > > > > > > > > Sent: Friday, June 2, 2023 9:19 AM
> > > > > > > > > > To: Kinney, Michael D ; Michael
> > > > > Kubacki
> > > > > > > > > > ; Sean Brogan
> > > > > > ;
> > > > > > > > > > edk2-devel-groups-io 
> > > > > > > > > > Subject: [edk2-devel] failed pr
> > > > > > > > > >
> > > > > > > > > > Could someone push the merge 

Re: [edk2-devel] failed pr

2023-06-07 Thread Rebecca Cran

I've pushed it.


--

Rebecca Cran


On 6/7/23 09:09, Ard Biesheuvel wrote:

I found the button too, and it works as expected.

While the ongoing CI issues are investigated, could someone please
push the button on this one? Thanks.

https://github.com/tianocore/edk2/pull/4529



On Wed, 7 Jun 2023 at 05:16, Wu, Hao A  wrote:

Thanks. Below approach works for me:


 From Conversation tab, scroll down to the "Some checks were not
successful"
area and scroll down to a check that failed.  The select "Details".
that will take you to the specific area of the Checks tab that contains
a failure and the drop down for "Re-run failed checks" is available.

Best Regards,
Hao Wu


-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, June 7, 2023 11:08 AM
To: Wu, Hao A ; devel@edk2.groups.io; Ard
Biesheuvel 
Cc: Michael Kubacki ; Sean Brogan
; Kinney, Michael D

Subject: RE: [edk2-devel] failed pr

There seems to be multiple views in the checks tab.

 From Conversation tab, scroll down to the "Some checks were not
successful"
area and scroll down to a check that failed.  The select "Details".
that will take you to the specific area of the Checks tab that contains
a failure and the drop down for "Re-run failed checks" is available.

Since we 4 groups of check jobs, you have to be in the right scope.

 From the checks tab, you can also select "Azure Pipelines" and scroll
down to a failed check.  Click on it, and "Re-run failed jobs" will be
available.  Most of the failures are in Azure Pipelines.

Mike


-Original Message-
From: Wu, Hao A 
Sent: Tuesday, June 6, 2023 7:02 PM
To: devel@edk2.groups.io; Kinney, Michael D

;

Ard Biesheuvel 
Cc: Michael Kubacki ; Sean Brogan

Subject: RE: [edk2-devel] failed pr

Hello Mike,

I do not see the "re-run failed jobs" under "Checks" tab for cancelled CI
tests:
https://github.com/tianocore/edk2/pull/4478
(Only "Re-run all checks" is found)

What should be done to handle such case?

Best Regards,
Hao Wu


-Original Message-
From: devel@edk2.groups.io  On Behalf Of

Michael

D Kinney
Sent: Wednesday, June 7, 2023 3:55 AM
To: Ard Biesheuvel ; devel@edk2.groups.io
Cc: Michael Kubacki ; Sean Brogan
; Kinney, Michael D

Subject: Re: [edk2-devel] failed pr

All EDK II Maintainers should now have permissions to "re-run failed

jobs".

If you see a PR with failed jobs that are not related to the code change,

then

try rerunning the specific failed job or select "re-run failed jobs"
from the drop down in top right of "Checks" tab.

Please let me know if you do not see this as an option.

Thanks,

Mike


-Original Message-
From: Kinney, Michael D 
Sent: Friday, June 2, 2023 5:07 PM
To: Ard Biesheuvel ; devel@edk2.groups.io
Cc: Michael Kubacki ; Sean Brogan
; Kinney, Michael D

Subject: RE: [edk2-devel] failed pr

Merged

Mike


-Original Message-
From: Ard Biesheuvel 
Sent: Friday, June 2, 2023 4:39 PM
To: devel@edk2.groups.io; Kinney, Michael D

Cc: Michael Kubacki ; Sean Brogan

Subject: Re: [edk2-devel] failed pr

Another one

https://github.com/tianocore/edk2/pull/4473

On Sat, 3 Jun 2023 at 00:32, Ard Biesheuvel  wrote:

Thanks!

On Fri, 2 Jun 2023 at 18:38, Michael D Kinney
 wrote:

Done.

Mike


-Original Message-
From: Kinney, Michael D 
Sent: Friday, June 2, 2023 9:37 AM
To: devel@edk2.groups.io; a...@kernel.org; Michael Kubacki
; Sean Brogan



Cc: Kinney, Michael D 
Subject: RE: [edk2-devel] failed pr

I am working on it.

Mike


-Original Message-
From: devel@edk2.groups.io  On

Behalf

Of

Ard

Biesheuvel
Sent: Friday, June 2, 2023 9:19 AM
To: Kinney, Michael D ; Michael

Kubacki

; Sean Brogan

;

edk2-devel-groups-io 
Subject: [edk2-devel] failed pr

Could someone push the merge button on this pr please?

https://github.com/tianocore/edk2/pull/4470






















-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105868): https://edk2.groups.io/g/devel/message/105868
Mute This Topic: https://groups.io/mt/99289807/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 1/1] ArmPkg: CpuDxe: Sync GCD Capabilities With Page Table Attributes

2023-06-07 Thread Oliver Smith-Denny

Per the discussion in the memory protections design meeting
this morning, I am kicking this patch back to the top of
the inbox for review. If folks would like me to resend this
patchset since the thread got bogged down with scheduling
meetings, just let me know.

I'll also pull up the BZ link for when the equivalent
change went into the x86 CpuDxe driver in 2017:

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

This contains lots of information about why the change went
in on the x86 side (some dead mail links, but they can be
retrieved through some digging). AFAICT, this change wasn't
applied to ARM at the time due to an oversight, not a general
design decision.

Thanks,
Oliver

On 4/25/2023 5:09 PM, Oliver Smith-Denny wrote:

When ArmPkg's CpuDxe driver initializes, it attempts to sync the

GCD with the page table. However, unlike when the UefiCpuPkg's

CpuDxe initializes, the Arm version does not update the GCD

capabilities with EFI_MEMORY_[RO|RP|XP] (this could also set

the capabilities to be the existing page table attributes for

this range, but the UefiCpuPkg CpuDxe sets the above attributes

as they are software constructs, possible to set for any memory

hardware).



As a result, when the GCD attributes are attempted to be set

against the old GCD capabilities, attributes that are set in the

page table can get lost because the new attributes are not in the

old GCD capabilities (and yet they are already set in the page

table) meaning that the attempted sync between the GCD and the

page table was a failure and drivers querying one vs the other

will see different state. This can lead to RWX memory regions

even with the no-execute policy set, because core drivers (such

as NonDiscoverablePciDeviceIo, to be fixed up in a later patchset)

allocate pages, query the GCD attributes, attempt to set a new

cache attribute and end up clearing the XP bit in the page table

because the GCD attributes do not have XP set.



This patch follows the UefiCpuPkg pattern and adds

EFI_MEMORY_[RO|RP|XP] to the GCD capabilities during CpuDxe

initialization. This ensures that memory regions which already have

these attributes set get them set in the GCD attributes, properly

syncing between the GCD and the page table.



This mitigates the issue seen, however, additional investigations

into setting the GCD attributes earlier and maintaining a better

sync between the GCD and the page table are being done.



Feedback on this proposal is greatly appreciated, particularly

any pitfalls or more architectural solutions to issues seen

with syncing the GCD and the page table.



PR: https://github.com/tianocore/edk2/pull/4311

Personal branch: 
https://github.com/os-d/edk2/tree/osde/sync_aarch64_gcd_capabilities_v1



Cc: Leif Lindholm 

Cc: Ard Biesheuvel 

Cc: Sami Mujawar 

Cc: Michael Kubacki 

Cc: Sean Brogan 



Signed-off-by: Oliver Smith-Denny 

---

  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 55 +---

  1 file changed, 49 insertions(+), 6 deletions(-)



diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c 
b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c

index 2e73719dce04..3ef0380e084f 100644

--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c

+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c

@@ -90,6 +90,7 @@ SetGcdMemorySpaceAttributes (

UINTN EndIndex;

EFI_PHYSICAL_ADDRESS  RegionStart;

UINT64RegionLength;

+  UINT64Capabilities;

  


DEBUG ((

  DEBUG_GCD,

@@ -146,14 +147,56 @@ SetGcdMemorySpaceAttributes (

RegionLength = MemorySpaceMap[Index].BaseAddress + 
MemorySpaceMap[Index].Length - RegionStart;

  }

  


+// Always add RO, RP, and XP as all memory is capable of supporting these 
types (they are software

+// constructs, not hardware features) and they are critical to maintaining 
a security boundary

+Capabilities = MemorySpaceMap[Index].Capabilities | EFI_MEMORY_RO | 
EFI_MEMORY_RP | EFI_MEMORY_XP;

+

  //

-// Set memory attributes according to MTRR attribute and the original 
attribute of descriptor

+// Update GCD capabilities as these may have changed in the page table 
since the GCD was created

+// this follows the same pattern as x86 GCD and Page Table syncing

  //

-gDS->SetMemorySpaceAttributes (

-   RegionStart,

-   RegionLength,

-   (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_CACHETYPE_MASK) | 
(MemorySpaceMap[Index].Capabilities & Attributes)

-   );

+Status = gDS->SetMemorySpaceCapabilities (

+RegionStart,

+RegionLength,

+Capabilities

+);

+

+if (EFI_ERROR (Status)) {

+  DEBUG ((

+DEBUG_ERROR,

+"%a - failed to update GCD capabilities: 0x%llx on memory region: 0x%llx 
length: 0x%llx Status: %r\n",

+__func__,

+Capabilities,

+RegionStart,

+RegionLength,

+Status

+ 

[edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/.github: add build check and uncrustify check

2023-06-07 Thread Nickle Wang via groups.io
Enable Github Actions to check below items:
- RedfishClientPkg build check on push and pull request
- Uncrustify check on pull request

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
---
 .github/.github/workflows/build.sh| 70 +++
 .github/.github/workflows/build.yml   | 61 
 .github/.github/workflows/uncrustify-check.sh | 51 ++
 .github/.github/workflows/uncrustify.yml  | 44 
 4 files changed, 226 insertions(+)
 create mode 100755 .github/.github/workflows/build.sh
 create mode 100644 .github/.github/workflows/build.yml
 create mode 100755 .github/.github/workflows/uncrustify-check.sh
 create mode 100644 .github/.github/workflows/uncrustify.yml

diff --git a/.github/.github/workflows/build.sh 
b/.github/.github/workflows/build.sh
new file mode 100755
index ..f919d551
--- /dev/null
+++ b/.github/.github/workflows/build.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+#
+# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+
+ARCH="X64"
+TARGET="DEBUG"
+TOOLCHAIN="GCC5"
+
+if [ $# -eq 0 ]
+then
+echo "usage: $0 [path to edk2] [path to edk2-redfish-client] [ARCH] 
[TARGET] [TOOLCHAIN]"
+exit 1
+fi
+
+EDK2_ROOT="$PWD/$1"
+EDK2_REDFISH_CLIENT="$PWD/$2"
+ARCH="$3"
+TARGET="$4"
+TOOLCHAIN="$5"
+
+if [ ! -e "$EDK2_ROOT" ]
+then
+echo "$EDK2_ROOT does not exist"
+exit 1
+fi
+
+if [ ! -e "$EDK2_REDFISH_CLIENT" ]
+then
+echo "$EDK2_REDFISH_CLIENT does not exist"
+exit 1
+fi
+
+export PACKAGES_PATH="$EDK2_ROOT:$EDK2_REDFISH_CLIENT"
+echo "PACKAGES_PATH: $PACKAGES_PATH"
+echo "ARCH: $ARCH"
+echo "TARGET: $TARGET"
+echo "TOOLCHAIN: $TOOLCHAIN"
+
+cd "$EDK2_ROOT"
+. edksetup.sh BaseTools
+
+if [ ! -e "BaseTools/Source/C/bin" ]
+then
+  echo "binary does not exist, rebuild it"
+
+  cd BaseTools
+  make
+  cd ..
+
+  echo "If there is build error related to nasm, try to use latest nasm from: 
https://www.nasm.us/pub/nasm/releasebuilds/";
+  echo ""
+  echo "1) wget 
https://www.nasm.us/pub/nasm/releasebuilds/2.15rc12/nasm-2.15rc12.tar.gz";
+  echo "2) tar zxvf nasm-2.15rc12.tar.gz"
+  echo "3) cd nasm-2.15rc12"
+  echo "4) ./configure --prefix=/usr"
+  echo "5) sudo make install"
+  echo "6) nasm -v"
+fi
+
+build -p RedfishClientPkg/RedfishClientPkg.dsc -a $ARCH -t $TOOLCHAIN -b 
$TARGET
+if [ $? -ne 0 ]
+then
+  exit 1
+fi
+
+exit 0
diff --git a/.github/.github/workflows/build.yml 
b/.github/.github/workflows/build.yml
new file mode 100644
index ..f44184b3
--- /dev/null
+++ b/.github/.github/workflows/build.yml
@@ -0,0 +1,61 @@
+# @file
+# GitHub Workflow for build checks
+#
+# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+name: "RedfishClientPkg Build"
+on:
+  push:
+branches:
+  - main
+  pull_request:
+branches:
+  - main
+paths-ignore:
+  - '**/*.bat'
+  - '**/*.md'
+  - '**/*.py'
+  - '**/*.rst'
+  - '**/*.sh'
+  - '**/*.txt'
+
+jobs:
+  edk2-redfish-client-build:
+strategy:
+  fail-fast: false
+  matrix:
+include:
+  - Target: "DEBUG"
+ArchList: "X64"
+ToolChain: "GCC5"
+  - Target: "RELEASE"
+ArchList: "X64"
+ToolChain: "GCC5"
+  - Target: "NOOPT"
+ArchList: "X64"
+ToolChain: "GCC5"
+runs-on: ubuntu-latest
+container:
+  image: ghcr.io/tianocore/containers/ubuntu-22-dev:latest
+  options: --user root -v ${{ github.workspace }}:/home/edk2
+  env:
+EDK2_DOCKER_USER_HOME: "/home/edk2"
+name: edk2 build test
+steps:
+  - name: checkout edk2
+uses: actions/checkout@v3
+with:
+  repository: tianocore/edk2
+  path: ./edk2
+  submodules: recursive
+  - name: checkout edk2-redfish-client
+uses: actions/checkout@v3
+with:
+  path: ./edk2-redfish-client
+  - name: edk2 build
+run: |
+  ./edk2-redfish-client/.github/workflows/build.sh ./edk2 
./edk2-redfish-client ${{ matrix.ArchList }} ${{ matrix.Target }} ${{ 
matrix.ToolChain }}
+shell: bash
+
diff --git a/.github/.github/workflows/uncrustify-check.sh 
b/.github/.github/workflows/uncrustify-check.sh
new file mode 100755
index ..b739a60b
--- /dev/null
+++ b/.github/.github/workflows/uncrustify-check.sh
@@ -0,0 +1,51 @@
+#!/bin/bash
+#
+# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+
+if [ $# -ne 3 ]
+then
+echo "usage: $0 [PATH to CONFIG FILE] [PATH to REPO.] [NO. of COMMITS]"
+exit 1
+fi
+
+CONFIG_FILE="$PWD/$1"
+REPO_PATH="$PWD/$2"
+NO_COMMITS="$3"
+
+if [ ! -e "$CONFIG_FILE" ]
+then
+echo "$CONFIG_FILE does not exist"
+exit 1
+fi
+
+if [ ! -e 

Re: [edk2-devel][PATCH v1 1/1] ArmPkg: CpuDxe: Report AARCH64 Memory Protections Attributes To GCD

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 18:13, Oliver Smith-Denny
 wrote:
>
> Hi Ard,
>
> Thanks for the review. Looks like there isn't much conversation
> on this one, just reviewed-by's from Michael and you. Can this be
> merged? It is ideal for this to go in before the GCD sync patchset,
> because without this patch, the GCD sync will show all memory as
> EFI_MEMORY_RP.
>

I already attempted to merge this iirc, but whether it got merged in
the end is anyone's guess :-)

>
> On 5/23/2023 1:48 PM, Ard Biesheuvel wrote:
> > On Tue, 23 May 2023 at 22:36, Oliver Smith-Denny
> >  wrote:
> >>
> >> When the AARCH64 CpuDxe attempts to SyncCacheConfig() with the GCD,
> >> it collects the page attributes as:
> >>
> >> EntryAttribute = Entry & TT_ATTR_INDX_MASK
> >>
> >> However, TT_ATTR_INDX_MASK only masks the cacheability attributes
> >> and drops the memory protections attributes. Importantly, it also
> >> drops the TT_AF (access flag) which is now wired up in EDK2 to
> >> represent EFI_MEMORY_RP, so by default all SystemMem pages will
> >> report as EFI_MEMORY_RP to the GCD. The GCD currently drops that
> >> silently, because the Capabilities field in the GCD does not support
> >> EFI_MEMORY_RP by default.
> >>
> >> However, some ranges may support EFI_MEMORY_RP and incorrectly
> >> mark those ranges as read protected. In conjunction with
> >> another change on the mailing list (see:
> >> https://edk2.groups.io/g/devel/topic/98505340#:~:text=This%20patch%20follows%20the%20UefiCpuPkg%20pattern%20and%20adds%3D0D,between%20the%20GCD%20and%20the%20page%20table.%3D0D%20%3D0D),
> >> this causes an access flag fault incorrectly. See the linked
> >> BZ below for full details.
> >>
> >> This patch exposes all memory protections attributes to the GCD
> >> layer so it can correctly set pages as EFI_MEMORY[RP|XP|RO] when
> >> it initially syncs.
> >>
> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4463
> >> Personal GitHub PR: https://github.com/tianocore/edk2/pull/4423
> >> Github branch: https://github.com/os-d/edk2/tree/aarch64_report_af_v1
> >>
> >> Cc: Leif Lindholm 
> >> Cc: Ard Biesheuvel 
> >> Cc: Sami Mujawar 
> >> Cc: Taylor Beebe 
> >> Cc: Michael Kubacki 
> >> Cc: Sean Brogan 
> >>
> >> Signed-off-by: Oliver Smith-Denny 
> >
> > Thanks for the fix.
> >
> > Reviewed-by: Ard Biesheuvel 
> >
> >> ---
> >>   ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c 
> >> b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> >> index 0859c7418a1f..1d02e41e18d8 100644
> >> --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> >> +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
> >> @@ -120,7 +120,7 @@ GetFirstPageAttribute (
> >> } else if (((FirstEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) ||
> >>((TableLevel == 3) && ((FirstEntry & TT_TYPE_MASK) == 
> >> TT_TYPE_BLOCK_ENTRY_LEVEL3)))
> >> {
> >> -return FirstEntry & TT_ATTR_INDX_MASK;
> >> +return FirstEntry & TT_ATTRIBUTES_MASK;
> >> } else {
> >>   return INVALID_ENTRY;
> >> }
> >> @@ -158,7 +158,7 @@ GetNextEntryAttribute (
> >> for (Index = 0; Index < EntryCount; Index++) {
> >>   Entry  = TableAddress[Index];
> >>   EntryType  = Entry & TT_TYPE_MASK;
> >> -EntryAttribute = Entry  & TT_ATTR_INDX_MASK;
> >> +EntryAttribute = Entry & TT_ATTRIBUTES_MASK;
> >>
> >>   // If Entry is a Table Descriptor type entry then go through the 
> >> sub-level table
> >>   if ((EntryType == TT_TYPE_BLOCK_ENTRY) ||
> >> --
> >> 2.40.1
> >>


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




Re: [edk2-devel] failed pr

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 17:39, Rebecca Cran  wrote:
>
> I've pushed it.
>


Thanks! How did you manage that?

>
> On 6/7/23 09:09, Ard Biesheuvel wrote:
> > I found the button too, and it works as expected.
> >
> > While the ongoing CI issues are investigated, could someone please
> > push the button on this one? Thanks.
> >
> > https://github.com/tianocore/edk2/pull/4529
> >
> >
> >
> > On Wed, 7 Jun 2023 at 05:16, Wu, Hao A  wrote:
> >> Thanks. Below approach works for me:
> >>
> >>>  From Conversation tab, scroll down to the "Some checks were not
> >>> successful"
> >>> area and scroll down to a check that failed.  The select "Details".
> >>> that will take you to the specific area of the Checks tab that contains
> >>> a failure and the drop down for "Re-run failed checks" is available.
> >> Best Regards,
> >> Hao Wu
> >>
> >>> -Original Message-
> >>> From: Kinney, Michael D 
> >>> Sent: Wednesday, June 7, 2023 11:08 AM
> >>> To: Wu, Hao A ; devel@edk2.groups.io; Ard
> >>> Biesheuvel 
> >>> Cc: Michael Kubacki ; Sean Brogan
> >>> ; Kinney, Michael D
> >>> 
> >>> Subject: RE: [edk2-devel] failed pr
> >>>
> >>> There seems to be multiple views in the checks tab.
> >>>
> >>>  From Conversation tab, scroll down to the "Some checks were not
> >>> successful"
> >>> area and scroll down to a check that failed.  The select "Details".
> >>> that will take you to the specific area of the Checks tab that contains
> >>> a failure and the drop down for "Re-run failed checks" is available.
> >>>
> >>> Since we 4 groups of check jobs, you have to be in the right scope.
> >>>
> >>>  From the checks tab, you can also select "Azure Pipelines" and scroll
> >>> down to a failed check.  Click on it, and "Re-run failed jobs" will be
> >>> available.  Most of the failures are in Azure Pipelines.
> >>>
> >>> Mike
> >>>
>  -Original Message-
>  From: Wu, Hao A 
>  Sent: Tuesday, June 6, 2023 7:02 PM
>  To: devel@edk2.groups.io; Kinney, Michael D
> >>> ;
>  Ard Biesheuvel 
>  Cc: Michael Kubacki ; Sean Brogan
>  
>  Subject: RE: [edk2-devel] failed pr
> 
>  Hello Mike,
> 
>  I do not see the "re-run failed jobs" under "Checks" tab for cancelled CI
>  tests:
>  https://github.com/tianocore/edk2/pull/4478
>  (Only "Re-run all checks" is found)
> 
>  What should be done to handle such case?
> 
>  Best Regards,
>  Hao Wu
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> >>> Michael
> > D Kinney
> > Sent: Wednesday, June 7, 2023 3:55 AM
> > To: Ard Biesheuvel ; devel@edk2.groups.io
> > Cc: Michael Kubacki ; Sean Brogan
> > ; Kinney, Michael D
> > 
> > Subject: Re: [edk2-devel] failed pr
> >
> > All EDK II Maintainers should now have permissions to "re-run failed
>  jobs".
> > If you see a PR with failed jobs that are not related to the code 
> > change,
>  then
> > try rerunning the specific failed job or select "re-run failed jobs"
> > from the drop down in top right of "Checks" tab.
> >
> > Please let me know if you do not see this as an option.
> >
> > Thanks,
> >
> > Mike
> >
> >> -Original Message-
> >> From: Kinney, Michael D 
> >> Sent: Friday, June 2, 2023 5:07 PM
> >> To: Ard Biesheuvel ; devel@edk2.groups.io
> >> Cc: Michael Kubacki ; Sean Brogan
> >> ; Kinney, Michael D
> >> 
> >> Subject: RE: [edk2-devel] failed pr
> >>
> >> Merged
> >>
> >> Mike
> >>
> >>> -Original Message-
> >>> From: Ard Biesheuvel 
> >>> Sent: Friday, June 2, 2023 4:39 PM
> >>> To: devel@edk2.groups.io; Kinney, Michael D
> >>> 
> >>> Cc: Michael Kubacki ; Sean Brogan
> >>> 
> >>> Subject: Re: [edk2-devel] failed pr
> >>>
> >>> Another one
> >>>
> >>> https://github.com/tianocore/edk2/pull/4473
> >>>
> >>> On Sat, 3 Jun 2023 at 00:32, Ard Biesheuvel  wrote:
>  Thanks!
> 
>  On Fri, 2 Jun 2023 at 18:38, Michael D Kinney
>   wrote:
> > Done.
> >
> > Mike
> >
> >> -Original Message-
> >> From: Kinney, Michael D 
> >> Sent: Friday, June 2, 2023 9:37 AM
> >> To: devel@edk2.groups.io; a...@kernel.org; Michael Kubacki
> >> ; Sean Brogan
> >>> 
> >> Cc: Kinney, Michael D 
> >> Subject: RE: [edk2-devel] failed pr
> >>
> >> I am working on it.
> >>
> >> Mike
> >>
> >>> -Original Message-
> >>> From: devel@edk2.groups.io  On
> >>> Behalf
> >>> Of
> >> Ard
> >>> Biesheuvel
> >>> Sent: Friday, June 2, 2023 9:19 AM
> >>> To: Kinney, Michael D ; Michael
> >> Kubacki
> >>> ; Sean Brogan
> >>> ;
> >>> edk2-devel-groups-io 
> >>> Subject: [edk2-devel] failed pr
> >>>

Re: [edk2-devel] failed pr

2023-06-07 Thread Rebecca Cran



On 6/7/23 11:00, Ard Biesheuvel wrote:

On Wed, 7 Jun 2023 at 17:39, Rebecca Cran  wrote:

I've pushed it.



Thanks! How did you manage that?



I just realized I pushed the wrong one! I went to the checks tab and 
clicked the button to re-run all the checks.



--

Rebecca Cran




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




Re: [edk2-devel] failed pr

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 19:02, Rebecca Cran  wrote:
>
>
> On 6/7/23 11:00, Ard Biesheuvel wrote:
> > On Wed, 7 Jun 2023 at 17:39, Rebecca Cran  wrote:
> >> I've pushed it.
> >>
> >
> > Thanks! How did you manage that?
>
>
> I just realized I pushed the wrong one! I went to the checks tab and
> clicked the button to re-run all the checks.
>

Ah ok, so just another roll of the CI dice :-)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105876): https://edk2.groups.io/g/devel/message/105876
Mute This Topic: https://groups.io/mt/99289807/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/2] UefiCpuPkg: CpuTimerDxeRiscV64: Fix incorrect value sent to SbiSetTimer

2023-06-07 Thread Tuan Phan
SbiSetTimer expects core tick value.

Cc: Andrei Warkentin 
Signed-off-by: Tuan Phan 
Reviewed-by: Sunil V L 
---
V2: Fixed format issue with uncrustify.

 .../CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf |  3 +++
 UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c | 26 ---
 UefiCpuPkg/CpuTimerDxeRiscV64/Timer.h |  2 +-
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf 
b/UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf
index c76bd9648373..aba660186dc0 100644
--- a/UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf
+++ b/UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf
@@ -40,6 +40,9 @@
   Timer.h
   Timer.c
 
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuCoreCrystalClockFrequency  ## CONSUMES
+
 [Protocols]
   gEfiCpuArchProtocolGuid   ## CONSUMES
   gEfiTimerArchProtocolGuid ## PRODUCES
diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c 
b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
index fa957ba5e3e9..358057e7c6a4 100644
--- a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
+++ b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.c
@@ -80,8 +80,15 @@ TimerInterruptHandler (
 return;
   }
 
-  mLastPeriodStart  = PeriodStart;
-  SbiSetTimer (PeriodStart += mTimerPeriod);
+  mLastPeriodStart = PeriodStart;
+  PeriodStart += DivU64x32 (
+   MultU64x32 (
+ mTimerPeriod,
+ PcdGet64 (PcdCpuCoreCrystalClockFrequency)
+ ),
+   100u
+   );  // convert to tick
+  SbiSetTimer (PeriodStart);
   RiscVEnableTimerInterrupt (); // enable SMode timer int
   gBS->RestoreTPL (OriginalTPL);
 }
@@ -163,6 +170,8 @@ TimerDriverSetTimerPeriod (
   IN UINT64   TimerPeriod
   )
 {
+  UINT64  PeriodStart;
+
   DEBUG ((DEBUG_INFO, "TimerDriverSetTimerPeriod(0x%lx)\n", TimerPeriod));
 
   if (TimerPeriod == 0) {
@@ -171,9 +180,18 @@ TimerDriverSetTimerPeriod (
 return EFI_SUCCESS;
   }
 
-  mTimerPeriod = TimerPeriod / 10; // convert unit from 100ns to 1us
+  mTimerPeriod = TimerPeriod / 10; // convert unit from 100ns to 1us
+
   mLastPeriodStart = RiscVReadTimer ();
-  SbiSetTimer (mLastPeriodStart + mTimerPeriod);
+  PeriodStart  = mLastPeriodStart;
+  PeriodStart += DivU64x32 (
+   MultU64x32 (
+ mTimerPeriod,
+ PcdGet64 (PcdCpuCoreCrystalClockFrequency)
+ ),
+   100u
+   ); // convert to tick
+  SbiSetTimer (PeriodStart);
 
   mCpu->EnableInterrupt (mCpu);
   RiscVEnableTimerInterrupt (); // enable SMode timer int
diff --git a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.h 
b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.h
index 586eb0cfadb4..9b3542230cb5 100644
--- a/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.h
+++ b/UefiCpuPkg/CpuTimerDxeRiscV64/Timer.h
@@ -21,7 +21,7 @@
 #include 
 
 //
-// RISC-V use 100us timer.
+// RISC-V use 100ns timer.
 // The default timer tick duration is set to 10 ms = 10 * 1000 * 10 100 ns 
units
 //
 #define DEFAULT_TIMER_TICK_DURATION  10
-- 
2.25.1



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




[edk2-devel] [PATCH v2 2/2] UefiCpuPkg: RISC-V: TimerLib: Fix delay function to use 64-bit

2023-06-07 Thread Tuan Phan
The timer compare register is 64-bit so simplifying the delay
function.

Cc: Andrei Warkentin 
Signed-off-by: Tuan Phan 
Reviewed-by: Sunil V L 
---
V2: Fix format issue with uncrustify.

 MdePkg/Include/Register/RiscV64/RiscVImpl.h   |  1 -
 .../BaseRiscV64CpuTimerLib/CpuTimerLib.c  | 53 ---
 2 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/MdePkg/Include/Register/RiscV64/RiscVImpl.h 
b/MdePkg/Include/Register/RiscV64/RiscVImpl.h
index ee5c2ba60377..6997de6cc001 100644
--- a/MdePkg/Include/Register/RiscV64/RiscVImpl.h
+++ b/MdePkg/Include/Register/RiscV64/RiscVImpl.h
@@ -20,6 +20,5 @@
   Name:
 
 #define ASM_FUNC(Name)  _ASM_FUNC(ASM_PFX(Name), .text. ## Name)
-#define RISCV_TIMER_COMPARE_BITS  32
 
 #endif
diff --git a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c 
b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
index 9c8efc0f3530..27d7276aaa8a 100644
--- a/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
+++ b/UefiCpuPkg/Library/BaseRiscV64CpuTimerLib/CpuTimerLib.c
@@ -22,26 +22,19 @@
   @param  Delay A period of time to delay in ticks.
 
 **/
+STATIC
 VOID
 InternalRiscVTimerDelay (
-  IN UINT32  Delay
+  IN UINT64  Delay
   )
 {
-  UINT32  Ticks;
-  UINT32  Times;
-
-  Times  = Delay >> (RISCV_TIMER_COMPARE_BITS - 2);
-  Delay &= ((1 << (RISCV_TIMER_COMPARE_BITS - 2)) - 1);
-  do {
-//
-// The target timer count is calculated here
-//
-Ticks = RiscVReadTimer () + Delay;
-Delay = 1 << (RISCV_TIMER_COMPARE_BITS - 2);
-while (((Ticks - RiscVReadTimer ()) & (1 << (RISCV_TIMER_COMPARE_BITS - 
1))) == 0) {
-  CpuPause ();
-}
-  } while (Times-- > 0);
+  UINT64  Ticks;
+
+  Ticks = RiscVReadTimer () + Delay;
+
+  while (RiscVReadTimer () <= Ticks) {
+CpuPause ();
+  }
 }
 
 /**
@@ -61,13 +54,13 @@ MicroSecondDelay (
   )
 {
   InternalRiscVTimerDelay (
-(UINT32)DivU64x32 (
-  MultU64x32 (
-MicroSeconds,
-PcdGet64 (PcdCpuCoreCrystalClockFrequency)
-),
-  100u
-  )
+DivU64x32 (
+  MultU64x32 (
+MicroSeconds,
+PcdGet64 (PcdCpuCoreCrystalClockFrequency)
+),
+  100u
+  )
 );
   return MicroSeconds;
 }
@@ -89,13 +82,13 @@ NanoSecondDelay (
   )
 {
   InternalRiscVTimerDelay (
-(UINT32)DivU64x32 (
-  MultU64x32 (
-NanoSeconds,
-PcdGet64 (PcdCpuCoreCrystalClockFrequency)
-),
-  10u
-  )
+DivU64x32 (
+  MultU64x32 (
+NanoSeconds,
+PcdGet64 (PcdCpuCoreCrystalClockFrequency)
+),
+  10u
+  )
 );
   return NanoSeconds;
 }
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105878): https://edk2.groups.io/g/devel/message/105878
Mute This Topic: https://groups.io/mt/99389885/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 1/1] ArmPkg: CpuDxe: Sync GCD Capabilities With Page Table Attributes

2023-06-07 Thread Ard Biesheuvel
On Wed, 7 Jun 2023 at 18:10, Oliver Smith-Denny
 wrote:
>
> Per the discussion in the memory protections design meeting
> this morning, I am kicking this patch back to the top of
> the inbox for review. If folks would like me to resend this
> patchset since the thread got bogged down with scheduling
> meetings, just let me know.
>
> I'll also pull up the BZ link for when the equivalent
> change went into the x86 CpuDxe driver in 2017:
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=753
>
> This contains lots of information about why the change went
> in on the x86 side (some dead mail links, but they can be
> retrieved through some digging). AFAICT, this change wasn't
> applied to ARM at the time due to an oversight, not a general
> design decision.
>

Thanks for the background, this is useful.

So I agree that for all system memory regions, we should be setting
the RP, RO and XP capabilities. But what I don't understand is why
these are not set to begin with.

IOW, the resource descriptor HOBs that the initial regions are based
on should have these capabilities set already, and then, we wouldn't
have to do anything to at this point. If there is anything missing
from the generic plumbing to make sure this transformation happens
correctly, we should fix that first, and fix the existing ARM
platforms to set the correct resource attributes.

For example, ArmVirtQemu uses

  ResourceAttributes = (
EFI_RESOURCE_ATTRIBUTE_PRESENT |
EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
EFI_RESOURCE_ATTRIBUTE_TESTED
);

for the resource descriptor HOBs, and afaict, this should include

EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE
EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTABLE
EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE

to accurately describe the region's capabilities.

WIth that out of the way, I wonder if we still need this patch at all.

Thanks,
Ard.



>
> On 4/25/2023 5:09 PM, Oliver Smith-Denny wrote:
> > When ArmPkg's CpuDxe driver initializes, it attempts to sync the
> >
> > GCD with the page table. However, unlike when the UefiCpuPkg's
> >
> > CpuDxe initializes, the Arm version does not update the GCD
> >
> > capabilities with EFI_MEMORY_[RO|RP|XP] (this could also set
> >
> > the capabilities to be the existing page table attributes for
> >
> > this range, but the UefiCpuPkg CpuDxe sets the above attributes
> >
> > as they are software constructs, possible to set for any memory
> >
> > hardware).
> >
> >
> >
> > As a result, when the GCD attributes are attempted to be set
> >
> > against the old GCD capabilities, attributes that are set in the
> >
> > page table can get lost because the new attributes are not in the
> >
> > old GCD capabilities (and yet they are already set in the page
> >
> > table) meaning that the attempted sync between the GCD and the
> >
> > page table was a failure and drivers querying one vs the other
> >
> > will see different state. This can lead to RWX memory regions
> >
> > even with the no-execute policy set, because core drivers (such
> >
> > as NonDiscoverablePciDeviceIo, to be fixed up in a later patchset)
> >
> > allocate pages, query the GCD attributes, attempt to set a new
> >
> > cache attribute and end up clearing the XP bit in the page table
> >
> > because the GCD attributes do not have XP set.
> >
> >
> >
> > This patch follows the UefiCpuPkg pattern and adds
> >
> > EFI_MEMORY_[RO|RP|XP] to the GCD capabilities during CpuDxe
> >
> > initialization. This ensures that memory regions which already have
> >
> > these attributes set get them set in the GCD attributes, properly
> >
> > syncing between the GCD and the page table.
> >
> >
> >
> > This mitigates the issue seen, however, additional investigations
> >
> > into setting the GCD attributes earlier and maintaining a better
> >
> > sync between the GCD and the page table are being done.
> >
> >
> >
> > Feedback on this proposal is greatly appreciated, particularly
> >
> > any pitfalls or more architectural solutions to issues seen
> >
> > with syncing the GCD and the page table.
> >
> >
> >
> > PR: https://github.com/tianocore/edk2/pull/4311
> >
> > Personal branch: 
> > https://github.com/os-d/edk2/tree/osde/sync_aarch64_gcd_capabilities_v1
> >
> >
> >
> > Cc: Leif Lindholm 
> >
> > Cc: Ard Biesheuvel 
> >
> > Cc: Sami Mujawar 
> >
> > Cc: Michael Kubacki 
> >
> > Cc: Sean Brogan 
> >
> >
> >
> > Signed-off-by: Oliver Smith-Denny 
> >
> > ---
> >
> >   ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 55 +---
> >
> >   1 file changed, 49 insertions(+), 6 deletions(-)
> >
> >
> >
> > diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c 
> > b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> >
> > index 2e73719dce04..3ef0380e084f 100644
> >
> > --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> >
> > +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> >
> > @@ -90,6 +90,7 @@ SetGcdMemorySp

Re: [edk2-devel][PATCH v1 1/1] ArmPkg: CpuDxe: Sync GCD Capabilities With Page Table Attributes

2023-06-07 Thread Oliver Smith-Denny

On 6/7/2023 10:31 AM, Ard Biesheuvel wrote:

On Wed, 7 Jun 2023 at 18:10, Oliver Smith-Denny
 wrote:


Per the discussion in the memory protections design meeting
this morning, I am kicking this patch back to the top of
the inbox for review. If folks would like me to resend this
patchset since the thread got bogged down with scheduling
meetings, just let me know.

I'll also pull up the BZ link for when the equivalent
change went into the x86 CpuDxe driver in 2017:

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

This contains lots of information about why the change went
in on the x86 side (some dead mail links, but they can be
retrieved through some digging). AFAICT, this change wasn't
applied to ARM at the time due to an oversight, not a general
design decision.



Thanks for the background, this is useful.

So I agree that for all system memory regions, we should be setting
the RP, RO and XP capabilities. But what I don't understand is why
these are not set to begin with.

IOW, the resource descriptor HOBs that the initial regions are based
on should have these capabilities set already, and then, we wouldn't
have to do anything to at this point. If there is anything missing
from the generic plumbing to make sure this transformation happens
correctly, we should fix that first, and fix the existing ARM
platforms to set the correct resource attributes.

For example, ArmVirtQemu uses

   ResourceAttributes = (
 EFI_RESOURCE_ATTRIBUTE_PRESENT |
 EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
 EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
 EFI_RESOURCE_ATTRIBUTE_TESTED
 );

for the resource descriptor HOBs, and afaict, this should include

EFI_RESOURCE_ATTRIBUTE_WRITE_PROTECTABLE
EFI_RESOURCE_ATTRIBUTE_EXECUTION_PROTECTABLE
EFI_RESOURCE_ATTRIBUTE_READ_ONLY_PROTECTABLE

to accurately describe the region's capabilities.

WIth that out of the way, I wonder if we still need this patch at all.

Thanks,
Ard.



I definitely agree this should be set at GCD initialization. Looking
at the code path you present, I think this could work. My initial
concern was that the resource descriptor HOB was passing attributes
not capabilities, but I see that it actually passes both in a field
called attributes :).

On ARM, as you point out, this looks like we would intercept at
MemoryInitPeiLib when it builds the HOBs. This would be a separate
method from what x86 does, but I can take a look at aligning x86
to initializing the capabilities through the resource descriptor
HOBs.

I'll take a crack at this and see how it shapes up. Seems reasonable
to me, though. I'll need to look into adding memory ranges, we would
want new ranges to have the capabilities, too.

Thanks,
Oliver






On 4/25/2023 5:09 PM, Oliver Smith-Denny wrote:

When ArmPkg's CpuDxe driver initializes, it attempts to sync the

GCD with the page table. However, unlike when the UefiCpuPkg's

CpuDxe initializes, the Arm version does not update the GCD

capabilities with EFI_MEMORY_[RO|RP|XP] (this could also set

the capabilities to be the existing page table attributes for

this range, but the UefiCpuPkg CpuDxe sets the above attributes

as they are software constructs, possible to set for any memory

hardware).



As a result, when the GCD attributes are attempted to be set

against the old GCD capabilities, attributes that are set in the

page table can get lost because the new attributes are not in the

old GCD capabilities (and yet they are already set in the page

table) meaning that the attempted sync between the GCD and the

page table was a failure and drivers querying one vs the other

will see different state. This can lead to RWX memory regions

even with the no-execute policy set, because core drivers (such

as NonDiscoverablePciDeviceIo, to be fixed up in a later patchset)

allocate pages, query the GCD attributes, attempt to set a new

cache attribute and end up clearing the XP bit in the page table

because the GCD attributes do not have XP set.



This patch follows the UefiCpuPkg pattern and adds

EFI_MEMORY_[RO|RP|XP] to the GCD capabilities during CpuDxe

initialization. This ensures that memory regions which already have

these attributes set get them set in the GCD attributes, properly

syncing between the GCD and the page table.



This mitigates the issue seen, however, additional investigations

into setting the GCD attributes earlier and maintaining a better

sync between the GCD and the page table are being done.



Feedback on this proposal is greatly appreciated, particularly

any pitfalls or more architectural solutions to issues seen

with syncing the GCD and the page table.



PR: https://github.com/tianocore/edk2/pull/4311

Personal branch: 
https://github.com/os-d/edk2/tree/osde/sync_aarch64_gcd_capabilities_v1



Cc: Leif Lindholm 

Cc: Ard Biesheuvel 

Cc: Sami Mujawar 

Cc: Michael Kubacki 

Cc: Sean Brogan 



Signed

Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

2023-06-07 Thread Jessica Clarke
On Wed, Sep 23, 2020 at 08:36 PM, Henz, Patrick wrote:

>
> From: Patrick Henz 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948
> 
> Timeouts in the XhciDxe driver are taking longer than
> expected due to the timeout loops not accounting for
> code execution time. As en example, 5 second timeouts
> have been observed to take around 36 seconds to complete.
> Use SetTimer and Create/CheckEvent from Boot Services to
> determine when timeout occurred.

This strategy doesn't work during ExitBootServices, as called from
XhcExitBootService via XhcHaltHC. The net result is that, if the XHCI
controller fails to halt upon clearing R/S, EDK2 hangs in an infinite
loop (something I just had the joy of debugging).

Jess


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




[edk2-devel] errors in ovmf log

2023-06-07 Thread A Sudheer
Hi,

On 'AMD platform + xen +ubuntu dom0', while booting Android domu ,
ovmf.log shows below errors. Can someone help me how to avoid these
errors.  Though boot succeeds, it takes a long time to boot.

1) Error: Image at 000EF785000 start failed: Unsupported
2) Error: Image at 000EF7C3000 start failed: Write Protected
3) Error: Image at 000EF6D start failed: Not Found
4) PciSioSerial: Create SIO child serial device - Device Error
5) UsbSelectConfig: failed to connect driver Not Found, ignored

Please let me know if any more details are needed.

Thanks & Regards
Sudheer


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




Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/.github: add build check and uncrustify check

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

Nice work Nickle!!

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Thursday, June 8, 2023 12:12 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> 
> Subject: [edk2-redfish-client][PATCH] RedfishClientPkg/.github: add build
> check and uncrustify check
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Enable Github Actions to check below items:
> - RedfishClientPkg build check on push and pull request
> - Uncrustify check on pull request
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> ---
>  .github/.github/workflows/build.sh| 70 +++
>  .github/.github/workflows/build.yml   | 61 
>  .github/.github/workflows/uncrustify-check.sh | 51 ++
>  .github/.github/workflows/uncrustify.yml  | 44 
>  4 files changed, 226 insertions(+)
>  create mode 100755 .github/.github/workflows/build.sh
>  create mode 100644 .github/.github/workflows/build.yml
>  create mode 100755 .github/.github/workflows/uncrustify-check.sh
>  create mode 100644 .github/.github/workflows/uncrustify.yml
>
> diff --git a/.github/.github/workflows/build.sh
> b/.github/.github/workflows/build.sh
> new file mode 100755
> index ..f919d551
> --- /dev/null
> +++ b/.github/.github/workflows/build.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash
> +#
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +
> +ARCH="X64"
> +TARGET="DEBUG"
> +TOOLCHAIN="GCC5"
> +
> +if [ $# -eq 0 ]
> +then
> +echo "usage: $0 [path to edk2] [path to edk2-redfish-client] [ARCH]
> [TARGET] [TOOLCHAIN]"
> +exit 1
> +fi
> +
> +EDK2_ROOT="$PWD/$1"
> +EDK2_REDFISH_CLIENT="$PWD/$2"
> +ARCH="$3"
> +TARGET="$4"
> +TOOLCHAIN="$5"
> +
> +if [ ! -e "$EDK2_ROOT" ]
> +then
> +echo "$EDK2_ROOT does not exist"
> +exit 1
> +fi
> +
> +if [ ! -e "$EDK2_REDFISH_CLIENT" ]
> +then
> +echo "$EDK2_REDFISH_CLIENT does not exist"
> +exit 1
> +fi
> +
> +export PACKAGES_PATH="$EDK2_ROOT:$EDK2_REDFISH_CLIENT"
> +echo "PACKAGES_PATH: $PACKAGES_PATH"
> +echo "ARCH: $ARCH"
> +echo "TARGET: $TARGET"
> +echo "TOOLCHAIN: $TOOLCHAIN"
> +
> +cd "$EDK2_ROOT"
> +. edksetup.sh BaseTools
> +
> +if [ ! -e "BaseTools/Source/C/bin" ]
> +then
> +  echo "binary does not exist, rebuild it"
> +
> +  cd BaseTools
> +  make
> +  cd ..
> +
> +  echo "If there is build error related to nasm, try to use latest nasm from:
> https://www.nasm.us/pub/nasm/releasebuilds/";
> +  echo ""
> +  echo "1) wget
> https://www.nasm.us/pub/nasm/releasebuilds/2.15rc12/nasm-
> 2.15rc12.tar.gz"
> +  echo "2) tar zxvf nasm-2.15rc12.tar.gz"
> +  echo "3) cd nasm-2.15rc12"
> +  echo "4) ./configure --prefix=/usr"
> +  echo "5) sudo make install"
> +  echo "6) nasm -v"
> +fi
> +
> +build -p RedfishClientPkg/RedfishClientPkg.dsc -a $ARCH -t $TOOLCHAIN -b
> $TARGET
> +if [ $? -ne 0 ]
> +then
> +  exit 1
> +fi
> +
> +exit 0
> diff --git a/.github/.github/workflows/build.yml
> b/.github/.github/workflows/build.yml
> new file mode 100644
> index ..f44184b3
> --- /dev/null
> +++ b/.github/.github/workflows/build.yml
> @@ -0,0 +1,61 @@
> +# @file
> +# GitHub Workflow for build checks
> +#
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +name: "RedfishClientPkg Build"
> +on:
> +  push:
> +branches:
> +  - main
> +  pull_request:
> +branches:
> +  - main
> +paths-ignore:
> +  - '**/*.bat'
> +  - '**/*.md'
> +  - '**/*.py'
> +  - '**/*.rst'
> +  - '**/*.sh'
> +  - '**/*.txt'
> +
> +jobs:
> +  edk2-redfish-client-build:
> +strategy:
> +  fail-fast: false
> +  matrix:
> +include:
> +  - Target: "DEBUG"
> +ArchList: "X64"
> +ToolChain: "GCC5"
> +  - Target: "RELEASE"
> +ArchList: "X64"
> +ToolChain: "GCC5"
> +  - Target: "NOOPT"
> +ArchList: "X64"
> +ToolChain: "GCC5"
> +runs-on: ubuntu-latest
> +container:
> +  image: ghcr.io/tianocore/containers/ubuntu-22-dev:latest
> +  options: --user root -v ${{ github.workspace }}:/home/edk2
> +  env:
> +EDK2_DOCKER_USER_HOME: "/home/edk2"
> +name: edk2 build test
> +steps:
> +  - name: checkout edk2
> +uses: actions/checkout@v3
> +with:
> +  repository: tianocore/edk2
> +  path: ./edk2
> +  submodules: recursive
> +  - name: checkout edk2-redfish-client
> +uses: actions/checkout@v3
> +with:
> +  path: ./edk2-redfish-client
> +  - name: edk2 build
> +run: |
> +  ./edk2-redfish-client/.git

Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/.github: add build check and uncrustify check

2023-06-07 Thread Igor Kulchytskyy via groups.io
Thank you, Nickle

Get Outlook for Android

From: Chang, Abner 
Sent: Wednesday, June 7, 2023 8:28:11 PM
To: Nickle Wang ; devel@edk2.groups.io 

Cc: Igor Kulchytskyy 
Subject: [EXTERNAL] RE: [edk2-redfish-client][PATCH] RedfishClientPkg/.github: 
add build check and uncrustify check


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

[AMD Official Use Only - General]

Nice work Nickle!!

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Thursday, June 8, 2023 12:12 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> 
> Subject: [edk2-redfish-client][PATCH] RedfishClientPkg/.github: add build
> check and uncrustify check
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Enable Github Actions to check below items:
> - RedfishClientPkg build check on push and pull request
> - Uncrustify check on pull request
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> ---
>  .github/.github/workflows/build.sh| 70 +++
>  .github/.github/workflows/build.yml   | 61 
>  .github/.github/workflows/uncrustify-check.sh | 51 ++
>  .github/.github/workflows/uncrustify.yml  | 44 
>  4 files changed, 226 insertions(+)
>  create mode 100755 .github/.github/workflows/build.sh
>  create mode 100644 .github/.github/workflows/build.yml
>  create mode 100755 .github/.github/workflows/uncrustify-check.sh
>  create mode 100644 .github/.github/workflows/uncrustify.yml
>
> diff --git a/.github/.github/workflows/build.sh
> b/.github/.github/workflows/build.sh
> new file mode 100755
> index ..f919d551
> --- /dev/null
> +++ b/.github/.github/workflows/build.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash
> +#
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +
> +ARCH="X64"
> +TARGET="DEBUG"
> +TOOLCHAIN="GCC5"
> +
> +if [ $# -eq 0 ]
> +then
> +echo "usage: $0 [path to edk2] [path to edk2-redfish-client] [ARCH]
> [TARGET] [TOOLCHAIN]"
> +exit 1
> +fi
> +
> +EDK2_ROOT="$PWD/$1"
> +EDK2_REDFISH_CLIENT="$PWD/$2"
> +ARCH="$3"
> +TARGET="$4"
> +TOOLCHAIN="$5"
> +
> +if [ ! -e "$EDK2_ROOT" ]
> +then
> +echo "$EDK2_ROOT does not exist"
> +exit 1
> +fi
> +
> +if [ ! -e "$EDK2_REDFISH_CLIENT" ]
> +then
> +echo "$EDK2_REDFISH_CLIENT does not exist"
> +exit 1
> +fi
> +
> +export PACKAGES_PATH="$EDK2_ROOT:$EDK2_REDFISH_CLIENT"
> +echo "PACKAGES_PATH: $PACKAGES_PATH"
> +echo "ARCH: $ARCH"
> +echo "TARGET: $TARGET"
> +echo "TOOLCHAIN: $TOOLCHAIN"
> +
> +cd "$EDK2_ROOT"
> +. edksetup.sh BaseTools
> +
> +if [ ! -e "BaseTools/Source/C/bin" ]
> +then
> +  echo "binary does not exist, rebuild it"
> +
> +  cd BaseTools
> +  make
> +  cd ..
> +
> +  echo "If there is build error related to nasm, try to use latest nasm from:
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nasm.us%2Fpub%2Fnasm%2Freleasebuilds%2F&data=05%7C01%7Cigork%40ami.com%7Cfe7c0814e24544d5839a08db67b73fa7%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C638217808989823384%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=51RRpRuxeci9god2Hs5e%2Fa4S7GdeejhRpofp%2FSpjPS4%3D&reserved=0";
> +  echo ""
> +  echo "1) wget
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nasm.us%2Fpub%2Fnasm%2Freleasebuilds%2F2.15rc12%2Fnasm-&data=05%7C01%7Cigork%40ami.com%7Cfe7c0814e24544d5839a08db67b73fa7%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C638217808989823384%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=0X%2B8JSzM7hjzAvLuE6AG1nf6dLKL4SHknyd%2Bv306N%2BA%3D&reserved=0
> 2.15rc12.tar.gz"
> +  echo "2) tar zxvf nasm-2.15rc12.tar.gz"
> +  echo "3) cd nasm-2.15rc12"
> +  echo "4) ./configure --prefix=/usr"
> +  echo "5) sudo make install"
> +  echo "6) nasm -v"
> +fi
> +
> +build -p RedfishClientPkg/RedfishClientPkg.dsc -a $ARCH -t $TOOLCHAIN -b
> $TARGET
> +if [ $? -ne 0 ]
> +then
> +  exit 1
> +fi
> +
> +exit 0
> diff --git a/.github/.github/workflows/build.yml
> b/.github/.github/workflows/build.yml
> new file mode 100644
> index ..f44184b3
> --- /dev/null
> +++ b/.github/.github/workflows/build.yml
> @@ -0,0 +1,61 @@
> +# @file
> +# GitHub Workflow for build checks
> +#
> +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +name: "RedfishClientPkg Build"
> +on:
> +  push:
> +branches:
> +  - main
> +  pull_reques

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

2023-06-07 Thread Gao, Zhichao
I create the PR: ShellPkg/SmbiosView: type 45 and type 46 support. by 
ZhichaoGao · Pull Request #4528 · tianocore/edk2 
(github.com)
But it fail because some CI canceled.

Thanks,
Zhichao

From: gaoliming 
Sent: Friday, June 2, 2023 9:22 AM
To: devel@edk2.groups.io; simow...@nvidia.com; Gao; Gao, Zhichao 

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

Simon:
 Can you create Pull Request for this patch to verify in on EDKII CI?

Thanks
Liming
发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Simon Wang via groups.io
发送时间: 2023年6月1日 21:02
收件人: Gao; Zhichao mailto:zhichao@intel.com>>; 
devel@edk2.groups.io
主题: Re: [edk2-devel] [PATCH] ShellPkg/SmbiosView: type 45 and type 46 support.

Hi,

May I know the plan of code merging?

Thanks,
Simon



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




Re: [edk2-devel] [PATCH] Maintainers.txt: Remove me from maintainers of UefiPayloadPkg,ShellPkg

2023-06-07 Thread Gao, Zhichao
Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

> -Original Message-
> From: Guo, Gua 
> Sent: Wednesday, June 7, 2023 2:19 PM
> To: devel@edk2.groups.io; Ni, Ray 
> Cc: Dong, Guo ; Rhodes, Sean
> ; Lu, James ; Gao, Zhichao
> 
> Subject: RE: [edk2-devel] [PATCH] Maintainers.txt: Remove me from
> maintainers of UefiPayloadPkg,ShellPkg
> 
> Reviewed-by: Gua Guo 
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Wednesday, June 7, 2023 11:31 AM
> To: devel@edk2.groups.io
> Cc: Dong, Guo ; Rhodes, Sean
> ; Lu, James ; Guo, Gua
> ; Gao, Zhichao 
> Subject: [edk2-devel] [PATCH] Maintainers.txt: Remove me from
> maintainers of UefiPayloadPkg,ShellPkg
> 
> Signed-off-by: Ray Ni 
> Cc: Guo Dong 
> Cc: Sean Rhodes 
> Cc: James Lu 
> Cc: Gua Guo 
> Cc: Zhichao Gao 
> ---
>  Maintainers.txt | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt index 5209e21449..e40361a0ae
> 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -592,7 +592,6 @@ R: Rahul Kumar  [rahul1-
> kumar]  ShellPkg F: ShellPkg/ W:
> https://github.com/tianocore/tianocore.github.io/wiki/ShellPkg-M: Ray Ni
>  [niruiyu] M: Zhichao Gao 
> [ZhichaoGao]  SignedCapsulePkg@@ -630,7 +629,6 @@ UefiPayloadPkg
>  F: UefiPayloadPkg/ W:
> https://github.com/tianocore/tianocore.github.io/wiki/UefiPayloadPkg M:
> Guo Dong  [gdong1]-M: Ray Ni 
> [niruiyu] M: Sean Rhodes  [Sean-StarLabs] M:
> James Lu  [jameslu8] R: Gua Guo 
> [gguo11837463]--
> 2.37.2.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#105843):
> https://edk2.groups.io/g/devel/message/105843
> Mute This Topic: https://groups.io/mt/99378233/6998960
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe:
> https://edk2.groups.io/g/devel/leave/11616314/6998960/1091779826/xyzzy
> [gua@intel.com] -=-=-=-=-=-=
> 



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




Re: [edk2-devel] [PATCH] Maintainers.txt: Remove UEFI Shell Binaries section

2023-06-07 Thread Gao, Zhichao
Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Wednesday, June 7, 2023 5:54 PM
> To: devel@edk2.groups.io; Ni, Ray 
> Cc: Gao, Zhichao ; Leif Lindholm
> 
> Subject: Re: [edk2-devel] [PATCH] Maintainers.txt: Remove UEFI Shell
> Binaries section
> 
> On Wed, 7 Jun 2023 at 05:26, Ni, Ray  wrote:
> >
> > The Shell binaries are not generated anymore in each stable tag
> > release.
> > So, remove the section.
> >
> > Cc: Zhichao Gao 
> > Cc: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > Signed-off-by: Ray Ni 
> 
> Reviewed-by: Ard Biesheuvel 
> 
> > ---
> >  Maintainers.txt | 8 
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/Maintainers.txt b/Maintainers.txt index
> > 9c54800f2f..5209e21449 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -81,14 +81,6 @@ EDK II Releases:
> >  W:
> > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-P
> > lanning
> >  M: Liming Gao  [lgao4]
> >
> > -UEFI Shell Binaries (ShellBinPkg.zip) from EDK II Releases:
> > 
> > -W: https://github.com/tianocore/edk2/releases/
> > -M: Ray Ni  [niruiyu](Ia32/X64)
> > -M: Zhichao Gao  [ZhichaoGao]   (Ia32/X64)
> > -M: Leif Lindholm  [leiflindholm]
> (ARM/AArch64)
> > -M: Ard Biesheuvel  [ardbiesheuvel]
> > (ARM/AArch64)
> > -
> >  EDK II Architectures:
> >  -
> >  ARM, AARCH64
> > --
> > 2.37.2.windows.2
> >
> >
> >
> > 
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#105842):
> > https://edk2.groups.io/g/devel/message/105842
> > Mute This Topic: https://groups.io/mt/99378199/1131722
> > Group Owner: devel+ow...@edk2.groups.io
> > Unsubscribe:
> > https://edk2.groups.io/g/devel/leave/9927449/1131722/1583048064/xyzzy
> > [a...@kernel.org]
> > 
> >
> >


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




Re: [edk2-devel] [PATCH] ShellPkg: Increase PcdShellPrintBufferSize from UINT16 to UINT32

2023-06-07 Thread Gao, Zhichao
Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

> -Original Message-
> From: Giri Mudusuru 
> Sent: Tuesday, June 6, 2023 2:02 PM
> To: devel@edk2.groups.io
> Cc: Giri Mudusuru ; Ni, Ray ; Gao,
> Zhichao ; Andrew Fish 
> Subject: [PATCH] ShellPkg: Increase PcdShellPrintBufferSize from UINT16 to
> UINT32
> 
> Increase max buffer size to support more than 64K.
> 
> Signed-off-by: Giri Mudusuru 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Cc: Andrew Fish 
> ---
>  ShellPkg/Application/Shell/Shell.c   |  5 +++--
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c | 19 ++-
>  ShellPkg/ShellPkg.dec|  4 ++--
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index f95c799bb2..01b4e37871 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -4,8 +4,9 @@
>Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
> 
>(C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
> 
>Copyright 2015-2018 Dell Technologies.
> 
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +  Copyright (C) 2023, Apple Inc. All rights reserved.
> 
> 
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> 
> 
>  #include "Shell.h"
> 
> @@ -2944,7 +2945,7 @@ RunScriptFileHandle (
>ASSERT (!ShellCommandGetScriptExit ());
> 
> 
> 
>PreScriptEchoState = ShellCommandGetEchoState ();
> 
> -  PrintBuffSize  = PcdGet16 (PcdShellPrintBufferSize);
> 
> +  PrintBuffSize  = PcdGet32 (PcdShellPrintBufferSize);
> 
> 
> 
>NewScriptFile = (SCRIPT_FILE *)AllocateZeroPool (sizeof (SCRIPT_FILE));
> 
>if (NewScriptFile == NULL) {
> 
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index a72767bd86..746c9ccece 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -4,8 +4,9 @@
>(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
>Copyright 2016-2018 Dell Technologies.
> 
>Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> 
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +  Copyright (C) 2023, Apple Inc. All rights reserved.
> 
> 
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> 
> 
>  #include "UefiShellLib.h"
> 
> @@ -2952,8 +2953,8 @@ InternalShellPrintWorker (
>CHAR16  *mPostReplaceFormat;
> 
>CHAR16  *mPostReplaceFormat2;
> 
> 
> 
> -  mPostReplaceFormat  = AllocateZeroPool (PcdGet16
> (PcdShellPrintBufferSize));
> 
> -  mPostReplaceFormat2 = AllocateZeroPool (PcdGet16
> (PcdShellPrintBufferSize));
> 
> +  mPostReplaceFormat  = AllocateZeroPool (PcdGet32
> (PcdShellPrintBufferSize));
> 
> +  mPostReplaceFormat2 = AllocateZeroPool (PcdGet32
> (PcdShellPrintBufferSize));
> 
> 
> 
>if ((mPostReplaceFormat == NULL) || (mPostReplaceFormat2 == NULL)) {
> 
>  SHELL_FREE_NON_NULL (mPostReplaceFormat);
> 
> @@ -2967,21 +2968,21 @@ InternalShellPrintWorker (
>//
> 
>// Back and forth each time fixing up 1 of our flags...
> 
>//
> 
> -  Status = ShellCopySearchAndReplace (Format, mPostReplaceFormat,
> PcdGet16 (PcdShellPrintBufferSize), L"%N", L"%%N", FALSE, FALSE);
> 
> +  Status = ShellCopySearchAndReplace (Format, mPostReplaceFormat,
> PcdGet32 (PcdShellPrintBufferSize), L"%N", L"%%N", FALSE, FALSE);
> 
>ASSERT_EFI_ERROR (Status);
> 
> -  Status = ShellCopySearchAndReplace (mPostReplaceFormat,
> mPostReplaceFormat2, PcdGet16 (PcdShellPrintBufferSize), L"%E", L"%%E",
> FALSE, FALSE);
> 
> +  Status = ShellCopySearchAndReplace (mPostReplaceFormat,
> mPostReplaceFormat2, PcdGet32 (PcdShellPrintBufferSize), L"%E", L"%%E",
> FALSE, FALSE);
> 
>ASSERT_EFI_ERROR (Status);
> 
> -  Status = ShellCopySearchAndReplace (mPostReplaceFormat2,
> mPostReplaceFormat, PcdGet16 (PcdShellPrintBufferSize), L"%H", L"%%H",
> FALSE, FALSE);
> 
> +  Status = ShellCopySearchAndReplace (mPostReplaceFormat2,
> mPostReplaceFormat, PcdGet32 (PcdShellPrintBufferSize), L"%H", L"%%H",
> FALSE, FALSE);
> 
>ASSERT_EFI_ERROR (Status);
> 
> -  Status = ShellCopySearchAndReplace (mPostReplaceFormat,
> mPostReplaceFormat2, PcdGet16 (PcdShellPrintBufferSize), L"%B", L"%%B",
> FALSE, FALSE);
> 
> +  Status = ShellCopySearchAndReplace (mPostReplaceFormat,
> mPostReplaceFormat2, PcdGet32 (PcdShellPrintBufferSize), L"%B", L"%%B",
> FALSE, FALSE);
> 
>ASSERT_EFI_ERROR (Status);
> 
> -  Status = ShellCopySearchAndReplace (mPostReplaceFormat2,
> mPostReplaceFormat, PcdGet16 (PcdShellPrintBufferSize), L"%V", L"%%V",
> FALSE, FALSE);
> 
> +  Status = ShellCopySearchAndReplace (mPostReplaceFormat2,
> mPostReplaceFormat, PcdGet32 (PcdShellPrintBufferSize), L"%V", L"%%V",
> FALSE, FALSE);
> 
>ASSERT_EFI_ERROR (Status);
> 
> 
> 
>//
> 
>// Use the last buffer from replacing to print from...
> 
>//
> 
> -  UnicodeV

[edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/.github: fix workflows folder location

2023-06-07 Thread Nickle Wang via groups.io
There is additional ".github" folder in early commit.
Move the workflows folder to correct location.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
---
 .github/{.github => }/workflows/build.sh| 0
 .github/{.github => }/workflows/build.yml   | 0
 .github/{.github => }/workflows/uncrustify-check.sh | 0
 .github/{.github => }/workflows/uncrustify.yml  | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename .github/{.github => }/workflows/build.sh (100%)
 rename .github/{.github => }/workflows/build.yml (100%)
 rename .github/{.github => }/workflows/uncrustify-check.sh (100%)
 rename .github/{.github => }/workflows/uncrustify.yml (100%)

diff --git a/.github/.github/workflows/build.sh b/.github/workflows/build.sh
similarity index 100%
rename from .github/.github/workflows/build.sh
rename to .github/workflows/build.sh
diff --git a/.github/.github/workflows/build.yml b/.github/workflows/build.yml
similarity index 100%
rename from .github/.github/workflows/build.yml
rename to .github/workflows/build.yml
diff --git a/.github/.github/workflows/uncrustify-check.sh 
b/.github/workflows/uncrustify-check.sh
similarity index 100%
rename from .github/.github/workflows/uncrustify-check.sh
rename to .github/workflows/uncrustify-check.sh
diff --git a/.github/.github/workflows/uncrustify.yml 
b/.github/workflows/uncrustify.yml
similarity index 100%
rename from .github/.github/workflows/uncrustify.yml
rename to .github/workflows/uncrustify.yml
-- 
2.17.1



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




Re: [edk2-devel] [edk2-redfish-client][PATCH] RedfishClientPkg/.github: fix workflows folder location

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

Reviewed-by: Abner Chang 

Get Outlook for Android


From: Nickle Wang 
Sent: Thursday, June 8, 2023 10:03:45 AM
To: devel@edk2.groups.io 
Cc: Chang, Abner ; Igor Kulchytskyy 
Subject: [edk2-redfish-client][PATCH] RedfishClientPkg/.github: fix workflows 
folder location

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


There is additional ".github" folder in early commit.
Move the workflows folder to correct location.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
---
 .github/{.github => }/workflows/build.sh| 0
 .github/{.github => }/workflows/build.yml   | 0
 .github/{.github => }/workflows/uncrustify-check.sh | 0
 .github/{.github => }/workflows/uncrustify.yml  | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename .github/{.github => }/workflows/build.sh (100%)
 rename .github/{.github => }/workflows/build.yml (100%)
 rename .github/{.github => }/workflows/uncrustify-check.sh (100%)
 rename .github/{.github => }/workflows/uncrustify.yml (100%)

diff --git a/.github/.github/workflows/build.sh b/.github/workflows/build.sh
similarity index 100%
rename from .github/.github/workflows/build.sh
rename to .github/workflows/build.sh
diff --git a/.github/.github/workflows/build.yml b/.github/workflows/build.yml
similarity index 100%
rename from .github/.github/workflows/build.yml
rename to .github/workflows/build.yml
diff --git a/.github/.github/workflows/uncrustify-check.sh 
b/.github/workflows/uncrustify-check.sh
similarity index 100%
rename from .github/.github/workflows/uncrustify-check.sh
rename to .github/workflows/uncrustify-check.sh
diff --git a/.github/.github/workflows/uncrustify.yml 
b/.github/workflows/uncrustify.yml
similarity index 100%
rename from .github/.github/workflows/uncrustify.yml
rename to .github/workflows/uncrustify.yml
--
2.17.1




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




[edk2-devel] [Patch V5 00/14] Use CpuPageTableLib to create and update smm page table

2023-06-07 Thread duntan
In V5 patch set:
1.Remove duplicated patches to add CpuPageTableLib into some DSC files.
2.Seperate the DEBUG_CODE of ConvertMemoryPageAttributes() to a new patch 'Add 
DEBUG_CODE for special case when clear RP'. Also fix issues in the DEBUG_CODE 
and add more detailed debug message about it
3.Fix boundary bug in  'Avoid setting non-present range to RO/NX' code and 
adjust the code to make it easier to understand.
4.Use local variable instead of AllocatePoll in 'Sort mSmmCpuSmramRanges in 
FindSmramInfo' and 'Sort mProtectionMemRange when ReadyToLock'
5.Remove mXdSupported check in 'Refinement to smm runtime InitPaging() code'

Dun Tan (14):
  OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
  MdeModulePkg: Remove RO and NX protection when unset guard page
  UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
  UefiCpuPkg: Add DEBUG_CODE for special case when clear RP
  UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX
  UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP
  UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table
  UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h
  UefiCpuPkg: Add GenSmmPageTable() to create smm page table
  UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table
  UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo
  UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock
  UefiCpuPkg: Refinement to smm runtime InitPaging() code
  UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function

 MdeModulePkg/Core/PiSmmCore/HeapGuard.c|   2 +-
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c |   6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   5 +++--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |   3 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 132 

 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  40 
++--
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 121 
++---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 795 
+++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 324 
++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 229 
-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c   |   3 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c |  20 

 14 files changed, 668 insertions(+), 1015 deletions(-)

-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry

2023-06-07 Thread duntan
Remove code that apply AddressEncMask to non-leaf entry when split
smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it
calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask
bit in page table for a specific range. In AMD SEV feature, this
AddressEncMask bit in page table is used to indicate if the memory
is guest private memory or shared memory. But all memory used by
page table are treated as encrypted regardless of encryption bit.
So remove the EncMask bit for smm non-leaf page table entry
doesn't impact AMD SEV feature.
If page split happens in the AddressEncMask bit clear process,
there will be some new non-leaf entries with AddressEncMask
applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe
module will use CpuPageTableLib to modify smm page table. So
remove code to apply AddressEncMask for new non-leaf entries
since CpuPageTableLib doesn't consume the EncMask PCD.

Signed-off-by: Dun Tan 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Tom Lendacky 
Cc: Ray Ni 
---
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index cf2441b551..aba2e8c081 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -233,7 +233,7 @@ Split2MPageTo4K (
   // Fill in 2M page entry.
   //
   *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 |
-  IA32_PG_P | IA32_PG_RW | AddressEncMask);
+  IA32_PG_P | IA32_PG_RW);
 }
 
 /**
@@ -352,7 +352,7 @@ SetPageTablePoolReadOnly (
 PhysicalAddress += LevelSize[Level - 1];
   }
 
-  PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
+  PageTable[Index] = (UINT64)(UINTN)NewPageTable |
  IA32_PG_P | IA32_PG_RW;
   PageTable = NewPageTable;
 }
@@ -440,7 +440,7 @@ Split1GPageTo2M (
   // Fill in 1G page entry.
   //
   *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry |
-  IA32_PG_P | IA32_PG_RW | AddressEncMask);
+  IA32_PG_P | IA32_PG_RW);
 
   PhysicalAddress2M = PhysicalAddress;
   for (IndexOfPageDirectoryEntries = 0;
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 02/14] MdeModulePkg: Remove RO and NX protection when unset guard page

2023-06-07 Thread duntan
Remove RO and NX protection when unset guard page.
When UnsetGuardPage(), remove all the memory attribute protection
for guarded page.

Signed-off-by: Dun Tan 
Cc: Liming Gao 
Cc: Ray Ni 
Cc: Jian J Wang 
---
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c 
b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
index 8f3bab6fee..7daeeccf13 100644
--- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
+++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
@@ -553,7 +553,7 @@ UnsetGuardPage (
  mSmmMemoryAttribute,
  BaseAddress,
  EFI_PAGE_SIZE,
- EFI_MEMORY_RP
+ 
EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
  );
 ASSERT_EFI_ERROR (Status);
 mOnGuarding = FALSE;
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 03/14] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.

2023-06-07 Thread duntan
Simplify the ConvertMemoryPageAttributes API to convert paging
attribute by CpuPageTableLib. In the new API, it calls
PageTableMap() to update the page attributes of a memory range.
With the PageTableMap() API in CpuPageTableLib, we can remove
the complicated page table manipulating code.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   3 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  28 
+---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 409 
-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|   7 ++-
 5 files changed, 122 insertions(+), 326 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 34bf6e1a25..9c8107080a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -1,7 +1,7 @@
 /** @file
 Page table manipulation functions for IA-32 processors
 
-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
@@ -31,6 +31,7 @@ SmmInitPageTable (
   InitializeSpinLock (mPFLock);
 
   mPhysicalAddressBits = 32;
+  mPagingMode  = PagingPae;
 
   if (FeaturePcdGet (PcdCpuSmmProfileEnable) ||
   HEAP_GUARD_NONSTOP_MODE ||
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a5c2bdd971..ba341cadc6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -50,6 +50,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -260,6 +261,7 @@ extern UINTN mNumberOfCpus;
 extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;
 extern EFI_MM_MP_PROTOCOLmSmmMp;
 extern BOOLEAN   m5LevelPagingNeeded;
+extern PAGING_MODE   mPagingMode;
 
 ///
 /// The mode of the CPU at the time an SMI occurs
@@ -1008,11 +1010,10 @@ SetPageTableAttributes (
   Length from their current attributes to the attributes specified by 
Attributes.
 
   @param[in]   PageTableBaseThe page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode   The paging mode.
   @param[in]   BaseAddress  The physical address that is the start address 
of a memory region.
   @param[in]   Length   The size in bytes of the memory region.
   @param[in]   Attributes   The bit mask of attributes to set for the 
memory region.
-  @param[out]  IsSplitted   TRUE means page table splitted. FALSE means 
page table not splitted.
 
   @retval EFI_SUCCESS   The attributes were set for the memory region.
   @retval EFI_ACCESS_DENIED The attributes for the memory resource range 
specified by
@@ -1030,12 +1031,11 @@ SetPageTableAttributes (
 **/
 EFI_STATUS
 SmmSetMemoryAttributesEx (
-  IN  UINTN PageTableBase,
-  IN  BOOLEAN   EnablePML5Paging,
-  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
-  IN  UINT64Length,
-  IN  UINT64Attributes,
-  OUT BOOLEAN   *IsSplitted  OPTIONAL
+  IN  UINTN PageTableBase,
+  IN  PAGING_MODE   PagingMode,
+  IN  PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length,
+  IN  UINT64Attributes
   );
 
 /**
@@ -1043,34 +1043,32 @@ SmmSetMemoryAttributesEx (
   Length from their current attributes to the attributes specified by 
Attributes.
 
   @param[in]   PageTableBaseThe page table base.
-  @param[in]   EnablePML5Paging If PML5 paging is enabled.
+  @param[in]   PagingMode   The paging mode.
   @param[in]   BaseAddress  The physical address that is the start address 
of a memory region.
   @param[in]   Length   The size in bytes of the memory region.
   @param[in]   Attributes   The bit mask of attributes to clear for the 
memory region.
-  @param[out]  IsSplitted   TRUE means page table splitted. FALSE means 
page table not splitted.
 
   @retval EFI_SUCCESS   The attributes were cleared for the memory 
region.
   @retval EFI_ACCESS_DENIED The attributes for the memory resource range 
specified by
 BaseAddress and Length cannot be modified.
   @retval EFI_INVALID_PARAMETER Length is zero.
   

[edk2-devel] [Patch V5 04/14] UefiCpuPkg: Add DEBUG_CODE for special case when clear RP

2023-06-07 Thread duntan
In ConvertMemoryPageAttributes() function, when clear RP for a
specific range [BaseAddress, BaseAddress + Length], it means to
set the present bit to 1 and assign default value for other
attributes in page table. The default attributes for the input
specific range are NX disabled and ReadOnly. If there is existing
present range in [BaseAddress, BaseAddress + Length] and the
attributes are not NX disabled or not ReadOnly, then output the
DEBUG message to indicate that the NX and ReadOnly attributes of
the existing present range are modified in the function.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 48 

 1 file changed, 48 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 12723e5750..862b3e9720 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -349,6 +349,8 @@ ConvertMemoryPageAttributes (
   IA32_MAP_ENTRY*Map;
   UINTN Count;
   UINTN Index;
+  UINT64OverlappedRangeBase;
+  UINT64OverlappedRangeLimit;
 
   ASSERT (Attributes != 0);
   ASSERT ((Attributes & ~EFI_MEMORY_ATTRIBUTE_MASK) == 0);
@@ -430,6 +432,52 @@ ConvertMemoryPageAttributes (
   // By default memory is Ring 3 accessble.
   //
   PagingAttribute.Bits.UserSupervisor = 1;
+
+  DEBUG_CODE_BEGIN ();
+  if (((Attributes & EFI_MEMORY_RO) == 0) || (((Attributes & 
EFI_MEMORY_XP) == 0) && (mXdSupported))) {
+//
+// When mapping a range to present and EFI_MEMORY_RO or EFI_MEMORY_XP 
is not specificed,
+// check if [BaseAddress, BaseAddress + Length] contains present range.
+// Existing Present range in [BaseAddress, BaseAddress + Length] is 
set to NX disable or ReadOnly.
+//
+Count  = 0;
+Map= NULL;
+Status = PageTableParse (PageTableBase, mPagingMode, NULL, &Count);
+
+while (Status == RETURN_BUFFER_TOO_SMALL) {
+  if (Map != NULL) {
+FreePool (Map);
+  }
+
+  Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
+  ASSERT (Map != NULL);
+  Status = PageTableParse (PageTableBase, mPagingMode, Map, &Count);
+}
+
+ASSERT_RETURN_ERROR (Status);
+for (Index = 0; Index < Count; Index++) {
+  if (Map[Index].LinearAddress >= BaseAddress + Length) {
+break;
+  }
+
+  if ((BaseAddress < Map[Index].LinearAddress + Map[Index].Length) && 
(BaseAddress + Length > Map[Index].LinearAddress)) {
+OverlappedRangeBase  = MAX (BaseAddress, Map[Index].LinearAddress);
+OverlappedRangeLimit = MIN (BaseAddress + Length, 
Map[Index].LinearAddress + Map[Index].Length);
+
+if (((Attributes & EFI_MEMORY_RO) == 0) && 
(Map[Index].Attribute.Bits.ReadWrite == 1)) {
+  DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: [0x%lx, 
0x%lx] is set from ReadWrite to ReadOnly\n", OverlappedRangeBase, 
OverlappedRangeLimit));
+}
+
+if (((Attributes & EFI_MEMORY_XP) == 0) && (mXdSupported) && 
(Map[Index].Attribute.Bits.Nx == 1)) {
+  DEBUG ((DEBUG_ERROR, "SMM ConvertMemoryPageAttributes: [0x%lx, 
0x%lx] is set from NX enabled to NX disabled\n", OverlappedRangeBase, 
OverlappedRangeLimit));
+}
+  }
+}
+
+FreePool (Map);
+  }
+
+  DEBUG_CODE_END ();
 }
   }
 
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX

2023-06-07 Thread duntan
In PiSmmCpuDxeSmm code, SetMemMapAttributes() marks memory ranges
in SmmMemoryAttributesTable to RO/NX. There may exist non-present
range in these memory ranges. Set other attributes for a non-present
range is not permitted in CpuPageTableMapLib. So add code to handle
this case. Only map the present ranges in SmmMemoryAttributesTable
to RO or NX.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 129 
+++--
 1 file changed, 107 insertions(+), 22 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 862b3e9720..3c79927c7b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -918,6 +918,70 @@ PatchGdtIdtMap (
 );
 }
 
+/**
+  This function set [Base, Limit] to the input MemoryAttribute.
+
+  @param  BaseStart address of range.
+  @param  Limit   Limit address of range.
+  @param  Attribute   The bit mask of attributes to modify for the memory 
region.
+  @param  Map Pointer to the array of Cr3 IA32_MAP_ENTRY.
+  @param  Count   Count of IA32_MAP_ENTRY in Map.
+**/
+VOID
+SetMemMapWithNonPresentRange (
+  UINT64  Base,
+  UINT64  Limit,
+  UINT64  Attribute,
+  IA32_MAP_ENTRY  *Map,
+  UINTN   Count
+  )
+{
+  UINTN   Index;
+  UINT64  NonPresentRangeStart;
+
+  NonPresentRangeStart = 0;
+  for (Index = 0; Index < Count; Index++) {
+if ((Map[Index].LinearAddress > NonPresentRangeStart) &&
+(Base < Map[Index].LinearAddress) && (Limit > NonPresentRangeStart))
+{
+  //
+  // We should NOT set attributes for non-present ragne.
+  //
+  //
+  // There is a non-present ( [NonPresentStart, Map[Index].LinearAddress] 
) range before current Map[Index]
+  // and it is overlapped with [Base, Limit].
+  //
+  if (Base < NonPresentRangeStart) {
+SmmSetMemoryAttributes (
+  Base,
+  NonPresentRangeStart - Base,
+  Attribute
+  );
+  }
+
+  Base = Map[Index].LinearAddress;
+}
+
+NonPresentRangeStart = Map[Index].LinearAddress + Map[Index].Length;
+if (NonPresentRangeStart >= Limit) {
+  break;
+}
+  }
+
+  Limit = MIN (NonPresentRangeStart, Limit);
+
+  if (Base < Limit) {
+//
+// There is no non-present range in current [Base, Limit] anymore.
+//
+SmmSetMemoryAttributes (
+  Base,
+  Limit - Base,
+  Attribute
+  );
+  }
+}
+
 /**
   This function sets memory attribute according to MemoryAttributesTable.
 **/
@@ -932,6 +996,11 @@ SetMemMapAttributes (
   UINTN DescriptorSize;
   UINTN Index;
   EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;
+  UINTN PageTable;
+  EFI_STATUSStatus;
+  IA32_MAP_ENTRY*Map;
+  UINTN Count;
+  UINT64MemoryAttribute;
 
   SmmGetSystemConfigurationTable (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID 
**)&MemoryAttributesTable);
   if (MemoryAttributesTable == NULL) {
@@ -958,36 +1027,52 @@ SetMemMapAttributes (
 MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
   }
 
+  Count = 0;
+  Map   = NULL;
+  PageTable = AsmReadCr3 ();
+  Status= PageTableParse (PageTable, mPagingMode, NULL, &Count);
+  while (Status == RETURN_BUFFER_TOO_SMALL) {
+if (Map != NULL) {
+  FreePool (Map);
+}
+
+Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
+ASSERT (Map != NULL);
+Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
+  }
+
+  ASSERT_RETURN_ERROR (Status);
+
   MemoryMap = MemoryMapStart;
   for (Index = 0; Index < MemoryMapEntryCount; Index++) {
 DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", 
MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
-switch (MemoryMap->Type) {
-  case EfiRuntimeServicesCode:
-SmmSetMemoryAttributes (
-  MemoryMap->PhysicalStart,
-  EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
-  EFI_MEMORY_RO
-  );
-break;
-  case EfiRuntimeServicesData:
-SmmSetMemoryAttributes (
-  MemoryMap->PhysicalStart,
-  EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
-  EFI_MEMORY_XP
-  );
-break;
-  default:
-SmmSetMemoryAttributes (
-  MemoryMap->PhysicalStart,
-  EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
-  EFI_MEMORY_XP
-  );
-break;
+if (MemoryMap->Type == EfiRuntimeServicesCode) {
+  MemoryAttribute = EFI_MEMORY_RO;
+

[edk2-devel] [Patch V5 06/14] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP

2023-06-07 Thread duntan
Add two functions to disable/enable CR0.WP. These two unctions
will also be used in later commits. This commit doesn't change any
functionality.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Reviewed-by: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  24 

 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 115 
++-
 2 files changed, 90 insertions(+), 49 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index ba341cadc6..e0c4ca76dc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1565,4 +1565,28 @@ SmmWaitForApArrival (
   VOID
   );
 
+/**
+  Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
+
+  @param[out]  WpEnabled  If Cr0.WP is enabled.
+  @param[out]  CetEnabled If CET is enabled.
+**/
+VOID
+DisableReadOnlyPageWriteProtect (
+  OUT BOOLEAN  *WpEnabled,
+  OUT BOOLEAN  *CetEnabled
+  );
+
+/**
+  Enable Write Protect on pages marked as read-only.
+
+  @param[out]  WpEnabled  If Cr0.WP should be enabled.
+  @param[out]  CetEnabled If CET should be enabled.
+**/
+VOID
+EnableReadOnlyPageWriteProtect (
+  BOOLEAN  WpEnabled,
+  BOOLEAN  CetEnabled
+  );
+
 #endif
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 3c79927c7b..d35058ed00 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -40,6 +40,64 @@ PAGE_TABLE_POOL  *mPageTablePool = NULL;
 //
 BOOLEAN  mIsReadOnlyPageTable = FALSE;
 
+/**
+  Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
+
+  @param[out]  WpEnabled  If Cr0.WP is enabled.
+  @param[out]  CetEnabled If CET is enabled.
+**/
+VOID
+DisableReadOnlyPageWriteProtect (
+  OUT BOOLEAN  *WpEnabled,
+  OUT BOOLEAN  *CetEnabled
+  )
+{
+  IA32_CR0  Cr0;
+
+  *CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+  Cr0.UintN   = AsmReadCr0 ();
+  *WpEnabled  = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
+  if (*WpEnabled) {
+if (*CetEnabled) {
+  //
+  // CET must be disabled if WP is disabled. Disable CET before clearing 
CR0.WP.
+  //
+  DisableCet ();
+}
+
+Cr0.Bits.WP = 0;
+AsmWriteCr0 (Cr0.UintN);
+  }
+}
+
+/**
+  Enable Write Protect on pages marked as read-only.
+
+  @param[out]  WpEnabled  If Cr0.WP should be enabled.
+  @param[out]  CetEnabled If CET should be enabled.
+**/
+VOID
+EnableReadOnlyPageWriteProtect (
+  BOOLEAN  WpEnabled,
+  BOOLEAN  CetEnabled
+  )
+{
+  IA32_CR0  Cr0;
+
+  if (WpEnabled) {
+Cr0.UintN   = AsmReadCr0 ();
+Cr0.Bits.WP = 1;
+AsmWriteCr0 (Cr0.UintN);
+
+if (CetEnabled) {
+  //
+  // re-enable CET.
+  //
+  EnableCet ();
+}
+  }
+}
+
 /**
   Initialize a buffer pool for page table use only.
 
@@ -62,10 +120,9 @@ InitializePageTablePool (
   IN UINTN  PoolPages
   )
 {
-  VOID  *Buffer;
-  BOOLEAN   CetEnabled;
-  BOOLEAN   WpEnabled;
-  IA32_CR0  Cr0;
+  VOID *Buffer;
+  BOOLEAN  WpEnabled;
+  BOOLEAN  CetEnabled;
 
   //
   // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for
@@ -102,34 +159,9 @@ InitializePageTablePool (
   // If page table memory has been marked as RO, mark the new pool pages as 
read-only.
   //
   if (mIsReadOnlyPageTable) {
-CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
-Cr0.UintN  = AsmReadCr0 ();
-WpEnabled  = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
-if (WpEnabled) {
-  if (CetEnabled) {
-//
-// CET must be disabled if WP is disabled. Disable CET before clearing 
CR0.WP.
-//
-DisableCet ();
-  }
-
-  Cr0.Bits.WP = 0;
-  AsmWriteCr0 (Cr0.UintN);
-}
-
+DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
 SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, 
EFI_PAGES_TO_SIZE (PoolPages), EFI_MEMORY_RO);
-if (WpEnabled) {
-  Cr0.UintN   = AsmReadCr0 ();
-  Cr0.Bits.WP = 1;
-  AsmWriteCr0 (Cr0.UintN);
-
-  if (CetEnabled) {
-//
-// re-enable CET.
-//
-EnableCet ();
-  }
-}
+EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
   }
 
   return TRUE;
@@ -1780,6 +1812,7 @@ SetPageTableAttributes (
   VOID
   )
 {
+  BOOLEAN  WpEnabled;
   BOOLEAN  CetEnabled;
 
   if (!IfReadOnlyPageTableNeeded ()) {
@@ -1792,15 +1825,7 @@ SetPageTableAttributes (
   // Disable write protection, because we need mark page table to be write 
protected.
   // We need *write* page table memory, to mark itself to be *read only*.
   //
-  CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
-  if (CetEnabled) {
-//
-// CET must be dis

[edk2-devel] [Patch V5 07/14] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table

2023-06-07 Thread duntan
Clear CR0.WP before modify smm page table. Currently, there is
an assumption that smm pagetable is always RW before ReadyToLock.
However, when AMD SEV is enabled, FvbServicesSmm driver calls
MemEncryptSevClearMmioPageEncMask to clear AddressEncMask bit
in smm page table for this range:
[PcdOvmfFdBaseAddress,PcdOvmfFdBaseAddress+PcdOvmfFirmwareFdSize]
If page slpit happens in this process, new memory for smm page
table is allocated. Then the newly allocated page table memory
is marked as RO in smm page table in this FvbServicesSmm driver,
which may lead to PF if smm code doesn't clear CR0.WP before
modify smm page table when ReadyToLock.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Reviewed-by: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 11 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |  5 +
 2 files changed, 16 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index d35058ed00..4ee99d06d7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1033,6 +1033,8 @@ SetMemMapAttributes (
   IA32_MAP_ENTRY*Map;
   UINTN Count;
   UINT64MemoryAttribute;
+  BOOLEAN   WpEnabled;
+  BOOLEAN   CetEnabled;
 
   SmmGetSystemConfigurationTable (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID 
**)&MemoryAttributesTable);
   if (MemoryAttributesTable == NULL) {
@@ -1075,6 +1077,8 @@ SetMemMapAttributes (
 
   ASSERT_RETURN_ERROR (Status);
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
   MemoryMap = MemoryMapStart;
   for (Index = 0; Index < MemoryMapEntryCount; Index++) {
 DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", 
MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
@@ -1103,6 +1107,7 @@ SetMemMapAttributes (
 MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
   }
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
   FreePool (Map);
 
   PatchSmmSaveStateMap ();
@@ -1409,9 +1414,13 @@ SetUefiMemMapAttributes (
   UINTN  MemoryMapEntryCount;
   UINTN  Index;
   EFI_MEMORY_DESCRIPTOR  *Entry;
+  BOOLEANWpEnabled;
+  BOOLEANCetEnabled;
 
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
   if (mUefiMemoryMap != NULL) {
 MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
 MemoryMap   = mUefiMemoryMap;
@@ -1490,6 +1499,8 @@ SetUefiMemMapAttributes (
 }
   }
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
   //
   // Do not free mUefiMemoryAttributesTable, it will be checked in 
IsSmmCommBufferForbiddenAddress().
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 1b0b6673e1..5625ba0cac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -574,6 +574,8 @@ InitPaging (
   BOOLEAN   Nx;
   IA32_CR4  Cr4;
   BOOLEAN   Enable5LevelPaging;
+  BOOLEAN   WpEnabled;
+  BOOLEAN   CetEnabled;
 
   Cr4.UintN  = AsmReadCr4 ();
   Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
@@ -620,6 +622,7 @@ InitPaging (
 NumberOfPdptEntries = 4;
   }
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
   //
   // Go through page table and change 2MB-page into 4KB-page.
   //
@@ -800,6 +803,8 @@ InitPaging (
 } // end for PML4
   } // end for PML5
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
   //
   // Flush TLB
   //
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 08/14] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h

2023-06-07 Thread duntan
Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h and remove
extern for mSmmShadowStackSize in c files to simplify code.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h| 1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c   | 2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  | 3 +--
 5 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 6c48a53f67..636dc8d92f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for Ia32 arch specific.
 
-Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -14,7 +14,6 @@ EFI_PHYSICAL_ADDRESS  mGdtBuffer;
 UINTN mGdtBufferSize;
 
 extern BOOLEAN  mCetSupported;
-extern UINTNmSmmShadowStackSize;
 
 X86_ASSEMBLY_PATCH_LABEL  mPatchCetPl0Ssp;
 X86_ASSEMBLY_PATCH_LABEL  mPatchCetInterruptSsp;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index baf827cf9d..1878252eac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -29,8 +29,6 @@ MM_COMPLETIONmSmmStartupThisApToken;
 //
 UINT32  *mPackageFirstThreadIndex = NULL;
 
-extern UINTN  mSmmShadowStackSize;
-
 /**
   Performs an atomic compare exchange operation to get semaphore.
   The compare exchange operation must be performed using
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index e0c4ca76dc..a7da9673a5 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -262,6 +262,7 @@ extern EFI_SMM_CPU_PROTOCOL  mSmmCpu;
 extern EFI_MM_MP_PROTOCOLmSmmMp;
 extern BOOLEAN   m5LevelPagingNeeded;
 extern PAGING_MODE   mPagingMode;
+extern UINTN mSmmShadowStackSize;
 
 ///
 /// The mode of the CPU at the time an SMI occurs
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 0bed857cae..46d8ff5d4e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -13,8 +13,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define PAGE_TABLE_PAGES  8
 #define ACC_MAX_BIT   BIT3
 
-extern UINTN  mSmmShadowStackSize;
-
 LIST_ENTRYmPagePool   = INITIALIZE_LIST_HEAD_VARIABLE 
(mPagePool);
 BOOLEAN   m1GPageTableSupport = FALSE;
 BOOLEAN   mCpuSmmRestrictedMemoryAccess;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index 00a284c369..c4f21e2155 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file
   SMM CPU misc functions for x64 arch specific.
 
-Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -12,7 +12,6 @@ EFI_PHYSICAL_ADDRESS  mGdtBuffer;
 UINTN mGdtBufferSize;
 
 extern BOOLEAN  mCetSupported;
-extern UINTNmSmmShadowStackSize;
 
 X86_ASSEMBLY_PATCH_LABEL  mPatchCetPl0Ssp;
 X86_ASSEMBLY_PATCH_LABEL  mPatchCetInterruptSsp;
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 09/14] UefiCpuPkg: Add GenSmmPageTable() to create smm page table

2023-06-07 Thread duntan
This commit is code refinement to current smm pagetable generation
code. Add a new GenSmmPageTable() API to create smm page table
based on the PageTableMap() API in CpuPageTableLib. Caller only
needs to specify the paging mode and the PhysicalAddressBits to map.
This function can be used to create both IA32 pae paging and X64
5level, 4level paging.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  15 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  65 
+
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 220 
++--
 4 files changed, 107 insertions(+), 195 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 9c8107080a..b11264ce4a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -63,7 +63,7 @@ SmmInitPageTable (
 InitializeIDTSmmStackGuard ();
   }
 
-  return Gen4GPageTable (TRUE);
+  return GenSmmPageTable (PagingPae, mPhysicalAddressBits);
 }
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a7da9673a5..5399659bc0 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -553,6 +553,21 @@ Gen4GPageTable (
   IN  BOOLEAN  Is32BitPageTable
   );
 
+/**
+  Create page table based on input PagingMode and PhysicalAddressBits in smm.
+
+  @param[in]  PagingMode   The paging mode.
+  @param[in]  PhysicalAddressBits  The bits of physical address to map.
+
+  @retval PageTable Address
+
+**/
+UINTN
+GenSmmPageTable (
+  IN PAGING_MODE  PagingMode,
+  IN UINT8PhysicalAddressBits
+  );
+
 /**
   Initialize global data for MP synchronization.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 4ee99d06d7..4e93d26eb4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1640,6 +1640,71 @@ EdkiiSmmClearMemoryAttributes (
   return SmmClearMemoryAttributes (BaseAddress, Length, Attributes);
 }
 
+/**
+  Create page table based on input PagingMode and PhysicalAddressBits in smm.
+
+  @param[in]  PagingMode   The paging mode.
+  @param[in]  PhysicalAddressBits  The bits of physical address to map.
+
+  @retval PageTable Address
+
+**/
+UINTN
+GenSmmPageTable (
+  IN PAGING_MODE  PagingMode,
+  IN UINT8PhysicalAddressBits
+  )
+{
+  UINTN   PageTableBufferSize;
+  UINTN   PageTable;
+  VOID*PageTableBuffer;
+  IA32_MAP_ATTRIBUTE  MapAttribute;
+  IA32_MAP_ATTRIBUTE  MapMask;
+  RETURN_STATUS   Status;
+  UINTN   GuardPage;
+  UINTN   Index;
+  UINT64  Length;
+
+  Length   = LShiftU64 (1, PhysicalAddressBits);
+  PageTable= 0;
+  PageTableBufferSize  = 0;
+  MapMask.Uint64   = MAX_UINT64;
+  MapAttribute.Uint64  = mAddressEncMask;
+  MapAttribute.Bits.Present= 1;
+  MapAttribute.Bits.ReadWrite  = 1;
+  MapAttribute.Bits.UserSupervisor = 1;
+  MapAttribute.Bits.Accessed   = 1;
+  MapAttribute.Bits.Dirty  = 1;
+
+  Status = PageTableMap (&PageTable, PagingMode, NULL, &PageTableBufferSize, 
0, Length, &MapAttribute, &MapMask, NULL);
+  ASSERT (Status == RETURN_BUFFER_TOO_SMALL);
+  DEBUG ((DEBUG_INFO, "GenSMMPageTable: 0x%x bytes needed for initial SMM page 
table\n", PageTableBufferSize));
+  PageTableBuffer = AllocatePageTableMemory (EFI_SIZE_TO_PAGES 
(PageTableBufferSize));
+  ASSERT (PageTableBuffer != NULL);
+  Status = PageTableMap (&PageTable, PagingMode, PageTableBuffer, 
&PageTableBufferSize, 0, Length, &MapAttribute, &MapMask, NULL);
+  ASSERT (Status == RETURN_SUCCESS);
+  ASSERT (PageTableBufferSize == 0);
+
+  if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
+//
+// Mark the 4KB guard page between known good stack and smm stack as 
non-present
+//
+for (Index = 0; Index < gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; 
Index++) {
+  GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE + Index * (mSmmStackSize 
+ mSmmShadowStackSize);
+  Status= ConvertMemoryPageAttributes (PageTable, PagingMode, 
GuardPage, SIZE_4KB, EFI_MEMORY_RP, TRUE, NULL);
+}
+  }
+
+  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) {
+//
+// Mark [0, 4k] as non-present
+//
+Status = ConvertMemoryPag

[edk2-devel] [Patch V5 10/14] UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table

2023-06-07 Thread duntan
Use GenSmmPageTable() to create both IA32 and X64 Smm S3
page table.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Reviewed-by: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c |   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c   | 130 
--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c  |  20 
 3 files changed, 5 insertions(+), 147 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
index bba4a6f058..650090e534 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
@@ -18,7 +18,7 @@ InitSmmS3Cr3 (
   VOID
   )
 {
-  mSmmS3ResumeState->SmmS3Cr3 = Gen4GPageTable (TRUE);
+  mSmmS3ResumeState->SmmS3Cr3 = GenSmmPageTable (PagingPae, 
mPhysicalAddressBits);
 
   return;
 }
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 1878252eac..f8b81fc96e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -999,136 +999,6 @@ APHandler (
   ReleaseSemaphore (mSmmMpSyncData->CpuData[BspIndex].Run);
 }
 
-/**
-  Create 4G PageTable in SMRAM.
-
-  @param[in]  Is32BitPageTable Whether the page table is 32-bit PAE
-  @return PageTable Address
-
-**/
-UINT32
-Gen4GPageTable (
-  IN  BOOLEAN  Is32BitPageTable
-  )
-{
-  VOID*PageTable;
-  UINTN   Index;
-  UINT64  *Pte;
-  UINTN   PagesNeeded;
-  UINTN   Low2MBoundary;
-  UINTN   High2MBoundary;
-  UINTN   Pages;
-  UINTN   GuardPage;
-  UINT64  *Pdpte;
-  UINTN   PageIndex;
-  UINTN   PageAddress;
-
-  Low2MBoundary  = 0;
-  High2MBoundary = 0;
-  PagesNeeded= 0;
-  if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-//
-// Add one more page for known good stack, then find the lower 2MB aligned 
address.
-//
-Low2MBoundary = (mSmmStackArrayBase + EFI_PAGE_SIZE) & ~(SIZE_2MB-1);
-//
-// Add two more pages for known good stack and stack guard page,
-// then find the lower 2MB aligned address.
-//
-High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize - mSmmShadowStackSize 
+ EFI_PAGE_SIZE * 2) & ~(SIZE_2MB-1);
-PagesNeeded= ((High2MBoundary - Low2MBoundary) / SIZE_2MB) + 1;
-  }
-
-  //
-  // Allocate the page table
-  //
-  PageTable = AllocatePageTableMemory (5 + PagesNeeded);
-  ASSERT (PageTable != NULL);
-
-  PageTable = (VOID *)((UINTN)PageTable);
-  Pte   = (UINT64 *)PageTable;
-
-  //
-  // Zero out all page table entries first
-  //
-  ZeroMem (Pte, EFI_PAGES_TO_SIZE (1));
-
-  //
-  // Set Page Directory Pointers
-  //
-  for (Index = 0; Index < 4; Index++) {
-Pte[Index] = ((UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1)) | 
mAddressEncMask |
- (Is32BitPageTable ? IA32_PAE_PDPTE_ATTRIBUTE_BITS : 
PAGE_ATTRIBUTE_BITS);
-  }
-
-  Pte += EFI_PAGE_SIZE / sizeof (*Pte);
-
-  //
-  // Fill in Page Directory Entries
-  //
-  for (Index = 0; Index < EFI_PAGE_SIZE * 4 / sizeof (*Pte); Index++) {
-Pte[Index] = (Index << 21) | mAddressEncMask | IA32_PG_PS | 
PAGE_ATTRIBUTE_BITS;
-  }
-
-  Pdpte = (UINT64 *)PageTable;
-  if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-Pages = (UINTN)PageTable + EFI_PAGES_TO_SIZE (5);
-GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE;
-for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += 
SIZE_2MB) {
-  Pte = (UINT64 
*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] & ~mAddressEncMask 
& ~(EFI_PAGE_SIZE - 1));
-  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | 
mAddressEncMask | PAGE_ATTRIBUTE_BITS;
-  //
-  // Fill in Page Table Entries
-  //
-  Pte = (UINT64 *)Pages;
-  PageAddress = PageIndex;
-  for (Index = 0; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) {
-if (PageAddress == GuardPage) {
-  //
-  // Mark the guard page as non-present
-  //
-  Pte[Index] = PageAddress | mAddressEncMask;
-  GuardPage += (mSmmStackSize + mSmmShadowStackSize);
-  if (GuardPage > mSmmStackArrayEnd) {
-GuardPage = 0;
-  }
-} else {
-  Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
-}
-
-PageAddress += EFI_PAGE_SIZE;
-  }
-
-  Pages += EFI_PAGE_SIZE;
-}
-  }
-
-  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) {
-Pte = (UINT64 *)(UINTN)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 
1));
-if ((Pte[0] & IA32_PG_PS) == 0) {
-  // 4K-page entries are already mapped. Just hide the first one anyway.
-  Pte = (UINT64 *)(UINTN)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE 
- 1));
-  Pte[0] &= ~(UINT64)IA32_PG_P; // Hide page 0
-} else {
-  // Crea

[edk2-devel] [Patch V5 11/14] UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo

2023-06-07 Thread duntan
Sort mSmmCpuSmramRanges after get the SMRAM info in
FindSmramInfo() function.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 32 

 1 file changed, 32 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2144d6ade8..2568ffd677 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1197,6 +1197,32 @@ PiCpuSmmEntry (
   return EFI_SUCCESS;
 }
 
+/**
+  Function to compare 2 EFI_SMRAM_DESCRIPTOR based on CpuStart.
+
+  @param[in] Buffer1pointer to Device Path poiner to compare
+  @param[in] Buffer2pointer to second DevicePath pointer to compare
+
+  @retval 0 Buffer1 equal to Buffer2
+  @retval <0Buffer1 is less than Buffer2
+  @retval >0Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+CpuSmramRangeCompare (
+  IN  CONST VOID  *Buffer1,
+  IN  CONST VOID  *Buffer2
+  )
+{
+  if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart > ((EFI_SMRAM_DESCRIPTOR 
*)Buffer2)->CpuStart) {
+return 1;
+  } else if (((EFI_SMRAM_DESCRIPTOR *)Buffer1)->CpuStart < 
((EFI_SMRAM_DESCRIPTOR *)Buffer2)->CpuStart) {
+return -1;
+  }
+
+  return 0;
+}
+
 /**
 
   Find out SMRAM information including SMRR base and SMRR size.
@@ -1218,6 +1244,7 @@ FindSmramInfo (
   UINTN Index;
   UINT64MaxSize;
   BOOLEAN   Found;
+  EFI_SMRAM_DESCRIPTOR  SmramDescriptor;
 
   //
   // Get SMM Access Protocol
@@ -1240,6 +1267,11 @@ FindSmramInfo (
 
   mSmmCpuSmramRangeCount = Size / sizeof (EFI_SMRAM_DESCRIPTOR);
 
+  //
+  // Sort the mSmmCpuSmramRanges
+  //
+  QuickSort (mSmmCpuSmramRanges, mSmmCpuSmramRangeCount, sizeof 
(EFI_SMRAM_DESCRIPTOR), (BASE_SORT_COMPARE)CpuSmramRangeCompare, 
&SmramDescriptor);
+
   //
   // Find the largest SMRAM range between 1MB and 4GB that is at least 256K - 
4K in size
   //
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 13/14] UefiCpuPkg: Refinement to smm runtime InitPaging() code

2023-06-07 Thread duntan
This commit is code refinement to current smm runtime InitPaging()
page table update code. In InitPaging(), if PcdCpuSmmProfileEnable
is TRUE, use ConvertMemoryPageAttributes() API to map the range in
mProtectionMemRange to the attrbute recorded in the attribute field
of mProtectionMemRange, map the range outside mProtectionMemRange
as non-present. If PcdCpuSmmProfileEnable is FALSE, only need to
set the ranges not in mSmmCpuSmramRanges as NX.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  37 
+
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 293 
-
 2 files changed, 101 insertions(+), 229 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 5399659bc0..12ad86028e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -725,6 +725,43 @@ SmmBlockingStartupThisAp (
   IN OUT  VOID  *ProcArguments OPTIONAL
   );
 
+/**
+  This function modifies the page attributes for the memory region specified 
by BaseAddress and
+  Length from their current attributes to the attributes specified by 
Attributes.
+
+  Caller should make sure BaseAddress and Length is at page boundary.
+
+  @param[in]   PageTableBaseThe page table base.
+  @param[in]   BaseAddress  The physical address that is the start address 
of a memory region.
+  @param[in]   Length   The size in bytes of the memory region.
+  @param[in]   Attributes   The bit mask of attributes to modify for the 
memory region.
+  @param[in]   IsSetTRUE means to set attributes. FALSE means to 
clear attributes.
+  @param[out]  IsModified   TRUE means page table modified. FALSE means 
page table not modified.
+
+  @retval RETURN_SUCCESS   The attributes were modified for the memory 
region.
+  @retval RETURN_ACCESS_DENIED The attributes for the memory resource 
range specified by
+   BaseAddress and Length cannot be modified.
+  @retval RETURN_INVALID_PARAMETER Length is zero.
+   Attributes specified an illegal combination 
of attributes that
+   cannot be set together.
+  @retval RETURN_OUT_OF_RESOURCES  There are not enough system resources to 
modify the attributes of
+   the memory resource range.
+  @retval RETURN_UNSUPPORTED   The processor does not support one or more 
bytes of the memory
+   resource range specified by BaseAddress and 
Length.
+   The bit mask of attributes is not support 
for the memory resource
+   range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+ConvertMemoryPageAttributes (
+  IN  UINTN PageTableBase,
+  IN  PAGING_MODE   PagingMode,
+  IN  PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length,
+  IN  UINT64Attributes,
+  IN  BOOLEAN   IsSet,
+  OUT BOOLEAN   *IsModified   OPTIONAL
+  );
+
 /**
   This function sets the attributes for the memory region specified by 
BaseAddress and
   Length from their current attributes to the attributes specified by 
Attributes.
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 2a1f132b29..ab6b79ddf4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -586,254 +586,89 @@ InitPaging (
   VOID
   )
 {
-  UINT64Pml5Entry;
-  UINT64Pml4Entry;
-  UINT64*Pml5;
-  UINT64*Pml4;
-  UINT64*Pdpt;
-  UINT64*Pd;
-  UINT64*Pt;
-  UINTN Address;
-  UINTN Pml5Index;
-  UINTN Pml4Index;
-  UINTN PdptIndex;
-  UINTN PdIndex;
-  UINTN PtIndex;
-  UINTN NumberOfPdptEntries;
-  UINTN NumberOfPml4Entries;
-  UINTN NumberOfPml5Entries;
-  UINTN SizeOfMemorySpace;
-  BOOLEAN   Nx;
-  IA32_CR4  Cr4;
-  BOOLEAN   Enable5LevelPaging;
-  BOOLEAN   WpEnabled;
-  BOOLEAN   CetEnabled;
-
-  Cr4.UintN  = AsmReadCr4 ();
-  Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
-
-  if (sizeof (UINTN) == sizeof (UINT64)) {
-if (!Enable5LevelPaging) {
-  Pml5Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
-  Pml5  = &Pml5Entry;
-} else {
-  Pml5 = (UINT64 *)(UINTN)mSmmProfileCr3;
-}
-
-SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
-ASSERT (SizeOfMemorySpace <= 52);
-
-//
-// Calculate the table entries of PML5E, PML4E and PDPTE.
-//
-NumberOfPml5

[edk2-devel] [Patch V5 12/14] UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock

2023-06-07 Thread duntan
Sort mProtectionMemRange in InitProtectedMemRange() when
ReadyToLock.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 5625ba0cac..2a1f132b29 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -375,6 +375,32 @@ IsAddressSplit (
   return FALSE;
 }
 
+/**
+  Function to compare 2 MEMORY_PROTECTION_RANGE based on range base.
+
+  @param[in] Buffer1pointer to Device Path poiner to compare
+  @param[in] Buffer2pointer to second DevicePath pointer to compare
+
+  @retval 0 Buffer1 equal to Buffer2
+  @retval <0Buffer1 is less than Buffer2
+  @retval >0Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+ProtectionRangeCompare (
+  IN  CONST VOID  *Buffer1,
+  IN  CONST VOID  *Buffer2
+  )
+{
+  if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base > 
((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
+return 1;
+  } else if (((MEMORY_PROTECTION_RANGE *)Buffer1)->Range.Base < 
((MEMORY_PROTECTION_RANGE *)Buffer2)->Range.Base) {
+return -1;
+  }
+
+  return 0;
+}
+
 /**
   Initialize the protected memory ranges and the 4KB-page mapped memory ranges.
 
@@ -397,6 +423,7 @@ InitProtectedMemRange (
   EFI_PHYSICAL_ADDRESS Base2MBAlignedAddress;
   UINT64   High4KBPageSize;
   UINT64   Low4KBPageSize;
+  MEMORY_PROTECTION_RANGE  MemProtectionRange;
 
   NumberOfDescriptors  = 0;
   NumberOfAddedDescriptors = mSmmCpuSmramRangeCount;
@@ -533,6 +560,11 @@ InitProtectedMemRange (
 
   mSplitMemRangeCount = NumberOfSpliteRange;
 
+  //
+  // Sort the mProtectionMemRange
+  //
+  QuickSort (mProtectionMemRange, mProtectionMemRangeCount, sizeof 
(MEMORY_PROTECTION_RANGE), (BASE_SORT_COMPARE)ProtectionRangeCompare, 
&MemProtectionRange);
+
   DEBUG ((DEBUG_INFO, "SMM Profile Memory Ranges:\n"));
   for (Index = 0; Index < mProtectionMemRangeCount; Index++) {
 DEBUG ((DEBUG_INFO, "mProtectionMemRange[%d].Base = %lx\n", Index, 
mProtectionMemRange[Index].Range.Base));
-- 
2.31.1.windows.1



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




[edk2-devel] [Patch V5 14/14] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function

2023-06-07 Thread duntan
Remove unnecessary function SetNotPresentPage(). We can directly
use ConvertMemoryPageAttributes to set a range to non-present.

Signed-off-by: Dun Tan 
Cc: Eric Dong 
Reviewed-by: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  8 ++--
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 16 
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 22 --
 3 files changed, 6 insertions(+), 40 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2568ffd677..c2452e6a29 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1074,10 +1074,14 @@ PiCpuSmmEntry (
 mSmmShadowStackSize
 );
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
-SetNotPresentPage (
+ConvertMemoryPageAttributes (
   Cr3,
+  mPagingMode,
   (EFI_PHYSICAL_ADDRESS)(UINTN)Stacks + mSmmStackSize + 
EFI_PAGES_TO_SIZE (1) + (mSmmStackSize + mSmmShadowStackSize) * Index,
-  EFI_PAGES_TO_SIZE (1)
+  EFI_PAGES_TO_SIZE (1),
+  EFI_MEMORY_RP,
+  TRUE,
+  NULL
   );
   }
 }
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 12ad86028e..0dc4d758cc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1247,22 +1247,6 @@ SetShadowStack (
   IN  UINT64Length
   );
 
-/**
-  Set not present memory.
-
-  @param[in]  Cr3  The page table base address.
-  @param[in]  BaseAddress  The physical address that is the start address 
of a memory region.
-  @param[in]  Length   The size in bytes of the memory region.
-
-  @retval EFI_SUCCESS   The not present memory is set.
-**/
-EFI_STATUS
-SetNotPresentPage (
-  IN  UINTN Cr3,
-  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
-  IN  UINT64Length
-  );
-
 /**
   Initialize the shadow stack related data structure.
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 4e93d26eb4..ab502b800b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -768,28 +768,6 @@ SetShadowStack (
   return Status;
 }
 
-/**
-  Set not present memory.
-
-  @param[in]  Cr3  The page table base address.
-  @param[in]  BaseAddress  The physical address that is the start address 
of a memory region.
-  @param[in]  Length   The size in bytes of the memory region.
-
-  @retval EFI_SUCCESS   The not present memory is set.
-**/
-EFI_STATUS
-SetNotPresentPage (
-  IN  UINTN Cr3,
-  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
-  IN  UINT64Length
-  )
-{
-  EFI_STATUS  Status;
-
-  Status = SmmSetMemoryAttributesEx (Cr3, mPagingMode, BaseAddress, Length, 
EFI_MEMORY_RP);
-  return Status;
-}
-
 /**
   Retrieves a pointer to the system configuration table from the SMM System 
Table
   based on a specified GUID.
-- 
2.31.1.windows.1



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




[edk2-devel] Event: TianoCore Community Meeting - APAC/NAMO - Thursday, June 8, 2023 #cal-reminder

2023-06-07 Thread Group Notification
*Reminder: TianoCore Community Meeting - APAC/NAMO*

*When:*
Thursday, June 8, 2023
7:30pm to 8:30pm
(UTC-07:00) America/Los Angeles

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

*Organizer:* Miki Demeter

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

*Description:*



Microsoft Teams meeting

*Join on your computer or mobile app*

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

Meeting ID: 283 318 374 436
Passcode: 633zLo

Download Teams ( https://www.microsoft.com/en-us/microsoft-teams/download-app ) 
| Join on the web ( https://www.microsoft.com/microsoft-teams/join-a-meeting )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 119 493 012 8

Alternate VTC instructions ( 
https://conf.intel.com/teams/?conf=1194930128&ivr=teams&d=conf.intel.com&test=test_call
 )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=6e4ce4c4-1242-431b-9a51-92cd01a5df3c&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_Y2M1NDE3ODYtN2M3Yy00MDMxLTk3OWYtMTlkNjhlNWFlMjA2@thread.v2&messageId=0&language=en-US
 )




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




[edk2-devel] [PATCH 1/4] MdePkg: Add new API GetMaxPlatformAddressBits

2023-06-07 Thread Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394

Add new API GetMaxPlatformAddressBits to get the max platform address
bits. Max physical address bits can be get from CPUID. When TME-MK
feature is enabled, the upper bits of the max physical address bits
are repurposed for usage as a KeyID.
Therefore, the max platform addressable bits is the max physical
address bits minus the upper bits used for KeyID if TME-MK is enable.

Cc: Michael D Kinney 
Cc: Liming Gao 
Signed-off-by: Zhiguang Liu 
---
 MdePkg/Include/Library/CpuLib.h   | 25 +++
 MdePkg/Library/BaseCpuLib/X86BaseCpuLib.c | 81 +++
 2 files changed, 106 insertions(+)

diff --git a/MdePkg/Include/Library/CpuLib.h b/MdePkg/Include/Library/CpuLib.h
index 3f29937dc7..a9bac083b7 100644
--- a/MdePkg/Include/Library/CpuLib.h
+++ b/MdePkg/Include/Library/CpuLib.h
@@ -87,6 +87,31 @@ GetCpuSteppingId (
   VOID
   );
 
+/**
+  Get the max platform addressable bits.
+  Max physical address bits can be get from CPUID. When TME-MK feature
+  is enabled, the upper bits of the max physical address bits are
+  repurposed for usage as a KeyID.
+  Therefore, the max platform addressable bits is the max physical
+  address bits minus the upper bits used for KeyID if TME-MK is enable.
+
+  @param[out] ValidAddressMask  Bitmask with valid address bits set to
+one; other bits are clear. Optional
+parameter.
+
+  @param[out] ValidPageBaseAddressMask  Bitmask with valid page base address
+bits set to one; other bits are clear.
+Optional parameter.
+
+  @return  The max platform addressable bits.
+**/
+UINT8
+EFIAPI
+GetMaxPlatformAddressBits (
+  OUT UINT64  *ValidAddressMask OPTIONAL,
+  OUT UINT64  *ValidPageBaseAddressMask OPTIONAL
+  );
+
 #endif
 
 #endif
diff --git a/MdePkg/Library/BaseCpuLib/X86BaseCpuLib.c 
b/MdePkg/Library/BaseCpuLib/X86BaseCpuLib.c
index 1cad32a4be..7b15cb3d73 100644
--- a/MdePkg/Library/BaseCpuLib/X86BaseCpuLib.c
+++ b/MdePkg/Library/BaseCpuLib/X86BaseCpuLib.c
@@ -14,6 +14,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 /**
   Determine if the standard CPU signature is "AuthenticAMD".
@@ -79,3 +81,82 @@ GetCpuSteppingId (
 
   return (UINT8)Eax.Bits.SteppingId;
 }
+
+/**
+  Get the max platform addressable bits.
+  Max physical address bits can be get from CPUID. When TME-MK feature
+  is enabled, the upper bits of the max physical address bits are
+  repurposed for usage as a KeyID.
+  Therefore, the max platform addressable bits is the max physical
+  address bits minus the upper bits used for KeyID if TME-MK is enable.
+
+  @param[out] ValidAddressMask  Bitmask with valid address bits set to
+one; other bits are clear. Optional
+parameter.
+
+  @param[out] ValidPageBaseAddressMask  Bitmask with valid page base address
+bits set to one; other bits are clear.
+Optional parameter.
+
+  @return  The max platform addressable bits.
+**/
+UINT8
+EFIAPI
+GetMaxPlatformAddressBits (
+  OUT UINT64  *ValidAddressMask OPTIONAL,
+  OUT UINT64  *ValidPageBaseAddressMask OPTIONAL
+  )
+{
+  UINT32   MaxExtendedFunction;
+  CPUID_VIR_PHY_ADDRESS_SIZE_EAX   VirPhyAddressSize;
+  UINT64   AddressMask;
+  UINT64   PageBaseAddressMask;
+  UINT32   MaxFunction;
+  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX  ExtendedFeatureFlagsEcx;
+  MSR_IA32_TME_ACTIVATE_REGISTER   TmeActivate;
+  MSR_IA32_TME_CAPABILITY_REGISTER TmeCapability;
+
+  AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedFunction, NULL, NULL, NULL);
+  if (MaxExtendedFunction >= CPUID_VIR_PHY_ADDRESS_SIZE) {
+AsmCpuid (
+  CPUID_VIR_PHY_ADDRESS_SIZE,
+  &VirPhyAddressSize.Uint32,
+  NULL,
+  NULL,
+  NULL
+  );
+  } else {
+VirPhyAddressSize.Bits.PhysicalAddressBits = 36;
+  }
+
+  //
+  // CPUID enumeration of MAX_PA is unaffected by TME-MK activation and will 
continue
+  // to report the maximum physical address bits available for software to use,
+  // irrespective of the number of KeyID bits.
+  // So, we need to check if TME is enabled and adjust the PA size accordingly.
+  //
+  AsmCpuid (CPUID_SIGNATURE, &MaxFunction, NULL, NULL, NULL);
+  if (MaxFunction >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
+AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, 
&ExtendedFeatureFlagsEcx.Uint32, NULL);
+if (ExtendedFeatureFlagsEcx.Bits.TME_EN == 1) {
+  TmeActivate.Uint64   = AsmReadMsr64 (MSR_IA32_TME_ACTIVATE);
+  TmeCapability.Uint64 = AsmReadMsr64 (MSR_IA32_TME_CAPABILITY)

[edk2-devel] [PATCH 2/4] PrmPkg: Use new API to replace MtrrLibInitializeMtrrMask

2023-06-07 Thread Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394

The function MtrrLibInitializeMtrrMask is a private function
in MtrrLib.c from UefiCpuPkg, and it can be replace with new
API GetMaxPlatformAddressBits.

Cc: Michael Kubacki 
Cc: Nate DeSimone 
Signed-off-by: Zhiguang Liu 
---
 .../PrmSampleHardwareAccessModule.c| 18 ++
 .../PrmSampleHardwareAccessModule.inf  |  1 +
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git 
a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c 
b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
index 1a1e735029..398497c3a9 100644
--- 
a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
+++ 
b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -37,21 +38,6 @@
 //
 extern CONST CHAR8  *mMtrrMemoryCacheTypeShortName[];
 
-/**
-  Initializes the valid bits mask and valid address mask for MTRRs.
-
-  This function initializes the valid bits mask and valid address mask for 
MTRRs.
-
-  @param[out]  MtrrValidBitsMask The mask for the valid bit of the MTRR
-  @param[out]  MtrrValidAddressMask  The valid address mask for the MTRR
-
-**/
-VOID
-MtrrLibInitializeMtrrMask (
-  OUT UINT64  *MtrrValidBitsMask,
-  OUT UINT64  *MtrrValidAddressMask
-  );
-
 /**
   Convert variable MTRRs to a RAW MTRR_MEMORY_RANGE array.
   One MTRR_MEMORY_RANGE element is created for each MTRR setting.
@@ -151,7 +137,7 @@ AccessAllMtrrs (
   MtrrGetAllMtrrs (&LocalMtrrs);
   Mtrrs = &LocalMtrrs;
 
-  MtrrLibInitializeMtrrMask (&MtrrValidBitsMask, &MtrrValidAddressMask);
+  GetMaxPlatformAddressBits (&MtrrValidBitsMask, &MtrrValidAddressMask);
   Ranges[0].BaseAddress = 0;
   Ranges[0].Length  = MtrrValidBitsMask + 1;
   Ranges[0].Type= MtrrGetDefaultMemoryType ();
diff --git 
a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
 
b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
index 46d4a88185..b15da817c1 100644
--- 
a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
+++ 
b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
@@ -34,6 +34,7 @@
   MtrrLib
   UefiDriverEntryPoint
   UefiLib
+  CpuLib
 
 [Depex]
   TRUE
-- 
2.31.1.windows.1



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




[edk2-devel] [PATCH 3/4] UefiCpuPkg: Clean up some Mtrr code using new API

2023-06-07 Thread Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394

With new API GetMaxPlatformAddressBits, the API
MtrrLibInitializeMtrrMask and InitializeMtrrMask can be replaced.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Zhiguang Liu 
---
 UefiCpuPkg/CpuDxe/CpuDxe.c   | 49 +---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 56 ++--
 2 files changed, 4 insertions(+), 101 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index 804ef5d1fe..7505801c01 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -494,52 +494,6 @@ CpuSetMemoryAttributes (
   return AssignMemoryPageAttributes (NULL, BaseAddress, Length, 
MemoryAttributes, NULL);
 }
 
-/**
-  Initializes the valid bits mask and valid address mask for MTRRs.
-
-  This function initializes the valid bits mask and valid address mask for 
MTRRs.
-
-**/
-VOID
-InitializeMtrrMask (
-  VOID
-  )
-{
-  UINT32   MaxExtendedFunction;
-  CPUID_VIR_PHY_ADDRESS_SIZE_EAX   VirPhyAddressSize;
-  UINT32   MaxFunction;
-  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX  ExtendedFeatureFlagsEcx;
-  MSR_IA32_TME_ACTIVATE_REGISTER   TmeActivate;
-
-  AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedFunction, NULL, NULL, NULL);
-
-  if (MaxExtendedFunction >= CPUID_VIR_PHY_ADDRESS_SIZE) {
-AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, &VirPhyAddressSize.Uint32, NULL, 
NULL, NULL);
-  } else {
-VirPhyAddressSize.Bits.PhysicalAddressBits = 36;
-  }
-
-  //
-  // CPUID enumeration of MAX_PA is unaffected by TME-MK activation and will 
continue
-  // to report the maximum physical address bits available for software to use,
-  // irrespective of the number of KeyID bits.
-  // So, we need to check if TME is enabled and adjust the PA size accordingly.
-  //
-  AsmCpuid (CPUID_SIGNATURE, &MaxFunction, NULL, NULL, NULL);
-  if (MaxFunction >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
-AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, 
&ExtendedFeatureFlagsEcx.Uint32, NULL);
-if (ExtendedFeatureFlagsEcx.Bits.TME_EN == 1) {
-  TmeActivate.Uint64 = AsmReadMsr64 (MSR_IA32_TME_ACTIVATE);
-  if (TmeActivate.Bits.TmeEnable == 1) {
-VirPhyAddressSize.Bits.PhysicalAddressBits -= 
TmeActivate.Bits.MkTmeKeyidBits;
-  }
-}
-  }
-
-  mValidMtrrBitsMask= LShiftU64 (1, 
VirPhyAddressSize.Bits.PhysicalAddressBits) - 1;
-  mValidMtrrAddressMask = mValidMtrrBitsMask & 0xf000ULL;
-}
-
 /**
   Gets GCD Mem Space type from MTRR Type.
 
@@ -740,8 +694,7 @@ RefreshMemoryAttributesFromMtrr (
   //
   // Initialize the valid bits mask and valid address mask for MTRRs
   //
-  InitializeMtrrMask ();
-
+  GetMaxPlatformAddressBits (&mValidMtrrBitsMask, &mValidMtrrAddressMask);
   //
   // Get the memory attribute of variable MTRRs
   //
diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 22ec8d2a48..95bf8d771f 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -741,56 +741,6 @@ MtrrLibTypeLeftPrecedeRight (
   return (BOOLEAN)(Left == CacheUncacheable || (Left == CacheWriteThrough && 
Right == CacheWriteBack));
 }
 
-/**
-  Initializes the valid bits mask and valid address mask for MTRRs.
-
-  This function initializes the valid bits mask and valid address mask for 
MTRRs.
-
-  @param[out]  MtrrValidBitsMask The mask for the valid bit of the MTRR
-  @param[out]  MtrrValidAddressMask  The valid address mask for the MTRR
-
-**/
-VOID
-MtrrLibInitializeMtrrMask (
-  OUT UINT64  *MtrrValidBitsMask,
-  OUT UINT64  *MtrrValidAddressMask
-  )
-{
-  UINT32   MaxExtendedFunction;
-  CPUID_VIR_PHY_ADDRESS_SIZE_EAX   VirPhyAddressSize;
-  UINT32   MaxFunction;
-  CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX  ExtendedFeatureFlagsEcx;
-  MSR_IA32_TME_ACTIVATE_REGISTER   TmeActivate;
-
-  AsmCpuid (CPUID_EXTENDED_FUNCTION, &MaxExtendedFunction, NULL, NULL, NULL);
-
-  if (MaxExtendedFunction >= CPUID_VIR_PHY_ADDRESS_SIZE) {
-AsmCpuid (CPUID_VIR_PHY_ADDRESS_SIZE, &VirPhyAddressSize.Uint32, NULL, 
NULL, NULL);
-  } else {
-VirPhyAddressSize.Bits.PhysicalAddressBits = 36;
-  }
-
-  //
-  // CPUID enumeration of MAX_PA is unaffected by TME-MK activation and will 
continue
-  // to report the maximum physical address bits available for software to use,
-  // irrespective of the number of KeyID bits.
-  // So, we need to check if TME is enabled and adjust the PA size accordingly.
-  //
-  AsmCpuid (CPUID_SIGNATURE, &MaxFunction, NULL, NULL, NULL);
-  if (MaxFunction >= CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
-AsmCpuidEx (CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, 0, NULL, NULL, 
&ExtendedFeatureFlagsEcx.Uint32, NULL);
-if (ExtendedFeatureFlagsEc

[edk2-devel] [PATCH 4/4] UefiCpuPkg: Init new MSR value for MtrrLib Unit Test

2023-06-07 Thread Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394

Using new API GetMaxPlatformAddressBits, MtrrLib Unit Test needs to
provide the value of MSR MSR_IA32_TME_CAPABILITY.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Zhiguang Liu 
---
 UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c | 7 +++
 UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc| 5 -
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c 
b/UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
index ba1de10034..25df09f882 100644
--- a/UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
+++ b/UefiCpuPkg/Library/MtrrLib/UnitTest/Support.c
@@ -18,6 +18,7 @@ MSR_IA32_MTRR_PHYSMASK_REGISTER  
mVariableMtrrsPhysMask[MTRR_NUMBER_
 MSR_IA32_MTRR_DEF_TYPE_REGISTER  mDefTypeMsr;
 MSR_IA32_MTRRCAP_REGISTERmMtrrCapMsr;
 MSR_IA32_TME_ACTIVATE_REGISTER   mTmeActivateMsr;
+MSR_IA32_TME_CAPABILITY_REGISTER mTmeCapabilityMsr;
 CPUID_VERSION_INFO_EDX   mCpuidVersionInfoEdx;
 CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX  mCpuidExtendedFeatureFlagsEcx;
 CPUID_VIR_PHY_ADDRESS_SIZE_EAX   mCpuidVirPhyAddressSizeEax;
@@ -266,6 +267,10 @@ UnitTestMtrrLibAsmReadMsr64 (
 return mTmeActivateMsr.Uint64;
   }
 
+  if (MsrIndex == MSR_IA32_TME_CAPABILITY) {
+return mTmeCapabilityMsr.Uint64;
+  }
+
   //
   // Should never fall through to here
   //
@@ -393,10 +398,12 @@ InitializeMtrrRegs (
 mCpuidExtendedFeatureFlagsEcx.Bits.TME_EN = 1;
 mTmeActivateMsr.Bits.TmeEnable= 1;
 mTmeActivateMsr.Bits.MkTmeKeyidBits   = 
SystemParameter->MkTmeKeyidBits;
+mTmeCapabilityMsr.Bits.MkTmeMaxKeyidBits  = 
SystemParameter->MkTmeKeyidBits;
   } else {
 mCpuidExtendedFeatureFlagsEcx.Bits.TME_EN = 0;
 mTmeActivateMsr.Bits.TmeEnable= 0;
 mTmeActivateMsr.Bits.MkTmeKeyidBits   = 0;
+mTmeCapabilityMsr.Bits.MkTmeMaxKeyidBits  = 0;
   }
 
   return UNIT_TEST_PASSED;
diff --git a/UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc 
b/UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc
index e72e4cd622..8f680ef711 100644
--- a/UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc
+++ b/UefiCpuPkg/Test/UefiCpuPkgHostTest.dsc
@@ -32,7 +32,10 @@
   #
   # Build HOST_APPLICATION that tests the MtrrLib
   #
-  UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf
+  UefiCpuPkg/Library/MtrrLib/UnitTest/MtrrLibUnitTestHost.inf {
+
+  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  }
 
   #
   # Build HOST_APPLICATION that tests the CpuPageTableLib
-- 
2.31.1.windows.1



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




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

2023-06-07 Thread Simon Wang via groups.io
Zhichao,

PR, ShellPkg/SmbiosView: type 45 and type 46 support. by wangsim · Pull Request 
#4482 · tianocore/edk2 (github.com) ( 
https://github.com/tianocore/edk2/pull/4482 )
Just finished CI/CD check.

Thanks,
Simon


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




Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis: Add USB RNDIS devices support

2023-06-07 Thread Wu, Hao A
Thanks.

Please help to:
* Update the DSC file in the last commit (patch 3/3 of the series). Patch 1/3 
only adds NetworkCommon & UsbRndis, this will cause build failure in certain 
scenario.
* Add the Bugzilla link information in the commit log message.

With above handled:
Acked-by: Hao A Wu 

Also sorry for a question:
  MdeModulePkg/Bus/Usb/UsbNetwork/NetworkCommon/NetworkCommon.inf
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcEcm/UsbCdcEcm.inf
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbCdcNcm/UsbCdcNcm.inf
  MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis/UsbRndis.inf
These drivers follow the UEFI Driver Model (install EFI Driver Binding 
Protocol), why they are listed as DXE_DRIVER instead of UEFI_DRIVER in the INF 
files?

Best Regards,
Hao Wu

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> RichardHo [何明忠] via groups.io
> Sent: Thursday, June 8, 2023 11:48 AM
> To: Wu, Hao A ; devel@edk2.groups.io
> Cc: Andrew Fish ; Leif Lindholm
> ; Kinney, Michael D
> ; Michael Kubacki
> ; Liu, Zhiguang ;
> Gao, Liming ; Ni, Ray ; Tinh
> Nguyen ; Rebecca Cran
> ; Tony Lo (�_金松) 
> Subject: Re: [edk2-devel] [PATCH v3 1/3]
> MdeModulePkg/Bus/Usb/UsbNetwork/UsbRndis: Add USB RNDIS devices
> support
> 
> Hi Hao Wu,
> 
> I have created it in 2023-05-07.
> https://bugzilla.tianocore.org/show_bug.cgi?id=4451
> 
> Thanks,
> Richard


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




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

2023-06-07 Thread Gao, Zhichao
It should be same with my PR. But you miss my Reviewed-by.
I just rebase my PR: ShellPkg/SmbiosView: type 45 and type 46 support. by 
ZhichaoGao · Pull Request #4528 · tianocore/edk2 
(github.com)
If you want the maintainer to push your patch directly with your PR. You need 
add the Reviewed-by in you commit message. And the maintainer can easily add 
the push label for CI to auto merge.

Thanks,
Zhichao

From: simowang via groups.io 
Sent: Thursday, June 8, 2023 2:13 PM
To: Gao; Gao, Zhichao ; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] ShellPkg/SmbiosView: type 45 and type 46 
support.

Zhichao,

PR, ShellPkg/SmbiosView: type 45 and type 46 support. by wangsim · Pull Request 
#4482 · tianocore/edk2 (github.com)
Just finished CI/CD check.

Thanks,
Simon


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




Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

2023-06-07 Thread Wu, Hao A
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ard
> Biesheuvel
> Sent: Monday, June 5, 2023 4:32 PM
> To: Ranbir Singh 
> Cc: devel@edk2.groups.io; pedro.falc...@gmail.com; Wu, Hao A
> ; Ni, Ray 
> Subject: Re: [edk2-devel] [PATCH 2/2]
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity
> issue
> 
> On Mon, 5 Jun 2023 at 10:10, Ranbir Singh 
> wrote:
> >
> > I counted myself as not the right person to decide what all to do if Status 
> > is
> not successful. Adding the DEBUG statement from the Coverity aspect
> doesn't count as a fix as RELEASE mode behavior remains the same.
> >
> > In the comments / description, I already mentioned - Assuming, this non-
> usage is deliberate, so as such I did not intend to hide anything - and left 
> it in
> the status quo.
> >
> > The patch proposed may not be appropriate, but should now give a
> thinking point to active module developers / owners / maintainers if they
> indeed feel that there is a bug here and it needs to be fixed.
> >
> 
> Thanks - I agree that it is a good thing that people are aware of this now.


Judging from the context in DetectAndConfigIdeDevice(), I think a fix can be:

  Status = SetDriveParameters (Instance, IdeChannel, IdeDevice, 
&DriveParameters, NULL);
  if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Set drive parameters Fail, Status = %r\n", Status));
continue;
  }

But I do not have the device and platform environment to verify the change.
Really sorry for this. I am not sure on the potential risk of making this 
change without at least some kind of 'no-break' tests.

Best Regards,
Hao Wu


> 
> > On Mon, Jun 5, 2023 at 4:33 AM Ard Biesheuvel  wrote:
> >>
> >> On Sat, 3 Jun 2023 at 18:04, Pedro Falcato 
> wrote:
> >> >
> >> > On Fri, Jun 2, 2023 at 8:42 PM Ranbir Singh 
> wrote:
> >> > >
> >> > > From: Ranbir Singh 
> >> > >
> >> > > The return value stored in Status after call to SetDriveParameters
> >> > > is not made of any use thereafter and hence it remains as UNUSED.
> >> > > Assuming, this non-usage is deliberate, the storage in Status can be
> >> > > done away with.
> >> > >
> >> > > Cc: Hao A Wu 
> >> > > Cc: Ray Ni 
> >> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> >> > > Signed-off-by: Ranbir Singh 
> >> > > ---
> >> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +-
> >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> >> > > index 75403886e44a..c6d637afa989 100644
> >> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> >> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> >> > > @@ -2555,7 +2555,7 @@ DetectAndConfigIdeDevice (
> >> > >DriveParameters.Heads  = (UINT8)(((ATA5_IDENTIFY_DATA
> *)(&Buffer.AtaData))->heads - 1);
> >> > >DriveParameters.MultipleSector = (UINT8)((ATA5_IDENTIFY_DATA
> *)(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt;
> >> > >
> >> > > -  Status = SetDriveParameters (Instance, IdeChannel, IdeDevice,
> &DriveParameters, NULL);
> >> > > +  SetDriveParameters (Instance, IdeChannel, IdeDevice,
> &DriveParameters, NULL);
> >> >
> >> > I'm /fairly/ sure this is wrong and that you need to use Status.
> >> >
> >>
> >> Yeah, removing the assignment fixes the coverity warning, but now you
> >> are hiding a bug instead of fixing it.
> >>
> >> SetDriveParameters () can apparently fail, and this is being ignored.
> >> At the very least, we should emit a diagnostic here in DEBUG mode to
> >> log this. E.g.,
> >>
> >> DEBUG ((DEBUG_WARN, "%a: SetDriveParameters () failed - %r\n",
> >> __func__, Status));
> 
> 
> 
> 



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