Re: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition of FileName on EFI_FILE_INFO

[Ren, Suqiang]

Hi Mike,

Thanks for reviewing. Patch updated here: 
https://edk2.groups.io/g/devel/message/114182.

From my check this change need not update with the same function header.

Can you help to review again?

Thanks
Ren, Suqiang

-Original Message-
From: Kinney, Michael D  
Sent: Tuesday, January 23, 2024 9:10 AM
To: Ren, SuqiangX ; devel@edk2.groups.io
Cc: Gao, Liming ; Liu, Zhiguang 
; Kinney, Michael D 
Subject: RE: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition of 
FileName on EFI_FILE_INFO

Hi Suqiang,

The comment line added look like is exceeds 80 columns.  Please reformat.

Also, there are implementations of this service in the edk2 repo.
Please update those with this same function header update.

Thanks,

Mike

> -Original Message-
> From: Ren, SuqiangX 
> Sent: Thursday, January 11, 2024 1:04 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming 
> ; Liu, Zhiguang 
> Subject: RE: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition 
> of FileName on EFI_FILE_INFO
> 
> Hi All,
> 
>   Any comments about this patch?
> 
> Thanks
> Ren, Suqiang
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ren, 
> Suqiang
> Sent: Tuesday, December 26, 2023 1:22 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming 
> ; Liu, Zhiguang 
> Subject: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition of 
> FileName on EFI_FILE_INFO
> 
> Add the description of FileName to align with UEFI spec 2.10.
> 
> REF: UEFI spec 2.10 Table 13.5.16
> 
> Signed-off-by: Suqiang Ren 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> ---
>  MdePkg/Include/Guid/FileInfo.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Guid/FileInfo.h 
> b/MdePkg/Include/Guid/FileInfo.h index 2b7edf36aabc..c152789b40c8 
> 100644
> --- a/MdePkg/Include/Guid/FileInfo.h
> +++ b/MdePkg/Include/Guid/FileInfo.h
> @@ -46,7 +46,7 @@ typedef struct {
>///
>UINT64  Attribute;
>///
> -  /// The Null-terminated name of the file.
> +  /// The Null-terminated name of the file. For a root directory, the
> name is an empty string.
>///
>CHAR16  FileName[1];
>  } EFI_FILE_INFO;
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 



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

[edk2-devel] [PATCH V3 1/1] MdePkg: Update the definition of FileName on EFI_FILE_INFO

[Ren, Suqiang]

Add the description of FileName[1] to align with UEFI spec 2.10.

REF: UEFI spec 2.10 section 13.5.16

Signed-off-by: Suqiang Ren 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
---
 MdePkg/Include/Guid/FileInfo.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/Guid/FileInfo.h b/MdePkg/Include/Guid/FileInfo.h
index 2b7edf36aabc..71bb289e1256 100644
--- a/MdePkg/Include/Guid/FileInfo.h
+++ b/MdePkg/Include/Guid/FileInfo.h
@@ -47,6 +47,7 @@ typedef struct {
   UINT64  Attribute;
   ///
   /// The Null-terminated name of the file.
+  /// For a root directory, the name is an empty string.
   ///
   CHAR16  FileName[1];
 } EFI_FILE_INFO;
-- 
2.26.2.windows.1



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

Re: [edk2-devel] [RFC PATCH v1 00/20] DynamicTablesPkg: Prepare to add RISC-V support

[Sunil V L]

Hi Sami,

On Mon, Jan 22, 2024 at 05:15:44PM +, Sami Mujawar wrote:
> Hi All,
> 
> DynamicTablesPkg currently supports Arm architecture, and we welcome the 
> adoption by other architectures.
> 
> Following is my proposal for moving forward.
> 
> Goals:
> - reuse common code
> - streamline the adoption by other architectures
> - minimise the impact on migration of the existing platforms
> - maintain flexibility across architectural components
> - use this opportunity to integrate Dynamic SMBIOS support
>   (Ref: https://edk2.groups.io/g/devel/message/107254)
> 
> The following steps would help in achieving the goals:
> 1.  Create an edk2 staging branch. For the edk2-platforms updates, I will 
> create a branch on my Github fork (note this is required as there is no 
> staging repo for edk2-platforms).
> 2. The design aspects and changes shall be discussed on the mailing list with 
> patches to support the details.
> 3. A new section in DynamicTablesPkg\Readme.md shall be added to reflect the 
> design updates, e.g. changes to CM Objects, Namespace definitions, etc.
> 4.  The design changes should typically be supported by patches for the 
> DynamicTables core framework and demonstrate the impact on the existing 
> platform code by typically providing patches for at least one existing 
> platform (possibly edk2-platforms/Platform/ARM/[Juno | FVP]).
> 5. The design changes should be small and typically be reflected in separate 
> patch series.
> 6. The first phase would be to partition the codebase into common code vs 
> architectural specific code. This would involve moving files and reflecting 
> the associated changes such that the build does not break.
> 7. Define a new namespace e.g. “ArchCommon” for the common architectural 
> components.
> 8. Identify the CM_ARM_OBJECTs that can be moved to the “ArchCommon” 
> namespace. As part of this identify if any object needs to be dropped, e.g. 
> EArmObjReserved29
> 9. Identify overlap of SMBIOS objects with existing CM Objects.
> 10. Submit patches to move CM objects from Arm Namespace to ArchCommon 
> Namespace. Ideally one object (and any dependencies) should be moved at a 
> time.
> 11. Submit patches to migrate upstream platforms that use DynamicTablesPkg
> 12. Define a new namespace for RISCV specific objects
> 13. Submit patches for enabling RISCV
> 14. In the next phase support for Dynamic SMBIOS can be enabled.
> 
> Notes: 
> a. Periodically rebase with edk2 & edk2-platforms master branch to sync with 
> latest changes.
> b. We can decide to merge the updates after point 11 above to edk2 & 
> edk2-platforms master branch.
> c. Similarly, the RISCV support can be merged after point 13.
> 
> I will send out a request for creating the staging branch shortly.
> 
This is great!. I think staging branches is a great idea considering the
amount of changes this work needs. Thank you very much!.

I can send small patch sets to partition the code base as per #6 along
with documentation changes. Let me know once staging branches are
created.

Thanks!
Sunil


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

Re: [edk2-devel] [PATCH 25/33] AMD/VanGoghBoard: Check in PlatformInitPei module.

[Chang, Abner via groups.io]

We don't need the space between @ param. Please check it.

Thanks
Abner


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114180): https://edk2.groups.io/g/devel/message/114180
Mute This Topic: https://groups.io/mt/103831198/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] MdePkg/BaseCacheMaintenanceLib: RV64 replace asserts with logs

[Dhaval Sharma]

Sunil,
I thought "WriteBackDataCacheRange not supported" is more explicit over
"CMO not available".

@Pedro Falcato  For the example you mentioned, is
your concern more about someone not being able to notice the problem (that
the system is non-coherent) at the time of development and later ending up
with corrupted data during production? And you are suggesting that an
Assert helps address that problem by making that problem more visible to
the developer and a verbose warning does not?

I can create a patch for CpuFlushCpuDataCache but I think we should avoid
CMO based return in there. Because in case of InvalidateDataCacheRange we
have an alternate implementation of fence in the absence of CMO. So it is
better to let riscvcache decide the right implementation.

=D


On Fri, Jan 19, 2024 at 11:06 AM Sunil V L  wrote:

> On Thu, Jan 18, 2024 at 03:20:18PM +0530, Dhaval wrote:
> > Some platforms do not implement cache management operations. Especially
> > for DMA drivers have code to manage data cache. The code seem to depend
> > on the underlying CPU/cache drivers to enact functionality and simply
> > return if such functionality is not implemented. However this causes
> > issue with CMO implementation which has an assert causing flow to
> > hang within debug environment. While it is not an issue in production
> > environment there is a recommendation to conver this assert in to
> > a harmless logger message. Eventually platform/drivers need to have
> > better guard for such functionality.
> >
> > Signed-off-by: Dhaval Sharma 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Zhiguang Liu 
> > Cc: Sunil V L 
> > Cc: Andrei Warkentin 
> > Cc: Laszlo Ersek 
> > Cc: Pedro Falcato 
> > Cc: Yang Cheng 
> > ---
> >  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 11 ++-
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > index 73a5a6b6b5d6..d99515bcf38b 100644
> > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> > @@ -183,9 +183,8 @@ WriteBackInvalidateDataCache (
> >VOID
> >)
> >  {
> > -  ASSERT (FALSE);
> >DEBUG ((
> > -DEBUG_ERROR,
> > +DEBUG_VERBOSE,
> >  "WriteBackInvalidateDataCache: RISC-V unsupported function.\n"
> >  ));
> >  }
> > @@ -226,7 +225,9 @@ WriteBackInvalidateDataCacheRange (
> >if (RiscVIsCMOEnabled ()) {
> >  CacheOpCacheRange (Address, Length, CacheOpFlush);
> >} else {
> > -ASSERT (FALSE);
> > +DEBUG (
> > +  (DEBUG_VERBOSE, "WriteBackInvalidateDataCacheRange not supported
> \n")
>
> Should this be CMO not enabled?
>
> > +  );
> >}
> >
> >return Address;
> > @@ -248,7 +249,7 @@ WriteBackDataCache (
> >VOID
> >)
> >  {
> > -  ASSERT (FALSE);
> > +  DEBUG ((DEBUG_VERBOSE, "WriteBackDataCache not supported \n"));
> >  }
> >
> >  /**
> > @@ -283,7 +284,7 @@ WriteBackDataCacheRange (
> >if (RiscVIsCMOEnabled ()) {
> >  CacheOpCacheRange (Address, Length, CacheOpClean);
> >} else {
> > -ASSERT (FALSE);
> > +DEBUG ((DEBUG_VERBOSE, "WriteBackDataCacheRange not supported \n"));
> Same comment as earlier.
>
> >}
> >
> >return Address;
> > --
> > 2.39.2
> >
>


-- 
Thanks!
=D


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

Re: [edk2-devel] [PATCH 29/33] AMD/VanGoghBoard: Check in SmramSaveState module.

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Yeah, please check if AMD specific SaveStatelib library under 
UefiCpuPkg/Library/MmSaveStateLib can cover the change or not.

Thanks
Abner

> -Original Message-
> From: Attar, AbdulLateef (Abdul Lateef) 
> Sent: Saturday, January 20, 2024 10:38 PM
> To: devel@edk2.groups.io; Zhai, MingXin (Duke) 
> Cc: Xing, Eric ; Fu, Igniculus ;
> Chang, Abner 
> Subject: RE: [edk2-devel] [PATCH 29/33] AMD/VanGoghBoard: Check in
> SmramSaveState module.
>
> [AMD Official Use Only - General]
>
> Why overriding the PiSmmCpuDxeSmm driver?
> UefiCpuPkg has AMD specific SaveStatelib library
> "UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf".
>
> Thanks
> AbduL
>
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of duke.zhai
> via groups.io
> Sent: Thursday, January 18, 2024 12:21 PM
> To: devel@edk2.groups.io
> Cc: Xing, Eric ; Zhai, MingXin (Duke)
> ; Fu, Igniculus ; Chang,
> Abner 
> Subject: [edk2-devel] [PATCH 29/33] AMD/VanGoghBoard: Check in
> SmramSaveState module.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> From: Duke Zhai 
>
>
> BZ #:4640
>
> Initial SmramSaveState module.
>
> This module provides services to access SMRAM Save State Map.
>
>
>
> Signed-off-by: Ken Yao 
>
> Cc: Eric Xing 
>
> Cc: Duke Zhai 
>
> Cc: Igniculus Fu 
>
> Cc: Abner Chang 
>
> ---
>
>  .../PiSmmCpuDxeSmm/SmramSaveState.c   | 715
> ++
>
>  1 file changed, 715 insertions(+)
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/PiSmmCpuDxeSm
> m/SmramSaveState.c
>
>
>
> diff --git
> a/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/PiSmmCpuDxe
> Smm/SmramSaveState.c
> b/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/PiSmmCpuDxe
> Smm/SmramSaveState.c
>
> new file mode 100644
>
> index 00..9e5a7d59fc
>
> --- /dev/null
>
> +++
> b/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/PiSmmCpuDxe
> Smm/SmramSaveState.c
>
> @@ -0,0 +1,715 @@
>
> +/** @file
>
> +  Implements SmramSaveState.c
>
> +
>
> +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +/* This file includes code originally published under the following license. 
> */
>
> +
>
> +/** @file
>
> +Provides services to access SMRAM Save State Map
>
> +
>
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#include 
>
> +
>
> +#include 
>
> +
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +
>
> +#include "PiSmmCpuDxeSmm.h"
>
> +
>
> +typedef struct {
>
> +  UINT64Signature;  // Offset 0x00
>
> +  UINT16Reserved1;  // Offset 0x08
>
> +  UINT16Reserved2;  // Offset 0x0A
>
> +  UINT16Reserved3;  // Offset 0x0C
>
> +  UINT16SmmCs;  // Offset 0x0E
>
> +  UINT16SmmDs;  // Offset 0x10
>
> +  UINT16SmmSs;  // Offset 0x12
>
> +  UINT16SmmOtherSegment;// Offset 0x14
>
> +  UINT16Reserved4;  // Offset 0x16
>
> +  UINT64Reserved5;  // Offset 0x18
>
> +  UINT64Reserved6;  // Offset 0x20
>
> +  UINT64Reserved7;  // Offset 0x28
>
> +  UINT64SmmGdtPtr;  // Offset 0x30
>
> +  UINT32SmmGdtSize; // Offset 0x38
>
> +  UINT32Reserved8;  // Offset 0x3C
>
> +  UINT64Reserved9;  // Offset 0x40
>
> +  UINT64Reserved10; // Offset 0x48
>
> +  UINT16Reserved11; // Offset 0x50
>
> +  UINT16Reserved12; // Offset 0x52
>
> +  UINT32Reserved13; // Offset 0x54
>
> +  UINT64Reserved14; // Offset 0x58
>
> +} PROCESSOR_SMM_DESCRIPTOR;
>
> +
>
> +extern CONST PROCESSOR_SMM_DESCRIPTOR  gcPsd;
>
> +
>
> +//
>
> +// EFER register LMA bit
>
> +//
>
> +#define LMA  BIT10
>
> +
>
> +///
>
> +/// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_LOOKUP_ENTRY
>
> +///
>
> +#define SMM_CPU_OFFSET(Field)  OFFSET_OF (SMRAM_SAVE_STATE_MAP,
> Field)
>
> +
>
> +///
>
> +/// Macro used to simplify the lookup table entries of type
> CPU_SMM_SAVE_STATE_REGISTER_RANGE
>
> +///
>
> +#define SMM_REGISTER_RANGE(Start, End)  { Start, End, End - Start + 1 }

Re: [edk2-devel] [PATCH 28/33] AMD/VanGoghBoard: Check in SmmCpuFeaturesLibCommon module.

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Please confirm if the latest edk2 SmmCpuFeatureLibCommon.c and 
AmdSmmCpuFeatureLib.c under SmmCpuFeatureLib can cover your changes in this 
patch or not.

Thanks
Abner

> -Original Message-
> From: duke.z...@amd.com 
> Sent: Thursday, January 18, 2024 2:51 PM
> To: devel@edk2.groups.io
> Cc: Xing, Eric ; Yao, Ken ; Fu,
> Igniculus ; Chang, Abner 
> Subject: [PATCH 28/33] AMD/VanGoghBoard: Check in
> SmmCpuFeaturesLibCommon module.
>
> From: Duke Zhai 
>
>
> BZ #:4640
>
> Initial SmmCpuFeaturesLibCommon module. The CPU specific programming
> for
>
> PiSmmCpuDxeSmm module when STM support is not included.
>
>
>
> Signed-off-by: Duke Zhai 
>
> Cc: Eric Xing 
>
> Cc: Ken Yao 
>
> Cc: Igniculus Fu 
>
> Cc: Abner Chang 
>
> ---
>
>  .../SmmCpuFeaturesLibCommon.c | 629 ++
>
>  1 file changed, 629 insertions(+)
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/Library/SmmCpu
> FeaturesLib/SmmCpuFeaturesLibCommon.c
>
>
>
> diff --git
> a/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/Library/SmmCp
> uFeaturesLib/SmmCpuFeaturesLibCommon.c
> b/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/Library/SmmC
> puFeaturesLib/SmmCpuFeaturesLibCommon.c
>
> new file mode 100644
>
> index 00..7b07425336
>
> --- /dev/null
>
> +++
> b/Platform/AMD/VanGoghBoard/Override/edk2/UefiCpuPkg/Library/SmmC
> puFeaturesLib/SmmCpuFeaturesLibCommon.c
>
> @@ -0,0 +1,629 @@
>
> +/** @file
>
> +  Implements AMD SmmCpuFeaturesLibCommon.c
>
> +
>
> +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +/* This file includes code originally published under the following license. 
> */
>
> +/** @file
>
> +Implementation shared across all library instances.
>
> +
>
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
>
> +Copyright (c) Microsoft Corporation.
>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include 
>
> +#include "CpuFeaturesLib.h"
>
> +
>
> +//
>
> +// Machine Specific Registers (MSRs)
>
> +//
>
> +#define  SMM_FEATURES_LIB_IA32_MTRR_CAP0x0FE
>
> +#define  SMM_FEATURES_LIB_IA32_FEATURE_CONTROL 0x03A
>
> +#define  SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE   0x1F2
>
> +#define  SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK   0x1F3
>
> +#define  SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE  0x0A0
>
> +#define  SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK  0x0A1
>
> +#defineEFI_MSR_SMRR_MASK   0xF000
>
> +#defineEFI_MSR_SMRR_PHYS_MASK_VALIDBIT11
>
> +#define  SMM_FEATURES_LIB_SMM_FEATURE_CONTROL  0x4E0
>
> +
>
> +//
>
> +// MSRs required for configuration of SMM Code Access Check
>
> +//
>
> +#define SMM_FEATURES_LIB_IA32_MCA_CAP  0x17D
>
> +#define   SMM_CODE_ACCESS_CHK_BIT  BIT58
>
> +
>
> +extern UINT8  mSmmSaveStateRegisterLma;
>
> +
>
> +//
>
> +// Set default value to assume SMRR is not supported
>
> +//
>
> +BOOLEAN  mSmrrSupported = FALSE;
>
> +
>
> +//
>
> +// Set default value to assume MSR_SMM_FEATURE_CONTROL is not
> supported
>
> +//
>
> +BOOLEAN  mSmmFeatureControlSupported = FALSE;
>
> +
>
> +//
>
> +// Set default value to assume IA-32 Architectural MSRs are used
>
> +//
>
> +UINT32  mSmrrPhysBaseMsr =
> SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
>
> +UINT32  mSmrrPhysMaskMsr =
> SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
>
> +
>
> +//
>
> +// Set default value to assume MTRRs need to be configured on each SMI
>
> +//
>
> +BOOLEAN  mNeedConfigureMtrrs = TRUE;
>
> +
>
> +//
>
> +// Array for state of SMRR enable on all CPUs
>
> +//
>
> +BOOLEAN  *mSmrrEnabled;
>
> +
>
> +/**
>
> +  Performs library initialization.
>
> +
>
> +  This initialization function contains common functionality shared betwen 
> all
>
> +  library instance constructors.
>
> +
>
> +**/
>
> +VOID
>
> +CpuFeaturesLibInitialization (
>
> +  VOID
>
> +  )
>
> +{
>
> +  UINT32  RegEax;
>
> +  UINT32  RegEdx;
>
> +  UINTN   FamilyId;
>
> +  UINTN   ModelId;
>
> +
>
> +  //
>
> +  // Retrieve CPU Family and Model
>
> +  //
>
> +  AsmCpuid (CPUID_VERSION_INFO, , NULL, NULL, );
>
> +  FamilyId = (RegEax >> 8) & 0xf;
>
> +  ModelId  = (RegEax >> 4) & 0xf;
>
> +  if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
>
> +ModelId = ModelId | ((RegEax >> 12) & 0xf0);
>
> +  }
>
> +
>
> +  //
>
> +  // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
>
> +  //
>
> +  if ((RegEdx & BIT12) != 0) {
>
> +//
>
> +// Check MTRR_CAP MSR bit 11 for SMRR support
>
> +//
>
> +if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) !=
> 0) {
>
> +  mSmrrSupported = TRUE;
>
> +}
>
> +  }
>
> +
>
> +  //
>
> +  // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
>
> +  // Volume 3C, Section 35.3 MSRs in the 

Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL

