Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Gerd Hoffmann]

On Wed, Feb 28, 2024 at 05:09:32AM +0100, Laszlo Ersek wrote:
> On 2/22/24 12:54, Gerd Hoffmann wrote:
> > Move code to clear the page tables to a nasm macro.
> > No functional change.
> > 
> > Signed-off-by: Gerd Hoffmann 
> > ---
> >  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ---
> >  1 file changed, 19 insertions(+), 16 deletions(-)
> > 
> > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
> > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > index 6fec6f2beeea..378ba2feeb4f 100644
> > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> > @@ -45,6 +45,24 @@ BITS32
> >  %define TDX_BSP 1
> >  %define TDX_AP  2
> >  
> > +;
> > +; For OVMF, build some initial page tables at
> > +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
> > +;
> > +; This range should match with PcdOvmfSecPageTablesSize which is
> > +; declared in the FDF files.
> > +;
> > +; At the end of PEI, the pages tables will be rebuilt into a
> > +; more permanent location by DxeIpl.
> > +;
> > +%macro ClearOvmfPageTables 0
> > +mov ecx, 6 * 0x1000 / 4
> > +xor eax, eax
> > +.clearPageTablesMemoryLoop:
> > +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
> > +loop.clearPageTablesMemoryLoop
> > +%endmacro
> > +
> >  ;
> >  ; Modified:  EAX, EBX, ECX, EDX
> >  ;
> 
> Ah, this made me read up on local labels:
> 
> https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9
> 
> Should we rather call the label
> 
>   ..@clearPageTablesMemoryLoop
> 
> ?

I've tried that and something (which I don't remember) didn't work as
expected.  Given that each branch which uses that macro will have a jump
label anyway (so the local label is expanded to something like
TdxBspInit.clearPageTablesMemoryLoop) I've figured this is good enough

take care,
  Gerd



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

Re: [edk2-devel] Tagging and releases of edk2-basetools

[Laszlo Ersek]

On 2/28/24 02:15, Rebecca Cran wrote:
> edk2-basetools is finally fixed and releases can once again be published
> to PyPI.
> 
> However, in moving from setup.py to pyproject.toml the process has
> changed, and Joey suggested the old way might have been chosen
> deliberately to be different from edk2-pytool-library and
> edk2-pytool-extensions.
> 
> 
> Previously it fetched the list of releases from PyPI and incremented the
> version number before publishing a new version, while it now depends on
> the git repo being tagged.
> 
> 
> Should I work on migrating the old code and figuring out how to make it
> work with pyproject.toml?
> 
> 

Adding Gerd, because of
 -- "[Feature]:
please add release tags to the repository".

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116095): https://edk2.groups.io/g/devel/message/116095
Mute This Topic: https://groups.io/mt/104615885/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 3/3] OvmfPkg/TdxDxe: Clear the registers before tdcall

[sunceping]

On Tuesday, February 27, 2024 8:26 PM Yamahata, Isaku wrote:
> > +%macro tdcall_regs_preamble 2
> > +mov rax, %1
> > +
> > +xor rcx, rcx
> > +mov ecx, %2
> > +
> > +; R10 = 0 (standard TDVMCALL)
> > +
> > +xor r10d, r10d
> > +
> > +; Zero out unused (for standard TDVMCALL) registers to avoid leaking
> > +; secrets to the VMM.
> > +
> > +xor esi, esi
> > +xor edi, edi
> > +
> > +xor edx, edx
> > +xor ebp, ebp
> > +xor r8d, r8d
> > +xor r9d, r9d
> > +xor r14, r14
> > +xor r15, r15
> 
> We can just clear the corresponding bit of TDVMCALL_EXPOSE_REGS_MASK in
> addition to RBP.
> Same to 1/3 and 3/3. We can eliminate tdcall_regs_postamble.
> Any reason to bother to zero those registers and pass them to VMM?
>
Zero out these registers to avoid leaking secrets to the VMM.
There are also some registers (e.g., r10, r14.. etc.) are output operands 
and should be cleared.
The tdcall_regs_preamble  was already using in the TdVmcall.nasm and 
TdVmcallCpuid.nasm .
For the ApRunLoop.nasm , it is fixed now.

Thanks
Ceping



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

[edk2-devel] [PATCH v4] 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 | 70 ++---
 .../FspSecCore/Ia32/SaveRestoreSseNasm.inc| 11 +++
 IntelFsp2Pkg/FspSecCore/SecFsp.c  | 17 +++-
 IntelFsp2Pkg/FspSecCore/SecFspApiChk.c|  4 +-
 IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 78 +++
 IntelFsp2Pkg/Include/FspEas/FspApi.h  |  5 +-
 .../Include/SaveRestoreSseAvxNasm.inc | 21 +
 IntelFsp2Pkg/IntelFsp2Pkg.dec |  5 ++
 .../SecRamInitData.c  |  3 +-
 14 files changed, 185 insertions(+), 36 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..020599ba89 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
@@ -16,6 +16,7 @@
 extern   ASM_PFX(PcdGet32 (PcdTemporaryRamBase))
 extern   ASM_PFX(PcdGet32 (PcdTemporaryRamSize))
 extern   ASM_PFX(PcdGet32 (PcdFspReservedBufferSize))
+extern   ASM_PFX(PcdGet32 (PcdGlobalDat

Re: [edk2-devel] [PATCH v2 14/23] Ovmfpkg/CcSvsmLib: Create CcSvsmLib to handle SVSM related services

[Gerd Hoffmann]

> +/**
> +  Perform a native PVALIDATE operation for the page ranges specified.
> +
> +  Validate or rescind the validation of the specified pages.
> +
> +  @param[in]   Info   Pointer to a page state change structure
> +
> +**/
> +STATIC
> +VOID
> +BasePvalidate (
> +  IN  SNP_PAGE_STATE_CHANGE_INFO  *Info
> +  )

This is not mentioned in the commit message.

Looks like you are moving or copying code from BaseMemEncryptSevLib.

Moving code is best done with a patch doing the move only, without other
functional changes.  If that can't be done easily this should explained
in the commit message.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116098): https://edk2.groups.io/g/devel/message/116098
Mute This Topic: https://groups.io/mt/104512963/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 15/23] UefiCpuPkg/MpInitLib: Use CcSvsmSnpVmsaRmpAdjust() to set/clear VMSA

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:54AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The RMPADJUST instruction is used to change the VMSA attribute of a page,
> but the VMSA attribute can only be changed when running at VMPL0. To
> prepare for running at a less priviledged VMPL, use the CcSvsmLib library
> API to perform the RMPADJUST. The CcSvsmLib library will perform the
> proper operation on behalf of the caller.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116099): https://edk2.groups.io/g/devel/message/116099
Mute This Topic: https://groups.io/mt/104512965/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 17/23] OvmfPkg: Create a calling area used to communicate with the SVSM

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:56AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> An SVSM requires a calling area page whose address (CAA) is used by the
> SVSM to communicate and process the SVSM request.
> 
> Add a pre-defined page area to the OvmfPkg and AmdSev packages and define
> corresponding PCDs used to communicate the location and size of the area.
> Keep the AmdSev package in sync with the OvmfPkg and adjust the AmdSev
> launch and hash area memory locations.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116100): https://edk2.groups.io/g/devel/message/116100
Mute This Topic: https://groups.io/mt/104512971/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 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Laszlo Ersek]

