Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

2023-11-19 Thread Gerd Hoffmann
On Fri, Nov 17, 2023 at 10:39:13PM +0100, Laszlo Ersek wrote:
> On 11/17/23 12:42, Gerd Hoffmann wrote:
> > On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
> >> (+Liming +Mike)
> >>
> >> On 11/16/23 10:01, Gerd Hoffmann wrote:
> >>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>  The struct used for GHCB-based page-state change requests uses a 40-bit
>  bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>  64-bit address. However, anything beyond 40-bits simply gets shifted off
>  when doing this, which will cause issues when dealing with 1TB+
>  addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>  prior to shifting it by PAGE_SHIFT.
> 
>  Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate 
>  system RAM")
>  Signed-off-by: Michael Roth 
> >>>
> >>> Reviewed-by: Gerd Hoffmann 
> >>>
> >>> take care,
> >>>   Gerd
> >>
> >> Is this hard feature freeze material?
> > 
> > It is a clear bugfix, so IMHO it qualifies.
> > 
> >> Also, the patch looks garbled to me on-list (superfluous line breaks).
> > 
> > Patch applies fine here.  I see mutt breaking the long line, but
> > that is just the local display rendering, the mail good.
> 
> Can you check the raw message? I did that and it seems broken.
> Superfluous newlines. I see *double* CRLFs.

Hmm, everything looks fine here, and 'git am' accepts the mail without
problems.  Pushed a branch:

https://github.com/kraxel/edk2/commits/b4/v2-20231115-michael-roth-ovmfpkg-memencryptsevlib-fix-address-overflow-during-pvalidate

take care,
  Gerd



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




[edk2-devel] [PATCH v2 1/1] OvmfPkg/Bhyve: use a proper PCI IO range

2023-11-19 Thread Corvin Köhne
Bhyve uses an io port range of [ 0x2000, 0x1 ] [1]. At the moment,
EDKII is using a subset of this range [ 0xC000, 0x1 ] [2]. Even
though the EDKII range doesn't exceed the bhyve range, it's causing
issues on some guests like OpenBSD [3]. We don't know why it's causing
issues yet. However, using the same IO port range in EDKII fixes the
issue and is a good idea anyway.

[1] 
https://github.com/freebsd/freebsd-src/blob/82ea0132c8b17a7a6067c8a36c6434e587ede6de/usr.sbin/bhyve/pci_emul.c#L133-L134
[2] 
https://github.com/tianocore/edk2/blob/fb044b7fe893a4545995bfe2701fd38e593355d9/OvmfPkg/Bhyve/PlatformPei/Platform.c#L156-L157
[3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=274389

Signed-off-by: Corvin Köhne 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Rebecca Cran 
Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Liming Gao 
Cc: Jiewen Yao 
---
 OvmfPkg/Bhyve/PlatformPei/Platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c 
b/OvmfPkg/Bhyve/PlatformPei/Platform.c
index f6d9a9038e12..bd1b22a9476e 100644
--- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
+++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
@@ -153,8 +153,8 @@ MemMapInitialization (
   UINT64 PciIoSize;
   RETURN_STATUS  PcdStatus;
 
-  PciIoBase = 0xC000;
-  PciIoSize = 0x4000;
+  PciIoBase = 0x2000;
+  PciIoSize = 0xE000;
 
   //
   // Create Memory Type Information HOB
-- 
2.42.0



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




Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: use a proper PCI IO range

2023-11-19 Thread Corvin Köhne
On Fri, 2023-11-17 at 17:09 +0100, Laszlo Ersek wrote:
> On 11/17/23 13:43, Corvin Köhne wrote:
> > Bhyve uses an io port range of [ 0x2000, 0x1 ] [1]. At the
> > moment,
> > EDKII is using a subset of this range [ 0xC000, 0x1 ] [2]. Even
> > though the EDKII range doesn't exeed the bhyve range, it's causing
> 
> s/exeed/exceed/
> 
> > issues on some guests like OpenBSD. We don't know why it's causing
> > issues yet. However, using the same IO port range in EDKII fixes
> > the
> > issue and is a good idea anyway.
> > 
> > [1]
> > https://github.com/freebsd/freebsd-src/blob/82ea0132c8b17a7a6067c8a36c6434e587ede6de/usr.sbin/bhyve/pci_emul.c#L133-L134
> > [2]
> > https://github.com/tianocore/edk2/blob/fb044b7fe893a4545995bfe2701fd38e593355d9/OvmfPkg/Bhyve/PlatformPei/Platform.c#L156-L157
> > 
> > Signed-off-by: Corvin Köhne 
> > Cc: Ard Biesheuvel 
> > Cc: Gerd Hoffmann 
> > Cc: Jiewen Yao 
> > Cc: Rebecca Cran 
> > ---
> >  OvmfPkg/Bhyve/PlatformPei/Platform.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > index f6d9a9038e12..bd1b22a9476e 100644
> > --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > @@ -153,8 +153,8 @@ MemMapInitialization (
> >    UINT64 PciIoSize;
> >    RETURN_STATUS  PcdStatus;
> >  
> > -  PciIoBase = 0xC000;
> > -  PciIoSize = 0x4000;
> > +  PciIoBase = 0x2000;
> > +  PciIoSize = 0xE000;
> >  
> >    //
> >    // Create Memory Type Information HOB
> 
> Reviewed-by: Laszlo Ersek 
> 
> Can you create a BZ for this issue? With that, I think this should be
> possible to merge during the hard feature freeze. Adding Liming.
> 

I've never created a BZ yet. I can't find a button to create a new
account at https://bugzilla.tianocore.org/.

> (For the typo fix in the commit message, either post v2, or ask
> Liming
> to fix it up upon merge.)
> 
> Thanks
> Laszlo
> 
> 
> 
> 
> 
> 

-- 
Kind regards,
Corvin


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




signature.asc
Description: This is a digitally signed message part


Re: [edk2-devel] [PATCH] CloudHv: Add CI for CloudHv on AArch64

2023-11-19 Thread Jianyong Wu
Thanks Laszlo. Will fix all comments next version.

Thanks
Jianyong

> -Original Message-
> From: Laszlo Ersek 
> Sent: 2023年11月18日 5:54
> To: devel@edk2.groups.io; Jianyong Wu ; Sami
> Mujawar 
> Cc: ardb+tianoc...@kernel.org
> Subject: Re: [edk2-devel] [PATCH] CloudHv: Add CI for CloudHv on AArch64
>
> On 11/15/23 07:20, Jianyong Wu wrote:
> > Add the long lost CI for CloudHv on AArch64.
> >
> > Signed-off-by: Jianyong Wu 
> > ---
> >  .../.azurepipelines/Ubuntu-GCC5.yml   | 13 
> >  ArmVirtPkg/PlatformCI/CloudHvBuild.py | 32
> +++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 ArmVirtPkg/PlatformCI/CloudHvBuild.py
>
> Please format and document patches such that they are easier to review.
> Otherwise reviewers have to dig too much.
>
> >
> > diff --git a/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > b/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > index d1772a65fc..ab8a2db530 100644
> > --- a/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > +++ b/ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
> > @@ -140,6 +140,19 @@ jobs:
> >  Build.Target: "RELEASE"
> >  Run: false
> >
> > +  CLOUDHV_AARCH64_DEBUG:
> > +Build.File: "$(package)/PlatformCI/CloudHvBuild.py"
> > +Build.Arch: "AARCH64"
> > +Build.Flags: ""
> > +Build.Target: "DEBUG"
> > +Run: false
> > +  CLOUDHV_AARCH64_RELEASE:
> > +Build.File: "$(package)/PlatformCI/CloudHvBuild.py"
> > +Build.Arch: "AARCH64"
> > +Build.Flags: ""
> > +Build.Target: "RELEASE"
> > +Run: false
> > +
> >  workspace:
> >clean: all
> >
>
> This seems like a copy of the KVMTOOL stanzas, except with (a) ARM removed,
> (b) KVMTOOL replaced by CLOUDHV, (c) KvmToolBuild.py replaced with
> CloudHvBuild.py.
>
> OK.
>
> > diff --git a/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > b/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > new file mode 100644
> > index 00..0192cd6577
> > --- /dev/null
> > +++ b/ArmVirtPkg/PlatformCI/CloudHvBuild.py
> > @@ -0,0 +1,32 @@
> > +# @file
> > +# Script to Build ArmVirtPkg UEFI firmware # # Copyright (c)
> > +Microsoft Corporation.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent ## import os import
> > +sys
> > +
> > +sys.path.append(os.path.dirname(os.path.abspath(__file__)))
> > +from PlatformBuildLib import SettingsManager from PlatformBuildLib
> > +import PlatformBuilder
> > +
> > +#
> 
> ### #
> > +#Common Configuration
> #
> > +#
> >
> +###
>  # class CommonPlatform():
> > +''' Common settings for this platform.  Define static data here and use
> > +for the different parts of stuart
> > +'''
> > +PackagesSupported = ("ArmVirtPkg",)
> > +ArchSupported = ("AARCH64", "ARM")
> > +TargetsSupported = ("DEBUG", "RELEASE")
> > +Scopes = ('armvirt', 'edk2-build')
> > +WorkspaceRoot = os.path.realpath(os.path.join(
> > +os.path.dirname(os.path.abspath(__file__)), "..", ".."))
> > +
> > +DscName = os.path.join("ArmVirtPkg", "ArmVirtCloudHv.dsc")
> > +FvQemuArg = "" # ignored
> > +
> > +import PlatformBuildLib
> > +PlatformBuildLib.CommonPlatform = CommonPlatform
>
> According to
>
>   git show --find-copies-harder
>
> this is a nearly identical copy of
> "ArmVirtPkg/PlatformCI/KvmToolBuild.py", the only difference is:
>
> -DscName = os.path.join("ArmVirtPkg", "ArmVirtKvmTool.dsc")
> +DscName = os.path.join("ArmVirtPkg", "ArmVirtCloudHv.dsc")
>
> It makes sense to me, but (a) this could have been documented in the commit
> message; (b) the patch could have been formatted with --find-copies-harder
> (and, indeed that option does not interfere with patch application, as long as
> the --base=master option is also given to git-format-patch -- then we know
> exactly where to check out a local branch for applying the patch, and to 
> rebase
> from.)
>
> Anyway:
>
> Reviewed-by: Laszlo Ersek 

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


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




Re: [edk2-devel] [PATCH V2 1/1] MdeModulePkg: Support customized FV Migration Information

2023-11-19 Thread Wang Fan
Hi Mike and Liming

Do you have time to take a look this update?

V3: https://edk2.groups.io/g/devel/message/110197
Pull Request: https://github.com/tianocore/edk2/pull/4970

Best Regards
Fan

-Original Message-
From: Wang, Fan 
Sent: Friday, October 27, 2023 4:24 PM
To: Kinney, Michael D ; devel@edk2.groups.io
Cc: Gao, Liming ; Jiang, Guomin 
; Bi, Dandan 
Subject: RE: [PATCH V2 1/1] MdeModulePkg: Support customized FV Migration 
Information

Hi Mike

Thank you for your feedback.

I have updated the patch to v3: https://edk2.groups.io/g/devel/message/110197

Pull Request: https://github.com/tianocore/edk2/pull/4970

Based on V2, this update includes changes:
- Add more descriptions for "gEdkiiToMigrateFvInfoGuid" PPI usages and 
background, but still keep the name.
- Update "FvOrgBase" to "FvTemporaryRamBase" in EDKII_TO_MIGRATE_FV_INFO.
- Remove flag "FLAGS_SKIP_FV_MIGRATION", since it's not needed.
- Add more descriptions to flag "FLAGS_SKIP_FV_RAW_DATA_COPY".
- Remove the variable MigrateAllFvs to simplify logic.
- Correct "allocate pool" to "allocate pages" to align with descriptions.

For other two questions you are mentioning:

1. For "Should FvOrgBase be a UINT64 or support temp RAM above 4GB?":
I think UINT32 should be enough, all pre-mem FVs are below 4G as far as I know, 
even we enabled X64 in pre-mem phase. This setting is also aligning with 
"FvOrgBase" defined in "EDKII_MIGRATED_FV_INFO".
2. For "The call to RemoveFvHobsInTemporaryMemory (Private) was removed":
Since this function will remove all FV hobs for physical addresses, it should 
be called only when all pre-mem FVs are migrated. In EvacuateTempRam(), when 
one FV is migrated, ConvertFvHob() will be called for this FV and all its' 
child FVs, this is enough to ensure already migrated FV hobs are all pointing 
to addresses on permanent address. 

What do you think?

Best Regards
Fan

-Original Message-
From: Kinney, Michael D 
Sent: Tuesday, October 17, 2023 5:00 AM
To: Wang, Fan ; devel@edk2.groups.io
Cc: Gao, Liming ; Jiang, Guomin 
; Bi, Dandan ; Kinney, Michael D 

Subject: RE: [PATCH V2 1/1] MdeModulePkg: Support customized FV Migration 
Information

Hi Fan,

The logic looks functional, but there are some names and descriptions that 
could be added to help describe this feature and some code changes to make the 
code easier to understand.

1) The GUID gEdkiiToMigrateFvInfoGuid is hard to understand what
   information it conveys from the name of the GUID variable.
   I know that the intent is that it is a GUID with data that 
   tells the PEI core which FVs in temporary RAM should be 
   migrated to permanent RAM.  And that the use of this information
   is only a performance optimization to not migrate FVs that 
   are not needed after the PEI Core migrates temp RAM to 
   permanent RAM.  The name and description in comments do not
   express all these details.

2) The structure name EDKII_TO_MIGRATE_FV_INFO has the same
   issue as (1).
   a) Should FvOrgBase be a UINT64 or support temp RAM above 4GB?
   b) Since only temp RAM to perm RAM migrations are supported
  the field name "FvOrgBase" should be "FvTemporaryRamBase".