[Xu, GuoX]

Hi Liming,
  I created a PR: https://github.com/tianocore/edk2/pull/5182, could you help 
push it ?

Thanks
Xu Guo

-Original Message-
From: gaoliming  
Sent: Tuesday, January 16, 2024 10:20 PM
To: devel@edk2.groups.io; Xu, GuoX 
Cc: Kinney, Michael D ; Liu, Zhiguang 

Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
EFI_FIRMWARE_MANAGEMENT_PROTOCOL

I am OK for this change. Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Xu, GuoX
> 发送时间: 2024年1月9日 17:24
> 收件人: devel@edk2.groups.io
> 抄送: Kinney, Michael D ; Gao, Liming 
> ; Liu, Zhiguang 
> 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi all, any comments about this patch?
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of GuoX Xu
> Sent: Monday, December 25, 2023 9:21 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming 
> ; Liu, Zhiguang ; 
> Li, Yi1 
> Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of 
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> Add the following sentence at the end of the Image parameter description.
> "May be NULL with a zero ImageSize in order to determine the size of 
> the buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The 
> ImageSize is not too small and Image is NULL."
> 
> 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> Add the following sentence at the end of the ImageInfo parameter 
> description."May be NULL with a zero ImageInfoSize in order to 
> determine the size of the buffer needed".
> 
> Modify the description of "EFI_INVALID_PARAMETER" return code as "The 
> ImageInfoSize is not too small and Image is NULL." and add new 
> descriptions for "EFI_INVALID_PARAMETER" return code.
> 
> REF: UEFI Spec 2.7B Chapter 23.1.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Yi Li 
> Signed-off-by: GuoX Xu 
> ---
>  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> b/MdePkg/Include/Protocol/FirmwareManagement.h
> index f37067df3455..93c8b7658e1a 100644
> --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> @@ -293,6 +293,8 @@ EFI_STATUS
>   to contain the image(s) 
> information if the buffer was too small.
>@param[in, out] ImageInfo  A pointer to the buffer in which
> firmware places the current image(s)
>   information. The information is 
> an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> + May be NULL with a zero
> ImageInfoSize in order to determine the size of the
> + buffer needed.
>@param[out] DescriptorVersion  A pointer to the location in which
> firmware returns the version number
>   associated with the 
> EFI_FIRMWARE_IMAGE_DESCRIPTOR.
>@param[out] DescriptorCountA pointer to the location in
> which firmware returns the number of
> @@ -313,7 +315,12 @@ EFI_STATUS
>@retval EFI_SUCCESSThe device was successfully
> updated with the new image.
>@retval EFI_BUFFER_TOO_SMALL   The ImageInfo buffer was too
> small. The current buffer size
>   needed to hold the image(s) 
> information is returned in ImageInfoSize.
> -  @retval EFI_INVALID_PARAMETER  ImageInfoSize is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is not too small
> and ImageInfo is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and
> DescriptorVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and
> DescriptorCount is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and
> DescriptorSize is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and
> PackageVersion is NULL.
> +  @retval EFI_INVALID_PARAMETER  ImageInfoSize is non-zero and
> PackageVersionName is NULL.
>@retval EFI_DEVICE_ERROR   Valid information could not be
> returned. Possible corrupted image.
> 
>  **/
> @@ -340,6 +347,8 @@ EFI_STATUS
>@param[in]  ImageIndex A unique number identifying the
> firmware image(s) within the device.
>   The number is between 1 and 
> DescriptorCount.
>@param[out] Image  Points to the buffer where the
> current image is copied to.
> + May be NULL with a zero
> ImageSize in order to determine the size of the
> + buffer needed.
>@param[in, out] ImageSize  On entry, points to the size of the
> buffer pointed to by Image, in bytes.
>   On 

Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs

[Feng, Bob C]

Hi Ashraf,

Besides DSC, PCD value also comes from the build tool command line option, DEC 
and INF. This patch looks only check the PCD changes from DSC, it's not cover 
all the cases. 

Thanks,
Bob

-Original Message-
From: Kinney, Michael D  
Sent: Tuesday, January 23, 2024 9:28 AM
To: S, Ashraf Ali ; devel@edk2.groups.io
Cc: Chen, Christine ; Rebecca Cran ; 
Liming Gao ; Feng, Bob C ; 
Chan, Amy ; Chaganty, Rangasai V 
; Solanki, Digant H 
; Kinney, Michael D 
Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue 
and CollectPlatformGuids APIs

Hi Ashraf,

The PcdValueInit feature is not limited to only PCDs of type VPD.  It is for 
any structured PCDs.  Did you test PCDs with all types (e.g. PcdsFixedAtBuild 
PcdsPactahbleInModule, PcdsDynamicHii, PcdsDynamicDatabase, PcdsDynamicVpd, 
PcdsDynamicExHii, PcdsDynamicExDatabase, PcdsDynamicExVpd)

And did you DSC test cases cover both changes in PCD types and changes in PCD 
values?

Thanks,

Mike

> -Original Message-
> From: S, Ashraf Ali 
> Sent: Sunday, January 21, 2024 4:14 AM
> To: Kinney, Michael D ; 
> devel@edk2.groups.io
> Cc: Chen, Christine ; Rebecca Cran 
> ; Liming Gao ; Feng, Bob 
> C ; Chan, Amy ; Chaganty, 
> Rangasai V ; Solanki, Digant H 
> 
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize 
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi., @Kinney, Michael D
> 
> The main API which is modified in this Patch is GenerateByteArrayValue.
> 
> This API is used to return the list of 
> SKUID.TokenSpaceGuid.VpdName|VPD
> STRUCT|Binary Data which will be stored in Output.txt
> Example:
> SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[
> ]|
> {0x01,0x01,0x05,0x09,0x02}
> 
> This VPD/PCD is coming from either the DEC file or the DSC file.
> 
> GenerateByteArrayValue API is used to create the PcdValueInit.exe and 
> it's equivalent C and Make File.
> 
> When there is no change in DEC or DSC VPD/PCD still it used to do the 
> same process again in the incremental build which is time consuming.
> In my project this API is used to take 3min of time with High end 
> server
> (64Threads) and 3.5Min in Laptop (16 threads with performance mode
> enabled) only for GenerateByteArrayValue API.
> 
> This  patch will check if there are any change in the DEC/DSC 
> VPDs/PCDs2, if there is any change in VPD the current flow is not 
> affect. (it will create the PcdRecordList.json and copy Output.txt for 
> Arch folder with the project. Ex:
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordL
> is
> t.json and
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt
> ) If there is no change the DSC/DEC VPDs it will read the data from 
> Output.txt and return the same data. (we will compare the Input 
> structure and PcdRecordList.json) if there is any mismatch then it 
> there change in VPDs if not then no change in VPDs).
> 
> Unit testing from my side as follows:
> * Build the Firmware.
>   * PcdRecordList.json and Output.txt will be created based on the 
> Arch.
> * Build the Firmware again without any change.
>   * It will read the Output.txt and return the data. (it will match 
> Input PCD/VPD list and compare with existing PcdRecordList.json)
>   * I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> * Build the Firmware again my modifying the VPDs in DSC file
>   * Change in VPDs, it will regenerate PcdRecordList.json file
>   *  it will create the Exe file and Output.txt
>   * New Output.txt will be copied to above path.
> * Build the Firmware again by modifying the VPDs in DEC file.
>   * Change in VPDs, it will regenerate PcdRecordList.json file
>   *  it will create the Exe file and Output.txt
>   * New Output.txt will be copied to above path.
> * Build the Firmware again without modifying the code.
>   * It will read the Output.txt and return the data.
>   * I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> 
> 
> There is no impact on the Boot/cache.
> This patch is used to reduce the incremental build time by checking if 
> the VPDs/PCDs are changed or not, if not changed then it will return 
> the previous stored data.
> 
> Thanks.,
> S, Ashraf Ali
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, January 20, 2024 7:36 AM
> To: devel@edk2.groups.io; S, Ashraf Ali 
> Cc: Chen, Christine ; Rebecca Cran 
> ; Liming Gao ; Feng, Bob 
> C ; Chan, Amy ; Chaganty, 
> Rangasai V ; Solanki, Digant H 
> ; Kinney, Michael D 
> 
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize 
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi Ashraf,
> 
> What is captured in the file?
> 
> What PCD/VPD changes will invalidate the cache?  Just the number and 
> type of PCD/VPD elements or their default values/sizes?
> 
> How was this tested?  Were all 

Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg

[Chang, Abner via groups.io]

ok, I got it. I don't have further questions about plan A.

Thanks
Abner


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114174): https://edk2.groups.io/g/devel/message/114174
Mute This Topic: https://groups.io/mt/103679850/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] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Min Xu]

Thanks much Johnson! We will investigate it based on your comments.

