Re: [edk2-devel] Fixing edk2-basetools CI

[Rebecca Cran]

I've updated the PR 
https://github.com/tianocore/edk2-basetools/pull/116/ to include commits 
from the pyflake8 disable PR and a couple more commits that cause CI to 
start passing. I realize that disabling both flake8 and codecov isn't 
good, but my thinking is to get builds working again then we can work on 
fixing the issues in the background. The edk2-basetools project needs 
lots of work.



Add pyproject.toml and fix setup.py deprecation warnings

Disable running Pyflakes in CI since it's blocking releases

Fix comments in build-publish-whl-steps.yml

Update basic-setup-steps.yml to require Python 3.10

Disable codecov to fix CI and due to very low coverage


--
Rebecca Cran


On 2/13/24 12:42, Joey Vagedes wrote:

I agree - there are multiple blocking issues that will require some fixes - One 
additional thing I noted is that multiple areas still perform relative path 
imports - i.e. the import starts with `from .`.

These will all need to be updated to import from edk2basetools as some point 
soon as importing via relative paths can result in it importing the wrong file 
if the pypath environment variable is influenced. This open issue actually 
stems from that: https://github.com/tianocore/edk2-basetools/issues/110 as the 
end result is that edk2-basetools pip module can unexpectedly end up using the 
BaseTools/* source code rather than the pip module source code.

Thanks,
Joey

-Original Message-
From: Rebecca Cran 
Sent: Tuesday, February 13, 2024 11:28 AM
To: Joey Vagedes ; Rebecca Cran ; 
devel@edk2.groups.io; Kinney, Michael D ; Sean 
; Michael Kubacki 
Subject: Re: [EXTERNAL] Re: [edk2-devel] Fixing edk2-basetools CI

[You don't often get email from rebe...@bsdio.com. Learn why this is important 
at https://aka.ms/LearnAboutSenderIdentification ]

Thanks, I've updated
https://github.com/tianocore/edk2-basetools/pull/116 with the changes you 
suggested.

Unfortunately several of the tools don't work with the projects.scripts section 
because they don't have Main etc. It turns out even Ecc is currently broken in 
the edk2-basetools PyPI package because PYTHONPATH doesn't contain the parent 
directory. I'll plan to fix the issues in the near future, once I've worked 
through the other PRs.


--

Rebecca Cran


On 2/12/24 10:06, Joey Vagedes wrote:

Hello! Regarding flake8, I see. That will take some time to work through - I 
completely understand.

When ready to work through that, I suggest switching to ruff. It can resolve 
some errors automatically for you and allows you to customize it more than 
flake8 (it is also faster).

I just ran ruff on the project, showing about 11k errors. One nice thing is 
that a large majority of it is regarding importing using *. By fixing those, 
you can reduce the errors by almost 70% (to 3400 errors), so it is not *quite* 
as bad as it seems (though 3400 is still A LOT).

--
-

Regarding the setup tools question, the build-backend part of pytoml (L3) is the entry 
point that is invoked - so we are still invoking setuptools. However, it pulls the 
necessary metadata from pyproject.toml rather than setup.py's setup() function. The main 
thing that is different is the "get_version" functionality, which we replicate 
in the pyproject.toml with setuptools-scm[toml] which simply uses the latest tag to set 
the version.

--
-

Additionally, I wanted to comment on the wrappers that you mentioned.

Using the [projects.scripts] section (i.e. CLI Commands feature) is in my 
opinion the correct way to move forward as users will be able to invoke 
commands themselves easily (i.e. just call Bin2Pcd, and it will work). However, 
the wrappers will continue to be necessary until all python source is removed 
from Basetools, so that users can toggle between pip or local. However, once 
source is removed form BaseTools, removing the wrappers is the next step 
forward.

In the meantime, you will be able to update the binpipwrappers to something 
akin to the following (untested, so I don't garuntee it will be exactly this:

@setlocal
Ecc %*

Rather than

@setlocal
@set ToolName=%~n0%
@%PYTHON_COMMAND% -m edk2basetools.%ToolName%.EccMain %*

--
-

One thing to keep in mind once Python is completely removed from BaseTools, is 
that consumers will need to get in the habit of verifying the correct version 
of basetools is installed for the commit they are on (or automate this 
checking) particularly for anyone that regularly switches between stable 
branches.

Thanks,
Joey

-Original Message-
From: Rebecca Cran 
Sent: Monday, February 12, 2024 8:42 AM
To: devel@edk2.groups.io; Joey Vagedes ;
Rebecca Cran ; Kinney,

Re: [edk2-devel] [PATCH edk2-test 1/1] uefi-sct/SctPkg: correct %LX SctPrint code

[G Edhaya Chandran]

The patch is upstreamed by the commit:
https://github.com/tianocore/edk2-test/commit/19e53ca9a39c5cb2deed1dc1d7758a6b0241991c


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

[edk2-devel] [PATCH v2] IntelFsp2Pkg: Fsp 2.x Changes

[cbduggap]

Changes to support spec changes

1. Remove usage of Pcd.
2. Change code to validate the Temporary Ram size input.
3. Consume the input saved in YMM Register

Cc: Sai Chaganty 
Cc: Nate DeSimone 
Cc: Chiu Chasel 
Cc: Duggapu Chinni B 


Signed-off-by: Duggapu Chinni B 
---
 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf |   2 +-
 IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf   |   2 +-
 IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf   |   1 +
 .../FspSecCore/Ia32/Fsp24ApiEntryM.nasm   |   1 -
 .../FspSecCore/Ia32/FspApiEntryM.nasm |   1 -
 .../FspSecCore/Ia32/FspApiEntryT.nasm | 110 --
 .../FspSecCore/Ia32/SaveRestoreSseNasm.inc|  11 ++
 IntelFsp2Pkg/FspSecCore/SecFsp.c  |  23 ++--
 IntelFsp2Pkg/FspSecCore/SecFspApiChk.c|   4 +-
 IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm |  79 +++--
 IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm|   6 +-
 IntelFsp2Pkg/Include/FspEas/FspApi.h  |   5 +-
 .../Include/SaveRestoreSseAvxNasm.inc |  21 
 IntelFsp2Pkg/IntelFsp2Pkg.dec |   4 +
 .../SecRamInitData.c  |   3 +-
 15 files changed, 206 insertions(+), 67 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf 
b/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
index cb011f99f9..cf8cb2eda9 100644
--- a/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
+++ b/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
@@ -63,11 +63,11 @@
 
 [Pcd]
   gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase  ## CONSUMES
-  gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamSize  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize   ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspHeapSizePercentage ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspMaxInterruptSupported  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspPrivateTemporaryRamSize## CONSUMES
+  gIntelFsp2PkgTokenSpaceGuid.PcdGlobalDataPointerAddress  ## CONSUMES
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid  ## PRODUCES
diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf 
b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
index 8029832235..717941c33f 100644
--- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
+++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
@@ -62,11 +62,11 @@
 
 [Pcd]
   gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase  ## CONSUMES
-  gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamSize  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize   ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspHeapSizePercentage ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspMaxInterruptSupported  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspPrivateTemporaryRamSize## CONSUMES
+  gIntelFsp2PkgTokenSpaceGuid.PcdGlobalDataPointerAddress  ## CONSUMES
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid  ## PRODUCES
diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf 
b/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
index e5a6eaa164..05c0d5f48b 100644
--- a/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
+++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
@@ -51,6 +51,7 @@
   gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamSize  ## CONSUMES
   gIntelFsp2PkgTokenSpaceGuid.PcdFspReservedBufferSize ## CONSUMES
+  gIntelFsp2PkgTokenSpaceGuid.PcdGlobalDataPointerAddress  ## CONSUMES
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid  ## PRODUCES
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
index 15f8ecea83..5fa5c03569 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
@@ -11,7 +11,6 @@
 ; Following are fixed PCDs
 ;
 extern   ASM_PFX(PcdGet32(PcdTemporaryRamBase))
-extern   ASM_PFX(PcdGet32(PcdTemporaryRamSize))
 extern   ASM_PFX(PcdGet32(PcdFspTemporaryRamSize))
 extern   ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage))
 
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
index 61ab4612a3..861cce4d01 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
@@ -11,7 +11,6 @@
 ; Following are fixed PCDs
 ;
 extern   ASM_PFX(PcdGet32(PcdTemporaryRamBase))
-extern   ASM_PFX(PcdGet32(PcdTemporaryRamSize))
 extern   ASM_PFX(PcdGet32(PcdFspTemporaryRamSize))
 extern   ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage))
 
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
index 900126b93b..2f8465df3d 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
@@ -109,7 +109,8 @@ struc LoadMicrocodeParamsFsp24
 .FsptArchReserved:resb3
 .FsptArchLength:  resd1
 .FspDebugHand

Re: [edk2-devel][PATCH v1 1/1] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations

[Ard Biesheuvel]

On Thu, 15 Feb 2024 at 20:55, Oliver Smith-Denny
 wrote:
>
> On 2/14/2024 11:50 PM, Ard Biesheuvel wrote:
> > I looked at the EFI spec again, and EfiACPIReclaimMemory is not
> > actually listed as a memory type that has this 64k alignment
> > requirement. This makes sense, given that this memory type has no
> > significance to the firmware itself, only to the OS. OTOH, reserved
> > memory does appear there.
> >
> > So I suggest we fix that first, and then drop any mention of
> > EfiACPIReclaimMemory from this patch. At least we'll have heap guard
> > coverage for ACPI table allocations on arm64 going forward.
> >
> > The logic in question was added in 2007 in commit 28a00297189c, so
> > this was probably the rule on Itanium, but that support is long gone.
> >
>
> I wanted to understand this a little bit further. I do see that the spec
> does not call out EfiACPIReclaimMemory as needing 64k alignment.
> However, your statement that the memory type does not have have
> significance to the FW, only to the OS, so we don't need the 64k
> alignment is confusing to me. Isn't the 64k alignment needed only for
> regions that the OS does care about? In order to read this information
> at their 64k page granularity, doesn't this need to be aligned to 64k?
>

It is not about accessing the information in the pages. Most consumers
don't care about this at all, and even code that does will rarely be
materially different between 4k and 64k OSes.

When running under the firmware, we run with 4k pages and programming
the memory attributes is entirely under the control of the firmware
itself. So in this phase, the alignment truly does not matter,
especially because all of RAM is 1:1 mapped anyway.

When running under the OS, all memory that is part of the OS's memory
pool is entirely under its control, and this includes ACPI reclaim
memory, given that the firmware itself should not be accessing this:
the purpose of this memory type is specifically to expose information
to the OS in memory that it can use as it sees fit after it is done
processing the information.

The alignment issues we need to avoid are:
- runtime DXEs where we want to map code RO and data NX, which implies
that the boundary between the two must be 64k aligned;
- other EFI_MEMORY_RUNTIME regions that may be described with
EFI_MEMORY_WC rather than EFI_MEMORY_WB; this could be related to
implementations of the varstore or other runtime services, and the OS
has no idea what the region is for and how the runtime code is using
it;
- reserved regions that are mapped, e.g., by AML code, as a
SystemMemory OpRegion; there are examples in RAS code where the memory
needs to be mapped uncached so that stores are immediately visible to
the BMC or other agents that sit on the interconnect. The existence of
a cacheable alias of the same mapping will interfere with this, i.e.,
in many cases it will simply make both mappings cacheable.


> Or are you saying the OS can just read the 64k page(s) containing these
> 4k aligned EfiACPIReclaimMemory pages and then start reading at the
> calculated offset from within the larger page, since these pages don't
> have any pointers to outside memory, unlike image sections?
>

In all these examples, the alignment *itself* is not the goal. The 64k
alignment is way to ensure that the OS is never in a situation where a
64k page frame is shared between a cacheable and an uncacheable
mapping, or between a ROX and a W+NX mapping. Note that, in the
runtime DXE case, the only way to map the 64k page that has both code
and data is to map it RWX, given that the contents need to appear
contiguous in virtual memory (we had great fun with that when we
designed the MAT)

For the other cases, there may be mappings that are completely
disjoint in the virtual address space. But if two mappings exist of
the same physical page, one cacheable via one alias, and one
non-cacheable via another, the result is unpredictable and things like
RAS or BMC comms from runtime firmware may be completely broken.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115534): https://edk2.groups.io/g/devel/message/115534
Mute This Topic: https://groups.io/mt/104364784/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] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations

[Oliver Smith-Denny]

On 2/14/2024 11:50 PM, Ard Biesheuvel wrote:

I looked at the EFI spec again, and EfiACPIReclaimMemory is not
actually listed as a memory type that has this 64k alignment
requirement. This makes sense, given that this memory type has no
significance to the firmware itself, only to the OS. OTOH, reserved
memory does appear there.

So I suggest we fix that first, and then drop any mention of
EfiACPIReclaimMemory from this patch. At least we'll have heap guard
coverage for ACPI table allocations on arm64 going forward.

The logic in question was added in 2007 in commit 28a00297189c, so
this was probably the rule on Itanium, but that support is long gone.



I wanted to understand this a little bit further. I do see that the spec
does not call out EfiACPIReclaimMemory as needing 64k alignment.
However, your statement that the memory type does not have have
significance to the FW, only to the OS, so we don't need the 64k
alignment is confusing to me. Isn't the 64k alignment needed only for
regions that the OS does care about? In order to read this information
at their 64k page granularity, doesn't this need to be aligned to 64k?

Or are you saying the OS can just read the 64k page(s) containing these
4k aligned EfiACPIReclaimMemory pages and then start reading at the
calculated offset from within the larger page, since these pages don't
have any pointers to outside memory, unlike image sections?

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115533): https://edk2.groups.io/g/devel/message/115533
Mute This Topic: https://groups.io/mt/104364784/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] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations

[Oliver Smith-Denny]

On 2/15/2024 9:21 AM, Ard Biesheuvel wrote:

Of the two options you presented in this paragraph, I prefer the one
where the allocation presented to the caller may not be aligned, but
the region plus guards is.

But disabling it entirely for these regions is still perfectly fine
with me, especially if the remove ACPI reclaim memory from the set.
Heap guard is a hardening feature, and if the implementation is too
complex to reason about comfortably, I don't think we can confidently
rely on it.

And as far as the OS is concerned: with the MAT, the runtime DXEs are
mapped in a way where the read-only regions are interleaved with the
read-write regions, and the holes in between are not mapped at all (at
least on Linux). IOW, there is some implicit guarding going on
already.



Looking back at the UEFI spec (section 2.3.6), I see this:

"If a 64KiB physical page contains any 4KiB page with any of the
following types listed below, then all 4KiB pages in the 64KiB page
must use identical ARM Memory Page Attributes" where the following
types are what you listed in the last email.

Then there is a further statement:

"Mixed attribute mappings within a larger page are not allowed."

So this would seem to indicate that pushing the guard pages inside
of the 64KiB would break the spec. Now, I think it could be hidden
and still meet the intent of the spec, which would be that the OS
gets consistent memory attributes reported, but still, the way the
spec is written this would seem to be a violation.

I'll send out a v2 with the type change.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115532): https://edk2.groups.io/g/devel/message/115532
Mute This Topic: https://groups.io/mt/104364784/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/4] NetworkPkg: : Updating SecurityFixes.yaml

[Doug Flick via groups.io]

At this point - yes - but I don't have the ability to edit it. The advisory 
should reflect the current status. SecurtiyFixes.yaml is a way to express which 
commits are needed to be cherrypicked by a downstream consumer and what the 
current release is protected against. 


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

Re: [edk2-devel] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules

[Michael D Kinney]

Hi Laszlo,

I was also thinking the INF Version would be best approach.

I recommend we identify the EDK II Build Specification and
EDK II INF Specification changes required to resolve this 
issue.

https://github.com/tianocore-docs/edk2-BuildSpecification
https://github.com/tianocore-docs/edk2-InfSpecification


The current INF Spec uses INF_VERSION of 1.27.

Should the new version be 1.28, or is there something I am
missing where 1.30 would be required?  Or are you wanting
to jump from 1.2x to 1.3x to indicate a behavior change?

Thanks,

Mike


> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, February 14, 2024 11:58 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> 
> Cc: Abdul Lateef Attar ; Abner Chang
> ; Warkentin, Andrei ;
> Andrew Fish ; Ard Biesheuvel
> ; S, Ashraf Ali ;
> Bibo Mao ; Feng, Bob C ;
> West, Catharine ; Chao Li
> ; Chiu, Chasel ; Duggapu,
> Chinni B ; Duke Zhai ;
> Aktas, Erdem ; Eric Xing ;
> Gerd Hoffmann ; Guo, Gua ; Dong,
> Guo ; Igniculus Fu ; Lu,
> James ; Yao, Jiewen ; Kelly
> Steele ; Ken Yao ; Leif
> Lindholm ; Liming Gao
> ; Michael Roth ; Xu,
> Min M ; Desimone, Nathaniel L
> ; Paul Grimes ;
> Kumar, Rahul R ; Ni, Ray ;
> Rebecca Cran ; Chaganty, Rangasai V
> ; Sami Mujawar ;
> Rhodes, Sean ; Zeng, Star ;
> Sunil V L ; Mohapatra, Susovan
> ; Kuo, Ted ; Tom
> Lendacky ; USER0FISH ;
> Xianglai li ; Chen, Christine
> ; caiyuqing379 ; dahogn
> ; meng-cz 
> Subject: Re: [edk2-devel] BaseTools/AutoGen: declare
> ProcessLibraryConstructorList() for SEC modules
> 
> On 2/8/24 17:40, Michael D Kinney wrote:
> > Hi Laszlo,
> >
> > I need to review the proposed BaseTools/AutoGen change to see what
> options
> > are available for compatibility.
> >
> > My main concern is downstream consumers that may break immediately
> with
> > a change like this and we need a way for them to be informed and have
> > time to update their components just like you outline a sequence to
> update
> > the edk2 repo components.
> 
> Should AutoGen declare ProcessLibraryConstructorList() for a SEC module
> if INF_VERSION >= 1.30?
> 
> Or should we introduce a new macro in [Defines]?
> 
> https://tianocore-docs.github.io/edk2-
> InfSpecification/draft/2_inf_overview/24_[defines]_section.html
> 
> "EDK II parsing utilities will use some of this section's information
> for generating AutoGen.c and AutoGen.h files."
> 
> I'd prefer (INF_VERSION >= 1.30) over a dedicated macro. We should
> ensure, over time, that ProcessLibraryConstructorList() is declared by
> default, for SEC modules. If that declaration depended on an explicit
> new macro in [Defines], it would much less likely become the default.
> 
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115530): https://edk2.groups.io/g/devel/message/115530
Mute This Topic: https://groups.io/mt/104210524/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] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations

[Ard Biesheuvel]

On Thu, 15 Feb 2024 at 18:08, Oliver Smith-Denny
 wrote:
>
> On 2/14/2024 11:50 PM, Ard Biesheuvel wrote> On Thu, 15 Feb 2024 at
> 01:34, Oliver Smith-Denny
> >  wrote >> This could also be fixed with 
> > rearchitecting the heap guard system to
> >> respect alignment requirements and shift the guard pages inside of the
> >> outer rounded allocation or by having guard pages be the runtime
> >> granularity. Both of these approaches have issues, in the former, the
> >> allocator of runtime memory would get an address that was not aligned
> >> with the runtime granularity (the head guard would be, but not the
> >> usuable address), which seems fraught with peril.
> >
> > This would be my preference, and I wouldn't expect huge problems with
> > code expecting a certain alignment for such allocations. The 64k
> > requirement is entirely to ensure that the OS does not have to guess
> > how it should map 64k pages that have conflicting memory attributes
> > due to being covered by two different misaligned entries in the UEFI
> > memory map.
> >
> > This is also why this is important for the MAT and runtime services
> > code/data regions: without 64k alignment, there will be a piece in the
> > middle of each runtime DXE that requires both write and execute
> > permissions.
> >
> >
>
> I do have a PR up to fix the misalignment bug (I was doing a CI check on
> it before sending the patch when I did some further testing to discover
> the guard pages pushing out the allocation size). Would you prefer that
> I update that to do the guard page allocation inside the 64k allocation?
>
> I can certainly do that, again my concern is that the code starts
> getting more complex, with more room for errors. The heap guard code now
> needs to know the actual size requested by the caller, not the rounded
> size, so we can see, oh, the allocation requested is 64k, so I need
> another 64k region to fit the guards into, unless we already have shared
> guard pages, in which case we may not need to, or one guard may be
> shared and the other isn't, etc. It is doable, but I worry about the
> added complexity for only a small window of protection for these runtime
> memory regions. We could also say no shared guard pages for runtime
> regions if you don't have runtime allocation granularity equal to the
> EFI_PAGE_SIZE.
>
> Based on our offline conversation, I thought you were ok with the simple
> approach of disable the guards for these regions, the value of
> protecting these regions at boot time is not worth the additional
> complexity. But, I can update my PR to put the guards inside the
> allocation and we can compare the relative complexity.
>

Of the two options you presented in this paragraph, I prefer the one
where the allocation presented to the caller may not be aligned, but
the region plus guards is.

But disabling it entirely for these regions is still perfectly fine
with me, especially if the remove ACPI reclaim memory from the set.
Heap guard is a hardening feature, and if the implementation is too
complex to reason about comfortably, I don't think we can confidently
rely on it.

And as far as the OS is concerned: with the MAT, the runtime DXEs are
mapped in a way where the read-only regions are interleaved with the
read-write regions, and the holes in between are not mapped at all (at
least on Linux). IOW, there is some implicit guarding going on
already.


> >> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> >> index a2cd83345f5b..884734aff592 100644
> >> --- a/MdeModulePkg/MdeModulePkg.dec
> >> +++ b/MdeModulePkg/MdeModulePkg.dec
> >> @@ -1027,6 +1027,11 @@ [PcdsFixedAtBuild]
> >> # free pages for all of them. The page allocation for the type related 
> >> to
> >> # cleared bits keeps the same as ususal.
> >> #
> >> +  # The heap guard system only supports guarding EfiRuntimeServicesCode, 
> >> EfiRuntimeServicesData,
> >> +  # EfiACPIReclaimMemory, and EfiACPIMemoryNVS memory types for systems 
> >> that have
> >
> > I looked at the EFI spec again, and EfiACPIReclaimMemory is not
> > actually listed as a memory type that has this 64k alignment
> > requirement. This makes sense, given that this memory type has no
> > significance to the firmware itself, only to the OS. OTOH, reserved
> > memory does appear there.
> >
> > So I suggest we fix that first, and then drop any mention of
> > EfiACPIReclaimMemory from this patch. At least we'll have heap guard
> > coverage for ACPI table allocations on arm64 going forward.
> >
> > The logic in question was added in 2007 in commit 28a00297189c, so
> > this was probably the rule on Itanium, but that support is long gone.
> >
>
> Thanks for looking this up. I'll update either this patch or the unsent
> patch I have depending on the direction we go.
>

Let's go with this approach.


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

[edk2-devel] [PATCH] IntelFsp2Pkg: Fsp 2.x Changes

[cbduggap]

Changes to support spec changes

1. Remove usage of Pcd.
2. Change code to validate the Temporary Ram size input.
3. Consume the input saved in YMM Register

Cc: Sai Chaganty 
Cc: Nate DeSimone 
Cc: Chiu Chasel 


Signed-off-by: cbduggap 
---
 .../FspSecCore/Ia32/Fsp24ApiEntryM.nasm   |   1 -
 .../FspSecCore/Ia32/FspApiEntryM.nasm |   1 -
 .../FspSecCore/Ia32/FspApiEntryT.nasm | 110 --
 .../FspSecCore/Ia32/SaveRestoreSseNasm.inc|  11 ++
 IntelFsp2Pkg/FspSecCore/SecFsp.c  |   1 -
 IntelFsp2Pkg/FspSecCore/SecFspApiChk.c|   4 +-
 IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm |  69 +--
 IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm|   6 +-
 IntelFsp2Pkg/Include/FspEas/FspApi.h  |   5 +-
 .../Include/SaveRestoreSseAvxNasm.inc |  21 
 .../SecRamInitData.c  |   3 +-
 11 files changed, 175 insertions(+), 57 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
index 15f8ecea83..5fa5c03569 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
@@ -11,7 +11,6 @@
 ; Following are fixed PCDs
 ;
 extern   ASM_PFX(PcdGet32(PcdTemporaryRamBase))
-extern   ASM_PFX(PcdGet32(PcdTemporaryRamSize))
 extern   ASM_PFX(PcdGet32(PcdFspTemporaryRamSize))
 extern   ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage))
 
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
index 61ab4612a3..861cce4d01 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
@@ -11,7 +11,6 @@
 ; Following are fixed PCDs
 ;
 extern   ASM_PFX(PcdGet32(PcdTemporaryRamBase))
-extern   ASM_PFX(PcdGet32(PcdTemporaryRamSize))
 extern   ASM_PFX(PcdGet32(PcdFspTemporaryRamSize))
 extern   ASM_PFX(PcdGet8 (PcdFspHeapSizePercentage))
 
diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
index 900126b93b..5fca46ca7a 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
@@ -109,7 +109,8 @@ struc LoadMicrocodeParamsFsp24
 .FsptArchReserved:resb3
 .FsptArchLength:  resd1
 .FspDebugHandler  resq1
-.FsptArchUpd: resd4
+.FspTemporaryRamSize: resd1  ; Supported only if ArchRevison is >= 
3
+.FsptArchUpd: resd3
 ; }
 ; FSPT_CORE_UPD {
 .MicrocodeCodeAddr:   resq1
@@ -178,29 +179,6 @@ endstruc
   jmp ebp   ; restore EIP from EBP
 %endmacro
 
-;
-; Load UPD region pointer in ECX
-;
-global ASM_PFX(LoadUpdPointerToECX)
-ASM_PFX(LoadUpdPointerToECX):
-  ;
-  ; esp + 4 is input UPD parameter
-  ; If esp + 4 is NULL the default UPD should be used
-  ; ecx will be the UPD region that should be used
-  ;
-  mov   ecx, dword [esp + 4]
-  cmp   ecx, 0
-  jnz   ParamValid
-
-  ;
-  ; Fall back to default UPD region
-  ;
-  CALL_EDI  ASM_PFX(AsmGetFspInfoHeaderNoStack)
-  mov   ecx, DWORD [eax + 01Ch]  ; Read FsptImageBaseAddress
-  add   ecx, DWORD [eax + 024h]  ; Get Cfg Region base address = 
FsptImageBaseAddress + CfgRegionOffset
-ParamValid:
-  RET_EBP
-
 ;
 ; @todo: The strong/weak implementation does not work.
 ;This needs to be reviewed later.
@@ -267,7 +245,7 @@ ASM_PFX(LoadMicrocodeDefault):
cmpbyte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2
jb Fsp20UpdHeader
cmpbyte [esp + LoadMicrocodeParamsFsp22.FsptArchRevision], 2
-   je Fsp24UpdHeader
+   jae Fsp24UpdHeader
jmpFsp22UpdHeader
 
 Fsp20UpdHeader:
@@ -405,7 +383,7 @@ CheckAddress:
cmp   byte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2
jb Fsp20UpdHeader1
cmpbyte [esp + LoadMicrocodeParamsFsp22.FsptArchRevision], 2
-   je Fsp24UpdHeader1;
+   jae Fsp24UpdHeader1;
jmpFsp22UpdHeader1
 
 Fsp20UpdHeader1:
@@ -497,7 +475,8 @@ ASM_PFX(EstablishStackFsp):
   ; Enable FSP STACK
   ;
   mov   esp, DWORD [ASM_PFX(PcdGet32 (PcdTemporaryRamBase))]
-  add   esp, DWORD [ASM_PFX(PcdGet32 (PcdTemporaryRamSize))]
+  LOAD_TEMPORARY_RAM_SIZE eax
+  add   esp, eax
 
   push  DATA_LEN_OF_MCUD ; Size of the data region
   push  4455434Dh; Signature of the  data region 'MCUD'
@@ -506,7 +485,7 @@ ASM_PFX(EstablishStackFsp):
   cmp   byte [edx + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2
   jbFsp20UpdHeader2
   cmp   byte [esp + LoadMicrocodeParamsFsp22.FsptArchRevision], 2
-  jeFsp24UpdHeader2
+  jaeFsp24UpdHeader2
   jmp   Fsp22UpdHeader2
 
 Fsp20UpdHeader2:
@@ -554,12 +533,13 @@ ContinueAfterUpdPush:
   ;
   ; Set ECX/EDX to the BootLoader temporary memory range
   ;
-  mov   ecx,  [ASM_PFX(PcdGet32 (PcdTemporaryRamBase)

Re: [edk2-devel] [PATCH v3 0/7] EDK2-TEST TCG MOR Tests

[Stuart Yoder]

On 2/14/24 9:30 PM, Abhi Singh wrote:

Updates after feedback from Stuart 

-add tests that checks if MOR variable can be deleted if
  MORLOCK is in a lock state.
-fix some grammar/spelling/capitalization errors.
-relaxed some tests to work with current edk2 implementation.
-corrected incorrectly listed test guids in SCT Spec
-removed internal Change-id on commit messages

Patch series:
These tests support platform firmware that implement
MemoryOverwriteRequestControl & MemoryOverwriteRequestControlLock
UEFI variables in accordance with TCG PC Platform Reset Attack
Mitigation Specification.

The first 6/7 patches are split according to the six sections
documented in the SCT spec document referenced below.

SCT spec: https://bugzilla.tianocore.org/show_bug.cgi?id=4374

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

GitHub: https://github.com/aabhi64/edk2-test/tree/tcgmortest_v3

Cc: G Edhaya Chandran 
Cc: Barton Gao 
Cc: Carolyn Gjertsen 
Cc: Stuart Yoder 
Cc: Sunny Wang 

Abhi Singh (6):
   uefi-sct/SctPkg: TCG MOR SetVariable Test
   uefi-sct/SctPkg: TCG MORLOCK SetVariable Test
   uefi-sct/SctPkg: TCG MORLOCK Unlocked State Test
   uefi-sct/SctPkg: TCG MORLOCK Locked No Key State Test
   uefi-sct/SctPkg: TCG MORLOCK Locked with Key State Test
   uefi-sct/SctPkg: TCG MOR & MORLOCK tests

Abhi.Singh (1):
   uefi-sct/SctPkg: TCG Platform Reset Check Test


Reviewed-by: Stuart Yoder 


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

Re: [edk2-devel] 回复: [edk2-stable202402] [PATCH v5 1/1] MdeModulePkg/AcpiTableDxe: Prefer xDSDT over DSDT when installing tables

[Leif Lindholm]

Excellent, thank you.
And it will still go in within the next few weeks, just not before we 
make the stable tag.


/
Leif

On 2024-02-15 10:40, Dhaval Sharma wrote:
For me it is not impacting a production system so I can wait a cycle 
more. @Liming Gao  I will send out the 
PR with your rb tag.


On Thu, Feb 15, 2024 at 3:26 PM Leif Lindholm > wrote:


Hi Liming,

On 2024-02-15 01:41, gaoliming via groups.io  wrote:
 > Hi, all
 >   This patch was reviewed before soft feature freeze. I would
like to merge
 > it for this stable tag. If you have any comments, please reply
this mail.

I agree this is a bugfix, but the criterion for hard freeze is supposed
to be *critical* bugfix. By definition this is a very invasive change
for systems where it has any effect. So I would feel more
comfortable if
it had more time before going into a stable tag.

Dhaval, how critical is this fix for you? Are you OK for it to go in
after stable tag?

Regards,

Leif


 > Thanks
 > Liming
 >> -邮件原件-
 >> 发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 gaoliming via
 >> groups.io 
 >> 发送时间: 2024年1月30日 9:21
 >> 收件人: devel@edk2.groups.io ;
dha...@rivosinc.com 
 >> 抄送: zhiguang@intel.com ;
dandan...@intel.com ;
 >> pedro.falc...@gmail.com ;
chasel.c...@intel.com 
 >> 主题: 回复: [edk2-devel] [PATCH v5 1/1] MdeModulePkg/AcpiTableDxe:
 >> Prefer xDSDT over DSDT when installing tables
 >>
 >> This version is good to me. Reviewed-by: Liming Gao
 >> mailto:gaolim...@byosoft.com.cn>>
 >>
 >>> -邮件原件-
 >>> 发件人: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> 代表 Dhaval
 >>> Sharma
 >>> 发送时间: 2024年1月28日 21:39
 >>> 收件人: devel@edk2.groups.io 
 >>> 抄送: gaolim...@byosoft.com.cn
; zhiguang@intel.com
;
 >>> dandan...@intel.com ;
pedro.falc...@gmail.com ;
chasel.c...@intel.com 
 >>> 主题: [edk2-devel] [PATCH v5 1/1] MdeModulePkg/AcpiTableDxe: Prefer
 >>> xDSDT over DSDT when installing tables
 >>>
 >>> As per ACPI Spec 6.5+ Table 5-9 if xDSDT is available,
 >>> it should be used first. Handle required flow when xDSDT
 >>> is absent or present.
 >>>
 >>> Test: Tested on RISCV64 Qemu platform with xDSDT and booted to
 >>> linux kernel.
 >>>
 >>> Cc: Liming Gao 
 >>> Cc: Zhiguang Liu 
 >>> Cc: Dandan Bi 
 >>> Cc: Pedro Falcato 
 >>> Cc: devel@edk2.groups.io 
 >>> Signed-off-by: Dhaval Sharma 
 >>> Acked-by: Chasel Chiu 
 >>> ---
 >>>
 >>> Notes:
 >>>      v5:
 >>>      - If DSDT is not found, throw error and continue to build
other
 > tables
 >>>      v4:
 >>>      - Fix typos and commit message adding more clarity to
patch subject
 >>>      v3:
 >>>      - Added description of ACPI spec clarification based on
which this
 >> patch is
 >>> created
 >>>      - Optimizing if-else flow
 >>>      v2:
 >>>      - Added proper indentation for else if
 >>>
 >>>   MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 24
 >>> ++--
 >>>   1 file changed, 17 insertions(+), 7 deletions(-)
 >>>
 >>> diff --git
 >> a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
 >>> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
 >>> index e09bc9b704f5..3879e10b3349 100644
 >>> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
 >>> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
 >>> @@ -1892,14 +1892,24 @@ InstallAcpiTableFromHob (
 >>>             }
 >>>           }
 >>>
 >>> -        if (((EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE
 >>> *)ChildTable)->Dsdt != 0) {
 >>> +        //
 >>> +        // First check if xDSDT is available, as 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) {
 >>>             T

Re: [edk2-devel] [PATCH 1/1] SbsaQemu: AcpiDxe: Create SRAT table at runtime

[Marcin Juszkiewicz]

W dniu 31.01.2024 o 2:15 PM, Xiong Yining pisze:

Add support to create SRAT(System resource affinity table) for
sbsa platform at runtime.

Signed-off-by: Xiong Yining 
Signed-off-by: Chen Baozi 
---
  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h | 27 ++
  .../Include/Library/SbsaQemuHardwareInfoLib.h | 11 +++
  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 92 +++
  .../SbsaQemuHardwareInfoLib.c | 36 
  4 files changed, 166 insertions(+)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
index 7595df4c8a2d..b8c29b803b81 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
@@ -63,4 +63,31 @@ typedef struct {
  



++#define SBSAQEMU_ACPI_GICC_AFFINITY_STRUCTURE_INIT(   
   \
+  ProximtyDomain, ACPIProcessorUID, Flags, ClockDomain)
   \


s/ProximtyDomain/ProximityDomain/ please

Applied it on top of 20240131120507.2829117-1-xiongyining1...@phytium.com.cn
series ("get rid of DeviceTree from SbsaQemu v5" one).

Code boots but only memory from the first node is listed by both EDK2 and Linux.

QEMU arguments:
-smp 4,sockets=4,maxcpus=4
-m 4G,slots=2,maxmem=5G
-object memory-backend-ram,size=1G,id=m0
-object memory-backend-ram,size=3G,id=m1
-numa node,nodeid=0,cpus=0-1,memdev=m0
-numa node,nodeid=1,cpus=2,memdev=m1
-numa node,nodeid=2,cpus=3

EDK2 memmap:

Type   StartEnd  # Pages  Attributes
Available  0100-01003841AFFF 0003841B 000E
LoaderCode 01003841B000-0100384F 00E5 000E
RT_Code01003850-01003857 0080 800E
RT_Data01003858-01003861 00A0 800E
RT_Code01003862-01003866 0050 800E
ACPI_Recl  01003867-0100386D 0070 000E
RT_Code0100386E-01003872 0050 800E
Available  01003873-01003A00DFFF 18DE 000E
BS_Data01003A00E000-01003A02BFFF 001E 000E
Available  01003A02C000-01003A039FFF 000E 000E
BS_Data01003A03A000-01003A056FFF 001D 000E
Available  01003A057000-01003A057FFF 0001 000E
BS_Data01003A058000-01003B623FFF 15CC 000E
Available  01003B624000-01003B7D3FFF 01B0 000E
BS_Code01003B7D4000-01003BBF 042C 000E
RT_Code01003BC0-01003BD8 0190 800E
RT_Data01003BD9-01003BFD 0250 800E
Available  01003BFE-01003BFFEFFF 001F 000E
BS_Data01003BFFF000-01003C01 0021 000E
Available  01003C02-01003F7D4FFF 37B5 000E
BS_Data01003F7D5000-01003F7F5FFF 0021 000E
BS_Code01003F7F6000-01003F83CFFF 0047 000E
BS_Data01003F83D000-01003FFD9FFF 079D 000E
BS_Code01003FFDA000-01003FFF7FFF 001E 000E
BS_Data01003FFF8000-01003FFF 0008 000E
MMIO   1000-106B 06C0 8001
MMIO   6001-60010FFF 0001 8001
 
  Reserved  :  0 Pages (0 Bytes)

  LoaderCode:229 Pages (937,984 Bytes)
  LoaderData:  0 Pages (0 Bytes)
  BS_Code   :  1,169 Pages (4,788,224 Bytes)
  BS_Data   :  7,662 Pages (31,383,552 Bytes)
  RT_Code   :688 Pages (2,818,048 Bytes)
  RT_Data   :752 Pages (3,080,192 Bytes)
  ACPI_Recl :112 Pages (458,752 Bytes)
  ACPI_NVS  :  0 Pages (0 Bytes)
  MMIO  :  1,729 Pages (7,081,984 Bytes)
  MMIO_Port :  0 Pages (0 Bytes)
  PalCode   :  0 Pages (0 Bytes)
  Available :251,532 Pages (1,030,275,072 Bytes)
  Persistent:  0 Pages (0 Bytes)
  --
Total Memory:  1,024 MB (1,073,741,824 Bytes)


SRAT table shows both memory nodes:

 --- SRAT Table ---

Address  : 0x100386DFD18
Length   : 200
  
 : 53 52 41 54 C8 00 00 00 - 03 48 4C 49 4E 41 52 4F   SRAT.HLINARO

0010 : 53 42 53 41 51 45 4D 55 - 10 08 20 20 4C 4E 52 4F   SBSAQEMU..  LNRO
0020 : 01 00 00 00 01 00 00 00 - 00 00 00 00 00 00 00 00   
0030 : 01 28 01 00 00 00 00 00 - 00 00 00 40 00 01 00 00   .(.@
0040 

Re: [edk2-devel] [PATCH v2 4/4] NetworkPkg: : Updating SecurityFixes.yaml

[Rebecca Cran]

I noticed the advisory at 
https://github.com/advisories/GHSA-h9v6-q439-p7j2 is labeled "Unreviewed".
Should it be updated, and should the 'package', 'affected' and 'patched' 
fields be updated?


--
Rebecca Cran


On 2/13/2024 11:46 AM, Doug Flick via groups.io wrote:

From: Doug Flick 

This captures the related security change for Dhcp6Dxe that is related
to CVE-2023-45229

Cc: Saloni Kasbekar 
Cc: Zachary Clark-williams 

Signed-off-by: Doug Flick [MSFT] 
---
  NetworkPkg/SecurityFixes.yaml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/NetworkPkg/SecurityFixes.yaml b/NetworkPkg/SecurityFixes.yaml
index 7e900483fec5..fa42025e0d82 100644
--- a/NetworkPkg/SecurityFixes.yaml
+++ b/NetworkPkg/SecurityFixes.yaml
@@ -8,6 +8,7 @@ CVE_2023_45229:
commit_titles:

  - "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Patch"

  - "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Unit Tests"

+- "NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch"

cve: CVE-2023-45229

date_reported: 2023-08-28 13:56 UTC

description: "Bug 01 - edk2/NetworkPkg: Out-of-bounds read when processing 
IA_NA/IA_TA options in a DHCPv6 Advertise message"





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

Re: [edk2-devel] mail-archive.com (secondary archive) feed resumed

[Rebecca Cran]

On 2/15/2024 4:28 AM, Laszlo Ersek wrote:

mail-archive.com is an important secondary archive because it offers
message-id-based lookup. "git am --message-id" captures patch email
message IDs in the commit messages, and then those can be used for
locating the related review discussion in the archive.


Similarly, I've been running an public-inbox instance at 
https://openfw.io/edk2-devel/ for a while.



--
Rebecca Cran



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

Re: [edk2-devel] The API in BaseCryptLib can't seed the pseudorandom number generator properly

[Laszlo Ersek]

On 2/15/24 12:09, eddie wang wrote:
> Hi Laszlo,
> Thanks for your reply. How can I enable the DEBUGs at RandomSeed() ? Or
> any suggesting information that I can provide?

Sorry, upon a closer look, I see you had already narrowed it down to
RAND_seed() and RAND_status(), which are direct OpenSSL APIs. So my
suggestion would amount to adding DEBUGs to OpenSSL, such as to
RAND_seed() in
"CryptoPkg/Library/OpensslLib/openssl/crypto/rand/rand_lib.c".

But, I think you may be able to do just that.
"CryptoPkg/Library/Include/CrtLibSupport.h" already includes
, and DebugLib is listed under [LibraryClasses] in each
instance of OpensslLib. So if you modify your
"CryptoPkg/Library/OpensslLib/openssl" submodule directory tree locally,
with the following patch:

| diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
| index 0fcf4fe3bc1e..e5f105268f52 100644
| --- a/crypto/rand/rand_lib.c
| +++ b/crypto/rand/rand_lib.c
| @@ -257,6 +257,8 @@ void RAND_seed(const void *buf, int num)
|  drbg = RAND_get0_primary(NULL);
|  if (drbg != NULL && num > 0)
|  EVP_RAND_reseed(drbg, 0, NULL, 0, buf, num);
| +
| +DEBUG ((DEBUG_INFO, "%a: hello\n", __func__));
|  }
|
|  void RAND_add(const void *buf, int num, double randomness)

then you should get usable debug messages -- at least it builds for me.

Inserting DEBUGs like this (over multiple rounds of testing / narrowing)
should lead you to the exact location that is responsible for the
initialization failure.

You mention you have encountered the problem with a UEFI application.
That is relevant for choosing your DebugLib instance. If you already
have a function DebugLib instance for your platform (logging to the
serial port, for example), then just use that.

Otherwise, consider building your UEFI application with a module scope
override in the DSC file, one that resolves DebugLib to

  MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf

or

  MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf

These will send DEBUG messages to the UEFI console or standard error
devices, respectively.

hth
Laszlo

> Laszlo Ersek mailto:ler...@redhat.com>> 於 2024年2月
> 8日 週四 上午5:03寫道：
>
> On 2/6/24 08:00, eddie wang wrote:
> > Hi all,
> > We had an UEFI application that used the EDK2(2023/12/05), and  we
> would
> > like to take advantage of the services in BaseCryptLib .However,
> the API
> > in CryptPkg "*RandomSeed()*"(X64, in CryptRandTsc.c) always returned
> > false because of  the pseudorandom number generator set up failed.
> I am
> > not sure this issue is from the *openssl configuration in
> OpensslLib(we
> > use the default configuration)* or is from the *openssl 3.0.9*.
> >
> > Is there any comments about this issue?
>
> Can you narrow it down by inserting DEBUGs starting at RandomSeed()
> [CryptoPkg/Library/BaseCryptLib/Rand/CryptRandTsc.c], and then digging
> down as necessary?
>
> Laszlo
>
>
>
> 
>
>



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

[edk2-devel] mail-archive.com (secondary archive) feed resumed

[Laszlo Ersek]

Hi,

you may have noticed that the secondary archive at
 stopped receiving
messages after 28 Jan 2024. The reason is described here:

  https://groups.io/static/reverifyfaq

A message about the reverification ("Update on Groups.io Reverification
Process") had also been sent to edk2 list owners and moderators, but
we've not done anything about it (to my knowledge) thus far, as the
reverification is a sensible thing for groups.io to do, and affected
members (humans) are indeed expected to re-verify.

However, that doesn't apply to the mail-archive.com bot.

It turns out that mail-archive.com is a subscriber to *several*
groups.io mailing lists. The "Action Required: Confirm Your Groups.io
Membership to Continue Receiving Group Emails" email sent out by
groups.io can be seen on the mail-archive.com "confirmation requests"
page:

  https://www.mail-archive.com/faq.html#newlist

-->

  https://www.mail-archive.com/archive@mail-archive.com/maillist.html

-->

  https://www.mail-archive.com/archive@mail-archive.com/msg56864.html
  https://www.mail-archive.com/archive@mail-archive.com/msg56866.html
  https://www.mail-archive.com/archive@mail-archive.com/msg56870.html
  https://www.mail-archive.com/archive@mail-archive.com/msg56871.html

(basically, four instances of the call upon verification), and each
instance lists several groups.io mailing lists (which the
mail-archive.com robot is a member of).

These emails offer two methods for re-verifying:

- responding to the message

- logging in to the web UI

Neither of those options work; we can't "respond" as the
mail-archive.com bot, and also cannot login on its behalf (it has no
password for the webui in the first place, and my attempt to ask for a
new password *also* failed -- the password reset email didn't reach the
"confirmation requests" list linked above).

So my only option was to forcibly delete the mail-archive.com bot from
the edk2-devel subscriber list, and then to re-invite it. ("Direct add"
without invitation is a paid-only feature on groups.io.) The invitation
email *did* show up on the confirmation requests list:

  https://www.mail-archive.com/archive@mail-archive.com/maillist.html

-->

  https://www.mail-archive.com/archive@mail-archive.com/msg56894.html

I clicked the invite confirmation link from that message, so now the
feed to mail-archive.com should be resumed. (Interestingly, on the
groups.io WebUI, in the subscriber list, the mail-archive.com robot is
not just listed as a brand new member at this point, but as a member
whose re-verification succeeded: it carries the "RS" badge.)

Thus, the secondary archive will have a 2 week long gap, essentially the
first half of February.

mail-archive.com is an important secondary archive because it offers
message-id-based lookup. "git am --message-id" captures patch email
message IDs in the commit messages, and then those can be used for
locating the related review discussion in the archive.

The URL format is the following -- using commit 0e9b124f9c67
("UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu
topology", 2024-01-31) as an example:

git show -s 0e9b124f9c67 | grep -i message-id

Message-Id: 


-->

  
d93822d37fd25dafd32795758cf47263b432e102.1705549445.git.AbdulLateef.Attar@amd.com">http://mid.mail-archive.com/d93822d37fd25dafd32795758cf47263b432e102.1705549445.git.AbdulLateef.Attar@amd.com
  
^
 ||
 |msgid of the patch email, from the commit
 |
 special FQDN for the message-id-based lookup

If you paste this URL into your browser, mail-archive.com performs the
lookup, and redirects to

  https://www.mail-archive.com/devel@edk2.groups.io/msg68931.html

You can see the original thread, the thread hierarchy at the bottom of
the page, and at the very bottom (on dark background), the message-id
itself is displayed in light gray, too.

Another nice benefit is that you can construct archive links immediately
after mailing out a patch, with git-send-email. The git-send-email log
contains the message identifier; you can paste that immediately into the
URL (and save the URL in a bugzilla comment, for example), without
having to check anything on the web at first. In fact, such links may
not work immediately (it takes mail-archive.com some time to index the
new message), but the link will *start* working *eventually*. IOW, you
don't depend on the archive to assign a unique identifier to your
posting (such as a groups.io message number); your local git tooling
will generate the message-id, and then mail-archive.com will just adhere
to it. I wish groups.io offered a similar search capability.

Laszlo



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

Re: [edk2-devel] The API in BaseCryptLib can't seed the pseudorandom number generator properly

[eddie wang]

Hi Laszlo,
Thanks for your reply. How can I enable the DEBUGs at RandomSeed() ? Or any
suggesting information that I can provide?

BR,
Eddie

Laszlo Ersek  於 2024年2月8日 週四 上午5:03寫道：

> On 2/6/24 08:00, eddie wang wrote:
> > Hi all,
> > We had an UEFI application that used the EDK2(2023/12/05), and  we would
> > like to take advantage of the services in BaseCryptLib .However, the API
> > in CryptPkg "*RandomSeed()*"(X64, in CryptRandTsc.c) always returned
> > false because of  the pseudorandom number generator set up failed. I am
> > not sure this issue is from the *openssl configuration in OpensslLib(we
> > use the default configuration)* or is from the *openssl 3.0.9*.
> >
> > Is there any comments about this issue?
>
> Can you narrow it down by inserting DEBUGs starting at RandomSeed()
> [CryptoPkg/Library/BaseCryptLib/Rand/CryptRandTsc.c], and then digging
> down as necessary?
>
> Laszlo
>
>
>
> 
>
>
>


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