3) The #define for FLAGS_SKIP_FV_MIGRATION should have a comment
   block that describes the meaning of this bit.  What is the 
   meaning when the bit is set and what is the meaning when the
   bit is clear.

4) The #define for FLAGS_SKIP_FV_RAW_DATA_COPY should have a comment
   block that describes the meaning of this bit.  What is the 
   meaning when the bit is set and what is the meaning when the
   bit is clear.

5) Why are there 2 bits?  If an FV is skipped, then that means
   do not copy.  If an FV is not skipped, then that means do
   copy.

6) I think the variable MigrateAllFvs can be removed, and the HOB
   list check can be made for gEdkiiToMigrateFvInfoGuid can be made
   on each FV found in temporary RAM.  This looks like a performance
   optimization that makes the logic harder to understand and it
   is not clear that the performance optimization has any benefit.

7) The call to RemoveFvHobsInTemporaryMemory (Private) was removed.  Why?

8) The comment where memory is allocated for the migrated FV says
   "allocate pool" when PeiServicesAllocatePages() is used.  Please 
   update the comment.

Mike



> -Original Message-
> From: Wang, Fan 
> Sent: Monday, September 11, 2023 2:52 AM
> To: devel@edk2.groups.io
> Cc: Wang, Fan ; Kinney, Michael D 
> ; Gao, Liming ; 
> Jiang, Guomin ; Bi, Dandan 
> 
> Subject: [PATCH V2 1/1] MdeModulePkg: Support customized FV Migration 
> Information
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4533
> 
> There are use cases which not all FVs need be migrated from TempRam to 
> permanent memory before TempRam tears down. This new guid is 
> introduced to avoid unnecessary FV migration to improve boot 
> performance.
> Platform
> can publish ToMigrateFvInfo hob with this guid to customize FV 
> migration info, and PeiCore

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

2023-11-19 Thread Dhaval Sharma
Did you mean to add it as a memory type by itself like
EFI_MEMORY_TYPE_INFORMATION?
My interpretation of Memory Type is that it is more of SW usability
construct while Memory Attr is more of HW behavioural construct. Together
they define how a memory region can be used.

On Fri, Nov 17, 2023 at 1:55 PM Laszlo Ersek  wrote:

> On 11/17/23 09:07, Dhaval Sharma wrote:
> > Hi,
> > I wanted to revisit this thread and I am maintaining the context as
> > there are a lot of details already mentioned here
> regarding EFI_MEMORY_SP.
> > Other than what has been addressed here, we also would like to have an
> > option in edk2 to *avoid* using this type of memory for its own
> > purposes. This seems like one of the motivations for original request
> > and is being honored by OS today but not edk2 as it does not have any
> > specific implementation today which takes this attribute into
> consideration.
> > I would like to add PCD based implementation which informs edk2 NOT to
> > use this memory for its own purposes and leave it alone (as still
> > available memory to OS).
> >
> > Specific-purpose memory (SPM). The memory is earmarked for
> > specific purposes such as for specific device drivers or applications.
> > The SPM attribute serves as a hint to the OS to avoid allocating this
> > memory for core OS data or code that can not be relocated.
> > Prolonged use of this memory for purposes other than the intended
> > purpose may result in suboptimal platform performance.
> >
> > Some more context:
> > https://lwn.net/Articles/784971/
>
> Why was EFI_MEMORY_SP introduced as a memory attribute, rather than its
> own memory type?
>
> Laszlo
>
>

-- 
Thanks!
=D


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




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

2023-11-19 Thread Yuanhao Xie
Hi Ray and Laszlo,

Thanks a lot for the feedbacks.

Please have a review on the patch v6 which:

-Kept execute disable bit both in MpExchangeInfo and CpuMpData.
-Added another patch in which I updated the comments of CpuMpData.

Regards
Yuanhao Xie

-Original Message-
From: Laszlo Ersek  
Sent: Friday, November 17, 2023 4:38 PM
To: devel@edk2.groups.io; Ni, Ray ; Xie, Yuanhao 

Cc: Dong, Eric ; Kumar, Rahul R ; 
Gerd Hoffmann 
Subject: Re: [edk2-devel] [Patch V4] UefiCpuPkg/MpInitLib: Enable execute 
disable bit.