> -Original Message-
> From: Brian J. Johnson 
> Sent: Tuesday, January 23, 2024 3:12 AM
> To: devel@edk2.groups.io; kra...@redhat.com; West, Catharine
> 
> Cc: Xu, Min M ; Ni, Ray ; Wu,
> MingliangX ; Yao, Jiewen
> ; Xue, Shengfeng ;
> Dong, Eric ; Kumar, Rahul R
> ; De, Debkumar 
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> Disable should not be set by default in CR0
> 
> On 1/18/24 09:46, Gerd Hoffmann wrote:
> > On Wed, Jan 10, 2024 at 04:43:47PM +, West, Catharine wrote:
> >> Disabling cache by default results in violation of BTG protections (if BTG
> enabled).
> >>
> >> BIOS cannot assume that cache is disabled before it executes as ACM may
> be required to enable NEM.
> >>
> >> Whatever solution needs to be done here cannot evict ACM-enabled
> NEM.
> >
> > Well, it's OVMF in a virtual machine.  No boot guard involved.
> > So we could probably go for a OVMF-specific patch here.
> >
> > But I'd prefer to figure what exactly is happening here before going
> > down that route.  An extreme slowdown just because we flip that bit
> > doesn't make sense to me.
> >
> >> Why is boot time increasing?
> >
> > Not clear.  It seems to be the lzma uncompress of the firmware volume
> > in rom / pflash which is very slow.  Also it is apparently only
> > triggered in case pci device assignment is used.
> 
> I've seen extreme slowness on physical platforms when we've mixed up the
> MTRRs or page tables, causing code to be mapped uncached.
> 
> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
> read uncached.  Lzma probably reads the data a byte at a time, which is the
> worst case for uncached accesses.  Since this is a VM, it's not actually
> uncached at the hardware level, but I don't know how QEMU/KVM handles
> uncached guest mappings It may be doing a VMEXIT for every byte.
> 
> Anyway, I suggest double-checking your page tables and MTRRs.
> --
> Brian J. Johnson
> Enterprise X86 Lab
> Hewlett Packard Enterprise


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

Re: [edk2-devel] [PATCH 07/33] AMD/VanGoghBoard: Check in PciPlatform

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -Original Message-
> From: duke.z...@amd.com 
> Sent: Thursday, January 18, 2024 2:50 PM
> To: devel@edk2.groups.io
> Cc: Xing, Eric ; Yao, Ken ; Fu,
> Igniculus ; Chang, Abner 
> Subject: [PATCH 07/33] AMD/VanGoghBoard: Check in PciPlatform
>
> From: Duke Zhai 
>
>
> BZ #:4640
>
> BIOS detects current IGPU device ID and install corresponding VBIOS.
>
> Inital PciPlatform module to load VBIOS and to provide interface for
>
> other option ROMs if necessary.
>
>
>
> Signed-off-by: Duke Zhai 
>
> Cc: Eric Xing 
>
> Cc: Ken Yao 
>
> Cc: Igniculus Fu 
>
> Cc: Abner Chang 
>
> ---
>
>  .../Include/Protocol/GlobalNvsArea.h  |  70 ++
>
>  .../PciPlatform/CommonHeader.h|  43 
>
>  .../PciPlatform/PciPlatform.c | 199 ++
>
>  .../PciPlatform/PciPlatform.h | 105 +
>
>  .../PciPlatform/PciPlatform.inf   |  66 ++
>
>  5 files changed, 483 insertions(+)
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Include/Protocol/GlobalN
> vsArea.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/CommonHea
> der.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/PciPlatform.c
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/PciPlatform.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/PciPlatform.in
> f
>
>
>
> diff --git
> a/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Include/Protocol/Global
> NvsArea.h
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Include/Protocol/Global
> NvsArea.h
>
> new file mode 100644
>
> index 00..0c5077f417
>
> --- /dev/null
>
> +++
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Include/Protocol/Global
> NvsArea.h
>
> @@ -0,0 +1,70 @@
>
> +/** @file
>
> +  GlobalNvsArea.h
>
> +
>
> +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +**/
>
> +/* This file includes code originally published under the following license. 
> */
>
> +
>
> +/** @file
>
> +Definition of the global NVS area protocol.  This protocol
>
> +publishes the address and format of a global ACPI NVS buffer
>
> +used as a communications buffer between SMM code and ASL code.
>
> +The format is derived from the ACPI reference code, version 0.95.
>
> +Note:  Data structures defined in this protocol are not naturally aligned.
>
> +
>
> +Copyright (c) 2013-2015 Intel Corporation.
>
> +
>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#ifndef _GLOBAL_NVS_AREA_H_


GLOBAL_NVS_AREA_H__


>
> +#define _GLOBAL_NVS_AREA_H_
>
> +
>
> +//
>
> +// Includes
>
> +//
>
> +#define GLOBAL_NVS_DEVICE_ENABLE   1
>
> +#define GLOBAL_NVS_DEVICE_DISABLE  0
>
> +
>
> +//
>
> +// Global NVS Area Protocol GUID
>
> +//
>
> +#define EFI_GLOBAL_NVS_AREA_PROTOCOL_GUID \
>
> +{ 0x74e1e48, 0x8132, 0x47a1, {0x8c, 0x2c, 0x3f, 0x14, 0xad, 0x9a, 0x66,
> 0xdc} }
>
> +
>
> +//
>
> +// Revision id - Added TPM related fields
>
> +//
>
> +#define GLOBAL_NVS_AREA_RIVISION_1  1
>
> +
>
> +//
>
> +// Extern the GUID for protocol users.
>
> +//
>
> +extern EFI_GUID  gEfiGlobalNvsAreaProtocolGuid;
>
> +
>
> +//
>
> +// Global NVS Area definition
>
> +//
>
> +#pragma pack (1)
>
> +typedef struct {
>
> +  //
>
> +  // Miscellaneous Dynamic Values, the definitions below need to be matched
>
> +  // GNVS definitions in Platform.ASL
>
> +  //
>
> +  UINT32TopOfMem;   // TOPM
>
> +  UINT8 NbIoApic;   // NAPC
>
> +  UINT32PcieBaseAddress;// PCBA
>
> +  UINT32PcieBaseLimit;  // PCBL
>
> +} EFI_GLOBAL_NVS_AREA;
>
> +#pragma pack ()
>
> +
>
> +//
>
> +// Global NVS Area Protocol
>
> +//
>
> +typedef struct _EFI_GLOBAL_NVS_AREA_PROTOCOL {
>
> +  EFI_GLOBAL_NVS_AREA*Area;
>
> +} EFI_GLOBAL_NVS_AREA_PROTOCOL;
>
> +
>
> +#endif
>
> diff --git
> a/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/CommonHe
> ader.h
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/CommonH
> eader.h
>
> new file mode 100644
>
> index 00..430d9f51dc
>
> --- /dev/null
>
> +++
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/PciPlatform/CommonH
> eader.h
>
> @@ -0,0 +1,43 @@
>
> +/** @file
>
> +  Implements CommonHeader.h
>
> +
>
> +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +/* This file includes code originally published under the following license. 
> */
>
> +
>
> +/** @file
>
> +Common header file shared by all source files.
>
> +
>
> +This file includes package header files, library classes and protocol, PPI &
> GUID definitions.

Remove below copyright, put Intel's above or under AMD copyright.



>
> +
>
> +Copyright (c) 2013-2015 Intel Corporation.
>
> +
>
> +This  program and the accompanying materials
>
> +are licensed and made 

Re: [edk2-devel] [PATCH 05/33] AMD/VanGoghBoard: Check in PlatformSecLib

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -Original Message-
> From: duke.z...@amd.com 
> Sent: Thursday, January 18, 2024 2:50 PM
> To: devel@edk2.groups.io
> Cc: Zhai, MingXin (Duke) ; Xing, Eric
> ; Fu, Igniculus ; Chang, Abner
> 
> Subject: [PATCH 05/33] AMD/VanGoghBoard: Check in PlatformSecLib
>
> From: Duke Zhai 
>
>
> BZ #:4640
>
> Chachani board jump to PlatformSec function after x86 releasing.
>
> This module provides the SEC entry function, which does platform-related
>
> early initialization.
>
>
>
> Signed-off-by: Ken Yao 
>
> Cc: Duke Zhai 
>
> Cc: Eric Xing 
>
> Cc: Igniculus Fu 
>
> Cc: Abner Chang 
>
> ---
>
>  .../Library/PlatformSecLib/Ia32/Flat32.nasm   | 551 ++
>
>  .../Library/PlatformSecLib/Ia32/Platform.inc  |  69 +++
>
>  .../Library/PlatformSecLib/PlatformSecLib.c   | 208 +++
>
>  .../Library/PlatformSecLib/PlatformSecLib.inf |  68 +++
>
>  .../PlatformSecLib/PlatformSecLibModStrs.uni  |  28 +
>
>  5 files changed, 924 insertions(+)
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/Ia
> 32/Flat32.nasm
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/Ia
> 32/Platform.inc
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/Pl
> atformSecLib.c
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/Pl
> atformSecLib.inf
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/Pl
> atformSecLibModStrs.uni
>
>
>
> diff --git
> a/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/I
> a32/Flat32.nasm
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/
> Ia32/Flat32.nasm
>
> new file mode 100644
>
> index 00..b11f45768c
>
> --- /dev/null
>
> +++
> b/Platform/AMD/VanGoghBoard/ChachaniBoardPkg/Library/PlatformSecLib/
> Ia32/Flat32.nasm
>
> @@ -0,0 +1,551 @@
>
> +;/** @file
>
> +; AMD VanGoghBoard PlatformSecLib
>
> +;
>
> +; Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +;
>
> +;**/
>
> +
>
> +; This file includes code originally published under the following license.
>
Same here.
We should remove below Intel copyright block and put below above or below AMD 
copy right.
Copyright (c) 2013-2015 Intel Corporation.

Please check the similar instances in this patch set.
Thanks
Abner



> +;--
>
> +;
>
> +; Copyright (c) 2013-2015 Intel Corporation.
>
> +;
>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +;
>
> +; Module Name:
>
> +;
>
> +;  Flat32.asm
>
> +;
>
> +; Abstract:
>
> +;
>
> +;  This is the code that goes from real-mode to protected mode.
>
> +;  It consumes the reset vector, configures the stack.
>
> +;
>
> +;
>
> +;--
>
> +
>
> +
>
> +;
>
> +; Include processor definitions
>
> +;
>
> +%use masm
>
> +
>
> +
>
> +%include "Platform.inc"
>
> +
>
> +;
>
> +; CR0 cache control bit definition
>
> +;
>
> +CR0_CACHE_DISABLE   EQU 04000h
>
> +CR0_NO_WRITEEQU 02000h
>
> +BSP_STACK_BASE_ADDR EQU FixedPcdGet32
> (PcdPeiCorePeiPreMemoryStackBaseAddress) ; Base address for core 0 stack
>
> +PRE_MEM_STACK_SIZE  EQU FixedPcdGet32
> (PcdPeiCorePeiPreMemoryStackSize)
>
> +PCIEX_LENGTH_BIT_SETTING EQU 011000b
>
> +
>
> +MSR_IA32_EFER   EQU  0c080h   ; Extended Feature Enable
> Register
>
> +MSR_IA32_EFER_LME   EQU  8; Long Mode Enable
>
> +
>
> +MSR_SMM_BASEEQU  0c0010111h   ; SMBASE Register
>
> +
>
> +SMM_BASE_DEFAULTEQU  3h   ; reset value of MSR
> MSR_SMM_BASE
>
> +
>
> +SMMMASK_ADDRESS EQU  0c0010113h   ; SMM TSeg Base Address
>
> +SMMMASK_ADDRESS_AE  EQU  0; Aseg Address Range Enable
>
> +SMMMASK_ADDRESS_TE  EQU  1; Tseg Address Range Enable
>
> +
>
> +;
>
> +; In Modified Conventional Resume S3 Design:
>
> +;   With Modified Conventional Resume path, the x86 resumes from sleep,
>
> +; begins executing code from a predefined SMM resume vector and then
>
> +; jump to ROM code to continue conventional resume.
>
> +; EDX is filled with special signature "0x55AABB66" when jump to Sec,
>
> +; this signature can be used to identify if resume back from SMM resume.
>
> +;
>
> +SMM_RESUME_SIGNATUREEQU  055AABB66h
>
> +
>
> +PCAT_RTC_ADDRESS_REGISTER  EQU  0x70
>
> +PCAT_RTC_DATA_REGISTER EQU  0x71
>
> +
>
> +NMI_DISABLE_BIT EQU  0x80
>
> +
>
> +RTC_ADDRESS_REGISTER_A  EQU  0x0A  ; R/W[0..6]  R0[7]
>
> +RTC_ADDRESS_REGISTER_B  EQU  0x0B  ; R/W
>
> +RTC_ADDRESS_REGISTER_C  EQU  0x0C  ; RO
>
> +RTC_ADDRESS_REGISTER_D  EQU  0x0D  ; R/W
>
> +
>
> +;
>
> +; External and public declarations
>
> +;  TopOfStack is used by C code
>
> +;  SecStartup 

Re: [edk2-devel] [PATCH 04/33] AMD/VanGoghBoard: Check in AgesaPublic pkg

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Please review all C header files in this patch set. Remove the leading 
underscore and use double underscore at trailing.
For example,
_AGESA_H_ -> AGESA_H__

Thanks
Abner