On 2/28/24 03:27, Zhiguang Liu wrote:
> In last patch, we add code support to unregister SMI handler inside
> itself. However, the code doesn't support unregister SMI handler
> insider other SMI handler. While this is not a must-have usage.
> So add check to disallow unregister SMI handler in other SMI handler.
> 
> Cc: Liming Gao 
> Cc: Jiaxin Wu 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Zhiguang Liu 
> ---
>  MdeModulePkg/Core/PiSmmCore/Smi.c | 32 +++
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c 
> b/MdeModulePkg/Core/PiSmmCore/Smi.c
> index 3489c130fd..1bfbc635fc 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> @@ -8,7 +8,8 @@
>  
>  #include "PiSmmCore.h"
>  
> -LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
> +SMI_HANDLER  *gCurrentSmiHandler = NULL;
> +LIST_ENTRY   mSmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
> (mSmiEntryList);
>  
>  SMI_ENTRY  mRootSmiEntry = {
>SMI_ENTRY_SIGNATURE,
> @@ -142,13 +143,18 @@ SmiManage (
>  // Link points to may be freed if unregister SMI handler.
>  //
>  Link = Link->ForwardLink;
> -
> -Status = SmiHandler->Handler (
> -   (EFI_HANDLE)SmiHandler,
> -   Context,
> -   CommBuffer,
> -   CommBufferSize
> -   );
> +//
> +// Assign gCurrentSmiHandle before calling the SMI handler and
> +// set to NULL when it returns.
> +//
> +gCurrentSmiHandler = SmiHandler;
> +Status = SmiHandler->Handler (
> +   (EFI_HANDLE)SmiHandler,
> +   Context,
> +   CommBuffer,
> +   CommBufferSize
> +   );
> +gCurrentSmiHandler = NULL;
>  
>  switch (Status) {
>case EFI_INTERRUPT_PENDING:
> @@ -328,6 +334,16 @@ SmiHandlerUnRegister (
>  return EFI_INVALID_PARAMETER;
>}
>  
> +  //
> +  // Check if unregister SMI handler inside a SMI Handler
> +  //
> +  if (gCurrentSmiHandler != NULL) {
> +//
> +// Only allow to unregister SMI Handler inside itself.
> +//
> +ASSERT (gCurrentSmiHandler == SmiHandler);
> +  }
> +
>SmiEntry = SmiHandler->SmiEntry;
>  
>RemoveEntryList (&SmiHandler->Link);

(1) Why not:

  if ((gCurrentSmiHandler != NULL) && (gCurrentSmiHandler != SmiHandler)) {
return EFI_INVALID_PARAMETER;
  }

?

(2) Why do we call the new global variable "gCurrentSmiHandler" rather than 
"mCurrentSmiHandler"?

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116101): https://edk2.groups.io/g/devel/message/116101
Mute This Topic: https://groups.io/mt/104616993/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/4] StandaloneMmPkg: Support to unregister MMI handler inside MMI handler

[Laszlo Ersek]

On 2/28/24 03:27, Zhiguang Liu wrote:
> To support unregister MMI handler inside MMI handler itself,
> get next node before MMI handler is executed, since LIST_ENTRY that
> Link points to may be freed if unregister MMI handler in MMI handler
> itself.
> 
> Cc: Liming Gao 
> Cc: Jiaxin Wu 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Signed-off-by: Zhiguang Liu 
> ---
>  StandaloneMmPkg/Core/Mmi.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
> index 0de6fd17fc..c1a1d76e85 100644
> --- a/StandaloneMmPkg/Core/Mmi.c
> +++ b/StandaloneMmPkg/Core/Mmi.c
> @@ -154,9 +154,14 @@ MmiManage (
>  Head = &MmiEntry->MmiHandlers;
>}
>  
> -  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
> +  for (Link = Head->ForwardLink; Link != Head;) {
>  MmiHandler = CR (Link, MMI_HANDLER, Link, MMI_HANDLER_SIGNATURE);
> -
> +//
> +// To support unregister MMI handler inside MMI handler itself,
> +// get next node before handler is executed, since LIST_ENTRY that
> +// Link points to may be freed if unregister MMI handler.
> +//
> +Link   = Link->ForwardLink;
>  Status = MmiHandler->Handler (
> (EFI_HANDLE)MmiHandler,
> Context,

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116102): https://edk2.groups.io/g/devel/message/116102
Mute This Topic: https://groups.io/mt/104616994/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] StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

[Laszlo Ersek]

On 2/28/24 03:27, Zhiguang Liu wrote:
> In last patch, we add code support to unregister MMI handler inside
> itself. However, the code doesn't support unregister MMI handler
> insider other MMI handler. While this is not a must-have usage.
> So add check to disallow unregister MMI handler in other MMI handler.
> 
> Cc: Liming Gao 
> Cc: Jiaxin Wu 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Ray Ni 
> Signed-off-by: Zhiguang Liu 
> ---
>  StandaloneMmPkg/Core/Mmi.c | 35 ++-
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
> index c1a1d76e85..54794c6b3d 100644
> --- a/StandaloneMmPkg/Core/Mmi.c
> +++ b/StandaloneMmPkg/Core/Mmi.c
> @@ -36,8 +36,9 @@ typedef struct {
>MMI_ENTRY *MmiEntry;
>  } MMI_HANDLER;
>  
> -LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
> (mRootMmiHandlerList);
> -LIST_ENTRY  mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
> (mMmiEntryList);
> +LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE 
> (mRootMmiHandlerList);
> +LIST_ENTRY   mMmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE 
> (mMmiEntryList);
> +MMI_HANDLER  *gCurrentMmiHandler = NULL;
>  
>  /**
>Finds the MMI entry for the requested handler type.
> @@ -161,13 +162,19 @@ MmiManage (
>  // get next node before handler is executed, since LIST_ENTRY that
>  // Link points to may be freed if unregister MMI handler.
>  //
> -Link   = Link->ForwardLink;
> -Status = MmiHandler->Handler (
> -   (EFI_HANDLE)MmiHandler,
> -   Context,
> -   CommBuffer,
> -   CommBufferSize
> -   );
> +Link = Link->ForwardLink;
> +//
> +// Assign gCurrentMmiHandle before calling the MMI handler and
> +// set to NULL when it returns.
> +//
> +gCurrentMmiHandler = MmiHandler;
> +Status = MmiHandler->Handler (
> +   (EFI_HANDLE)MmiHandler,
> +   Context,
> +   CommBuffer,
> +   CommBufferSize
> +   );
> +gCurrentMmiHandler = NULL;
>  
>  switch (Status) {
>case EFI_INTERRUPT_PENDING:
> @@ -314,6 +321,16 @@ MmiHandlerUnRegister (
>  return EFI_INVALID_PARAMETER;
>}
>  
> +  //
> +  // Check if unregister MMI handler inside a MMI Handler
> +  //
> +  if (gCurrentMmiHandler != NULL) {
> +//
> +// Only allow to unregister MMI Handler inside itself.
> +//
> +ASSERT (gCurrentMmiHandler == MmiHandler);
> +  }
> +
>MmiEntry = MmiHandler->MmiEntry;
>  
>RemoveEntryList (&MmiHandler->Link);

same comment as for patch#2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116103): https://edk2.groups.io/g/devel/message/116103
Mute This Topic: https://groups.io/mt/104616995/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 16/23] OvmfPkg/BaseMemEncryptSevLib: Use CcSvsmSnpPvalidate() to validate pages

[Gerd Hoffmann]

> -STATIC
> -VOID
> -PvalidateRange (
> -  IN  SNP_PAGE_STATE_CHANGE_INFO  *Info
> -  )
> -{

Ah, here you are completing the code move started in patch #14.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116104): https://edk2.groups.io/g/devel/message/116104
Mute This Topic: https://groups.io/mt/104512967/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 18/23] OvmfPkg/CcSvsmLib: Add support for the SVSM_CORE_PVALIDATE call

[Gerd Hoffmann]

  Hi,

> +// Clear the buffer in prep for creating all new entries
> +SetMem (Caa->SvsmBuffer, sizeof (Caa->SvsmBuffer), 0);

Minor nit: There is a ZeroMem() for this purpose.

Acked-by: Gerd Hoffmann 

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116105): https://edk2.groups.io/g/devel/message/116105
Mute This Topic: https://groups.io/mt/104512972/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 19/23] OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:58AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> Similar to the Page State Change optimization added previously, also take
> into account the possiblity of using the SVSM for PVALIDATE instructions.
> Conditionally adjust the maximum number of entries based on how many
> entries the SVSM calling area can support.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116106): https://edk2.groups.io/g/devel/message/116106
Mute This Topic: https://groups.io/mt/104512975/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 20/23] OvmfPkg/CcSvsmLib: Add support for the SVSM create/delete vCPU calls

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:29:59AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The RMPADJUST instruction is used to alter the VMSA attribute of a page,
> but the VMSA attribute can only be changed when running at VMPL0. When
> an SVSM is present, use the SVSM_CORE_CREATE_VCPU and SVSM_CORE_DELTE_VCPU
> calls to add or remove the VMSA attribute on a page instead of issuing
> the RMPADJUST instruction directly.
> 
> Implement the CcSvsmSnpVmsaRmpAdjust() API to perform the proper operation
> to update the VMSA attribute.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116107): https://edk2.groups.io/g/devel/message/116107
Mute This Topic: https://groups.io/mt/104512977/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 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Zhiguang Liu]

Thanks Laszlo. Both are good comments. I will follow in next version

Thanks
Zhiguang

> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, February 28, 2024 4:45 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Liming Gao ; Wu, Jiaxin
> ; Ni, Ray 
> Subject: Re: [edk2-devel] [PATCH v2 2/4] MdeModulePkg/SMM: Disallow
> unregister SMI handler in other SMI handler
> 
> On 2/28/24 03:27, Zhiguang Liu wrote:
> > In last patch, we add code support to unregister SMI handler inside
> > itself. However, the code doesn't support unregister SMI handler
> > insider other SMI handler. While this is not a must-have usage.
> > So add check to disallow unregister SMI handler in other SMI handler.
> >
> > Cc: Liming Gao 
> > Cc: Jiaxin Wu 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  MdeModulePkg/Core/PiSmmCore/Smi.c | 32
> > +++
> >  1 file changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
> > b/MdeModulePkg/Core/PiSmmCore/Smi.c
> > index 3489c130fd..1bfbc635fc 100644
> > --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> > +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> > @@ -8,7 +8,8 @@
> >
> >  #include "PiSmmCore.h"
> >
> > -LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE
> > (mSmiEntryList);
> > +SMI_HANDLER  *gCurrentSmiHandler = NULL;
> > +LIST_ENTRY   mSmiEntryList   = INITIALIZE_LIST_HEAD_VARIABLE
> (mSmiEntryList);
> >
> >  SMI_ENTRY  mRootSmiEntry = {
> >SMI_ENTRY_SIGNATURE,
> > @@ -142,13 +143,18 @@ SmiManage (
> >  // Link points to may be freed if unregister SMI handler.
> >  //
> >  Link = Link->ForwardLink;
> > -
> > -Status = SmiHandler->Handler (
> > -   (EFI_HANDLE)SmiHandler,
> > -   Context,
> > -   CommBuffer,
> > -   CommBufferSize
> > -   );
> > +//
> > +// Assign gCurrentSmiHandle before calling the SMI handler and
> > +// set to NULL when it returns.
> > +//
> > +gCurrentSmiHandler = SmiHandler;
> > +Status = SmiHandler->Handler (
> > +   (EFI_HANDLE)SmiHandler,
> > +   Context,
> > +   CommBuffer,
> > +   CommBufferSize
> > +   );
> > +gCurrentSmiHandler = NULL;
> >
> >  switch (Status) {
> >case EFI_INTERRUPT_PENDING:
> > @@ -328,6 +334,16 @@ SmiHandlerUnRegister (
> >  return EFI_INVALID_PARAMETER;
> >}
> >
> > +  //
> > +  // Check if unregister SMI handler inside a SMI Handler  //  if
> > + (gCurrentSmiHandler != NULL) {
> > +//
> > +// Only allow to unregister SMI Handler inside itself.
> > +//
> > +ASSERT (gCurrentSmiHandler == SmiHandler);  }
> > +
> >SmiEntry = SmiHandler->SmiEntry;
> >
> >RemoveEntryList (&SmiHandler->Link);
> 
> (1) Why not:
> 
>   if ((gCurrentSmiHandler != NULL) && (gCurrentSmiHandler != SmiHandler)) {
> return EFI_INVALID_PARAMETER;
>   }
> 
> ?
> 
> (2) Why do we call the new global variable "gCurrentSmiHandler" rather than
> "mCurrentSmiHandler"?
> 
> Thanks
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116108): https://edk2.groups.io/g/devel/message/116108
Mute This Topic: https://groups.io/mt/104616993/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 22/23] Ovmfpkg/CcExitLib: Provide SVSM discovery support

[Gerd Hoffmann]

On Thu, Feb 22, 2024 at 11:30:01AM -0600, Tom Lendacky wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654
> 
> The SVSM specification documents an alternative method of discovery for
> the SVSM using a reserved CPUID bit and a reserved MSR.
> 
> For the CPUID support, the #VC handler of an SEV-SNP guest should modify
> the returned value in the EAX register for the 0x801f CPUID function
> by setting bit 28 when an SVSM is present.
> 
> For the MSR support, new reserved MSR 0xc001f000 has been defined. A #VC
> should be generated when accessing this MSR. The #VC handler is expected
> to ignore writes to this MSR and return the physical calling area address
> (CAA) on reads of this MSR.
> 
> Signed-off-by: Tom Lendacky 

Acked-by: Gerd Hoffmann 



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

[edk2-devel] [PATCH] BaseTools: FMMT Skip empty Lines while parsing FMMTConfig.ini

[Ashraf Ali S]

When the FMMTConf.ini file has empty lines then it used to throw errors
GuidTool load error!, this patch is to skip checking for empty lines in
the ini file

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Chen Christine 
Cc: Chaganty Rangasai V 

Signed-off-by: Ashraf Ali 
---
 BaseTools/Source/Python/FMMT/core/GuidTools.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/FMMT/core/GuidTools.py 
b/BaseTools/Source/Python/FMMT/core/GuidTools.py
index f6bdeffa50..f9cfd4ead0 100644
--- a/BaseTools/Source/Python/FMMT/core/GuidTools.py
+++ b/BaseTools/Source/Python/FMMT/core/GuidTools.py
@@ -153,7 +153,7 @@ class GUIDTools:
 config_data = fd.readlines()
 for line in config_data:
 try:
-if not line.startswith("#"):
+if not line.startswith("#") and line.strip():
 guid, short_name, command = line.split()
 new_format_guid = 
struct2stream(ModifyGuidFormat(guid.strip()))
 self.tooldef[new_format_guid] = GUIDTool(
-- 
2.42.0.windows.2



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

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE

[Laszlo Ersek]

On 2/27/24 18:15, Albecki, Mateusz wrote:
> Is the idea to refactor PciSioSerialDxe to extract functions that access
> the HW and wrap it in the SerialPortLib instance? That solution would
> still save us some maintenance cost. However exploring the idea further
> I see following problems:
> 
> 1. Relying on driver binding produces a fairly nice flow: platform
> driver initializes enough platform HW for UART to work -> platform
> driver calls ConnectController -> driver initializes UART. With
> SerialDxe a depex would have to be injected through our instance of
> SerialPortLib to stop the SerialDxe dispatch until platform driver made
> the platform ready.
> 2. I am wondering how it would work in case we want to allow dynamic
> configuration of debug port(basically selecting which UART controller
> would be used). With current solution we can select which one(or which
> ones) will be used by simply installing and connecting corresponding
> handles. With library solution I guess library would have to locate some
> protocols(possibly the same PCI_IO and DEVICE_PATH) that were installed
> by platform driver. It would also need to install notify on those
> protocols installation in case platform wants to add more uarts later on
> in the boot flow.

I think these requirements are *way* out of scope, and too clever, for
producing debug output. I'm now tempted to think that you are actually
best served by your existent separate driver.

SerialDxe and SerialPortLib are ill-suited if your system has multiple
serial ports, not to mention if you want different configurations from
boot to boot.

I think I'm permitted to disagree with a proposed patch without having
to counter-propose an alternative myself (i.e., the burden is on you, to
find an alternative). But, I'll brainstorm one more:

a. I'll assume that you store the debug port config in some non-volatile
UEFI variable.

b. In the PEI phase, have a PEIM turn that variable into a GUID HOB.
(This is doable because PEI can have read-only variable access.) Or
produce that HOB in some other way.

c. In the early platform DXE driver (= debug driver), read the HOB, and
publish a single instance of a new gEdkiiLpssUartDebugProtocolGuid.

d. The protocol should have a single member function,
WriteToAllConfiguredDebugPorts(). The debug driver should replicate the
message that is passed to this API to all ports enabled in the HOB.

e. Introduce a new DebugLib instance that, for DXE_DRIVER modules, has a
DEPEX on gEdkiiLpssUartDebugProtocolGuid:

[Depex.common.DXE_DRIVER]
  gEdkiiLpssUartDebugProtocolGuid

f. The debug ports should never appear in the system as EFI handles
(i.e., neither SerialIo nor PciIo nor DevicePath protocols -- no handles
at all).

g. The debug driver should use PciLib for configuring and accessing the
ports. Configuration includes any necessary "platform configuration" too.

h. If there is any LPSS UART access logic that's worth sharing -- for
maintenance reasons -- between PciSioSerialDxe and the debug driver,
then that should be extracted to new library class, and *two* library
instances (same directory, though). Both instances would nearly be
identical, but they'd have to *internally* abstract away PCI access. The
DXE instance (underlying the debug driver) would use PciLib, the UEFI
instance (underlying PciSioSerialDxe) would use PciIo. The code that
interacted with the LPSS UARTs would be common though.

i. The debug driver could register protocol notifies for the HII
protocols (which would become available much later), and once those
appeared, the debug driver could install HII forms, for managing the
non-volatile UEFI variable that configures the debug ports. (This could
perhaps be used for truly dynamic configuration, i.e., without having to
reboot; although personally I'd steer very clear of that avenue.)


As lower-hanging fruit, you could probably just implement step (h) in
isolation, and rebase both your existent driver, and PciSioSerialDxe,
onto that new library class (and its two library instances).

Other opinions are very welcome, of course! Personally, I'm out of
cycles on this topic.

Laszlo

> 3. We still end up with 2 UART drivers in flash since PciSioSerialDxe is
> needed for PCI UARTs.
> 
> I also think this solution is comparable in effort to refactoring the
> PciSioSerialDxe so that it doesn't use driver binding when used as a DXE
> driver. In this solution driver would have one .c file for code with
> driver binding and one .c file for code when everything is done in entry
> point, it would still use DEVICE_PATH and PCI_IO/SIO. While I still
> think using the driver as is in DXE is best for us, in case that
> solution gets blocked I would like to understand if everyone would be ok
> with such refactoring.
> 
> Thanks,
> Mateusz
> 



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

Re: [edk2-devel] CodeQL Analysis in edk2

[Gerd Hoffmann]

On Tue, Feb 27, 2024 at 11:04:47AM -0500, Michael Kubacki wrote:
> Hi Gerd,
> 
> A real-world example is here: 
> https://github.com/microsoft/mu_basecore/blob/release/202311/CodeQlFilters.yml
> 
> That can currently operate at the file and CodeQL rule level granularity. In
> this case, the null pointer test rule ("cpp/missing-null-test" as shown in
> https://github.com/tianocore/edk2/security/code-scanning/1277) could be
> excluded in MpLib.c.

CodeQL apparently has support for assertions[1].  The documentation
sounds like this can be extended.  So maybe it is possible to add an
'Edk2Assert' class, to have CodeQL recognize ASSERT() + variants in the
edk2 source code?

That should reduce the number of filter rules needed and simplify
maintenance long-term.

take care,
  Gerd

[1] 
https://codeql.github.com/codeql-standard-libraries/cpp/semmle/code/cpp/commons/Assertions.qll/module.Assertions.html



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

Re: [edk2-devel] Tagging and releases of edk2-basetools

[Gerd Hoffmann]

On Wed, Feb 28, 2024 at 09:27:57AM +0100, Laszlo Ersek wrote:
> On 2/28/24 02:15, Rebecca Cran wrote:
> > edk2-basetools is finally fixed and releases can once again be published
> > to PyPI.
> > 
> > However, in moving from setup.py to pyproject.toml the process has
> > changed, and Joey suggested the old way might have been chosen
> > deliberately to be different from edk2-pytool-library and
> > edk2-pytool-extensions.
> > 
> > Previously it fetched the list of releases from PyPI and incremented the
> > version number before publishing a new version, while it now depends on
> > the git repo being tagged.
> > 
> > Should I work on migrating the old code and figuring out how to make it
> > work with pyproject.toml?

Not sure what exactly you want migrate.  I think releases being
triggered by tags in the repo makes alot of sense.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116113): https://edk2.groups.io/g/devel/message/116113
Mute This Topic: https://groups.io/mt/104615885/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 2/6] RedfishPkg: implement Redfish HTTP protocol

[Nickle Wang via groups.io]

Hi @Mike Maslenkin,



May I have your reviewed-by if version 3 patch set look good to you?



Thanks,

Nickle



> -Original Message-

> From: devel@edk2.groups.io  On Behalf Of Nickle Wang

> via groups.io

> Sent: Tuesday, February 27, 2024 8:49 AM

> To: Mike Maslenkin 

> Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner Chang

> ; Nick Ramirez 

> Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP

> protocol

>

> External email: Use caution opening links or attachments

>

>

> Thanks for your confirmation, Mike!

>

> Version 3 patch set is here: https://edk2.groups.io/g/devel/message/115985

>

> Regards,

> Nickle

>

> > -Original Message-

> > From: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

> > Sent: Tuesday, February 27, 2024 8:13 AM

> > To: Nickle Wang mailto:nick...@nvidia.com>>

> > Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> > mailto:ig...@ami.com>>; Abner

> > Chang mailto:abner.ch...@amd.com>>; Nick Ramirez 
> > mailto:nrami...@nvidia.com>>

> > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish

> > HTTP protocol

> >

> > External email: Use caution opening links or attachments

> >

> >

> > Hii Nickle,

> >

> >

> > On Mon, Feb 26, 2024 at 4:44 PM Nickle Wang 
> > mailto:nick...@nvidia.com>> wrote:

> > >

> > > Hi Mike,

> > >

> > > > So finally we have

> > > > HttpFreeHeaderFields (Response->Headers, Response->HeaderCount);

> > > > but

> > > > Response->HeaderCount does not count partially allocated elements. 
> > > > Right?

> > > >

> > > > To fix this, it is required to set *DstHeaderCount =

> > > > SrcHeaderCount unconditionally right after DstHeaders  allocation,

> > > > and HttpFreeHeaderFields() will do the work then.

> > >

> > > I follow your suggestion to update DstHeaderCount right after

> > > DstHeaders is

> > allocated.  So, HttpFreeHeaderFields can release headers correctly. I

> > also create a macro to implemented AsciiStrCpy. Please check below link to 
> > see

> my changes:

> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi

> > > th

> > >

> >

> ub.com%2Ftianocore%2Fedk2%2Fcompare%2F0f391b1c2f988d90a3ac723b314a

> > c28b

> > >

> >

> a7b0b8df..f0fa1b8fdcd933beb52fd3127c2476443c00ef8d&data=05%7C02%7Cnic

> > k

> > >

> >

> lew%40nvidia.com%7Cf3870f71360e44f3b4e208dc3728ff87%7C43083d1572734

> > 0c1

> > >

> >

> b7db39efd9ccc17a%7C0%7C0%7C638445896465360452%7CUnknown%7CTWFp

> > bGZsb3d8

> > >

> >

> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%

> > 7C0

> > >

> >

> %7C%7C%7C&sdata=K%2FEA2QWpk%2F8NHQ1QhzqkvQqao4db%2BILn1Jt%2BB

> > qQ5n1E%3D

> > > &reserved=0

> >

> > These changes looks good. Internal strings

> > initialization/deinitialization code much cleaner now and possible leak 
> > seems to

> have been fixed.

> >

> > Thank you!

> >

> > Regards,

> > Mike.

>

>

> 

>




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116114): https://edk2.groups.io/g/devel/message/116114
Mute This Topic: https://groups.io/mt/104505404/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] UefiCpuPkg/MpInitLib: add struct MP_HAND_OFF_CONFIG

[Gerd Hoffmann]

Move the WaitLoopExecutionMode and StartupSignalValue fields to a
separate HOB with the new struct.

WaitLoopExecutionMode and StartupSignalValue are independent of
processor index ranges; they are global to MpInitLib (i.e., the entire
system). Therefore they shouldn't be repeated in every MpHandOff GUID
HOB.

Signed-off-by: Gerd Hoffmann 
---
 UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 47 +---
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 34 ++---
 4 files changed, 75 insertions(+), 22 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h 
b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
index 77854d6a81f8..ae93b7e3d7c9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
@@ -15,7 +15,13 @@
 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a, 
0x60 } \
   }
 
+#define MP_HANDOFF_CONFIG_GUID \
+  { \
+0xdabbd793, 0x7b46, 0x4144, {0x8a, 0xd4, 0x10, 0x1c, 0x7c, 0x08, 0xeb, 
0xfa } \
+  }
+
 extern EFI_GUID  mMpHandOffGuid;
+extern EFI_GUID  mMpHandOffConfigGuid;
 
 //
 // The information required to transfer from the PEI phase to the
@@ -43,8 +49,11 @@ typedef struct {
   //
   UINT32ProcessorIndex;
   UINT32CpuCount;
-  UINT32WaitLoopExecutionMode;
-  UINT32StartupSignalValue;
   PROCESSOR_HAND_OFFInfo[];
 } MP_HAND_OFF;
+
+typedef struct {
+  UINT32WaitLoopExecutionMode;
+  UINT32StartupSignalValue;
+} MP_HAND_OFF_CONFIG;
 #endif
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 3a7b9896cff4..d26035559f22 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -482,7 +482,8 @@ GetWakeupBuffer (
 **/
 VOID
 SwitchApContext (
-  IN CONST MP_HAND_OFF  *FirstMpHandOff
+  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
+  IN CONST MP_HAND_OFF *FirstMpHandOff
   );
 
 /**
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8c186211fb38..9bac62f289e0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -15,6 +15,7 @@
 
 EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
 EFI_GUID  mMpHandOffGuid   = MP_HANDOFF_GUID;
+EFI_GUID  mMpHandOffConfigGuid = MP_HANDOFF_CONFIG_GUID;
 
 /**
   Save the volatile registers required to be restored following INIT IPI.
@@ -1935,11 +1936,13 @@ GetBspNumber (
   This procedure allows the AP to switch to another section of
   memory and continue its loop there.
 
-  @param[in] FirstMpHandOff  Pointer to first MP hand-off HOB body.
+  @param[in] MpHandOffConfig  Pointer to MP hand-off config HOB body.
+  @param[in] FirstMpHandOff   Pointer to first MP hand-off HOB body.
 **/
 VOID
 SwitchApContext (
-  IN CONST MP_HAND_OFF  *FirstMpHandOff
+  IN CONST MP_HAND_OFF_CONFIG  *MpHandOffConfig,
+  IN CONST MP_HAND_OFF *FirstMpHandOff
   )
 {
   UINTN  Index;
@@ -1955,7 +1958,7 @@ SwitchApContext (
 for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
   if (MpHandOff->ProcessorIndex + Index != BspNumber) {
 *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = 
(UINTN)SwitchContextPerAp;
-*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
MpHandOff->StartupSignalValue;
+*(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = 
MpHandOffConfig->StartupSignalValue;
   }
 }
   }
@@ -1975,6 +1978,26 @@ SwitchApContext (
   }
 }
 
+/**
+  Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.
+
+  @return  The pointer to MP_HAND_OFF_CONFIG structure.
+**/
+MP_HAND_OFF_CONFIG *
+GetMpHandOffConfigHob (
+  VOID
+  )
+{
+  EFI_HOB_GUID_TYPE  *GuidHob;
+
+  GuidHob = GetFirstGuidHob (&mMpHandOffConfigGuid);
+  if (GuidHob == NULL) {
+return NULL;
+  }
+
+  return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
+}
+
 /**
   Get pointer to next MP_HAND_OFF GUIDed HOB body.
 
@@ -2022,6 +2045,7 @@ MpInitLibInitialize (
   VOID
   )
 {
+  MP_HAND_OFF_CONFIG   *MpHandOffConfig;
   MP_HAND_OFF  *FirstMpHandOff;
   MP_HAND_OFF  *MpHandOff;
   CPU_INFO_IN_HOB  *CpuInfoInHob;
@@ -2239,13 +2263,24 @@ MpInitLibInitialize (
   }
 }
 
+MpHandOffConfig = GetMpHandOffConfigHob ();
+if (MpHandOffConfig == NULL) {
+  DEBUG ((
+DEBUG_ERROR,
+"%a: at least one MpHandOff HOB, but no MpHandOffConfig HOB\n",
+__func__
+));
+  ASSERT (MpHandOffConfig != NULL);
+  CpuDeadLoop ();
+}
+
 DEBUG ((
   DEBUG_INFO,
   "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n",
-  FirstMpHandOff->WaitLoopExecutionMode,
+  MpHandOffConfig->WaitLoopExecutionMode,
   sizeof (VOID *)
   ));
-if (FirstMpHandOff->W

Re: [edk2-devel] [PATCH v1 2/3] DynamicTablesPkg: Adds ACPI HPET Table generator

[Abdul Lateef Attar via groups.io]

Hi Pierre Gondois,

    Thanks for review the comment, i will make the changes accordingly.

Please find my response in line.

Thanks

AbduL

On 27-02-2024 21:32, Pierre Gondois wrote:
Caution: This message originated from an External Source. Use proper 
caution when opening attachments, clicking links, or responding.



Hello Abdul,

From the HPET spec:
"""
For the case where there may be additional Event Timer Blocks 
implemented in the system, their base

addresses would be described in ACPI Name space.
"""
So it seems it might be good (but not necessary) to to add a 
description in

a SSDT/DSDT table of the object (with _HID=PNP0103).
[Abdul] I will submit another Generator library for creating the 
platform devices in ACPI namespace.


---

Same comment about than for the other patches about adding objects to
the common Arch namespace.`

[Abdul] This Arch namespace changes are non-disruptive and wont cause 
any failure to existing ARM namespace.


I'll submit a separate patch for acpiview to parse the HPET and WSMT table.



On 2/20/24 07:48, Abdul Lateef Attar wrote:

From: Abdul Lateef Attar 

Adds generic ACPI HPET table generator library.
Register/Deregister HPET table.
Update the HPET table during boot as per specification.

Cc: Sami Mujawar 
Cc: Pierre Gondois 
Signed-off-by: Abdul Lateef Attar 
---
  DynamicTablesPkg/DynamicTables.dsc.inc    |   2 +
  DynamicTablesPkg/DynamicTablesPkg.ci.yaml |   3 +-
  DynamicTablesPkg/Include/AcpiTableGenerator.h |   1 +
  .../Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf  |  38 
  .../Library/Acpi/AcpiHpetLib/HpetGenerator.c  | 208 ++
  5 files changed, 251 insertions(+), 1 deletion(-)
  create mode 100644 
DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
  create mode 100644 
DynamicTablesPkg/Library/Acpi/AcpiHpetLib/HpetGenerator.c


diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
b/DynamicTablesPkg/DynamicTables.dsc.inc

index 5ec9ffac06..af70785520 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -35,6 +35,7 @@
    # Generators
    #
    DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
+  DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf

    #
    # Dynamic Table Factory Dxe
@@ -42,6 +43,7 @@
DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf 
{

  
NULL|DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
+ NULL|DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
    }

  [Components.ARM, Components.AARCH64]
diff --git a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml 
b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml

index 1ad5540e24..cacdaa1df6 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
+++ b/DynamicTablesPkg/DynamicTablesPkg.ci.yaml
@@ -53,7 +53,8 @@
  "EmbeddedPkg/EmbeddedPkg.dec",
  "DynamicTablesPkg/DynamicTablesPkg.dec",
  "MdeModulePkg/MdeModulePkg.dec",
-    "MdePkg/MdePkg.dec"
+    "MdePkg/MdePkg.dec",
+    "PcAtChipsetPkg/PcAtChipsetPkg.dec"
  ],
  # For host based unit tests
  "AcceptableDependencies-HOST_APPLICATION":[
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h 
b/DynamicTablesPkg/Include/AcpiTableGenerator.h

index d0eda011c3..18b5f99f47 100644and
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -99,6 +99,7 @@ typedef enum StdAcpiTableId {
    EStdAcpiTableIdSsdtCpuTopology,   ///< SSDT Cpu Topology
    EStdAcpiTableIdSsdtPciExpress,    ///< SSDT Pci 
Express Generator

    EStdAcpiTableIdPcct,  ///< PCCT Generator
+  EStdAcpiTableIdHpet,  ///< HPET Generator
    EStdAcpiTableIdMax
  } ESTD_ACPI_TABLE_ID;

diff --git 
a/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf 
b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf

new file mode 100644
index 00..74a1358ffe
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
@@ -0,0 +1,38 @@
+## @file
+#  HPET Table Generator
+#
+#  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION    = 1.27
+  BASE_NAME  = AcpiHpetLib
+  FILE_GUID  = 4E75F653-C356-48B3-B32C-D1B901ECF90A
+  VERSION_STRING = 1.0
+  MODULE_TYPE    = DXE_DRIVER
+  LIBRARY_CLASS  = NULL|DXE_DRIVER
+  CONSTRUCTOR    = AcpiHpetLibConstructor
+  DESTRUCTOR = AcpiHpetLibDestructor
+
+[Sources]
+  HpetGenerator.c
+
+[Packages]
+  DynamicTablesPkg/DynamicTablesPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec


I think the dependency could be deleted, along with:
HpetGenerator.c:19:#include 


+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  PcAtChipsetPkg/PcAtChipsetPkg.dec


(for Sami)
A dependency over the PcAtChipsetPkg is introduced here.


+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  IoLib
+  PcdLib
+
+[Pcd]

[edk2-devel] CI: GCC13 and lcov problem

[Oliver Steffen]

Hi,

I am working on switching the Linux CI jobs over to the Fedora 39 image
which comes with gcc 13.

Unfortunately, some jobs fail due to some error related to lcov /
geninfo, see below.

Does anybody know from the top of the head what this is about / what to do?
PR: https://github.com/tianocore/edk2/pull/5412

Job log:

INFO - Cmd to run is: lcov --capture --directory
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/ --output-file
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/coverage-test.info --rc
lcov_branch_coverage=1
INFO - 
INFO - --Cmd Output Starting---
INFO - 
INFO - Capturing coverage data from
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/
INFO - geninfo cmd: '/usr/bin/geninfo
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/ --output-filename
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/coverage-test.info --rc
lcov_branch_coverage=1 --memory 0 --branch-coverage'
INFO - geninfo: WARNING: RC option 'lcov_branch_coverage' is
deprecated.  Consider using 'branch_coverage. instead.
(Backward-compatible support will be removed in the future
INFO - Found gcov version: 13.2.1
INFO - Using intermediate gcov format
INFO - Writing temporary data to /tmp/geninfo_datXdcz
INFO - Scanning /__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/ for
.gcda files ...
INFO - Found 99 data files in /__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLibHost/OUTPUT/UnitTestDebugAssertLibHost.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersistenceLibNull/OUTPUT/UnitTestPersistenceLibNull.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/Posix/DebugLibPosix/DebugLibPosix/OUTPUT/DebugLibPosix.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/Posix/MemoryAllocationLibPosix/MemoryAllocationLibPosix/OUTPUT/MemoryAllocationLibPosix.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibDebugLib/OUTPUT/UnitTestResultReportLib.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibDebugLib/OUTPUT/UnitTestResultReportLibDebugLib.gcda
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/CmockaLib/CmockaLib/OUTPUT/cmocka/src/cmocka.gcda
INFO - geninfo: WARNING:
/__w/1/s/UnitTestFrameworkPkg/Library/CmockaLib/cmocka/src/cmocka.c:725:
unexecuted block on non-branch line with non-zero hit count.  Use
"geninfo --rc geninfo_unexecuted_blocks=1 to set count to zero.
INFO - Processing
/__w/1/s/Build/SecurityPkg/HostTest/NOOPT_GCC5/X64/UnitTestFrameworkPkg/Library/GoogleTestLib/GoogleTestLib/OUTPUT/googletest/googlemock/src/gmock-all.gcda
INFO - geninfo: ERROR: "/usr/include/c++/13/bits/stl_tree.h":2129:
mismatched exception tag for id 1, 1: '1' -> '0'
INFO -  (use "geninfo --ignore-errors mismatch ..." to bypass this error)
INFO - 
INFO - --Cmd Output Finished---
INFO - - Running Time (mm:ss): 00:00 --
INFO - --- Return Code: 0x0001 
INFO - 
ERROR - UnitTest Coverage: Failed to build coverage data for tested files.
ERROR - Plugin Failed: Host-Based Unit Test Runner returned 1
CRITICAL - Post Build failed
PROGRESS - End time: 2024-02-26 16:18:23.207202  Total time Elapsed: 0:00:32
ERROR - --->Test Failed: Host Unit Test Compiler Plugin NOOPT returned 1


Thanks!

 Oliver



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

Re: [edk2-devel] [edk2][PATCH V2 1/2] ArmPkg/ArmGicArchLib: Add macros for SPI and extended SPI ranges

[Sami Mujawar]

Hi Ard, Leif,

This patch adds macros that can be used to validate that the SPI ranges are 
valid.
These have been define here so that we do not duplicate it at multiple places.

Can you let me know if I can merge this patch, please?

Regards,

Sami Mujawar


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116118): https://edk2.groups.io/g/devel/message/116118
Mute This Topic: https://groups.io/mt/103518972/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 14/23] Ovmfpkg/CcSvsmLib: Create CcSvsmLib to handle SVSM related services

[Lendacky, Thomas via groups.io]

On 2/28/24 02:40, Gerd Hoffmann wrote:

+/**
+  Perform a native PVALIDATE operation for the page ranges specified.
+
+  Validate or rescind the validation of the specified pages.
+
+  @param[in]   Info   Pointer to a page state change structure
+
+**/
+STATIC
+VOID
+BasePvalidate (
+  IN  SNP_PAGE_STATE_CHANGE_INFO  *Info
+  )


This is not mentioned in the commit message.

Looks like you are moving or copying code from BaseMemEncryptSevLib.

Moving code is best done with a patch doing the move only, without other
functional changes.  If that can't be done easily this should explained
in the commit message.


I can leave this as unsupported in this patch and then when switching over 
to using the functions in patch #16, move the code at that time.


For the VMSA update, that isn't as easy because of the interaction between 
UefiCpuPkg (MpInitLib) and OvmfPkg and requires two separate patches, 
which would cause bisection breakage.


Or I could keep this all here and expand the commit message to indicate 
that the base support is being implemented based off of the existing support.


Thoughts?

Thanks,
Tom



take care,
   Gerd




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116119): https://edk2.groups.io/g/devel/message/116119
Mute This Topic: https://groups.io/mt/104512963/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 18/23] OvmfPkg/CcSvsmLib: Add support for the SVSM_CORE_PVALIDATE call

[Lendacky, Thomas via groups.io]

On 2/28/24 02:50, Gerd Hoffmann wrote:

   Hi,


+// Clear the buffer in prep for creating all new entries
+SetMem (Caa->SvsmBuffer, sizeof (Caa->SvsmBuffer), 0);


Minor nit: There is a ZeroMem() for this purpose.


I use SetMem() in a few places, I'll change them over to ZeroMem().

Thanks,
Tom



Acked-by: Gerd Hoffmann 

take care,
   Gerd




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116120): https://edk2.groups.io/g/devel/message/116120
Mute This Topic: https://groups.io/mt/104512972/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 00/23] Provide SEV-SNP support for running under an SVSM

[Lendacky, Thomas via groups.io]

On 2/28/24 00:14, Yao, Jiewen wrote:

Some feedback:

1) 0002-MdePkg-GHCB-APIC-ID-retrieval-support-definitions