On 11/17/23 08:01, Ni, Ray wrote:
> Laszlo,
> I agree that the BSP's XD status is saved in both CpuMpData and 
> ExchangeInfo.
> But, thinking from another perspective, ExchangeInfo is "only" used by 
> the assembly code. That's why the BSP code needs to save the XD status 
> in CpuMpData and ExchangeInfo.
> 
> If we remove the XD status field in ExchangeInfo, then the assembly 
> code has to understand the structure layout of CpuMpData, which is 
> what I prefer to avoid.
> 
> If you compare all fields in ExchangeInfo and CpuMpData, following 
> fields are already duplicated:
> * CpuMpData.CpuInfoHob <-> MpExchangeInfo.CpuInfo
> * InitFlag
> * SevEsIsEnabled
> * SevSnpIsEnabled
> * GhcbBase
> 
> 
> So, I prefer to keep the current change proposed in Yuanhao's patch.

Very good explanation, thank you.

Can we perhaps document, in an additional patch:

- in "UefiCpuPkg/Library/MpInitLib/MpEqu.inc", that the assembly routines are 
not supposed to access the internals of CPU_MP_DATA,

- the same statement above "struct _CPU_MP_DATA" in 
"UefiCpuPkg/Library/MpInitLib/MpLib.h"?

A number of structures in "UefiCpuPkg/Library/MpInitLib/MpLib.h"
document whether they are to be used by assembly code vs. C code (vs.
both), but CPU_MP_DATA doesn't seem to have such comments.

For the current patch:

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo



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




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

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

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

Signed-off-by: Yuanhao Xie 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index af296f6ac0..924677d823 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -239,7 +239,11 @@ typedef struct {
 #pragma pack()
 
 //
-// CPU MP Data save in memory
+// CPU MP Data save in memory, and intended for use in C code.
+// There are some duplicated fields, such as XD status, between
+// CpuMpData and ExchangeInfo. These duplications in CpuMpData
+// are present to avoid to be direct accessed and comprehended
+// in assembly code.
 //
 struct _CPU_MP_DATA {
   UINT64   CpuInfoInHob;
-- 
2.39.1.windows.1



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




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

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

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

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

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

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

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

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



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




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

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

It also fixed the build error in V5.

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

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

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  7 ++-
 2 files changed, 17 insertions(+), 4 deletions(-)

--
2.39.1.windows.1



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




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

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

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

Thnaks
Abner

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



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




[edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

2023-11-19 Thread Dhaval Sharma
As per ACPI Spec 6.5+ Table 5-9 if xDSDT is avaialble,
it should be used first. Handle required flow when xDSDT
is abscent or present.

Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
linux kernel.

Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Dandan Bi 
Signed-off-by: Dhaval Sharma 
---

Notes:
v2:
- Added proper indentation for else if

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22 
+---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c 
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e09bc9b704f5..ead8376177c9 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1892,14 +1892,22 @@ InstallAcpiTableFromHob (
   }
 }
 
-if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 
0) {
+//
+// First check if xDSDT is available that is preferred as per
+// ACPI Spec 6.5+ Table 5-9 X_DSDT definition
+//
+if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt 
!= 0) {
+  TableToInstall = (VOID 
*)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt;
+} else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE 
*)ChildTable)->Dsdt != 0) {
   TableToInstall = (VOID 
*)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt;
-  Status = AddTableToList (AcpiTableInstance, TableToInstall, 
TRUE, Version, TRUE, &TableKey);
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI 
table DSDT\n"));
-ASSERT_EFI_ERROR (Status);
-break;
-  }
+} else {
+  break;
+}
+Status = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, 
Version, TRUE, &TableKey);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI 
table DSDT\n"));
+  ASSERT_EFI_ERROR (Status);
+  break;
 }
   }
 }
-- 
2.39.2



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




[edk2-devel] [PATCH v3 0/1] Add support for XDSDT table

2023-11-19 Thread Dhaval Sharma
Enable detection of XDSDT table from ACPI HOB and use it to comply
with ACPI spec 6.5+ Table 5-9.

Dhaval (1):
  MdeModulePkg: Fix issue with ACPI table creation

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22 
+---
 1 file changed, 15 insertions(+), 7 deletions(-)

-- 
2.39.2



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




[edk2-devel] [PATCH v3 1/1] MdeModulePkg: Fix issue with ACPI table creation

2023-11-19 Thread Dhaval Sharma
As per ACPI Spec 6.5+ Table 5-9 if xDSDT is avaialble,
it should be used first. Handle required flow when xDSDT
is abscent or present.

Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
linux kernel.

Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Dandan Bi 
Signed-off-by: Dhaval Sharma 
---

Notes:
v2:
- Added proper indentation for else if

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22 
+---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c 
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e09bc9b704f5..ead8376177c9 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1892,14 +1892,22 @@ InstallAcpiTableFromHob (
   }
 }
 
-if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt != 
0) {
+//
+// First check if xDSDT is available that is preferred as per
+// ACPI Spec 6.5+ Table 5-9 X_DSDT definition
+//
+if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt 
!= 0) {
+  TableToInstall = (VOID 
*)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->XDsdt;
+} else if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE 
*)ChildTable)->Dsdt != 0) {
   TableToInstall = (VOID 
*)(UINTN)((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)ChildTable)->Dsdt;
-  Status = AddTableToList (AcpiTableInstance, TableToInstall, 
TRUE, Version, TRUE, &TableKey);
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI 
table DSDT\n"));
-ASSERT_EFI_ERROR (Status);
-break;
-  }
+} else {
+  break;
+}
+Status = AddTableToList (AcpiTableInstance, TableToInstall, TRUE, 
Version, TRUE, &TableKey);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "InstallAcpiTableFromHob: Fail to add ACPI 
table DSDT\n"));
+  ASSERT_EFI_ERROR (Status);
+  break;
 }
   }
 }
-- 
2.39.2



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




[edk2-devel] [PATCH v3 0/1] Add support for XDSDT table

2023-11-19 Thread Dhaval Sharma
Enable detection of XDSDT table from ACPI HOB and use it to comply
with ACPI spec 6.5+ Table 5-9.

Dhaval (1):
  MdeModulePkg: Fix issue with ACPI table creation

 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 22 
+---
 1 file changed, 15 insertions(+), 7 deletions(-)

-- 
2.39.2



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




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

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

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

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

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  7 ++-
 2 files changed, 17 insertions(+), 4 deletions(-)

--
2.39.1.windows.1



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




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

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

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

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

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

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

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

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



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




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

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

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