> -Original Message-
> From: duke.z...@amd.com 
> Sent: Thursday, January 18, 2024 2:50 PM
> To: devel@edk2.groups.io
> Cc: Xing, Eric ; Yao, Ken ; Fu,
> Igniculus ; Chang, Abner 
> Subject: [PATCH 04/33] AMD/VanGoghBoard: Check in AgesaPublic pkg
>
> From: Duke Zhai 
>
>
> BZ #:4640
>
> Chachani board platform code depends on some AGESA-related PCDs/GUIDs.
>
> Add AgesaPublicPkg for AGESA-related PCDs/GUIDs to support platfrom build.
>
>
>
> Signed-off-by: Duke Zhai 
>
> Cc: Eric Xing 
>
> Cc: Ken Yao 
>
> Cc: Igniculus Fu 
>
> Cc: Abner Chang 
>
> ---
>
>  .../VanGoghBoard/AgesaPublic/AgesaPublic.dec  |  61 +
>
>  .../VanGoghBoard/AgesaPublic/Include/AGESA.h  |  35 +++
>
>  .../VanGoghBoard/AgesaPublic/Include/AMD.h| 189 +
>
>  .../AgesaPublic/Include/AmdPspDirectory.h |  55 
>
>  .../AgesaPublic/Include/FchRegistersCommon.h  |  23 ++
>
>  .../Include/Guid/AmdMemoryInfoHob.h   |  51 
>
>  .../Include/Library/AmdPspBaseLibV2.h | 248 ++
>
>  .../Include/Library/AmdPspCommonLib.h |  29 ++
>
>  .../Include/Library/AmdPspFtpmLib.h   |  94 +++
>
>  .../AgesaPublic/Include/Ppi/AmdPspFtpmPpi.h   |  80 ++
>
>  .../Include/Protocol/AmdPspFtpmProtocol.h | 112 
>
>  11 files changed, 977 insertions(+)
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/AgesaPublic.dec
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/AGESA.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/AMD.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/AmdPspDirectory.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/FchRegistersCommon.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Guid/AmdMemoryInfoH
> ob.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Library/AmdPspBaseLibV
> 2.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Library/AmdPspCommo
> nLib.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Library/AmdPspFtpmLib.
> h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Ppi/AmdPspFtpmPpi.h
>
>  create mode 100644
> Platform/AMD/VanGoghBoard/AgesaPublic/Include/Protocol/AmdPspFtpmPr
> otocol.h
>
>
>
> diff --git a/Platform/AMD/VanGoghBoard/AgesaPublic/AgesaPublic.dec
> b/Platform/AMD/VanGoghBoard/AgesaPublic/AgesaPublic.dec
>
> new file mode 100644
>
> index 00..e987b9b603
>
> --- /dev/null
>
> +++ b/Platform/AMD/VanGoghBoard/AgesaPublic/AgesaPublic.dec
>
> @@ -0,0 +1,61 @@
>
> +## @file
>
> +# EDK II AgesaPublic.dec file
>
> +#
>
> +# Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
>
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +##
>
> +
>
> +[Defines]
>
> +  DEC_SPECIFICATION  = 0x00010005
>
> +  PACKAGE_NAME   = AgesaPublic
>
> +  PACKAGE_GUID   = EA54B0FA-908C-43DE-95A5-5E821A893CA4
>
> +  PACKAGE_VERSION= 0.1
>
> +
>
> +[Includes]
>
> +  Include
>
> +
>
> +[Guids]
>
> +  gEfiAmdAgesaModulePkgTokenSpaceGuid  = { 0x7788adf0, 0x9788,
> 0x4a3f, { 0x83, 0xfa, 0xcb, 0x51, 0x2e, 0x7c, 0xf8, 0xdd } }
>
> +  gEfiAmdAgesaPkgTokenSpaceGuid= { 0xd4d8435f, 0xfffb, 0x4acb,
> { 0xa0, 0x4d, 0xff, 0x0f, 0xad, 0x67, 0x7f, 0xe9 } }
>
> +  gAmdCpmPkgTokenSpaceGuid = { 0x916e0ddd, 0x2bd2, 0x4704,
> { 0x93, 0xb9, 0x59, 0x4b, 0x01, 0xa5, 0xfa, 0x9f } }
>
> +  gAmdResourceSizeForEachRbGuid= { 0x542b8f2f, 0xbd52, 0x4233,
> { 0x8c, 0x3d, 0x66, 0x53, 0x0d, 0xe8, 0xa3, 0x69 } }
>
> +  gAmdPbsSystemConfigurationGuid   = { 0xa339d746, 0xf678, 0x49b3,
> { 0x9f, 0xc7, 0x54, 0xce, 0x0f, 0x9d, 0xf2, 0x26 } }
>
> +  gAmdTotalNumberOfRootBridgesGuid = { 0xfb5703f5, 0xf8a7, 0xf401,
> { 0x18, 0xb4, 0x3f, 0x10, 0x8d, 0xeb, 0x26, 0x12 } }
>
> +  gApSyncFlagNvVariableGuid= { 0xad3f6761, 0xf0a3, 0x46c8, { 
> 0xa4,
> 0xcb, 0x19, 0xb7, 0x0f, 0xfd, 0xb3, 0x05 } }
>
> +  gAmdMemoryInfoHobGuid= { 0x1bce3d14, 0xa5fe, 0x4a0b,
> { 0x9a, 0x8d, 0x69, 0xca, 0x5d, 0x98, 0x38, 0xd3 } }
>
> +  gAmdPspApobHobGuid   = { 0x30b174f3, 0x7712, 0x4cca, { 
> 0xbd,
> 0x13, 0xd0, 0xb8, 0xa8, 0x80, 0x19, 0x97 } }
>
> +
>
> +[Protocols]
>
> +  gPspFlashAccSmmCommReadyProtocolGuid = { 0x9f373486, 0xda76,
> 0x4c9f, { 0x81, 0x55, 0x6c, 0xcd, 0xdb, 0x0b, 0x0b, 0x04 } }
>
> +  gAmdPspFtpmProtocolGuid  = { 0xac234e04, 0xb036, 0x476c,
> { 0x91, 0x66, 0xbe, 0x47, 0x52, 0xa0, 0x95, 0x09 } }
>
> +  gFchInitDonePolicyProtocolGuid   = { 0xc63c0c73, 0xf612, 0x4c02,
> { 0x84, 0xa3, 0xc6, 0x40, 0xad, 0x0b, 0xa6, 0x22 } }
>
> +  

Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg

[Chao Li]

Hi Abner,

The ExceptionLib is different from other libs such as Mp and Timer. 
Since ExceptionLib can provide for 32-bit service for LoongArch32 in the 
future, 64-bit private files are located in LoongArch/LoongArch64/ and 
the 32-bit will be located in LoongArch/LoongArch32, although the 32-bit 
LoongArch is not upstream yet.



Thanks,
Chao
On 2024/1/23 11:10, Chang, Abner wrote:

[AMD Official Use Only - General]

HI all,
I have no problem with the plan A, except the folder structure under 
CpuExecptionHandlerLib.
It has a LoongArch folder that contains the common source files for LoongArch 
and LoongArch64 folder under LoongArch for 64-buite architecture. This folder 
structure is different from other lib such as MpInitLib and CpuTimerLib.
Do you have 32-bit LoongArch? If not for now, then you probably can move the 
files under LoongArch to LoongArch64. Len you can a consistent folder structure 
for LoongArch.

Thanks
Abner


-Original Message-
From: Laszlo Ersek
Sent: Friday, January 19, 2024 9:48 PM
To: Ni, Ray;devel@edk2.groups.io;lic...@loongson.cn;
Sunil V L
Cc: Kinney, Michael D; Gerd Hoffmann
; Chang, Abner
Subject: Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg

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


On 1/19/24 11:17, Ni, Ray wrote:

Chao,



In the plan A, CpuDxe.inf contains reference to
gUefiCpuPkgTokenSpaceGuid.PcdCpuExceptionVectorBaseAddress in [Pcd]
section. But I guess it’s only needed by LoongArch64. That’s why I
didn’t like the common-inf idea.



But after looking at the other INF changes, I changed my mind. I think
the common-inf looks good as the other INF files don’t have so much
difference between LoonArch and x86.



So, I am ok with plan A. Thanks for preparing both changes for review.



Mike, Laszlo, Gerd, Abner, any comments?

I'm also OK with plan A.

Thanks
Laszlo






Thanks,

Ray

*From:* Chao Li
*Sent:* Thursday, January 18, 2024 4:27 PM
*To:* Ni, Ray; Sunil V L
*Cc:*devel@edk2.groups.io; Kinney, Michael D
; Laszlo Ersek; Gerd
Hoffmann; Abner Chang
*Subject:* Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg



Hi Ray and Sunil,

Sorry, I'm late, I have very busy these days.

I created two PRs in my private repo.

*Plan A:*

Moved most of LoongArch libraries and drivers to current matching
folders, removed LoongArch private INF, adjusted common INF.

URL:https://github.com/kilaterlee/edk2/pull/8




*Plan B:*

Moved most of LoongArch libraries and drivers to current matching
folders, keeping to use LoongArch private INF.

URL:https://github.com/kilaterlee/edk2/pull/9




After you reading these two PRs, let's discuss how to deal with INF.

I prefer plan A, it is better to use the same INF for all ARCHs and have
only one INF per modules. I was discussed this plan with Ray and Abner,
but it was not allowed at the time.



Thanks,
Chao

On 2024/1/16 08:59, Chao Li wrote:

 OK, let me finish this work tomorrow, I'm a little busy today.

 On 2024/1/15 16:11, Ni, Ray wrote:

 Yes. Fine to me. Thanks!



 Thanks,

 Ray

 -Original Message-

 From: Sunil V L



 Sent: Monday, January 15, 2024 2:46 PM

 To: Chao Li  

 Cc:devel@edk2.groups.io  ; Ni, Ray

  ; Kinney, Michael D

   ;

Laszlo Ersek  ; Gerd

 Hoffmann  

 Subject: Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg



 On Mon, Jan 15, 2024 at 02:17:09PM +0800, Chao Li wrote:

 Ray and Sunil,



 I plan send two example PRs to github to  show how to deal the

INFs, one to

 adjust current INF, and another PR to move the libraris to the 
current

 folders. I hope after these two PRs are sent we can decide 
what to do

with

 the INF. What do you think of this plan?





 Hi Chao, That's fine with me.



 Thanks,

 Sunil

 Thanks,

 Chao

 On 2024/1/15 13:44, Ni, Ray wrote:

 That will be great!



 Thanks,

 Ray

 -Original Message-

 From: Sunil V L



 Sent: Monday, January 15, 2024 12:44 PM

 To: Ni, Ray  


 Cc:devel@edk2.groups.io;lic...@loongson.cn

; Kinney, Michael D

 

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Huang, Jenny]

Reviewed by Jenny Huang 

-Original Message-
From: Sheng, W  
Sent: Sunday, January 21, 2024 10:47 PM
To: devel@edk2.groups.io
Cc: Ni, Ray ; Huang, Jenny ; Chiang, 
Chris 
Subject: [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

PciIoMap () need to feedback the status of
mIoMmuProtocol->SetAttribute () return value.

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

Cc: Ray Ni 
Cc: Huang Jenny 
Cc: Chiang Chris 
Signed-off-by: Sheng Wei 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 14bed54729..e85544d08d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1024,12 +1024,12 @@ PciIoMap (
   return EFI_INVALID_PARAMETER;
   }
 
-  mIoMmuProtocol->SetAttribute (
-mIoMmuProtocol,
-PciIoDevice->Handle,
-*Mapping,
-IoMmuAttribute
-);
+  Status = mIoMmuProtocol->SetAttribute (
+ mIoMmuProtocol,
+ PciIoDevice->Handle,
+ *Mapping,
+ IoMmuAttribute
+ );
 }
   }
 
-- 
2.26.2.windows.1



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

Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

HI all,
I have no problem with the plan A, except the folder structure under 
CpuExecptionHandlerLib.
It has a LoongArch folder that contains the common source files for LoongArch 
and LoongArch64 folder under LoongArch for 64-buite architecture. This folder 
structure is different from other lib such as MpInitLib and CpuTimerLib.
Do you have 32-bit LoongArch? If not for now, then you probably can move the 
files under LoongArch to LoongArch64. Len you can a consistent folder structure 
for LoongArch.

Thanks
Abner

> -Original Message-
> From: Laszlo Ersek 
> Sent: Friday, January 19, 2024 9:48 PM
> To: Ni, Ray ; devel@edk2.groups.io; lic...@loongson.cn;
> Sunil V L 
> Cc: Kinney, Michael D ; Gerd Hoffmann
> ; Chang, Abner 
> Subject: Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 1/19/24 11:17, Ni, Ray wrote:
> > Chao,
> >
> >
> >
> > In the plan A, CpuDxe.inf contains reference to
> > gUefiCpuPkgTokenSpaceGuid.PcdCpuExceptionVectorBaseAddress in [Pcd]
> > section. But I guess it’s only needed by LoongArch64. That’s why I
> > didn’t like the common-inf idea.
> >
> >
> >
> > But after looking at the other INF changes, I changed my mind. I think
> > the common-inf looks good as the other INF files don’t have so much
> > difference between LoonArch and x86.
> >
> >
> >
> > So, I am ok with plan A. Thanks for preparing both changes for review.
> >
> >
> >
> > Mike, Laszlo, Gerd, Abner, any comments?
>
> I'm also OK with plan A.
>
> Thanks
> Laszlo
>
> >
> >
> >
> >
> >
> > Thanks,
> >
> > Ray
> >
> > *From:* Chao Li 
> > *Sent:* Thursday, January 18, 2024 4:27 PM
> > *To:* Ni, Ray ; Sunil V L 
> > *Cc:* devel@edk2.groups.io; Kinney, Michael D
> > ; Laszlo Ersek ; Gerd
> > Hoffmann ; Abner Chang 
> > *Subject:* Re: [edk2-devel] RFC: Folder layout change in UefiCpuPkg
> >
> >
> >
> > Hi Ray and Sunil,
> >
> > Sorry, I'm late, I have very busy these days.
> >
> > I created two PRs in my private repo.
> >
> > *Plan A:*
> >
> > Moved most of LoongArch libraries and drivers to current matching
> > folders, removed LoongArch private INF, adjusted common INF.
> >
> > URL: https://github.com/kilaterlee/edk2/pull/8
> > 
> >
> >
> >
> > *Plan B:*
> >
> > Moved most of LoongArch libraries and drivers to current matching
> > folders, keeping to use LoongArch private INF.
> >
> > URL: https://github.com/kilaterlee/edk2/pull/9
> > 
> >
> >
> >
> > After you reading these two PRs, let's discuss how to deal with INF.
> >
> > I prefer plan A, it is better to use the same INF for all ARCHs and have
> > only one INF per modules. I was discussed this plan with Ray and Abner,
> > but it was not allowed at the time.
> >
> >
> >
> > Thanks,
> > Chao
> >
> > On 2024/1/16 08:59, Chao Li wrote:
> >
> > OK, let me finish this work tomorrow, I'm a little busy today.
> >
> > On 2024/1/15 16:11, Ni, Ray wrote:
> >
> > Yes. Fine to me. Thanks!
> >
> >
> >
> > Thanks,
> >
> > Ray
> >
> > -Original Message-
> >
> > From: Sunil V L 
> 
> >
> > Sent: Monday, January 15, 2024 2:46 PM
> >
> > To: Chao Li  
> >
> > Cc: devel@edk2.groups.io ; Ni, Ray
>  ; Kinney, Michael D
> >
> >  
> > ;
> Laszlo Ersek  ; Gerd
> >
> > Hoffmann  
> >
> > Subject: Re: [edk2-devel] RFC: Folder layout change in 
> > UefiCpuPkg
> >
> >
> >
> > On Mon, Jan 15, 2024 at 02:17:09PM +0800, Chao Li wrote:
> >
> > Ray and Sunil,
> >
> >
> >
> > I plan send two example PRs to github to  show how to deal 
> > the
> INFs, one to
> >
> > adjust current INF, and another PR to move the libraris to 
> > the current
> >
> > folders. I hope after these two PRs are sent we can decide 
> > what to do
> with
> >
> > the INF. What do you think of this plan?
> >
> >
> >
> >
> >
> > Hi Chao, That's fine with me.
> >
> >
> >
> > Thanks,
> >
> > Sunil
> >
> > Thanks,
> >
> > Chao
> >
> > On 2024/1/15 13:44, Ni, Ray wrote:
> >
> > That will be great!
> >
> >
> >
> > Thanks,
> >
> > Ray
> >
> > -Original Message-
> >
> > From: Sunil V L
> 
> >
> > Sent: Monday, January 15, 2024 12:44 PM
> >
> > To: Ni, Ray 
> > 

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, January 23, 2024 #cal-reminder

[Group Notification]

*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, January 23, 2024
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

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

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

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

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

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

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

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

