[edk2-devel] [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

[Jacque Lin via groups.io]

Remove checking SMM Rev ID in AMD Save State lib when reading Save
State Register EFI_MM_SAVE_STATE_REGISTER_IO.
For AMD, it is not necessary to check SmmRevId when reading Save State
Register EFI_MM_SAVE_STATE_REGISTER_IO.

Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Signed-off-by: Jacque Lin 
---
 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
index 3315a6cc44..c4bf6ad4bb 100644
--- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
+++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
@@ -102,7 +102,6 @@ MmSaveStateReadRegister (
   OUT VOID*Buffer

   )

 {

-  UINT32 SmmRevId;

   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;

   AMD_SMRAM_SAVE_STATE_MAP   *CpuSaveState;

   UINT8  DataWidth;

@@ -124,18 +123,6 @@ MmSaveStateReadRegister (
 

   // Check for special EFI_MM_SAVE_STATE_REGISTER_IO

   if (Register == EFI_MM_SAVE_STATE_REGISTER_IO) {

-//

-// Get SMM Revision ID

-//

-MmSaveStateReadRegisterByIndex (CpuIndex, 
AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );

-

-//

-// See if the CPU supports the IOMisc register in the save state

-//

-if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {

-  return EFI_NOT_FOUND;

-}

-

 // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.

 if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {

   return EFI_NOT_FOUND;

-- 
2.36.1.windows.1



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

Re: [edk2-devel] [PATCH v7 0/5] Cache Management Operations Support For RISC-V

[Dhaval Sharma]

Here we go. https://github.com/tianocore/edk2/pull/4974


On Tue, Oct 31, 2023 at 9:46 AM Warkentin, Andrei <
andrei.warken...@intel.com> wrote:

> Hi Dhaval,
>
> Do you mind sharing the repo with the full patch set? Like a github link?
>
> A
>
> > -Original Message-
> > From: Dhaval 
> > Sent: Sunday, October 29, 2023 9:46 AM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel ; Yao, Jiewen
> > ; Justen, Jordan L ;
> Gerd
> > Hoffmann ; Sunil V L ;
> > Warkentin, Andrei ; Laszlo Ersek
> > ; Kinney, Michael D ;
> > Gao, Liming ; Liu, Zhiguang
> > ; Daniel Schaefer 
> > Subject: [PATCH v7 0/5] Cache Management Operations Support For RISC-V
> >
> > Implementing code to support Cache Management Operations (CMO) defined
> > by RISC-V CMO instructions.https://github.com/riscv/riscv-CMOs
> > This is a re-write of original series v5.
> > The patchset contains 5 patches- created based on V5 feedback.
> > 1. Restructuring of existing code and move instruction declarations into
> > BaseLib 2. Renaming existing functions to denote type of instruction
> used to
> > maanage cache.
> >This is useful for further patches where more cache management
> > instructions are added.
> > 3. Add the new cache maintenance operations to BaseLib, including the
> >new assembly instruction encodings.
> > 4. Update BaseCacheMaintenanceLib (utilizing the new BaseLib primitives)
> 5.
> > Add platform level PCD to allow overriding of RISC-V features.
> >
> > Cc: Ard Biesheuvel 
> > Cc: Jiewen Yao 
> > Cc: Jordan Justen 
> > Cc: Gerd Hoffmann 
> > Cc: Sunil V L 
> > Cc: Andrei Warkentin 
> > Cc: Laszlo Ersek 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Daniel Schaefer 
> >
> > Dhaval (5):
> >   MdePkg: Move RISC-V Cache Management Declarations Into BaseLib
> >   MdePkg: Rename Cache Management Function To Clarify Fence Based Op
> >   MdePkg: Implement RISC-V Cache Management Operations
> >   MdePkg: Utilize Cache Management Operations Implementation For RISC-V
> >   OvmfPkg/RiscVVirt: Override for RV CPU Features
> >
> >  MdePkg/MdePkg.dec  |
>  8 +
> >  OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc|
>  1 +
> >  MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf |
> > 5 +
> >  MdePkg/Library/BaseLib/BaseLib.inf |
>  2 +-
> >  MdePkg/Include/Library/BaseLib.h   |
> 53 ++
> >  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 172
> > 
> >  MdePkg/Include/RiscV64/RiscVasm.inc|
> 19 +++
> >  MdePkg/Library/BaseLib/RiscV64/FlushCache.S|
> 21 ---
> >  MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S|
> 38 +
> >  MdePkg/MdePkg.uni  |
>  4 +
> >  10 files changed, 269 insertions(+), 54 deletions(-)  create mode 100644
> > MdePkg/Include/RiscV64/RiscVasm.inc
> >  delete mode 100644 MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> >  create mode 100644 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> >
> > --
> > 2.39.2
>
>

-- 
Thanks!
=D


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

Re: [edk2-devel] [PATCH v7 0/5] Cache Management Operations Support For RISC-V

[Andrei Warkentin]

Hi Dhaval,

Do you mind sharing the repo with the full patch set? Like a github link?

A

> -Original Message-
> From: Dhaval 
> Sent: Sunday, October 29, 2023 9:46 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen
> ; Justen, Jordan L ; Gerd
> Hoffmann ; Sunil V L ;
> Warkentin, Andrei ; Laszlo Ersek
> ; Kinney, Michael D ;
> Gao, Liming ; Liu, Zhiguang
> ; Daniel Schaefer 
> Subject: [PATCH v7 0/5] Cache Management Operations Support For RISC-V
> 
> Implementing code to support Cache Management Operations (CMO) defined
> by RISC-V CMO instructions.https://github.com/riscv/riscv-CMOs
> This is a re-write of original series v5.
> The patchset contains 5 patches- created based on V5 feedback.
> 1. Restructuring of existing code and move instruction declarations into
> BaseLib 2. Renaming existing functions to denote type of instruction used to
> maanage cache.
>This is useful for further patches where more cache management
> instructions are added.
> 3. Add the new cache maintenance operations to BaseLib, including the
>new assembly instruction encodings.
> 4. Update BaseCacheMaintenanceLib (utilizing the new BaseLib primitives) 5.
> Add platform level PCD to allow overriding of RISC-V features.
> 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Sunil V L 
> Cc: Andrei Warkentin 
> Cc: Laszlo Ersek 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Daniel Schaefer 
> 
> Dhaval (5):
>   MdePkg: Move RISC-V Cache Management Declarations Into BaseLib
>   MdePkg: Rename Cache Management Function To Clarify Fence Based Op
>   MdePkg: Implement RISC-V Cache Management Operations
>   MdePkg: Utilize Cache Management Operations Implementation For RISC-V
>   OvmfPkg/RiscVVirt: Override for RV CPU Features
> 
>  MdePkg/MdePkg.dec  |   8 +
>  OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc|   1 +
>  MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf |
> 5 +
>  MdePkg/Library/BaseLib/BaseLib.inf |   2 +-
>  MdePkg/Include/Library/BaseLib.h   |  53 
> ++
>  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 172
> 
>  MdePkg/Include/RiscV64/RiscVasm.inc|  19 +++
>  MdePkg/Library/BaseLib/RiscV64/FlushCache.S|  21 ---
>  MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S|  38 
> +
>  MdePkg/MdePkg.uni  |   4 +
>  10 files changed, 269 insertions(+), 54 deletions(-)  create mode 100644
> MdePkg/Include/RiscV64/RiscVasm.inc
>  delete mode 100644 MdePkg/Library/BaseLib/RiscV64/FlushCache.S
>  create mode 100644 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> 
> --
> 2.39.2



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

Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features

[Andrei Warkentin]

Looks fine to me. While it's not necessary to add a default value PCD to every 
platform DSC, it might me a good idea to do so for "example" platforms like 
RiscVVirt, so that folks who borrow code are aware of the feature?

Reviewed-by: Andrei Warkentin  

> -Original Message-
> From: Dhaval 
> Sent: Sunday, October 29, 2023 9:46 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen
> ; Justen, Jordan L ; Gerd
> Hoffmann ; Sunil V L ;
> Warkentin, Andrei ; Laszlo Ersek
> 
> Subject: [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features
> 
> This PCD provides a way for platform to override any HW features that are
> default enabled by previous stages of FW (like OpenSBI). For the case where
> previous/prev stage has disabled the feature, this override is not useful and
> its usage should be avoided.
> 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Sunil V L 
> Cc: Andrei Warkentin 
> Cc: Laszlo Ersek 
> 
> Signed-off-by: Dhaval Sharma 
> Acked-by: Laszlo Ersek 
> ---
> 
> Notes:
> V7:
> - Added RB tag
> v6:
> - Modify PCD name according to changes made in Baselib implementation
> V5:
> - Introduce PCD for platform
> 
>  OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc
> b/OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc
> index fe320525153f..5d66f7fe6ae6 100644
> --- a/OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc
> +++ b/OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc
> @@ -203,6 +203,7 @@ [PcdsFeatureFlag]
>gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> [PcdsFixedAtBuild.common]+
> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0
> gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|100
> gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|100
> gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|0--
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110362): https://edk2.groups.io/g/devel/message/110362
Mute This Topic: https://groups.io/mt/102256471/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] Maintainers.txt: Update based on active community members

[Andrei Warkentin]

Reviewed-by: Andrei Warkentin 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Saturday, October 28, 2023 2:24 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Leif Lindholm
> ; Warkentin, Andrei
> ; West, Catharine ;
> Bi, Dandan ; Daniel Schaefer
> ; David Woodhouse ; De,
> Debkumar ; Dong, Eric ;
> Jiang, Guomin ; Wu, Hao A ;
> James Bottomley ; Wang, Jian J
> ; Justen, Jordan L ; Julien
> Grall ; Peter Grehan ; Zhang, Qi1
> ; Ng, Ray Han Lim ; Stefan
> Berger ; Hou, Wenxing ;
> Lu, Xiaoyu1 
> Subject: [Patch 1/1] Maintainers.txt: Update based on active community
> members
> 
> Over the past few months, all the of the Maintainers and Reviewers listed in
> Maintainers.txt have been contacted to make sure Maintainers.txt accurately
> represents the TianoCore community members that are actively participating
> in their roles.  Based on specific feedback, bounced emails, and no
> responses, updates have been made.
> 
> * RISCV64: Daniel Schaefer replaced with Andrei Warkentin
> * ArmVirtPkg Xen has no remaining reviewers and review
>   responsibility defaults to ArmVirtPkg Maintainers/Reviewers.
> * ACPI modules related to S3 has no remaining reviewers and
>   review responsibility defaults to MdeModulePkg Maintainers/
>   Reviewers.
> * OVMF CSM modules has no remaining reviewers and review
>   responsibility defaults to OvmfPkg Maintainers/Reviewers.
> * Bounce: Chan Laura 
> * Many smaller updates removing individuals that are no
>   longer involved or have replacement coverage.
> 
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Andrei Warkentin 
> Cc: Catharine West 
> Cc: Dandan Bi 
> Cc: Daniel Schaefer 
> Cc: David Woodhouse 
> Cc: Debkumar De 
> Cc: Eric Dong 
> Cc: Guomin Jiang 
> Cc: Hao A Wu 
> Cc: James Bottomley 
> Cc: Jian J Wang 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Peter Grehan 
> Cc: Qi Zhang 
> Cc: Ray Han Lim Ng 
> Cc: Stefan Berger 
> Cc: Wenxing Hou 
> Cc: Xiaoyu Lu 
> Signed-off-by: Michael D Kinney 
> ---
>  Maintainers.txt | 53 ++---
>  1 file changed, 2 insertions(+), 51 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt index
> 3f40cdeb5554..2b03ccbe54aa 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -93,7 +93,7 @@ M: Sami Mujawar 
> [samimujawar]
>  RISCV64
>  F: */RiscV64/
>  M: Sunil V L  [vlsunil]
> -R: Daniel Schaefer  [JohnAZoidberg]
> +R: Andrei Warkentin  [andreiw]
> 
>  LOONGARCH64
>  F: */LoongArch64/
> @@ -157,16 +157,6 @@ R: Leif Lindholm 
> [leiflindholm]
>  R: Sami Mujawar  [samimujawar]
>  R: Gerd Hoffmann  [kraxel]
> 
> -ArmVirtPkg: modules used on Xen
> -F: ArmVirtPkg/ArmVirtXen.*
> -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
> -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
> -F: ArmVirtPkg/PrePi/
> -F: ArmVirtPkg/XenAcpiPlatformDxe/
> -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
> -F: ArmVirtPkg/XenioFdtDxe/
> -R: Julien Grall  [jgrall]
> -
>  BaseTools
>  F: BaseTools/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> @@ -187,8 +177,7 @@ F: CryptoPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>  M: Jiewen Yao  [jyao1]
>  M: Yi Li  [liyi77]
> -R: Xiaoyu Lu  [xiaoyuxlu]
> -R: Guomin Jiang  [guominjia]
> +R: Wenxing Hou  [Wenxing-hou]
> 
>  DynamicTablesPkg
>  F: DynamicTablesPkg/
> @@ -202,7 +191,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
>  M: Leif Lindholm  [leiflindholm]
>  M: Ard Biesheuvel  [ardbiesheuvel]
>  M: Abner Chang  [changab]
> -R: Daniel Schaefer  [JohnAZoidberg]
> 
>  EmulatorPkg
>  F: EmulatorPkg/
> @@ -228,7 +216,6 @@ F: FmpDevicePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/FmpDevicePkg
>  M: Liming Gao  [lgao4]
>  M: Michael D Kinney  [mdkinney]
> -R: Guomin Jiang  [guominjia]
>  R: Wei6 Xu  [xuweiintel]
> 
>  IntelFsp2Pkg
> @@ -237,7 +224,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg
>  M: Chasel Chiu  [ChaselChiu]
>  M: Nate DeSimone  [nate-desimone]
>  M: Duggapu Chinni B  [cbduggap]
> -M: Ray Han Lim Ng  [rayhanlimng]
>  R: Star Zeng  [lzeng14]
>  R: Ted Kuo  [tedkuo1]
>  R: Ashraf Ali S  [AshrafAliS] @@ -258,7 +244,6 @@
> R: Susovan Mohapatra  [susovanmohapatra]
> MdeModulePkg
>  F: MdeModulePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> -M: Jian J Wang  [jwang36]
>  M: Liming Gao  [lgao4]
> 
>  MdeModulePkg: ACPI modules
> @@ -268,15 +253,6 @@ R: Zhiguang Liu 
> [LiuZhiguang001]
>  R: Dandan Bi  [dandanbi]
>  R: Liming Gao  [lgao4]
> 
> -MdeModulePkg: ACPI modules related to S3
> -F: MdeModulePkg/*LockBox*/
> -F: MdeModulePkg/Include/*BootScript*.h
> -F: MdeModulePkg/Include/*LockBox*.h
> -F: MdeModulePkg/Include/*S3*.h
> -F: MdeModulePkg/Library/*S3*/
> -R: Hao A Wu  [hwu25]
> -R: Eric Dong  [ydong10]
> -
>  MdeModulePkg: BDS modules
>  F: MdeModulePkg/*BootManager*/
>  F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -326,7 +302,6 @@ F:
> 

Re: [edk2-devel] [PATCH v1 1/1] .github/workflows: Add Stale Check

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

Mike

> -Original Message-
> From: mikub...@linux.microsoft.com 
> Sent: Monday, October 30, 2023 6:41 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan ; Michael Kubacki
> ; Kinney, Michael D
> ; Laszlo Ersek 
> Subject: [PATCH v1 1/1] .github/workflows: Add Stale Check
> 
> From: Michael Kubacki 
> 
> Adds a GitHub workflow that uses the actions/stale GitHub action to
> automatically leave notifications on and close PRs that have had no
> activity for a long time.
> 
> Note: Modifications to a PR reset the staleness counter. This
>   includes pushing to the PR, adding a label to the PR,
>   commenting on the PR, etc.
> 
>   If a PR has been marked "stale", simply leaving a comment will
>   reset the counter.
> 
> Configuration choices:
> 
> 1. Do not attempt to close edk2 GitHub issues.
> 2. Mark edk2 PRs as stale if no activity in the last 60 days. Close
>PRs marked stale if no further activity in 7 days.
> 3. Do not exempt PRs with a "push" label.
> 4. Run the check once daily. Allow manual runs from those that have
>permission to run GitHub workflows.
> 5. Add the label "stale" to the PR when it enters the stale state.
> 
> Rationale:
> 
> 1. We do not use issues often enough. The limited usage of GitHub
>issues in Tianocore org GitHub projects are in another repo not
>impacted by this workflow and expected to track long term tasks.
> 2. This is the default value. In non-edk2 projects, I've seen these
>times work fairly well to identify PRs that have fallen stale.
> 3. Adding a "push" label resets the stale timer. If a PR has had a
>"push" label for 60+ days and has not been fixed for submission,
>then it is has very likely been abandoned.
> 4. This is sufficient to update PRs on the day granularity the
>configuration settings are applied against.
> 5. The label makes it easy to filter stale PRs in the PR list and
>write automation around PRs that are stale. It's also an obvious
>visual identifier that a PR needs attention in the PR list.
> 
> Cc: Sean Brogan 
> Cc: Michael Kubacki 
> Cc: Michael D Kinney 
> Cc: Laszlo Ersek 
> Signed-off-by: Michael Kubacki 
> ---
> 
> Notes:
> I tested this workflow on my edk2 fork:
> 
> https://github.com/makubacki/edk2/actions/runs/6700887619
> 
> Here's an example of a PR it did not mark stale there:
> 
> https://github.com/makubacki/edk2/pull/136
> 
> Here's an example of a PR it did mark stale there:
> 
> https://github.com/makubacki/edk2/pull/4
> 
>  .github/workflows/stale.yml | 44 
>  1 file changed, 44 insertions(+)
> 
> diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
> new file mode 100644
> index ..b9160b548ab3
> --- /dev/null
> +++ b/.github/workflows/stale.yml
> @@ -0,0 +1,44 @@
> +# This workflow warns and then closes issues and PRs that have had no
> activity
> +# for a specified amount of time.
> +#
> +# For more information, see:
> +# https://github.com/actions/stale
> +#
> +# Copyright (c) Microsoft Corporation.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +name: Stale Check
> +
> +on:
> +  schedule:
> +# At 23:35 on every day-of-week from Sunday through Saturday
> +# https://crontab.guru/#35_23_*_*_0-6
> +- cron: '35 23 * * 0-6'
> +  workflow_dispatch:
> +
> +jobs:
> +  stale:
> +name: Stale
> +runs-on: ubuntu-latest
> +permissions:
> +  issues: write
> +  pull-requests: write
> +
> +steps:
> +- name: Check for Stale Items
> +  uses: actions/stale@v8
> +  with:
> +days-before-issue-close: -1
> +days-before-issue-stale: -1
> +days-before-pr-stale: 60
> +days-before-pr-close: 7
> +stale-pr-message: >
> +  This PR has been automatically marked as stale because it
> has not had
> +  activity in 60 days. It will be closed if no further
> activity occurs within
> +  7 days. Thank you for your contributions.
> +close-pr-message: >
> +  This pull request has been automatically been closed
> because it did not have any
> +  activity in 60 days and no follow up within 7 days after
> being marked stale.
> +  Thank you for your contributions.
> +stale-pr-label: stale
> --
> 2.42.0.windows.2



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

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

[Michael D Kinney]

Does using a static volatile global instead of a volatile local work?

Mike

From: Ni, Ray 
Sent: Monday, October 30, 2023 7:52 PM
To: Andrew (EFI) Fish ; edk2-devel-groups-io 
; Rebecca Cran ; Hernandez Miramontes, 
Jose Miguel 
Cc: Kinney, Michael D 
Subject: Re: [edk2-devel] CpuDeadLoop() is optimized by compiler

It's been a while.

Is there any better solution? Can we go with assembly solution?

Thanks,
Ray

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

I don't think the atomic is going to help. The compiler honored the volatile by 
doing a read, but assumed it would never change due to scoping. As you can see 
in my example if the compiler thinks DeadLoopCount can be changed it will put 
the check back in and assume the function can return. So an assembly function 
that does nothing called IncreaseScope()
 would fix this issue too.


void IncreaseScope(int *ptr);



void CpuDeadLoopFix(void) {

volatile
int DeadLoopCount = 0;

while(DeadLoopCount ==
0) {

IncreaseScope();

}

}



void CpuDeadLoop(void) {

volatile
int DeadLoopCount = 0;

while(DeadLoopCount ==
0);

}



Gives us:


voltbl
SEGMENT

voltbl
ENDS

voltbl
SEGMENT

voltbl
ENDS



DeadLoopCount$ =
48

CpuDeadLoopFix PROC
; COMDAT

$LN12:

sub
rsp, 40
; 0028H

mov
DWORD PTR
DeadLoopCount$[rsp],
0

jmp
SHORT $LN10@CpuDeadLoo

$LL2@CpuDeadLoo:

lea
rcx, QWORD
PTR DeadLoopCount$[rsp]

call
IncreaseScope

$LN10@CpuDeadLoo:

mov
eax, DWORD
PTR DeadLoopCount$[rsp]

test
eax, eax

je
SHORT $LL2@CpuDeadLoo

add
rsp, 40
; 0028H

ret
0

CpuDeadLoopFix ENDP



DeadLoopCount$ =
8

CpuDeadLoop PROC
; COMDAT

mov
DWORD PTR
DeadLoopCount$[rsp],
0

$LL2@CpuDeadLoo:

mov
eax, DWORD
PTR DeadLoopCount$[rsp]

jmp
SHORT $LL2@CpuDeadLoo

CpuDeadLoop ENDP



Compiler 
Explorer
godbolt.org

[edk2-devel] [PATCH edk2-platforms 1/1] IpmiFeaturePkg/SmmIpmiBaseLib: Support Standalone MM.

[Xu, Wei6]

Add MM_STANDALONE type support for SmmIpmiBaseLib instance. Replace Smst
with Mmst. Remove unused UefiBootServicesTableLib reference.

Cc: Abner Chang 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Wei6 Xu 
---
 .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c | 17 -
 .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf   |  5 ++---
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c
index 6282adc269f8..1c618e48abee 100644
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c
@@ -6,11 +6,10 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-#include 
+#include 
 #include 
 #include 
-#include 
-#include 
+#include 
 #include 
 
 STATIC IPMI_TRANSPORT *mIpmiTransport = NULL;
@@ -37,7 +36,7 @@ NotifyIpmiTransportCallback (
   EFI_STATUS  Status;
   Status = EFI_SUCCESS;
   if (mIpmiTransport == NULL) {
-Status = gSmst->SmmLocateProtocol (
+Status = gMmst->MmLocateProtocol (
   ,
   NULL,
   (VOID **) 
@@ -60,15 +59,15 @@ InitializeIpmiBase (
 {
   EFI_STATUS  Status;
   if (mIpmiTransport == NULL) {
-Status = gSmst->SmmLocateProtocol (
+Status = gMmst->MmLocateProtocol (
   ,
   NULL,
   (VOID **) 
   );
 if (EFI_ERROR (Status)) {
-  Status = gSmst->SmmRegisterProtocolNotify (
+  Status = gMmst->MmRegisterProtocolNotify (
 ,
-(EFI_SMM_NOTIFY_FN) NotifyIpmiTransportCallback,
+(EFI_MM_NOTIFY_FN) NotifyIpmiTransportCallback,
 
 );
 }
@@ -127,7 +126,7 @@ Returns:
 {
   EFI_STATUS  Status;
 
-  Status = gSmst->SmmLocateProtocol (, NULL, 
(VOID **) );
+  Status = gMmst->MmLocateProtocol (, NULL, 
(VOID **) );
   if (EFI_ERROR (Status)) {
 ASSERT_EFI_ERROR (Status);
 return Status;
@@ -164,7 +163,7 @@ GetBmcStatus (
 {
   EFI_STATUS  Status;
 
-  Status = gSmst->SmmLocateProtocol (, NULL, 
(VOID **) );
+  Status = gMmst->MmLocateProtocol (, NULL, 
(VOID **) );
   if (EFI_ERROR (Status)) {
 ASSERT_EFI_ERROR (Status);
 return Status;
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf
index 7ea688fc165f..cf6b91c471fc 100644
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf
@@ -11,7 +11,7 @@
   FILE_GUID  = 2B5AD78E-5CF8-45d2-B2AC-749A09425911
   MODULE_TYPE= DXE_SMM_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = IpmiBaseLib|DXE_SMM_DRIVER SMM_CORE
+  LIBRARY_CLASS  = IpmiBaseLib|DXE_SMM_DRIVER SMM_CORE 
MM_STANDALONE
 
 [sources]
   SmmIpmiBaseLib.c
@@ -21,9 +21,8 @@
   IpmiFeaturePkg/IpmiFeaturePkg.dec
 
 [LibraryClasses]
-  UefiBootServicesTableLib
   DebugLib
-  SmmServicesTableLib
+  MmServicesTableLib
 
 [Protocols]
   gSmmIpmiTransportProtocolGuid
-- 
2.29.2.windows.2



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

[edk2-devel] [PATCH edk2-platforms 0/1] Add Standalone MM support for SmmIpmiBaseLib

[Xu, Wei6]

This patch is to add Standalone MM support for SmmIpmiBaseLib.
PR: https://github.com/tianocore/edk2-platforms/pull/107

Cc: Abner Chang 
Cc: Nate DeSimone 
Cc: Liming Gao 

Wei6 Xu (1):
  IpmiFeaturePkg/SmmIpmiBaseLib: Support Standalone MM.

 .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c | 17 -
 .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf   |  5 ++---
 2 files changed, 10 insertions(+), 12 deletions(-)

-- 
2.29.2.windows.2



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

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

[Ni, Ray]

It's been a while.

Is there any better solution? Can we go with assembly solution?

Thanks,
Ray

From: Andrew (EFI) Fish 
Sent: Saturday, May 20, 2023 12:31 AM
To: edk2-devel-groups-io ; Rebecca Cran 

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

I don’t think the atomic is going to help. The compiler honored the volatile by 
doing a read, but assumed it would never change due to scoping. As you can see 
in my example if the compiler thinks DeadLoopCount can be changed it will put 
the check back in and assume the function can return. So an assembly function 
that does nothing called IncreaseScope()  would fix this issue too.

void IncreaseScope(int *ptr);

void CpuDeadLoopFix(void) {
volatile int DeadLoopCount = 0;
while(DeadLoopCount == 0) {
IncreaseScope();
}
}

void CpuDeadLoop(void) {
volatile int DeadLoopCount = 0;
while(DeadLoopCount == 0);
}

Gives us:

voltbl SEGMENT
voltbl ENDS
voltbl SEGMENT
voltbl ENDS

DeadLoopCount$ = 48
CpuDeadLoopFix PROC ; COMDAT
$LN12:
sub rsp, 40 ; 0028H
mov DWORD PTR DeadLoopCount$[rsp], 0
jmp SHORT $LN10@CpuDeadLoo
$LL2@CpuDeadLoo:
lea rcx, QWORD PTR DeadLoopCount$[rsp]
call IncreaseScope
$LN10@CpuDeadLoo:
mov eax, DWORD PTR DeadLoopCount$[rsp]
test eax, eax
je SHORT $LL2@CpuDeadLoo
add rsp, 40 ; 0028H
ret 0
CpuDeadLoopFix ENDP

DeadLoopCount$ = 8
CpuDeadLoop PROC ; COMDAT
mov DWORD PTR DeadLoopCount$[rsp], 0
$LL2@CpuDeadLoo:
mov eax, DWORD PTR DeadLoopCount$[rsp]
jmp SHORT $LL2@CpuDeadLoo
CpuDeadLoop ENDP



Compiler 
Explorer

[edk2-devel] [PATCH v1 1/1] .github/workflows: Add Stale Check

[Michael Kubacki]

From: Michael Kubacki 

Adds a GitHub workflow that uses the actions/stale GitHub action to
automatically leave notifications on and close PRs that have had no
activity for a long time.

Note: Modifications to a PR reset the staleness counter. This
  includes pushing to the PR, adding a label to the PR,
  commenting on the PR, etc.

  If a PR has been marked "stale", simply leaving a comment will
  reset the counter.

Configuration choices:

1. Do not attempt to close edk2 GitHub issues.
2. Mark edk2 PRs as stale if no activity in the last 60 days. Close
   PRs marked stale if no further activity in 7 days.
3. Do not exempt PRs with a "push" label.
4. Run the check once daily. Allow manual runs from those that have
   permission to run GitHub workflows.
5. Add the label "stale" to the PR when it enters the stale state.

Rationale:

1. We do not use issues often enough. The limited usage of GitHub
   issues in Tianocore org GitHub projects are in another repo not
   impacted by this workflow and expected to track long term tasks.
2. This is the default value. In non-edk2 projects, I've seen these
   times work fairly well to identify PRs that have fallen stale.
3. Adding a "push" label resets the stale timer. If a PR has had a
   "push" label for 60+ days and has not been fixed for submission,
   then it is has very likely been abandoned.
4. This is sufficient to update PRs on the day granularity the
   configuration settings are applied against.
5. The label makes it easy to filter stale PRs in the PR list and
   write automation around PRs that are stale. It's also an obvious
   visual identifier that a PR needs attention in the PR list.

Cc: Sean Brogan 
Cc: Michael Kubacki 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Signed-off-by: Michael Kubacki 
---

Notes:
I tested this workflow on my edk2 fork:

https://github.com/makubacki/edk2/actions/runs/6700887619

Here's an example of a PR it did not mark stale there:

https://github.com/makubacki/edk2/pull/136

Here's an example of a PR it did mark stale there:

https://github.com/makubacki/edk2/pull/4

 .github/workflows/stale.yml | 44 
 1 file changed, 44 insertions(+)

diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
new file mode 100644
index ..b9160b548ab3
--- /dev/null
+++ b/.github/workflows/stale.yml
@@ -0,0 +1,44 @@
+# This workflow warns and then closes issues and PRs that have had no activity
+# for a specified amount of time.
+#
+# For more information, see:
+# https://github.com/actions/stale
+#
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+name: Stale Check
+
+on:
+  schedule:
+# At 23:35 on every day-of-week from Sunday through Saturday
+# https://crontab.guru/#35_23_*_*_0-6
+- cron: '35 23 * * 0-6'
+  workflow_dispatch:
+
+jobs:
+  stale:
+name: Stale
+runs-on: ubuntu-latest
+permissions:
+  issues: write
+  pull-requests: write
+
+steps:
+- name: Check for Stale Items
+  uses: actions/stale@v8
+  with:
+days-before-issue-close: -1
+days-before-issue-stale: -1
+days-before-pr-stale: 60
+days-before-pr-close: 7
+stale-pr-message: >
+  This PR has been automatically marked as stale because it has not had
+  activity in 60 days. It will be closed if no further activity occurs 
within
+  7 days. Thank you for your contributions.
+close-pr-message: >
+  This pull request has been automatically been closed because it did 
not have any
+  activity in 60 days and no follow up within 7 days after being 
marked stale.
+  Thank you for your contributions.
+stale-pr-label: stale
-- 
2.42.0.windows.2



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

Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

[Wu, Hao A]

(Add MdePkg maintainers for their feedbacks)

Sorry that I do not have strong opinion on this one.

Some of my thoughts are:
* If you find the to-be-added APIs can be used in serveral places to reduce
  repetative codes, then it will be worthwhile to add new library APIs.

* TimerLib has many instance implementations, my take is that many post-mem
  instances use library level global variable to store information like timer
  frequency to avoid calculating it every time.
  For pre-mem instances, such caching is not feasible. I think it will be the
  API consumer's responsibility to limit the usage of these APIs for performance
  consideration.

Best Regards,
Hao Wu

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Henz,
> Patrick
> Sent: Tuesday, October 31, 2023 4:36 AM
> To: Wu, Hao A ; devel@edk2.groups.io; Michael Brown
> 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance
> Timer for XHCI Timeouts
> 
> My changes have been in for a little over a month now, I've been looking into
> updating the TimerLib API but I'm questioning if it still makes sense to go 
> down
> that path.  The functions I had implemented, XhcGetElapsedTicks () and
> XhcConvertTimeToTicks (), now rely heavily on the use of global variables for
> caching return values from GetPerformanceCounterProperties. As-is these
> functions won't work for all instances of TimerLib, specifically if the 
> instance is
> used before memory (RAM) is available. I could implement the functions as
> they originally were, but I wouldn't be able to replace the Xhc functions
> without adding additional parameters to said TimerLib functions (e.g. adding
> Frequency, StartValue, StopValue to ConvertTimeToTicks), which feels clunky
> to me. Would a new library make sense here? Something that sits between
> the driver and the TimerLib library? Or do we just leave things as they
> currently are?
> 
> Thanks,
> Patrick Henz
> 
> -Original Message-
> From: Wu, Hao A 
> Sent: Thursday, August 10, 2023 8:43 PM
> To: Henz, Patrick ; devel@edk2.groups.io; Michael
> Brown 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance
> Timer for XHCI Timeouts
> 
> My personal preference is to do this as two steps.
> Address the issue in XhciDxe first. Then add new TimerLib API to replace all
> driver/library internal implementations.
> 
> Best Regards,
> Hao Wu
> 
> > -Original Message-
> > From: Henz, Patrick 
> > Sent: Friday, August 11, 2023 6:45 AM
> > To: devel@edk2.groups.io; Wu, Hao A ; Michael
> > Brown 
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use
> > Performance Timer for XHCI Timeouts
> >
> > I can certainly make that change.
> >
> > For what it's worth I have been working on adding the new function,
> > GetElapsedTicks, to the various implementations of TimerLib. I've
> > finished up testing, I would just need to clean up the commits for a
> > patch. Should I move forward with that, or would we rather I just add
> > XhcGetElapsedTime to XhciDxe for the time being?
> >
> > Thanks,
> > Patrick Henz
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Wu,
> Hao
> > A
> > Sent: Sunday, July 30, 2023 9:57 PM
> > To: devel@edk2.groups.io; Henz, Patrick ;
> > Michael Brown 
> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use
> > Performance Timer for XHCI Timeouts
> >
> > For the 2 occurrences of in XhcWaitOpRegBit & XhcExecTransfer:
> >   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND (Timeout *
> > XHC_1_MILLISECOND); How about changing them to:
> >   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND ((UINT64)
> Timeout
> > * XHC_1_MILLISECOND); to address possible overflow during "Timeout *
> > XHC_1_MILLISECOND"?
> >
> > For extending XhcGetElapsedTime as a TimerLib API, I am fine to put it
> > in XhciDxe at this moment.
> > If package maintainers suggest to make it as a public library API, my
> > take is that this should be done in a separate commit.
> >
> > Best Regards,
> > Hao Wu
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Henz,
> > > Patrick
> > > Sent: Thursday, July 6, 2023 4:16 AM
> > > To: devel@edk2.groups.io
> > > Cc: Henz, Patrick 
> > > Subject: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance
> > > Timer for XHCI Timeouts
> > >
> > > REF:INVALID URI REMOVED
> > > bu
> > > g.cgi?id=2948__;!!NpxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-
> > NKbMjOV-lctg5
> > > _B3K1Lcta452Gx-1twRt8At3cueAYDq_n$
> > >
> > > XhciDxe uses the timer functionality provided by the boot services
> > > table to detect timeout conditions. This breaks the driver's
> > > ExitBootServices call back, as CoreExitBootServices halts the timer
> > > before signaling the ExitBootServices event. If the host controller
> > > fails to halt in the call back, the timeout condition will never
> > > occur and the boot gets stuck in an indefinite spin loop. Use the
> > > free running timer provided by TimerLib to calculate 

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

[Group Notification]

*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, October 31, 2023
6:30pm to 7:30pm
(UTC-07: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=2072473 )

*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 (#110352): https://edk2.groups.io/g/devel/message/110352
Mute This Topic: https://groups.io/mt/102289493/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 0/2] Add Platform Hook Lib into StandaloneMmCore

[Xu, Wei6]

Hi Michael and Laszlo,

Thanks a lot for the review.
We reconsidered the solution and find better way to implement the 'platform 
hook'. So Let's drop this patch.
Thanks Ray for suggesting a better solution.

BR,
Wei

-Original Message-
From: Michael Kubacki  
Sent: Tuesday, October 31, 2023 3:27 AM
To: devel@edk2.groups.io; ler...@redhat.com; Xu, Wei6 
Cc: Ard Biesheuvel ; Sami Mujawar 
; Ni, Ray 
Subject: Re: [edk2-devel] [PATCH 0/2] Add Platform Hook Lib into 
StandaloneMmCore

On 10/28/2023 10:03 AM, Laszlo Ersek wrote:
> On 10/27/23 18:08, Michael Kubacki wrote:
>> This allows ambiguous "platform" code in the critical path of the MM 
>> core. Is this necessary?
>>
>> Do you need this for one feature that others might too and can be 
>> abstracted? Or, do you plan to perform an unknown and arbitrary 
>> number of changes behind the hook over time?
> 
> Not sure if it's necessary, but it's somewhat "customary". Platform 
> hook libs are not uncommon; for example PiSmmCpuDxeSmm consumes 
> SmmCpuPlatformHookLib and SmmCpuFeaturesLib.
> 
> My request would be for Wei to file a TianoCore Feature Request 
> bugzilla, with a bit more information than "We need this library to 
> implement our feature". Reference the BZ in the commit messages, then 
> add the BZ to the 
> .
> 
That would be helpful. Hooks are sometimes necessary of course, but I generally 
consider these open-ended hook libraries a design wart. They introduce an 
inherently incohesive interface and a convenient place for new APIs to land 
instead of more focused interfaces that may be more appropriate. This extends 
to the implementation where, over time, it can become a mess of dependencies 
and unrelated code coupled together.

I'd like to see if there's a way to avoid that or such a library interface is 
the best choice.

> Laszlo
> 
>>
>> Thanks,
>> Michael
>>
>> On 10/26/2023 11:28 PM, Xu, Wei6 wrote:
>>> This patch set is to add StandaloneMmCorePlatformHookLib into 
>>> StandaloneMmCore.
>>>
>>> This library class defines a set of platform hooks called by the 
>>> Standalone Mm Core. With this library, platform can perform specific 
>>> tasks before and after invoking registered MMI handlers.
>>> We need this library to implement our feature.
>>>
>>> PR: https://github.com/tianocore/edk2/pull/4949
>>>
>>>
>>>
>>> Cc: Ard Biesheuvel 
>>>
>>> Cc: Sami Mujawar 
>>>
>>> Cc: Ray Ni 
>>>
>>>
>>> Wei6 Xu (2):
>>>     StandaloneMmPkg: Add Standalone Mm Core platform hook lib.
>>>     StandaloneMmPkg/Core: Consumes Standalone Mm Core Platform Hook Lib.
>>>
>>>    StandaloneMmPkg/Core/StandaloneMmCore.c   |  7 ++-
>>>    .../StandaloneMmCorePlatformHookLibNull.c | 45 
>>> +++
>>>    StandaloneMmPkg/Core/StandaloneMmCore.h   |  1 +
>>>    StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
>>>    .../Library/StandaloneMmCorePlatformHookLib.h | 44 
>>> ++
>>>    .../StandaloneMmCorePlatformHookLibNull.inf   | 30 +
>>>    StandaloneMmPkg/StandaloneMmPkg.dec   |  4 ++
>>>    StandaloneMmPkg/StandaloneMmPkg.dsc   |  2 +
>>>    8 files changed, 133 insertions(+), 1 deletion(-)
>>>    create mode 100644
>>> StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/Standalo
>>> neMmCorePlatformHookLibNull.c
>>>    create mode 100644
>>> StandaloneMmPkg/Include/Library/StandaloneMmCorePlatformHookLib.h
>>>    create mode 100644
>>> StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/Standalo
>>> neMmCorePlatformHookLibNull.inf
>>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110351): https://edk2.groups.io/g/devel/message/110351
Mute This Topic: https://groups.io/mt/102214566/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 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA

[Erdem Aktas via groups.io]

On Sun, Oct 29, 2023 at 11:42 PM Sun, CepingX  wrote:
>
> On Saturday, October 28, 2023 12:45 AM, Erdem Aktas wrote:
> This should be the [PATCH V1 2/2] I assume?
> Yes, the name is same with [PATCH v1 0/2] , may be confusion, I would update 
> in next version to avoid the same title name.
>
>
> On Thu, Oct 26, 2023 at 5:58 PM sunceping  wrote:
>  [Sources]
>VirtualMemory.h
>MemoryEncryption.c
> +  X64/TdVmCallMapGPA.nasm
> I do not think we need another TdVmCallMapGPA definition, do we?
> Currently,  the TdVmCall always clears the R11 if the return code is not 
> successful,  which means we need to change TdVmCall if we don't add 
> TdVmCallMapGPA.
> Refer the GHCI Spec , if the returns code is not successful, the R11 value is 
> not valid for the sub-functions except MapGPA,  which means  TdVmCall should 
> clear the value on
> unsuccessful returns and only save the value if MapGPA returns 
> unsuccessfully.  If an update is required, the logic in TdVmCall could be 
> complex.

It seems like TdVmCallMapGPA function is actually duplicating most of
the code that the TdVmCall function is already doing.
According to the spec, R11 has meaningful data when mapgpa has RETRY,
GPA_IN_USE or ALIGN_ERROR.
I do not think the TdVmCall change logic would be complex (or not more
than what TdVmCallMapGPA is already doing).
I would like to see what others are saying on this.

>
>  [LibraryClasses]
>BaseLib
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c 
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index b47f56b391a5..1f29f9194c30 100644
> --- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> +++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> @@ -38,6 +38,10 @@ typedef enum {
>
>  STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
>
> +#define TDVMCALL_STATUS_RETRY  0x1
> +
> +#define MAX_RETRIES_PER_PAGE  3
> +
>  /**
>This function is used to help request the host VMM to map a GPA range as
>private or shared-memory mappings.
> @@ -546,6 +550,13 @@ SetOrClearSharedBit (
>EFI_STATUSStatus;
>EDKII_MEMORY_ACCEPT_PROTOCOL  *MemoryAcceptProtocol;
>
> +  UINT64  MapGpaRetryaddr;
> Should be replaced with MapGpaRetryAddr for consistency in variable name 
> casing style ?
> Yes, it would be updated in next version.
>
> +  UINT32  RetryCount;
> +  UINT64  EndAddress;
> +
> +  MapGpaRetryaddr = 0;
> +  RetryCount  = 0;
> +
>AddressEncMask = GetMemEncryptionAddressMask ();
>
>//
> @@ -559,7 +570,30 @@ SetOrClearSharedBit (
>  PhysicalAddress   &= ~AddressEncMask;
>}
>
> -  TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
> +  while (RetryCount < MAX_RETRIES_PER_PAGE) {
> +TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, );
> Why not this?
>  TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, 
> );
> The TdVmCall always clears the R11 value when unsuccessful returns as above 
> comments, therefor add the TdVmCallMapGPA to handle it.
right, the tdvmcall does not handle the R11 correctly for mapGPA. I
think it should be an easy fix in that function instead of creating a
whole copy of that function.
Is there a reason why we think it is complicated?

>
> +if (TdStatus != TDVMCALL_STATUS_RETRY) {
> +  break;
> +}
> +
> +DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is 
> %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, 
> MapGpaRetryaddr));
> +
> +EndAddress = PhysicalAddress + Length;
> +if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > 
> EndAddress)) {
>  should be?
>  if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >= EndAddress))
> Yes, that’s right, it would be updated in next version.
>
> Thanks
> Ceping
>


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

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

[Group Notification]

*Tools, CI, Code base construction meeting series*

*When:*
Monday, October 30, 2023
4:30pm to 5:30pm
(UTC-07: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=2072476 )

*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 (#110349): https://edk2.groups.io/g/devel/message/110349
Mute This Topic: https://groups.io/mt/102287729/21656
Mute #cal-notice:https://edk2.groups.io/g/devel/mutehashtag/cal-notice
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 1/1] ArmVirtPkg/PlatformCI/ReadMe.md: Update contents

[Michael Kubacki]

From: Michael Kubacki 

Since the code is most regularly tested in CI, distro/versioning
details are updated to match the latest CI configuration.

CI has moved from Ubuntu 18.04 to Ubuntu 22.04 since the time of the
file's creation, but the code is actually built in a Fedora container
so Fedora is mentioned as the primary build/test environment.

Updates the following information:

- Build OS: Fedora 37 Linux
- Supported Configuration: Additional DSCs added
- Python: 3.12.x
- Packaging Tool: dnf instead of apt
- Container Details: Added
- Primary Build Example: QemuBuild.py instead of PlatformBuild.py

Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Julien Grall 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Signed-off-by: Michael Kubacki 
---

Notes:
I don't use ArmVirtPkg that often. I was reviewing the
file for the latest build instructions and realized it
was quite out of date, leading to this patch.

The project is using Python 3.11.x right now, but a
patch is going in the next day or so that has shown
Python 3.12 will work:
https://edk2.groups.io/g/devel/message/110323

So I went ahead and made the Python version in this
patch mention 3.12.x.

 ArmVirtPkg/PlatformCI/ReadMe.md | 53 +---
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/ArmVirtPkg/PlatformCI/ReadMe.md b/ArmVirtPkg/PlatformCI/ReadMe.md
index ee8d8cd61e73..c0f2b2a43b3c 100644
--- a/ArmVirtPkg/PlatformCI/ReadMe.md
+++ b/ArmVirtPkg/PlatformCI/ReadMe.md
@@ -5,28 +5,43 @@ to use the same Pytools based build infrastructure locally.
 
 ## Supported Configuration Details
 
-This solution for building and running ArmVirtPkg has only been validated with 
Ubuntu
-18.04 and the GCC5 toolchain. Two different firmware builds are supported and 
are
-described below.
+This solution for building and running ArmVirtPkg has been validated with 
Fedora
+37 Linux and the GCC5 toolchain. Two different firmware builds are supported
+and are described below.
 
-| Configuration name  | Architecture   | DSC File |Additional 
Flags |
-| :-- | :- | :-   | :  
 |
-| AARCH64 | AARCH64| ArmVirtQemu.dsc  | None   
 |
-| ARM | ARM| ArmVirtQemu.dsc  | None   
 |
+| Configuration name  | Architecture   | DSC File   | 
Additional Flags |
+| :-- | :- | :- | 
:|
+| AARCH64 - KVM Cloud HV  | AARCH64| ArmVirtCloudHv.dsc | None 
|
+| ARM - KVM Cloud HV  | ARM| ArmVirtCloudHv.dsc | None 
|
+| AARCH64 - kvmtool   | AARCH64| ArmVirtKvmTool.dsc | None 
|
+| ARM - kvmtool   | ARM| ArmVirtKvmTool.dsc | None 
|
+| AARCH64 - QEMU  | AARCH64| ArmVirtQemu.dsc| None 
|
+| ARM - QEMU  | ARM| ArmVirtQemu.dsc| None 
|
+| AARCH64 - QEMU Kernel   | AARCH64| ArmVirtQemuKernel.dsc  | None 
|
+| ARM - QEMU Kernel   | ARM| ArmVirtQemuKernel.dsc  | None 
|
+| AARCH64 - Xen HV| AARCH64| ArmVirtXen.dsc | None 
|
+| ARM - Xen HV| ARM| ArmVirtXen.dsc | None 
|
 
 ## EDK2 Developer environment
 
-- [Python 3.8.x - Download & Install](https://www.python.org/downloads/)
+- [Python 3.12.x - Download & Install](https://www.python.org/downloads/)
 - [GIT - Download & Install](https://git-scm.com/download/)
 - [QEMU - Download, Install, and add to your 
path](https://www.qemu.org/download/)
 - [Edk2 Source](https://github.com/tianocore/edk2)
-- Additional packages found necessary for Ubuntu 18.04
-  - apt-get install gcc g++ make uuid-dev
+- Additional packages found necessary for Fedora Linux 37
+  - dnf install gcc g++ make libuuid-devel
 
 Note: edksetup, Submodule initialization and manual installation of NASM, 
iASL, or
 the required cross-compiler toolchains are **not** required, this is handled 
by the
 Pytools build system.
 
+The code is built in CI using a container. The latest Fedora Linux 37 
container is
+available in this GitHub container registry feed
+[fedora-37-test](https://github.com/tianocore/containers/pkgs/container/containers%2Ffedora-37-test).
+
+The exact container version tested in CI is maintained in this file
+[edk2/.azurepipelines/templates/default.yml](https://github.com/tianocore/edk2/blob/HEAD/.azurepipelines/templates/defaults.yml).
+
 ## Building with Pytools for ArmVirtPkg
 
 If you are unfamiliar with Pytools, it is recommended to first read through
@@ -57,16 +72,16 @@ the generic set of edk2 [Build 
Instructions](https://github.com/tianocore/tianoc
 pip install --upgrade -r pip-requirements.txt
 

Re: [edk2-devel] [PATCH v3 4/4] ArmVirtPkg: Add varpolicy shell command

[Ard Biesheuvel]

On Mon, 30 Oct 2023 at 21:31,  wrote:
>
> From: Michael Kubacki 
>
> Adds the varpolicy EFI shell command to all DSC files that
> currently include other dynamic shell commands from ShellPkg.
>
> This command allows variable policies to be dumped in the EFI
> shell for convenient auditing and debug.
>
> Use the command in the EFI shell as follows:
>
> - `"varpolicy"` dumps platform variables
> - `"varpolicy -?"` shows help text
> - `"varpolicy -b"` pages output as expected
> - `"varpolicy -s"` shows accurate variable statistic information
> - `"varpolicy -p"` shows accurate UEFI variable policy information
> - `"varpolicy-v -b"` dumps all information including variable data hex dump
>
> Cc: Ard Biesheuvel 
> Cc: Gerd Hoffmann 
> Cc: Julien Grall 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Sami Mujawar 
> Signed-off-by: Michael Kubacki 

Reviewed-by: Ard Biesheuvel 

> ---
>  ArmVirtPkg/ArmVirt.dsc.inc   | 4 
>  ArmVirtPkg/ArmVirtCloudHv.fdf| 1 +
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
>  ArmVirtPkg/ArmVirtXen.fdf| 1 +
>  4 files changed, 7 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 4ed86b979e1a..fe6488ee9910 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -393,6 +393,10 @@ [Components.common]
>  
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>}
> +  
> ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
>  {
> +
> +  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> +  }
>OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
>  
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
> index a5f172d79bfc..56d1ea6e8c1b 100644
> --- a/ArmVirtPkg/ArmVirtCloudHv.fdf
> +++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
> @@ -169,6 +169,7 @@ [FV.FvMain]
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> +  INF 
> ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
>INF 
> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
>
>#
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
> b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 2894bc853a46..9b3e37d5c998 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -103,6 +103,7 @@ [FV.FvMain]
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> +  INF 
> ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
>INF 
> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
>
>#
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 770fbf7289be..ea835551b375 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -180,6 +180,7 @@ [FV.FvMain]
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> +  INF 
> ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
>INF 
> OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
>
>#
> --
> 2.42.0.windows.2
>


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

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

[Group Notification]

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

*When:*
Monday, October 30, 2023
4:30pm to 5:30pm
(UTC-07: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=2072476 )

*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 (#110346): https://edk2.groups.io/g/devel/message/110346
Mute This Topic: https://groups.io/mt/102265424/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 1/1] Maintainers.txt: Update based on active community members

[Michael D Kinney]

Hi Laszlo,

I do not support orphaned categories and that option should be
removed from Maintainer.txt. One of the motivations to get 
Maintainers.txt updated is to work on the set of tasks related to 
using GitHub PRs for code review.  If a component is orphaned,
then nobody would be assigned to a PR in that area and the PR
would be stuck and would eventually be deleted for no activity.
A terrible experience for a submitter.

If there is a feature for which there is no longer any support,
then I recommend we find a way to remove it from the head of the
repository.  The feature is still available in the history and
in previous releases when it was supported.

If there is a future need for the feature and there are those that
are willing to support it, it can always be resurrected from the
history.

If it is a critical feature that will break the entire project 
if it is removed, then we must find community members that are 
willing to own it.

The immediate backup for this scenario is the EDK II Stewards, but
They may not have the background on the specific feature to maintain
it well.  For example, I am currently helping with the NetworkPkg
because there are no maintainers and I have been recruiting without
success.

I would like the see the SignedCapulePkg removed.  There are a
couple platforms in edk2-platforms that depend on it.  There is
another task to review the actively supported platforms in 
edk2-platforms.  If those platforms are removed, then SignedCapulsePkg
could be safely removed from the head of edk2.

SourceLevelDebugPkg has a similar issues of no maintainers.  The
platforms maintained in edk2 repo do not depend on it to do source
level debug.  It is more of a physical platform debug capability.
Perhaps this feature should be moved to the edk2-platform.  There
was a brief discuss at the UEFI Plugfest to update this debug 
feature because the current one depends on very old tools.

Mike

> -Original Message-
> From: Laszlo Ersek 
> Sent: Monday, October 30, 2023 4:29 AM
> To: Kinney, Michael D ; Yao, Jiewen
> ; devel@edk2.groups.io; pedro.falc...@gmail.com
> Cc: Andrew Fish ; Leif Lindholm
> ; Warkentin, Andrei
> ; West, Catharine
> ; Bi, Dandan ; Daniel
> Schaefer ; David Woodhouse
> ; De, Debkumar ; Dong,
> Eric ; Jiang, Guomin ;
> Wu, Hao A ; James Bottomley ;
> Wang, Jian J ; Justen, Jordan L
> ; Julien Grall ; Peter
> Grehan ; Zhang, Qi1 ; Ng, Ray
> Han Lim ; Stefan Berger
> ; Hou, Wenxing ; Lu,
> Xiaoyu1 
> Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on
> active community members
> 
> On 10/30/23 06:31, Kinney, Michael D wrote:
> > There is a very good discussion here on the roles and responsibility
> > and potential suggestions for changes to the Wiki page that document
> > those roles and responsibilities.
> >
> > May I suggest that someone start a new thread that discusses
> > the proposed changes to the Wiki page and leave this thread for the
> > review of the changes to Maintainers.txt?
> 
> These are connected topics, but yes, back to "Maintainers.txt" -- do
> you
> feel that "S: Orphan" sections are acceptable in general? They're a
> first (AFAICT) for edk2's "Maintainers.txt"; we've always had the
> mechanism documented (since the creation of "Maintainers.txt", or so),
> but we never seem to have put it to use.
> 
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110345): https://edk2.groups.io/g/devel/message/110345
Mute This Topic: https://groups.io/mt/102245264/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] MdeModulePkg/XhciDxe: Use Performance Timer for XHCI Timeouts

[Henz, Patrick]

My changes have been in for a little over a month now, I've been looking into 
updating the TimerLib API but I'm questioning if it still makes sense to go 
down that path.  The functions I had implemented, XhcGetElapsedTicks () and 
XhcConvertTimeToTicks (), now rely heavily on the use of global variables for 
caching return values from GetPerformanceCounterProperties. As-is these 
functions won't work for all instances of TimerLib, specifically if the 
instance is used before memory (RAM) is available. I could implement the 
functions as they originally were, but I wouldn't be able to replace the Xhc 
functions without adding additional parameters to said TimerLib functions (e.g. 
adding Frequency, StartValue, StopValue to ConvertTimeToTicks), which feels 
clunky to me. Would a new library make sense here? Something that sits between 
the driver and the TimerLib library? Or do we just leave things as they 
currently are?

Thanks,
Patrick Henz

-Original Message-
From: Wu, Hao A  
Sent: Thursday, August 10, 2023 8:43 PM
To: Henz, Patrick ; devel@edk2.groups.io; Michael Brown 

Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance Timer 
for XHCI Timeouts

My personal preference is to do this as two steps.
Address the issue in XhciDxe first. Then add new TimerLib API to replace all 
driver/library internal implementations.

Best Regards,
Hao Wu

> -Original Message-
> From: Henz, Patrick 
> Sent: Friday, August 11, 2023 6:45 AM
> To: devel@edk2.groups.io; Wu, Hao A ; Michael 
> Brown 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
> Performance Timer for XHCI Timeouts
> 
> I can certainly make that change.
> 
> For what it's worth I have been working on adding the new function, 
> GetElapsedTicks, to the various implementations of TimerLib. I've 
> finished up testing, I would just need to clean up the commits for a 
> patch. Should I move forward with that, or would we rather I just add 
> XhcGetElapsedTime to XhciDxe for the time being?
> 
> Thanks,
> Patrick Henz
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu, Hao 
> A
> Sent: Sunday, July 30, 2023 9:57 PM
> To: devel@edk2.groups.io; Henz, Patrick ; 
> Michael Brown 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use 
> Performance Timer for XHCI Timeouts
> 
> For the 2 occurrences of in XhcWaitOpRegBit & XhcExecTransfer:
>   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND (Timeout * 
> XHC_1_MILLISECOND); How about changing them to:
>   TimeoutTime = XHC_MICROSECOND_TO_NANOSECOND ((UINT64) Timeout
> * XHC_1_MILLISECOND); to address possible overflow during "Timeout * 
> XHC_1_MILLISECOND"?
> 
> For extending XhcGetElapsedTime as a TimerLib API, I am fine to put it 
> in XhciDxe at this moment.
> If package maintainers suggest to make it as a public library API, my 
> take is that this should be done in a separate commit.
> 
> Best Regards,
> Hao Wu
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Henz, 
> > Patrick
> > Sent: Thursday, July 6, 2023 4:16 AM
> > To: devel@edk2.groups.io
> > Cc: Henz, Patrick 
> > Subject: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Use Performance 
> > Timer for XHCI Timeouts
> >
> > REF:INVALID URI REMOVED
> > bu
> > g.cgi?id=2948__;!!NpxR!kyFQM5IkKYAG9CRBO4xphwBnzi_jhb2oU-
> NKbMjOV-lctg5
> > _B3K1Lcta452Gx-1twRt8At3cueAYDq_n$
> >
> > XhciDxe uses the timer functionality provided by the boot services 
> > table to detect timeout conditions. This breaks the driver's 
> > ExitBootServices call back, as CoreExitBootServices halts the timer 
> > before signaling the ExitBootServices event. If the host controller 
> > fails to halt in the call back, the timeout condition will never 
> > occur and the boot gets stuck in an indefinite spin loop. Use the 
> > free running timer provided by TimerLib to calculate timeouts, 
> > avoiding the potential hang.
> >
> > Signed-off-by: Patrick Henz 
> > Reviewed-by:
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +--
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
> > +---
> >  5 files changed, 129 insertions(+), 86 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > index 62682dd27c..1dcbe20512 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > @@ -1,6 +1,7 @@
> >  /** @file
> >
> >The XHCI controller driver.
> >
> >
> >
> > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> >
> >  Copyright (c) 2011 - 2022, Intel Corporation. All rights 
> > reserved.
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> >
> > @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
> >
> >
> >return EFI_SUCCESS;
> >
> 

[edk2-devel] [PATCH v3 4/4] ArmVirtPkg: Add varpolicy shell command

[Michael Kubacki]

From: Michael Kubacki 

Adds the varpolicy EFI shell command to all DSC files that
currently include other dynamic shell commands from ShellPkg.

This command allows variable policies to be dumped in the EFI
shell for convenient auditing and debug.

Use the command in the EFI shell as follows:

- `"varpolicy"` dumps platform variables
- `"varpolicy -?"` shows help text
- `"varpolicy -b"` pages output as expected
- `"varpolicy -s"` shows accurate variable statistic information
- `"varpolicy -p"` shows accurate UEFI variable policy information
- `"varpolicy-v -b"` dumps all information including variable data hex dump

Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Julien Grall 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Signed-off-by: Michael Kubacki 
---
 ArmVirtPkg/ArmVirt.dsc.inc   | 4 
 ArmVirtPkg/ArmVirtCloudHv.fdf| 1 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
 ArmVirtPkg/ArmVirtXen.fdf| 1 +
 4 files changed, 7 insertions(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 4ed86b979e1a..fe6488ee9910 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -393,6 +393,10 @@ [Components.common]
 
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
   }
+  
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
 {
+
+  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
   OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf {
 
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
diff --git a/ArmVirtPkg/ArmVirtCloudHv.fdf b/ArmVirtPkg/ArmVirtCloudHv.fdf
index a5f172d79bfc..56d1ea6e8c1b 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.fdf
+++ b/ArmVirtPkg/ArmVirtCloudHv.fdf
@@ -169,6 +169,7 @@ [FV.FvMain]
   INF ShellPkg/Application/Shell/Shell.inf
   INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
   INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+  INF 
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
   INF OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 
   #
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 2894bc853a46..9b3e37d5c998 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -103,6 +103,7 @@ [FV.FvMain]
   INF ShellPkg/Application/Shell/Shell.inf
   INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
   INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+  INF 
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
   INF OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 
   #
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 770fbf7289be..ea835551b375 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -180,6 +180,7 @@ [FV.FvMain]
   INF ShellPkg/Application/Shell/Shell.inf
   INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
   INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
+  INF 
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
   INF OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf
 
   #
-- 
2.42.0.windows.2



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

[edk2-devel] [PATCH v3 2/4] ShellPkg: Add varpolicy dynamic shell command and app

[Michael Kubacki]

From: Michael Kubacki 

Adds a new module (dynamic shell command) to ShellPkg that lists
variable policy information for all UEFI variables on the system.

Some other UEFI variable related functionality is also included to
give a greater sense of platform UEFI variable state. This command
is intended to help make variable policies more transparent and
easier to understand and configure on a platform.

Like all dynamic shell commands, a platform only needs to include
`VariablePolicyDynamicCommand.inf` in their flash image to have
the command registered in their UEFI shell.

Include the following lines in platform DSC (in DXE components section):

```
  
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
 {

  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
  }
```

Include the following line in platform FDF:

```
INF  
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
```

A standalone UEFI application can also be built that uses the same
underlying functional code as the dynamic shell command.

The path to use in the DSC and FDF for the app:

```
  ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyApp.inf
```

Cc: Zhichao Gao 
Cc: Michael D Kinney 
Signed-off-by: Michael Kubacki 
Reviewed-by: Ard Biesheuvel 
Reviewed-by: Zhichao Gao 
---
 ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.c  
   | 897 
 ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyApp.c   
   |  59 ++
 
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.c
   | 157 
 ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.h  
   | 129 +++
 ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.uni
   |  86 ++
 ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyApp.inf 
   |  62 ++
 
ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicyDynamicCommand.inf
 |  61 ++
 ShellPkg/ShellPkg.dsc  
   |   5 +
 8 files changed, 1456 insertions(+)

diff --git 
a/ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.c 
b/ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.c
new file mode 100644
index ..ed991be4ed00
--- /dev/null
+++ b/ShellPkg/DynamicCommand/VariablePolicyDynamicCommand/VariablePolicy.c
@@ -0,0 +1,897 @@
+/** @file
+  Main file for the "varpolicy" dynamic UEFI shell command and application.
+
+  This feature can provide detailed UEFI variable policy configuration
+  information in the UEFI shell.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "VariablePolicy.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define VAR_POLICY_FLAG_STATS_STRL"-s"
+#define VAR_POLICY_FLAG_POLICY_STR   L"-p"
+#define VAR_POLICY_FLAG_VERBOSE_STR  L"-v"
+
+#define VAR_POLICY_CMD_MIN_ATTR_STR_LEN  64
+
+EFI_HII_HANDLE  mVarPolicyShellCommandHiiHandle = NULL;
+
+STATIC CONST SHELL_PARAM_ITEM  ParamList[] = {
+  { VAR_POLICY_FLAG_POLICY_STR,  TypeFlag },
+  { VAR_POLICY_FLAG_STATS_STR,   TypeFlag },
+  { VAR_POLICY_FLAG_VERBOSE_STR, TypeFlag },
+  { NULL,TypeMax  }
+};
+
+STATIC CONST VAR_POLICY_CMD_VAR_NAMESPACE  mVarNamespaces[] = {
+  {
+VariableVendorCapsule,
+,
+L"Capsule"
+  },
+  {
+VariableVendorCapsuleReport,
+,
+L"Capsule Reporting"
+  },
+  {
+VariableVendorGlobal,
+,
+L"UEFI Global"
+  },
+  {
+VariableVendorMemoryTypeInfo,
+,
+L"Memory Type Information"
+  },
+  {
+VariableVendorMonotonicCounter,
+,
+L"Monotonic Counter"
+  },
+  {
+VariableVendorMorControl,
+,
+L"Memory Overwrite Request (MOR) Control Lock"
+  },
+  {
+VariableVendorShell,
+,
+L"UEFI Shell"
+  },
+  {
+VariableVendorShell,
+,
+L"UEFI Shell Alias"
+  }
+};
+
+/**
+  Returns UEFI variable attribute information in a string.
+
+  AttributesStrSize must at least be VAR_POLICY_CMD_MIN_ATTR_STR_LEN in length
+  or EFI_INVALID_PARAMETER will be returned.
+
+  @param[in]  Attributes The UEFI variable attributes.
+  @param[in]  AttributesStrSize  The size, in bytes, of AttributesStr.
+  @param[out] AttributesStr  The Unicode string for the given 
attributes.
+
+  @retval EFI_SUCCESS   The attributes were converted to a string 
successfully.
+  @retval EFI_INVALID_PARAMETER The AttributesStr pointer is NULL.
+
+**/
+EFI_STATUS
+GetAttributesString (
+  IN  UINT32  Attributes,
+  IN  UINTN   AttributesStrSize,
+  OUT CHAR16  *AttributesStr
+  )
+{
+  if ((AttributesStr == NULL) || (AttributesStrSize < 
VAR_POLICY_CMD_MIN_ATTR_STR_LEN)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  

[edk2-devel] [PATCH v3 1/4] MdeModulePkg/VariablePolicy: Add more granular variable policy querying

[Michael Kubacki]

From: Michael Kubacki 

Introduces two new APIs to EDKII_VARIABLE_POLICY_PROTOCOL:
  1. GetVariablePolicyInfo()
  2. GetLockOnVariableStateVariablePolicyInfo()

These allow a caller to retrieve policy information associated with
a UEFI variable given the variable name and vendor GUID.

GetVariablePolicyInfo() - Returns the variable policy applied to the
UEFI variable. If the variable policy is applied toward an individual
UEFI variable, that name can optionally be returned.

GetLockOnVariableStateVariablePolicyInfo() - Returns the Lock on
Variable State policy applied to the UEFI variable. If the Lock on
Variable State policy is applied to a specific variable name, that
name can optionally be returned.

These functions can be useful for a variety of purposes such as
auditing, testing, and functional flows.

Also fixed some variable name typos in code touched by the changes.

Cc: Dandan Bi 
Cc: Hao A Wu 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
Reviewed-by: Ard Biesheuvel 
Reviewed-by: Liming Gao 
---
 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c| 174 
--
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c| 304 
+
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c  |   4 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | 346 
+++-
 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h |  39 ++-
 MdeModulePkg/Include/Library/VariablePolicyLib.h  | 107 ++
 MdeModulePkg/Include/Protocol/VariablePolicy.h| 133 
+++-
 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf  |   1 +
 8 files changed, 1062 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c 
b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
index 5de46133bb26..1448af8a 100644
--- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
+++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
@@ -76,14 +76,20 @@ VarCheckPolicyLibMmiHandler (
   VOID *InternalCommBuffer;
   EFI_STATUS   Status;
   EFI_STATUS   SubCommandStatus;
-  VAR_CHECK_POLICY_COMM_HEADER *PolicyCommmHeader;
-  VAR_CHECK_POLICY_COMM_HEADER *InternalPolicyCommmHeader;
+  VAR_CHECK_POLICY_COMM_HEADER *PolicyCommHeader;
+  VAR_CHECK_POLICY_COMM_HEADER *InternalPolicyCommHeader;
   VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS  *IsEnabledParams;
   VAR_CHECK_POLICY_COMM_DUMP_PARAMS*DumpParamsIn;
   VAR_CHECK_POLICY_COMM_DUMP_PARAMS*DumpParamsOut;
+  VAR_CHECK_POLICY_COMM_GET_INFO_PARAMS*GetInfoParamsInternal;
+  VAR_CHECK_POLICY_COMM_GET_INFO_PARAMS*GetInfoParamsExternal;
+  CHAR16   *InternalCopyOfOutputVariableName;
+  CHAR16   *ExternalCopyOfOutputVariableName;
   UINT8*DumpInputBuffer;
   UINT8*DumpOutputBuffer;
+  UINTNAllowedOutputVariableNameSize;
   UINTNDumpTotalPages;
+  UINTNLocalSize;
   VARIABLE_POLICY_ENTRY*PolicyEntry;
   UINTNExpectedSize;
   UINT32   TempSize;
@@ -122,21 +128,21 @@ VarCheckPolicyLibMmiHandler (
   //
   InternalCommBuffer = [0];
   CopyMem (InternalCommBuffer, CommBuffer, InternalCommBufferSize);
-  PolicyCommmHeader = CommBuffer;
-  InternalPolicyCommmHeader = InternalCommBuffer;
+  PolicyCommHeader = CommBuffer;
+  InternalPolicyCommHeader = InternalCommBuffer;
   // Check the revision and the signature of the comm header.
-  if ((InternalPolicyCommmHeader->Signature != VAR_CHECK_POLICY_COMM_SIG) ||
-  (InternalPolicyCommmHeader->Revision != VAR_CHECK_POLICY_COMM_REVISION))
+  if ((InternalPolicyCommHeader->Signature != VAR_CHECK_POLICY_COMM_SIG) ||
+  (InternalPolicyCommHeader->Revision != VAR_CHECK_POLICY_COMM_REVISION))
   {
 DEBUG ((DEBUG_INFO, "%a - Signature or revision are incorrect!\n", 
__func__));
 // We have verified the buffer is not null and have enough size to hold 
Result field.
-PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
+PolicyCommHeader->Result = EFI_INVALID_PARAMETER;
 return EFI_SUCCESS;
   }
 
   // If we're in the middle of a paginated dump and any other command is sent,
   // pagination cache must be cleared.
-  if ((mPaginationCache != NULL) && (InternalPolicyCommmHeader->Command != 
mCurrentPaginationCommand)) {
+  if ((mPaginationCache != NULL) && (InternalPolicyCommHeader->Command != 
mCurrentPaginationCommand)) {
 FreePool (mPaginationCache);
 mPaginationCache  = NULL;
 mPaginationCacheSize   

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

[Ashish Singhal via groups.io]

AndroidBootImgLib allows for platforms to append to kernel command
line but does not allow for the overall kernel command line to go
beyond the limit set by the image header. Address this limitation
by adding a pcd where platform can tell how many extra characters
they expect on their platform in addition to what the image header
specifies.

Signed-off-by: Ashish Singhal 
---
 EmbeddedPkg/EmbeddedPkg.dec | 5 +
 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c   | 2 +-
 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 341ef5e6a6..94dc3c9b76 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -183,3 +183,8 @@
   # Selection between DT and ACPI as a default
   #
   gEmbeddedTokenSpaceGuid.PcdDefaultDtPref|TRUE|BOOLEAN|0x059
+
+  #
+  # Expected Overflow Android Kernel Command Line Characters
+  #
+  
gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow|0|UINT32|0x05C
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c 
b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 02769cd0df..abd3f87d14 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -643,7 +643,7 @@ AndroidBootImgBoot (
 goto Exit;
   }
 
-  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE + PcdGet32 
(PcdAndroidKernelCommandLineOverflow);
   NewKernelArg = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
   if (NewKernelArg == NULL) {
 DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf 
b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
index 8eefeef4f9..9754664df5 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
@@ -45,5 +45,6 @@
   gEfiAcpiTableGuid
   gFdtTableGuid
 
-[FeaturePcd]
+[Pcd]
   gEmbeddedTokenSpaceGuid.PcdAndroidBootLoadFile2
+  gEmbeddedTokenSpaceGuid.PcdAndroidKernelCommandLineOverflow
-- 
2.17.1



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

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

[Ashish Singhal via groups.io]

Curently, AndroidBootImgLib expects input kernel command line
to never exceed 256 unicode characters where the image header
allows for 512 ascii characters. If image header allows 512
ascii characters, similar number of unicode characters should be
allowed at the minimum.

Signed-off-by: Ashish Singhal 
---
 .../AndroidBootImgLib/AndroidBootImgLib.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c 
b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
index 1359a66db2..02769cd0df 100644
--- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
@@ -321,8 +321,9 @@ AndroidBootImgGetFdt (
 
 EFI_STATUS
 AndroidBootImgUpdateArgs (
-  IN  VOID  *BootImg,
-  OUT VOID  *KernelArgs
+  IN  VOID*BootImg,
+  OUT VOID*KernelArgs,
+  IN  UINT32  KernelArgsSize
   )
 {
   CHAR8   ImageKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
@@ -337,13 +338,13 @@ AndroidBootImgUpdateArgs (
   AsciiStrToUnicodeStrS (
 ImageKernelArgs,
 KernelArgs,
-ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1
+KernelArgsSize
 );
   // Append platform kernel arguments
   if (mAndroidBootImg->AppendArgs) {
 Status = mAndroidBootImg->AppendArgs (
 KernelArgs,
-ANDROID_BOOTIMG_KERNEL_ARGS_SIZE
+KernelArgsSize
 );
   }
 
@@ -611,11 +612,16 @@ AndroidBootImgBoot (
   MEMORY_DEVICE_PATH KernelDevicePath;
   EFI_HANDLE ImageHandle;
   VOID   *NewKernelArg;
+  UINT32 NewKernelArgSize;
   EFI_LOADED_IMAGE_PROTOCOL  *ImageInfo;
   VOID   *RamdiskData;
   UINTN  RamdiskSize;
   IN  VOID   *FdtBase;
 
+  if ((Buffer == NULL) || (BufferSize == 0)) {
+return EFI_INVALID_PARAMETER;
+  }
+
   NewKernelArg = NULL;
   ImageHandle  = NULL;
 
@@ -637,14 +643,15 @@ AndroidBootImgBoot (
 goto Exit;
   }
 
-  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
+  NewKernelArgSize = ANDROID_BOOTIMG_KERNEL_ARGS_SIZE;
+  NewKernelArg = AllocateZeroPool (sizeof (CHAR16) * NewKernelArgSize);
   if (NewKernelArg == NULL) {
 DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
 Status = EFI_OUT_OF_RESOURCES;
 goto Exit;
   }
 
-  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg);
+  Status = AndroidBootImgUpdateArgs (Buffer, NewKernelArg, NewKernelArgSize);
   if (EFI_ERROR (Status)) {
 goto Exit;
   }
-- 
2.17.1



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

Re: [edk2-devel] [PATCH 0/2] Add Platform Hook Lib into StandaloneMmCore

[Michael Kubacki]

On 10/28/2023 10:03 AM, Laszlo Ersek wrote:

On 10/27/23 18:08, Michael Kubacki wrote:

This allows ambiguous "platform" code in the critical path of the MM
core. Is this necessary?

Do you need this for one feature that others might too and can be
abstracted? Or, do you plan to perform an unknown and arbitrary number
of changes behind the hook over time?


Not sure if it's necessary, but it's somewhat "customary". Platform hook
libs are not uncommon; for example PiSmmCpuDxeSmm consumes
SmmCpuPlatformHookLib and SmmCpuFeaturesLib.

My request would be for Wei to file a TianoCore Feature Request
bugzilla, with a bit more information than "We need this library to
implement our feature". Reference the BZ in the commit messages, then
add the BZ to the
.

That would be helpful. Hooks are sometimes necessary of course, but I 
generally consider these open-ended hook libraries a design wart. They 
introduce an inherently incohesive interface and a convenient place for 
new APIs to land instead of more focused interfaces that may be more 
appropriate. This extends to the implementation where, over time, it can 
become a mess of dependencies and unrelated code coupled together.


I'd like to see if there's a way to avoid that or such a library 
interface is the best choice.



Laszlo



Thanks,
Michael

On 10/26/2023 11:28 PM, Xu, Wei6 wrote:

This patch set is to add StandaloneMmCorePlatformHookLib into
StandaloneMmCore.

This library class defines a set of platform hooks called by the
Standalone Mm Core. With this library, platform can perform specific
tasks before and after invoking registered MMI handlers.
We need this library to implement our feature.

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



Cc: Ard Biesheuvel 

Cc: Sami Mujawar 

Cc: Ray Ni 


Wei6 Xu (2):
    StandaloneMmPkg: Add Standalone Mm Core platform hook lib.
    StandaloneMmPkg/Core: Consumes Standalone Mm Core Platform Hook Lib.

   StandaloneMmPkg/Core/StandaloneMmCore.c   |  7 ++-
   .../StandaloneMmCorePlatformHookLibNull.c | 45 +++
   StandaloneMmPkg/Core/StandaloneMmCore.h   |  1 +
   StandaloneMmPkg/Core/StandaloneMmCore.inf |  1 +
   .../Library/StandaloneMmCorePlatformHookLib.h | 44 ++
   .../StandaloneMmCorePlatformHookLibNull.inf   | 30 +
   StandaloneMmPkg/StandaloneMmPkg.dec   |  4 ++
   StandaloneMmPkg/StandaloneMmPkg.dsc   |  2 +
   8 files changed, 133 insertions(+), 1 deletion(-)
   create mode 100644
StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/StandaloneMmCorePlatformHookLibNull.c
   create mode 100644
StandaloneMmPkg/Include/Library/StandaloneMmCorePlatformHookLib.h
   create mode 100644
StandaloneMmPkg/Library/StandaloneMmCorePlatformHookLibNull/StandaloneMmCorePlatformHookLibNull.inf
















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

Re: [edk2-devel] [PATCH v2 0/2] dp command without ACPI

[Jeff Brasen via groups.io]

Anything else needed to get this merged as the November stable release is 
coming up.

Thanks,
Jeff

> -Original Message-
> From: Gao, Zhichao 
> Sent: Monday, October 16, 2023 8:39 PM
> To: Jeff Brasen ; Gao, Liming
> ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Bi, Dandan
> 
> Subject: RE: [PATCH v2 0/2] dp command without ACPI
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Liming,
> 
> This patch set is already reviewed. Can you help on the merge?
> 
> Thanks,
> Zhichao
> 
> > -Original Message-
> > From: Jeff Brasen 
> > Sent: Wednesday, October 4, 2023 10:37 PM
> > To: Gao, Liming ; devel@edk2.groups.io
> > Cc: Wang, Jian J ; Bi, Dandan
> > ; Gao, Zhichao 
> > Subject: RE: [PATCH v2 0/2] dp command without ACPI
> >
> > Anything else needed on this to get it merged?
> >
> > Thanks,
> > Jeff
> >
> > > -Original Message-
> > > From: gaoliming 
> > > Sent: Tuesday, September 12, 2023 6:59 AM
> > > To: Jeff Brasen ; devel@edk2.groups.io
> > > Cc: jian.j.w...@intel.com; dandan...@intel.com;
> > > zhichao@intel.com
> > > Subject: 回复: [PATCH v2 0/2] dp command without ACPI
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > For the change in MdeModulePkg, Reviewed-by: Liming Gao
> > > 
> > >
> > > > -邮件原件-
> > > > 发件人: Jeff Brasen 
> > > > 发送时间: 2023年9月12日 4:39
> > > > 收件人: devel@edk2.groups.io
> > > > 抄送: jian.j.w...@intel.com; gaolim...@byosoft.com.cn;
> > > > dandan...@intel.com; zhichao@intel.com
> > > > 主题: RE: [PATCH v2 0/2] dp command without ACPI
> > > >
> > > >
> > > > Any additional thoughts/feedback on this patch series?
> > > >
> > > > Thanks,
> > > > Jeff
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Jeff Brasen 
> > > > > Sent: Friday, June 30, 2023 11:30 AM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: jian.j.w...@intel.com; gaolim...@byosoft.com.cn;
> > > > dandan...@intel.com;
> > > > > zhichao@intel.com; Jeff Brasen 
> > > > > Subject: [PATCH v2 0/2] dp command without ACPI
> > > > >
> > > > > Systems that do not boot with ACPI (system that use device tree
> > > > > for
> > > > example)
> > > > > can not use the shell dp command. This patch adds this to the
> > > configuration
> > > > > table so that dp command can get this without the FPDT table.
> > > > >
> > > > > I am open to other ways for this to be passed if desired
> > > > > (Installed
> > > protocol,
> > > > > handler of the status code, etc) but wanted to post this to at
> > > > > least get thoughts on this.
> > > > >
> > > > > Change Log
> > > > > v2 - Fix missing cast for IA32 builds
> > > > >
> > > > > -Jeff
> > > > >
> > > > > Jeff Brasen (2):
> > > > >   MdeModulePkg/DxeCorePerformanceLib: Install BPDT in config
> table
> > > > >   ShellPkg/Dp: Allow dp command to work without ACPI
> > > > >
> > > > >  ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf|  1 +
> > > > >  .../DpDynamicCommand/DpDynamicCommand.inf |  1 +
> > > > >  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c |  2 ++
> > > > >  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 11
> > > > -
> > > > > --
> > > > >  4 files changed, 12 insertions(+), 3 deletions(-)
> > > > >
> > > > > --
> > > > > 2.25.1
> > >
> > >



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

Re: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Allow relocation of images with large address

[Jeff Brasen via groups.io]

Anything else needed to get this merged would as the November stable release is 
coming up.

Thanks,
Jeff

> -Original Message-
> From: gaoliming 
> Sent: Friday, October 6, 2023 11:23 PM
> To: Jeff Brasen ; devel@edk2.groups.io
> Cc: jian.j.w...@intel.com; dandan...@intel.com; Ashish Singhal
> ; michael.d.kin...@intel.com
> Subject: 回复: [PATCH] MdeModulePkg/DxeCore: Allow relocation of
> images with large address
> 
> External email: Use caution opening links or attachments
> 
> 
> Jeff:
>   I have no better solution than your proposal for this problem. So, this 
> patch
> is good to me. Reviewed-by: Liming Gao 
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: Jeff Brasen 
> > 发送时间: 2023年9月12日 4:14
> > 收件人: devel@edk2.groups.io
> > 抄送: jian.j.w...@intel.com; gaolim...@byosoft.com.cn;
> > dandan...@intel.com; Ashish Singhal 
> > 主题: RE: [PATCH] MdeModulePkg/DxeCore: Allow relocation of images
> with
> > large address
> >
> > Any  thoughts/feedback on this patch?
> >
> > Thanks,
> > Jeff
> >
> >
> > > -Original Message-
> > > From: Jeff Brasen 
> > > Sent: Monday, May 15, 2023 5:49 PM
> > > To: devel@edk2.groups.io
> > > Cc: jian.j.w...@intel.com; gaolim...@byosoft.com.cn;
> > dandan...@intel.com;
> > > Jeff Brasen ; Ashish Singhal
> > > 
> > > Subject: [PATCH] MdeModulePkg/DxeCore: Allow relocation of images
> > > with large address
> > >
> > > Add PCD to control if modules with start addresses in PE/COFF >
> > > 0x10 attempt to load at specified address.
> > > If a module has an address in this range and there is untested
> > > memory DxeCore will attempt to promote all memory to tested which
> > > bypasses any memory testing that would occur later in boot.
> > >
> > > There are several existing AARCH64 option roms that have base
> > > addresses
> > of
> > > 0x18000.
> > >
> > > Signed-off-by: Jeff Brasen 
> > > Reviewed-by: Ashish Singhal 
> > > ---
> > >  MdeModulePkg/Core/Dxe/DxeMain.inf   | 1 +
> > >  MdeModulePkg/Core/Dxe/Image/Image.c | 4 +++-
> > >  MdeModulePkg/MdeModulePkg.dec   | 7 +++
> > >  3 files changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > index 35d5bf0dee..16871f2021 100644
> > > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > @@ -187,6 +187,7 @@
> > >gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > > ## CONSUMES
> > >gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> > ##
> > > CONSUMES
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth
> > > ## CONSUMES
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad
> > > ## CONSUMES
> > >
> > >  # [Hob]
> > >  # RESOURCE_DESCRIPTOR   ## CONSUMES
> > > diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> > > b/MdeModulePkg/Core/Dxe/Image/Image.c
> > > index 9dbfb2a1fa..6bc3a549ae 100644
> > > --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> > > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> > > @@ -680,7 +680,9 @@ CoreLoadPeImage (
> > > );
> > >}
> > >  } else {
> > > -  if ((Image->ImageContext.ImageAddress >= 0x10) || Image-
> > > >ImageContext.RelocationsStripped) {
> > > +  if ((PcdGetBool (PcdImageLargeAddressLoad) && ((Image-
> > > >ImageContext.ImageAddress) >= 0x10)) ||
> > > +  Image->ImageContext.RelocationsStripped)
> > > +  {
> > >  Status = CoreAllocatePages (
> > > AllocateAddress,
> > > (EFI_MEMORY_TYPE)(Image-
> > > >ImageContext.ImageCodeMemoryType),
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > b/MdeModulePkg/MdeModulePkg.dec index 95dd077e19..6fd1bd7b8f
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -1116,6 +1116,13 @@
> > ># @Prompt Output MMIO address of Trace Hub message.
> > >
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdTraceHubDebugMmioAddress|0|UI
> N
> > > T64|0x30001058
> > >
> > > +  ## Indicates if images with large load address (>0x10) should
> > > attempted to load at specified location.
> > > +  #  If enabled, attempt to allocate at specfied location will be
> attempted
> > with
> > > a fall back to any address.
> > > +  #   TRUE  - UEFI will attempt to load at specified location.
> > > +  #   FALSE - UEFI will load at any address
> > > +  # @Prompt Enable large address image loading.
> > > +
> > > +
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageLargeAddressLoad|TRUE|BO
> O
> > L
> > > EAN|0
> > > + x30001059
> > > +
> > >  [PcdsFixedAtBuild, PcdsPatchableInModule]
> > >## Dynamic type PCD can be registered callback function for Pcd
> setting
> > > action.
> > >#  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum
> > > number of callback function
> > > --
> > > 2.25.1
> 
> 



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

Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members

[Ng, Ray Han Lim]

For changes to item "> -M: Ray Han Lim Ng  
[rayhanlimng]":
Reviewed-by: Ray Han Lim Ng 


-Original Message-
From: Pedro Falcato  
Sent: Sunday, October 29, 2023 10:17 AM
To: devel@edk2.groups.io; Kinney, Michael D 
Cc: Andrew Fish ; Leif Lindholm ; 
Warkentin, Andrei ; West, Catharine 
; Bi, Dandan ; Daniel Schaefer 
; David Woodhouse ; De, Debkumar 
; Dong, Eric ; Jiang, Guomin 
; Wu, Hao A ; James Bottomley 
; Wang, Jian J ; Justen, Jordan L 
; Julien Grall ; Peter Grehan 
; Zhang, Qi1 ; Ng, Ray Han Lim 
; Stefan Berger ; Hou, Wenxing 
; Lu, Xiaoyu1 
Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active 
community members

On Sat, Oct 28, 2023 at 8:23 PM Michael D Kinney  
wrote:
>
> Over the past few months, all the of the Maintainers and Reviewers 
> listed in Maintainers.txt have been contacted to make sure 
> Maintainers.txt accurately represents the TianoCore community members 
> that are actively participating in their roles.  Based on specific 
> feedback, bounced emails, and no responses, updates have been made.
>
> * RISCV64: Daniel Schaefer replaced with Andrei Warkentin
> * ArmVirtPkg Xen has no remaining reviewers and review
>   responsibility defaults to ArmVirtPkg Maintainers/Reviewers.
> * ACPI modules related to S3 has no remaining reviewers and
>   review responsibility defaults to MdeModulePkg Maintainers/
>   Reviewers.
> * OVMF CSM modules has no remaining reviewers and review
>   responsibility defaults to OvmfPkg Maintainers/Reviewers.
> * Bounce: Chan Laura 
> * Many smaller updates removing individuals that are no
>   longer involved or have replacement coverage.

Mike,

Thank you so much for doing this thankless task. Some comments:

> diff --git a/Maintainers.txt b/Maintainers.txt index 
> 3f40cdeb5554..2b03ccbe54aa 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -93,7 +93,7 @@ M: Sami Mujawar  [samimujawar]
>  RISCV64
>  F: */RiscV64/
>  M: Sunil V L  [vlsunil]
> -R: Daniel Schaefer  [JohnAZoidberg]
> +R: Andrei Warkentin  [andreiw]
>
>  LOONGARCH64
>  F: */LoongArch64/
> @@ -157,16 +157,6 @@ R: Leif Lindholm  
> [leiflindholm]
>  R: Sami Mujawar  [samimujawar]
>  R: Gerd Hoffmann  [kraxel]
>
> -ArmVirtPkg: modules used on Xen
> -F: ArmVirtPkg/ArmVirtXen.*
> -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
> -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
> -F: ArmVirtPkg/PrePi/
> -F: ArmVirtPkg/XenAcpiPlatformDxe/
> -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
> -F: ArmVirtPkg/XenioFdtDxe/
> -R: Julien Grall  [jgrall]

ArmVirtPkg Xen modules seize to have a dedicated maintainer. Can the generic 
ArmVirtPkg maintainers handle *more code* (particularly, functionality that's 
not trivial to test, unless you actively use Xen)?

>  BaseTools
>  F: BaseTools/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> @@ -187,8 +177,7 @@ F: CryptoPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>  M: Jiewen Yao  [jyao1]
>  M: Yi Li  [liyi77]
> -R: Xiaoyu Lu  [xiaoyuxlu]
> -R: Guomin Jiang  [guominjia]
> +R: Wenxing Hou  [Wenxing-hou]
>
>  DynamicTablesPkg
>  F: DynamicTablesPkg/
> @@ -202,7 +191,6 @@ W: 
> https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
>  M: Leif Lindholm  [leiflindholm]
>  M: Ard Biesheuvel  [ardbiesheuvel]
>  M: Abner Chang  [changab]
> -R: Daniel Schaefer  [JohnAZoidberg]
>
>  EmulatorPkg
>  F: EmulatorPkg/
> @@ -228,7 +216,6 @@ F: FmpDevicePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/FmpDevicePkg
>  M: Liming Gao  [lgao4]
>  M: Michael D Kinney  [mdkinney]
> -R: Guomin Jiang  [guominjia]
>  R: Wei6 Xu  [xuweiintel]
>
>  IntelFsp2Pkg
> @@ -237,7 +224,6 @@ W: 
> https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg
>  M: Chasel Chiu  [ChaselChiu]
>  M: Nate DeSimone  [nate-desimone]
>  M: Duggapu Chinni B  [cbduggap]
> -M: Ray Han Lim Ng  [rayhanlimng]
>  R: Star Zeng  [lzeng14]
>  R: Ted Kuo  [tedkuo1]
>  R: Ashraf Ali S  [AshrafAliS] @@ -258,7 
> +244,6 @@ R: Susovan Mohapatra  
> [susovanmohapatra]  MdeModulePkg
>  F: MdeModulePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> -M: Jian J Wang  [jwang36]
>  M: Liming Gao  [lgao4]

MdeModulePkg now only has a single maintainer (Liming, who also handles a 
myriad of other tasks and packages)
>
>  MdeModulePkg: ACPI modules
> @@ -268,15 +253,6 @@ R: Zhiguang Liu  
> [LiuZhiguang001]
>  R: Dandan Bi  [dandanbi]
>  R: Liming Gao  [lgao4]
>
> -MdeModulePkg: ACPI modules related to S3
> -F: MdeModulePkg/*LockBox*/
> -F: MdeModulePkg/Include/*BootScript*.h
> -F: MdeModulePkg/Include/*LockBox*.h
> -F: MdeModulePkg/Include/*S3*.h
> -F: MdeModulePkg/Library/*S3*/
> -R: Hao A Wu  [hwu25]
> -R: Eric Dong  [ydong10]
> -
>  MdeModulePkg: BDS modules
>  F: MdeModulePkg/*BootManager*/
>  F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -326,7 +302,6 @@ F: MdeModulePkg/Library/DxeSecurityManagementLib/
>  F: MdeModulePkg/Universal/PCD/
>  F: 

[edk2-devel] [PATCH edk2-platforms v2 4/4] IpmiFeaturePkg: Add FRU drivers

[Zhen Gong]

Add GenericFruDriver and generate data based on SMBIOS data.

Signed-off-by: Zhen Gong 

# Conflicts:
#   Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFru/IpmiFru.inf
---
 .../IpmiFeaturePkg/IpmiFeaturePkg.dec |   4 +
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   3 +-
 .../IpmiFeaturePkg/Include/PostMemory.fdf |   3 +-
 .../IpmiFeaturePkg/GenericFru/GenericFru.inf  |  42 ++
 .../IpmiFeaturePkg/IpmiFru/IpmiFru.inf|  36 --
 .../IpmiRedirFru/IpmiRedirFru.inf |  51 ++
 .../GenericFru/GenericFruDriver.h | 178 ++
 .../Include/Protocol/GenericFru.h | 103 
 .../Include/Protocol/RedirFru.h   |  81 +++
 .../IpmiRedirFru/IpmiRedirFru.h   | 149 +
 .../IpmiFeaturePkg/GenericFru/GenericFru.c|  68 +++
 .../GenericFru/GenericFruDriver.c | 513 ++
 .../IpmiFeaturePkg/IpmiFru/IpmiFru.c  |  67 ---
 .../IpmiFeaturePkg/IpmiRedirFru/FruSmbios.c   | 469 
 .../IpmiRedirFru/IpmiRedirFru.c   | 479 
 15 files changed, 2141 insertions(+), 105 deletions(-)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericFru/GenericFru.inf
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFru/IpmiFru.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiRedirFru/IpmiRedirFru.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericFru/GenericFruDriver.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Protocol/GenericFru.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Protocol/RedirFru.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiRedirFru/IpmiRedirFru.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericFru/GenericFru.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericFru/GenericFruDriver.c
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFru/IpmiFru.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiRedirFru/FruSmbios.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiRedirFru/IpmiRedirFru.c

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
index be0a11e2adb1..d586931a6d6f 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
@@ -63,6 +63,8 @@ [LibraryClasses]
 [Guids]
   gIpmiFeaturePkgTokenSpaceGuid  =  {0xc05283f6, 0xd6a8, 0x48f3, {0x9b, 0x59, 
0xfb, 0xca, 0x71, 0x32, 0x0f, 0x12}}
   gPeiIpmiHobGuid= {0xcb4d3e13, 0x1e34, 0x4373, {0x8a, 0x81, 
0xe9, 0x0, 0x10, 0xf1, 0xdb, 0xa4}}
+  gEfiIpmiFormatFruGuid  = { 0x3531fdc6, 0xeae,  0x4cd2, { 0xb0, 0xa6, 
0x5f, 0x48, 0xa0, 0xdf, 0xe3, 0x8  } }
+  gEfiSystemTypeFruGuid  = { 0xaab16018, 0x679d, 0x4461, { 0xba, 0x20, 
0xe7, 0xc,  0xf7, 0x86, 0x6a, 0x9b } }
 
 [Ppis]
   gPeiIpmiTransportPpiGuid = {0x7bf5fecc, 0xc5b5, 0x4b25, {0x81, 0x1b, 0xb4, 
0xb5, 0xb, 0x28, 0x79, 0xf7}}
@@ -80,6 +82,8 @@ [Protocols]
   gSmmGenericElogProtocolGuid = { 0x664ef1f6, 0x19bf, 0x4498, { 0xab, 0x6a, 
0xfc, 0x05, 0x72, 0xfb, 0x98, 0x51 } }
   gEfiRedirElogProtocolGuid = { 0x16d11030, 0x71ba, 0x4e5e, { 0xa9, 0xf9, 
0xb4, 0x75, 0xa5, 0x49, 0x4, 0x8a } }
   gSmmRedirElogProtocolGuid = { 0x79ac2d9c, 0x9216, 0x43c5, { 0xa0, 0x74, 
0x0b, 0x45, 0xc7, 0x64, 0x22, 0xc1 } }
+  gEfiRedirFruProtocolGuid  = { 0x28638cfa, 0xea88, 0x456c, { 0x92, 0xa5, 
0xf2, 0x49, 0xca, 0x48, 0x85, 0x35 } }
+  gEfiGenericFruProtocolGuid = { 0xc8eebf0e, 0x0e10, 0x47f7, { 0x81, 0xbd, 
0x39, 0xdb, 0x75, 0xca, 0x93, 0x9f } }
 
 [PcdsFeatureFlag]
   gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable|FALSE|BOOLEAN|0xA001
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
index 7e663236d9a1..3ceedb2fa3c4 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
@@ -129,7 +129,8 @@ [Components.X64]
   IpmiFeaturePkg/GenericElog/Dxe/GenericElog.inf
   IpmiFeaturePkg/GenericElog/Smm/GenericElog.inf
   IpmiFeaturePkg/Frb/FrbDxe.inf
-  IpmiFeaturePkg/IpmiFru/IpmiFru.inf
+  IpmiFeaturePkg/IpmiRedirFru/IpmiRedirFru.inf
+  IpmiFeaturePkg/GenericFru/GenericFru.inf
   IpmiFeaturePkg/IpmiInit/DxeIpmiInit.inf
   IpmiFeaturePkg/OsWdt/OsWdt.inf
   IpmiFeaturePkg/SolStatus/SolStatus.inf
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf
index 9b692f07dcd8..810a041983c1 100644
--- 

[edk2-devel] [PATCH edk2-platforms v2 3/4] IpmiFeaturePkg: Add ACPI power state drivers

[Zhen Gong]

Add DXE and SMM drivers that send "Set ACPI Power State" command to BMC.

Signed-off-by: Zhen Gong 
---
 .../IpmiFeaturePkg/IpmiFeaturePkg.dec |   1 +
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   2 +
 .../IpmiFeaturePkg/Include/PostMemory.fdf |   2 +
 .../BmcAcpiState/BmcAcpiState.inf |  40 
 .../BmcAcpiSwChild/BmcAcpiSwChild.inf |  39 
 .../BmcAcpiState/BmcAcpiState.h   |  26 +++
 .../BmcAcpiSwChild/BmcAcpiSwChild.h   |  82 
 .../Include/Protocol/BmcAcpiSwChildPolicy.h   |  31 +++
 .../BmcAcpiState/BmcAcpiState.c   |  93 +
 .../BmcAcpiSwChild/BmcAcpiSwChild.c   | 189 ++
 10 files changed, 505 insertions(+)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Protocol/BmcAcpiSwChildPolicy.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.c

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
index 22bc4e69be8a..be0a11e2adb1 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
@@ -75,6 +75,7 @@ [Protocols]
   gEfiVideoPrintProtocolGuid = {0x3dbf3e06, 0x9d0c, 0x40d3, {0xb2, 0x17, 
0x45, 0x5f, 0x33, 0x9e, 0x29, 0x09}}
   gIpmiTransport2ProtocolGuid = { 0x4A1D0E66, 0x5271, 0x4E22, { 0x83, 0xFE, 
0x90, 0x92, 0x1B, 0x74, 0x82, 0x13 }}
   gSmmIpmiTransport2ProtocolGuid = { 0x1DBD1503, 0x0A60, 0x4230, { 0xAA, 0xA3, 
0x80, 0x16, 0xD8, 0xC3, 0xDE, 0x2F }}
+  gEfiBmcAcpiSwChildPolicyProtocolGuid = { 0x89843c0b, 0x5701, 0x4ff6, { 0xa4, 
0x73, 0x65, 0x75, 0x99, 0x04, 0xf7, 0x35 } }
   gEfiGenericElogProtocolGuid = { 0x59d02fcd, 0x9233, 0x4d34, { 0xbc, 0xfe, 
0x87, 0xca, 0x81, 0xd3, 0xdd, 0xa7 } }
   gSmmGenericElogProtocolGuid = { 0x664ef1f6, 0x19bf, 0x4498, { 0xab, 0x6a, 
0xfc, 0x05, 0x72, 0xfb, 0x98, 0x51 } }
   gEfiRedirElogProtocolGuid = { 0x16d11030, 0x71ba, 0x4e5e, { 0xa9, 0xf9, 
0xb4, 0x75, 0xa5, 0x49, 0x4, 0x8a } }
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
index fc9001e98473..7e663236d9a1 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
@@ -122,6 +122,8 @@ [Components.X64]
   IpmiFeaturePkg/GenericIpmi/Dxe/GenericIpmi.inf
   IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf
   IpmiFeaturePkg/BmcAcpi/BmcAcpi.inf
+  IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
+  IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.inf
   IpmiFeaturePkg/BmcElog/DxeBmcElog.inf
   IpmiFeaturePkg/BmcElog/SmmBmcElog.inf
   IpmiFeaturePkg/GenericElog/Dxe/GenericElog.inf
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf
index f29810bc0b34..9b692f07dcd8 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/PostMemory.fdf
@@ -10,6 +10,8 @@
 INF IpmiFeaturePkg/GenericIpmi/Dxe/GenericIpmi.inf
 INF IpmiFeaturePkg/IpmiInit/DxeIpmiInit.inf
 INF RuleOverride = DRIVER_ACPITABLE IpmiFeaturePkg/BmcAcpi/BmcAcpi.inf
+INF IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
+INF IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.inf
 INF IpmiFeaturePkg/BmcElog/DxeBmcElog.inf
 INF IpmiFeaturePkg/BmcElog/SmmBmcElog.inf
 INF IpmiFeaturePkg/Frb/FrbDxe.inf
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
new file mode 100644
index ..f1b750d6a20a
--- /dev/null
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
@@ -0,0 +1,40 @@
+### @file
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = BmcAcpiState
+  FILE_GUID  = 04103e59-48cc-417a-baec-9929c69c20f6
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT  

Re: [edk2-devel] [PATCH edk2-platforms 0/4] IpmiFeaturePkg: Add server management features

[Zhen Gong]

Hi Abner,

Thanks for the heads-up.
I caught your commit right before submitting the patch. MdeModulePKg is already 
consumed by all libraries referencing the header.
However there's a merge conflict now and I just sent out a v2 version to 
address it.

Regarding to the migration to ManageabilityPkg, we have evaluated whether to 
adopt it directly.
Given the long time frame of silicon development, we are not going to make big 
changes for current platforms.
And we chose to keep using IpmiFeaturePkg and upstream drivers with minimum 
changes required for now.

The decision is more about lifecycle management, other than technical 
standpoint.
Going forward, we will re-evaluate the open and bring it to newer platforms.

Thanks,
Zhen

-Original Message-
From: Chang, Abner  
Sent: Saturday, October 28, 2023 6:59 PM
To: devel@edk2.groups.io; Gong, Zhen 
Subject: RE: [edk2-devel] [PATCH edk2-platforms 0/4] IpmiFeaturePkg: Add server 
management features

[AMD Official Use Only - General]

Hi Gong,
Please note that your code may have conflict as IpmiCommandLib was removed 
(please check  https://edk2.groups.io/g/devel/message/109510), now we are using 
the one under MdeModulePKg.
Second, I had cleaned up those server management feature drivers and migrated 
those to under ManageabilityPkg with Issac RB, please check commit ID from 
b6a5124e to d6f18259. That would be not good if Intel keeps updating 
IpmiFeaturePkg. As those drivers are higher level applications on top of 
transport, It shouldn't have a problem to just update the changes against 
ManageabilityPkg. Is there any issues Intel met if uses IPMI feature drivers 
from Manageability? If yes, we can address the issue instead of making them 
diverging.


Thanks
Abner

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhen 
> Gong via groups.io
> Sent: Saturday, October 28, 2023 4:11 AM
> To: devel@edk2.groups.io
> Cc: Zhen Gong 
> Subject: [edk2-devel] [PATCH edk2-platforms 0/4] IpmiFeaturePkg: Add 
> server management features
>
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
>
>
> This patch set adds serveral IPMI features to support server management:
>
> BmcAcpiState: A DXE driver to notify BMC of S0 power state.
> BmcAcpiSwChild: An SMM driver to notify BMC of ACPI power state 
> changes and add  SEL records.
> BmcElog: PEI, DXE, and SMM drivers to support BMC event log functions.
> GenericElog: DXE and SMM drivers to support generic event log functions.
> GenericFru: A runtime driver to support generic FRU functions.
> IpmiRedirFru: A DXE driver to support BMC FRU functions and generate 
> data based  on SMBIOS data.
> ServerManagementLib: A library to provide essential functions for 
> server  management drivers.
>
>
> Zhen Gong (4):
>   IpmiFeaturePkg: Add Elog drivers
>   IpmiFeaturePkg: Add ServerManagementLib
>   IpmiFeaturePkg: Add ACPI power state drivers
>   IpmiFeaturePkg: Add FRU drivers
>
>  .../IpmiFeaturePkg/IpmiFeaturePkg.dec |  10 +
>  .../IpmiFeaturePkg/Include/IpmiFeature.dsc|  13 +-
>  .../IpmiFeaturePkg/Include/PostMemory.fdf |  10 +-
>  .../IpmiFeaturePkg/Include/PreMemory.fdf  |   1 +
>  .../BmcAcpiState/BmcAcpiState.inf |  40 +
>  .../BmcAcpiSwChild/BmcAcpiSwChild.inf |  39 +
>  .../BmcElog/{BmcElog.inf => DxeBmcElog.inf}   |  25 +-
>  .../IpmiFeaturePkg/BmcElog/PeiBmcElog.inf |  43 ++
>  .../IpmiFeaturePkg/BmcElog/SmmBmcElog.inf |  44 ++
>  .../GenericElog/Dxe/GenericElog.inf   |  38 +
>  .../GenericElog/Smm/GenericElog.inf   |  38 +
>  .../IpmiFeaturePkg/GenericFru/GenericFru.inf  |  42 ++
>  .../IpmiFeaturePkg/IpmiFru/IpmiFru.inf|  35 -
>  .../IpmiRedirFru/IpmiRedirFru.inf |  51 ++
>  .../ServerManagementLib.inf   |  35 +
>  .../ServerManagementLibNull.inf   |  38 +
>  .../BmcAcpiState/BmcAcpiState.h   |  26 +
>  .../BmcAcpiSwChild/BmcAcpiSwChild.h   |  82 +++
>  .../BmcElog/Common/BmcElogCommon.h| 144 
>  .../IpmiFeaturePkg/BmcElog/Dxe/BmcElog.h  |  42 ++
>  .../IpmiFeaturePkg/BmcElog/Pei/BmcElog.h  |  44 ++
>  .../IpmiFeaturePkg/BmcElog/Smm/BmcElog.h  |  43 ++
>  .../GenericElog/Dxe/GenericElog.h | 194 +
>  .../GenericElog/Smm/GenericElog.h | 216 ++
>  .../GenericFru/GenericFruDriver.h | 178 +
>  .../Include/Library/ServerMgmtRtLib.h | 147 
>  .../IpmiFeaturePkg/Include/Ppi/GenericElog.h  |  84 +++
>  .../Include/Protocol/BmcAcpiSwChildPolicy.h   |  31 +
>  .../Include/Protocol/GenericElog.h|  99 +++
>  .../Include/Protocol/GenericFru.h | 103 +++
>  .../Include/Protocol/RedirFru.h   |  81 ++
>  .../IpmiRedirFru/IpmiRedirFru.h   | 149 
>  .../BmcAcpiState/BmcAcpiState.c   |  93 +++
>  .../BmcAcpiSwChild/BmcAcpiSwChild.c 

[edk2-devel] [PATCH edk2-platforms v2 2/4] IpmiFeaturePkg: Add ServerManagementLib

[Zhen Gong]

Lightweight lib to support Server Management drivers.

Signed-off-by: Zhen Gong 
---
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   1 +
 .../ServerManagementLib.inf   |  35 +
 .../ServerManagementLibNull.inf   |  38 +
 .../Include/Library/ServerMgmtRtLib.h | 147 
 .../ServerManagementLib/ServerManagementLib.c | 696 ++
 .../ServerManagementLibNull.c | 144 
 6 files changed, 1061 insertions(+)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Library/ServerMgmtRtLib.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.c

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
index fa98e5672d83..fc9001e98473 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
@@ -51,6 +51,7 @@ 
[LibraryClasses.common.DXE_DRIVER,LibraryClasses.common.UEFI_DRIVER]
 [LibraryClasses.common]
   
BmcCommonInterfaceLib|IpmiFeaturePkg/Library/BmcInterfaceCommonAccess/BmcCommonInterfaceLib.inf
   
BtInterfaceLib|IpmiFeaturePkg/Library/BmcInterfaceCommonAccess/BtInterfaceLib/BtInterfaceLib.inf
+  
ServerManagementLib|IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
 
 [LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.DXE_DRIVER]
   
SsifInterfaceLib|IpmiFeaturePkg/Library/BmcInterfaceCommonAccess/SsifInterfaceLib/DxeSsifInterfaceLib.inf
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
new file mode 100644
index ..25a3a7540762
--- /dev/null
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
@@ -0,0 +1,35 @@
+### @file
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = ServerManagementLib
+  FILE_GUID  = D43C86F0-7C8B-4992-9593-8CB6FBC66A0A
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  PI_SPECIFICATION_VERSION   = 0x0001000A
+  LIBRARY_CLASS  = ServerManagementLib
+
+[Sources]
+  ServerManagementLib.c
+
+[Packages]
+  IpmiFeaturePkg/IpmiFeaturePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  IoLib
+  DebugLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gEfiGenericElogProtocolGuid
+
+
+
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.inf
new file mode 100644
index ..6b66b44857f3
--- /dev/null
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.inf
@@ -0,0 +1,38 @@
+### @file
+#
+# Copyright (c) 2023, Intel Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+###
+
+
+[Defines]
+  INF_VERSION   = 0x00010005
+  BASE_NAME = ServerManagementLibNull
+  FILE_GUID = 3AC01DE9-5A96-4df5-A645-ECD092C29AF9
+  MODULE_TYPE   = BASE
+  VERSION_STRING= 1.0
+  PI_SPECIFICATION_VERSION  = 0x0001000A
+  LIBRARY_CLASS = ServerManagementLib
+
+#
+# The following information is for reference only and not required by the 
build tools.
+#
+#  VALID_ARCHITECTURES  = IA32 X64
+#
+
+[Sources.common]
+  ServerManagementLibNull.c
+
+
+[Packages]
+  MdePkg/MdePkg.dec
+  ServerPlatformPkg/PlatformPkg.dec
+
+[LibraryClasses]
+  DebugLib
+
+[Protocols]
+
+
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Library/ServerMgmtRtLib.h
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Library/ServerMgmtRtLib.h
new file mode 100644
index ..12b2e82f9e6e
--- /dev/null
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Library/ServerMgmtRtLib.h
@@ -0,0 +1,147 @@
+/** @file
+  Server Management Driver Lib.
+
+Copyright (c) 2023, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _SERVER_MGMT_LIB_H_

[edk2-devel] [PATCH edk2-platforms v2 1/4] IpmiFeaturePkg: Add Elog drivers

[Zhen Gong]

Add generic Elog driver and support BMC Elog operations.

Signed-off-by: Zhen Gong 
---
 .../IpmiFeaturePkg/IpmiFeaturePkg.dec |   5 +
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   7 +-
 .../IpmiFeaturePkg/Include/PostMemory.fdf |   5 +-
 .../IpmiFeaturePkg/Include/PreMemory.fdf  |   1 +
 .../BmcElog/{BmcElog.inf => DxeBmcElog.inf}   |  19 +-
 .../IpmiFeaturePkg/BmcElog/PeiBmcElog.inf |  43 ++
 .../IpmiFeaturePkg/BmcElog/SmmBmcElog.inf |  44 ++
 .../GenericElog/Dxe/GenericElog.inf   |  38 ++
 .../GenericElog/Smm/GenericElog.inf   |  38 ++
 .../BmcElog/Common/BmcElogCommon.h| 144 +
 .../IpmiFeaturePkg/BmcElog/Dxe/BmcElog.h  |  42 ++
 .../IpmiFeaturePkg/BmcElog/Pei/BmcElog.h  |  44 ++
 .../IpmiFeaturePkg/BmcElog/Smm/BmcElog.h  |  43 ++
 .../GenericElog/Dxe/GenericElog.h | 194 ++
 .../GenericElog/Smm/GenericElog.h | 216 +++
 .../IpmiFeaturePkg/Include/Ppi/GenericElog.h  |  84 +++
 .../Include/Protocol/GenericElog.h|  99 +++
 .../IpmiFeaturePkg/BmcElog/BmcElog.c  | 236 ---
 .../BmcElog/Common/BmcElogCommon.c| 465 ++
 .../IpmiFeaturePkg/BmcElog/Dxe/BmcElog.c  | 287 +
 .../IpmiFeaturePkg/BmcElog/Pei/BmcElog.c  | 297 +
 .../IpmiFeaturePkg/BmcElog/Smm/BmcElog.c  | 288 +
 .../GenericElog/Dxe/GenericElog.c | 576 ++
 .../GenericElog/Smm/GenericElog.c | 558 +
 24 files changed, 3530 insertions(+), 243 deletions(-)
 rename Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/{BmcElog.inf 
=> DxeBmcElog.inf} (60%)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/PeiBmcElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/SmmBmcElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Dxe/GenericElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Smm/GenericElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Common/BmcElogCommon.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Dxe/BmcElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Pei/BmcElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Smm/BmcElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Dxe/GenericElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Smm/GenericElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Ppi/GenericElog.h
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Protocol/GenericElog.h
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/BmcElog.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Common/BmcElogCommon.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Dxe/BmcElog.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Pei/BmcElog.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/Smm/BmcElog.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Dxe/GenericElog.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Smm/GenericElog.c

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
index 5df71300cbd1..22bc4e69be8a 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
@@ -67,6 +67,7 @@ [Guids]
 [Ppis]
   gPeiIpmiTransportPpiGuid = {0x7bf5fecc, 0xc5b5, 0x4b25, {0x81, 0x1b, 0xb4, 
0xb5, 0xb, 0x28, 0x79, 0xf7}}
   gPeiIpmiTransport2PpiGuid = {0x8122CEBD, 0xF4FD, 0x4EA8, { 0x97, 0x6C, 0xF0, 
0x30, 0xAD, 0xDC, 0x4C, 0xB4 }}
+  gPeiRedirElogPpiGuid = { 0x7a7c1591, 0xfc67, 0x4f69, { 0xa3, 0x78, 0xfc, 
0x3d, 0x4a, 0xd7, 0x92, 0xf7 } }
 
 [Protocols]
   gIpmiTransportProtocolGuid  = {0x6bb945e8, 0x3743, 0x433e, {0xb9, 0x0e, 
0x29, 0xb3, 0x0d, 0x5d, 0xc6, 0x30}}
@@ -74,6 +75,10 @@ [Protocols]
   gEfiVideoPrintProtocolGuid = {0x3dbf3e06, 0x9d0c, 0x40d3, {0xb2, 0x17, 
0x45, 0x5f, 0x33, 0x9e, 0x29, 0x09}}
   gIpmiTransport2ProtocolGuid = { 0x4A1D0E66, 0x5271, 0x4E22, { 0x83, 0xFE, 
0x90, 0x92, 0x1B, 0x74, 0x82, 0x13 }}
   gSmmIpmiTransport2ProtocolGuid = { 0x1DBD1503, 0x0A60, 0x4230, { 0xAA, 0xA3, 
0x80, 0x16, 0xD8, 0xC3, 0xDE, 0x2F }}
+  gEfiGenericElogProtocolGuid = { 0x59d02fcd, 0x9233, 0x4d34, { 0xbc, 0xfe, 
0x87, 0xca, 0x81, 0xd3, 0xdd, 0xa7 } }
+  gSmmGenericElogProtocolGuid = { 0x664ef1f6, 0x19bf, 0x4498, { 0xab, 0x6a, 
0xfc, 0x05, 0x72, 0xfb, 0x98, 0x51 } }
+  

[edk2-devel] [PATCH edk2-platforms v2 0/4] IpmiFeaturePkg: Add server management features

[Zhen Gong]

This patch set adds serveral IPMI features to support server management:

BmcAcpiState: A DXE driver to notify BMC of S0 power state.
BmcAcpiSwChild: An SMM driver to notify BMC of ACPI power state changes and add
 SEL records.
BmcElog: PEI, DXE, and SMM drivers to support BMC event log functions.
GenericElog: DXE and SMM drivers to support generic event log functions.
GenericFru: A runtime driver to support generic FRU functions.
IpmiRedirFru: A DXE driver to support BMC FRU functions and generate data based
 on SMBIOS data.
ServerManagementLib: A library to provide essential functions for server
 management drivers.

Notes:
V2:
- Rebased to resolve merge conflict from upstream changes

Zhen Gong (4):
  IpmiFeaturePkg: Add Elog drivers
  IpmiFeaturePkg: Add ServerManagementLib
  IpmiFeaturePkg: Add ACPI power state drivers
  IpmiFeaturePkg: Add FRU drivers

 .../IpmiFeaturePkg/IpmiFeaturePkg.dec |  10 +
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|  13 +-
 .../IpmiFeaturePkg/Include/PostMemory.fdf |  10 +-
 .../IpmiFeaturePkg/Include/PreMemory.fdf  |   1 +
 .../BmcAcpiState/BmcAcpiState.inf |  40 +
 .../BmcAcpiSwChild/BmcAcpiSwChild.inf |  39 +
 .../BmcElog/{BmcElog.inf => DxeBmcElog.inf}   |  19 +-
 .../IpmiFeaturePkg/BmcElog/PeiBmcElog.inf |  43 ++
 .../IpmiFeaturePkg/BmcElog/SmmBmcElog.inf |  44 ++
 .../GenericElog/Dxe/GenericElog.inf   |  38 +
 .../GenericElog/Smm/GenericElog.inf   |  38 +
 .../IpmiFeaturePkg/GenericFru/GenericFru.inf  |  42 ++
 .../IpmiFeaturePkg/IpmiFru/IpmiFru.inf|  36 -
 .../IpmiRedirFru/IpmiRedirFru.inf |  51 ++
 .../ServerManagementLib.inf   |  35 +
 .../ServerManagementLibNull.inf   |  38 +
 .../BmcAcpiState/BmcAcpiState.h   |  26 +
 .../BmcAcpiSwChild/BmcAcpiSwChild.h   |  82 +++
 .../BmcElog/Common/BmcElogCommon.h| 144 
 .../IpmiFeaturePkg/BmcElog/Dxe/BmcElog.h  |  42 ++
 .../IpmiFeaturePkg/BmcElog/Pei/BmcElog.h  |  44 ++
 .../IpmiFeaturePkg/BmcElog/Smm/BmcElog.h  |  43 ++
 .../GenericElog/Dxe/GenericElog.h | 194 +
 .../GenericElog/Smm/GenericElog.h | 216 ++
 .../GenericFru/GenericFruDriver.h | 178 +
 .../Include/Library/ServerMgmtRtLib.h | 147 
 .../IpmiFeaturePkg/Include/Ppi/GenericElog.h  |  84 +++
 .../Include/Protocol/BmcAcpiSwChildPolicy.h   |  31 +
 .../Include/Protocol/GenericElog.h|  99 +++
 .../Include/Protocol/GenericFru.h | 103 +++
 .../Include/Protocol/RedirFru.h   |  81 ++
 .../IpmiRedirFru/IpmiRedirFru.h   | 149 
 .../BmcAcpiState/BmcAcpiState.c   |  93 +++
 .../BmcAcpiSwChild/BmcAcpiSwChild.c   | 189 +
 .../IpmiFeaturePkg/BmcElog/BmcElog.c  | 236 --
 .../BmcElog/Common/BmcElogCommon.c| 465 
 .../IpmiFeaturePkg/BmcElog/Dxe/BmcElog.c  | 287 
 .../IpmiFeaturePkg/BmcElog/Pei/BmcElog.c  | 297 
 .../IpmiFeaturePkg/BmcElog/Smm/BmcElog.c  | 288 
 .../GenericElog/Dxe/GenericElog.c | 576 +++
 .../GenericElog/Smm/GenericElog.c | 558 ++
 .../IpmiFeaturePkg/GenericFru/GenericFru.c|  68 ++
 .../GenericFru/GenericFruDriver.c | 513 +
 .../IpmiFeaturePkg/IpmiFru/IpmiFru.c  |  67 --
 .../IpmiFeaturePkg/IpmiRedirFru/FruSmbios.c   | 469 
 .../IpmiRedirFru/IpmiRedirFru.c   | 479 
 .../ServerManagementLib/ServerManagementLib.c | 696 ++
 .../ServerManagementLibNull.c | 144 
 48 files changed, 7237 insertions(+), 348 deletions(-)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiState/BmcAcpiState.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcAcpiSwChild/BmcAcpiSwChild.inf
 rename Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/{BmcElog.inf 
=> DxeBmcElog.inf} (60%)
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/PeiBmcElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/BmcElog/SmmBmcElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Dxe/GenericElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericElog/Smm/GenericElog.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericFru/GenericFru.inf
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFru/IpmiFru.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiRedirFru/IpmiRedirFru.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLibNull/ServerManagementLibNull.inf
 create 

Re: [edk2-devel] [PATCH v1 1/1] .azurepipelines: Fix Python version (to 3.12)

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: Joey Vagedes 
> Sent: Monday, October 30, 2023 9:42 AM
> To: devel@edk2.groups.io
> Cc: Sean Brogan ; Michael Kubacki
> ; Kinney, Michael D
> ; Gao, Liming 
> Subject: [PATCH v1 1/1] .azurepipelines: Fix Python version (to 3.12)
> 
> Upgrades python to 3.12 for build as it has been released and all
> supporting tools have been updated to also support 3.12.
> 
> Cc: Sean Brogan 
> Cc: Michael Kubacki 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Signed-off-by: Joey Vagedes 
> ---
>  .azurepipelines/Ubuntu-PatchCheck.yml  | 2 +-
>  .azurepipelines/templates/defaults.yml | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/.azurepipelines/Ubuntu-PatchCheck.yml
> b/.azurepipelines/Ubuntu-PatchCheck.yml
> index 5e79474999b6..a397cf3f4db5 100644
> --- a/.azurepipelines/Ubuntu-PatchCheck.yml
> +++ b/.azurepipelines/Ubuntu-PatchCheck.yml
> @@ -27,7 +27,7 @@ steps:
> 
> 
>  - task: UsePythonVersion@0
> 
>inputs:
> 
> -versionSpec: '3.11'
> 
> +versionSpec: '3.12'
> 
>  architecture: 'x64'
> 
> 
> 
>  - script: |
> 
> diff --git a/.azurepipelines/templates/defaults.yml
> b/.azurepipelines/templates/defaults.yml
> index d50aa3e6d4d0..bc1cd058cc49 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -8,5 +8,5 @@
>  ##
> 
> 
> 
>  variables:
> 
> -  default_python_version: "3.11"
> 
> +  default_python_version: "3.12"
> 
>default_linux_image: "ghcr.io/tianocore/containers/fedora-37-
> test:a0dd931"
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110324): https://edk2.groups.io/g/devel/message/110324
Mute This Topic: https://groups.io/mt/102279161/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 v7 3/5] MdePkg: Implement RISC-V Cache Management Operations

[Pedro Falcato]

On Mon, Oct 30, 2023 at 9:38 AM Laszlo Ersek  wrote:
>
> On 10/29/23 20:12, Pedro Falcato wrote:
> > On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma  wrote:
> >>
> >> Implement Cache Management Operations (CMO) defined by
> >> RISC-V spec https://github.com/riscv/riscv-CMOs.
> >>
> >> Notes:
> >> 1. CMO only supports block based Operations. Meaning cache
> >>flush/invd/clean Operations are not available for the entire
> >>range. In that case we fallback on fence.i instructions.
> >> 2. Operations are implemented using Opcodes to make them compiler
> >>independent. binutils 2.39+ compilers support CMO instructions.
> >>
> >> Test:
> >> 1. Ensured correct instructions are refelecting in asm
> >
> > nit: reflecting
> >
> >> 2. Not able to verify actual instruction in HW as Qemu ignores
> >>any actual cache operations.
> >
> > Do you have no way to test this in hardware? Since Rivos is a RISCV
> > vendor and all ;)
> > I don't like inviting the idea of merging CPU architectural changes
> > without actually testing them in something resembling real silicon
> > (i.e QEMU KVM is _fine_, QEMU TCG really isn't).
> >
>
> Hopefully I'm not drawing an incorrect parallel here, but, as I recall
> arm64 enablement in 2014, nearly all initial enablement in RHEL occurred
> on software emulators (ARM Foundation Model, ARM FVP, then QEMU TCG).
> You need to start somewhere. In particular, qemu-system-aarch64 was a
> huge step forward (performance-wise) once it *existed*, relative to the
> Foundation Model / FVP, even though qemu-system-aarch64 wouldn't emulate
> CPU caches (IIRC).

Right. I don't know how faithful those early ARM simulators were, but
QEMU TCG is not very faithful and uarch details *can* slip through the
cracks.
In arm64 it's easy to miss a dsb or a isb if you're not extra careful
(or read the ARM ARM wrong).

RISCV has a bunch of fun gotchas too. For instance, did you know you
need to flush the TLB using sfence.vma even when only mapping a page?
This "small" detail results in boot failures on real hardware (such as
the visionfive 2), but is completely silent in QEMU TCG.

So this is why I would much prefer a test on real silicon. It's hard
to prove correctness when all you have is QEMU's spotty simulation
(rightfully so, it's not a simulator).

--
Pedro


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

Re: [edk2-devel] [PATCH 5/7] OvmfPkg: Add CcProbeLib in PlatformInitLib.inf

[Ard Biesheuvel]

On Fri, 27 Oct 2023 at 07:43, Dun Tan  wrote:
>
> PlatformInitLib uses the CcProbe API in MemDetect.c
> but doensn't add CcProbeLib in .inf LibraryClasses
> section. Add CcProbeLib to fix the build error.
>
> Signed-off-by: Dun Tan 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Ray Ni 

Reviewed-by: Ard Biesheuvel 

> ---
>  OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf 
> b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> index 5a79d95b68..8ff9e699b1 100644
> --- a/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> +++ b/OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
> @@ -2,7 +2,7 @@
>  #  Platform Initialization Lib
>  #
>  #  This module provides platform specific function to detect boot mode.
> -#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -52,6 +52,7 @@
>PcdLib
>PciLib
>PeiHardwareInfoLib
> +  CcProbeLib
>
>  [LibraryClasses.X64]
>TdxLib
> --
> 2.31.1.windows.1
>


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

Re: [edk2-devel] [PATCH 6/7] OvmfPkg: use BaseIoLibIntrinsic.inf in dsc files

[Ard Biesheuvel]

On Fri, 27 Oct 2023 at 07:43, duntan  wrote:
>
> Use BaseIoLibIntrinsic.inf in dsc files. The
> BaseIoLibIntrinsic supports Tdx and sev now.
> The BaseIoLibIntrinsicSev will be removed in
> the following patches.
>
> Signed-off-by: Dun Tan 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Ray Ni 

Reviewed-by: Ard Biesheuvel 
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
>  OvmfPkg/Bhyve/BhyveX64.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 +-
>  9 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 302c90e7c2..06086e0cc5 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -142,7 +142,7 @@
>
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index 6693342c5f..1376dd7468 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -149,7 +149,7 @@
>
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index 35942e02df..1d2a13dd92 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -159,7 +159,7 @@
>
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc 
> b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 182ec3705d..6623196c8b 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -146,7 +146,7 @@
>
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 0f26f2a9a9..c76a135f9e 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -157,7 +157,7 @@
>
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
> -  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index fcd3a3fda5..a850465e77 

Re: [edk2-devel] [PATCH edk2-platforms v3 0/3] Platform/ARM: Add dynamic CPU node, TRBE & ETE support to FVP

[Sami Mujawar]

Merged as dc8de7cc888f..1ae7cffab740

Thanks.

Regards,

Sami Mujawar


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

Re: [edk2-devel] [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support

[Sami Mujawar]

Merged as 4f3ee7fbafc8..a671a14e63fd

Thanks.

Regards,

Sami Mujawar


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

Re: [edk2-devel] [PATCH] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

[Laszlo Ersek]

+Gerd, +Paolo

On 10/30/23 07:12, Jacque Lin via groups.io wrote:
> Remove checking SMM Rev ID in AMD Save State lib when reading Save
> State Register EFI_MM_SAVE_STATE_REGISTER_IO.
> For AMD, it is not necessary to check SmmRevId when reading Save State
> Register EFI_MM_SAVE_STATE_REGISTER_IO.
>
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Signed-off-by: Jacque Lin 
> ---
>  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
>  1 file changed, 13 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
> b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> index 3315a6cc44..c4bf6ad4bb 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> @@ -102,7 +102,6 @@ MmSaveStateReadRegister (
>OUT VOID*Buffer
>
>)
>
>  {
>
> -  UINT32 SmmRevId;
>
>EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;
>
>AMD_SMRAM_SAVE_STATE_MAP   *CpuSaveState;
>
>UINT8  DataWidth;
>
> @@ -124,18 +123,6 @@ MmSaveStateReadRegister (
>
>
>// Check for special EFI_MM_SAVE_STATE_REGISTER_IO
>
>if (Register == EFI_MM_SAVE_STATE_REGISTER_IO) {
>
> -//
>
> -// Get SMM Revision ID
>
> -//
>
> -MmSaveStateReadRegisterByIndex (CpuIndex, 
> AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );
>
> -
>
> -//
>
> -// See if the CPU supports the IOMisc register in the save state
>
> -//
>
> -if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {
>
> -  return EFI_NOT_FOUND;
>
> -}
>
> -
>
>  // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.
>
>  if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {
>
>return EFI_NOT_FOUND;
>

I still don't understand.

Are you saying that the

  SmmRevId < AMD_SMM_MIN_REV_ID_X64

check will always evaluate to FALSE, and so the "return EFI_NOT_FOUND"
branch is dead code?

If that's the case, then the patch seems right, but *why* is SmmRevId
always greater than or equal to AMD_SMM_MIN_REV_ID_X64?

SmmRevId is used to tell apart x86 from x64 (i.e., to distinguish the
two formats of the save state map). Is the intent of this patch to
remove 32-bit (x86) support?

That makes me uncomfortable, because it could break SMM support in
*IA32* OVMF. Note that commit f2188fe5d155 ("OvmfPkg: Uses
MmSaveStateLib library", 2023-07-03) also updated
"OvmfPkg/OvmfPkgIa32.dsc".

In fact, I'm worried that the conversion of OVMF to MmSaveStateLib may
have been incorrect. Note that commit f2188fe5d155 removed the following
comment from "OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c":

> //
> // No support for I/O restart
> //

Furthermore, commit f2188fe5d155 removed the following functions:
GetRegisterIndex(), ReadSaveStateRegisterByIndex(),
SmmCpuFeaturesReadSaveStateRegister(),
SmmCpuFeaturesWriteSaveStateRegister().

In particular, the latter two functions contained comments and code
like:

>   //
>   // Check for special EFI_SMM_SAVE_STATE_REGISTER_IO
>   //
>   if (Register == EFI_SMM_SAVE_STATE_REGISTER_IO) {
> return EFI_NOT_FOUND;
>   }

and

>   //
>   // Writes to EFI_SMM_SAVE_STATE_REGISTER_IO are not supported
>   //
>   if (Register == EFI_SMM_SAVE_STATE_REGISTER_IO) {
> return EFI_NOT_FOUND;
>   }

All of these came from Paolo's original commit 4036b4e57cce ("OvmfPkg:
SmmCpuFeaturesLib: implement SMRAM state save map access", 2015-11-30).

Consider the following commits from Paolo (including 4036b4e57cce):

> commit 86d71589c1fdb099c04a0f9e548fe868a2fef266
> Author: Paolo Bonzini 
> Date:   Mon Nov 30 18:46:27 2015 +
>
> OvmfPkg: import SmmCpuFeaturesLib from UefiCpuPkg
>
> The next patches will customize the implementation, but let's start from
> the common version to better show the changes.

and

> commit d7e71b2925012c9706d1d044ca466173aac802a8
> Author: Paolo Bonzini 
> Date:   Mon Nov 30 18:46:32 2015 +
>
> OvmfPkg: SmmCpuFeaturesLib: remove unnecessary bits
>
> SMRR, MTRR, and SMM Feature Control support is not needed on a virtual
> platform.

and

> commit 4036b4e57ccef4e0fa48d8389acf6390826c2bed
> Author: Paolo Bonzini 
> Date:   Mon Nov 30 18:46:37 2015 +
>
> OvmfPkg: SmmCpuFeaturesLib: implement SMRAM state save map access
>
> This implementation copies SMRAM state save map access from the
> PiSmmCpuDxeSmm module.
>
> The most notable change is:
>
> - dropping support for EFI_SMM_SAVE_STATE_REGISTER_IO
>
> - changing the implementation of EFI_SMM_SAVE_STATE_REGISTER_LMA to use
>   the SMM revision id instead of a local variable (which
>   UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c initializes from CPUID's LM
>   bit).  This accounts for QEMU's implementation of x86_64, which always
>   uses revision 0x20064 even if the LM bit is zero.

and

> commit c1fcd80bf42e6b1e91c1c742d222f1ba421b1d1d
> Author: Paolo Bonzini 
> Date:   Mon Nov 30 18:46:42 2015 +
>
> 

Re: [edk2-devel] [PATCH v3 4/4] StandaloneMmPkg/Core: Fix the failure to find uncompressed inner FV

[Laszlo Ersek]

On 10/30/23 08:49, Wei6 Xu wrote:
> The MmCoreFfsFindMmDriver only checks for encapsulated compressed FVs.
> When an inner FV is uncompressed, StandaloneMmCore will miss the FV and
> all the MM drivers in the FV will not be dispatched.
> Add checks for uncompressed inner FV to fix this issue.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Signed-off-by: Wei6 Xu 
> ---
>  StandaloneMmPkg/Core/FwVol.c | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index fa335d62c252..783dbaf9b048 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -116,6 +116,25 @@ MmCoreFfsFindMmDriver (
>break;
>  }
>  
> +//
> +// Check uncompressed firmware volumes
> +//
> +Status = FfsFindSectionData (
> +   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
> +   FileHeader,
> +   ,
> +   
> +   );
> +if (!EFI_ERROR (Status)) {
> +  if (SectionDataSize > sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
> +InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)SectionData;
> +MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
> +  }
> +}
> +
> +//
> +// Check compressed firmware volumes
> +//
>  Status = FfsFindSection (
> EFI_SECTION_GUID_DEFINED,
> FileHeader,

(1) In case we find an EFI_SECTION_FIRMWARE_VOLUME_IMAGE, do we still want to 
look for an EFI_SECTION_GUID_DEFINED?

I think that's quite unlikely. If you agree, can you add a "continue" right 
after the new MmCoreFfsFindMmDriver() call?

Reviewed-by: Laszlo Ersek 

(2) We have a separate "InnerFvHeader" assignment, near the bottom of this 
(first) loop in the function:

 Status = FindFfsSectionInSections (
DstBuffer,
DstBufferSize,
EFI_SECTION_FIRMWARE_VOLUME_IMAGE,

);
 if (EFI_ERROR (Status)) {
   goto FreeDstBuffer;
 }
 
 InnerFvHeader = (VOID *)(Section + 1);
 Status= MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);

That doesn't look right to me; what if Section in fact points to an 
EFI_COMMON_SECTION_HEADER2?

FfsFindSectionData() handles this internally; it checks IS_SECTION2(), and then 
adjusts SectionData accordingly:

  if (IS_SECTION2 (Section)) {
*SectionData = (VOID *)((EFI_COMMON_SECTION_HEADER2 *)Section + 1);
...
  } else {
*SectionData = (VOID *)(Section + 1);
...
  }

I think we need to set InnerFvHeader similarly, here. Can you please append a 
separate patch for fixing that?

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110316): https://edk2.groups.io/g/devel/message/110316
Mute This Topic: https://groups.io/mt/102270549/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 v3 3/4] StandaloneMmPkg/Core: Fix issue that section address might be wrong

[Laszlo Ersek]

On 10/30/23 08:49, Wei6 Xu wrote:
> MmCoreFfsFindMmDriver() assumes FileHeader is EFI_FFS_FILE_HEADER.
> If FileHeader is an EFI_FFS_FILE_HEADER2, 'FileHeader + 1' will get a
> wrong section address. Use FfsFindSection to get the section directly,
> instead of 'FileHeader + 1' to avoid this issue.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Signed-off-by: Wei6 Xu 
> ---
>  StandaloneMmPkg/Core/FwVol.c | 18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index 9d0ce66ef839..fa335d62c252 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -116,23 +116,21 @@ MmCoreFfsFindMmDriver (
>break;
>  }
>  
> -Status = FfsFindSectionData (
> +Status = FfsFindSection (
> EFI_SECTION_GUID_DEFINED,
> FileHeader,
> -   ,
> -   
> +   
> );
>  if (EFI_ERROR (Status)) {
>break;
>  }
>  
> -Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> -Status  = ExtractGuidedSectionGetInfo (
> -Section,
> -,
> -,
> -
> -);
> +Status = ExtractGuidedSectionGetInfo (
> +   Section,
> +   ,
> +   ,
> +   
> +   );
>  if (EFI_ERROR (Status)) {
>break;
>  }

(1) Can you remove the SectionData and SectionDataSize variables as
well? I think they are unused at this point.

With that:

Reviewed-by: Laszlo Ersek 

Ah wait, you're going to use those variables in the next patch, again.
OK then. Just take my R-b for this patch.


(2) Now that I'm looking at the code in more depth, I don't even
understand what the original intent of the FfsFindSectionData() call was!

The output values SectionData and SectionDataSize were not used for
anything!

So it seems like FfsFindSectionData() was called just to make sure an
EFI_SECTION_GUID_DEFINED section *existed*. And then we'd treat the very
*first* section after the file header -- not too robustly identified, at
that -- as a GUIDed section, for extracting its info.

So this patch actually fixes two warts: one, the file header size is now
considered more generally, two, we don't just assume that the very first
section is the GUID-defined section, but look it up. Phew.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110315): https://edk2.groups.io/g/devel/message/110315
Mute This Topic: https://groups.io/mt/102270548/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 v3 2/4] StandaloneMmPkg/Core: Fix potential memory leak issue

[Laszlo Ersek]

On 10/30/23 08:49, Wei6 Xu wrote:
> In MmCoreFfsFindMmDriver(), ScratchBuffer is not freed in the error
> return path that DstBuffer page allocation fails. Free ScratchBuffer
> before return with error.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Signed-off-by: Wei6 Xu 
> ---
>  StandaloneMmPkg/Core/FwVol.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index e1e20ffd14ac..9d0ce66ef839 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -150,6 +150,7 @@ MmCoreFfsFindMmDriver (
>  //
>  DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES 
> (DstBufferSize));
>  if (DstBuffer == NULL) {
> +  FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
>return EFI_OUT_OF_RESOURCES;
>  }
>  

This patch is good, with regard to ScratchBuffer:

Reviewed-by: Laszlo Ersek 

However, upon further staring at the code, I think that we have a
DstBuffer life-cycle problem as well, independently of ScratchBuffer:

(1) ExtractGuidedSectionDecode() does not necessarily use the
caller-allocated buffer. The library class header file
"MdePkg/Include/Library/ExtractGuidedSectionLib.h" says that, "If the
decoded buffer is identical to the data in InputSection, then
OutputBuffer is set to point at the data in InputSection.  Otherwise,
the decoded data will be placed in caller allocated buffer specified by
OutputBuffer."

This means that the ExtractGuidedSectionDecode() call may change the
value of DstBuffer (rather than changing the contents of the buffer that
DstBuffer points at) -- in which case freeing DstBuffer is wrong.

This means we need a second variable. One variable needs to preserve the
allocation address, and the address of the other variable must be passed
to ExtractGuidedSectionDecode(). After the call, we need to free the
*original* variable (the one that ExtractGuidedSectionDecode() could not
have overwritten).

(2) As far as I can tell, we leak our original DstBuffer allocation in
two cases:

- Upon every iteration of the loop after the first iteration, we
overwrite the DstBuffer variable with the new allocation address. The
old one is lost (leaked).

My understanding is that, after the recursive MmCoreFfsFindMmDriver()
call returns, we no longer need the decompressed DstBuffer, therefore,
we should free the *original* DstBuffer allocation (per (1)) right there.

- The last (potentially: only one) iteration of the loop allocates
DstBuffer, and that allocation is never freed. We don't overwrite the
address with a new allocation's address, but still we never free the
original allocation. The FreeDstBuffer label is apparently never reached.

(3) And finally, a logic bug (or at least questionable behavior):

The loop at the *top* of the function scans the firmware volume for
embedded firmware volumes (recursing into them if any are found), while
the loop at the *bottom* of the function scans the *same* firmware
volume for MM driver binaries (adding them to the "MM driver list"),
starting anew from the beginning of the firmware volume.

Now, there are many exit points in the function-top loop. Those can be
classified in two groups: "break", and "return/goto". The former class
makes sense. The latter class does not seem to make sense to me.

Consider: just because we fail to scan the firmware volume for embedded
firmware volumes, for any reason really, should we really abandon
scanning the same firmware volume for MM driver binaries? What I don't
understand here in particular is the *inconsistency* between the exit
points, in the function-top loop:

- if we realize there are no (more) embedded FVs, we break out; good

- if we realize the next embedded FV is not "GUID defined", we break
out; good (well, questionable -- perhaps we should continue scanning?
the next embedded FV could be GUID defined after all!)

- if ExtractGuidedSectionGetInfo() fails, we break out again; good (or,
well, we could continue the scanning, but anyway)

- if the *decoding* fails, including the allocations, or we fail to find
a proper FV image section, or the recursive MmCoreFfsFindMmDriver() call
fails, then we *abandon* the MM driver images in the *current* firmware
image. That is what does not make any sense to me, compared to the
above-noted exit points. Just because we couldn't extract a compressed,
embedded FV image, why ignore the MM drivers in *this* image?

Sorry for creating more and more work for you, but I'm starting to think
that the whole loop should be rewritten. :/

Well, even if we don't change this scanning logic, at least properly
releasing DstBuffer would be nice (i.e., addressing points (1) and (2)).

Thanks for bearing with me
Laszlo



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

Re: [edk2-devel] [PATCH v3 1/4] StandaloneMmPkg/Core: Limit FwVol encapsulation section recursion

[Laszlo Ersek]

On 10/30/23 08:49, Wei6 Xu wrote:
> MmCoreFfsFindMmDriver() is called recursively for encapsulation sections.
> Currently this recursion is not limited. Introduce a new PCD
> (fixed-at-build, or patchable-in-module), and make MmCoreFfsFindMmDriver()
> track the section nesting depth against that PCD.
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Signed-off-by: Wei6 Xu 
> ---
>  StandaloneMmPkg/Core/FwVol.c  | 16 ++--
>  StandaloneMmPkg/Core/StandaloneMmCore.c   |  5 +++--
>  StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 +++
>  StandaloneMmPkg/StandaloneMmPkg.dec   |  5 +
>  4 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index 1f6d7714ba97..e1e20ffd14ac 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -48,6 +48,9 @@ FvIsBeingProcessed (
>MM driver and return its PE32 image.
>  
>@param [in] FwVolHeader   Pointer to memory mapped FV
> +  @param [in] Depth Nesting depth of encapsulation sections. Callers
> +different from MmCoreFfsFindMmDriver() are
> +responsible for passing in a zero Depth.
>  
>@retval  EFI_SUCCESSSuccess.
>@retval  EFI_INVALID_PARAMETER  Invalid parameter.
> @@ -55,11 +58,15 @@ FvIsBeingProcessed (
>@retval  EFI_OUT_OF_RESOURCES   Out of resources.
>@retval  EFI_VOLUME_CORRUPTED   Firmware volume is corrupted.
>@retval  EFI_UNSUPPORTEDOperation not supported.
> +  @retval  EFI_ABORTEDRecursion aborted because Depth has been
> +  greater than or equal to
> +  PcdFwVolMmMaxEncapsulationDepth.
>  
>  **/
>  EFI_STATUS
>  MmCoreFfsFindMmDriver (
> -  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
> +  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader,
> +  IN  UINT32  Depth
>)
>  {
>EFI_STATUS  Status;
> @@ -84,6 +91,11 @@ MmCoreFfsFindMmDriver (
>  
>DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
>  
> +  if (Depth >= PcdGet32 (PcdFwVolMmMaxEncapsulationDepth)) {
> +DEBUG ((DEBUG_ERROR, "%a: recursion aborted due to nesting depth\n", 
> __func__));
> +return EFI_ABORTED;
> +  }
> +
>if (FvHasBeenProcessed (FwVolHeader)) {
>  return EFI_SUCCESS;
>}
> @@ -172,7 +184,7 @@ MmCoreFfsFindMmDriver (
>  }
>  
>  InnerFvHeader = (VOID *)(Section + 1);
> -Status= MmCoreFfsFindMmDriver (InnerFvHeader);
> +Status= MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
>  if (EFI_ERROR (Status)) {
>goto FreeDstBuffer;
>  }
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c 
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index d221f1d1115d..523ea0a632a1 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -11,7 +11,8 @@
>  
>  EFI_STATUS
>  MmCoreFfsFindMmDriver (
> -  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
> +  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader,
> +  IN  UINT32  Depth
>);
>  
>  EFI_STATUS
> @@ -643,7 +644,7 @@ StandaloneMmMain (
>//
>DEBUG ((DEBUG_INFO, "Mm Dispatch StandaloneBfvAddress - 0x%08x\n", 
> gMmCorePrivate->StandaloneBfvAddress));
>if (gMmCorePrivate->StandaloneBfvAddress != 0) {
> -MmCoreFfsFindMmDriver ((EFI_FIRMWARE_VOLUME_HEADER 
> *)(UINTN)gMmCorePrivate->StandaloneBfvAddress);
> +MmCoreFfsFindMmDriver ((EFI_FIRMWARE_VOLUME_HEADER 
> *)(UINTN)gMmCorePrivate->StandaloneBfvAddress, 0);
>  MmDispatcher ();
>}
>  
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf 
> b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index c44b9ff33303..02ecd68f37e2 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -76,6 +76,9 @@ [Guids]
>gEfiEventExitBootServicesGuid
>gEfiEventReadyToBootGuid
>  
> +[Pcd]
> +  gStandaloneMmPkgTokenSpaceGuid.PcdFwVolMmMaxEncapsulationDepth
> ##CONSUMES
> +
>  #
>  # This configuration fails for CLANGPDB, which does not support PIE in the 
> GCC
>  # sense. Such however is required for ARM family StandaloneMmCore
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec 
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 46784d94e421..c43632d6d8ae 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -48,3 +48,8 @@ [Guids]
>gEfiStandaloneMmNonSecureBufferGuid  = { 0xf00497e3, 0xbfa2, 0x41a1, { 
> 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
>gEfiArmTfCpuDriverEpDescriptorGuid   = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 
> 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
>  
> +[PcdsFixedAtBuild, PcdsPatchableInModule]
> +  ## Maximum permitted encapsulation levels of sections in a firmware volume,
> +  #  in the MM phase. Minimum value 

Re: [edk2-devel] [PATCH v7 3/5] MdePkg: Implement RISC-V Cache Management Operations

[Sunil V L]

On Mon, Oct 30, 2023 at 10:38:21AM +0100, Laszlo Ersek wrote:
> On 10/29/23 20:12, Pedro Falcato wrote:
> > On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma  wrote:
> >>
> >> Implement Cache Management Operations (CMO) defined by
> >> RISC-V spec https://github.com/riscv/riscv-CMOs.
> >>
> >> Notes:
> >> 1. CMO only supports block based Operations. Meaning cache
> >>flush/invd/clean Operations are not available for the entire
> >>range. In that case we fallback on fence.i instructions.
> >> 2. Operations are implemented using Opcodes to make them compiler
> >>independent. binutils 2.39+ compilers support CMO instructions.
> >>
> >> Test:
> >> 1. Ensured correct instructions are refelecting in asm
> > 
> > nit: reflecting
> > 
> >> 2. Not able to verify actual instruction in HW as Qemu ignores
> >>any actual cache operations.
> > 
> > Do you have no way to test this in hardware? Since Rivos is a RISCV
> > vendor and all ;)
> > I don't like inviting the idea of merging CPU architectural changes
> > without actually testing them in something resembling real silicon
> > (i.e QEMU KVM is _fine_, QEMU TCG really isn't).
> > 
> 
> Hopefully I'm not drawing an incorrect parallel here, but, as I recall
> arm64 enablement in 2014, nearly all initial enablement in RHEL occurred
> on software emulators (ARM Foundation Model, ARM FVP, then QEMU TCG).
> You need to start somewhere. In particular, qemu-system-aarch64 was a
> huge step forward (performance-wise) once it *existed*, relative to the
> Foundation Model / FVP, even though qemu-system-aarch64 wouldn't emulate
> CPU caches (IIRC).
> 
I agree. As per my knowledge, we don't have a publicly available silicon
implementing these features as of today. So, we are taking the approach
of how linux merged these features when the code adhered to the spec. It
will be great for downstream to get these patches merged.

Thanks,
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110312): https://edk2.groups.io/g/devel/message/110312
Mute This Topic: https://groups.io/mt/102256466/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] Maintainers.txt: Update based on active community members

[Laszlo Ersek]

On 10/30/23 06:31, Kinney, Michael D wrote:
> There is a very good discussion here on the roles and responsibility
> and potential suggestions for changes to the Wiki page that document
> those roles and responsibilities.
> 
> May I suggest that someone start a new thread that discusses
> the proposed changes to the Wiki page and leave this thread for the
> review of the changes to Maintainers.txt?

These are connected topics, but yes, back to "Maintainers.txt" -- do you
feel that "S: Orphan" sections are acceptable in general? They're a
first (AFAICT) for edk2's "Maintainers.txt"; we've always had the
mechanism documented (since the creation of "Maintainers.txt", or so),
but we never seem to have put it to use.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110311): https://edk2.groups.io/g/devel/message/110311
Mute This Topic: https://groups.io/mt/102245264/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] Maintainers.txt: Update based on active community members

[Laszlo Ersek]

On 10/29/23 20:01, Pedro Falcato wrote:
> On Sun, Oct 29, 2023 at 1:30 PM Laszlo Ersek  wrote:
>>
>> On 10/29/23 03:16, Pedro Falcato wrote:
 diff --git a/Maintainers.txt b/Maintainers.txt
 index 3f40cdeb5554..2b03ccbe54aa 100644
 --- a/Maintainers.txt
 +++ b/Maintainers.txt
 @@ -93,7 +93,7 @@ M: Sami Mujawar  [samimujawar]
  RISCV64
  F: */RiscV64/
  M: Sunil V L  [vlsunil]
 -R: Daniel Schaefer  [JohnAZoidberg]
 +R: Andrei Warkentin  [andreiw]

  LOONGARCH64
  F: */LoongArch64/
 @@ -157,16 +157,6 @@ R: Leif Lindholm  
 [leiflindholm]
  R: Sami Mujawar  [samimujawar]
  R: Gerd Hoffmann  [kraxel]

 -ArmVirtPkg: modules used on Xen
 -F: ArmVirtPkg/ArmVirtXen.*
 -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
 -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
 -F: ArmVirtPkg/PrePi/
 -F: ArmVirtPkg/XenAcpiPlatformDxe/
 -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
 -F: ArmVirtPkg/XenioFdtDxe/
 -R: Julien Grall  [jgrall]
>>>
>>> ArmVirtPkg Xen modules seize to have a dedicated maintainer. Can the
>>> generic ArmVirtPkg maintainers handle *more code* (particularly,
>>> functionality that's not trivial to test, unless you actively use
>>> Xen)?
>>
>> An alternative to removing this entire section is to replace Julien's
>> line with the following status line:
>>
>> S: Orphan
>>
>> The definition in Maintainers.txt is:
>>
>>  Orphan: No current maintainer [but maybe you could take the
>>  role as you write your new code].
>>
>> I think this might be clearer for all three of: contributors, consumers,
>> and existent maintainers.
>>
>> - Contributors: An ArmVirtPkg maintainer may techincally merge your
>> code, but you won't get substantive feedback
>>
>> - Consumers: you can build and run this code, but if it breaks, you get
>> to keep both parts
>>
>> - Existent ArmVirtPkg maintainers: you can rest assured in the knowledge
>> that you are not saddled with deep technical reviews for this subsystem
>> that you can't even boot in your environment. You're only responsible
>> for the technical act of merging patches.
> 
> I agree with this solution, but I do think there should be a "time
> limit" for orphaned code. You don't want to keep orphaned code for too
> long, this is not a practice we should support (which may lead to
> corporate code dumps where corps just dump a bunch of patches on the
> mailing list, fire and forget, and they're still "supported").

Very difficult question; there could be end-users relying on the feature
still, without anyone shouldering the maintenance costs :( I certainly
see your point, I just can't either agree *or* disagree with it!

(I recently cleaned up even *build* breakages in edk2-platforms; I also
found libraries that were not used *at all* by in-tree platforms. Those
obviously come from ancient corporate code-drops, and nobody must have
built them very recently, but if we remove them, we could still
ultimately harm end-users.)

[...]


>> This leads me to my main point: it may be time for edk2 to adopt a
>> leaner contribution process.
>>
>> We can insist on no patch going in without maintainer approval, but that
>> -- i.e., maintainer authority -- only works as long as it goes hand in
>> hand with maintainer responsibility: timely reviews. If the community
>> cannot offer enough working hours for reviewing patches for a subsystem,
>> then the requirements to contribute to that subsystem should be relaxed.
>> The other alternative is that the subsystem goes into stasis, where it
>> becomes effectively impossible to contribute to a subsystem.
>>
>> (NB this "relaxation of contribution rules" is entirely orthogonal to
>> using a mailing list vs. github pull requests. I still strongly prefer
>> the mailing list.)
>>
>> So maybe we could say, if there is no patch review for like 7 working
>> days (approx. one and half calendar weeks), then the patch should be
>> merged by default. Put differently, switch from "rejected by default" to
>> "accepted by default".
> 
> I understand your idea. I do, however, see it going in 2 different ways:
> 1) This pressures maintainers/corporations to be faster at reviewing
> patches, keeping a smooth flow of careful, reviewed patches. Things
> continue to work smoothly.
> 2) Maintainers keep being unresponsive, patches get "auto merged" and
> people need to deal with any ensuing breakage. Things /may/ regularly
> break.

You are too polite; it's quite likely that things *will* break :)


> IMO, this solution does not solve anything. If maintainers are short
> on time (or simply spread too thin), they will still be short on time
> unless corps give them more time for FOSS work. This just adds a fear
> factor ("Complete shite may be automerged if you don't have people on
> it!!").

The alternative is stasis, as I wrote above. Everything grinds to a
halt, and people can't proceed with careful, justified work either.

The true 

Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V

[Sunil V L]

On Mon, Oct 30, 2023 at 04:48:18PM +0530, Sunil V L wrote:
> On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote:
> > Use newly defined cache management operations for RISC-V where possible
> > It builds up on the support added for RISC-V cache management
> > instructions in BaseLib.
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Laszlo Ersek 
> > 
> > Signed-off-by: Dhaval Sharma 
> > Acked-by: Laszlo Ersek 
> > ---
> > 
> > Notes:
> > V7:
> > - Added PcdLib
> > - Restructure DEBUG message based on feedback on V6
> > - Make naming consistent to CMO, remove all CBO references
> > - Add ASSERT for not supported functions instead of plain debug message
> > - Added RB tag
> > V6:
> > - Utilize cache management instructions if HW supports it
> >   This patch is part of restructuring on top of v5
> > 
> >  MdePkg/MdePkg.dec  |   8 +
> >  MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf |   5 +
> >  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 
> > +---
> >  MdePkg/MdePkg.uni  |   4 +
> >  4 files changed, 165 insertions(+), 20 deletions(-)
> > 
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index ac54338089e8..fa92673ff633 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, 
> > PcdsPatchableInModule.AARCH64]
> ># @Prompt CPU Rng algorithm's GUID.
> >
> > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037
> >  
> > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64]
> > +  #
> > +  # Configurability to override RISC-V CPU Features
> > +  # BIT 0 = Cache Management Operations. This bit is relevant only if
> > +  # previous stage has feature enabled and user wants to disable it.
> NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that
> it is explicit.
> 
> > +  #
> > +  
> > gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69
> > +
> Instead of this, can default value match only those features which are
> enabled by default for qemu virt machine? That way, I think we can avoid
> having this PCD defined again in RiscVVirt.
> 
Sorry, I take back. This is common for all platforms. So, we can't take
qemu as reference.

Thanks,
Sunil


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

Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V

[Sunil V L]

On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote:
> Use newly defined cache management operations for RISC-V where possible
> It builds up on the support added for RISC-V cache management
> instructions in BaseLib.
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Laszlo Ersek 
> 
> Signed-off-by: Dhaval Sharma 
> Acked-by: Laszlo Ersek 
> ---
> 
> Notes:
> V7:
> - Added PcdLib
> - Restructure DEBUG message based on feedback on V6
> - Make naming consistent to CMO, remove all CBO references
> - Add ASSERT for not supported functions instead of plain debug message
> - Added RB tag
> V6:
> - Utilize cache management instructions if HW supports it
>   This patch is part of restructuring on top of v5
> 
>  MdePkg/MdePkg.dec  |   8 +
>  MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf |   5 +
>  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 
> +---
>  MdePkg/MdePkg.uni  |   4 +
>  4 files changed, 165 insertions(+), 20 deletions(-)
> 
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index ac54338089e8..fa92673ff633 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, 
> PcdsPatchableInModule.AARCH64]
># @Prompt CPU Rng algorithm's GUID.
>
> gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037
>  
> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64]
> +  #
> +  # Configurability to override RISC-V CPU Features
> +  # BIT 0 = Cache Management Operations. This bit is relevant only if
> +  # previous stage has feature enabled and user wants to disable it.
NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that
it is explicit.

> +  #
> +  
> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69
> +
Instead of this, can default value match only those features which are
enabled by default for qemu virt machine? That way, I think we can avoid
having this PCD defined again in RiscVVirt.

>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>## This value is used to set the base address of PCI express hierarchy.
># @Prompt PCI Express Base Address.
> diff --git 
> a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf 
> b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> index 6fd9cbe5f6c9..601a38d6c109 100644
> --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
> @@ -56,3 +56,8 @@ [LibraryClasses]
>BaseLib
>DebugLib
>  
> +[LibraryClasses.RISCV64]
> +  PcdLib
> +
> +[Pcd.RISCV64]
> +  gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride  ## CONSUMES
> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c 
> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> index 4eb18edb9aa7..5b3104afb67e 100644
> --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> @@ -2,6 +2,7 @@
>RISC-V specific functionality for cache.
>  
>Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
> reserved.
> +  Copyright (c) 2023, Rivos Inc. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> @@ -9,10 +10,115 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +
> +//
> +// TODO: Grab cache block size and make Cache Management Operation
> +// enabling decision based on RISC-V CPU HOB in
> +// future when it is available.
> +//
> +#define RISCV_CACHE_BLOCK_SIZE 64
> +#define RISCV_CPU_FEATURE_CMO_BITMASK  0x1
> +
Can we define these bits in the header file so that the definitions can
be used by multiple modules?

Thanks,
Sunil


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

Re: [edk2-devel] [PATCH v7 3/5] MdePkg: Implement RISC-V Cache Management Operations

[Sunil V L]

On Sun, Oct 29, 2023 at 08:16:11PM +0530, Dhaval wrote:
> Implement Cache Management Operations (CMO) defined by
> RISC-V spec https://github.com/riscv/riscv-CMOs.
> 
> Notes:
> 1. CMO only supports block based Operations. Meaning cache
>flush/invd/clean Operations are not available for the entire
>range. In that case we fallback on fence.i instructions.
> 2. Operations are implemented using Opcodes to make them compiler
>independent. binutils 2.39+ compilers support CMO instructions.
> 
> Test:
> 1. Ensured correct instructions are refelecting in asm
> 2. Not able to verify actual instruction in HW as Qemu ignores
>any actual cache operations.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Sunil V L 
> Cc: Daniel Schaefer 
> Cc: Laszlo Ersek 
> 
> Signed-off-by: Dhaval Sharma 
> Reviewed-by: Laszlo Ersek 
> ---
> 
> Notes:
> V7:
> - Modify instruction names as per feedback from V6
> - Added RB
> V6:
> - Implement Cache management instructions in Baselib
> 
>  MdePkg/Library/BaseLib/BaseLib.inf|  2 +-
>  MdePkg/Include/Library/BaseLib.h  | 33 
> 
>  MdePkg/Include/RiscV64/RiscVasm.inc   | 19 
> +++
>  MdePkg/Library/BaseLib/RiscV64/{FlushCache.S => RiscVCacheMgmt.S} | 17 
> ++
>  4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
> b/MdePkg/Library/BaseLib/BaseLib.inf
> index 03c7b02e828b..53389389448c 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -400,7 +400,7 @@ [Sources.RISCV64]
>RiscV64/RiscVCpuBreakpoint.S  | GCC
>RiscV64/RiscVCpuPause.S   | GCC
>RiscV64/RiscVInterrupt.S  | GCC
> -  RiscV64/FlushCache.S  | GCC
> +  RiscV64/RiscVCacheMgmt.S  | GCC
>RiscV64/CpuScratch.S  | GCC
>RiscV64/ReadTimer.S   | GCC
>RiscV64/RiscVMmu.S| GCC
> diff --git a/MdePkg/Include/Library/BaseLib.h 
> b/MdePkg/Include/Library/BaseLib.h
> index d4b56a9601da..c42cc165dc82 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -226,6 +226,39 @@ RiscVInvalidateDataCacheAsmFence (
>VOID
>);
>  
> +/**
> +  RISC-V flush cache block. Atomically perform a clean operation
> +  followed by an invalidate operation
> +
> +**/
> +VOID
> +EFIAPI
> +RiscVCpuCacheFlushAsmCmo (

NIT: I would keep Asm at the end for these interface names.

Otherwise,
Reviewed-by: Sunil V L 

Thanks,
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110307): https://edk2.groups.io/g/devel/message/110307
Mute This Topic: https://groups.io/mt/102256466/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] Maintainers.txt: Update based on active community members

[Julien Grall]

Hi Michael,

On 28/10/2023 20:23, Michael D Kinney wrote:

Over the past few months, all the of the Maintainers and
Reviewers listed in Maintainers.txt have been contacted to make
sure Maintainers.txt accurately represents the TianoCore
community members that are actively participating in their
roles.  Based on specific feedback, bounced emails, and no
responses, updates have been made.

* RISCV64: Daniel Schaefer replaced with Andrei Warkentin
* ArmVirtPkg Xen has no remaining reviewers and review
   responsibility defaults to ArmVirtPkg Maintainers/Reviewers.
* ACPI modules related to S3 has no remaining reviewers and
   review responsibility defaults to MdeModulePkg Maintainers/
   Reviewers.
* OVMF CSM modules has no remaining reviewers and review
   responsibility defaults to OvmfPkg Maintainers/Reviewers.
* Bounce: Chan Laura 
* Many smaller updates removing individuals that are no
   longer involved or have replacement coverage.

Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Andrei Warkentin 
Cc: Catharine West 
Cc: Dandan Bi 
Cc: Daniel Schaefer 
Cc: David Woodhouse 
Cc: Debkumar De 
Cc: Eric Dong 
Cc: Guomin Jiang 
Cc: Hao A Wu 
Cc: James Bottomley 
Cc: Jian J Wang 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Peter Grehan 
Cc: Qi Zhang 
Cc: Ray Han Lim Ng 
Cc: Stefan Berger 
Cc: Wenxing Hou 
Cc: Xiaoyu Lu 
Signed-off-by: Michael D Kinney 
---
  Maintainers.txt | 53 ++---
  1 file changed, 2 insertions(+), 51 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 3f40cdeb5554..2b03ccbe54aa 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -93,7 +93,7 @@ M: Sami Mujawar  [samimujawar]
  RISCV64
  F: */RiscV64/
  M: Sunil V L  [vlsunil]
-R: Daniel Schaefer  [JohnAZoidberg]
+R: Andrei Warkentin  [andreiw]
  
  LOONGARCH64

  F: */LoongArch64/
@@ -157,16 +157,6 @@ R: Leif Lindholm  [leiflindholm]
  R: Sami Mujawar  [samimujawar]
  R: Gerd Hoffmann  [kraxel]
  
-ArmVirtPkg: modules used on Xen

-F: ArmVirtPkg/ArmVirtXen.*
-F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
-F: ArmVirtPkg/Library/XenVirtMemInfoLib/
-F: ArmVirtPkg/PrePi/
-F: ArmVirtPkg/XenAcpiPlatformDxe/
-F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
-F: ArmVirtPkg/XenioFdtDxe/
-R: Julien Grall  [jgrall]
-
A few months ago, I have asked the Xen community if someone wanted to 
take over the responsibility. Unfortunately, no-one step up. So this 
seems to be the best approach:


Acked-by: Julien Grall 

Cheers,

--
Julien Grall


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110306): https://edk2.groups.io/g/devel/message/110306
Mute This Topic: https://groups.io/mt/102245264/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] Maintainers.txt: Update based on active community members

[Laszlo Ersek]

On 10/30/23 03:40, Yao, Jiewen wrote:
> Thanks Mike. I am reading the WIKI page.
> 
> 
>> and/or provides testing or regression testing for the Package (or some 
>> modules thereof), in certain platforms and environments.
> 
> [Jiewen] Are we expecting Reviewer to provide testing or regression testing 
> for the package?
> Is that what the reviewer *commits* to do?
> For example, Maintainer may ask the reviewer to do some testing, right?

It depends on the reviewer's individual commitment.

First of all, the burden of testing / regression-testing, to a
reasonable extent [1], is on the submitter.

[1] In some cases, the submitter cannot test the code they modify in all
possible environments / circumstances. In such cases, the submitter
should test the change code wherever they can, as widely as they can,
and be upfront (in the commit message) about lacking coverage in other
environments they might be aware of. It is then fine for the maintainer
(or even reviewer) to ask others for further / wider testing, but trying
to saddle someone with that testing as an *obligation* will never fly.
That would only alienate people from contributing.

This was the primary reason for splitting Xen code as sharply as
possible from non-Xen code in both ArmVirtPkg and OvmfPkg. Xen and
QEMU/KVM are so different environments, with so distinct audiences, that
keeping code common was *worse* than duplicating and customizing code.
We could never *sufficiently* regression-test our changes for each
other. It was best to separate those areas of interest. Demanding that I
regression-test on Xen, or that Anthony or Julien regression-test on
QEMU/KVM, would have lead nowhere.

Second, the level of commitment varies. A reviewer may have a fleeting
interest in a module (just want to be in the loop), or else they may be
completely invested in it, and they might actually prefer being a
maintainer.

For example, I had seen many bad regressions in OVMF due to UefiCpuPkg
patches, thus, even thouogh I absolutely didn't welcome the additional
responsibility, I asked to be added as a Reviewer for UefiCpuPkg. With
that, I wanted to formalize my request to be CC'd on all UefiCpuPkg
patches, but I also committed to regression testing, and maybe even
reviewing, those patches. It worked out quite well, but of course I was
still selective in what I would review and test. If I could immediately
determine that the patch modified code in UefiCpuPkg that never ran on
(or wasn't even built into) OVMF, I would clearly state on the list that
I'd not review or test the patch, i.e., that nobody should wait for me.

> 
> 
>> Reviewer is responsible for timely responses on emails addressed to them 
>> (preferably less than calendar week).
> 
> [Jiewen]
> Is that what the reviewer *commits* to do?

What I think we can expect a reviewer to *commit* to is to say
*something* reasonably quickly. The whole idea is to support others in
making a *decision*, in making progress. So the "something" the reviewer
says may well be:

- "this does not apply to the area I have expertise or interest in, so
please proceed with this patch without my feedback (testing or review or
opinion etc)"

- "I don't have time for this right now, so please go ahead; if it
breaks, we'll figure it out later" (the maintainer need not accept this,
and might want to block the patch independently for a bit longer, until
someone else provides the desired review or testing, but the reviewer is
still totally OK to say this)

- "please give me a few more days to review this set".

> For example, Maintainer may ask the reviewer to provide feedback, right?

IMO, the maintainer is welcome to request feedback, of course; that's
presumably why the reviewer wanted to be listed in Maintainers.txt in
the first place.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110305): https://edk2.groups.io/g/devel/message/110305
Mute This Topic: https://groups.io/mt/102245264/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 v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V

[Laszlo Ersek]

On 10/29/23 20:07, Pedro Falcato wrote:
> On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma  wrote:
>>
>> Use newly defined cache management operations for RISC-V where possible
>> It builds up on the support added for RISC-V cache management
>> instructions in BaseLib.
>> Cc: Michael D Kinney 
>> Cc: Liming Gao 
>> Cc: Zhiguang Liu 
>> Cc: Laszlo Ersek 
>>
>> Signed-off-by: Dhaval Sharma 
>> Acked-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> V7:
>> - Added PcdLib
>> - Restructure DEBUG message based on feedback on V6
>> - Make naming consistent to CMO, remove all CBO references
>> - Add ASSERT for not supported functions instead of plain debug message
>> - Added RB tag
>> V6:
>> - Utilize cache management instructions if HW supports it
>>   This patch is part of restructuring on top of v5
>>
>>  MdePkg/MdePkg.dec  |   8 +
>>  MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf |   5 +
>>  MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 
>> +---
>>  MdePkg/MdePkg.uni  |   4 +
>>  4 files changed, 165 insertions(+), 20 deletions(-)
>>
>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>> index ac54338089e8..fa92673ff633 100644
>> --- a/MdePkg/MdePkg.dec
>> +++ b/MdePkg/MdePkg.dec
>> @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, 
>> PcdsPatchableInModule.AARCH64]
>># @Prompt CPU Rng algorithm's GUID.
>>
>> gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037
>>
>> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64]
>> +  #
>> +  # Configurability to override RISC-V CPU Features
>> +  # BIT 0 = Cache Management Operations. This bit is relevant only if
>> +  # previous stage has feature enabled and user wants to disable it.
>> +  #
>> +  
>> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69
>> +
>>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>>## This value is used to set the base address of PCI express hierarchy.
>># @Prompt PCI Express Base Address.
>> diff --git 
>> a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf 
>> b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
>> index 6fd9cbe5f6c9..601a38d6c109 100644
>> --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
>> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
>> @@ -56,3 +56,8 @@ [LibraryClasses]
>>BaseLib
>>DebugLib
>>
>> +[LibraryClasses.RISCV64]
>> +  PcdLib
>> +
>> +[Pcd.RISCV64]
>> +  gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride  ## CONSUMES
>> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c 
>> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
>> index 4eb18edb9aa7..5b3104afb67e 100644
>> --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
>> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
>> @@ -2,6 +2,7 @@
>>RISC-V specific functionality for cache.
>>
>>Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
>> reserved.
>> +  Copyright (c) 2023, Rivos Inc. All rights reserved.
>>
>>SPDX-License-Identifier: BSD-2-Clause-Patent
>>  **/
>> @@ -9,10 +10,115 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +
>> +//
>> +// TODO: Grab cache block size and make Cache Management Operation
>> +// enabling decision based on RISC-V CPU HOB in
>> +// future when it is available.
>> +//
>> +#define RISCV_CACHE_BLOCK_SIZE 64
>> +#define RISCV_CPU_FEATURE_CMO_BITMASK  0x1
>> +
>> +typedef enum {
>> +  Clean,
>> +  Flush,
>> +  Invld,
>> +} CACHE_OP;
> 
> small nit: You may want to do something like CACHE_OP_{CLEAN, FLUSH,
> INVID}. but since this is a file-local enum I don't consider this
> merge-blocking.
> 

Agree with you on the prefix, but not on the uppercase spelling; edk2
(and UEFI) uses CamelCase enumeration constants. Compare: at the very
top of "MdePkg/Include/Uefi/UefiSpec.h", we have EFI_ALLOCATE_TYPE with
constants like AllocateAnyPages.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110304): https://edk2.groups.io/g/devel/message/110304
Mute This Topic: https://groups.io/mt/102256468/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 v7 3/5] MdePkg: Implement RISC-V Cache Management Operations

[Laszlo Ersek]

On 10/29/23 20:12, Pedro Falcato wrote:
> On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma  wrote:
>>
>> Implement Cache Management Operations (CMO) defined by
>> RISC-V spec https://github.com/riscv/riscv-CMOs.
>>
>> Notes:
>> 1. CMO only supports block based Operations. Meaning cache
>>flush/invd/clean Operations are not available for the entire
>>range. In that case we fallback on fence.i instructions.
>> 2. Operations are implemented using Opcodes to make them compiler
>>independent. binutils 2.39+ compilers support CMO instructions.
>>
>> Test:
>> 1. Ensured correct instructions are refelecting in asm
> 
> nit: reflecting
> 
>> 2. Not able to verify actual instruction in HW as Qemu ignores
>>any actual cache operations.
> 
> Do you have no way to test this in hardware? Since Rivos is a RISCV
> vendor and all ;)
> I don't like inviting the idea of merging CPU architectural changes
> without actually testing them in something resembling real silicon
> (i.e QEMU KVM is _fine_, QEMU TCG really isn't).
> 

Hopefully I'm not drawing an incorrect parallel here, but, as I recall
arm64 enablement in 2014, nearly all initial enablement in RHEL occurred
on software emulators (ARM Foundation Model, ARM FVP, then QEMU TCG).
You need to start somewhere. In particular, qemu-system-aarch64 was a
huge step forward (performance-wise) once it *existed*, relative to the
Foundation Model / FVP, even though qemu-system-aarch64 wouldn't emulate
CPU caches (IIRC).

Laszlo



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Correct file description for MpHandOff header file

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: Xie, Yuanhao  
Sent: Saturday, October 7, 2023 2:32 PM
To: devel@edk2.groups.io
Cc: Dong, Eric ; Kumar, Rahul R ; 
Tom Lendacky ; Xie, Yuanhao 
Subject: [PATCH] UefiCpuPkg: Correct file description for MpHandOff header file

Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h 
b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
index 83e4055ec9..77854d6a81 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -1,6 +1,9 @@
 /** @file
-  Defines the HOB GUID used to describe the MSEG memory region allocated in 
PEI.
+  Defines the HOB GUID, which is utilized for transferring essential  
+ information from the PEI to the DXE phase.
+
   Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.
+
   SPDX-License-Identifier: BSD-2-Clause-Patent  **/
 
--
2.36.1.windows.1



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

Re: [edk2-devel] [PATCH v2 0/1] StandaloneMmCore finds drivers in uncompressed inner fv.

[Xu, Wei6]

Hi Laszlo and Ray,

Thanks a lot for the review. Patch V3 is sent out to:
1. Separate patch per individual issue fix
2. Limit FwVol encapsulation section recursion in MmCoreFfsFindMmDriver().
(https://edk2.groups.io/g/devel/message/110296)

Could you please help to review it again? Thanks a lot!

BR,
Wei


-Original Message-
From: Laszlo Ersek  
Sent: Saturday, October 28, 2023 7:08 PM
To: Ni, Ray ; Xu, Wei6 ; 
devel@edk2.groups.io
Cc: Ard Biesheuvel ; Sami Mujawar 

Subject: Re: [PATCH v2 0/1] StandaloneMmCore finds drivers in uncompressed 
inner fv.

On 10/27/23 07:49, Ni, Ray wrote:
> Wei,
> Thanks for fixing the 3 issues.
> Can you kindly separate the one patch to at least 2 patches?
> One patch is to fix minor issues.
> The other is to add support of nested uncompressed FV.

Yes please!

I'd even prefer a separate patch per individual issue fix (especially if you 
count the recursion limiting too).

Thanks!
Laszlo

> 
> Thanks,
> Ray
> --
> --
> *From:* Xu, Wei6 
> *Sent:* Friday, October 27, 2023 8:59 AM
> *To:* devel@edk2.groups.io 
> *Cc:* Xu, Wei6 ; Laszlo Ersek ; 
> Ard Biesheuvel ; Sami Mujawar 
> ; Ni, Ray 
> *Subject:* [PATCH v2 0/1] StandaloneMmCore finds drivers in 
> uncompressed inner fv.
>  
> V1:
> This patch is to fix the issue that StandaloneMmCore fails to detect 
> uncompressed inner FV.
> PR: https://github.com/tianocore/edk2/pull/4943
> 
> 
> V2:
> Based on V1, fix some other issues
> 1. Add Missing object size checks before casting pointers to header 
> types
>   a. InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)SectionData;
>  This is introduced in V1, add the size check on SectionDataSize 
> against EFI_FIRMWARE_VOLUME_HEADER
>   b. Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
>  Use FfsFindSection instead of FfsFindSectionData to avoid pointer 
> casting.
> 2. Fix potential memory leak issue that ScratchBuffer is not freed 
> when page allocation for DstBuffer fails.
> PR: https://github.com/tianocore/edk2/pull/4965
> 
> 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> 
> Wei6 Xu (1):
>   StandaloneMmPkg: Fix some issues in function MmCoreFfsFindMmDriver.
> 
>  StandaloneMmPkg/Core/FwVol.c | 34 ++
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> --
> 2.29.2.windows.2
> 



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

[edk2-devel] [PATCH v3 4/4] StandaloneMmPkg/Core: Fix the failure to find uncompressed inner FV

[Xu, Wei6]

The MmCoreFfsFindMmDriver only checks for encapsulated compressed FVs.
When an inner FV is uncompressed, StandaloneMmCore will miss the FV and
all the MM drivers in the FV will not be dispatched.
Add checks for uncompressed inner FV to fix this issue.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Wei6 Xu 
---
 StandaloneMmPkg/Core/FwVol.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index fa335d62c252..783dbaf9b048 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -116,6 +116,25 @@ MmCoreFfsFindMmDriver (
   break;
 }
 
+//
+// Check uncompressed firmware volumes
+//
+Status = FfsFindSectionData (
+   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
+   FileHeader,
+   ,
+   
+   );
+if (!EFI_ERROR (Status)) {
+  if (SectionDataSize > sizeof (EFI_FIRMWARE_VOLUME_HEADER)) {
+InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)SectionData;
+MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
+  }
+}
+
+//
+// Check compressed firmware volumes
+//
 Status = FfsFindSection (
EFI_SECTION_GUID_DEFINED,
FileHeader,
-- 
2.29.2.windows.2



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

[edk2-devel] [PATCH v3 3/4] StandaloneMmPkg/Core: Fix issue that section address might be wrong

[Xu, Wei6]

MmCoreFfsFindMmDriver() assumes FileHeader is EFI_FFS_FILE_HEADER.
If FileHeader is an EFI_FFS_FILE_HEADER2, 'FileHeader + 1' will get a
wrong section address. Use FfsFindSection to get the section directly,
instead of 'FileHeader + 1' to avoid this issue.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Wei6 Xu 
---
 StandaloneMmPkg/Core/FwVol.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 9d0ce66ef839..fa335d62c252 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -116,23 +116,21 @@ MmCoreFfsFindMmDriver (
   break;
 }
 
-Status = FfsFindSectionData (
+Status = FfsFindSection (
EFI_SECTION_GUID_DEFINED,
FileHeader,
-   ,
-   
+   
);
 if (EFI_ERROR (Status)) {
   break;
 }
 
-Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
-Status  = ExtractGuidedSectionGetInfo (
-Section,
-,
-,
-
-);
+Status = ExtractGuidedSectionGetInfo (
+   Section,
+   ,
+   ,
+   
+   );
 if (EFI_ERROR (Status)) {
   break;
 }
-- 
2.29.2.windows.2



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

[edk2-devel] [PATCH v3 2/4] StandaloneMmPkg/Core: Fix potential memory leak issue

[Xu, Wei6]

In MmCoreFfsFindMmDriver(), ScratchBuffer is not freed in the error
return path that DstBuffer page allocation fails. Free ScratchBuffer
before return with error.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Wei6 Xu 
---
 StandaloneMmPkg/Core/FwVol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index e1e20ffd14ac..9d0ce66ef839 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -150,6 +150,7 @@ MmCoreFfsFindMmDriver (
 //
 DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES 
(DstBufferSize));
 if (DstBuffer == NULL) {
+  FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
   return EFI_OUT_OF_RESOURCES;
 }
 
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110298): https://edk2.groups.io/g/devel/message/110298
Mute This Topic: https://groups.io/mt/102270547/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/4] StandaloneMmPkg/Core: Limit FwVol encapsulation section recursion

[Xu, Wei6]

MmCoreFfsFindMmDriver() is called recursively for encapsulation sections.
Currently this recursion is not limited. Introduce a new PCD
(fixed-at-build, or patchable-in-module), and make MmCoreFfsFindMmDriver()
track the section nesting depth against that PCD.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Ray Ni 
Signed-off-by: Wei6 Xu 
---
 StandaloneMmPkg/Core/FwVol.c  | 16 ++--
 StandaloneMmPkg/Core/StandaloneMmCore.c   |  5 +++--
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 +++
 StandaloneMmPkg/StandaloneMmPkg.dec   |  5 +
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 1f6d7714ba97..e1e20ffd14ac 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -48,6 +48,9 @@ FvIsBeingProcessed (
   MM driver and return its PE32 image.
 
   @param [in] FwVolHeader   Pointer to memory mapped FV
+  @param [in] Depth Nesting depth of encapsulation sections. Callers
+different from MmCoreFfsFindMmDriver() are
+responsible for passing in a zero Depth.
 
   @retval  EFI_SUCCESSSuccess.
   @retval  EFI_INVALID_PARAMETER  Invalid parameter.
@@ -55,11 +58,15 @@ FvIsBeingProcessed (
   @retval  EFI_OUT_OF_RESOURCES   Out of resources.
   @retval  EFI_VOLUME_CORRUPTED   Firmware volume is corrupted.
   @retval  EFI_UNSUPPORTEDOperation not supported.
+  @retval  EFI_ABORTEDRecursion aborted because Depth has been
+  greater than or equal to
+  PcdFwVolMmMaxEncapsulationDepth.
 
 **/
 EFI_STATUS
 MmCoreFfsFindMmDriver (
-  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
+  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader,
+  IN  UINT32  Depth
   )
 {
   EFI_STATUS  Status;
@@ -84,6 +91,11 @@ MmCoreFfsFindMmDriver (
 
   DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
 
+  if (Depth >= PcdGet32 (PcdFwVolMmMaxEncapsulationDepth)) {
+DEBUG ((DEBUG_ERROR, "%a: recursion aborted due to nesting depth\n", 
__func__));
+return EFI_ABORTED;
+  }
+
   if (FvHasBeenProcessed (FwVolHeader)) {
 return EFI_SUCCESS;
   }
@@ -172,7 +184,7 @@ MmCoreFfsFindMmDriver (
 }
 
 InnerFvHeader = (VOID *)(Section + 1);
-Status= MmCoreFfsFindMmDriver (InnerFvHeader);
+Status= MmCoreFfsFindMmDriver (InnerFvHeader, Depth + 1);
 if (EFI_ERROR (Status)) {
   goto FreeDstBuffer;
 }
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c 
b/StandaloneMmPkg/Core/StandaloneMmCore.c
index d221f1d1115d..523ea0a632a1 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -11,7 +11,8 @@
 
 EFI_STATUS
 MmCoreFfsFindMmDriver (
-  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
+  IN  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader,
+  IN  UINT32  Depth
   );
 
 EFI_STATUS
@@ -643,7 +644,7 @@ StandaloneMmMain (
   //
   DEBUG ((DEBUG_INFO, "Mm Dispatch StandaloneBfvAddress - 0x%08x\n", 
gMmCorePrivate->StandaloneBfvAddress));
   if (gMmCorePrivate->StandaloneBfvAddress != 0) {
-MmCoreFfsFindMmDriver ((EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)gMmCorePrivate->StandaloneBfvAddress);
+MmCoreFfsFindMmDriver ((EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)gMmCorePrivate->StandaloneBfvAddress, 0);
 MmDispatcher ();
   }
 
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf 
b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index c44b9ff33303..02ecd68f37e2 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -76,6 +76,9 @@ [Guids]
   gEfiEventExitBootServicesGuid
   gEfiEventReadyToBootGuid
 
+[Pcd]
+  gStandaloneMmPkgTokenSpaceGuid.PcdFwVolMmMaxEncapsulationDepth##CONSUMES
+
 #
 # This configuration fails for CLANGPDB, which does not support PIE in the GCC
 # sense. Such however is required for ARM family StandaloneMmCore
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec 
b/StandaloneMmPkg/StandaloneMmPkg.dec
index 46784d94e421..c43632d6d8ae 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -48,3 +48,8 @@ [Guids]
   gEfiStandaloneMmNonSecureBufferGuid  = { 0xf00497e3, 0xbfa2, 0x41a1, { 
0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
   gEfiArmTfCpuDriverEpDescriptorGuid   = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 
0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
 
+[PcdsFixedAtBuild, PcdsPatchableInModule]
+  ## Maximum permitted encapsulation levels of sections in a firmware volume,
+  #  in the MM phase. Minimum value is 1. Sections nested more deeply are 
rejected.
+  # @Prompt Maximum permitted FwVol section nesting depth (exclusive) in MM.
+  
gStandaloneMmPkgTokenSpaceGuid.PcdFwVolMmMaxEncapsulationDepth|0x10|UINT32|0x0001
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You 

[edk2-devel] [PATCH v3 0/4] StandaloneMmCore finds drivers in uncompressed inner fv.

[Xu, Wei6]

V1:
This patch is to fix the issue that StandaloneMmCore fails to detect 
uncompressed inner FV.
PR: https://github.com/tianocore/edk2/pull/4943

V2:
Based on V1, fix some other issues
1. Add Missing object size checks before casting pointers to header types
  a. InnerFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)SectionData; 
 This is introduced in V1, add the size check on SectionDataSize against 
EFI_FIRMWARE_VOLUME_HEADER
  b. Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
 Use FfsFindSection instead of FfsFindSectionData to avoid pointer casting.
2. Fix potential memory leak issue that ScratchBuffer is not freed when page 
allocation for DstBuffer fails.
PR: https://github.com/tianocore/edk2/pull/4965

V3:
1. Separate patch per individual issue fix on patch V2.
2. Fix one more issue: Limit FwVol encapsulation section recursion in 
MmCoreFfsFindMmDriver().
PR: https://github.com/tianocore/edk2/pull/4975


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

Wei6 Xu (4):
  StandaloneMmPkg/Core: Limit FwVol encapsulation section recursion
  StandaloneMmPkg/Core: Fix potential memory leak issue
  StandaloneMmPkg/Core: Fix issue that section address might be wrong
  StandaloneMmPkg/Core: Fix the failure to find uncompressed inner FV

 StandaloneMmPkg/Core/FwVol.c  | 50 ++-
 StandaloneMmPkg/Core/StandaloneMmCore.c   |  5 ++-
 StandaloneMmPkg/Core/StandaloneMmCore.inf |  3 ++
 StandaloneMmPkg/StandaloneMmPkg.dec   |  5 +++
 4 files changed, 51 insertions(+), 12 deletions(-)

-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110296): https://edk2.groups.io/g/devel/message/110296
Mute This Topic: https://groups.io/mt/102270545/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] Maintainers.txt: Update based on active community members

[Wu, Hao A]

For changes to item "Hao A Wu  [hwu25]":
Reviewed-by: Hao A Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: Kinney, Michael D 
> Sent: Sunday, October 29, 2023 3:24 AM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Leif Lindholm
> ; Warkentin, Andrei
> ; West, Catharine
> ; Bi, Dandan ; Daniel
> Schaefer ; David Woodhouse
> ; De, Debkumar ; Dong,
> Eric ; Jiang, Guomin ; Wu,
> Hao A ; James Bottomley ;
> Wang, Jian J ; Justen, Jordan L
> ; Julien Grall ; Peter Grehan
> ; Zhang, Qi1 ; Ng, Ray Han Lim
> ; Stefan Berger ; Hou,
> Wenxing ; Lu, Xiaoyu1 
> Subject: [Patch 1/1] Maintainers.txt: Update based on active community
> members
> 
> Over the past few months, all the of the Maintainers and Reviewers listed in
> Maintainers.txt have been contacted to make sure Maintainers.txt accurately
> represents the TianoCore community members that are actively participating
> in their roles.  Based on specific feedback, bounced emails, and no responses,
> updates have been made.
> 
> * RISCV64: Daniel Schaefer replaced with Andrei Warkentin
> * ArmVirtPkg Xen has no remaining reviewers and review
>   responsibility defaults to ArmVirtPkg Maintainers/Reviewers.
> * ACPI modules related to S3 has no remaining reviewers and
>   review responsibility defaults to MdeModulePkg Maintainers/
>   Reviewers.
> * OVMF CSM modules has no remaining reviewers and review
>   responsibility defaults to OvmfPkg Maintainers/Reviewers.
> * Bounce: Chan Laura 
> * Many smaller updates removing individuals that are no
>   longer involved or have replacement coverage.
> 
> Cc: Andrew Fish 
> Cc: Leif Lindholm 
> Cc: Andrei Warkentin 
> Cc: Catharine West 
> Cc: Dandan Bi 
> Cc: Daniel Schaefer 
> Cc: David Woodhouse 
> Cc: Debkumar De 
> Cc: Eric Dong 
> Cc: Guomin Jiang 
> Cc: Hao A Wu 
> Cc: James Bottomley 
> Cc: Jian J Wang 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Peter Grehan 
> Cc: Qi Zhang 
> Cc: Ray Han Lim Ng 
> Cc: Stefan Berger 
> Cc: Wenxing Hou 
> Cc: Xiaoyu Lu 
> Signed-off-by: Michael D Kinney 
> ---
>  Maintainers.txt | 53 ++---
>  1 file changed, 2 insertions(+), 51 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt index
> 3f40cdeb5554..2b03ccbe54aa 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -93,7 +93,7 @@ M: Sami Mujawar 
> [samimujawar]
>  RISCV64
>  F: */RiscV64/
>  M: Sunil V L  [vlsunil]
> -R: Daniel Schaefer  [JohnAZoidberg]
> +R: Andrei Warkentin  [andreiw]
> 
>  LOONGARCH64
>  F: */LoongArch64/
> @@ -157,16 +157,6 @@ R: Leif Lindholm 
> [leiflindholm]
>  R: Sami Mujawar  [samimujawar]
>  R: Gerd Hoffmann  [kraxel]
> 
> -ArmVirtPkg: modules used on Xen
> -F: ArmVirtPkg/ArmVirtXen.*
> -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
> -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
> -F: ArmVirtPkg/PrePi/
> -F: ArmVirtPkg/XenAcpiPlatformDxe/
> -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
> -F: ArmVirtPkg/XenioFdtDxe/
> -R: Julien Grall  [jgrall]
> -
>  BaseTools
>  F: BaseTools/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> @@ -187,8 +177,7 @@ F: CryptoPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>  M: Jiewen Yao  [jyao1]
>  M: Yi Li  [liyi77]
> -R: Xiaoyu Lu  [xiaoyuxlu]
> -R: Guomin Jiang  [guominjia]
> +R: Wenxing Hou  [Wenxing-hou]
> 
>  DynamicTablesPkg
>  F: DynamicTablesPkg/
> @@ -202,7 +191,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
>  M: Leif Lindholm  [leiflindholm]
>  M: Ard Biesheuvel  [ardbiesheuvel]
>  M: Abner Chang  [changab]
> -R: Daniel Schaefer  [JohnAZoidberg]
> 
>  EmulatorPkg
>  F: EmulatorPkg/
> @@ -228,7 +216,6 @@ F: FmpDevicePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/FmpDevicePkg
>  M: Liming Gao  [lgao4]
>  M: Michael D Kinney  [mdkinney]
> -R: Guomin Jiang  [guominjia]
>  R: Wei6 Xu  [xuweiintel]
> 
>  IntelFsp2Pkg
> @@ -237,7 +224,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg
>  M: Chasel Chiu  [ChaselChiu]
>  M: Nate DeSimone  [nate-desimone]
>  M: Duggapu Chinni B  [cbduggap]
> -M: Ray Han Lim Ng  [rayhanlimng]
>  R: Star Zeng  [lzeng14]
>  R: Ted Kuo  [tedkuo1]
>  R: Ashraf Ali S  [AshrafAliS] @@ -258,7 +244,6 @@
> R: Susovan Mohapatra  [susovanmohapatra]
> MdeModulePkg
>  F: MdeModulePkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> -M: Jian J Wang  [jwang36]
>  M: Liming Gao  [lgao4]
> 
>  MdeModulePkg: ACPI modules
> @@ -268,15 +253,6 @@ R: Zhiguang Liu 
> [LiuZhiguang001]
>  R: Dandan Bi  [dandanbi]
>  R: Liming Gao  [lgao4]
> 
> -MdeModulePkg: ACPI modules related to S3
> -F: MdeModulePkg/*LockBox*/
> -F: MdeModulePkg/Include/*BootScript*.h
> -F: MdeModulePkg/Include/*LockBox*.h
> -F: MdeModulePkg/Include/*S3*.h
> -F: MdeModulePkg/Library/*S3*/
> -R: Hao A Wu  [hwu25]
> -R: Eric Dong  [ydong10]
> -
>  MdeModulePkg: BDS modules
>  F: MdeModulePkg/*BootManager*/
>  F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> 

Re: [edk2-devel] [edk2-platforms][PATCH] ManageabilityPkg: Uncrustify on C source files

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang 

Regards,
Nickle

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, October 26, 2023 5:50 PM
> To: devel@edk2.groups.io; Chang, Abner 
> Cc: Attar, AbdulLateef (Abdul Lateef) ; Nickle
> Wang ; Konstantin Aladyshev 
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH] ManageabilityPkg: Uncrustify
> on C source files
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On 10/26/23 10:06, Chang, Abner via groups.io wrote:
> > From: Abner Chang 
> >
> > Run Uncrustify on the C source files those were
> > modified in commit ID from 3f4c4384 to 28ff8f29.
> >
> > Signed-off-by: Abner Chang 
> > Cc: Abdul Lateef Attar 
> > Cc: Nickle Wang 
> > Cc: Konstantin Aladyshev 
> > ---
> >  .../Include/Library/BasePldmProtocolLib.h |  6 ++---
> >  .../PldmProtocol/Common/PldmProtocolCommon.h  |  4 ++--
> >  .../Common/KcsCommon.c|  2 +-
> >  .../PldmProtocolLibrary/Dxe/PldmProtocolLib.c | 10 -
> >  .../MctpProtocol/Common/MctpProtocolCommon.c  | 22 ---
> >  .../Universal/MctpProtocol/Dxe/MctpProtocol.c |  6 +++--
> >  .../PldmProtocol/Common/PldmProtocolCommon.c  | 12 +-
> >  .../Universal/PldmProtocol/Dxe/PldmProtocol.c |  8 +++
> >  .../PldmSmbiosTransferDxe.c   |  2 +-
> >  9 files changed, 40 insertions(+), 32 deletions(-)
> >
> > diff --git a/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
> b/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
> > index a698197263..404474a023 100644
> > --- a/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
> > +++ b/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
> > @@ -21,9 +21,9 @@
> >  **/
> >  EFI_STATUS
> >  PldmSetTerminus (
> > -  IN  UINT8   SourceId,
> > -  IN  UINT8   DestinationId
> > -);
> > +  IN  UINT8  SourceId,
> > +  IN  UINT8  DestinationId
> > +  );
> >
> >  /**
> >This service enables submitting commands via EDKII PLDM protocol.
> > diff --git
> a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCo
> mmon.h
> b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCo
> mmon.h
> > index eb273c4f46..30f4d95847 100644
> > ---
> a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCo
> mmon.h
> > +++
> b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCo
> mmon.h
> > @@ -12,8 +12,8 @@
> >  #include 
> >  #include 
> >
> > -#define GET_PLDM_MESSAGE_PAYLOAD_SIZE(PayloadSize) (PayloadSize -
> sizeof (PLDM_RESPONSE_HEADER))
> > -#define GET_PLDM_MESSAGE_PAYLOAD_PTR(PayloadPtr) ((UINT8
> *)PayloadPtr + sizeof (PLDM_RESPONSE_HEADER))
> > +#define GET_PLDM_MESSAGE_PAYLOAD_SIZE(PayloadSize)  (PayloadSize -
> sizeof (PLDM_RESPONSE_HEADER))
> > +#define GET_PLDM_MESSAGE_PAYLOAD_PTR(PayloadPtr)((UINT8
> *)PayloadPtr + sizeof (PLDM_RESPONSE_HEADER))
> >
> >  typedef struct {
> >UINT8 PldmType;
> > diff --git
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/K
> csCommon.c
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/K
> csCommon.c
> > index 4f7e7d450f..d80267cb57 100644
> > ---
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/K
> csCommon.c
> > +++
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/K
> csCommon.c
> > @@ -496,7 +496,6 @@ KcsReadResponseHeader (
> >
> >*ResponseHeader = NULL;
> >if (CompareGuid (, mSingleSessionToken-
> >Token.ManageabilityProtocolSpecification)) {
> > -
> >  // For MCTP over KCS
> >  ExpectedHeaderSize = sizeof (MANAGEABILITY_MCTP_KCS_HEADER);
> >  DEBUG ((
> > @@ -711,6 +710,7 @@ KcsTransportSendCommand (
> >
> >*ResponseDataSize = ((MANAGEABILITY_MCTP_KCS_HEADER
> *)RspHeader)->ByteCount;
> >  }
> > +
> >  FreePool (RspHeader);
> >
> >  ExpectedResponseDataSize = *ResponseDataSize;
> > diff --git
> a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.
> c
> b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.
> c
> > index 37231b0756..9e1e664d6b 100644
> > ---
> a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.
> c
> > +++
> b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.
> c
> > @@ -29,9 +29,9 @@ UINT8mDestinationPldmTerminusId = 0;
> >  **/
> >  EFI_STATUS
> >  PldmSetTerminus (
> > -  IN  UINT8   SourceId,
> > -  IN  UINT8   DestinationId
> > -)
> > +  IN  UINT8  SourceId,
> > +  IN  UINT8  DestinationId
> > +  )
> >  {
> >mSourcePldmTerminusId  = SourceId;
> >mDestinationPldmTerminusId = DestinationId;
> > @@ -111,6 +111,7 @@ PldmSubmitCommand (
> >
> >return Status;
> >  }
> > +
> >  /**
> >
> >Initialize mSourcePldmTerminusId and mDestinationPldmTerminusId.
> > @@ -128,7 +129,6 @@ 

Re: [edk2-devel] [edk2-redfish-client][PATCH v2 0/11] RedfishClientPkg: various minor fixes

[Nickle Wang via groups.io]

Reviewed this patch series.

Reviewed-by: Nickle Wang 

Regards,
Nickle

> -Original Message-
> From: Mike Maslenkin 
> Sent: Friday, October 27, 2023 7:54 AM
> To: devel@edk2.groups.io
> Cc: abner.ch...@amd.com; Nickle Wang ;
> ig...@ami.com; Mike Maslenkin 
> Subject: [edk2-redfish-client][PATCH v2 0/11] RedfishClientPkg: various minor
> fixes
>
> External email: Use caution opening links or attachments
>
>
> This patchset contains fixes of wrong format and number of arguments passed to
> DEBUG macro.
>
> Also a number of memory leaks were fixed.
>
> Here is a link to PR:
> https://github.co/
> m%2Ftianocore%2Fedk2-redfish-
> client%2Fpull%2F52=05%7C01%7Cnicklew%40nvidia.com%7C0bdf0747ba5
> e409a66dd08dbd67ed2b5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0
> %7C638339612414397202%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
> =cXyazzKCZehvzfJ4ZeqpNRz53OA%2BobJ%2Bky3En74b7eY%3D
> =0
>
> Cc: Abner Chang 
> Cc: Nickle Wang 
> Cc: Igor Kulchytskyy 
> Signed-off-by: Mike Maslenkin 
> ---
> Changes in v2:
> added patches 10,11.
>



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

[edk2-devel] [edk2-platforms][PATCH] Features/IpmiFeaturePkg: Remove IpmiCommandLib from IpmiFeaturePkg

[Chang, Abner via groups.io]

From: Abner Chang 

Remove IpmiCommandLib from IpmiFeaturePkg as ManageabilityPkg
already had one and is newer.

Signed-off-by: Abner Chang 
Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Chasel Chiu 
Cc: Li-Xia Huang 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
---
 .../IpmiFeaturePkg/IpmiFeaturePkg.dec |   4 -
 .../IpmiFeaturePkg/Include/IpmiFeature.dsc|   2 +-
 .../Library/IpmiCommandLib/IpmiCommandLib.inf |  32 --
 .../IpmiCommandLib/IpmiCommandLibNetFnApp.c   | 335 --
 .../IpmiCommandLibNetFnChassis.c  | 144 
 .../IpmiCommandLibNetFnStorage.c  | 274 --
 .../IpmiCommandLibNetFnTransport.c| 123 ---
 7 files changed, 1 insertion(+), 913 deletions(-)
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLib.inf
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnApp.c
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnChassis.c
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnStorage.c
 delete mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnTransport.c

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
index 5df71300cb..f26741e244 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
@@ -32,10 +32,6 @@
   OsWdt
 
 [LibraryClasses]
-  ## @libraryclass  Provides services to send IPMI commands.
-  #
-  IpmiCommandLib|Include/Library/IpmiCommandLib.inf
-
   ## @libraryclass  Provides an API for platform-specific IPMI hooks.
   #
   IpmiCommandLib|Include/Library/IpmiPlatformHookLib.h
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
index 0401974b82..063638ce56 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc
@@ -37,7 +37,7 @@
 [LibraryClasses]
   IpmiLib|MdeModulePkg/Library/BaseIpmiLibNull/BaseIpmiLibNull.inf
 
-  IpmiCommandLib|IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLib.inf
+  IpmiCommandLib|ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLib.inf
   
IpmiPlatformHookLib|IpmiFeaturePkg/Library/IpmiPlatformHookLibNull/IpmiPlatformHookLibNull.inf
 
 [LibraryClasses.common.PEI_CORE,LibraryClasses.common.PEIM]
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLib.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLib.inf
deleted file mode 100644
index d5c14ff2a4..00
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLib.inf
+++ /dev/null
@@ -1,32 +0,0 @@
-### @file
-# Component description file for IPMI Command Library.
-#
-# Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-###
-
-[Defines]
-  INF_VERSION= 0x00010005
-  BASE_NAME  = IpmiCommandLib
-  FILE_GUID  = E599C9C7-5913-40A0-8669-67282E2BEC53
-  MODULE_TYPE= UEFI_DRIVER
-  VERSION_STRING = 1.0
-  LIBRARY_CLASS  = IpmiCommandLib
-
-[sources]
-  IpmiCommandLibNetFnApp.c
-  IpmiCommandLibNetFnTransport.c
-  IpmiCommandLibNetFnChassis.c
-  IpmiCommandLibNetFnStorage.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
-  IpmiFeaturePkg/IpmiFeaturePkg.dec
-
-[LibraryClasses]
-  BaseMemoryLib
-  DebugLib
-  IpmiBaseLib
diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnApp.c
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnApp.c
deleted file mode 100644
index 2e34909f3e..00
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiCommandLib/IpmiCommandLibNetFnApp.c
+++ /dev/null
@@ -1,335 +0,0 @@
-/** @file
-  IPMI Command - NetFnApp.
-
-  Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.
-  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-EFI_STATUS
-EFIAPI
-IpmiGetDeviceId (
-  OUT IPMI_GET_DEVICE_ID_RESPONSE  *DeviceId
-  )
-{
-  EFI_STATUS   Status;
-  UINT32   DataSize;
-
-  DataSize = sizeof(*DeviceId);
-  Status = IpmiSubmitCommand (
- IPMI_NETFN_APP,
- IPMI_APP_GET_DEVICE_ID,
-   

Re: [edk2-devel] [PATCH V1 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA

[sunceping]

On Saturday, October 28, 2023 12:45 AM, Erdem Aktas wrote:
This should be the [PATCH V1 2/2] I assume?
Yes, the name is same with [PATCH v1 0/2] , may be confusion, I would update in 
next version to avoid the same title name.


On Thu, Oct 26, 2023 at 5:58 PM sunceping 
mailto:cepingx@intel.com>> wrote:
 [Sources]
   VirtualMemory.h
   MemoryEncryption.c
+  X64/TdVmCallMapGPA.nasm
I do not think we need another TdVmCallMapGPA definition, do we?
Currently,  the TdVmCall always clears the R11 if the return code is not 
successful,  which means we need to change TdVmCall if we don't add 
TdVmCallMapGPA.
Refer the GHCI Spec , if the returns code is not successful,  the R11 value is 
not valid for the sub-functions except MapGPA,  which means  TdVmCall should 
clear the value on
unsuccessful returns and only save the value if MapGPA returns unsuccessfully.  
If an update is required, the logic in TdVmCall could be complex.

 [LibraryClasses]
   BaseLib
diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c 
b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
index b47f56b391a5..1f29f9194c30 100644
--- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
+++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
@@ -38,6 +38,10 @@ typedef enum {

 STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;

+#define TDVMCALL_STATUS_RETRY  0x1
+
+#define MAX_RETRIES_PER_PAGE  3
+
 /**
   This function is used to help request the host VMM to map a GPA range as
   private or shared-memory mappings.
@@ -546,6 +550,13 @@ SetOrClearSharedBit (
   EFI_STATUSStatus;
   EDKII_MEMORY_ACCEPT_PROTOCOL  *MemoryAcceptProtocol;

+  UINT64  MapGpaRetryaddr;
Should be replaced with MapGpaRetryAddr for consistency in variable name casing 
style ?
Yes, it would be updated in next version.

+  UINT32  RetryCount;
+  UINT64  EndAddress;
+
+  MapGpaRetryaddr = 0;
+  RetryCount  = 0;
+
   AddressEncMask = GetMemEncryptionAddressMask ();

   //
@@ -559,7 +570,30 @@ SetOrClearSharedBit (
 PhysicalAddress   &= ~AddressEncMask;
   }

-  TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
+  while (RetryCount < MAX_RETRIES_PER_PAGE) {
+TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, );
Why not this?
 TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, 
);
The TdVmCall always clears the R11 value when unsuccessful returns as above 
comments, therefor add the TdVmCallMapGPA to handle it.

+if (TdStatus != TDVMCALL_STATUS_RETRY) {
+  break;
+}
+
+DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is 
%llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
+
+EndAddress = PhysicalAddress + Length;
+if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > EndAddress)) 
{
 should be?
 if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >= EndAddress))
Yes, that’s right, it would be updated in next version.

Thanks
 Ceping



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

[Chang, Abner via groups.io]

Acked-by: Abner Chang 

Still need Abdul's RB.


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

[edk2-devel] [PATCH] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

[Jacque Lin via groups.io]

Remove checking SMM Rev ID in AMD Save State lib when reading Save
State Register EFI_MM_SAVE_STATE_REGISTER_IO.
For AMD, it is not necessary to check SmmRevId when reading Save State
Register EFI_MM_SAVE_STATE_REGISTER_IO.

Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Signed-off-by: Jacque Lin 
---
 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
index 3315a6cc44..c4bf6ad4bb 100644
--- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
+++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
@@ -102,7 +102,6 @@ MmSaveStateReadRegister (
   OUT VOID*Buffer

   )

 {

-  UINT32 SmmRevId;

   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;

   AMD_SMRAM_SAVE_STATE_MAP   *CpuSaveState;

   UINT8  DataWidth;

@@ -124,18 +123,6 @@ MmSaveStateReadRegister (
 

   // Check for special EFI_MM_SAVE_STATE_REGISTER_IO

   if (Register == EFI_MM_SAVE_STATE_REGISTER_IO) {

-//

-// Get SMM Revision ID

-//

-MmSaveStateReadRegisterByIndex (CpuIndex, 
AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );

-

-//

-// See if the CPU supports the IOMisc register in the save state

-//

-if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {

-  return EFI_NOT_FOUND;

-}

-

 // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.

 if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {

   return EFI_NOT_FOUND;

-- 
2.36.1.windows.1



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