MdePkg only contains the definition in the standard.

Question: Is EFI_APIC_IDS_GUID definition in some AMD/SVSM specification?


The structure is documented in the GHCB specification, but the GUID is not.

Is the request to move the GUID to someplace other than MdePkg?



2) 0012-UefiCpuPkg-CcSvsmLib-Create-the-CcSvsmLib-library-to-support-an-SVSM

I am not sure the position of SVSM.
If the SVSM interface is AMD specific, the it should be AmdSvsmLib.


I believe TDX is also looking at the SVSM for TDX partitioning, but I'm 
not certain of that.



If the SVSM interface is generic, then we should define everything in a generic 
way.

It is very confusing to mix a generic CcSvsm lib with AMD specific 
.


I can certainly change the name to be AMD specific fow now. It can always 
be changed to something else later if need be, much like VmgExitLib was 
changed to CcExitLib.


Thanks,
Tom




Thank you
Yao, Jiewen


-Original Message-
From: Tom Lendacky 
Sent: Friday, February 23, 2024 1:30 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel ; Aktas, Erdem
; Gerd Hoffmann ; Yao, Jiewen
; Laszlo Ersek ; Liming Gao
; Kinney, Michael D ;
Xu, Min M ; Liu, Zhiguang ;
Kumar, Rahul R ; Ni, Ray ; Michael
Roth 
Subject: [PATCH v2 00/23] Provide SEV-SNP support for running under an SVSM


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