*Or call in (audio only)*

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

Phone Conference ID: 774 638 21#

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

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114165): https://edk2.groups.io/g/devel/message/114165
Mute This Topic: https://groups.io/mt/103902241/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] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/22/2024 6:53 PM, Oliver Smith-Denny wrote:


I was able to repro your bug (by just turning on page guards on
ArmVirtQemu, allocating runtime mem and freeing it). I think you
are the first person to free runtime mem on ARM64 with page guards
enabled (and to care when it failed :).

The heap guard code is not written with ARM64 in mind (nor is much of
the codebase, of course). Specifically in this case the heap guard code
only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
runtime page granularity required.

Let me take a look at this, I'm working on a solution here, but I want
to test this out further. I'll try to send a patch later this week or
next.


Thanks! I wonder if the same problem occurs on LoongArch64, which also 
defines the runtime page allocation granularity to be 0x1?



MdePkg/Include/X64/ProcessorBind.h
261:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/LoongArch64/ProcessorBind.h
89:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1)

MdePkg/Include/RiscV64/ProcessorBind.h
120:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ia32/ProcessorBind.h
262:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/AArch64/ProcessorBind.h
164:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1)

MdePkg/Include/Arm/ProcessorBind.h
170:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ebc/ProcessorBind.h
125:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

--
Rebecca Cran


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