Signed-off-by: Yuanhao Xie 
Cc: Laszlo Ersek ler...@redhat.com
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 0afac7c78c..242935816c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -239,7 +239,11 @@ typedef struct {
 #pragma pack()
 
 //
-// CPU MP Data save in memory
+// CPU MP Data save in memory, and intended for use in C code.
+// There are some duplicated fields, such as XD status, between
+// CpuMpData and ExchangeInfo. These duplications in CpuMpData
+// are present to avoid to be direct accessed and comprehended
+// in assembly code.
 //
 struct _CPU_MP_DATA {
   UINT64   CpuInfoInHob;
-- 
2.39.1.windows.1



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




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

2023-11-19 Thread Ranbir Singh
On Wed, Nov 15, 2023 at 3:20 PM Laszlo Ersek  wrote:

> On 11/14/23 17:21, Kinney, Michael D wrote:
> > Hi Ranbir,
> >
> >
> >
> > First I want to recognize your efforts to collect Coverity issues and
> > propose changes to address
> > them.
> >
> > I still disagree with adding CpuDealLoop() for any static analysis
> issues.
> >
> > There have been previous discussions about adding a PANIC() or FATAL()
> > macro that would
> > perform platform specific actions if a condition is detected where the
> > boot of the platform
> > can not continue.  A platform get to make the choice to log or reboot or
> > hang, etc.  Not the
> > code that detected the condition.
> >
> > https://edk2.groups.io/g/devel/message/86597
> > 
>
> This is indeed a great idea.
>
> I didn't know about that discussion. Perhaps a new DebugLib API would
> handle this (i.e., "panic").
>
> I've been certainly proposing CpuDeadLoop() as a means to panic.
>
> Did the thread conclude anything tangible?
>
> > Unfortunately, in order to fix some of these static analysis issues
> > correctly, we are going
> > to have to identify the use of ASSERT() that really is a fatal condition
> > that can not continue
>
> Absolutely.
>
> > and introduce an implementation approach that provides a platform
> > handler and
> > satisfies the static analysis tools.
>
> The "platform handler" is the difficult part. If the above-noted
> discussion from 2022 didn't produce an agreement, should we really block
> the static analyzer fixes on an agreed-upon panic API? I'm concerned
> that would just cause these fixes to get stuck. And I don't consider
> CpuDeadLoop()s added for this purpose serious technical debt. They are
> easy to find and update later, assuming we also add comments.
>
>
I agree with the approach to not gate current fixes adding CpuDeadLoop().
Later on, it can be updated with the desired panic API and I can contribute
for those required changes related to patches submitted by me.

I can update current patches to carry additional comment in suffix form to
ease later search like
CpuDeadLoop (); // TBD: replace with Panic API in future

Laszlo, Mike - Let me know if that works for now.


> > We also have to evaluate if a return error status with a DEBUG_ERROR
> > message would be a better
> > choice than an ASSERT() that can be filtered out by build options.
>
> I agree 100% that this would be better for the codebase, but the work
> needed for this is (IMO) impossible to contain. ASSERT() has been abused
> for a long time *because* it seemed to allow the programmer to ignore
> any related error paths. If we now want to implement those error paths
> retroactively (which would be absolutely the right thing to do from a
> software engineering perspective), then immense amounts of work are
> going to be needed (patch review and regression testing), and I think
> it's just not practical to dump all that on someone that wants to
> improve the status quo. Replacing an invalid ASSERT() with a panic is
> honest about the current situation, is safer regarding RELEASE builds,
> and its work demand (regression testing, patch review) is tolerable.
>
> I do agree that, if the error path mostly exists already, then returning
> errors for data/environment-based error conditions (i.e., not for
> algorithmic invariant failures) is best.
>
> Where we need to be extremely vigilant is new patches. We must
> uncompromisingly reject *new* abuses of ASSERT(), in new patches.
>
> Anyway, it seems that we've been trying to steer Ranbir in opposite
> directions. I'll let you take the lead on this; for one, I've not been
> aware of the panic api discussion for 2022!
>
> (I don't feel especially pushy about fixing coverity issues, it's just
> that Ranbir has been contributing such patches, which I've found very
> welcome, and I wanted to help out with reviews.)
>
> Laszlo
>
>


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




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

2023-11-19 Thread Yuanhao Xie
The microcode loading during Mp initialization of the DXE stage can be
removed regardless of bit mode.

Cc: Ray Ni ray...@intel.com
Cc: Eric Dong eric.d...@intel.com
Cc: Rahul Kumar rahul1.ku...@intel.com
Cc: Tom Lendacky thomas.lenda...@amd.com
Cc: Laszlo Ersek ler...@redhat.com
Signed-off-by: Yuanhao Xie yuanhao@intel.com
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 57 +++-
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..1c68c803d9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -433,6 +433,26 @@ ApFuncEnableX2Apic (
   SetApicMode (LOCAL_APIC_MODE_X2APIC);
 }
 
+/**
+  Sync BSP's MTRR table to AP during waking upAp
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+ApMtrrSync (
+  IN OUT VOID  *Buffer
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+
+  CpuMpData = (CPU_MP_DATA *)Buffer;
+
+  //
+  // Sync BSP's MTRR table to AP
+  //
+  MtrrSetAllMtrrs (&CpuMpData->MtrrTable);
+}
+
 /**
   Do sync on APs.
 
@@ -2162,6 +2182,23 @@ MpInitLibInitialize (
   //
   CollectProcessorCount (CpuMpData);
 }
+
+if (!GetMicrocodePatchInfoFromHob (
+   &CpuMpData->MicrocodePatchAddress,
+   &CpuMpData->MicrocodePatchRegionSize
+   ))
+{
+  //
+  // The microcode patch information cache HOB does not exist, which means
+  // the microcode patches data has not been loaded into memory yet
+  //
+  ShadowMicrocodeUpdatePatch (CpuMpData);
+}
+
+//
+// Detect and apply Microcode on BSP
+//
+MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   } else {
 //
 // APs have been wakeup before, just get the CPU Information
@@ -2224,22 +2261,6 @@ MpInitLibInitialize (
 }
   }
 
-  if (!GetMicrocodePatchInfoFromHob (
- &CpuMpData->MicrocodePatchAddress,
- &CpuMpData->MicrocodePatchRegionSize
- ))
-  {
-//
-// The microcode patch information cache HOB does not exist, which means
-// the microcode patches data has not been loaded into memory yet
-//
-ShadowMicrocodeUpdatePatch (CpuMpData);
-  }
-
-  //
-  // Detect and apply Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   //
   // Store BSP's MTRR setting
   //
@@ -2256,9 +2277,11 @@ MpInitLibInitialize (
   // in DXE.
   //
   CpuMpData->InitFlag = ApInitReconfig;
+  WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
+} else {
+  WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 }
 
-WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 //
 // Wait for all APs finished initialization
 //
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111443): https://edk2.groups.io/g/devel/message/111443
Mute This Topic: https://groups.io/mt/102701619/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 13/39] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg

2023-11-19 Thread Chao Li

Hi Anderi,

There are indeed typos and I will fix them in V4. Thank you!


Thanks,
Chao
On 2023/11/18 04:18, Andrei Warkentin wrote:

+Tuan as a heads-up.

This seems reasonable to me. What does the "Uint" mean in 
ConfigureMemoryManagementUint?

Did you intend to say "Unit"?

Reviewed-by: Andrei Warkentin


-Original Message-
From: Chao Li
Sent: Friday, November 17, 2023 4:00 AM
To:devel@edk2.groups.io
Cc: Dong, Eric; Ni, Ray; Kumar,
Rahul R; Gerd Hoffmann;
Leif Lindholm; Ard Biesheuvel
; Sami Mujawar;
Sunil V L; Warkentin, Andrei

Subject: [PATCH v3 13/39] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg

Add a new header file CpuMmuLib.h, whitch is referenced from
ArmPkg/Include/Library/ArmMmuLib.h. Currently, only support for
LoongArch64 is added, and more architectures can be accommodated in the
future.

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

Cc: Eric Dong
Cc: Ray Ni
Cc: Rahul Kumar
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
  UefiCpuPkg/Include/Library/CpuMmuLib.h | 155
+
  UefiCpuPkg/UefiCpuPkg.dec  |   4 +
  2 files changed, 159 insertions(+)
  create mode 100644 UefiCpuPkg/Include/Library/CpuMmuLib.h

diff --git a/UefiCpuPkg/Include/Library/CpuMmuLib.h
b/UefiCpuPkg/Include/Library/CpuMmuLib.h
new file mode 100644
index 00..23b2fe34ac
--- /dev/null
+++ b/UefiCpuPkg/Include/Library/CpuMmuLib.h
@@ -0,0 +1,155 @@
+/** @file
+
+  Copyright (c) 2023 Loongson Technology Corporation Limited. All
+ rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CPU_MMU_LIB_H_
+#define CPU_MMU_LIB_H_
+
+#include 
+
+#define EFI_MEMORY_CACHETYPE_MASK  (EFI_MEMORY_UC  | \
+EFI_MEMORY_WC  | \
+EFI_MEMORY_WT  | \
+EFI_MEMORY_WB  | \
+EFI_MEMORY_UCE   \
+)
+
+typedef struct {
+  EFI_PHYSICAL_ADDRESSPhysicalBase;
+  EFI_VIRTUAL_ADDRESS VirtualBase;
+  UINTN   Length;
+  UINTN   Attributes;
+} MEMORY_REGION_DESCRIPTOR;
+
+/**
+  Converts EFI Attributes to corresponding architecture Attributes.
+
+  @param[in]  EfiAttributes Efi Attributes.
+
+  @retval  Corresponding architecture attributes.
+**/
+UINTN
+EfiAttributeConverse (
+  IN UINTN  EfiAttributes
+  );
+
+/**
+  Finds the length and memory properties of the memory region
corresponding to the specified base address.
+
+  @param[in]  BaseAddressTo find the base address of the memory
region.
+  @param[in]  EndAddress To find the end address of the memory region.
+  @param[out]  RegionLengthThe length of the memory region found.
+  @param[out]  RegionAttributesProperties of the memory region found.
+
+  @retval  EFI_SUCCESSThe corresponding memory area was successfully
found
+   EFI_NOT_FOUNDNo memory area found
+**/
+EFI_STATUS
+GetMemoryRegionAttribute (
+  IN UINTN  BaseAddress,
+  IN UINTN  EndAddress,
+  OUTUINTN  *RegionLength,
+  OUTUINTN  *RegionAttributes
+  );
+
+/**
+  Sets the Attributes  of the specified memory region
+
+  @param[in]  BaseAddressThe base address of the memory region to set
the Attributes.
+  @param[in]  Length The length of the memory region to set the
Attributes.
+  @param[in]  Attributes The Attributes to be set.
+  @param[in]  AttributeMask  Mask of memory attributes to take into
account.
+
+  @retval  EFI_SUCCESSThe Attributes was set successfully
+**/
+EFI_STATUS
+SetMemoryRegionAttributes (
+  IN EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN UINTN Length,
+  IN UINTN Attributes,
+  IN UINT64AttributeMask
+  );
+
+/**
+  Sets the non-executable Attributes for the specified memory region
+
+  @param[in]  BaseAddress  The base address of the memory region to set
the Attributes.
+  @param[in]  Length   The length of the memory region to set the
Attributes.
+
+  @retval  EFI_SUCCESSThe Attributes was set successfully
+**/
+EFI_STATUS
+SetMemoryRegionNoExec (
+  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINTN Length
+  );
+
+/**
+  Clears the non-executable Attributes for the specified memory region
+
+  @param[in]  BaseAddress  The base address of the memory region to clear
the Attributes.
+  @param[in]  Length   The length of the memory region to clear the
Attributes.
+
+  @retval  EFI_SUCCESSThe Attributes was clear successfully
+**/
+EFI_STATUS
+EFIAPI
+ClearMemoryRegionNoExec (
+  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length
+  );
+
+/**
+  Sets the read-only Attributes for the specified memory region
+
+  @param[in]  BaseAddress  The base address of the memory region to set
the Attributes.
+  @param[in]  Length   The length of the memory region to set the
A

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

2023-11-19 Thread Chao Li

Hi Leif,

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



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

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

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

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

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

/
 Leif


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

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

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 6dd91e6941..7af25a91a1 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -143,7 +143,6 @@
  
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
  
-  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf

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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Use NonSmm BSP as BSP if BSP election is not enabled.

2023-11-19 Thread Zhiguang Liu
Thanks Laszlo for the comments.
To avoid using MpService Protocol, and also for S3 boot flow, the newer version 
of patch chooses to use global variable gSmmCpuPrivate instead of WhoAmI.
Please help review [PATCH v2] UefiCpuPkg/PiSmmCpuDxeSmm: Use NonSmm BSP as 
default SMM BSP.

Thanks
Zhiguang

> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, November 15, 2023 12:54 AM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Ni, Ray ; Kumar, Rahul R ;
> Gerd Hoffmann 
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Use NonSmm BSP as BSP if
> BSP election is not enabled.
> 
> Small patch, but I have several comments :)
> 
> On 11/14/23 03:08, Zhiguang Liu wrote:
> > Currently, if BSP election is not enabled, will use Core0 as SMM BSP,
> > however, Core0 does not always have the highest performance core.
> > So, we can used NonSmm BSP as default BSP.
> 
> (1) Please consider mentioning in the commit message that the change from
> this patch will take effect in SmiRendezvous().
> 
> (2) Please put "UefiCpuPkg/PiSmmCpuDxeSmm: ..." in the subject.
> 
> >
> > Cc: Ray Ni 
> > Cc: Rahul Kumar 
> > Cc: Gerd Hoffmann 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 10 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 48
> > +++---
> >  2 files changed, 34 insertions(+), 24 deletions(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index 25d058c5b9..a4f83bb122 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -29,6 +29,8 @@ MM_COMPLETION
> mSmmStartupThisApToken;
> >  //
> >  UINT32  *mPackageFirstThreadIndex = NULL;
> >
> > +extern EFI_MP_SERVICES_PROTOCOL  *mMpServices;
> > +
> >  /**
> >Performs an atomic compare exchange operation to get semaphore.
> >The compare exchange operation must be performed using @@ -1953,6
> > +1955,14 @@ InitializeMpSyncData (
> >// Enable BSP election by setting BspIndex to -1
> >//
> >mSmmMpSyncData->BspIndex = (UINT32)-1;
> > +} else {
> > +  //
> > +  // Use NonSMM BSP as SMM BSP
> > +  //
> > +  ASSERT (mMpServices != NULL);
> > +  if (mMpServices != NULL) {
> > +mMpServices->WhoAmI (mMpServices, (UINTN
> *)&mSmmMpSyncData->BspIndex);
> > +  }
> >  }
> >
> >  mSmmMpSyncData->EffectiveSyncMode = mCpuSmmSyncMode;
> 
> 
> (3) This branch cannot be tested on OVMF, due to commit 43df61878d94
> ("OvmfPkg: enable SMM Monarch Election in PiSmmCpuDxeSmm", 2020-03-
> 04).
> 
> That's not a problem with the patch of course, just saying that I can't offer
> regression testing.
> 
> 
> (4) Not checking the return value of WhoAmI() is actually valid here.
> Per PI spec, WhoAmI() can only fail if we pass a null pointer for
> "ProcessorNumber" (and we don't do that here).
> 
> Not sure about static analysis tools though. If they complain, we might want
> to cast the return value to (VOID) explicitly:
> 
>   (VOID)mMpServices->WhoAmI (...);
> 
> Not needed unless those tools complain, of course.
> 
> 
> (5) Passing the following argument for the "ProcessorNumber" parameter
> 
>   (UINTN *)&mSmmMpSyncData->BspIndex
> 
> is undefined behavior, for two reasons.
> 
> First, "SMM_DISPATCHER_MP_SYNC_DATA.BspIndex" is volatile-qualified, but
> the access here (or more precisely, inside WhoAmI()) happens through an
> lvalue that doesn't necessarily have volatile-qualified type. That's UB.
> 
> Second, "SMM_DISPATCHER_MP_SYNC_DATA.BspIndex" has type "(volatile)
> UINT32", but WhoAmI() writes an UINTN. On IA32, you're going to corrupt
> memory.
> 
> Therefore you should to do this:
> 
>   UINTN DxeBspNumber;
> 
>   MpServices->WhoAmI (mMpServices, &DxeBspNumber);
>   if (DxeBspNumber <= MAX_UINT32) {
> mSmmMpSyncData->BspIndex = (UINT32)DxeBspNumber;
>   }
> 
> In other words, use a local variable of the right type, and range check it. 
> If the
> range check fails, then just fall back to the current behavior (leave 
> BspIndex at
> zero). Otherwise, employ an explicit assignment, including casting. This will 
> in
> particular keep "volatile"
> happy (and there can't be any overflow either, after the range check).
> 
> One more comment below:
> 
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > index 1d022a7051..18c77c59e6 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > @@ -128,7 +128,8 @@ SPIN_LOCK  *mConfigSmmCodeAccessCheckLock =
> NULL;
> > EFI_SMRAM_DESCRIPTOR  *mSmmCpuSmramRanges;
> >  UINTN mSmmCpuSmramRangeCount;
> >
> > -UINT8  mPhysicalAddressBits;
> > +UINT8 mPhysicalAddressBits;
> > +EFI_MP_SERVICES_PROTOCOL  *mMpServices;
> >
> >  //
> >  // Control register contents saved for SMM S3 resume state initialization.
> > @@ -603,26 +604,25 @@ PiCpuSmmEntry (

[edk2-devel] [PATCH v2] UefiCpuPkg/PiSmmCpuDxeSmm: Use NonSmm BSP as default SMM BSP.

2023-11-19 Thread Zhiguang Liu
Currently, if BSP election is not enabled, will use Core0 as SMM BSP.
However, Core0 does not always have the highest performance.
So, we can used NonSmm BSP as default BSP.
This will take effect in normal SMM init flow and S3 boot flow

Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Laszlo Ersek 
Signed-off-by: Zhiguang Liu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 25d058c5b9..b279f5dfcc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1953,6 +1953,16 @@ InitializeMpSyncData (
   // Enable BSP election by setting BspIndex to -1
   //
   mSmmMpSyncData->BspIndex = (UINT32)-1;
+} else {
+  //
+  // Use NonSMM BSP as SMM BSP
+  //
+  for (CpuIndex = 0; CpuIndex < 
gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; CpuIndex++) {
+if (GetApicId () == 
gSmmCpuPrivate->ProcessorInfo[CpuIndex].ProcessorId) {
+  mSmmMpSyncData->BspIndex = (UINT32)CpuIndex;
+  break;
+}
+  }
 }
 
 mSmmMpSyncData->EffectiveSyncMode = mCpuSmmSyncMode;
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111439): https://edk2.groups.io/g/devel/message/111439
Mute This Topic: https://groups.io/mt/102701170/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 09/39] MdePkg: Add a new library named PeiServicesTablePointerLibReg

2023-11-19 Thread Chao Li

Hi Leif,

There are indeed typos and I will fix them in V4. Thank you!


Thanks,
Chao
On 2023/11/17 19:35, Leif Lindholm wrote:

Not my package, just spotted a typo below:

On Fri, Nov 17, 2023 at 17:59:49 +0800, Chao Li wrote:

Since some ARCH or platform not require execute code on memory during
PEI phase, some values may transferred via CPU registers.

Adding PeiServcieTablePointerLibReg to allow set and get the PEI service
table pointer depend by a CPU register, this library can accommodate lot
of platforms who not require execte code on memory during PEI phase.

Adding PeiServiceTablePointerLibReg to allows setting and getting the
PEI service table pointer via CPU registers, and the library can
accommodate many platforms that do not need to execute code on memory
during the PEI phase.

The idea of this library is derived from
ArmPkg/Library/PeiServicesTablePointerLib/

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

Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
Cc: Laszlo Ersek
Cc: Sunil V L
Signed-off-by: Chao Li
---
  .../Library/PeiServicesTablePointerLib.h  | 37 +++-
  .../PeiServicesTablePointer.c | 86 +++
  .../PeiServicesTablePointerLib.uni| 20 +
  .../PeiServicesTablePointerLibReg.inf | 40 +
  MdePkg/MdePkg.dsc |  1 +
  5 files changed, 180 insertions(+), 4 deletions(-)
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLib.uni
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLibReg.inf

diff --git a/MdePkg/Include/Library/PeiServicesTablePointerLib.h 
b/MdePkg/Include/Library/PeiServicesTablePointerLib.h
index 61635eff00..f5c764cb13 100644
--- a/MdePkg/Include/Library/PeiServicesTablePointerLib.h
+++ b/MdePkg/Include/Library/PeiServicesTablePointerLib.h
@@ -52,10 +52,11 @@ SetPeiServicesTablePointer (
immediately preceding the Interrupt Descriptor Table (IDT) in memory.
For X64 CPUs, the PEI Services Table pointer is stored in the 8 bytes
immediately preceding the Interrupt Descriptor Table (IDT) in memory.
-  For Itanium and ARM CPUs, a the PEI Services Table Pointer is stored in
-  a dedicated CPU register.  This means that there is no memory storage
-  associated with storing the PEI Services Table pointer, so no additional
-  migration actions are required for Itanium or ARM CPUs.
+  For Itanium, ARM and LoongArch CPUs, a the PEI Services Table Pointer
+  is stored in a dedicated CPU register.  This means that there is no
+  memory storage associated with storing the PEI Services Table pointer,
+  so no additional migration actions are required for Itanium, ARM and
+  LoongArch CPUs.
  
  **/

  VOID
@@ -64,4 +65,32 @@ MigratePeiServicesTablePointer (
VOID
);
  
+/**

+  Retrieves the cached value of the PEI Services Table pointer from a CPU 
register.
+
+  Returns the cached value of the PEI Services Table pointer in a CPU specific 
manner
+  as specified in the CPU binding section of the Platform Initialization 
Pre-EFI
+  Initialization Core Interface Specification.
+
+  @return  The pointer to PeiServices.
+**/
+CONST EFI_PEI_SERVICES **
+EFIAPI
+GetPeiServicesTablePointerFromRegister (
+  VOID
+  );
+
+/**
+  Set the pointer PEI Service Table to a CPU register.
+
+  Caches the pointer to the PEI Services Table specified by 
PeiServicesTablePointer
+  in a platform specific manner.
+
+  @paramPeiServicesTablePointer   The address of PeiServices.
+**/
+VOID
+EFIAPI
+SetPeiServicesTablePointerToRegester (

SetPeiServicesTablePointerToRegester ->
SetPeiServicesTablePointerToRegister

Regester -> Register.

/
 Leif


+  IN UINTN  PeiServicesTablePointer
+  );
  #endif
diff --git 
a/MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c 
b/MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c
new file mode 100644
index 00..0227f98871
--- /dev/null
+++ b/MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c
@@ -0,0 +1,86 @@
+/** @file
+  PEI Services Table Pointer Library For Reigseter Mechanism.
+
+  This library is used for PEIM which does executed from flash device directly 
but
+  executed in memory.
+
+  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2011 Hewlett-Packard Corporation. All rights reserved.
+  Copyright (c) 2023 Loongson Technology Corporation Limited. All rights 
reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+
+/**
+  Caches a pointer PEI Services Table.
+
+  Caches the pointer to the PEI Services Table specified by 
PeiServicesTablePointer
+  in a platform specific manner.
+
+  If PeiServicesTablePointer is NULL, then ASSERT().
+
+  @paramPeiServicesTabl

Re: [edk2-devel] [PATCH v3 23/39] OvmfPkg/RiscVVirt: Enable UefiCpuPkg version CpuIo2Dxe

2023-11-19 Thread Chao Li

Hi Andrei,

Yes, the RISCV version is same as the ArmPkg version. The ArmPkg version 
is almost similar to UefiCpuPkg version, excapt that UefiCpuPkg version 
doesn't have MMIO methods on CpuIoServiceRead and CpuIoServiceWrite, the 
MMIO methods have been added in the patch 20, please check.



Thanks,
Chao
On 2023/11/18 04:15, Andrei Warkentin wrote:

So are you saying the UefiCpuPkg version of CpuIo2 is equivalent in function to 
the OvmfPkg one?


-Original Message-
From: Chao Li
Sent: Friday, November 17, 2023 4:02 AM
To:devel@edk2.groups.io
Cc: Sunil V L; Warkentin, Andrei

Subject: [PATCH v3 23/39] OvmfPkg/RiscVVirt: Enable UefiCpuPkg version
CpuIo2Dxe

Since the UefiCpuPkg/CpuIo2Dxe already supports MMIO, it is enabled at
this thime.

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

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

Cc: Sunil V L
Cc: Andrei Warkentin
---
  OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 4 +++-
OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 2 +-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
index 34b2037824..499902e445 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
@@ -143,6 +143,8 @@

gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE

+  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslationIsEnabled|TRUE
+
  [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
@@ -445,7 +447,7 @@
#
# PCI support
#
-  OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.inf {
+  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf {
  
NULL|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
}
diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
index 40d12e0f4c..dd138957a0 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
@@ -184,7 +184,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
  #
  # PCI support
  #
-INF  OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
+INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
  INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
  INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
  INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
--
2.27.0








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




[edk2-devel] [PATCH v3 23/39] OvmfPkg/RiscVVirt: Enable UefiCpuPkg version CpuIo2Dxe

2023-11-19 Thread Chao Li
Since the UefiCpuPkg/CpuIo2Dxe already supports MMIO, it is enabled at
this thime.

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

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

Cc: Sunil V L 
Cc: Andrei Warkentin 
---
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc | 4 +++-
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc 
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
index 34b2037824..499902e445 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc
@@ -143,6 +143,8 @@
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
 
+  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslationIsEnabled|TRUE
+
 [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
@@ -445,7 +447,7 @@
   #
   # PCI support
   #
-  OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.inf {
+  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf {
 
   NULL|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   }
diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf 
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
index 40d12e0f4c..dd138957a0 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
@@ -184,7 +184,7 @@ INF  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
 #
 # PCI support
 #
-INF  OvmfPkg/RiscVVirt/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
+INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
-- 
2.27.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111436): https://edk2.groups.io/g/devel/message/111436
Mute This Topic: https://groups.io/mt/102700998/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/3] EmulatorPkg: Format with Uncrustify 73.0.8

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

Thanks,
Ray

From: mikub...@linux.microsoft.com 
Sent: Wednesday, November 15, 2023 4:22 AM
To: devel@edk2.groups.io 
Cc: Andrew Fish ; Ni, Ray 
Subject: [PATCH v1 2/3] EmulatorPkg: Format with Uncrustify 73.0.8

From: Michael Kubacki 

Cc: Andrew Fish 
Cc: Ray Ni 
Signed-off-by: Michael Kubacki 
---
 EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c 
b/EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c
index bf2f0ac9808c..2234d29def44 100644
--- a/EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c
+++ b/EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c
@@ -112,9 +112,9 @@ EFI_FVB_MEDIA_INFO  mPlatformFvbMediaInfo[] = {
   {
 {
   (FixedPcdGet32 (PcdFlashNvStorageVariableSize) + \
-   FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) + \
-   FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize) + \
-   FixedPcdGet32 (PcdEmuFlashNvStorageEventLogSize)) / FixedPcdGet32 
(PcdEmuFirmwareBlockSize),
+FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) + \
+FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize) + \
+FixedPcdGet32 (PcdEmuFlashNvStorageEventLogSize)) / FixedPcdGet32 
(PcdEmuFirmwareBlockSize),
   FixedPcdGet32 (PcdEmuFirmwareBlockSize),
 }
   }
--
2.42.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111435): https://edk2.groups.io/g/devel/message/111435
Mute This Topic: https://groups.io/mt/102591692/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 3/3] Maintainers.txt: add Laszlo Ersek as a UefiCpuPkg maintainer

2023-11-19 Thread Ni, Ray
Laszlo,
Welcome!
Thanks for monitoring the changes in UefiCpuPkg. I appreciate your efforts on 
improving the code quality of UefiCpuPkg.

Reviewed-by: Ray Ni 

Thanks,
Ray

From: Laszlo Ersek 
Sent: Friday, November 17, 2023 5:50 AM
To: devel@edk2.groups.io 
Cc: Andrew Fish ; Gerd Hoffmann ; Leif 
Lindholm ; Kinney, Michael D 
; Kumar, Rahul R ; Ni, Ray 

Subject: [PATCH 3/3] Maintainers.txt: add Laszlo Ersek as a UefiCpuPkg 
maintainer

I intend to assist with the maintenance of the following files and
directories:

  UefiCpuPkg/CpuDxe/
  UefiCpuPkg/CpuIo2Dxe/
  UefiCpuPkg/CpuIo2Smm/CpuIo2Mm.c
  UefiCpuPkg/CpuIo2Smm/CpuIo2Mm.h
  UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
  UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
  UefiCpuPkg/CpuMpPei/
  UefiCpuPkg/CpuS3DataDxe/
  UefiCpuPkg/Include/AcpiCpuData.h
  UefiCpuPkg/Include/CpuHotPlugData.h
  UefiCpuPkg/Include/Library/CpuPageTableLib.h
  UefiCpuPkg/Include/Library/LocalApicLib.h
  UefiCpuPkg/Include/Library/MmSaveStateLib.h
  UefiCpuPkg/Include/Library/MpInitLib.h
  UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
  UefiCpuPkg/Include/Library/SmmCpuPlatformHookLib.h
  UefiCpuPkg/Include/Protocol/SmmCpuService.h
  UefiCpuPkg/Include/Register/
  UefiCpuPkg/Include/StuffRsbNasm.inc
  UefiCpuPkg/Library/BaseXApicX2ApicLib/
  UefiCpuPkg/Library/CpuExceptionHandlerLib/
  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTable.h
  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableLib.inf
  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableParse.c
  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
  UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
  UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
  UefiCpuPkg/Library/MpInitLib/Ia32/CreatePageTable.c
  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
  UefiCpuPkg/Library/MpInitLib/MpEqu.inc
  UefiCpuPkg/Library/MpInitLib/MpHandOff.h
  UefiCpuPkg/Library/MpInitLib/MpLib.c
  UefiCpuPkg/Library/MpInitLib/MpLib.h
  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
  UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c
  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
  UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c
  UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
  UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/
  UefiCpuPkg/Library/SmmCpuRendezvousLib/
  UefiCpuPkg/PiSmmCpuDxeSmm/
  UefiCpuPkg/UefiCpuPkg.dec
  UefiCpuPkg/UefiCpuPkg.dsc
  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/

Cc: Andrew Fish 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Rahul Kumar 
Cc: Ray Ni 
Signed-off-by: Laszlo Ersek 
---
 Maintainers.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 2583ba639038..5cedd339f55a 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -617,6 +617,7 @@ UefiCpuPkg
 F: UefiCpuPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/UefiCpuPkg
 M: Ray Ni  [niruiyu]
+M: Laszlo Ersek  [lersek]
 R: Rahul Kumar  [rahul1-kumar]
 R: Gerd Hoffmann  [kraxel]



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

2023-11-19 Thread Yoshinoya
Hi, Michael:
Got it.


It seems linux kernel has introduced PRMT support default.
And ARM vendors usually use this PRMT mechnism.


So how aout x86 platform?
Does Win11 have supported PRMT on any X86 platform?




best wishes,











At 2023-11-18 00:04:26, "Michael Kubacki"  wrote:
>On 11/17/2023 3:42 AM, Laszlo Ersek wrote:
>> On 11/17/23 03:15, Yoshinoya wrote:
>>> Hi,
>>> I find there is a PrmPkg in udk source code.
>>> Based on its Readme.md, its goal is to offload smm code to sci os
>>> mechanisms.
>>>
>>> So, is there any actual use case on real platform now?
>>>
>>> It seems it's just a conceptional prototype.
>> 
>> It's way too big for it to be unused.
>> 
>> The original BZ was .
>> 
>> I'm sure Microsoft uses it in production. Client code for this
>> infrastructure may be present in Project Mu (I didn't try to check), or
>> in proprietary repositories. Perhaps Michael (CC'd) can share some details.
>> 
>I can't speak to how it is being used everywhere but it is used in 
>production. Other vendors have been involved (at least at various points 
>in time).
>
>The ACPI 6.4 spec reserved the PRMT table signature. The ACPI 6.5 spec 
>defined the _SB._OSC bit for an OS to declare PRM compatibility and 
>define a PRM OpRegion identifier. Support has been in iasl since 20200528.
>
>The PRM spec is on uefi.org. I believe this was ultimately pushed there 
>by Intel.
>
>https://uefi.org/sites/default/files/resources/Platform%20Runtime%20Mechanism%20-%20with%20legal%20notice.pdf
>
>It was added to edk2 to provide code for specifications on uefi.org, 
>make it available to vendors that do not use Mu but use it, and 
>similarly, in response to interest from others.
>
>The Code in PrmPkg is infrastructure to support loading custom handlers 
>so it is not expected to receive a large amount of churn.
>
>> Laszlo
>> 
>> 
>> 
>> 
>> 
>
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111433): https://edk2.groups.io/g/devel/message/111433
Mute This Topic: https://groups.io/mt/102640402/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 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use processor extended information

2023-11-19 Thread Ni, Ray
let me add more to explain:

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

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

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

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

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

2. CPU Feature Initialization

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

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

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

Thanks,
Ray


From: Laszlo Ersek 
Sent: Saturday, November 18, 2023 5:05 AM
To: devel@edk2.groups.io ; Ni, Ray ; 
Wu, Jiaxin 
Cc: Dong, Eric ; Kumar, Rahul R ; 
Gerd Hoffmann ; Zeng, Star 
Subject: Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use 
processor extended information

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

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

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

Thanks,
Laszlo



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




[edk2-devel] [PATCH] MdePkg:Added support for Smbios 3.7.0 Spec

2023-11-19 Thread Shenbagadevi R via groups.io
As per Smbios 3.7.0 spec, added CXL 3.0 support in Type 9, also added
PMIC & RCD manufacturer ID and Revision information in Type17.

Signed-off-by: Shenbagadevi R 

CC: Gaoliming 
CC: Sainadh N 
CC: Sundaresan S 
CC: Srinivasan M 
CC: Ramesh R 

---
 MdePkg/Include/IndustryStandard/SmBios.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h 
b/MdePkg/Include/IndustryStandard/SmBios.h
index 56cec615a0..cfefc2fa52 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -5,7 +5,7 @@ Copyright (c) 2006 - 2023, Intel Corporation. All rights 
reserved.
 (C) Copyright 2015-2017 Hewlett Packard Enterprise Development LP

 (C) Copyright 2015 - 2019 Hewlett Packard Enterprise Development LP

 Copyright (c) 2022, AMD Incorporated. All rights reserved.

-Copyright (c) 1985 - 2022, American Megatrends International LLC.

+Copyright (c) 1985 - 2023, American Megatrends International LLC.

 SPDX-License-Identifier: BSD-2-Clause-Patent



 **/

@@ -1509,7 +1509,7 @@ typedef struct {
   UINT8AsyncSurpriseRemoval: 1;

   UINT8FlexbusSlotCxl10Capable : 1;

   UINT8FlexbusSlotCxl20Capable : 1;

-  UINT8Reserved: 1; ///< Set to 0.

+  UINT8FlexbusSlotCxl30Capable : 1; ///SMBIOS spec 3.7.0 updated CXL 3.0 
support

 } MISC_SLOT_CHARACTERISTICS2;



 ///

@@ -2012,6 +2012,13 @@ typedef struct {
   //

   UINT32 ExtendedSpeed;

   UINT32 ExtendedConfiguredMemorySpeed;

+  //

+  // Add for smbios 3.7.0

+  //

+  UINT16 Pmic0ManufacturerID;

+  UINT16 Pmic0RevisionNumber;

+  UINT16 RcdManufacturerID;

+  UINT16 RcdRevisionNumber;

 } SMBIOS_TABLE_TYPE17;



 ///

--
2.38.0.windows.1
-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited. Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.


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




[edk2-devel] [PATCH] Modified ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c to follow error handling instead of success handling

2023-11-19 Thread Daniel Nguyen
Signed-off-by: Daniel Nguyen 
---
 .../UefiShellLevel2CommandsLib/Reset.c| 43 +++
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
index 57ba3c90f3..361c47e430 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
@@ -79,30 +79,35 @@ ShellCommandRunReset (
   &DataSize,
   &OsIndications
   );
-if (!EFI_ERROR (Status)) {
-  if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
-DataSize = sizeof (OsIndications);
-Status   = gRT->GetVariable (
-  EFI_OS_INDICATIONS_VARIABLE_NAME,
-  &gEfiGlobalVariableGuid,
-  &Attr,
-  &DataSize,
-  &OsIndications
-  );
-if (!EFI_ERROR (Status)) {
-  OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
-} else {
-  OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
-}
-
-Status = gRT->SetVariable (
+
+if (EFI_ERROR (Status)) {
+  ShellStatus = SHELL_UNSUPPORTED;
+  goto Error;
+}
+
+if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
+  DataSize = sizeof (OsIndications);
+  Status   = gRT->GetVariable (
 EFI_OS_INDICATIONS_VARIABLE_NAME,
 &gEfiGlobalVariableGuid,
-EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-sizeof (OsIndications),
+&Attr,
+&DataSize,
 &OsIndications
 );
+
+  if (EFI_ERROR (Status)) {
+OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+  } else {
+OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
   }
+
+  Status = gRT->SetVariable (
+  EFI_OS_INDICATIONS_VARIABLE_NAME,
+  &gEfiGlobalVariableGuid,
+  EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+  sizeof (OsIndications),
+  &OsIndications
+  );
 }
 
 if (EFI_ERROR (Status)) {
-- 
2.25.1



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




[edk2-devel] Fail to build boot with tools code failure

2023-11-19 Thread Jackie Lien (Temp) via groups.io
Hi team,

Please help with the build error in BOOT.MXF.1.1.1. The command and log are 
listed below.

Sync & build command:

  1.  python sync_crm.py BOOT.MXF.1.1.1-00175-Olympic-1
  2.  copy manifest to my local sync (copy 
\\snowcone\builds906\PROD\BOOT.MXF.1.1.1-00175-Olympic-1\boot_images\boot_tools\manifest\build\manifest.xml
 to 
\\jacklien-linux\workspace\BOOT.MXF.1.1.1-00175-Olympic-1\boot_images\boot_tools\manifest\build\manifest.xml)
  3.  python buildex.py -t Olympic -v LE -r DEBUG



Build log:
build.py...
: error C0DE: Tools code failure
Please send email to devel@edk2.groups.io 
for help, attaching following call stack trace!

Traceback (most recent call last):
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/GenFds.py",
 line 371, in GenFdsApi
GenFds.GenFd('', FdfParserObj, BuildWorkSpace, ArchList)
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/GenFds.py",
 line 510, in GenFd
FdObj.GenFd()
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Fd.py",
 line 131, in GenFd
RegionObj.AddToBuffer (FdBuffer, self.BaseAddress, self.BlockSizeList, 
self.ErasePolarity, GenFdsGlobalVariable.ImageBinDict, self.DefineVarDict, 
Flag=Flag)
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Region.py",
 line 134, in AddToBuffer
FvObj.AddToBuffer(FvBuffer, FvBaseAddress, BlockSize, BlockNum, 
ErasePolarity, Flag=Flag)
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Fv.py",
 line 127, in AddToBuffer
FileName = FfsFile.GenFfs(MacroDict, FvParentAddr=BaseAddress, 
IsMakefile=Flag, FvName=self.UiFvName)
   
^^
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/FfsInfStatement.py",
 line 518, in GenFfs
InputSectList, InputSectAlignments = self.__GenComplexFileSection__(Rule, 
FvChildAddr, FvParentAddr, IsMakefile=IsMakefile)
 
^^
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/FfsInfStatement.py",
 line 969, in __GenComplexFileSection__
SectList, Align = Sect.GenSection(self.OutputPath, self.ModuleGuid, 
SecIndex, self.KeyStringList, self, IsMakefile = IsMakefile)
  
^^
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/EfiSection.py",
 line 263, in GenSection
ImageObj = PeImageClass (File)
   ^^^
  File 
"/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/Common/Misc.py",
 line 1638, in __init__
if ByteArray.tostring() != b'PE\0\0':
   ^^
AttributeError: 'array.array' object has no attribute 'tostring'



build.py...
: error 7000: Failed to execute command


- Failed -
Build end time: 11:15:43, Nov.16 2023
Build total time: 00:00:23

ERROR: buildex::Run  ['python', 
'/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/build/build.py',
 '-p', 'QcomPkg/SocPkg/Olympic/LE/Core.dsc', '-j', 
'QcomPkg/SocPkg/Olympic/LE/build_Core.log', '-w', '-a', 'ARM', '-b', 'DEBUG', 
'-t', 'CLANG100LINUX', '-D', 
'BUILDROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images', 
'-D', 
'BOOTROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/boot',
 '-D', 'REL=DEBUG', '-D', 'PRODMODE=DEBUGMODE', '-D', 'VAR=LE', '-D', 
'COMPLIER=', '-D', 'FIRSTDLL=', '-D', 'SECONDDLL=', '-D', 'XBLSEC=', '-D', 
'TARGETROOT=QcomPkg/SocPkg/Olympic', '-D', 'TARGETID=SocOlympic', '-D', 
'TARGETPKG=SocPkg', '-D', 'QCOMDIR=', '-D', 
'BOOTTOOLS=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/boot_tools',
 '-D', 
'EDK2ROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2',
 '-D', 'SECTOOLS_DIR=/pkg/sectools/glue/latest', '-D', 
'SECTOOLSROOT=/pkg/sectools/glue/latest', '-D', 'COMPILER=CLANG100LINUX', '-D', 
'AARCH=ARM'] Build Subprocess failed with error...
None




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




[edk2-devel] BaseTools KeyError When Building EmulatorPkg

2023-11-19 Thread ryderkeys via groups.io
Hi all,

I am trying to build EDK2 from most recent Github (namely, EmulatorPkg) with 
the new Stuart build system on Windows 10 22H2 64 bit with VS 2019. I'm getting 
the following error which directed me to this mailing list. More details on my 
build process below.  

INFO - build.py...
INFO -  : error C0DE: Unknown fatal error when processing 
[c:\users\ryder\edk2_2023\edk2\EmulatorPkg\EmulatorPkg.dsc]
INFO - 
INFO - (Please send email to devel@edk2.groups.io for help, attaching following 
call stack trace!)
INFO - 
INFO - (Python 3.11.5 on win32) Traceback (most recent call last):
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
2692, in Main
INFO - MyBuild = Build(Target, Workspace, Option,LogQ)
INFO -   ^
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
815, in __init__
INFO - self.InitPreBuild()
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
1015, in InitPreBuild
INFO - self.LoadConfiguration()
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
971, in LoadConfiguration
INFO - self.GetToolChainAndFamilyFromDsc (self.PlatformFile)
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
905, in GetToolChainAndFamilyFromDsc
INFO - dscobj = self.BuildDatabase[File, BuildArch]
INFO -  ~~^
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\WorkspaceDatabase.py",
 line 104, in __getitem__
INFO - BuildObject = self.CreateBuildObject(FilePath, Arch, Target, 
Toolchain)
INFO -   
^
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\WorkspaceDatabase.py",
 line 125, in CreateBuildObject
INFO - BuildObject = self._GENERATOR_[FileType](
INFO -   ^^^
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
 line 231, in __init__
INFO - self.SkuIdMgr = SkuClass(self.SkuName, self.SkuIds)
INFO -  
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
 line 510, in SkuName
INFO - self._GetHeaderInfo()
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
 line 310, in _GetHeaderInfo
INFO - RecordList = self._RawData[MODEL_META_DATA_HEADER, self._Arch]
INFO -  ~
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
 line 250, in __getitem__
INFO - self._PostProcess()
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
 line 1429, in _PostProcess
INFO - Processer[self._ItemType]()
INFO -   File 
"C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
 line 1609, in __ProcessDirective
INFO - __IncludeMacros['WORKSPACE'] = GlobalData.gGlobalDefines['WORKSPACE']
INFO -~^
INFO - KeyError: 'WORKSPACE'

I followed the directions on 

 and . 
Are these the correct set of instructions to follow? If so, I entered the 
following commands and received the error above.

git clone https://github.com/tianocore/edk2.git
cd edk2
python -m venv .venv
.\.venv\Scripts\Activate.ps1
stuart_setup -c EmulatorPkg/PlatformCI/PlatformBuild.py TOOL_CHAIN_TAG=VS2019 
-a X64
stuart_update -c EmulatorPkg/PlatformCI/PlatformBuild.py TOOL_CHAIN_TAG=VS2019 
-a X64
stuart_build -c EmulatorPkg/PlatformCI/PlatformBuild.py TOOL_CHAIN_TAG=VS2019 
-a X64

A quick couple of print statements in the python code of basetools reveal that 
gGlobalDefines is an empty dictionary. Help would be greatly appreciated. Let 
me know if you need more information.

Thank you,
Ryder


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111431): https://edk2.groups.io/g/devel/message/111431
Mute This Topic: https://groups.io/mt/102698986/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, November 20, 2023 #cal-reminder

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

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

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

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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




[edk2-devel] 回复: [edk2-202311] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

2023-11-19 Thread gaoliming via groups.io
Laszlo and Gerd:
  I agree this is a critical bug fix. If it plans to catch this stable tag, I 
am OK. 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Laszlo Ersek
> 发送时间: 2023年11月18日 5:39
> 收件人: devel@edk2.groups.io; kra...@redhat.com
> 抄送: Michael Roth ; Ray Ni ;
> Erdem Aktas ; Jiewen Yao
> ; Min Xu ; Tom Lendacky
> ; Liming Gao (Byosoft address)
> ; Michael Kinney 
> 主题: Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address
> overflow during PVALIDATE
> 
> On 11/17/23 12:42, Gerd Hoffmann wrote:
> > On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
> >> (+Liming +Mike)
> >>
> >> On 11/16/23 10:01, Gerd Hoffmann wrote:
> >>> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>  The struct used for GHCB-based page-state change requests uses a
> 40-bit
>  bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>  64-bit address. However, anything beyond 40-bits simply gets shifted off
>  when doing this, which will cause issues when dealing with 1TB+
>  addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>  prior to shifting it by PAGE_SHIFT.
> 
>  Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to
> validate system RAM")
>  Signed-off-by: Michael Roth 
> >>>
> >>> Reviewed-by: Gerd Hoffmann 
> >>>
> >>> take care,
> >>>   Gerd
> >>
> >> Is this hard feature freeze material?
> >
> > It is a clear bugfix, so IMHO it qualifies.
> >
> >> Also, the patch looks garbled to me on-list (superfluous line breaks).
> >
> > Patch applies fine here.  I see mutt breaking the long line, but
> > that is just the local display rendering, the mail good.
> 
> Can you check the raw message? I did that and it seems broken.
> Superfluous newlines. I see *double* CRLFs.
> 
> Laszlo
> 
> 
> 
> 
> 





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