This series adds SEV-SNP support for running OVMF under an Secure VM
Service Module (SVSM) at a less privileged VM Privilege Level (VMPL).
By running at a less priviledged VMPL, the SVSM can be used to provide
services, e.g. a virtual TPM, for the guest OS within the SEV-SNP
confidential VM (CVM) rather than trust such services from the hypervisor.

Currently, OVMF expects to run at the highest VMPL, VMPL0, and there are
certain SNP related operations that require that VMPL level. Specifically,
the PVALIDATE instruction and the RMPADJUST instruction when setting the
the VMSA attribute of a page (used when starting APs).

If OVMF is to run at a less privileged VMPL, e.g. VMPL2, then it must
use an SVSM (which is running at VMPL0) to perform the operations that
it is no longer able to perform.

When running under an SVSM, OVMF must know the APIC IDs of the vCPUs that
it will be starting. As a result, the GHCB APIC ID retrieval action must
be performed. Since this service can also work with SEV-SNP running at
VMPL0, the patches to make use of this feature are near the beginning of
the series.

How OVMF interacts with and uses the SVSM is documented in the SVSM
specification [1] and the GHCB specification [2].

This support creates a new CcSvsmLib library that is used by MpInitLib.
This requires an update to the edk2-platform DSC files to add the new
library. The edk2-platform change would be needed after patch 12, but
before patch 15.

This series introduces support to run OVMF under an SVSM. It consists
of:
   - Retrieving the list of vCPU APIC IDs and starting up all APs without
 performing a broadcast SIPI
   - Reorganizing the page state change support to not directly use the
 GHCB buffer since an SVSM will use the calling area buffer, instead
   - Detecting the presence of an SVSM
   - When not running at VMPL0, invoking the SVSM for page validation and
 VMSA page creation/deletion
   - Detecting and allowing OVMF to run in a VMPL other than 0 when an
 SVSM is present