[edk2-devel] [Patch 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB

[Michael D Kinney]

Provide an optional method for PEI to declare a specific address
range to use for the Memory Type Information bins. The current
algorithm uses heuristics that tends to place the Memory Type
Information bins in the same location, but memory configuration
changes across boots or algorithm changes across a firmware
updates could potentially change the Memory Type Information bin
location.

If the HOB List contains a Resource Descriptor HOB that
describes tested system memory and has an Owner GUID of
gEfiMemoryTypeInformationGuid, then use the address range
described by the Resource Descriptor HOB as the preferred
location of the Memory Type Information bins. If this HOB is
not detected, then the current behavior is preserved.

The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
is ignored for the following conditions:
* The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
  is smaller than the Memory Type Information bins.
* The HOB list contains more than one Resource Descriptor HOB
  with an owner GUID of gEfiMemoryTypeInformationGuid.
* The Resource Descriptor HOB with an Owner GUID of
  gEfiMemoryTypeInformationGuid is the same Resource Descriptor
  HOB that that describes the PHIT memory range.

Update the DxeMain initialization order to initialize GCD
services before any runtime allocations are performed.  This
is required to prevent runtime data fragmentation when the
UEFI System Table and UEFI Runtime Service Table is allocated.

Cc: Liming Gao 
Cc: Aaron Li 
Cc: Liu Yun 
Cc: Andrew Fish 
Signed-off-by: Michael D Kinney 
---
 MdeModulePkg/Core/Dxe/DxeMain.h |  6 ++
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 23 +++---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 65 +++-
 MdeModulePkg/Core/Dxe/Mem/Page.c| 99 +
 4 files changed, 182 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 86a7be2f5188..53e26703f8c7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -277,6 +277,12 @@ CoreInitializePool (
   VOID
   );
 
+VOID
+CoreSetMemoryTypeInformationRange (
+  IN EFI_PHYSICAL_ADDRESS  Start,
+  IN UINT64Length
+  );
+
 /**
   Called to initialize the memory map and add descriptors to
   the current descriptor list.
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c 
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 0e0f9769b99d..17d510a287e5 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -276,6 +276,18 @@ DxeMain (
 
   MemoryProfileInit (HobStart);
 
+  //
+  // Start the Image Services.
+  //
+  Status = CoreInitializeImageServices (HobStart);
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Initialize the Global Coherency Domain Services
+  //
+  Status = CoreInitializeGcdServices (, MemoryBaseAddress, 
MemoryLength);
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Allocate the EFI System Table and EFI Runtime Service Table from 
EfiRuntimeServicesData
   // Use the templates to initialize the contents of the EFI System Table and 
EFI Runtime Services Table
@@ -289,16 +301,9 @@ DxeMain (
   gDxeCoreST->RuntimeServices = gDxeCoreRT;
 
   //
-  // Start the Image Services.
+  // Update DXE Core Loaded Image Protocol with allocated UEFI System Table
   //
-  Status = CoreInitializeImageServices (HobStart);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Initialize the Global Coherency Domain Services
-  //
-  Status = CoreInitializeGcdServices (, MemoryBaseAddress, 
MemoryLength);
-  ASSERT_EFI_ERROR (Status);
+  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
 
   //
   // Call constructor for all libraries
diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index 792cd2e0af23..189a87470251 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
   EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
   EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
   EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
+  EFI_HOB_RESOURCE_DESCRIPTOR  *MemoryTypeInformationResourceHob;
+  UINTNCount;
   EFI_PHYSICAL_ADDRESS BaseAddress;
   UINT64   Length;
   UINT64   Attributes;
@@ -2289,12 +2291,42 @@ CoreInitializeMemoryServices (
   //
   // See if a Memory Type Information HOB is available
   //
+  MemoryTypeInformationResourceHob = NULL;
   GuidHob = GetFirstGuidHob ();
   if (GuidHob != NULL) {
 EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
 DataSize = GET_GUID_HOB_DATA_SIZE (GuidHob);
 if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) && (DataSize <= 
(EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
   CopyMem (, EfiMemoryTypeInformation, DataSize);
+
+  //
+  // Look for Resource Descriptor HOB with a ResourceType of System Memory
+  // and an Owner GUID of 

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/22/2024 2:06 PM, Rebecca Cran via groups.io wrote:

On 1/19/2024 1:03 PM, Oliver Smith-Denny wrote:


Thanks for trying. In lieu of being able to test myself, all I can offer
is adding some more prints, when the memory gets allocated, making sure
it is 64k aligned then. I'd be curious to see what the address is that
is attempting to be freed.

My guess (as it was earlier) is that it is going to be aligned to
64k but + 4k. I.e the guard page at the front is throwing it off. There
may have just been an error in my attempt to fix the check for that.

If however that address is not 64k + 4k aligned, then something else is
afoot.

Happy to look at some more data if you get it or can engineer an example
on an open source system (can you force the system to call this function
twice even without the extra SMBIOS entries, etc.).


These are the addresses it's allocating with and without HeapGuard (with 
the original, upstream Page.c code).



Without HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xFB7C)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xEF11, 1)
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (4153), 0xFB8AEC8E)


--

WITH HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xED36F000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xED38F000)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xED36F000, 1)




I was able to repro your bug (by just turning on page guards on
ArmVirtQemu, allocating runtime mem and freeing it). I think you
are the first person to free runtime mem on ARM64 with page guards
enabled (and to care when it failed :).

The heap guard code is not written with ARM64 in mind (nor is much of
the codebase, of course). Specifically in this case the heap guard code
only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
runtime page granularity required.

Let me take a look at this, I'm working on a solution here, but I want
to test this out further. I'll try to send a patch later this week or
next.

Oliver


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

Re: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of HiiConfigAccess ExtractConfig

[Michael D Kinney]

Hi Tan Ming,

In this case, the function header in the implementation needs to be
updated to match the MdePkg header file changes.

I agree that the device error would have to propagate from the HII
Config Routing Protocol that may use services such as UEFI Variable
Services or other device specific services to read/write config
data and that is where an EFI_DEVICE_ERROR can occur.

Mike

> -Original Message-
> From: Tan, Ming 
> Sent: Sunday, January 21, 2024 5:54 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> ; Ren, SuqiangX ;
> gaoliming 
> Cc: Liu, Zhiguang ; Li, Yi1 
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> HiiConfigAccess ExtractConfig
> 
> Mike:
>   For the change of https://github.com/tianocore/edk2/pull/5170.
>   We checked all the files for function XxxExtractConfig, find there is
> not function will return EFI_DEVICE_ERROR in the function code directly.
>   One possible place is just call xxx ->BlockToConfig(), and the
> XxxExtractConfig will return the Status of call BlockToConfig().
>   For example in
> https://github.com/tianocore/edk2/blob/c251015292cc5f4ca003894e5922a40b0
> 8cd14b0/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c#L808
> Status = HiiConfigRouting->BlockToConfig (
>  HiiConfigRouting,
>  ConfigRequest,
>  (UINT8 *)>Configuration,
>  BufferSize,
>  Results,
>  Progress
>  );
> ..
> return Status;
> 
>   So need we change the code, if such BlockToConfig return failed, then
> ignore the detail reason, but return EFI_DEVICE_ERROR in function
> XxxExtractConfig?
> 
>   BR/Tan Ming.
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael D
> Kinney
> Sent: Saturday, January 20, 2024 10:32 AM
> To: Ren, SuqiangX ; gaoliming
> ; devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Li, Yi1 ;
> Kinney, Michael D 
> Subject: Re: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> HiiConfigAccess ExtractConfig
> 
> Hi Suqiang,
> 
> For the Browser/HII related changes to the MdePkg can you also prepare a
> patch to update the function headers in the implementation of these APIs
> and make sure the implementation conforms to the update header file
> changes?
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Ren, SuqiangX 
> > Sent: Sunday, January 14, 2024 6:09 PM
> > To: gaoliming ; devel@edk2.groups.io
> > Cc: Kinney, Michael D ; Liu, Zhiguang
> > ; Li, Yi1 
> > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> > HiiConfigAccess ExtractConfig
> >
> > Hi Liming,
> >
> > Could you help to merge below patches which all your reviewed-by?
> > Thanks!
> > https://github.com/tianocore/edk2/pull/5170
> > https://github.com/tianocore/edk2/pull/5186
> > https://github.com/tianocore/edk2/pull/5190
> >
> >
> > Thanks
> > Ren, Suqiang
> >
> > -Original Message-
> > From: Ren, SuqiangX
> > Sent: Monday, January 8, 2024 9:31 PM
> > To: gaoliming ; devel@edk2.groups.io
> > Cc: Kinney, Michael D ; Liu, Zhiguang
> > ; Li, Yi1 
> > Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> > HiiConfigAccess ExtractConfig
> >
> > Hi Liming,
> >
> > Could you please help to check and merge this patch?
> > https://github.com/tianocore/edk2/pull/5170
> >
> >
> > Thanks
> > Ren, Suqiang
> >
> > -Original Message-
> > From: gaoliming 
> > Sent: Saturday, December 23, 2023 10:10 AM
> > To: devel@edk2.groups.io; Ren, SuqiangX 
> > Cc: Kinney, Michael D ; Liu, Zhiguang
> > ; Li, Yi1 
> > Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> > HiiConfigAccess ExtractConfig
> >
> > Reviewed-by: Liming Gao 
> >
> > > -邮件原件-
> > > 发件人: devel@edk2.groups.io  代表 SuqiangX Ren
> > > 发送时间: 2023年12月21日 10:41
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Ren,Suqiang ; Michael D Kinney
> > > ; Liming Gao ;
> > > Zhiguang Liu ; Yi Li 
> > > 主题: [edk2-devel] [PATCH 1/1] MdePkg: Update the comments of
> > > HiiConfigAccess ExtractConfig
> > >
> > > From: "Ren,Suqiang" 
> > >
> > > Add status code returned for HiiConfigAccess ExtractConfig to align
> > > with UEFI spec 2.10.
> > >
> > > REF: UEFI spec 2.10 Table 35.5.2
> > >
> > > Signed-off-by: SuqiangX Ren 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Cc: Zhiguang Liu 
> > > Cc: Yi Li 
> > > ---
> > >  MdePkg/Include/Protocol/HiiConfigAccess.h | 9 -
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdePkg/Include/Protocol/HiiConfigAccess.h
> > > b/MdePkg/Include/Protocol/HiiConfigAccess.h
> > > index 3baf91e07b2e..fbee7c52b021 100644
> > > --- a/MdePkg/Include/Protocol/HiiConfigAccess.h
> > > +++ b/MdePkg/Include/Protocol/HiiConfigAccess.h
> > > @@ -102,9 +102,16 @@ typedef UINTN EFI_BROWSER_ACTION;
> > >

Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs

[Michael D Kinney]

Hi Ashraf,

The PcdValueInit feature is not limited to only PCDs of type VPD.  It is
for any structured PCDs.  Did you test PCDs with all types (e.g. 
PcdsFixedAtBuild
PcdsPactahbleInModule, PcdsDynamicHii, PcdsDynamicDatabase, PcdsDynamicVpd,
PcdsDynamicExHii, PcdsDynamicExDatabase, PcdsDynamicExVpd)

And did you DSC test cases cover both changes in PCD types and changes in PCD 
values?

Thanks,

Mike

> -Original Message-
> From: S, Ashraf Ali 
> Sent: Sunday, January 21, 2024 4:14 AM
> To: Kinney, Michael D ; devel@edk2.groups.io
> Cc: Chen, Christine ; Rebecca Cran
> ; Liming Gao ; Feng, Bob C
> ; Chan, Amy ; Chaganty,
> Rangasai V ; Solanki, Digant H
> 
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi., @Kinney, Michael D
> 
> The main API which is modified in this Patch is GenerateByteArrayValue.
> 
> This API is used to return the list of SKUID.TokenSpaceGuid.VpdName|VPD
> STRUCT|Binary Data which will be stored in Output.txt
> Example:
> SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[]|
> {0x01,0x01,0x05,0x09,0x02}
> 
> This VPD/PCD is coming from either the DEC file or the DSC file.
> 
> GenerateByteArrayValue API is used to create the PcdValueInit.exe and
> it's equivalent C and Make File.
> 
> When there is no change in DEC or DSC VPD/PCD still it used to do the
> same process again in the incremental build which is time consuming.
> In my project this API is used to take 3min of time with High end server
> (64Threads) and 3.5Min in Laptop (16 threads with performance mode
> enabled) only for GenerateByteArrayValue API.
> 
> This  patch will check if there are any change in the DEC/DSC
> VPDs/PCDs2, if there is any change in VPD the current flow is not
> affect. (it will create the PcdRecordList.json and copy Output.txt for
> Arch folder with the project. Ex:
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordLis
> t.json and
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt)
> If there is no change the DSC/DEC VPDs it will read the data from
> Output.txt and return the same data. (we will compare the Input
> structure and PcdRecordList.json) if there is any mismatch then it there
> change in VPDs if not then no change in VPDs).
> 
> Unit testing from my side as follows:
> * Build the Firmware.
>   * PcdRecordList.json and Output.txt will be created based on the
> Arch.
> * Build the Firmware again without any change.
>   * It will read the Output.txt and return the data. (it will match
> Input PCD/VPD list and compare with existing PcdRecordList.json)
>   * I verified the return length and content GenerateByteArrayValue
> (it's same with and without my changes)
> * Build the Firmware again my modifying the VPDs in DSC file
>   * Change in VPDs, it will regenerate PcdRecordList.json file
>   *  it will create the Exe file and Output.txt
>   * New Output.txt will be copied to above path.
> * Build the Firmware again by modifying the VPDs in DEC file.
>   * Change in VPDs, it will regenerate PcdRecordList.json file
>   *  it will create the Exe file and Output.txt
>   * New Output.txt will be copied to above path.
> * Build the Firmware again without modifying the code.
>   * It will read the Output.txt and return the data.
>   * I verified the return length and content GenerateByteArrayValue
> (it's same with and without my changes)
> 
> 
> There is no impact on the Boot/cache.
> This patch is used to reduce the incremental build time by checking if
> the VPDs/PCDs are changed or not, if not changed then it will return the
> previous stored data.
> 
> Thanks.,
> S, Ashraf Ali
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, January 20, 2024 7:36 AM
> To: devel@edk2.groups.io; S, Ashraf Ali 
> Cc: Chen, Christine ; Rebecca Cran
> ; Liming Gao ; Feng, Bob C
> ; Chan, Amy ; Chaganty,
> Rangasai V ; Solanki, Digant H
> ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi Ashraf,
> 
> What is captured in the file?
> 
> What PCD/VPD changes will invalidate the cache?  Just the number and
> type of PCD/VPD elements or their default values/sizes?
> 
> How was this tested?  Were all conditions that invalidate the cache
> tested?  I ask because incremental build is a very important feature and
> if there is any logic error in the cache management of a file like this,
> it will cause unexpected behavior and developers will not trust
> incremental builds.
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ashraf
> > Ali S
> > Sent: Tuesday, January 16, 2024 11:55 PM
> > To: devel@edk2.groups.io
> > Cc: S, Ashraf Ali ; Chen, Christine
> > ; Rebecca Cran ; Liming Gao
> > ; Feng, Bob C ; Chan,
> > Amy ; Chaganty, Rangasai V
> > ; Solanki, Digant H
> > 
> > Subject: 

Re: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition of FileName on EFI_FILE_INFO

[Michael D Kinney]

Hi Suqiang,

The comment line added look like is exceeds 80 columns.  Please reformat.

Also, there are implementations of this service in the edk2 repo.
Please update those with this same function header update.

Thanks,

Mike

> -Original Message-
> From: Ren, SuqiangX 
> Sent: Thursday, January 11, 2024 1:04 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: RE: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition
> of FileName on EFI_FILE_INFO
> 
> Hi All,
> 
>   Any comments about this patch?
> 
> Thanks
> Ren, Suqiang
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ren,
> Suqiang
> Sent: Tuesday, December 26, 2023 1:22 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: [edk2-devel] [PATCH V2 1/1] MdePkg: Update the definition of
> FileName on EFI_FILE_INFO
> 
> Add the description of FileName to align with UEFI spec 2.10.
> 
> REF: UEFI spec 2.10 Table 13.5.16
> 
> Signed-off-by: Suqiang Ren 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> ---
>  MdePkg/Include/Guid/FileInfo.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Guid/FileInfo.h
> b/MdePkg/Include/Guid/FileInfo.h index 2b7edf36aabc..c152789b40c8 100644
> --- a/MdePkg/Include/Guid/FileInfo.h
> +++ b/MdePkg/Include/Guid/FileInfo.h
> @@ -46,7 +46,7 @@ typedef struct {
>///
>UINT64  Attribute;
>///
> -  /// The Null-terminated name of the file.
> +  /// The Null-terminated name of the file. For a root directory, the
> name is an empty string.
>///
>CHAR16  FileName[1];
>  } EFI_FILE_INFO;
> --
> 2.26.2.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114159): https://edk2.groups.io/g/devel/message/114159
Mute This Topic: https://groups.io/mt/103366883/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 1/1] MdePkg: Add EFI_UNSUPPORTED return for some Runtime Service functions

[Michael D Kinney]

Hi Suqiang,

The changes to this one .h file look ok.

However, there are implementations of the Runtime Services in the edk2 repo
that also need their function headers updates.  Can you please add those
changes to this patch series?

Thanks,

Mike





> -Original Message-
> From: Ren, SuqiangX 
> Sent: Thursday, January 11, 2024 1:05 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Add EFI_UNSUPPORTED return
> for some Runtime Service functions
> 
> Hi All,
> 
>   Any comments about this patch?
> 
> Thanks
> Ren, Suqiang
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ren,
> Suqiang
> Sent: Wednesday, December 27, 2023 4:47 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: [edk2-devel] [PATCH 1/1] MdePkg: Add EFI_UNSUPPORTED return for
> some Runtime Service functions
> 
> According to UEFI Spec 2.10 page 206, if any EFI_RUNTIME_SERVICES* calls
> are not supported for use by the OS at runtime, an
> EFI_RT_PROPERTIES_TABLE configuration table should be published
> describing which runtime services are supported at runtime. So need to
> add EFI_UNSUPPORTED return for some Runtime Service functions.
> 
> REF: UEFI spec 2.10 section 8 Services — Runtime Services
> 
> Signed-off-by: Suqiang Ren 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> ---
>  MdePkg/Include/Uefi/UefiSpec.h | 40 --
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Uefi/UefiSpec.h
> b/MdePkg/Include/Uefi/UefiSpec.h index 5de00e8ea2af..b25485b06763 100644
> --- a/MdePkg/Include/Uefi/UefiSpec.h
> +++ b/MdePkg/Include/Uefi/UefiSpec.h
> @@ -320,6 +320,9 @@ EFI_STATUS
>  map that requires a mapping.
>@retval EFI_NOT_FOUND A virtual address was supplied for an
> address that is not found
>  in the memory map.
> +  @retval EFI_UNSUPPORTED   This call is not supported by this
> platform at the time the call is made.
> +The platform should describe this
> runtime service as unsupported at runtime
> +via an EFI_RT_PROPERTIES_TABLE
> configuration table.
> 
>  **/
>  typedef
> @@ -415,6 +418,9 @@ EFI_STATUS
>  not have the EFI_OPTIONAL_PTR bit set.
>@retval EFI_NOT_FOUND The pointer pointed to by Address was
> not found to be part
>  of the current memory map. This is
> normally fatal.
> +  @retval EFI_UNSUPPORTED   This call is not supported by this
> platform at the time the call is made.
> +The platform should describe this
> runtime service as unsupported at runtime
> +via an EFI_RT_PROPERTIES_TABLE
> configuration table.
> 
>  **/
>  typedef
> @@ -679,6 +685,10 @@ VOID
>@retval EFI_INVALID_PARAMETER  The DataSize is not too small and Data
> is NULL.
>@retval EFI_DEVICE_ERROR   The variable could not be retrieved
> due to a hardware error.
>@retval EFI_SECURITY_VIOLATION The variable could not be retrieved
> due to an authentication failure.
> +  @retval EFI_UNSUPPORTEDAfter ExitBootServices() has been
> called, this return code may be returned
> + if no variable storage is supported.
> The platform should describe this
> + runtime service as unsupported at
> runtime via an EFI_RT_PROPERTIES_TABLE
> + configuration table.
> 
>  **/
>  typedef
> @@ -715,6 +725,10 @@ EFI_STATUS
>@retval EFI_INVALID_PARAMETER Null-terminator is not found in the
> first VariableNameSize bytes of
>  the input VariableName buffer.
>@retval EFI_DEVICE_ERROR  The variable could not be retrieved due
> to a hardware error.
> +  @retval EFI_UNSUPPORTED   After ExitBootServices() has been
> called, this return code may be returned
> +if no variable storage is supported.
> The platform should describe this
> +runtime service as unsupported at
> runtime via an EFI_RT_PROPERTIES_TABLE
> +configuration table.
> 
>  **/
>  typedef
> @@ -757,6 +771,9 @@ EFI_STATUS
>   but the AuthInfo does NOT pass the
> validation check carried out by the firmware.
> 
>@retval EFI_NOT_FOUND  The variable trying to be updated or
> deleted was not found.
> +  @retval EFI_UNSUPPORTEDThis call is not supported by this
> platform at the time the call is made.
> + The platform should describe this
> runtime service as unsupported at runtime
> + via an EFI_RT_PROPERTIES_TABLE
> configuration 

[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, January 22, 2024 #cal-notice

[Group Notification]

*Tools, CI, Code base construction meeting series*

*When:*
Monday, January 22, 2024
4:30pm to 5:30pm
(UTC-08:00) America/Los Angeles

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

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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

[Michael D Kinney]

Hi Ray,

+Andrew Fish

That optimization was imported into git history 17 years ago, so
it has effectively always been there.  I do not recall the performance
improvement at the time the optimization was originally implemented.

The difference in behavior is that caching the result may miss
an uninstall later before dispatch. Always evaluating right before 
dispatch is safer because is guarantees that the expression is TRUE
and all conditions met when the image is started.

Your example is possible, but not likely to appear in practice for
the types of protocols used in dependency expressions.

Protocols that are uninstalled are more typically associated with 
the UEFI Driver model for buses/devices that can be added/removed.

If CoreLocateProtocol() was optimized, then perhaps this optimization
would have never been implemented.

I see no harm in removing the optimization, especially for only
Standalone MM.

If there is a need to treat DEPEX section of all images as const,
then there are other places that the cached evaluation could be
stored to enable this specific optimization for all image types.

Best regards,

Mike

> -Original Message-
> From: Ni, Ray 
> Sent: Sunday, January 21, 2024 7:05 PM
> To: Nhi Pham ; devel@edk2.groups.io; Kinney,
> Michael D 
> Cc: ardb+tianoc...@kernel.org; sami.muja...@arm.com; ler...@redhat.com
> Subject: RE: [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for
> depex evaluation
> 
> Reviewed-by: Ray Ni 
> 
> PI spec does not define the REPLACE_TRUE op-code.
> Existing dispatchers (DXE, SMM and Standalone MM) use REPLACE_TRUE to
> optimize the protocol evaluation. PEI dispatcher does not use this
> optimization as the Depex binary might be in flash.
> 
> Now this patch only modifies standalone MM version to not use the
> optimization. I think it's a right way.
> 
> 
> 
> Because the optimization cannot handle a case:
> 1. driver A installs protocol #a.
> 2. driver B depends on protocol #a.
> 3. driver A uninstalls the protocol #a in a callback (protocol callback,
> timer callback, or a service provided by A that driver B invokes)
> 4. driver B gets dispatched after the callback. <--- problem here. The
> protocol #a does not exist!!
> 
> @Kinney, Michael D, do you have history data of which optimization
> result it achieved? Do you think that the optimization can be in
> CoreLocateProtocol() instead of in the callers?
> 
> Thanks,
> Ray
> > -Original Message-
> > From: Nhi Pham 
> > Sent: Friday, January 19, 2024 12:57 PM
> > To: devel@edk2.groups.io
> > Cc: ardb+tianoc...@kernel.org; Ni, Ray ;
> > sami.muja...@arm.com; ler...@redhat.com; Nhi Pham
> > 
> > Subject: [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for
> > depex evaluation
> >
> > From: Laszlo Ersek 
> >
> > The current dependency evaluator violates the memory access permission
> > when patching depex grammar directly in the read-only depex memory
> area.
> >
> > Laszlo pointed out the optimization issue in the thread (1) "Memory
> > Attribute for depex section" and provided suggested patch to remove
> the
> > perf optimization.
> >
> > In my testing, removing the optimization does not make significant
> perf
> > reduction. That makes sense that StandaloneMM dispatcher only searches
> > in MM protocol database and does not depend on UEFI/DXE protocol
> > database. Also, we don't have many protocols in StandaloneMM like
> > UEFI/DXE.
> >
> > From Laszlo,
> >
> > "The patch removes the EFI_DEP_REPLACE_TRUE handling altogether, plus
> it
> > CONST-ifies the Iterator pointer (which points into the DEPEX
> section),
> > so that the compiler catch any possible accesses at *build time* that
> > would write to the write-protected DEPEX memory area."
> >
> > (1) https://edk2.groups.io/g/devel/message/113531
> >
> > Signed-off-by: Nhi Pham 
> > ---
> >  StandaloneMmPkg/Core/Dependency.c | 37 
> >  1 file changed, 7 insertions(+), 30 deletions(-)
> >
> > diff --git a/StandaloneMmPkg/Core/Dependency.c
> > b/StandaloneMmPkg/Core/Dependency.c
> > index 440fe3e45238..2bcb07d34666 100644
> > --- a/StandaloneMmPkg/Core/Dependency.c
> > +++ b/StandaloneMmPkg/Core/Dependency.c
> > @@ -13,16 +13,6 @@
> >
> >
> >  #include "StandaloneMmCore.h"
> >
> >
> >
> > -///
> >
> > -/// EFI_DEP_REPLACE_TRUE - Used to dynamically patch the dependency
> > expression
> >
> > -///to save time.  A EFI_DEP_PUSH is
> > evaluated one an
> >
> > -///replaced with EFI_DEP_REPLACE_TRUE. If
> > PI spec's Vol 2
> >
> > -///Driver Execution Environment Core
> > Interface use 0xff
> >
> > -///as new DEPEX opcode.
> > EFI_DEP_REPLACE_TRUE should be
> >
> > -///defined to a new value that is not
> > conflicting with PI spec.
> >
> > -///
> >
> > -#define EFI_DEP_REPLACE_TRUE  0xff
> >
> > -
> >
> >  ///
> >
> >  /// Define the initial size of the dependency expression evaluation
> stack
> 

[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, January 22, 2024 #cal-reminder

[Group Notification]

*Reminder: Tools, CI, Code base construction meeting series*

*When:*
Monday, January 22, 2024
4:30pm to 5:30pm
(UTC-08:00) America/Los Angeles

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

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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

Re: [edk2-devel] [PATCH v1 1/1] .pytool/Plugin: UncrustifyCheck: use stat instead of os.stat

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: Joey Vagedes 
> Sent: Monday, January 22, 2024 3:21 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao ; Kinney, Michael D
> ; Sean Brogan 
> Subject: [PATCH v1 1/1] .pytool/Plugin: UncrustifyCheck: use stat
> instead of os.stat
> 
> The UncrustifyCheck plugin passes os.stat.S_IWRITE to os.chmod, when
> attempting to change file permissions. os.stat.S_IWRITE does not exist
> as os.stat is a function. The correct value is stat.S_IWRITE.
> 
> Signed-off-by: Joey Vagedes 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Sean Brogan 
> ---
>  .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> index 9aeef5a5a3..73dc03c0dc 100644
> --- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> +++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
> @@ -12,6 +12,7 @@ import logging
>  import os
> 
>  import pathlib
> 
>  import shutil
> 
> +import stat
> 
>  import timeit
> 
>  from edk2toolext.environment import version_aggregator
> 
>  from edk2toolext.environment.plugin_manager import PluginManager
> 
> @@ -628,7 +629,7 @@ class UncrustifyCheck(ICiBuildPlugin):
>  """
> 
>  Private function to attempt to change permissions on
> file/folder being deleted.
> 
>  """
> 
> -os.chmod(path, os.stat.S_IWRITE)
> 
> +os.chmod(path, stat.S_IWRITE)
> 
>  func(path)
> 
> 
> 
>  for _ in range(3):  # retry up to 3 times
> 
> --
> 2.40.1.vfs.0.0



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

[edk2-devel] [PATCH v1 1/1] .pytool/Plugin: UncrustifyCheck: use stat instead of os.stat

[Joey Vagedes via groups.io]

The UncrustifyCheck plugin passes os.stat.S_IWRITE to os.chmod, when
attempting to change file permissions. os.stat.S_IWRITE does not exist
as os.stat is a function. The correct value is stat.S_IWRITE.

Signed-off-by: Joey Vagedes 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Sean Brogan 
---
 .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py 
b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
index 9aeef5a5a3..73dc03c0dc 100644
--- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
+++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py
@@ -12,6 +12,7 @@ import logging
 import os
 import pathlib
 import shutil
+import stat
 import timeit
 from edk2toolext.environment import version_aggregator
 from edk2toolext.environment.plugin_manager import PluginManager
@@ -628,7 +629,7 @@ class UncrustifyCheck(ICiBuildPlugin):
 """
 Private function to attempt to change permissions on file/folder 
being deleted.
 """
-os.chmod(path, os.stat.S_IWRITE)
+os.chmod(path, stat.S_IWRITE)
 func(path)
 
 for _ in range(3):  # retry up to 3 times
-- 
2.40.1.vfs.0.0



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

[edk2-devel] [PATCH v1 0/1] UncrustifyCheck: use stat instead of os.stat

[Joey Vagedes via groups.io]

When UncrustifyCheck attempts to update the permissions of a file
( Only happens when a different error occurs ), it incorrectly uses
os.stat.S_IWRITE, which does not exist. The correct value is 
stat.S_IWRITE.

see os.chmod documentation: https://docs.python.org/3/library/os.html#os.chmod

Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Sean Brogan 

Joey Vagedes (1):
  .pytool/Plugin: UncrustifyCheck: use stat instead of os.stat

 .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.40.1.vfs.0.0



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

Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of EFI_FIRMWARE_MANAGEMENT_PROTOCOL

[Michael D Kinney]

Hi Guo,

Thank you for starting this task to update Firmware Management Protocol.

There are additional tasks to make sure all required code changes are
also implemented for a specification update like this.

* Create a TianoCore Bugzilla that describes the specification update
  needed with links to the public documents with the required
  specification update. 

* Review/update edk2 repo include files.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2 repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2 repo to make sure all error 
  return conditions are checked.

* Review/update function headers of the implementations of the
  Firmware Management Protocol in the edk2-platforms repo.

* Review/update logic in implementations of the Firmware
  Management Protocol in the edk2-platforms repo to make sure 
  all error return conditions are checked.

* Review/update test cases edk2-test repo for the UEFI SCTs for
  these error return conditions.


The "REF" in the commit message should be a link to the TianoCore
Bugzilla.  The reference to the specification is also required, but
should be in the text of the commit message.

Thanks,

Mike


> -Original Message-
> From: Xu, GuoX 
> Sent: Sunday, January 21, 2024 9:37 PM
> To: gaoliming ; devel@edk2.groups.io
> Cc: Kinney, Michael D ; Liu, Zhiguang
> 
> Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> Hi Liming,
>   I created a PR: https://github.com/tianocore/edk2/pull/5182, could you
> help push it ?
> 
> Thanks
> Xu Guo
> 
> -Original Message-
> From: gaoliming 
> Sent: Tuesday, January 16, 2024 10:20 PM
> To: devel@edk2.groups.io; Xu, GuoX 
> Cc: Kinney, Michael D ; Liu, Zhiguang
> 
> Subject: 回复: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> 
> I am OK for this change. Reviewed-by: Liming Gao
> 
> 
> > -邮件原件-
> > 发件人: devel@edk2.groups.io  代表 Xu, GuoX
> > 发送时间: 2024年1月9日 17:24
> > 收件人: devel@edk2.groups.io
> > 抄送: Kinney, Michael D ; Gao, Liming
> > ; Liu, Zhiguang 
> > 主题: Re: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > Hi all, any comments about this patch?
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of GuoX Xu
> > Sent: Monday, December 25, 2023 9:21 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D ; Gao, Liming
> > ; Liu, Zhiguang ;
> > Li, Yi1 
> > Subject: [edk2-devel] [PATCH 1/1] MdePkg:Updated the comments of
> > EFI_FIRMWARE_MANAGEMENT_PROTOCOL
> >
> > 1.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImage():
> > Add the following sentence at the end of the Image parameter
> description.
> > "May be NULL with a zero ImageSize in order to determine the size of
> > the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> > ImageSize is not too small and Image is NULL."
> >
> > 2.For EFI_FIRMWARE_MANAGEMENT_PROTOCOL.GetImageInfo():
> > Add the following sentence at the end of the ImageInfo parameter
> > description."May be NULL with a zero ImageInfoSize in order to
> > determine the size of the buffer needed".
> >
> > Modify the description of "EFI_INVALID_PARAMETER" return code as "The
> > ImageInfoSize is not too small and Image is NULL." and add new
> > descriptions for "EFI_INVALID_PARAMETER" return code.
> >
> > REF: UEFI Spec 2.7B Chapter 23.1.
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Yi Li 
> > Signed-off-by: GuoX Xu 
> > ---
> >  MdePkg/Include/Protocol/FirmwareManagement.h | 13 +++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/Protocol/FirmwareManagement.h
> > b/MdePkg/Include/Protocol/FirmwareManagement.h
> > index f37067df3455..93c8b7658e1a 100644
> > --- a/MdePkg/Include/Protocol/FirmwareManagement.h
> > +++ b/MdePkg/Include/Protocol/FirmwareManagement.h
> > @@ -293,6 +293,8 @@ EFI_STATUS
> >   to contain the image(s)
> > information if the buffer was too small.
> >@param[in, out] ImageInfo  A pointer to the buffer in which
> > firmware places the current image(s)
> >   information. The information is
> > an array of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
> > + May be NULL with a zero
> > ImageInfoSize in order to determine the size of the
> > + buffer needed.
> >@param[out] DescriptorVersion  A pointer to the location in
> which
> > firmware returns the version number
> >   associated with the
> > EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> >@param[out] DescriptorCountA pointer to the location in
> > which firmware returns the number of
> > @@ -313,7 +315,12 @@ EFI_STATUS
> >@retval 

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/19/2024 1:03 PM, Oliver Smith-Denny wrote:


Thanks for trying. In lieu of being able to test myself, all I can offer
is adding some more prints, when the memory gets allocated, making sure
it is 64k aligned then. I'd be curious to see what the address is that
is attempting to be freed.

My guess (as it was earlier) is that it is going to be aligned to
64k but + 4k. I.e the guard page at the front is throwing it off. There
may have just been an error in my attempt to fix the check for that.

If however that address is not 64k + 4k aligned, then something else is
afoot.

Happy to look at some more data if you get it or can engineer an example
on an open source system (can you force the system to call this function
twice even without the extra SMBIOS entries, etc.).


These are the addresses it's allocating with and without HeapGuard (with 
the original, upstream Page.c code).



Without HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xFB7C)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xEF11, 1)
Allocated 0xEF11 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (4153), 0xFB8AEC8E)


--

WITH HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0xED36F000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0xED38F000)

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0xED36F000, 1)


--
Rebecca Cran


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

Re: [edk2-devel] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

[Rebecca Cran]

On 1/22/2024 1:24 PM, Oliver Smith-Denny wrote:

Allocating pool memory will never be 4KB aligned because of the
POOL_HEAD. See: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Pool.c#L463-L537.


The allocated page is 4KB aligned, of course, but we store the POOL_HEAD
at the beginning of that page and return Head->Data to the caller.

In addition, this is only in the case where you allocate a new page for
the pool. In the case where the pool has the pages it needs, you don't
have any alignment guarantee, I believe, it is just offset further
into one of the already allocated pages.


Thanks. I'll update the script to require users to pass in the address 
of the EFI_SYSTEM_TABLE instead of searching for it!



--
Rebecca Cran



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

Re: [edk2-devel] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

[Michael D Kinney]

Hi Rebecca,

I do not recall any statements in the EFI Spec that require 4KB
alignment of the UEFI System Table, Boot Services Table, 
or Runtime Services Table.

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Rebecca
> Cran
> Sent: Monday, January 22, 2024 11:53 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] EFI_SYSTEM_TABLE allocated by
> AllocateRuntimeCopyPool isn't aligned to 4KB
> 
> I've been working on updating the T32 script EfiLoadDxe.cmm in
> EmbeddedPkg/Scripts/LauterbachT32 and one of the issues I've run into is
> that the EFI_SYSTEM_TABLE - pointed to by `gST` and `gDxeCoreST` - isn't
> aligned to 4KB like the script expects.
> 
> Instead, I'm seeing AllocateRuntimeCopyPool return 0xFB7D0018 in
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:
> 
> //
> // Allocate the EFI System Table and EFI Runtime Service Table from
> EfiRuntimeServicesData
> // Use the templates to initialize the contents of the EFI System Table
> and EFI Runtime Services Table
> //
> gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE),
> );
> ASSERT (gDxeCoreST != NULL);
> 
> I'm wondering which is wrong: should AllocateRuntimeCopyPool be
> returning a 4KB-aligned pointer, or is the script's assumption wrong?
> 
> 
> --
> Rebecca Cran
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114148): https://edk2.groups.io/g/devel/message/114148
Mute This Topic: https://groups.io/mt/103894456/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] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

[Oliver Smith-Denny]

On 1/22/2024 11:52 AM, Rebecca Cran wrote:
I've been working on updating the T32 script EfiLoadDxe.cmm in 
EmbeddedPkg/Scripts/LauterbachT32 and one of the issues I've run into is 
that the EFI_SYSTEM_TABLE - pointed to by `gST` and `gDxeCoreST` - isn't 
aligned to 4KB like the script expects.


Instead, I'm seeing AllocateRuntimeCopyPool return 0xFB7D0018 in 
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:


//
// Allocate the EFI System Table and EFI Runtime Service Table from 
EfiRuntimeServicesData
// Use the templates to initialize the contents of the EFI System Table 
and EFI Runtime Services Table

//
gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE), 
);

ASSERT (gDxeCoreST != NULL);

I'm wondering which is wrong: should AllocateRuntimeCopyPool be 
returning a 4KB-aligned pointer, or is the script's assumption wrong?





Allocating pool memory will never be 4KB aligned because of the
POOL_HEAD. See: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Mem/Pool.c#L463-L537.


The allocated page is 4KB aligned, of course, but we store the POOL_HEAD
at the beginning of that page and return Head->Data to the caller.

In addition, this is only in the case where you allocate a new page for
the pool. In the case where the pool has the pages it needs, you don't
have any alignment guarantee, I believe, it is just offset further
into one of the already allocated pages.

Oliver


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

[edk2-devel] EFI_SYSTEM_TABLE allocated by AllocateRuntimeCopyPool isn't aligned to 4KB

[Rebecca Cran]

I've been working on updating the T32 script EfiLoadDxe.cmm in 
EmbeddedPkg/Scripts/LauterbachT32 and one of the issues I've run into is 
that the EFI_SYSTEM_TABLE - pointed to by `gST` and `gDxeCoreST` - isn't 
aligned to 4KB like the script expects.


Instead, I'm seeing AllocateRuntimeCopyPool return 0xFB7D0018 in 
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:


//
// Allocate the EFI System Table and EFI Runtime Service Table from 
EfiRuntimeServicesData
// Use the templates to initialize the contents of the EFI System Table 
and EFI Runtime Services Table

//
gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE), 
);

ASSERT (gDxeCoreST != NULL);

I'm wondering which is wrong: should AllocateRuntimeCopyPool be 
returning a 4KB-aligned pointer, or is the script's assumption wrong?



--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114146): https://edk2.groups.io/g/devel/message/114146
Mute This Topic: https://groups.io/mt/103894456/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] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Brian J. Johnson]

On 1/18/24 09:46, Gerd Hoffmann wrote:

On Wed, Jan 10, 2024 at 04:43:47PM +, West, Catharine wrote:

Disabling cache by default results in violation of BTG protections (if BTG 
enabled).
  
BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.


Whatever solution needs to be done here cannot evict ACM-enabled NEM.


Well, it's OVMF in a virtual machine.  No boot guard involved.
So we could probably go for a OVMF-specific patch here.

But I'd prefer to figure what exactly is happening here before going
down that route.  An extreme slowdown just because we flip that bit
doesn't make sense to me.


Why is boot time increasing?


Not clear.  It seems to be the lzma uncompress of the firmware volume
in rom / pflash which is very slow.  Also it is apparently only
triggered in case pci device assignment is used.


I've seen extreme slowness on physical platforms when we've mixed up the 
MTRRs or page tables, causing code to be mapped uncached.


Lzma uncompress of ROM could be pretty slow as well, if the ROM is being 
read uncached.  Lzma probably reads the data a byte at a time, which is 
the worst case for uncached accesses.  Since this is a VM, it's not 
actually uncached at the hardware level, but I don't know how QEMU/KVM 
handles uncached guest mappings It may be doing a VMEXIT for every byte.


Anyway, I suggest double-checking your page tables and MTRRs.
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise


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

[edk2-devel] Multi-ISA Driver Compatibility Survey

[Rebecca Cran]

Originally posted at 
https://twitter.com/UEFIForum/status/1745518769232077208


The Multi-ISA Driver Compatibility Survey is at 
https://docs.google.com/forms/d/e/1FAIpQLScXjwaSBgLeqB1coEDxCPxho5JWF3AMqshOTJ2wd6Tf0He4LA/viewform


From that page:

Did you know Tiano today supports four 64-bit architectures, yet plug-in 
device OpRoms are still mostly limited to x64 and CSM? While 
binary-translation approaches are a useful stop-gap solution for both 
AArch64 and RV64 ecosystems, we need a common approach that is not a 
technical debt nightmare and that will be adopted by IHVs and endorsed 
by OSVs.


The  Multi-ISA Driver Compatibility talk went over some of the possible 
approaches, as a lead-in for an open discussion, and raised the 
long-term importance of solving cross-architecture compatibility issues 
for OpRom drivers.


This survey is an opportunity provide feedback to guide further 
exploration in this space.


The discussed options were:

- Do nothing. This is an IHV problem. IHV should continue shipping 
support for whatever architectures they deem necessary.
- Resurrect EFI Byte Code. Invest in compilers and tooling (e.g. 
addr2line, JIT, etc) to get parity with existing native drivers.
- Look at WASM, and solve tooling constraints (around sandboxing) that 
prevent adoption.
- eBPF, and solve tooling constraints (around sandboxing) that prevent 
adoption.
- Constrained, well-defined subset of x64. This meets IHVs half-way by 
avoiding significant perturbations to existing driver development and 
release processes, and achieves compatibility with existing x64 systems 
in the market "for free", while addressing most of the concerns around 
binary translation of x64 code.