The series is based off of commit:

   2ca8d5597443 ("UefiCpuPkg/PiSmmCpuDxeSmm: Check BspIndex first before
lock cmpxchg")

[1] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-
docs/specifications/58019.pdf
[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-
docs/specifications/56421.pdf

---

Changes in v2:
- Move the APIC IDs retrieval support to the beginning of the patch series
 - Use a GUIDed HOB to hold the APIC ID list instead of a PCD
- Split up Page State Change reorganization into multiple patches
- Created CcSvsmLib library instead of extending CcExitLib
 - This will require a corresponding update to edk2-platform DSC files
 - Removed Ray Ni's Acked-by since it is not a minor change
- Variable name changes and other misc changes

Tom Lendacky (23):
   OvmfPkg/BaseMemEncryptLib: Fix error check from AsmRmpAdjust()
   MdePkg: GHCB APIC ID retrieval support definitions
   OvmfPkg/PlatformPei: Retrieve APIC IDs from the hypervisor
   UefiCpuPkg/MpInitLib: Always use AP Create if PcdSevSnpApicIds is set
   OvmfPkg/BaseMemEncryptSevLib: Fix uncrustify errors
   OvmfPkg/BaseMemEncryptSevLib: Calculate memory size for Page State
 Change
   MdePkg: Avoid hardcoded value for number of Page State Change entries
   OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support
   OvmfPkg/BaseMemEncryptSevLib: Maximize Page State Change efficiency
   MdePkg/Register/Amd: Define the SVSM related information
   MdePkg/BaseLib: Add a

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE

[Albecki, Mateusz]

Sorry, I didn't want to make an impression that I expected solution to be 
delivered, I was merely trying to explain some of the complexity we are trying 
to handle on our side and why we didn't went for SerialDxe and instead opted to 
make PciSioSerialDxe work for our use case. Anyway thanks for the ideas, we 
will definitely look into implementing step h you mentioned in case other 
maintainers also disagree with this patch.

Thanks,
Mateusz


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

Re: [edk2-devel] [edk2][PATCH V2 1/2] ArmPkg/ArmGicArchLib: Add macros for SPI and extended SPI ranges

[Sami Mujawar]

+Resending with email address for maintainers.

Hi Ard, Leif,

This patch adds macros that can be used to validate that the SPI ranges are 
valid.
These have been define here so that we do not duplicate it at multiple places.

Can you let me know if I can merge this patch, please?

Regards,

Sami Mujawar

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

Re: [edk2-devel] Tagging and releases of edk2-basetools

[Rebecca Cran]

Thanks.

Since Sean commented on 
https://github.com/tianocore/edk2-basetools/issues/85, adding him for 
more context around the original request.



--
Rebecca Cran


On 2/28/24 04:35, Gerd Hoffmann wrote:

On Wed, Feb 28, 2024 at 09:27:57AM +0100, Laszlo Ersek wrote:

On 2/28/24 02:15, Rebecca Cran wrote:

edk2-basetools is finally fixed and releases can once again be published
to PyPI.

However, in moving from setup.py to pyproject.toml the process has
changed, and Joey suggested the old way might have been chosen
deliberately to be different from edk2-pytool-library and
edk2-pytool-extensions.

Previously it fetched the list of releases from PyPI and incremented the
version number before publishing a new version, while it now depends on
the git repo being tagged.

Should I work on migrating the old code and figuring out how to make it
work with pyproject.toml?

Not sure what exactly you want migrate.  I think releases being
triggered by tags in the repo makes alot of sense.

take care,
   Gerd




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

Re: [edk2-devel] [PATCH v2 0/3] RISC-V: Support Svpbmt extension

[Tuan Phan]

On Tue, Feb 27, 2024 at 8:42 PM Sunil V L  wrote:

> Hi Tuan,
>
> On Mon, Feb 26, 2024 at 08:34:22PM -0800, Tuan Phan wrote:
> > Hi Sunil/ Andrei,
> > Any comments on this series?
> >
> Did I miss your response to Laszlo's feedback on PATCH 2 - [1]? Apart
> from that, don't we need to handle EFI_MEMORY_WT similar to
> EFI_MEMORY_WC?
>
> Somehow I missed that feedback. Thanks.
About EFI_MEMORY_WT, ARM treats it as EFI_MEMORY_WC under hood but I don't
see RISC-V specs mentions it explicitly so don't feel confident to add
that.

> [1] - https://edk2.groups.io/g/devel/message/115243
>
> Thanks,
> Sunil
> > Regards,
> >
> > On Wed, Feb 14, 2024 at 10:16 PM Tuan Phan via groups.io  > ventanamicro@groups.io> wrote:
> >
> > >
> > >
> > > On Wed, Feb 14, 2024 at 9:43 PM Warkentin, Andrei <
> > > andrei.warken...@intel.com> wrote:
> > >
> > >> Do you mind sharing a GH branch with the patch set?
> > >>
> > > https://github.com/pttuan/edk2/tree/tphan/riscv_mmu_svpbmt
> > > Tuan
> > >
> > >>
> > >> A
> > >>
> > >> > -Original Message-
> > >> > From: Tuan Phan 
> > >> > Sent: Tuesday, February 6, 2024 7:29 PM
> > >> > To: devel@edk2.groups.io
> > >> > Cc: Kinney, Michael D ;
> > >> > gaolim...@byosoft.com.cn; Liu, Zhiguang ;
> > >> > kra...@redhat.com; ler...@redhat.com; Kumar, Rahul R
> > >> > ; Ni, Ray ;
> > >> > suni...@ventanamicro.com; Yao, Jiewen ;
> > >> Warkentin,
> > >> > Andrei ; ardb+tianoc...@kernel.org;
> Tuan
> > >> Phan
> > >> > 
> > >> > Subject: [PATCH v2 0/3] RISC-V: Support Svpbmt extension
> > >> >
> > >> > This patchset adds support for RISC-V Svpbmt extension.
> > >> >
> > >> > The GCD EFI_MEMORY_UC and EFI_MEMORY_WC attributes will be mapped to
> > >> > IO and NC mode defined in PBMT field.
> > >> >
> > >> > v2:
> > >> >   - Generated patch for each package.
> > >> >
> > >> > Tuan Phan (3):
> > >> >   MdePkg.dec: RISC-V: Define override bit for Svpbmt extension
> > >> >   UefiCpuPkg: RISC-V: MMU: Support Svpbmt extension
> > >> >   OvmfPkg/RiscVVirt: Override Svpbmt extension
> > >> >
> > >> >  MdePkg/MdePkg.dec |  2 ++
> > >> >  OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc   |  2 +-
> > >> >  .../Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c | 25
> ++-
> > >> >  .../BaseRiscVMmuLib/BaseRiscVMmuLib.inf   |  1 +
> > >> >  4 files changed, 28 insertions(+), 2 deletions(-)
> > >> >
> > >> > --
> > >> > 2.25.1
> > >>
> > >> 
> > >
> > >
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116125): https://edk2.groups.io/g/devel/message/116125
Mute This Topic: https://groups.io/mt/104211191/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 2/6] RedfishPkg: implement Redfish HTTP protocol

[Mike Maslenkin]

On Wed, Feb 28, 2024 at 2:47 PM Nickle Wang  wrote:
>
> Hi @Mike Maslenkin,
>
>
>
> May I have your reviewed-by if version 3 patch set look good to you?
>

Sure!

Reviewed-by: Mike Maslenkin 

BTW I'm just curious, there is a mention in patch 2 "We currently only
support gzip Content-Encoding."
But I didn't see any implementation of gzip coding/encoding for edk2.
Do you know of any?

I hope you know that patch 5 breaks edk2-redfish-client compilation
(Instance of library class [RedfishHttpLib] is not found)
But I understand these changes are not atomic for edk2 and edk2-redfish-client.

Regards,
Mike.


>
>
> Thanks,
>
> Nickle
>
>
>
> > -Original Message-
>
> > From: devel@edk2.groups.io  On Behalf Of Nickle Wang
>
> > via groups.io
>
> > Sent: Tuesday, February 27, 2024 8:49 AM
>
> > To: Mike Maslenkin 
>
> > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner Chang
>
> > ; Nick Ramirez 
>
> > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP
>
> > protocol
>
> >
>
> > External email: Use caution opening links or attachments
>
> >
>
> >
>
> > Thanks for your confirmation, Mike!
>
> >
>
> > Version 3 patch set is here: https://edk2.groups.io/g/devel/message/115985
>
> >
>
> > Regards,
>
> > Nickle
>
> >
>
> > > -Original Message-
>
> > > From: Mike Maslenkin 
>
> > > Sent: Tuesday, February 27, 2024 8:13 AM
>
> > > To: Nickle Wang 
>
> > > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner
>
> > > Chang ; Nick Ramirez 
>
> > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish
>
> > > HTTP protocol
>
> > >
>
> > > External email: Use caution opening links or attachments
>
> > >
>
> > >
>
> > > Hii Nickle,
>
> > >
>
> > >
>
> > > On Mon, Feb 26, 2024 at 4:44 PM Nickle Wang  wrote:
>
> > > >
>
> > > > Hi Mike,
>
> > > >
>
> > > > > So finally we have
>
> > > > > HttpFreeHeaderFields (Response->Headers, Response->HeaderCount);
>
> > > > > but
>
> > > > > Response->HeaderCount does not count partially allocated elements. 
> > > > > Right?
>
> > > > >
>
> > > > > To fix this, it is required to set *DstHeaderCount =
>
> > > > > SrcHeaderCount unconditionally right after DstHeaders  allocation,
>
> > > > > and HttpFreeHeaderFields() will do the work then.
>
> > > >
>
> > > > I follow your suggestion to update DstHeaderCount right after
>
> > > > DstHeaders is
>
> > > allocated.  So, HttpFreeHeaderFields can release headers correctly. I
>
> > > also create a macro to implemented AsciiStrCpy. Please check below link 
> > > to see
>
> > my changes:
>
> > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
>
> > > > th
>
> > > >
>
> > >
>
> > ub.com%2Ftianocore%2Fedk2%2Fcompare%2F0f391b1c2f988d90a3ac723b314a
>
> > > c28b
>
> > > >
>
> > >
>
> > a7b0b8df..f0fa1b8fdcd933beb52fd3127c2476443c00ef8d&data=05%7C02%7Cnic
>
> > > k
>
> > > >
>
> > >
>
> > lew%40nvidia.com%7Cf3870f71360e44f3b4e208dc3728ff87%7C43083d1572734
>
> > > 0c1
>
> > > >
>
> > >
>
> > b7db39efd9ccc17a%7C0%7C0%7C638445896465360452%7CUnknown%7CTWFp
>
> > > bGZsb3d8
>
> > > >
>
> > >
>
> > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>
> > > 7C0
>
> > > >
>
> > >
>
> > %7C%7C%7C&sdata=K%2FEA2QWpk%2F8NHQ1QhzqkvQqao4db%2BILn1Jt%2BB
>
> > > qQ5n1E%3D
>
> > > > &reserved=0
>
> > >
>
> > > These changes looks good. Internal strings
>
> > > initialization/deinitialization code much cleaner now and possible leak 
> > > seems to
>
> > have been fixed.
>
> > >
>
> > > Thank you!
>
> > >
>
> > > Regards,
>
> > > Mike.
>
> >
>
> >
>
> > 
>
> >
>
>


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

Re: [edk2-devel] [edk2-redfish-client][PATCH] edk2-Redfish-client: Clarify HTTP method used for provisioning

[Mike Maslenkin]

Reviewed-by: Mike Maslenkin 

Regards,
Mike.

On Mon, Feb 26, 2024 at 7:55 AM  wrote:
>
> From: Abner Chang 
>
> Clarify the HTTP method that is used to provision BIOS
> managed Redfish resource.
>
> Signed-off-by: Abner Chang 
> Cc: Nickle Wang 
> Cc: Igor Kulchytskyy 
> Cc: Mike Maslenkin 
> ---
>  RedfishClientPkg/Readme.md| 35 ---
>  .../Media/redfish-call-flow-provisioning.svg  |  2 +-
>  .../Media/redfish-synchronization-design.svg  |  4 +--
>  3 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/RedfishClientPkg/Readme.md b/RedfishClientPkg/Readme.md
> index 82cb9c8c99..1789dff6f8 100644
> --- a/RedfishClientPkg/Readme.md
> +++ b/RedfishClientPkg/Readme.md
> @@ -310,21 +310,32 @@ job.
>
>  Several interfaces defined in EDKII Redfish Resource Config Protocol work 
> together to support Redfish synchronization:
>  - Identify()
> -  - This function is used to check if the given Redfish resource is the one 
> the feature driver wants to manage. A platform
> -library `RedfishResourceIdentifyLib` is introduced for platform to 
> implement its own policy to identify Redfish resource.
> +  - This function is used to check if the given Redfish resource is the one 
> the feature driver
> +wants to manage. A platform library `RedfishResourceIdentifyLib` is 
> introduced for
> +platform to implement its own policy to identify Redfish resource.
>  - Check()
> -  - This function is used to check the attribute status on Redfish service. 
> If all attributes the feature driver manages
> -are presented in Redfish service, feature driver must provision them 
> already. Otherwise, Provisioning() will be called
> -to perform resource provisioning job.
> +  - This function is used to check the attribute status on Redfish service. 
> If all attributes
> +the feature driver manages are presented in Redfish service, feature 
> driver must provision
> +them already. Otherwise, Provisioning() will be called to perform 
> resource provisioning
> +job.
>  - Provisioning()
> -  - When this function is called, feature driver will provision all 
> attributes that it managed to Redfish service. This
> -operation usually create new resource at Redfish service and require 
> different operation that specified by Redfish service.
> +  - When this function is called, feature driver will provision all 
> attributes that it managed
> +to Redfish service. This operation usually creates the new Redfish 
> properties at the
> +existing URI in Redfish service. Use HTTP PATCH to provision Redfish 
> properties as BIOS
> +may only manage some but not all of the properties of the resource. See 
> [Redfish-edk2 
> implementation](#Redfish-Service-Implementation-that-Incorporates-with-EDK2-Redfish)
>  for
> +the details. HTTP POST is still used for creating a collection member, 
> such as the
> +collection member of processor or memory for the Redfish inventory 
> management.
> +However, HTTP PUT to overwrite an entire Redfish resource is not used in 
> edk2 Redfish
> +implementation as edk2 Redfish implementation has no idea of whether the 
> Redfish resource
> +is entirely managed by BIOS or not.
>  - Consume()
> -  - When there is pending settings in Redfish service, this function is 
> called for feature driver to consume pending settings
> -requested by user.
> +  - When there is pending settings in Redfish service, this function is 
> called for feature
> +driver to consume pending settings requested by user. HTTP GET is the 
> method used
> +to retrieve Redfish properties.
>  - Update()
> -  - When platform configuration is updated, this function is called to 
> update configuration changes to Redfish service and
> -Redfish service can show the latest settings on platform.
> +  - When platform configuration is updated, this function is called to 
> update configuration
> +changes to Redfish service and Redfish service can show the latest 
> settings on platform.
> +HTTP PATCH is the method used to update the properties of Redfish 
> resource.
>
>  The EDKII Redfish Resource Addendum Protocol is introduced to provide 
> platform addendum data that Redfish service requires.
>  This protocol will be called at Provisioning() and Update() functions so 
> platform can add OEM attribute or any other attribute
> @@ -338,7 +349,7 @@ struct _EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL {
>  };
>  ```
>
> -### Redfish Service Implementation that Incorporates with EDK2 Redfish
> +###  name="Redfish-Service-Implementation-that-Incorporates-with-EDK2-Redfish">Redfish
>  Service Implementation that Incorporates with EDK2 Redfish
>  The idea of Redfish synchronization design is to manage Redfish resource 
> directly by platform host
>  firmware. To do this, Redfish synchronization functions have to work with 
> Redfish service implementation
>  in BMC firmware. This is because the mechanism between platform host 
> fi

Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg: RISC-V: MMU: Support Svpbmt extension

[Tuan Phan]

On Wed, Feb 7, 2024 at 10:15 AM Laszlo Ersek  wrote:

> On 2/7/24 02:29, Tuan Phan wrote:
> > The GCD EFI_MEMORY_UC and EFI_MEMORY_WC attributes will be
> > supported when Svpbmt extension available.
> >
> > Signed-off-by: Tuan Phan 
> > ---
> >  .../Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c | 25 ++-
> >  .../BaseRiscVMmuLib/BaseRiscVMmuLib.inf   |  1 +
> >  2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> > index 826a1d32a1d4..c50a28e97e4b 100644
> > --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> > +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> > @@ -36,6 +36,15 @@
> >  #define PTE_PPN_SHIFT 10
> >  #define RISCV_MMU_PAGE_SHIFT  12
> >
> > +#define RISCV_CPU_FEATURE_PBMT_BITMASK  BIT2
> > +#define PTE_PBMT_NC BIT61
> > +#define PTE_PBMT_IO BIT62
> > +#define PTE_PBMT_MASK   (PTE_PBMT_NC | PTE_PBMT_IO)
> > +
> > +#define EFI_MEMORY_CACHETYPE_MASK  (EFI_MEMORY_UC | EFI_MEMORY_WC |  \
> > + EFI_MEMORY_WT | EFI_MEMORY_WB | \
> > + EFI_MEMORY_UCE)
> > +
>
> (1) I've stated this elsewhere -- introducing such a macro is justified,
> but calling it EFI_* is not. The EFI_ prefix is reserved for the spec.
>
Will fix it.


>
> >  STATIC UINTN  mModeSupport[] = { SATP_MODE_SV57, SATP_MODE_SV48,
> SATP_MODE_SV39, SATP_MODE_OFF };
> >  STATIC UINTN  mMaxRootTableLevel;
> >  STATIC UINTN  mBitPerLevel;
> > @@ -514,6 +523,20 @@ GcdAttributeToPageAttribute (
> >  RiscVAttributes &= ~RISCV_PG_X;
> >}
> >
> > +  if ((PcdGet64 (PcdRiscVFeatureOverride) &
> RISCV_CPU_FEATURE_PBMT_BITMASK) != 0) {
> > +switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) {
> > +  case EFI_MEMORY_UC:
> > +RiscVAttributes |= PTE_PBMT_IO;
> > +break;
> > +  case EFI_MEMORY_WC:
> > +RiscVAttributes |= PTE_PBMT_NC;
> > +break;
> > +  default:
> > +// Default PMA mode
> > +break;
> > +}
> > +  }
> > +
> >return RiscVAttributes;
> >  }
> >
>
> Several questions / observations:
>
> (2) If the feature is cleared in the PCD, does it deserve a warning that
> the attribute setting request cannot be honored?
>
Sure, I will add a warning if the feature has not been enabled.

>
> (3) The memory cacheability attributes are expressed as distinct bits of
> a bitmask because, for expressing *capabilities*, they must be possible
> to OR together. However, when setting actual attributes, I think the
> bitmask should contain *exactly* one bit set -- in other words, the
> value of the bitmask should be an integral power of two (that's not hard
> to check).
>
> Do you agree about this? If so, I'd suggest rejecting the request (with
> an appropriate status code) if zero bits, or multiple bits, are set.
>
>   UINT64  CacheTypeMask;
>
>   CacheType = GcdAttributes & MEMORY_CACHETYPE_MASK;
>   if ((CacheType == 0) ||
>   (((CacheType - 1) & CacheType) != 0)) {
> return EFI_INVALID_PARAMETER;
>   }
>   switch (CacheType) {
> ...
>   }
>
> This would of course require changing the GcdAttributeToPageAttribute()
> prototype, because right now the function cannot return an error.
>
> That makes sense. Will fix it. Thanks

>
> > @@ -559,7 +582,7 @@ RiscVSetMemoryAttributes (
> > BaseAddress,
> > Length,
> > PageAttributesSet,
> > -   PTE_ATTRIBUTES_MASK,
> > +   PTE_ATTRIBUTES_MASK | PTE_PBMT_MASK,
> > (UINTN *)RiscVGetRootTranslateTable (),
> > TRUE
> > );
>
> (4) I feel we shouldn't try to clear PTE_PBMT_MASK if
> PcdRiscVFeatureOverride tells us that Svpbmt is not available. Just a
> thought.
>
Sure.

>
> > diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> > index 51ebe1750e97..1dbaa81f3608 100644
> > --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> > +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> > @@ -28,3 +28,4 @@
> >
> >  [Pcd]
> >gUefiCpuPkgTokenSpaceGuid.PcdCpuRiscVMmuMaxSatpMode  ## CONSUMES
> > +  gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES
>
> Thanks
> Laszlo
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116128): https://edk2.groups.io/g/devel/message/116128
Mute This Topic: https://groups.io/mt/104211195/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 2/6] RedfishPkg: implement Redfish HTTP protocol

[Nickle Wang via groups.io]

> Sure!
> 
> Reviewed-by: Mike Maslenkin 

Thanks, Mike!

> But I didn't see any implementation of gzip coding/encoding for edk2.
> Do you know of any?

I just talked to Aber about this. We are working to see if we can provide gzip 
implementation in edk2 or not. It seems to me that we need 3rd party library to 
edk2 for supporting gzip. Anber, please feel free to correct me if I am wrong.

> I hope you know that patch 5 breaks edk2-redfish-client compilation (Instance 
> of
> library class [RedfishHttpLib] is not found) But I understand these changes 
> are not
> atomic for edk2 and edk2-redfish-client.

Yes, I also have patch for edk2-redfish-client to use Redfish HTTP protocol. I 
had tested Redfish HTTP protocol on edk2-redfish-client. I will send out patch 
for review after Redfish HTTP protocol gets merged in edk2.

Regards,
Nickle

> -Original Message-
> From: Mike Maslenkin 
> Sent: Thursday, February 29, 2024 1:56 AM
> To: Nickle Wang 
> Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner Chang
> ; Nick Ramirez 
> Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP
> protocol
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Feb 28, 2024 at 2:47 PM Nickle Wang  wrote:
> >
> > Hi @Mike Maslenkin,
> >
> >
> >
> > May I have your reviewed-by if version 3 patch set look good to you?
> >
> 
> Sure!
> 
> Reviewed-by: Mike Maslenkin 
> 
> BTW I'm just curious, there is a mention in patch 2 "We currently only support
> gzip Content-Encoding."
> But I didn't see any implementation of gzip coding/encoding for edk2.
> Do you know of any?
> 
> I hope you know that patch 5 breaks edk2-redfish-client compilation (Instance 
> of
> library class [RedfishHttpLib] is not found) But I understand these changes 
> are not
> atomic for edk2 and edk2-redfish-client.
> 
> Regards,
> Mike.
> 
> 
> >
> >
> > Thanks,
> >
> > Nickle
> >
> >
> >
> > > -Original Message-
> >
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Nickle Wang
> >
> > > via groups.io
> >
> > > Sent: Tuesday, February 27, 2024 8:49 AM
> >
> > > To: Mike Maslenkin 
> >
> > > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner
> > > Chang
> >
> > > ; Nick Ramirez 
> >
> > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement
> > > Redfish HTTP
> >
> > > protocol
> >
> > >
> >
> > > External email: Use caution opening links or attachments
> >
> > >
> >
> > >
> >
> > > Thanks for your confirmation, Mike!
> >
> > >
> >
> > > Version 3 patch set is here:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
> > >
> k2.groups.io%2Fg%2Fdevel%2Fmessage%2F115985&data=05%7C02%7Cnicklew
> %4
> > >
> 0nvidia.com%7Ca30038f7379c4f8dad3b08dc3886a03b%7C43083d15727340c1b
> 7d
> > >
> b39efd9ccc17a%7C0%7C0%7C638447398077724632%7CUnknown%7CTWFpbG
> Zsb3d8e
> > >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C
> > >
> 0%7C%7C%7C&sdata=0tXBIuafvJuG7AM1DpAgSGGLf1QeUbOOmCq2WQCYpeg%
> 3D&rese
> > > rved=0
> >
> > >
> >
> > > Regards,
> >
> > > Nickle
> >
> > >
> >
> > > > -Original Message-
> >
> > > > From: Mike Maslenkin 
> >
> > > > Sent: Tuesday, February 27, 2024 8:13 AM
> >
> > > > To: Nickle Wang 
> >
> > > > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner
> >
> > > > Chang ; Nick Ramirez 
> >
> > > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement
> > > > Redfish
> >
> > > > HTTP protocol
> >
> > > >
> >
> > > > External email: Use caution opening links or attachments
> >
> > > >
> >
> > > >
> >
> > > > Hii Nickle,
> >
> > > >
> >
> > > >
> >
> > > > On Mon, Feb 26, 2024 at 4:44 PM Nickle Wang 
> wrote:
> >
> > > > >
> >
> > > > > Hi Mike,
> >
> > > > >
> >
> > > > > > So finally we have
> >
> > > > > > HttpFreeHeaderFields (Response->Headers,
> > > > > > Response->HeaderCount);
> >
> > > > > > but
> >
> > > > > > Response->HeaderCount does not count partially allocated elements.
> Right?
> >
> > > > > >
> >
> > > > > > To fix this, it is required to set *DstHeaderCount =
> >
> > > > > > SrcHeaderCount unconditionally right after DstHeaders
> > > > > > allocation,
> >
> > > > > > and HttpFreeHeaderFields() will do the work then.
> >
> > > > >
> >
> > > > > I follow your suggestion to update DstHeaderCount right after
> >
> > > > > DstHeaders is
> >
> > > > allocated.  So, HttpFreeHeaderFields can release headers
> > > > correctly. I
> >
> > > > also create a macro to implemented AsciiStrCpy. Please check below
> > > > link to see
> >
> > > my changes:
> >
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > >
> 2Fgi%2F&data=05%7C02%7Cnicklew%40nvidia.com%7Ca30038f7379c4f8dad
> > > > >
> 3b08dc3886a03b%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6384
> > > > >
> 47398077735545%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QI
> > > > >
> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=dZf
> > > > >
> %2BXZMEyp4%2BC%2BZgFnVCr12fIyXn1ZDsFfk2ejkYGO8%3D&reserved=0
> >
> > > > >

Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP protocol

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -Original Message-
> From: Nickle Wang 
> Sent: Thursday, February 29, 2024 8:11 AM
> To: Mike Maslenkin 
> Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Chang, Abner
> ; Nick Ramirez 
> Subject: RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish
> HTTP protocol
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> > Sure!
> >
> > Reviewed-by: Mike Maslenkin 
>
> Thanks, Mike!
>
> > But I didn't see any implementation of gzip coding/encoding for edk2.
> > Do you know of any?
>
> I just talked to Aber about this. We are working to see if we can provide gzip
> implementation in edk2 or not. It seems to me that we need 3rd party library
> to edk2 for supporting gzip. Anber, please feel free to correct me if I am 
> wrong.
Yes and we hope someone can provide the implementation. @Igor Kulchytskyy, does 
AMI has the implementation of gzip? 😊
BTW, we do have a proposal that introduces EFI_SOURCE_CODING_PROTOCOL long time 
ago while I was worked for HPE. I think we should pick up this one and promote 
this protocol in UEFI spec, we can work with AMI on this as well.

Thanks
Abner


>
> > I hope you know that patch 5 breaks edk2-redfish-client compilation
> (Instance of
> > library class [RedfishHttpLib] is not found) But I understand these changes
> are not
> > atomic for edk2 and edk2-redfish-client.
>
> Yes, I also have patch for edk2-redfish-client to use Redfish HTTP protocol. I
> had tested Redfish HTTP protocol on edk2-redfish-client. I will send out patch
> for review after Redfish HTTP protocol gets merged in edk2.
>
> Regards,
> Nickle
>
> > -Original Message-
> > From: Mike Maslenkin 
> > Sent: Thursday, February 29, 2024 1:56 AM
> > To: Nickle Wang 
> > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner Chang
> > ; Nick Ramirez 
> > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish
> HTTP
> > protocol
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Wed, Feb 28, 2024 at 2:47 PM Nickle Wang 
> wrote:
> > >
> > > Hi @Mike Maslenkin,
> > >
> > >
> > >
> > > May I have your reviewed-by if version 3 patch set look good to you?
> > >
> >
> > Sure!
> >
> > Reviewed-by: Mike Maslenkin 
> >
> > BTW I'm just curious, there is a mention in patch 2 "We currently only
> support
> > gzip Content-Encoding."
> > But I didn't see any implementation of gzip coding/encoding for edk2.
> > Do you know of any?
> >
> > I hope you know that patch 5 breaks edk2-redfish-client compilation
> (Instance of
> > library class [RedfishHttpLib] is not found) But I understand these changes
> are not
> > atomic for edk2 and edk2-redfish-client.
> >
> > Regards,
> > Mike.
> >
> >
> > >
> > >
> > > Thanks,
> > >
> > > Nickle
> > >
> > >
> > >
> > > > -Original Message-
> > >
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Nickle Wang
> > >
> > > > via groups.io
> > >
> > > > Sent: Tuesday, February 27, 2024 8:49 AM
> > >
> > > > To: Mike Maslenkin 
> > >
> > > > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner
> > > > Chang
> > >
> > > > ; Nick Ramirez 
> > >
> > > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement
> > > > Redfish HTTP
> > >
> > > > protocol
> > >
> > > >
> > >
> > > > External email: Use caution opening links or attachments
> > >
> > > >
> > >
> > > >
> > >
> > > > Thanks for your confirmation, Mike!
> > >
> > > >
> > >
> > > > Version 3 patch set is here:
> > > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
> > > >
> >
> k2.groups.io%2Fg%2Fdevel%2Fmessage%2F115985&data=05%7C02%7Cnic
> klew
> > %4
> > > >
> >
> 0nvidia.com%7Ca30038f7379c4f8dad3b08dc3886a03b%7C43083d157273
> 40c1b
> > 7d
> > > >
> >
> b39efd9ccc17a%7C0%7C0%7C638447398077724632%7CUnknown%7CTW
> FpbG
> > Zsb3d8e
> > > >
> >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7
> > C
> > > >
> >
> 0%7C%7C%7C&sdata=0tXBIuafvJuG7AM1DpAgSGGLf1QeUbOOmCq2WQCY
> peg%
> > 3D&rese
> > > > rved=0
> > >
> > > >
> > >
> > > > Regards,
> > >
> > > > Nickle
> > >
> > > >
> > >
> > > > > -Original Message-
> > >
> > > > > From: Mike Maslenkin 
> > >
> > > > > Sent: Tuesday, February 27, 2024 8:13 AM
> > >
> > > > > To: Nickle Wang 
> > >
> > > > > Cc: devel@edk2.groups.io; Igor Kulchytskyy ; Abner
> > >
> > > > > Chang ; Nick Ramirez
> 
> > >
> > > > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement
> > > > > Redfish
> > >
> > > > > HTTP protocol
> > >
> > > > >
> > >
> > > > > External email: Use caution opening links or attachments
> > >
> > > > >
> > >
> > > > >
> > >
> > > > > Hii Nickle,
> > >
> > > > >
> > >
> > > > >
> > >
> > > > > On Mon, Feb 26, 2024 at 4:44 PM Nickle Wang 
> > wrote:
> > >
> > > > > >
> > >
> > > > > > Hi Mike,
> > >
> > > > > >
> > >
> > > > > > > So finally we have
> > >
> > > > > > > HttpFreeHeaderFields (Response->Headers,
> > > > > > > Re

Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP protocol

[Igor Kulchytskyy via groups.io]

-Original Message-
From: Chang, Abner 
Sent: Wednesday, February 28, 2024 7:31 PM
To: Nickle Wang ; Mike Maslenkin 
; Igor Kulchytskyy 
Cc: devel@edk2.groups.io; Nick Ramirez 
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement 
Redfish HTTP protocol





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



[AMD Official Use Only - General]



> -Original Message-

> From: Nickle Wang mailto:nick...@nvidia.com>>

> Sent: Thursday, February 29, 2024 8:11 AM

> To: Mike Maslenkin mailto:mike.maslen...@gmail.com>>

> Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> mailto:ig...@ami.com>>; Chang, Abner

> mailto:abner.ch...@amd.com>>; Nick Ramirez 
> mailto:nrami...@nvidia.com>>

> Subject: RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish

> HTTP protocol

>

> Caution: This message originated from an External Source. Use proper caution

> when opening attachments, clicking links, or responding.

>

>

> > Sure!

> >

> > Reviewed-by: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

>

> Thanks, Mike!

>

> > But I didn't see any implementation of gzip coding/encoding for edk2.

> > Do you know of any?

>

> I just talked to Aber about this. We are working to see if we can provide gzip

> implementation in edk2 or not. It seems to me that we need 3rd party library

> to edk2 for supporting gzip. Anber, please feel free to correct me if I am 
> wrong.

Yes and we hope someone can provide the implementation. @Igor Kulchytskyy, does 
AMI has the implementation of gzip? 😊

BTW, we do have a proposal that introduces EFI_SOURCE_CODING_PROTOCOL long time 
ago while I was worked for HPE. I think we should pick up this one and promote 
this protocol in UEFI spec, we can work with AMI on this as well.



Thanks

Abner



Hi Abner,

Unfortunately, AMI does not have the implementation of gzip.

Why do you think AMI has it? 😊

I can investigate the 3rd party libraries to be adopted for using in UEFI 
environment.

Thank you,

Igor



>

> > I hope you know that patch 5 breaks edk2-redfish-client compilation

> (Instance of

> > library class [RedfishHttpLib] is not found) But I understand these changes

> are not

> > atomic for edk2 and edk2-redfish-client.

>

> Yes, I also have patch for edk2-redfish-client to use Redfish HTTP protocol. I

> had tested Redfish HTTP protocol on edk2-redfish-client. I will send out patch

> for review after Redfish HTTP protocol gets merged in edk2.

>

> Regards,

> Nickle

>

> > -Original Message-

> > From: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

> > Sent: Thursday, February 29, 2024 1:56 AM

> > To: Nickle Wang mailto:nick...@nvidia.com>>

> > Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> > mailto:ig...@ami.com>>; Abner Chang

> > mailto:abner.ch...@amd.com>>; Nick Ramirez 
> > mailto:nrami...@nvidia.com>>

> > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish

> HTTP

> > protocol

> >

> > External email: Use caution opening links or attachments

> >

> >

> > On Wed, Feb 28, 2024 at 2:47 PM Nickle Wang 
> > mailto:nick...@nvidia.com>>

> wrote:

> > >

> > > Hi @Mike Maslenkin,

> > >

> > >

> > >

> > > May I have your reviewed-by if version 3 patch set look good to you?

> > >

> >

> > Sure!

> >

> > Reviewed-by: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

> >

> > BTW I'm just curious, there is a mention in patch 2 "We currently only

> support

> > gzip Content-Encoding."

> > But I didn't see any implementation of gzip coding/encoding for edk2.

> > Do you know of any?

> >

> > I hope you know that patch 5 breaks edk2-redfish-client compilation

> (Instance of

> > library class [RedfishHttpLib] is not found) But I understand these changes

> are not

> > atomic for edk2 and edk2-redfish-client.

> >

> > Regards,

> > Mike.

> >

> >

> > >

> > >

> > > Thanks,

> > >

> > > Nickle

> > >

> > >

> > >

> > > > -Original Message-

> > >

> > > > From: devel@edk2.groups.io 
> > > > mailto:devel@edk2.groups.io>> On Behalf Of

> > > > Nickle Wang

> > >

> > > > via groups.io

> > >

> > > > Sent: Tuesday, February 27, 2024 8:49 AM

> > >

> > > > To: Mike Maslenkin 
> > > > mailto:mike.maslen...@gmail.com>>

> > >

> > > > Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> > > > mailto:ig...@ami.com>>; Abner

> > > > Chang

> > >

> > > > mailto:abner.ch...@amd.com>>; Nick Ramirez 
> > > > mailto:nrami...@nvidia.com>>

> > >

> > > > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement

> > > > Redfish HTTP

> > >

> > > > protocol

> > >

> > > >

> > >

> > > > External email: Use caution opening links or attachments

> > >

> > > >

> > >

> > > >

> > >

> > > > Thanks for your confirmation, Mike!

> > >

> > > >

> > >

> > > > Ver

Re: [edk2-devel] [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec

[Tan, Ming]

Jiewen:
  This patch is only for UEFI spec mantis 1908 change in SecureBootConfigDxe.
  This spec change just ask some drivers do some modification, it does not ask 
the HII core to be modified, so this spec change will not cause compatibility 
issue.

  For this patch, it only touch the UI setting, did not touch the modification 
of pk, kek, db, dbx, dbt EFI variable.
  I did the following unit test in EmulatorPkg WinHost.exe and Intel AlderLake 
RVP:
  1. In PK Options, Enroll PK, check the "Attempt Secure Boot" is not gray and 
enabled.
  2. In RVP, Set "Attempt Secure Boot" to enable and disable, check it does 
work, and device can boot to Windows.
  3. In PK Options, delete PK, check the "Attempt Secure Boot" is gray and 
disabled.
  4. In DBX Options, Enroll Signature, check it does work, and the "Delete All 
Signature List" is not gray.
  5. In DBX Options, Delete all signature, check it does work, and the "Delete 
All Signature List" is gray after all signatures are deleted.
  6. In KEK Options, Enroll KEK, check it does work.
  7. In KEK Options, Delete KEK, check it does work.
  8. In DB Options, Enroll Signature, check it does work.
  9. In DB Options, Delete Signature, check it does work.
  10. In DBT Options, Enroll Signature, check it does work.
  11. In DBT Options, Delete Signature, check it does work.

  @Pingle, Sneha S Would you like to help to verify the secure boot functions 
again in another Intel RVP?
  @Felix Polyudov Would you like to help to verify this patch with AMI browser? 
Since AMI submitted this UEFI spec change, if verify pass, would you like to 
add a "Reviewed-by' for this patch?
  @Bi, Dandan is HII expert and the reviewer of edk2 HII and UI modules, would 
you like to review this patch? And add a "Reviewed-by" for this patch?


  And I create a PR for this patch: 
https://github.com/tianocore/edk2/pull/5411, all CI checking are passed.

  Thank you.
  Tan Ming.


-Original Message-
From: Yao, Jiewen  
Sent: Wednesday, February 28, 2024 1:56 PM
To: Tan, Ming ; devel@edk2.groups.io
Cc: Xu, Min M 
Subject: RE: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI according to 
UEFI spec

Thanks for the update.

First, would you please clarify which test you have done for this patch set.
Have you tested all previous function to ensure it still works?

Second, would you please clarify if there is any compatibility issue to follow 
the new UEFI 2.10?
For example, what if the core HII is still UEFI 2.9? would that still work?

Third, because I am not HII expert, I would like to have HII expert to comment 
the HII/Browser related change.

Thank you
Yao, Jiewen

> -Original Message-
> From: Tan, Ming 
> Sent: Tuesday, February 27, 2024 10:59 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M ; Yao, Jiewen 
> Subject: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI 
> according to UEFI spec
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713
> 
> In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
> EFI_BROWSER_ACTION_FORM_OPEN:
> NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with 
> this browser action because question values have not been retrieved yet.
> 
> So should not call HiiGetBrowserData() and HiiSetBrowserData() in 
> FORM_OPEN call back function.
> 
> Now call SecureBootExtractConfigFromVariable() to save the change to 
> EFI variable, then HII use EFI variable to control the UI.
> 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Signed-off-by: Ming Tan 
> ---
>   V2: Change code style to pass uncrustify check.
> 
>  .../SecureBootConfigImpl.c| 37 ++-
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igIm
> pl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igIm
> pl.c
> index 2c11129526..e2e61d1e07 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igIm
> pl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igIm
> pl.c
> @@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
>  ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
> 
>}
> 
> 
> 
> +  ConfigData->ListCount = Private->ListCount;
> 
> +
> 
>//
> 
>// If it is Physical Presence User, set the PhysicalPresent to true.
> 
>//
> 
> @@ -4541,12 +4543,13 @@ SecureBootCallback (
>EFI_HII_POPUP_PROTOCOL  *HiiPopup;
> 
>EFI_HII_POPUP_SELECTION UserSelection;
> 
> 
> 
> -  Status = EFI_SUCCESS;
> 
> -  SecureBootEnable   = NULL;
> 
> -  SecureBootMode = NULL;
> 
> -  SetupMode  = NULL;
> 
> -  File   = NULL;
> 
> -  EnrollKeyErrorCode = None_Error;
> 
> +  Status   = EFI_SUCCESS;
> 
> +  SecureBootEnable = NULL;
> 
> +  SecureBootMode   = NULL;
> 
> +  SetupMode= NULL;
> 
> +  File = NULL;
> 
> +  EnrollKeyErrorCode   = None_Er

Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish HTTP protocol

[Chang, Abner via groups.io]

[AMD Official Use Only - General]



From: Igor Kulchytskyy 
Sent: Thursday, February 29, 2024 9:38 AM
To: Chang, Abner ; Nickle Wang ; Mike 
Maslenkin 
Cc: devel@edk2.groups.io; Nick Ramirez 
Subject: RE: [EXTERNAL] RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement 
Redfish HTTP protocol


[AMD Official Use Only - General]

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






-Original Message-
From: Chang, Abner 
Sent: Wednesday, February 28, 2024 7:31 PM
To: Nickle Wang ; Mike Maslenkin 
; Igor Kulchytskyy 
Cc: devel@edk2.groups.io; Nick Ramirez 
Subject: [EXTERNAL] RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement 
Redfish HTTP protocol





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



[AMD Official Use Only - General]



> -Original Message-

> From: Nickle Wang mailto:nick...@nvidia.com>>

> Sent: Thursday, February 29, 2024 8:11 AM

> To: Mike Maslenkin mailto:mike.maslen...@gmail.com>>

> Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> mailto:ig...@ami.com>>; Chang, Abner

> mailto:abner.ch...@amd.com>>; Nick Ramirez 
> mailto:nrami...@nvidia.com>>

> Subject: RE: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish

> HTTP protocol

>

> Caution: This message originated from an External Source. Use proper caution

> when opening attachments, clicking links, or responding.

>

>

> > Sure!

> >

> > Reviewed-by: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

>

> Thanks, Mike!

>

> > But I didn't see any implementation of gzip coding/encoding for edk2.

> > Do you know of any?

>

> I just talked to Aber about this. We are working to see if we can provide gzip

> implementation in edk2 or not. It seems to me that we need 3rd party library

> to edk2 for supporting gzip. Anber, please feel free to correct me if I am 
> wrong.

Yes and we hope someone can provide the implementation. @Igor Kulchytskyy, does 
AMI has the implementation of gzip? 😊

BTW, we do have a proposal that introduces EFI_SOURCE_CODING_PROTOCOL long time 
ago while I was worked for HPE. I think we should pick up this one and promote 
this protocol in UEFI spec, we can work with AMI on this as well.



Thanks

Abner



Hi Abner,

Unfortunately, AMI does not have the implementation of gzip.

Why do you think AMI has it? 😊

I can investigate the 3rd party libraries to be adopted for using in UEFI 
environment.

Thank you,

Igor



I thought AMI has the implementation as you and Nickle considered the content 
encoded parameter for the Redfish HTTP protocol. I guess you have gzip 
implemented in AMI BIOS as Nvidia doesn’t. 😊



Abner



>

> > I hope you know that patch 5 breaks edk2-redfish-client compilation

> (Instance of

> > library class [RedfishHttpLib] is not found) But I understand these changes

> are not

> > atomic for edk2 and edk2-redfish-client.

>

> Yes, I also have patch for edk2-redfish-client to use Redfish HTTP protocol. I

> had tested Redfish HTTP protocol on edk2-redfish-client. I will send out patch

> for review after Redfish HTTP protocol gets merged in edk2.

>

> Regards,

> Nickle

>

> > -Original Message-

> > From: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

> > Sent: Thursday, February 29, 2024 1:56 AM

> > To: Nickle Wang mailto:nick...@nvidia.com>>

> > Cc: devel@edk2.groups.io; Igor Kulchytskyy 
> > mailto:ig...@ami.com>>; Abner Chang

> > mailto:abner.ch...@amd.com>>; Nick Ramirez 
> > mailto:nrami...@nvidia.com>>

> > Subject: Re: [edk2-devel] [PATCH v2 2/6] RedfishPkg: implement Redfish

> HTTP

> > protocol

> >

> > External email: Use caution opening links or attachments

> >

> >

> > On Wed, Feb 28, 2024 at 2:47 PM Nickle Wang 
> > mailto:nick...@nvidia.com>>

> wrote:

> > >

> > > Hi @Mike Maslenkin,

> > >

> > >

> > >

> > > May I have your reviewed-by if version 3 patch set look good to you?

> > >

> >

> > Sure!

> >

> > Reviewed-by: Mike Maslenkin 
> > mailto:mike.maslen...@gmail.com>>

> >

> > BTW I'm just curious, there is a mention in patch 2 "We currently only

> support

> > gzip Content-Encoding."

> > But I didn't see any implementation of gzip coding/encoding for edk2.

> > Do you know of any?

> >

> > I hope you know that patch 5 breaks edk2-redfish-client compilation

> (Instance of

> > library class [RedfishHttpLib] is not found) But I understand these changes

> are not

> > atomic for edk2 and edk2-redfish-client.

> >

> > Regards,

> > Mike.

> >

> >

> > >

> > >

> > > Thanks,

> > >

> > > Nickle

> > >

> > >

> > >

> > > > -Original Message-

> > >

> > > > From: devel@edk2.groups.io 
> > > > mailto:devel@edk2.groups.io>> On Behalf Of

> > > > Nickle Wang

> > >

> > > > via groups.io

> > >

> > > > Sent: Tuesday, F

Re: [edk2-devel] [PATCH] IntelSiliconPkg/VTd: Reset the one-shot bits before modifing GCMD_REG

[Huang, Jenny]

Reviewed-by: Jenny Huang 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sheng, W
Sent: Monday, February 19, 2024 7:37 PM
To: devel@edk2.groups.io
Cc: Ni, Ray ; Chaganty, Rangasai V 
; Huang, Jenny 
Subject: [edk2-devel] [PATCH] IntelSiliconPkg/VTd: Reset the one-shot bits 
before modifing GCMD_REG

Here is the process of modify GCMD_REG.
  Read GSTS_REG
  Reset the one-shot bits.
  Modify the target comamnd value.
  Write the command value to GCMD_REG.
  Wait until GSTS_REG indicates command is serviced.

Cc: Ray Ni 
Cc: Rangasai V Chaganty 
Cc: Jenny Huang 
Signed-off-by: Sheng Wei 
---
 .../Feature/VTd/IntelVTdCoreDxe/VtdReg.c  | 13 ++
 .../VTd/IntelVTdCorePei/IntelVTdDmar.c|  9 +---
 .../VTd/IntelVTdDmarPei/IntelVTdDmar.c| 43 +-
 .../Feature/VTd/IntelVTdDxe/VtdReg.c  | 44 +--
 .../Feature/VTd/IntelVTdPmrPei/VtdReg.c   |  1 +
 .../IntelVTdPeiDxeLib/IntelVTdPeiDxeLib.c | 12 ++---
 6 files changed, 51 insertions(+), 71 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
index edeb4b3ff..21e2d5f1b 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
@@ -112,13 +112,8 @@ PerpareCacheInvalidationInterface (
   // Enable the queued invalidation interface through the Global Command 
Register.

   // When enabled, hardware sets the QIES field in the Global Status Register.

   //

-  Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);

-  Reg32 |= B_GMCD_REG_QIE;

-  MmioWrite32 (VtdUnitBaseAddress + R_GCMD_REG, Reg32);

-  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface. GCMD_REG = 
0x%x\n", Reg32));

-  do {

-Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);

-  } while ((Reg32 & B_GSTS_REG_QIES) == 0);

+  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface.\n"));

+  VtdLibSetGlobalCommandRegisterBits (VtdUnitBaseAddress, B_GMCD_REG_QIE);

 

   VTdLogAddEvent (VTDLOG_DXE_QUEUED_INVALIDATION, VTD_LOG_QI_ENABLE, 
VtdUnitBaseAddress);

 

@@ -577,7 +572,7 @@ DumpVtdCapRegs (
   IN VTD_CAP_REG *CapReg

   )

 {

-  DEBUG((DEBUG_INFO, "  CapReg   - 0x%x\n", CapReg->Uint64));

+  DEBUG((DEBUG_INFO, "  CapReg   - 0x%lx\n", CapReg->Uint64));

   DEBUG((DEBUG_INFO, "ND - 0x%x\n", CapReg->Bits.ND));

   DEBUG((DEBUG_INFO, "AFL- 0x%x\n", CapReg->Bits.AFL));

   DEBUG((DEBUG_INFO, "RWBF   - 0x%x\n", CapReg->Bits.RWBF));

@@ -737,7 +732,7 @@ DumpVtdIfError (
 if (HasError) {

   REPORT_STATUS_CODE (EFI_ERROR_CODE, PcdGet32 (PcdErrorCodeVTdError));

   DEBUG((DEBUG_INFO, "\n ERROR \n"));

-  DumpVtdRegs (mVtdUnitInformation[Num].VtdUnitBaseAddress);
+  DumpVtdRegs (mVtdUnitInformation[Num].VtdUnitBaseAddress);

   DEBUG((DEBUG_INFO, " ERROR \n\n"));

   //

   // Clear

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
index 93207ba52..549313dbf 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
@@ -120,13 +120,8 @@ PerpareCacheInvalidationInterface (
   // Enable the queued invalidation interface through the Global Command 
Register.

   // When enabled, hardware sets the QIES field in the Global Status Register.

   //

-  Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);

-  Reg32 |= B_GMCD_REG_QIE;

-  MmioWrite32 (VtdUnitBaseAddress + R_GCMD_REG, Reg32);

-  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface. GCMD_REG = 
0x%x\n", Reg32));

-  do {

-Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);

-  } while ((Reg32 & B_GSTS_REG_QIES) == 0);

+  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface.\n"));

+  VtdLibSetGlobalCommandRegisterBits (VtdUnitBaseAddress, B_GMCD_REG_QIE);

 

   VTdLogAddEvent (VTDLOG_PEI_QUEUED_INVALIDATION, VTD_LOG_QI_ENABLE, 
VtdUnitBaseAddress);

 

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
index e1b867973..533fb2b9a 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
@@ -20,6 +20,18 @@
 #include 

 #include "IntelVTdDmarPei.h"

 

+VOID

+SetGlobalCommandRegisterBits (

+  IN UINTN VtdUnitBaseAddress,

+  IN UINT32BitMask

+  );

+

+VOID

+ClearGlobalCommandRegisterBits (

+  IN UINTN VtdUnitBaseAddress,

+  IN UINT32BitMask

+  );

+

 /**

   Flush VTD page table and context table memory.

 

@@ -58,6 +70,7 @@ FlushWriteBuffer (
 

   if (CapReg.Bits.RWBF != 0) {

 Reg32 = MmioRead32 (VtdUnit

Re: [edk2-devel] [PATCH 02/10] OvmfPkg/ResetVector: add ClearOvmfPageTables macro

[Laszlo Ersek]

On 2/28/24 09:22, Gerd Hoffmann wrote:
> On Wed, Feb 28, 2024 at 05:09:32AM +0100, Laszlo Ersek wrote:
>> On 2/22/24 12:54, Gerd Hoffmann wrote:
>>> Move code to clear the page tables to a nasm macro.
>>> No functional change.
>>>
>>> Signed-off-by: Gerd Hoffmann 
>>> ---
>>>  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 35 ---
>>>  1 file changed, 19 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
>>> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> index 6fec6f2beeea..378ba2feeb4f 100644
>>> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>>> @@ -45,6 +45,24 @@ BITS32
>>>  %define TDX_BSP 1
>>>  %define TDX_AP  2
>>>  
>>> +;
>>> +; For OVMF, build some initial page tables at
>>> +; PcdOvmfSecPageTablesBase - (PcdOvmfSecPageTablesBase + 0x6000).
>>> +;
>>> +; This range should match with PcdOvmfSecPageTablesSize which is
>>> +; declared in the FDF files.
>>> +;
>>> +; At the end of PEI, the pages tables will be rebuilt into a
>>> +; more permanent location by DxeIpl.
>>> +;
>>> +%macro ClearOvmfPageTables 0
>>> +mov ecx, 6 * 0x1000 / 4
>>> +xor eax, eax
>>> +.clearPageTablesMemoryLoop:
>>> +mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
>>> +loop.clearPageTablesMemoryLoop
>>> +%endmacro
>>> +
>>>  ;
>>>  ; Modified:  EAX, EBX, ECX, EDX
>>>  ;
>>
>> Ah, this made me read up on local labels:
>>
>> https://www.nasm.us/xdoc/2.16.01/html/nasmdoc3.html#section-3.9
>>
>> Should we rather call the label
>>
>>   ..@clearPageTablesMemoryLoop
>>
>> ?
> 
> I've tried that and something (which I don't remember) didn't work as
> expected.  Given that each branch which uses that macro will have a jump
> label anyway (so the local label is expanded to something like
> TdxBspInit.clearPageTablesMemoryLoop) I've figured this is good enough

Sure, if you've tried it already, then the current method is fine!

Thanks!
Laszlo



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

Re: [edk2-devel] [PATCH] IntelSiliconPkg/VTd: Reset the one-shot bits before modifing GCMD_REG

[Sheng Wei]

Hi Ray,
Could you help to review and merge this Vtd driver patch to edk2platforms 
branch?
This patch is used to fix a bug about missing to mask one-shot bits when write 
VTD GCMD_REG register.
Here is the PR of this patch.
https://github.com/tianocore/edk2-platforms/pull/125
Thank you.
BR
Sheng Wei

> -Original Message-
> From: Huang, Jenny 
> Sent: Thursday, February 29, 2024 3:10 PM
> To: devel@edk2.groups.io; Sheng, W 
> Cc: Ni, Ray ; Chaganty, Rangasai V
> 
> Subject: RE: [edk2-devel] [PATCH] IntelSiliconPkg/VTd: Reset the one-shot bits
> before modifing GCMD_REG
> 
> Reviewed-by: Jenny Huang 
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sheng, W
> Sent: Monday, February 19, 2024 7:37 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Chaganty, Rangasai V
> ; Huang, Jenny 
> Subject: [edk2-devel] [PATCH] IntelSiliconPkg/VTd: Reset the one-shot bits
> before modifing GCMD_REG
> 
> Here is the process of modify GCMD_REG.
>   Read GSTS_REG
>   Reset the one-shot bits.
>   Modify the target comamnd value.
>   Write the command value to GCMD_REG.
>   Wait until GSTS_REG indicates command is serviced.
> 
> Cc: Ray Ni 
> Cc: Rangasai V Chaganty 
> Cc: Jenny Huang 
> Signed-off-by: Sheng Wei 
> ---
>  .../Feature/VTd/IntelVTdCoreDxe/VtdReg.c  | 13 ++
>  .../VTd/IntelVTdCorePei/IntelVTdDmar.c|  9 +---
>  .../VTd/IntelVTdDmarPei/IntelVTdDmar.c| 43 +-
>  .../Feature/VTd/IntelVTdDxe/VtdReg.c  | 44 +--
>  .../Feature/VTd/IntelVTdPmrPei/VtdReg.c   |  1 +
>  .../IntelVTdPeiDxeLib/IntelVTdPeiDxeLib.c | 12 ++---
>  6 files changed, 51 insertions(+), 71 deletions(-)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
> index edeb4b3ff..21e2d5f1b 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCoreDxe/VtdReg.c
> @@ -112,13 +112,8 @@ PerpareCacheInvalidationInterface (
>// Enable the queued invalidation interface through the Global Command
> Register.
> 
>// When enabled, hardware sets the QIES field in the Global Status 
> Register.
> 
>//
> 
> -  Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
> 
> -  Reg32 |= B_GMCD_REG_QIE;
> 
> -  MmioWrite32 (VtdUnitBaseAddress + R_GCMD_REG, Reg32);
> 
> -  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface. GCMD_REG =
> 0x%x\n", Reg32));
> 
> -  do {
> 
> -Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
> 
> -  } while ((Reg32 & B_GSTS_REG_QIES) == 0);
> 
> +  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface.\n"));
> 
> +  VtdLibSetGlobalCommandRegisterBits (VtdUnitBaseAddress,
> B_GMCD_REG_QIE);
> 
> 
> 
>VTdLogAddEvent (VTDLOG_DXE_QUEUED_INVALIDATION,
> VTD_LOG_QI_ENABLE, VtdUnitBaseAddress);
> 
> 
> 
> @@ -577,7 +572,7 @@ DumpVtdCapRegs (
>IN VTD_CAP_REG *CapReg
> 
>)
> 
>  {
> 
> -  DEBUG((DEBUG_INFO, "  CapReg   - 0x%x\n", CapReg->Uint64));
> 
> +  DEBUG((DEBUG_INFO, "  CapReg   - 0x%lx\n", CapReg->Uint64));
> 
>DEBUG((DEBUG_INFO, "ND - 0x%x\n", CapReg->Bits.ND));
> 
>DEBUG((DEBUG_INFO, "AFL- 0x%x\n", CapReg->Bits.AFL));
> 
>DEBUG((DEBUG_INFO, "RWBF   - 0x%x\n", CapReg->Bits.RWBF));
> 
> @@ -737,7 +732,7 @@ DumpVtdIfError (
>  if (HasError) {
> 
>REPORT_STATUS_CODE (EFI_ERROR_CODE, PcdGet32
> (PcdErrorCodeVTdError));
> 
>DEBUG((DEBUG_INFO, "\n ERROR \n"));
> 
> -  DumpVtdRegs (mVtdUnitInformation[Num].VtdUnitBaseAddress);
> +  DumpVtdRegs (mVtdUnitInformation[Num].VtdUnitBaseAddress);
> 
>DEBUG((DEBUG_INFO, " ERROR \n\n"));
> 
>//
> 
>// Clear
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
> index 93207ba52..549313dbf 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdCorePei/IntelVTdDmar.c
> @@ -120,13 +120,8 @@ PerpareCacheInvalidationInterface (
>// Enable the queued invalidation interface through the Global Command
> Register.
> 
>// When enabled, hardware sets the QIES field in the Global Status 
> Register.
> 
>//
> 
> -  Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
> 
> -  Reg32 |= B_GMCD_REG_QIE;
> 
> -  MmioWrite32 (VtdUnitBaseAddress + R_GCMD_REG, Reg32);
> 
> -  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface. GCMD_REG =
> 0x%x\n", Reg32));
> 
> -  do {
> 
> -Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
> 
> -  } while ((Reg32 & B_GSTS_REG_QIES) == 0);
> 
> +  DEBUG ((DEBUG_INFO, "Enable Queued Invalidation Interface.\n"));
> 
> +  VtdLibSetGlobalCommandRegisterBits (VtdUnitBaseAddress,
> B_GMCD_REG_QIE);
> 
> 
> 
>VTdLogAddEvent (VTDLOG_PEI_QUEUED_I

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE

[Laszlo Ersek]

On 2/28/24 17:27, Albecki, Mateusz wrote:
> Sorry, I didn't want to make an impression that I expected solution to
> be delivered, I was merely trying to explain some of the complexity we
> are trying to handle on our side and why we didn't went for SerialDxe
> and instead opted to make PciSioSerialDxe work for our use case. Anyway
> thanks for the ideas, we will definitely look into implementing step h
> you mentioned in case other maintainers also disagree with this patch.

... BTW IIRC Mike recommended producing SerialIo protocol instances in
your platform driver, rather than PciIo, for your debug ports. I feel a
bit torn about that; on one hand, the uniformity of SerialIo protocols
looks flexible, on the other hand, how would you distinguish the debug
ports from the "normal" serial ports (mixing debug output with UEFI
console IO is not great; speaking from experience). Then you'd need to
get into DevicePath parsing (or call PciIo->GetLocation), and/or make
sure your ConIn/ConOut/StdErr variables wouldn't include the debug
ports... It's hard to form an opinion not knowing the platform and the
goals specifically. But, anyway, I've kind of run out of steam on this.

Laszlo



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