Note: the last three options (WASM, eBPF and constrained x64) are not 
neutral with respect to host natural register width, which will mean a 
break in compatibility with future TBD 128-bit environments, unless a 
TBD OpRom sandboxing technology is invented.


Note 2: emails and names/sign-ins are not collected, this is an 
anonymous response.




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

[edk2-devel] Resources for Creating Packages

[ryderkeys via groups.io]

Hello,

(Originally sent to edk2 discuss but it looks like my message has been stuck in 
moderation for a week, so I thought I would try here instead.)

I am new to UEFI and trying to learn how to use EDK 2. I have been able to 
build EmulatorPkg with stuart but I had a couple questions on where to go from 
here. I was hoping someone might be able to give some guidance or pointers. Any 
help is much appreciated.

1. How can I build a UEFI application with a new package? I see the 
https://github.com/tianocore/tianocore.github.io/wiki/Getting-Started-Writing-Simple-Application
 suggests to edit an existing package DSC file with the INF file for my module. 
Which works fine. But where would I begin if I wanted to create my own package 
without commandeering the build for an already existing package? Would I just 
have to create a DSC file on my own? Is there a resource I can read on how to 
create new packages with EDK 2, or is this not the recommended way to get 
started building applications?

2. What exactly is in MdeModulePkg? I just noticed it has a similar name to 
MdePkg and wondered what the difference between the two was. The wiki 
(https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg) says "This 
package provides the modules that conform to UEFI/PI Industry standards. It 
also provides the defintions(including PPIs/PROTOCOLs/GUIDs and library 
classes) and libraries instances, which are used for those modules." but I'm 
not completely sure what this means. Is someone able to elaborate?

Thank you,
Ryder


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

Re: [edk2-devel] [RFC PATCH v1 00/20] DynamicTablesPkg: Prepare to add RISC-V support

[Sami Mujawar]

Hi All,

DynamicTablesPkg currently supports Arm architecture, and we welcome the 
adoption by other architectures.

Following is my proposal for moving forward.

Goals:
- reuse common code
- streamline the adoption by other architectures
- minimise the impact on migration of the existing platforms
- maintain flexibility across architectural components
- use this opportunity to integrate Dynamic SMBIOS support
  (Ref: https://edk2.groups.io/g/devel/message/107254)

The following steps would help in achieving the goals:
1.  Create an edk2 staging branch. For the edk2-platforms updates, I will 
create a branch on my Github fork (note this is required as there is no staging 
repo for edk2-platforms).
2. The design aspects and changes shall be discussed on the mailing list with 
patches to support the details.
3. A new section in DynamicTablesPkg\Readme.md shall be added to reflect the 
design updates, e.g. changes to CM Objects, Namespace definitions, etc.
4.  The design changes should typically be supported by patches for the 
DynamicTables core framework and demonstrate the impact on the existing 
platform code by typically providing patches for at least one existing platform 
(possibly edk2-platforms/Platform/ARM/[Juno | FVP]).
5. The design changes should be small and typically be reflected in separate 
patch series.
6. The first phase would be to partition the codebase into common code vs 
architectural specific code. This would involve moving files and reflecting the 
associated changes such that the build does not break.
7. Define a new namespace e.g. “ArchCommon” for the common architectural 
components.
8. Identify the CM_ARM_OBJECTs that can be moved to the “ArchCommon” namespace. 
As part of this identify if any object needs to be dropped, e.g. 
EArmObjReserved29
9. Identify overlap of SMBIOS objects with existing CM Objects.
10. Submit patches to move CM objects from Arm Namespace to ArchCommon 
Namespace. Ideally one object (and any dependencies) should be moved at a time.
11. Submit patches to migrate upstream platforms that use DynamicTablesPkg
12. Define a new namespace for RISCV specific objects
13. Submit patches for enabling RISCV
14. In the next phase support for Dynamic SMBIOS can be enabled.

Notes: 
a. Periodically rebase with edk2 & edk2-platforms master branch to sync with 
latest changes.
b. We can decide to merge the updates after point 11 above to edk2 & 
edk2-platforms master branch.
c. Similarly, the RISCV support can be merged after point 13.

I will send out a request for creating the staging branch shortly.

Regards,

Sami Mujawar

On 10/01/2024, 21:56, "Jeshua Smith" mailto:jesh...@nvidia.com>> wrote:


> > It looks like instead of moving the common code to
> EObjNameSpaceStandard namespace or a new (Arch? Common?) namespace,
> you're renaming the entire EObjNameSpaceArm namespace to
> EObjNameSpaceArch. It seems to me that if ARM code vs. common code is
> being separated out, then the EObjNameSpaceArm namespace should
> continue to be used for the ARM-specific code and a common namespace
> should be used for the common code.
> 
> I agree. I started with separating common things into new common space and
> create one for risc-v. However, I dropped that approach for two reasons.
> 
> 1) The commit "b2bbe3df5470 DynamicTablesPkg: Remove PPTT ID structure
> from ACPI 6.4 generator" when removed one of the enums from
> ArmObjectID, didn't change the other values for other enums but reserved the
> removed one. So, I thought there may be some assumptions which will break
> if the enum value changes.


I'm not familiar with why that was done. Hopefully someone else can comment. I 
do know that 
edk2/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 has arrays (StdNamespaceObjectParser and ArmNamespaceObjectParser) that need 
to be kept in sync with all of the namespace enums, but other than that I'm not 
aware of any places that need to be changes when the enums are changed.


> 2) DynamicPlatformRepositoryInfo structure has ArmCmObjList and
> ArmCmObjArray. With separate spaces for Arm, RiscV and Common, list
> management needs some redesign and I was not sure it is worth it.
> 
> Hence, I thought a single list of all possible Obj Ids for all architectures 
> and
> common things would be a good trade off. But I can go back to that approach
> in v2 if above issues are fine.


Hopefully ARM can give input on the best direction before you make more 
changes. The DynamicPlatRepo currently only supports the ARM namespace, but 
comments such as "only Arm objects are supported for now." (line 144) seem to 
imply that support for more namespaces was considered.





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

Re: [edk2-devel] [PATCH v2 2/2] MdeModulePkg: Add missing Iret.h to NestedInterruptTplLib sources list

[Laszlo Ersek]

On 1/20/24 18:08, Michael Brown wrote:
> Suggested-by: Ray Ni 
> Signed-off-by: Michael Brown 
> ---
>  .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf  | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git 
> a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf 
> b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> index 1e03e1364e0f..f130d6dcd213 100644
> --- a/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +++ b/MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> @@ -20,6 +20,7 @@ [Defines]
>  
>  [Sources]
>Tpl.c
> +  Iret.h
>Iret.c
>  
>  [Packages]

(1) Please add a sentence to the commit message body: "This will allow
the build system to notice changes to Iret.h".

(The statement may be trivial :) I just don't like it when a commit
message only consists of a subject line.)

Thanks for considering!

Reviewed-by: Laszlo Ersek 



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

Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg: Move NestedInterruptTplLib to MdeModulePkg

[Laszlo Ersek]

Two comments:

On 1/20/24 18:08, Michael Brown wrote:
> NestedInterruptTplLib provides a way for timer interrupt handlers
> (which must support nested interrupts) to prevent unbounded stack
> consumption.
>
> The underlying issue was first observed in OvmfPkg, since interrupt
> storms can arise more easily in virtual machines due to CPU
> starvation.  However, careful investigation shows that the unbounded
> stack consumption can also occur in physical machines.
>
> Move NestedInterruptTplLib from OvmfPkg to MdeModulePkg so that it can
> more easily be consumed by drivers outside of OvmfPkg.
>
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Michael D Kinney 
> Signed-off-by: Michael Brown 
> ---
>  MdeModulePkg/MdeModulePkg.dec | 4 
>  OvmfPkg/OvmfPkg.dec   | 4 
>  OvmfPkg/AmdSev/AmdSevX64.dsc  | 2 +-
>  OvmfPkg/CloudHv/CloudHvX64.dsc| 2 +-
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc  | 2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc| 2 +-
>  OvmfPkg/OvmfPkgIa32.dsc   | 2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc| 2 +-
>  OvmfPkg/OvmfPkgX64.dsc| 2 +-
>  OvmfPkg/OvmfXen.dsc   | 2 +-
>  UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
>  .../Library/NestedInterruptTplLib/NestedInterruptTplLib.inf   | 2 +-
>  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf   | 1 +
>  .../Include/Library/NestedInterruptTplLib.h   | 0
>  .../Library/NestedInterruptTplLib/Iret.h  | 0
>  .../Library/NestedInterruptTplLib/Iret.c  | 0
>  {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c | 0
>  17 files changed, 15 insertions(+), 14 deletions(-)
>  rename {OvmfPkg => 
> MdeModulePkg}/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf (91%)
>  rename {OvmfPkg => MdeModulePkg}/Include/Library/NestedInterruptTplLib.h 
> (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.h (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Iret.c (100%)
>  rename {OvmfPkg => MdeModulePkg}/Library/NestedInterruptTplLib/Tpl.c (100%)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index a2cd83345f5b..d6fb729af5a7 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -169,6 +169,10 @@ [LibraryClasses]
>#
>ImagePropertiesRecordLib|Include/Library/ImagePropertiesRecordLib.h
>
> +  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
> +  #
> +  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
> +
>  [Guids]
>## MdeModule package token space guid
># Include/Guid/MdeModulePkgTokenSpace.h
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index b44fa039f76c..05d43d5a6861 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -41,10 +41,6 @@ [LibraryClasses]
>#
>MemEncryptTdxLib|Include/Library/MemEncryptTdxLib.h
>
> -  ##  @libraryclass  Handle TPL changes within nested interrupt handlers
> -  #
> -  NestedInterruptTplLib|Include/Library/NestedInterruptTplLib.h
> -
>##  @libraryclass  Save and restore variables using a file
>#
>NvVarsFileLib|Include/Library/NvVarsFileLib.h
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index a31a89344a60..80456f878a22 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -354,7 +354,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  
> NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
>
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index b522fa10594d..9c6c68ae2c35 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -394,7 +394,7 @@ [LibraryClasses.common.DXE_DRIVER]
>  !endif
>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> -  
> NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> +  
> NestedInterruptTplLib|MdeModulePkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
>QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>
> 

Re: [edk2-devel] [PATCH 1/6] UefiCpuPkg/LocalApicTimerDxe: Duplicate OvmfPkg/LocalApicTimerDxe driver

[Gerd Hoffmann]

  Hi,

> I do appreciate that it's difficult to understand the internals of
> NestedInterruptTplLib.  It's fundamentally having to solve a very difficult
> problem within the constraints of the UEFI API.  I think the solution that
> NestedInterruptTplLib provides is as simple as it's possible to get, and it
> does at least have the advantage that all of the complexity is hidden inside
> the library.

I think NestedInterruptTplLib is a good solution for the problem and I
fully support moving it to MdeModulePkg.

Yes, the library is complex.  I reviewed it when it was merged to
OvmfPkg, and even with the very good comments it took me a few hours to
fully understand the logic.  But the problem is complex too, I also
think there is no simpler way to solve it.

take care,
  Gerd



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

[Zhi Jin]

CoreConnectSingleController() searches for the Driver Family Override
Protocol drivers by looping and checking each Driver Binding Handles.
This loop can be skipped by checking if any Driver Family Override
Protocol installed in the platform first, to improve the performance.

Cc: Liming Gao 
Cc: Ray Ni 
Reviewed-by: Michael D Kinney 
Signed-off-by: Zhi Jin 
---
 MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c 
b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
index 0b824c62b7..64d7474f15 100644
--- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
+++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
@@ -497,7 +497,12 @@ CoreConnectSingleController (
   //
   // Add the Driver Family Override Protocol drivers for ControllerHandle
   //
-  while (TRUE) {
+  Status = CoreLocateProtocol (
+ ,
+ NULL,
+ (VOID **)
+ );
+  while (!EFI_ERROR (Status) && (DriverFamilyOverride != NULL)) {
 HighestIndex   = DriverBindingHandleCount;
 HighestVersion = 0;
 for (Index = 0; Index < DriverBindingHandleCount; Index++) {
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114138): https://edk2.groups.io/g/devel/message/114138
Mute This Topic: https://groups.io/mt/103883258/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] MdeModulePkg: Remove the handle validation check in CoreGetProtocolInterface

[Zhi Jin]

CoreGetProtocolInterface() is called by CoreOpenProtocol(),
CoreCloseProtocol() and CoreOpenProtocolInformation().
Before CoreOpenProtocol() calls CoreGetProtocolInterface(), the input
parameter UserHandle has been already checked for validation. So does
CoreCloseProtocol().
Removing the handle validation check in CoreGetProtocolInterface()
could improve the performance, as CoreOpenProtocol() is called very
frequently.
To ensure the assumption that the caller of CoreGetProtocolInterface()
must pass in a valid UserHandle that is checked with CoreValidateHandle(),
add the parameter check in CoreOpenProtocolInformation(), and declare
CoreGetProtocolInterface() as static.

v1 -> v2:
  1. Update the description of UserHandle to state that the caller
 must pass in a valid UserHandle that is checked with
 CoreValidateHandle().
  2. Declare CoreGetProtocolInterface() as static.

Cc: Liming Gao 
Cc: Ray Ni 
Cc: Michael D Kinney 
Signed-off-by: Zhi Jin 
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 51e5b5d3b3..24e4fbf5f3 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -918,28 +918,25 @@ CoreUninstallMultipleProtocolInterfaces (
   Locate a certain GUID protocol interface in a Handle's protocols.
 
   @param  UserHandle The handle to obtain the protocol interface on
+ The caller must pass in a valid UserHandle 
that
+ is checked with CoreValidateHandle().
   @param  Protocol   The GUID of the protocol
 
   @return The requested protocol interface for the handle
 
 **/
+STATIC
 PROTOCOL_INTERFACE  *
 CoreGetProtocolInterface (
   IN  EFI_HANDLE  UserHandle,
   IN  EFI_GUID*Protocol
   )
 {
-  EFI_STATUS  Status;
   PROTOCOL_ENTRY  *ProtEntry;
   PROTOCOL_INTERFACE  *Prot;
   IHANDLE *Handle;
   LIST_ENTRY  *Link;
 
-  Status = CoreValidateHandle (UserHandle);
-  if (EFI_ERROR (Status)) {
-return NULL;
-  }
-
   Handle = (IHANDLE *)UserHandle;
 
   //
@@ -1392,6 +1389,15 @@ CoreOpenProtocolInformation (
   //
   CoreAcquireProtocolLock ();
 
+  //
+  // Check for invalid UserHandle
+  //
+  Status = CoreValidateHandle (UserHandle);
+  if (EFI_ERROR (Status)) {
+Status = EFI_NOT_FOUND;
+goto Done;
+  }
+
   //
   // Look at each protocol interface for a match
   //
-- 
2.39.2



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