Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info

[Zhenyu Zhang]

> My suggestion is to wrap the newly added DEBUG statement with an if:
>
>   if (OptionIndex == -1) {
> Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> +if (Status == EFI_OUT_OF_RESOURCES) {
> +  DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
> +}
> ASSERT_EFI_ERROR (Status);
>   }
>
> The ASSERT_EFI_ERROR was already there. It will halt the execution in
> case Status contains an error. Aparently the firmware is not able to
> continue with a full variable store.

Agreed, I'll add it in the next patch.

> > Why not return ERROR?
> >

> This is in VOID PlatformRegisterFvBootOption (), returning ERROR is not
> possible.

Indeed so.

Best wishes,
 Zhenyu

On Thu, Sep 7, 2023 at 9:25 PM Oliver Steffen  wrote:
>
> Quoting Yao, Jiewen (2023-09-07 14:26:31)
> > I don't think using ASSERT is a good idea here.
> >
>
> Some context:
>
> We observed that EDK2 hits an ASSERT (Out of Resources) when booting
> with a full variable store. The message provided in this case is not
> helpful for non-experts.
> I think the goal here is to emit a more user friendly message.
>
> My suggestion is to wrap the newly added DEBUG statement with an if:
>
>if (OptionIndex == -1) {
>  Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> +if (Status == EFI_OUT_OF_RESOURCES) {
> +  DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
> +}
>  ASSERT_EFI_ERROR (Status);
>}
>
> The ASSERT_EFI_ERROR was already there. It will halt the execution in
> case Status contains an error. Aparently the firmware is not able to
> continue with a full variable store.
>
> > Why not return ERROR?
> >
>
> This is in VOID PlatformRegisterFvBootOption (), returning ERROR is not
> possible.
>
> Thanks,
>  Oliver
>
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Zhenyu
> > > Zhang
> > > Sent: Thursday, September 7, 2023 7:11 PM
> > > To: devel@edk2.groups.io
> > > Cc: zheny...@redhat.com; ostef...@redhat.com; kra...@redhat.com;
> > > marcandre.lur...@redhat.com; stef...@linux.ibm.com;
> > > anthony.per...@citrix.com; jul...@xen.org
> > > Subject: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full 
> > > debug
> > > info
> > >
> > > From: "Zhenyu Zhang" 
> > >
> > > When the variable store is full, edk2 will directly assert.
> > > Add debug information to help users understand what caused
> > > the assertion.
> > >
> > >  Actual results:
> > >  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
> > >  48BCD90AD31A - 0x0003 - 0x7E
> > >  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize =
> > >  0x3FF98
> > >  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
> > >  48BCD90AD31A - 0x0003 - 0x92
> > >  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = 0x3FF98
> > >
> > >  Synchronous Exception at 0x00023CA60374
> > >  ..
> > >  ASSERT_EFI_ERROR (Status = Out of Resources)
> > >  ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/
> > >  PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_
> > >  STATUS)(Status)) < 0)
> > >
> > > Cc: Oliver Steffen 
> > > Cc: Gerd Hoffmann 
> > > Cc: Marc-André Lureau 
> > > Cc: Stefan Berger 
> > > Cc: Anthony Perard 
> > > Cc: Julien Grall 
> > > Signed-off-by: Zhenyu Zhang 
> > > ---
> > >  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > > b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > > index 8dc2bbf97371..c95c7931a3f5 100644
> > > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > > @@ -139,6 +139,7 @@ PlatformRegisterFvBootOption (
> > >
> > >if (OptionIndex == -1) {
> > >  Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> > > +DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
> > >  ASSERT_EFI_ERROR (Status);
> > >}
> > >
> > > --
> > > 2.39.3
> > >
>



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

[Wu, Hao A]

Thanks. I am fine with the proposal below.

Could you please help to follow the step 11 in page: 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
and check the CI test results for the upcoming patch?

Best Regards,
Hao Wu

> -Original Message-
> From: Henz, Patrick 
> Sent: Friday, September 8, 2023 2:39 AM
> To: devel@edk2.groups.io; Wu, Hao A ; Mike
> Maslenkin 
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> Performance Timer for XHCI Timeouts
> 
> I don’t have strong opinions either. If we would prefer to cache the values
> returned by GetPerformanceCounterProperties I would be more than happy
> to do that. I could also modify XhcGetElapsedTime to return the elapsed ticks
> instead of the elapsed time in nano seconds, the functions that call into
> XhcGetElapedTime/Ticks could convert the millisecond timeout value into a
> tick count and compare the elapsed ticks against that, this would remove the
> reliance on GetTimeInNanoSecond. Does that sound more appropriate?
> 
> Thanks,
> Patrick Henz
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu, Hao
> A
> Sent: Thursday, September 7, 2023 2:48 AM
> To: Mike Maslenkin ; devel@edk2.groups.io
> Cc: Henz, Patrick 
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> Performance Timer for XHCI Timeouts
> 
> I do not have strong opinion on this considering it is an IO driver.
> The call of GetTimeInNanoSecond() is also likely to invoke
> GetPerformanceCounterProperties() call.
> 
> I will let the patch owner to decide on the open raised below.
> 
> Best Regards,
> Hao Wu
> 
> > -Original Message-
> > From: Mike Maslenkin 
> > Sent: Thursday, September 7, 2023 3:36 PM
> > To: devel@edk2.groups.io; Wu, Hao A 
> > Cc: Henz, Patrick 
> > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > Performance Timer for XHCI Timeouts
> >
> > Hello Hao Wu,
> >
> > Isn't GetPerformanceCounterProperties (, ) call
> > too "heavy" for XHCI paths?
> > May be it's better to cache timer direction once?
> > I'm asking in a context of PcAtChipsetPkg/Library/AcpiTimerLib
> > instance used by a number of Intel's platform in edk-platforms.
> > For this  library GetPerformanceCounterProperties finally calls
> > InternalCalculateTscFrequency, that isn't simple.
> >
> > Regards,
> > Mike
> >
> > On Thu, Sep 7, 2023 at 4:27 AM Wu, Hao A  wrote:
> > >
> > > Sorry for the late response.
> > > Reviewed-by: Hao A Wu 
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Henz, Patrick
> > > > Sent: Thursday, September 7, 2023 4:37 AM
> > > > To: devel@edk2.groups.io
> > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > > Performance Timer for XHCI Timeouts
> > > >
> > > > I sent this patch out a few weeks ago now but haven't seen a
> > > > reply, just checking in to make sure it didn't get missed.
> > > >
> > > > Thanks,
> > > > Patrick Henz
> > > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Henz, Patrick
> > > > Sent: Monday, August 14, 2023 10:52 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Henz, Patrick 
> > > > Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > > Performance Timer for XHCI Timeouts
> > > >
> > > > REF:https://urldefense.com/v3/__https://bugzilla.tianocore.org/sho
> > > > w_bug.cgi?id=2948__;!!NpxR!mZRuMBKPdeR-
> UvY7sr8tWNOfIQDe0W_TW3q-K8M
> > > > tNN1cynyRZ1_bls8osaEqFWupFmjR29X_zvw0Zx1Y$
> > > >
> > > > XhciDxe uses the timer functionality provided by the boot services
> > > > table to detect timeout conditions. This breaks the driver's
> > > > ExitBootServices call back, as CoreExitBootServices halts the
> > > > timer before signaling the ExitBootServices event. If the host
> > > > controller fails to halt in the call back, the timeout condition
> > > > will never occur and the boot gets stuck in an indefinite spin
> > > > loop. Use the free running timer provided by TimerLib to calculate
> > > > timeouts, avoiding
> > the potential hang.
> > > >
> > > > Signed-off-by: Patrick Henz 
> > > > Reviewed-by:
> > > > ---
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +-
> -
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
> > > > +---
> > > >  5 files changed, 129 insertions(+), 86 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > index 62682dd27c..1dcbe20512 100644
> > > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > @@ -1,6 +1,7 @@
> > > >  /** @file
> > > >The XHCI controller driver.
> > > >
> > > > 

[edk2-devel] [PATCH v1 1/1] MdeModulePkg/BootMaintenanceManagerUiLib: Check array index before access

[Michael Kubacki]

From: Michael Kubacki 

Many arrays are defined with a length of MAX_MENU_NUMBER in
FormGuid.h. Two of those are BootOptionOrder and DriverOptionOrder.

In UpdatePage.c, a pointer is set to either of those arrays. The
array buffer is accessed using an index whose range is checked after
the pointer to the array is dereferenced. This change moves the check
before the dereference.

In another place in the file, the ConsoleCheck pointer is also set to
an array buffer with MAX_MENU_NUMBER elements. Only an ASSERT()
currently checks the range of the array index. This change
conditionalizes the pointer dereference itself on the range of Index.

Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
Cc: Eric Dong 
Signed-off-by: Michael Kubacki 
---
 MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c 
b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
index ca81b7f35264..b1d1e2ee44f4 100644
--- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
+++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
@@ -527,9 +527,12 @@ UpdateConsolePage (
 ((NewTerminalContext->IsStdErr != 0) && (UpdatePageId == 
FORM_CON_ERR_ID))
 )
 {
-  CheckFlags |= EFI_IFR_CHECKBOX_DEFAULT;
-  ConsoleCheck[Index] = TRUE;
-} else {
+  CheckFlags |= EFI_IFR_CHECKBOX_DEFAULT;
+
+  if (Index < MAX_MENU_NUMBER) {
+ConsoleCheck[Index] = TRUE;
+  }
+} else if (Index < MAX_MENU_NUMBER) {
   ConsoleCheck[Index] = FALSE;
 }
 
@@ -622,7 +625,7 @@ UpdateOrderPage (
   ASSERT (OptionsOpCodeHandle != NULL);
 
   NewMenuEntry = NULL;
-  for (OptionIndex = 0; (OptionOrder[OptionIndex] != 0 && OptionIndex < 
MAX_MENU_NUMBER); OptionIndex++) {
+  for (OptionIndex = 0; (OptionIndex < MAX_MENU_NUMBER && 
OptionOrder[OptionIndex] != 0); OptionIndex++) {
 BootOptionFound = FALSE;
 for (Index = 0; Index < OptionMenu->MenuNumber; Index++) {
   NewMenuEntry = BOpt_GetMenuEntry (OptionMenu, Index);
-- 
2.42.0.windows.2



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

Re: [edk2-devel] [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg :Updated PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

[Nate DeSimone]

Pushed: https://github.com/tianocore/edk2-platforms/commit/bb6841e

-Original Message-
From: Ramkumar Krishnamoorthi  
Sent: Thursday, August 31, 2023 10:53 AM
To: devel@edk2.groups.io
Cc: DOPPALAPUDI, HARIKRISHNA ; Ponnusamy, Suresh 
; KARPAGAVINAYAGAM, MANICKAVASAKAM 
; Venkatesan, Selvaraj ; Ramkumar 
Krishnamoorthi ; Isaac Oram ; 
Desimone, Nathaniel L 
Subject: [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg :Updated 
PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

Subject: [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg : Updated  
PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

Fix to resolve the build error for JunctionCity and Aowanda platform.
Updated PcdFlashFvSecuritySize in JunctionCity\PlatformPkg.fdf and 
Aowanda\PlatformPkg.fdf


Cc: Harikrishna Doppalapudi 
Cc: Sureshkumar Ponnusamy 
Cc: Manickavasakam Karpagavinayagam 
Cc: Selvaraj V 
Cc: Ramkumar Krishnamoorthi 

Cc: Isaac Oram 

Cc: Nate DeSimone 



Signed-off-by: Ramkumar Krishnamoorthi 
---
 Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf  | 2 +-
 Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf 
b/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
index 2d3cd094fd..3fe78b7e6f 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
@@ -189,7 +189,7 @@ SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize 
= 0x0100
   # This makes it easy to adjust the various sizes without having to manually 
calculate the offsets.
   # These are out of flash layout order because FvAdvanced gets any remaining 
space
   #
-  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0004
+  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0005
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize= 0x0023
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize  = 0x0004C000
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvAdvancedSize  = 
gCpPlatFlashTokenSpaceGuid.PcdFlashFdMainSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf 
b/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
index c63442508b..e120ff143a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
@@ -189,7 +189,7 @@ SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize 
= 0x0100
   # This makes it easy to adjust the various sizes without having to manually 
calculate the offsets.
   # These are out of flash layout order because FvAdvanced gets any remaining 
space
   #
-  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0004
+  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0005
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize= 0x0023
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize  = 0x0004C000
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvAdvancedSize  = 
gCpPlatFlashTokenSpaceGuid.PcdFlashFdMainSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize
--
2.37.1.windows.1
-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited. Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.


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

Re: [edk2-devel] [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg :Updated PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

[Nate DeSimone]

Reviewed-by: Nate DeSimone 

-Original Message-
From: Ramkumar Krishnamoorthi  
Sent: Thursday, August 31, 2023 10:53 AM
To: devel@edk2.groups.io
Cc: DOPPALAPUDI, HARIKRISHNA ; Ponnusamy, Suresh 
; KARPAGAVINAYAGAM, MANICKAVASAKAM 
; Venkatesan, Selvaraj ; Ramkumar 
Krishnamoorthi ; Isaac Oram ; 
Desimone, Nathaniel L 
Subject: [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg :Updated 
PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

Subject: [edk2-platfoms][PATCH V1] WhitleyOpenBoardPkg : Updated  
PcdFlashFvSecuritySize for JunctionCity and Aowanda platform

Fix to resolve the build error for JunctionCity and Aowanda platform.
Updated PcdFlashFvSecuritySize in JunctionCity\PlatformPkg.fdf and 
Aowanda\PlatformPkg.fdf


Cc: Harikrishna Doppalapudi 
Cc: Sureshkumar Ponnusamy 
Cc: Manickavasakam Karpagavinayagam 
Cc: Selvaraj V 
Cc: Ramkumar Krishnamoorthi 

Cc: Isaac Oram 

Cc: Nate DeSimone 



Signed-off-by: Ramkumar Krishnamoorthi 
---
 Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf  | 2 +-
 Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf 
b/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
index 2d3cd094fd..3fe78b7e6f 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Aowanda/PlatformPkg.fdf
@@ -189,7 +189,7 @@ SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize 
= 0x0100
   # This makes it easy to adjust the various sizes without having to manually 
calculate the offsets.
   # These are out of flash layout order because FvAdvanced gets any remaining 
space
   #
-  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0004
+  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0005
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize= 0x0023
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize  = 0x0004C000
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvAdvancedSize  = 
gCpPlatFlashTokenSpaceGuid.PcdFlashFdMainSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf 
b/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
index c63442508b..e120ff143a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/JunctionCity/PlatformPkg.fdf
@@ -189,7 +189,7 @@ SET gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize 
= 0x0100
   # This makes it easy to adjust the various sizes without having to manually 
calculate the offsets.
   # These are out of flash layout order because FvAdvanced gets any remaining 
space
   #
-  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0004
+  SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize  = 0x0005
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize= 0x0023
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize  = 0x0004C000
   SET gMinPlatformPkgTokenSpaceGuid.PcdFlashFvAdvancedSize  = 
gCpPlatFlashTokenSpaceGuid.PcdFlashFdMainSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvUefiBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvOsBootSize - 
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvSecuritySize
--
2.37.1.windows.1
-The information contained in this message may be confidential and proprietary 
to American Megatrends (AMI). This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited. Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.


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

Re: [edk2-devel] [PATCH v5] MdeModulePkg/HiiDatabase: Fix incorrect AllocateCopyPool size

[Dandan Bi]

Reviewed-by: Dandan Bi 


Thanks,
Dandan

-Original Message-
From: Mike Beaton  
Sent: Thursday, September 7, 2023 11:35 AM
To: devel@edk2.groups.io
Cc: Dong, Eric ; Bi, Dandan ; Ard 
Biesheuvel ; Mike Beaton 
Subject: [PATCH v5] MdeModulePkg/HiiDatabase: Fix incorrect AllocateCopyPool 
size

The immediately preceding call, GetBestLanguage, plus the implementation of 
HiiGetString, which is called immediately afterwards, make it clear that 
BestLanguage is a null-terminated ASCII string, and not just a five byte, 
non-null terminated buffer.

Therefore AsciiStrLen is one byte too short, meaning that whether the space 
allocated is really sufficient and whether the resultant string is really 
null-terminated becomes implementation-dependent. Rather than switching to 
AsciiStrSize, we use an explicitly compile-time string length calculation (both 
compile-time and run-time approaches are currently used elsewhere in the 
codebase for copying static strings).

Signed-off-by: Mike Beaton 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
index 96e05d4cf9..6e791783a6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
@@ -1987,7 +1987,7 @@ GetNameFromId (
NULL
);
   if (BestLanguage == NULL) {
-BestLanguage = AllocateCopyPool (AsciiStrLen ("en-US"), "en-US");
+BestLanguage = AllocateCopyPool (sizeof ("en-US"), "en-US");
 ASSERT (BestLanguage != NULL);
   }
 
--
2.41.0



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

Re: 回复: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib: Check array index before access

[Michael Kubacki]

Hi Liming,

I'm running the CodeQL CLI 
(https://docs.github.com/en/code-security/codeql-cli/getting-started-with-the-codeql-cli) 
locally against the code with some new queries.


The queries in the codeql/cpp-queries pack listed here are relatively 
easy to experiment with https://codeql.github.com/codeql-query-help/cpp/.


The particular query related to this patch was 
https://codeql.github.com/codeql-query-help/cpp/cpp-offset-use-before-range-check/.


Thanks,
Michael

On 9/7/2023 8:40 PM, gaoliming wrote:

Michael:
  How do you detect those issues? Do you use the tool or do code review?

  For this change,  Reviewed-by: Liming Gao 


-邮件原件-
发件人: devel@edk2.groups.io  代表 Michael
Kubacki
发送时间: 2023年9月7日 1:41
收件人: devel@edk2.groups.io
抄送: Zhichao Gao ; Michael D Kinney

主题: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib:
Check array index before access

From: Michael Kubacki 

Moves the range check for the index into the array before attempting
any accesses using the array index.

Cc: Zhichao Gao 
Cc: Michael D Kinney 
Signed-off-by: Michael Kubacki 
---
  ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
index 7c80bba46581..5cb92c485b47 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
@@ -382,7 +382,7 @@ IfConfig6PrintIpAddr (

ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
(STR_IFCONFIG6_INFO_COLON), gShellNetwork2HiiHandle);

-  while ((Ip->Addr[Index] == 0) && (Ip->Addr[Index + 1] == 0) &&
(Index < PREFIXMAXLEN)) {
+  while ((Index < PREFIXMAXLEN) && (Ip->Addr[Index] == 0) &&
(Ip->Addr[Index + 1] == 0)) {
  Index = Index + 2;
  if (Index > PREFIXMAXLEN - 2) {
break;
--
2.42.0.windows.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108336):
https://edk2.groups.io/g/devel/message/108336
Mute This Topic: https://groups.io/mt/101198333/4905953
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaolim...@byosoft.com.cn]
-=-=-=-=-=-=







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

回复: [edk2-devel] [PATCH 0/2] Add New Intel Processor family

[gaoliming via groups.io]

Avinash:
  Seemly, this patch set doesn't include the change in ShellPkg. 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Avinash
> 发送时间: 2023年9月8日 3:00
> 收件人: devel@edk2.groups.io
> 主题: [edk2-devel] [PATCH 0/2] Add New Intel Processor family
> 
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> *** BLURB HERE ***
> 
> Avinash (1):
>   MdePkg/SmBios.h : Add New Intel Processor family
> 
> Nhi Pham (1):
>   ShellPkg/SmbiosView: Update display of PCIe system slot ID
> 
>  ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c | 2
> +-
>  MdePkg/Include/IndustryStandard/SmBios.h
> | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> --
> 2.37.3.windows.1
> 
> 
> 
> 
> 





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

回复: [edk2-devel] [PATCH v5] MdeModulePkg/HiiDatabase: Fix incorrect AllocateCopyPool size

[gaoliming via groups.io]

Mike:
  Thanks for your detail information in the commit message. This change is
very clear. 

  Reviewed-by: Liming Gao 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Mike Beaton
> 发送时间: 2023年9月7日 11:35
> 收件人: devel@edk2.groups.io
> 抄送: Eric Dong ; Dandan Bi ;
> Ard Biesheuvel ; Mike Beaton 
> 主题: [edk2-devel] [PATCH v5] MdeModulePkg/HiiDatabase: Fix incorrect
> AllocateCopyPool size
> 
> The immediately preceding call, GetBestLanguage, plus the implementation
of
> HiiGetString, which is called immediately afterwards, make it clear that
> BestLanguage is a null-terminated ASCII string, and not just a five byte,
> non-null terminated buffer.
> 
> Therefore AsciiStrLen is one byte too short, meaning that whether the
space
> allocated is really sufficient and whether the resultant string is really
> null-terminated becomes implementation-dependent. Rather than switching
> to
> AsciiStrSize, we use an explicitly compile-time string length calculation
> (both compile-time and run-time approaches are currently used elsewhere in
> the codebase for copying static strings).
> 
> Signed-off-by: Mike Beaton 
> ---
>  MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> index 96e05d4cf9..6e791783a6 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> @@ -1987,7 +1987,7 @@ GetNameFromId (
> NULL
> );
>if (BestLanguage == NULL) {
> -BestLanguage = AllocateCopyPool (AsciiStrLen ("en-US"), "en-US");
> +BestLanguage = AllocateCopyPool (sizeof ("en-US"), "en-US");
>  ASSERT (BestLanguage != NULL);
>}
> 
> --
> 2.41.0
> 
> 
> 
> 
> 





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

回复: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib: Check array index before access

[gaoliming via groups.io]

Michael:
 How do you detect those issues? Do you use the tool or do code review?

 For this change,  Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Michael
> Kubacki
> 发送时间: 2023年9月7日 1:41
> 收件人: devel@edk2.groups.io
> 抄送: Zhichao Gao ; Michael D Kinney
> 
> 主题: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib:
> Check array index before access
> 
> From: Michael Kubacki 
> 
> Moves the range check for the index into the array before attempting
> any accesses using the array index.
> 
> Cc: Zhichao Gao 
> Cc: Michael D Kinney 
> Signed-off-by: Michael Kubacki 
> ---
>  ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> index 7c80bba46581..5cb92c485b47 100644
> --- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> @@ -382,7 +382,7 @@ IfConfig6PrintIpAddr (
> 
>ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG6_INFO_COLON), gShellNetwork2HiiHandle);
> 
> -  while ((Ip->Addr[Index] == 0) && (Ip->Addr[Index + 1] == 0) &&
> (Index < PREFIXMAXLEN)) {
> +  while ((Index < PREFIXMAXLEN) && (Ip->Addr[Index] == 0) &&
> (Ip->Addr[Index + 1] == 0)) {
>  Index = Index + 2;
>  if (Index > PREFIXMAXLEN - 2) {
>break;
> --
> 2.42.0.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#108336):
> https://edk2.groups.io/g/devel/message/108336
> Mute This Topic: https://groups.io/mt/101198333/4905953
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaolim...@byosoft.com.cn]
> -=-=-=-=-=-=
> 





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

回复: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/BaseRngLib: Fix include guard

[gaoliming via groups.io]

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Rebecca Cran 
> 发送时间: 2023年9月7日 3:47
> 收件人: devel@edk2.groups.io; mikub...@linux.microsoft.com
> 抄送: Michael D Kinney ; Liming Gao
> ; Zhiguang Liu ;
> Rebecca Cran 
> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/BaseRngLib: Fix
> include guard
> 
> On 9/6/2023 11:29 AM, Michael Kubacki via groups.io wrote:
> > From: Michael Kubacki 
> >
> > The include guard is incomplete and does not define the macro.
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Rebecca Cran 
> > Signed-off-by: Michael Kubacki 
> > ---
> >   MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > index a9cbce15302f..d0abcc3452ee 100644
> > --- a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > +++ b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   **/
> >
> >   #ifndef BASE_RNGLIB_INTERNALS_H_
> > +#define BASE_RNGLIB_INTERNALS_H_
> >
> >   /**
> > Generates a 16-bit random number.
> 
> Reviewed-by: Rebecca Cran 




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

回复: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/TdxLib: Remove unnecessary comparison

[gaoliming via groups.io]

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Yao, Jiewen 
> 发送时间: 2023年9月7日 7:32
> 收件人: devel@edk2.groups.io; mikub...@linux.microsoft.com
> 抄送: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang ;
> Rebecca Cran 
> 主题: RE: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/TdxLib: Remove
> unnecessary comparison
> 
> Reviewed-by: Jiewen Yao 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Michael
> > Kubacki
> > Sent: Thursday, September 7, 2023 1:34 AM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D ; Gao, Liming
> > ; Liu, Zhiguang ;
> Rebecca
> > Cran 
> > Subject: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/TdxLib: Remove
> > unnecessary comparison
> >
> > From: Michael Kubacki 
> >
> > Removes the comparison since unsigned values are always greater than
> > or equal to 0.
> >
> > See the following CodeQL query for more info:
> > /cpp/cpp-unsigned-comparison-zero/
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Rebecca Cran 
> > Signed-off-by: Michael Kubacki 
> > ---
> >  MdePkg/Library/TdxLib/Rtmr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdePkg/Library/TdxLib/Rtmr.c b/MdePkg/Library/TdxLib/Rtmr.c
> > index a0283966b353..cbf1dda7bcff 100644
> > --- a/MdePkg/Library/TdxLib/Rtmr.c
> > +++ b/MdePkg/Library/TdxLib/Rtmr.c
> > @@ -51,7 +51,7 @@ TdExtendRtmr (
> >
> >ASSERT (Data != NULL);
> >ASSERT (DataLen == SHA384_DIGEST_SIZE);
> > -  ASSERT (Index >= 0 && Index < RTMR_COUNT);
> > +  ASSERT (Index < RTMR_COUNT);
> >
> >if ((Data == NULL) || (DataLen != SHA384_DIGEST_SIZE) || (Index >=
> > RTMR_COUNT)) {
> >  return EFI_INVALID_PARAMETER;
> > --
> > 2.42.0.windows.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#108335):
> https://edk2.groups.io/g/devel/message/108335
> > Mute This Topic: https://groups.io/mt/101198214/1772286
> > Group Owner: devel+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [jiewen@intel.com]
> > -=-=-=-=-=-=
> >





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

Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Sean]

On 9/7/2023 1:54 PM, Laszlo Ersek wrote:

On 9/7/23 19:50, Sean Brogan wrote:

I would argue that by declaring that your library class supports type
DXE_CORE (or any core type) that you have declared you understand the
uniqueness of the environment and have accounted for it.

For instances that support DXE_CORE or MM_CORE module types we often use
a global variable (to the library) to determine if the init routine has
been completed.  This does require a single byte check on each serial
message write (hot path) but given all the code on that path this is not
noticeable in performance measurements.   In the case below this pattern
could be used by the FdtPL011SerialPortLib to detect if it's init
routine has been called.

Good point, but then I still claim that the "init check in each API"
should be done in a dedicated "DxeCoreDebugLibSerialPort" instance, and
not in a SerialPortLib instance. Here's why:

(1) The SerialPortLib *class* requires SerialPortInitialize() to be
called before the other APIs.


Where do you see this? Looking at the file here: 
edk2/MdePkg/Include/Library/SerialPortLib.h at master · tianocore/edk2 
(github.com) 
 
I don't see that.  I don't necessarily disagree with you but I am just 
trying to find out where this is documented.




  The FdtPL011SerialPortLib instance does
nothing in its implementation of that function, because it relies on the
constructor doing the same work. Therefore I agree that
FdtPL011SerialPortLib is not suitable for DXE_CORE, and I would suggest
removing DXE_CORE from LIBRARY_CLASS in the INF file, after the pipe
sign ("|").

Agree


(2) A new SerialPortLib instance should be added, very similar to
FdtPL011SerialPortLib -- the difference should be that it should have no
constructor, and the same job should be done in SerialPortInitialize().
This library instance sould be suitable for *direct use* in DXE_CORE
(and should likely be restricted to DXE_CORE exclusively).

The reason for that is the following. The DXE Core is entitled to
consume a lib instance without calling its constructor, in case the lib
instance declares itself DXE_CORE-capable (this is your argument). (In
fact such a lib instance is not supposed to have a constructor at all --
it might not be called anyway.) However, the DXE Core is *not* entitled
to ignore library *class* restrictions, and an explicit call to
SerialPortInitialize() is required by the SerialPortLib *class*. IOW, if
the DXE Core ever wanted to use SerialPortLib *directly*, it would have
to call SerialPortInitialize() before calling the other SerialPortLib
APIs, regardless of where and when the DXE Core ran the library
constructor list.

So that's why such a new FdtPL011SerialPortLib variant would be proper
for DXE_CORE.

(3) In turn, the new DxeCoreDebugLibSerialPort instance -- which would
have no constructor -- would be responsible for tracking in each API
implementation whether SerialPortInitialize() had been called before.

Agree


(4) This also means that the current BaseDebugLibSerialPort in MdePkg is
unsuitable for DXE_CORE usage, and so its LIBRARY_CLASS module type list
should be made explicit -- it should *exclude* the DXE_CORE. Even though
BaseDebugLibSerialPort has a BASE type entry point, this lib instance
relies on having a constructor (where it calls SerialPortInitialize()!),
and that rules it out for DXE_CORE usage.

Agree



IOW, I agree with you; my point is only that the serial init tracking
belongs in a new DebugLib instance (because, at the *class* level,
DebugLib permits the DXE_CORE to call its APIs in any order; whereas
SerialPortLib requires SerialPortInitialize() to be called first, also
at the *class* level).

Laszlo

Just for discussions sake you could also imagine a solution where the 
"base" instance does init tracking and then a new library instance is 
used only for XIP PEI that executes from RO memory (flash or 
otherwise).   Also note that this isn't just a DXE_CORE problem.  SEC, 
PEI_CORE, MM_CORE_STANDALONE and SMM_CORE types may have these similar 
restrictions.



Thanks

Sean



On 9/7/2023 8:24 AM, Oliver Smith-Denny wrote:

On 9/7/2023 6:10 AM, Laszlo Ersek wrote:

(replying on the webui... sorry!)

The problem is actually embedded in MdePkg and MdeModulePkg.

- In DxeMain() (and in functions called by DxeMain()), we call
DebugLib APIs *before* reaching ProcessLibraryConstructorList().

- In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to
BaseDebugLibSerialPort (from MdePkg).

- BaseDebugLibSerialPort has a constructor function
(BaseDebugLibSerialPortConstructor()).

These already suffice for broken DebugLib behavior. APIs of a library
class should not be called because the library instance has a chance
to initialize.

The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor
calls SerialPortInitialize, but our SerialPortInitialize (in
FdtPL011SerialPortLib) does nothing. 

Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version

[Michael D Kinney]

I was asking about the property of the global variables
being used in this patch.  Are they already guaranteed to
be in BSD format and in range 0..9.  If so, then no additional
code changes would be required.  However, it would be good 
to add comments about the properties of those global variables
and why they can be used to directly assign to fields that
are required to be in BSD format.

Mike

> -Original Message-
> From: Lien, HoraceX 
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel@edk2.groups.io; Kinney, Michael D ;
> Gao, Liming 
> Cc: Liu, Zhiguang ; Bi, Dandan
> ; Zeng, Star ; Gao, Zhichao
> ; Lien, HoraceX 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Hi Mike,
> 
> Could you please reply for me?
> If you want to filter range 0-9, then I will send PR again.
> 
> Thanks,
> Horace Lien
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Lien,
> HoraceX
> Sent: Friday, September 1, 2023 3:06 PM
> To: Kinney, Michael D ; devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Bi, Dandan
> ; Zeng, Star ; Gao, Zhichao
> 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Hi Mike,
> 
> I have change code to
> EntryPointStructureData.SmbiosBcdRevision =
> ((mPrivateData.Smbios.MajorVersion & 0x0f) << 4) |
> (mPrivateData.Smbios.MinorVersion & 0x0f); Add &0x0F to mask upper nibble
> bit, do we still need to guarantee that range is between 0-9? Because the
> old code only filtered 4 bits, instead of accurately filtering the number
> range 0-9.
> 
> Thanks,
> Horace Lien
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel@edk2.groups.io; Lien, HoraceX 
> Cc: Liu, Zhiguang ; Bi, Dandan
> ; Zeng, Star ; Gao, Zhichao
> ; Kinney, Michael D 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Are mPrivateData.Smbios.MajorVersion and mPrivateData.Smbios.MinorVersion
> guaranteed to be in range 0..9?
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > horacex.l...@intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel@edk2.groups.io
> > Cc: Lien, HoraceX ; Liu, Zhiguang
> > ; Bi, Dandan ; Zeng, Star
> > ; Gao, Zhichao 
> > Subject: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> > is not match with SMBIOS version
> >
> > From: HoraceX Lien 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4544
> >
> > These value of Major/Minor version are updated from SMBIOS memory
> > data, but BCD Revision is updated from PCD PcdSmbiosVersion.
> > We should also update PCD PcdSmbiosVersion from SMBIOS memory data, to
> > ensure that get consistent version value.
> >
> > Cc: Zhiguang Liu 
> > Cc: Dandan Bi 
> > Cc: Star Zeng 
> > Cc: Zhichao Gao 
> > Signed-off-by: HoraceX Lien 
> > ---
> >  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > index 1a86e69d3c..e3f6215033 100644
> > --- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > +++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > @@ -1072,7 +1072,7 @@ SmbiosCreateTable (
> >  DEBUG ((DEBUG_INFO, "SmbiosCreateTable: Initialize 32-bit entry
> > point structure\n"));
> >
> >  EntryPointStructureData.MajorVersion  =
> > mPrivateData.Smbios.MajorVersion;
> >
> >  EntryPointStructureData.MinorVersion  =
> > mPrivateData.Smbios.MinorVersion;
> >
> > -EntryPointStructureData.SmbiosBcdRevision = (UINT8)((PcdGet16
> > (PcdSmbiosVersion) >> 4) & 0xf0) | (UINT8)(PcdGet16 (PcdSmbiosVersion)
> > & 0x0f);
> >
> > +EntryPointStructureData.SmbiosBcdRevision =
> > (mPrivateData.Smbios.MajorVersion << 4) |
> > mPrivateData.Smbios.MinorVersion;
> >
> >  PhysicalAddress   = 0x;
> >
> >  Status= gBS->AllocatePages (
> >
> >
> > AllocateMaxAddress,
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#108150):
> > https://edk2.groups.io/g/devel/message/108150
> > Mute This Topic: https://groups.io/mt/101057293/1643496
> > Group Owner: devel+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [michael.d.kin...@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib: Check array index before access

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: mikub...@linux.microsoft.com 
> Sent: Wednesday, September 6, 2023 10:41 AM
> To: devel@edk2.groups.io
> Cc: Gao, Zhichao ; Kinney, Michael D
> 
> Subject: [PATCH v1 1/1] ShellPkg/UefiShellNetwork2CommandsLib: Check
> array index before access
> 
> From: Michael Kubacki 
> 
> Moves the range check for the index into the array before attempting
> any accesses using the array index.
> 
> Cc: Zhichao Gao 
> Cc: Michael D Kinney 
> Signed-off-by: Michael Kubacki 
> ---
>  ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> index 7c80bba46581..5cb92c485b47 100644
> --- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> @@ -382,7 +382,7 @@ IfConfig6PrintIpAddr (
> 
>ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG6_INFO_COLON), gShellNetwork2HiiHandle);
> 
> -  while ((Ip->Addr[Index] == 0) && (Ip->Addr[Index + 1] == 0) &&
> (Index < PREFIXMAXLEN)) {
> +  while ((Index < PREFIXMAXLEN) && (Ip->Addr[Index] == 0) && (Ip-
> >Addr[Index + 1] == 0)) {
>  Index = Index + 2;
>  if (Index > PREFIXMAXLEN - 2) {
>break;
> --
> 2.42.0.windows.2



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

Re: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/TdxLib: Remove unnecessary comparison

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: mikub...@linux.microsoft.com 
> Sent: Wednesday, September 6, 2023 10:34 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang ;
> Rebecca Cran 
> Subject: [PATCH v1 1/1] MdePkg/Library/TdxLib: Remove unnecessary
> comparison
> 
> From: Michael Kubacki 
> 
> Removes the comparison since unsigned values are always greater than
> or equal to 0.
> 
> See the following CodeQL query for more info:
> /cpp/cpp-unsigned-comparison-zero/
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Rebecca Cran 
> Signed-off-by: Michael Kubacki 
> ---
>  MdePkg/Library/TdxLib/Rtmr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/TdxLib/Rtmr.c b/MdePkg/Library/TdxLib/Rtmr.c
> index a0283966b353..cbf1dda7bcff 100644
> --- a/MdePkg/Library/TdxLib/Rtmr.c
> +++ b/MdePkg/Library/TdxLib/Rtmr.c
> @@ -51,7 +51,7 @@ TdExtendRtmr (
> 
>ASSERT (Data != NULL);
>ASSERT (DataLen == SHA384_DIGEST_SIZE);
> -  ASSERT (Index >= 0 && Index < RTMR_COUNT);
> +  ASSERT (Index < RTMR_COUNT);
> 
>if ((Data == NULL) || (DataLen != SHA384_DIGEST_SIZE) || (Index >=
> RTMR_COUNT)) {
>  return EFI_INVALID_PARAMETER;
> --
> 2.42.0.windows.2



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

Re: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/BaseRngLib: Fix include guard

[Michael D Kinney]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: Rebecca Cran 
> Sent: Wednesday, September 6, 2023 12:47 PM
> To: devel@edk2.groups.io; mikub...@linux.microsoft.com
> Cc: Kinney, Michael D ; Gao, Liming
> ; Liu, Zhiguang ;
> Rebecca Cran 
> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg/Library/BaseRngLib: Fix
> include guard
> 
> On 9/6/2023 11:29 AM, Michael Kubacki via groups.io wrote:
> > From: Michael Kubacki 
> >
> > The include guard is incomplete and does not define the macro.
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Rebecca Cran 
> > Signed-off-by: Michael Kubacki 
> > ---
> >   MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > index a9cbce15302f..d0abcc3452ee 100644
> > --- a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > +++ b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
> > @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   **/
> >
> >   #ifndef BASE_RNGLIB_INTERNALS_H_
> > +#define BASE_RNGLIB_INTERNALS_H_
> >
> >   /**
> > Generates a 16-bit random number.
> 
> Reviewed-by: Rebecca Cran 


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

Re: [edk2-devel] [PATCH v1 7/7] .pytool/Plugin: Add DebugMacroCheck

[Michael Kubacki]

There's not package level control at the moment from a given package CI 
YAML file. We've treated this as fundamental for "core" packages (those 
often built from a single CISettings.py file) and on-by-default but with 
a disable option via the env var for individual platform build files 
(like the disable I applied for OvmfPkg in Patch 6 of the series).


If you'd like a disable option available in CI YAML files, it can easily 
be consumed in DebugMacroCheckBuildPlugin.py and I'll add it in v2. Let 
me know.


Thanks,
Michael

On 9/6/2023 9:35 PM, Michael D Kinney wrote:

Reviewed-by: Michael D Kinney 

Really like the inclusion of unit tests.  If any issues are
found, can update unit test to cover that case.

One quick question.  I see a global disable through env var.
I also see it says that is it runs on packages that use the
compiler plugin.  Is there a way to disable this plugin at
the package scope? Many plugins support a "skip" setting today.

Thanks,

Mike


-Original Message-
From: mikub...@linux.microsoft.com 
Sent: Monday, August 14, 2023 1:49 PM
To: devel@edk2.groups.io
Cc: Sean Brogan ; Kinney, Michael D
; Gao, Liming 
Subject: [PATCH v1 7/7] .pytool/Plugin: Add DebugMacroCheck

From: Michael Kubacki 

Adds a plugin that finds debug macro formatting issues. These errors
often creep into debug prints in error conditions not frequently
executed and make debug more difficult when they are encountered.

The code can be as a standalone script which is useful to find
problems in a large codebase that has not been checked before or as
a CI plugin that notifies a developer of an error right away.

The script was already used to find numerous issues in edk2 in the
past so there's not many code fixes in this change. More details
are available in the readme file:

.pytool\Plugin\DebugMacroCheck\Readme.md

Cc: Sean Brogan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
  .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py
| 127 +++
  .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_in.yaml
|  11 +
  .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py
| 859 
  .pytool/Plugin/DebugMacroCheck/Readme.md
| 253 ++
  .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py
| 674 +++
  .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py
| 131 +++
  .pytool/Plugin/DebugMacroCheck/tests/__init__.py
|   0
  .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py
| 201 +
  8 files changed, 2256 insertions(+)

diff --git
a/.pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.p
y
b/.pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.p
y
new file mode 100644
index ..b1544666025e
--- /dev/null
+++
b/.pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.p
y
@@ -0,0 +1,127 @@
+# @file DebugMacroCheckBuildPlugin.py
+#
+# A build plugin that checks if DEBUG macros are formatted properly.
+#
+# In particular, that print format specifiers are defined
+# with the expected number of arguments in the variable
+# argument list.
+#
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+import logging
+import os
+import pathlib
+import sys
+import yaml
+
+# Import the build plugin
+plugin_file = pathlib.Path(__file__)
+sys.path.append(str(plugin_file.parent.parent))
+
+# flake8 (E402): Ignore flake8 module level import not at top of file
+import DebugMacroCheck  # noqa: E402
+
+from edk2toolext import edk2_logging   #
noqa: E402
+from edk2toolext.environment.plugintypes.uefi_build_plugin import \
+IUefiBuildPlugin   #
noqa: E402
+from edk2toolext.environment.uefi_build import UefiBuilder #
noqa: E402
+from edk2toollib.uefi.edk2.path_utilities import Edk2Path  #
noqa: E402
+from pathlib import Path   #
noqa: E402
+
+
+class DebugMacroCheckBuildPlugin(IUefiBuildPlugin):
+
+def do_pre_build(self, builder: UefiBuilder) -> int:
+"""Debug Macro Check pre-build functionality.
+
+The plugin is invoked in pre-build since it can operate
independently
+of build tools and to notify the user of any errors earlier in
the
+build process to reduce feedback time.
+
+Args:
+builder (UefiBuilder): A UEFI builder object for this build.
+
+Returns:
+int: The number of debug macro errors found. Zero indicates
the
+check either did not run or no errors were found.
+"""
+
+# Check if disabled in the environment
+env_disable = builder.env.GetValue("DISABLE_DEBUG_MACRO_CHECK")
+if env_disable:
+return 0
+
+# Only run on targets with compilation
+build_target = builder.env.GetValue("TARGET").lower()
+if "no-target" in 

Re: [edk2-discuss][edk2-devel] UEFI variables not getting stored in nvram disk

[Laszlo Ersek]

On 9/7/23 22:24, Het Gala wrote:
> 
> 
> 
>  Forwarded Message 
> Subject:  [edk2-discuss] UEFI variables not getting stored in nvram disk
> Date: Mon, 17 Jul 2023 23:24:45 +0530
> From: Het Gala 
> To:   disc...@edk2.groups.io
> CC:   Kallol Biswas [C] 
> 
> 
> 
> Hi edk2 community,
> 
> 
> We have a scenario - testcase1, with uefi enabled VM having an Empty
> CDROM. PxE boot, EFI internal Shell (already mentioned by default by
> UEFI) and CDROM are the boot options. We also have a pflash / nvram disk
> by default attached if it is an uefi enabled VM. So, we have a nvram
> disk attached to the system.
> 
> 
> In testcase1, (opened the UEFI Boot Manager --> changed Boot Order -->
> saved Boot Order --> continue --> Guest reboot followed by power cycle
> --> again open UEFI Boot Manager) changed Boot Order is not persisted
> and falls back to the default Boot Order. Same observation is seen for
> other uefi variables (ex. autoboot timeout / Next Boot / Display
> resolution changes) too. None of the modified / changed uefi variables
> are persisted after guest reboot followed by power cycle.
> 
> 
> On the other hand, when we have a similar scenario testcase2, but
> instead of an empty CDROM, we have a vdisk (~ 10G) with an guest OS
> image (~ 1.7G) mounted on that disk. We observed that, the modified uefi
> variables (ex. change in Boot order / change in display resolution) are
> persisted after modifying uefi variable value followed by guest reboot
> and power cycle. The modified variables are stored inside NvVars file (~
> 200 MB). This file is inside efi/boot partition, which is made on the
> same vdisk where guest OS image is also mounted, because the vdisk is
> having space left.
> 
> 
> IIUC, if SMM driver enabled while building OVMF binary, and have an
> external pflash drive to save the uefi variables (nvram disk), the
> modified uefi variables are stored inside the nvram disk, otherwise in
> absence of SMM driver stack, the NvVars file gets mounted on the OS disk
> if enough space is available.
> 
> In testcase2, SMM driver option (-D SMM_REQUIRE) is present in the OVMF
> binary file, and also has external nvram disk attached to the VM, even
> then the uefi variables are stored on the vdisk instead of nvram disk.
> 
> 
> Question is : How can we make Boot Order (and other modified uefi
> variables) persistent when a OS vdisk is not mounted to the uefi enabled
> VM ?

There are too many disparate things going on in your setup:

- controlling the boot order from within the guest as opposed to via fw_cfg

- using pflash vs. not using pflash

- building with SMM_REQUIRE or not.

Short suggestion:

- always use pflash; if you see an NvVars file, your setup is broken (we
can safely say this in 2023). In fact use *two* pflash chips, one for
code, one for varstore.

- if your QEMU command line includes the "bootorder" property for any
device, then OVMF BDS will filter and reorder the UEFI boot options,
most likely overriding or otherwise masking your earlier in-guest
settings. This is intentional.

- SMM_REQUIRE or not is not relevant for the Boot and BootOrder
variables.

Please provide your QEMU and edk2 versions, your edk2 build command, and
the QEMU command line.

Laszlo

> 
> 
> ---
> 
> (Attaching references for testcase1 and testcase2 below)
> 
> 
> For testcase1 : attaching some debug statements here of the default Boot
> order (1.a), the changed boot order (1.b), and the boot order followed
> by guest reboot and power cycle (1.c).
> 
> (1.a) Here it shows the default initial Boot order
> 
> 
>  [Bds]OsIndication: ^M
>  [Bds]=Begin Load Options Dumping ...=^M
>    Driver Options:^M
>    SysPrep Options:^M
>    Boot Options:^M
>  Boot: UiApp  0x0109^M
>  Boot0001: UEFI QEMU DVD-ROM QM5  0x0001^M
>  Boot0002: UEFI PXEv4 (MAC:506B8D83F4DB)  0x0001^M
>  Boot0003: EFI Internal Shell 0x0001^M
>    PlatformRecovery Options:^M
>  PlatformRecovery: Default PlatformRecovery   0x0001^M
>  [Bds]=End Load Options Dumping=^M
>  [Bds]BdsWait ...Zzzz...^M
>  [Bds]Exit the waiting!^M
>  ValidateSetVariable - Variable
> (8BE4DF61-93CA-11D2-AA0D-00E098032B8C:BootCurrent) returning Success.^M
> 
> 
> (1.b) Changed the order from 1->2->3 to 1->3->2 from the UEFI Boot
> Manager Menu. To verify whether the Boot order has actually changed or
> not after saving the Boot order from uefi GUI, added some debug
> statments
> after 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c#L614
>  
> .
> 
> 
>   

Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Laszlo Ersek]

On 9/7/23 19:50, Sean Brogan wrote:
> I would argue that by declaring that your library class supports type
> DXE_CORE (or any core type) that you have declared you understand the
> uniqueness of the environment and have accounted for it.
> 
> For instances that support DXE_CORE or MM_CORE module types we often use
> a global variable (to the library) to determine if the init routine has
> been completed.  This does require a single byte check on each serial
> message write (hot path) but given all the code on that path this is not
> noticeable in performance measurements.   In the case below this pattern
> could be used by the FdtPL011SerialPortLib to detect if it's init
> routine has been called.

Good point, but then I still claim that the "init check in each API"
should be done in a dedicated "DxeCoreDebugLibSerialPort" instance, and
not in a SerialPortLib instance. Here's why:

(1) The SerialPortLib *class* requires SerialPortInitialize() to be
called before the other APIs. The FdtPL011SerialPortLib instance does
nothing in its implementation of that function, because it relies on the
constructor doing the same work. Therefore I agree that
FdtPL011SerialPortLib is not suitable for DXE_CORE, and I would suggest
removing DXE_CORE from LIBRARY_CLASS in the INF file, after the pipe
sign ("|").

(2) A new SerialPortLib instance should be added, very similar to
FdtPL011SerialPortLib -- the difference should be that it should have no
constructor, and the same job should be done in SerialPortInitialize().
This library instance sould be suitable for *direct use* in DXE_CORE
(and should likely be restricted to DXE_CORE exclusively).

The reason for that is the following. The DXE Core is entitled to
consume a lib instance without calling its constructor, in case the lib
instance declares itself DXE_CORE-capable (this is your argument). (In
fact such a lib instance is not supposed to have a constructor at all --
it might not be called anyway.) However, the DXE Core is *not* entitled
to ignore library *class* restrictions, and an explicit call to
SerialPortInitialize() is required by the SerialPortLib *class*. IOW, if
the DXE Core ever wanted to use SerialPortLib *directly*, it would have
to call SerialPortInitialize() before calling the other SerialPortLib
APIs, regardless of where and when the DXE Core ran the library
constructor list.

So that's why such a new FdtPL011SerialPortLib variant would be proper
for DXE_CORE.

(3) In turn, the new DxeCoreDebugLibSerialPort instance -- which would
have no constructor -- would be responsible for tracking in each API
implementation whether SerialPortInitialize() had been called before.

(4) This also means that the current BaseDebugLibSerialPort in MdePkg is
unsuitable for DXE_CORE usage, and so its LIBRARY_CLASS module type list
should be made explicit -- it should *exclude* the DXE_CORE. Even though
BaseDebugLibSerialPort has a BASE type entry point, this lib instance
relies on having a constructor (where it calls SerialPortInitialize()!),
and that rules it out for DXE_CORE usage.


IOW, I agree with you; my point is only that the serial init tracking
belongs in a new DebugLib instance (because, at the *class* level,
DebugLib permits the DXE_CORE to call its APIs in any order; whereas
SerialPortLib requires SerialPortInitialize() to be called first, also
at the *class* level).

Laszlo


> On 9/7/2023 8:24 AM, Oliver Smith-Denny wrote:
>> On 9/7/2023 6:10 AM, Laszlo Ersek wrote:
>>> (replying on the webui... sorry!)
>>>
>>> The problem is actually embedded in MdePkg and MdeModulePkg.
>>>
>>> - In DxeMain() (and in functions called by DxeMain()), we call
>>> DebugLib APIs *before* reaching ProcessLibraryConstructorList().
>>>
>>> - In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to
>>> BaseDebugLibSerialPort (from MdePkg).
>>>
>>> - BaseDebugLibSerialPort has a constructor function
>>> (BaseDebugLibSerialPortConstructor()).
>>>
>>> These already suffice for broken DebugLib behavior. APIs of a library
>>> class should not be called because the library instance has a chance
>>> to initialize.
>>>
>>> The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor
>>> calls SerialPortInitialize, but our SerialPortInitialize (in
>>> FdtPL011SerialPortLib) does nothing. Well, the latter doesn't need to
>>> do anything, because FdtPL011SerialPortLib has its own constructor
>>> (FdtPL011SerialPortLibInitialize), thus, if constructors were called
>>> properly, then BaseDebugLibSerialPort + FdtPL011SerialPortLib would
>>> work properly together, regardless of SerialPortInitialize being
>>> empty in the latter.
>>>
>>> Basically the DXE Core has a hidden requirement -- it can only use
>>> such DebugLib instances that need no explicit initialization.
>>>
>>> The proposed patch works around the problem by satisfying that hidden
>>> requirement one level lower down: in the SerialPortLib instance. The
>>> initialization of BaseDebugLibSerialPort is still busted (its

Re: [edk2-devel] VT100 terminal for UEFI shell

[Andrew Fish via groups.io]

> On Sep 7, 2023, at 12:11 PM, Marcin Juszkiewicz 
>  wrote:
> 
> W dniu 7.09.2023 o 19:40, Andrew (EFI) Fish pisze:
>>> On Sep 7, 2023, at 8:00 AM, Marcin Juszkiewicz wrote:
>>> 
>>> Is there a way to have VT100 (or any other black/white, non-ANSI) terminal 
>>> for UEFI Shell?
>>> 
>>> I do many runs of QEMU/sbsa-ref with logging and all those ANSI colour 
>>> codes only make problems.
> 
> 
>> Thus we get to the TL;DR part… The console NVRAM variables that point to the 
>> UART has a MESSAGING_DEVICE_PATH MSG_VENDOR_DP node after the UART 
>> definition that has a UUID (EFI_GUID) that defines the type of terminal 
>> emulation to use.
>> The definition of the ConIn and ConOut variables is here [4]
> 
> Feels like adding alias into ~/.bashrc is easier solution:
> 
> alias strip_ansi=" sed -e 's/\x1b\[[0-9;]*m//g' "
> 

Well like everything  the terminal default is optimized for the personal 
preference of who ever did OVMF port. 

> EDK2 code feels like ancient Perl to me too often. Someone wrote it, everyone 
> uses it, hard to find someone who know why it is that way.
> 
> 

Feel free to ask I’ll probably be 24 years into at some point soon or maybe 
already I kind of forgot when I started.  I was around for most of the big 
picture stuff so that is one of the reasons I answer these general questions on 
the list.

Thanks,

Andrew Fish

> 
> 
> 



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

Re: [edk2-devel] [edk2][RFC] OvmfPkg/AcpiPlatformDxe: patch FADT PSCI bits if FDT advertises it

[Laszlo Ersek]

On 9/7/23 17:17, Ard Biesheuvel wrote:
> On Thu, 7 Sept 2023 at 16:51, Laszlo Ersek  wrote:
>>
>> On 9/7/23 16:27, Ard Biesheuvel wrote:
>>> On Mon, 16 Jan 2023 at 12:39, Evgeny Iakovlev
>>>  wrote:

 EL3 firmware may implement PSCI interface on aarch64 platforms,
 including qemu-tcg-aarch64. However, EL3 firmware does not usually own
 pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
 EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
 firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
 EL3 firmware is present.

 PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
 expect to see all information published in ACPI. To better support
 running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
 and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
 bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
 advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
 FDT on arm, so it is ideally poised to handle this.

 This change illustrates how it could potentially be done. I am looking
 for comments on overall validity of the idea to patch FADT and whether
 or not this particular approach of handling it in AcpiPlatformDxe is the
 way to do it or maybe it is better to handle it via
 gQemuAcpiTableNotifyProtocolGuid somehow.

 Signed-off-by: Evgeny Iakovlev 
>>>
>>> Thanks for the patch, and apologies for the lack of response.
>>>
>>> First of all, I suspect this patch breaks non-ARM users of this
>>> driver, so the patch is problematic as is. (It makes
>>> gFdtClientProtocolGuid mandatory, right?)
>>>
>>> Then, I'd like to hear from other folks on cc what they think about
>>> this. Perhaps it is simply a matter of tweaking QEMU so it exposes the
>>> correct PSCI setting in the FADT when it emulates secure world.
>>> Patching it like this feels like a last resort to me, rather than a
>>> well designed interface.
>>
>> Thanks for the CC; both of your concerns are valid.
>>
>> The FDT client proto GUID has no reason to exist in (e.g.) an X64 OVMF
>> build.
>>
>> Second, and more importantly, this is a total layering violation for
>> AcpiPlatformDxe. QEMU is the single source of truth for AcpiPlatformDxe,
>> and AcpiPlatformDxe must remain as blind as possible to the actual ACPI
>> content.
>>
>> In the situation described by the commit message, the ACPI content
>> exposed by QEMU is simply invalid. That's what should be fixed in QEMU
>> (and not papered over in edk2). Something somewhere is responsible for
>> setting the property value in question to "hvc"; that something
>> precisely is responsible (directly or indirectly) for making QEMU expose
>> the proper FADT.
>>
>> I've now grepped the QEMU source tree for '"hvc"'; the relevant hit
>> seems to be in "hw/arm/boot.c", function fdt_add_psci_node(), under case
>> label QEMU_PSCI_CONDUIT_HVC. So, whatever sets psci-conduit to
>> QEMU_PSCI_CONDUIT_HVC should also make sure the FADT matches it.
>>
>> Taking one step back, in "hw/arm/virt.c" we have:
>>
>> if (vms->secure && firmware_loaded) {
>> vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
>> } else if (vms->virt) {
>> vms->psci_conduit = QEMU_PSCI_CONDUIT_SMC;
>> } else {
>> vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC;
>> }
>>
> 
> The problem here is that QEMU does not know whether the EL3 firmware
> running in the guest implements PSCI or not.
> 
>> So I figure the ACPI generator should be steered off the same information.
>>
>> BTW... I see the following in "hw/arm/virt-acpi-build.c", function
>> build_fadt_rev6():
>>
>> case QEMU_PSCI_CONDUIT_HVC:
>> fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
>>  ACPI_FADT_ARM_PSCI_USE_HVC;
>> break;
>>
>> That dates back minimally as far as commit 79e993a0a804
>> ("hw/arm/virt-acpi-build: use SMC if booting in EL2", 2017-01-20).
>>
>> So why is it not taking effect? Patching edk2 should not be necessary at
>> all, QEMU should already be doing the right thing.
>>
>> The commit message states, "Qemu itself also won't advertise PSCI in
>> [...] ACPI if EL3 firmware is present"; if that's correct (I can't
>> tell), then it may be the problem.
>>
> 
> Exactly.
> 
> When not emulating EL2 or EL3 (which is equivalent to the KVM case),
> PSCI calls are made using HVC instructions, which are handled by QEMU
> directly.
> 
> When EL2 emulation is enabled, PSCI calls are made using SMC
> instructions but using the same handling in QEMU.
> 
> When EL3 emulation is enabled, QEMU can no longer 'overrule' the side
> effects of SMC instructions but has to deliver them to the firmware
> that occupies EL3. Whether or not that firmware implements PSCI is not
> known to QEMU, and so it assumes it is not, and populates the FADT
> fields accordingly.

Whether the EL3 firmware 

Re: [edk2-devel] VT100 terminal for UEFI shell

[Marcin Juszkiewicz]

W dniu 7.09.2023 o 19:40, Andrew (EFI) Fish pisze:

On Sep 7, 2023, at 8:00 AM, Marcin Juszkiewicz wrote:

Is there a way to have VT100 (or any other black/white, non-ANSI) 
terminal for UEFI Shell?


I do many runs of QEMU/sbsa-ref with logging and all those ANSI colour 
codes only make problems.



Thus we get to the TL;DR part… The console NVRAM variables that point to 
the UART has a MESSAGING_DEVICE_PATH MSG_VENDOR_DP node after the UART 
definition that has a UUID (EFI_GUID) that defines the type of terminal 
emulation to use.


The definition of the ConIn and ConOut variables is here [4]


Feels like adding alias into ~/.bashrc is easier solution:

alias strip_ansi=" sed -e 's/\x1b\[[0-9;]*m//g' "

EDK2 code feels like ancient Perl to me too often. Someone wrote it, 
everyone uses it, hard to find someone who know why it is that way.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108414): https://edk2.groups.io/g/devel/message/108414
Mute This Topic: https://groups.io/mt/101216135/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] OvmfPkg/RiscVVirt/README.md: bring your own OpenSBI

[Andrei Warkentin]

This looks fine to me as far as an edit, but I didn't validate the commands.

Reviewed-by: Andrei Warkentin 

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, September 7, 2023 9:58 AM
> To: ler...@redhat.com; edk2-devel-groups-io 
> Cc: Warkentin, Andrei ; Ard Biesheuvel
> ; Gerd Hoffmann ; Yao,
> Jiewen ; Justen, Jordan L
> ; Sunil V L 
> Subject: [PATCH 1/1] OvmfPkg/RiscVVirt/README.md: bring your own
> OpenSBI
> 
> Explain how users can compose their pre-OS environment purely from
> binaries they've built themselves.
> 
> Cc: Andrei Warkentin 
> Cc: Ard Biesheuvel 
> Cc: Gerd Hoffmann 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Sunil V L 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> rendered version:
> 
> https://github.com/lersek/edk2/tree/bring-your-own-
> opensbi/OvmfPkg/RiscVVirt#test-with-your-own-opensbi-binary
> 
>  OvmfPkg/RiscVVirt/README.md | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/OvmfPkg/RiscVVirt/README.md
> b/OvmfPkg/RiscVVirt/README.md index 8c3ac37b802a..dbb40bbe89b0
> 100644
> --- a/OvmfPkg/RiscVVirt/README.md
> +++ b/OvmfPkg/RiscVVirt/README.md
> @@ -69,3 +69,20 @@ Below example shows how to boot openSUSE
> Tumbleweed E20.
>  -device virtio-net-pci,netdev=net0 \
>  -device virtio-blk-device,drive=hd0 \
>  -drive file=openSUSE-Tumbleweed-RISC-V-E20-
> efi.riscv64.raw,format=raw,id=hd0
> +
> +## Test with your own OpenSBI binary
> +Using the above QEMU command line, **RISCV_VIRT_CODE.fd** is
> launched
> +by the OpenSBI binary that is bundled with QEMU. You can build your own
> +OpenSBI binary as well:
> +
> +OPENSBI_DIR=...
> +git clone https://github.com/riscv/opensbi.git $OPENSBI_DIR
> +make -C $OPENSBI_DIR \
> +-j $(getconf _NPROCESSORS_ONLN) \
> +CROSS_COMPILE=riscv64-linux-gnu- \
> +PLATFORM=generic
> +
> +then specify that binary for QEMU, with the following additional
> +command line
> +option:
> +
> +-bios $OPENSBI_DIR/build/platform/generic/firmware/fw_dynamic.bin


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

[edk2-devel] [PATCH 2/2] MdePkg/SmBios.h : Add New Intel Processor family

[Avinash]

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

Add New Intel Processor family for SMBIOS Type 4
Hex value - 16h
Name - Intel® Processor

Signed-off-by: avinashbhargava 
Cc: Zhiguang Liu 
Cc: Dandan Bi 
Cc: Star Zeng 
Cc: Zhichao Gao 
Cc: Benny Lin 
Cc: Gua Guo 
Cc: Prakashan Krishnadas Veliyathuparambil 

---
 MdePkg/Include/IndustryStandard/SmBios.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h 
b/MdePkg/Include/IndustryStandard/SmBios.h
index 89985bb4186b..acb23d556c50 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -554,6 +554,7 @@ typedef enum {
   ProcessorFamilyM2  = 0x13,

   ProcessorFamilyIntelCeleronM   = 0x14,

   ProcessorFamilyIntelPentium4Ht = 0x15,

+  ProcessorFamilyIntel   = 0x16,

   ProcessorFamilyAmdDuron= 0x18,

   ProcessorFamilyK5  = 0x19,

   ProcessorFamilyK6  = 0x1A,

-- 
2.37.3.windows.1



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

[edk2-devel] [PATCH v1 1/1] MdePkg/SmBios.h: Add New Intel Processor family for SMBIOS

[Avinash]

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

Add New Intel Processor family for SMBIOS
Hex value - 16h
Name - Intel® Processor

Signed-off-by: avinashbhargava 
Cc: Zhiguang Liu 
Cc: Dandan Bi 
Cc: Star Zeng 
Cc: Zhichao Gao 
---
 MdePkg/Include/IndustryStandard/SmBios.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h 
b/MdePkg/Include/IndustryStandard/SmBios.h
index 40bdc9a937c0..56cec615a010 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -554,6 +554,7 @@ typedef enum {
   ProcessorFamilyM2  = 0x13,

   ProcessorFamilyIntelCeleronM   = 0x14,

   ProcessorFamilyIntelPentium4Ht = 0x15,

+  ProcessorFamilyIntel   = 0x16,

   ProcessorFamilyAmdDuron= 0x18,

   ProcessorFamilyK5  = 0x19,

   ProcessorFamilyK6  = 0x1A,

-- 
2.37.3.windows.1



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

[edk2-devel] [PATCH 0/2] Add New Intel Processor family

[Avinash]

Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

*** BLURB HERE ***

Avinash (1):
  MdePkg/SmBios.h : Add New Intel Processor family

Nhi Pham (1):
  ShellPkg/SmbiosView: Update display of PCIe system slot ID

 ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c | 2 +-
 MdePkg/Include/IndustryStandard/SmBios.h   | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.37.3.windows.1



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

[Henz, Patrick]

I don’t have strong opinions either. If we would prefer to cache the values 
returned by GetPerformanceCounterProperties I would be more than happy to do 
that. I could also modify XhcGetElapsedTime to return the elapsed ticks instead 
of the elapsed time in nano seconds, the functions that call into 
XhcGetElapedTime/Ticks could convert the millisecond timeout value into a tick 
count and compare the elapsed ticks against that, this would remove the 
reliance on GetTimeInNanoSecond. Does that sound more appropriate?

Thanks,
Patrick Henz

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Wu, Hao A
Sent: Thursday, September 7, 2023 2:48 AM
To: Mike Maslenkin ; devel@edk2.groups.io
Cc: Henz, Patrick 
Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use Performance 
Timer for XHCI Timeouts

I do not have strong opinion on this considering it is an IO driver.
The call of GetTimeInNanoSecond() is also likely to invoke 
GetPerformanceCounterProperties() call.

I will let the patch owner to decide on the open raised below.

Best Regards,
Hao Wu

> -Original Message-
> From: Mike Maslenkin 
> Sent: Thursday, September 7, 2023 3:36 PM
> To: devel@edk2.groups.io; Wu, Hao A 
> Cc: Henz, Patrick 
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use 
> Performance Timer for XHCI Timeouts
> 
> Hello Hao Wu,
> 
> Isn't GetPerformanceCounterProperties (, ) call 
> too "heavy" for XHCI paths?
> May be it's better to cache timer direction once?
> I'm asking in a context of PcAtChipsetPkg/Library/AcpiTimerLib
> instance used by a number of Intel's platform in edk-platforms.
> For this  library GetPerformanceCounterProperties finally calls 
> InternalCalculateTscFrequency, that isn't simple.
> 
> Regards,
> Mike
> 
> On Thu, Sep 7, 2023 at 4:27 AM Wu, Hao A  wrote:
> >
> > Sorry for the late response.
> > Reviewed-by: Hao A Wu 
> >
> > Best Regards,
> > Hao Wu
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of 
> > > Henz, Patrick
> > > Sent: Thursday, September 7, 2023 4:37 AM
> > > To: devel@edk2.groups.io
> > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use 
> > > Performance Timer for XHCI Timeouts
> > >
> > > I sent this patch out a few weeks ago now but haven't seen a 
> > > reply, just checking in to make sure it didn't get missed.
> > >
> > > Thanks,
> > > Patrick Henz
> > >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of 
> > > Henz, Patrick
> > > Sent: Monday, August 14, 2023 10:52 AM
> > > To: devel@edk2.groups.io
> > > Cc: Henz, Patrick 
> > > Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use 
> > > Performance Timer for XHCI Timeouts
> > >
> > > REF:INVALID URI REMOVED
> > > w_bug.cgi?id=2948__;!!NpxR!mZRuMBKPdeR-UvY7sr8tWNOfIQDe0W_TW3q-K8M
> > > tNN1cynyRZ1_bls8osaEqFWupFmjR29X_zvw0Zx1Y$
> > >
> > > XhciDxe uses the timer functionality provided by the boot services 
> > > table to detect timeout conditions. This breaks the driver's 
> > > ExitBootServices call back, as CoreExitBootServices halts the 
> > > timer before signaling the ExitBootServices event. If the host 
> > > controller fails to halt in the call back, the timeout condition 
> > > will never occur and the boot gets stuck in an indefinite spin 
> > > loop. Use the free running timer provided by TimerLib to calculate 
> > > timeouts, avoiding
> the potential hang.
> > >
> > > Signed-off-by: Patrick Henz 
> > > Reviewed-by:
> > > ---
> > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +--
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
> > > +---
> > >  5 files changed, 129 insertions(+), 86 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > index 62682dd27c..1dcbe20512 100644
> > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > @@ -1,6 +1,7 @@
> > >  /** @file
> > >The XHCI controller driver.
> > >
> > > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> > >  Copyright (c) 2011 - 2022, Intel Corporation. All rights 
> > > reserved.
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
> > >
> > >return EFI_SUCCESS;
> > >  }
> > > +
> > > +/**
> > > +  Computes and returns the elapsed time in nanoseconds since
> > > +  PreviousTick. The value of PreviousTick is overwritten with the
> > > +  current performance counter value.
> > > +
> > > +  @param  PreviousTickPointer to PreviousTick count.
> > > +  @return The elapsed time in nanoseconds since PreviousCount.
> > > +  PreviousCount is overwritten with the current performance
> > > +  counter value.
> > > +**/
> > > 

Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Sean]

I would argue that by declaring that your library class supports type 
DXE_CORE (or any core type) that you have declared you understand the 
uniqueness of the environment and have accounted for it.


For instances that support DXE_CORE or MM_CORE module types we often use 
a global variable (to the library) to determine if the init routine has 
been completed.  This does require a single byte check on each serial 
message write (hot path) but given all the code on that path this is not 
noticeable in performance measurements.   In the case below this pattern 
could be used by the FdtPL011SerialPortLib to detect if it's init 
routine has been called.


Thanks

Sean


On 9/7/2023 8:24 AM, Oliver Smith-Denny wrote:

On 9/7/2023 6:10 AM, Laszlo Ersek wrote:

(replying on the webui... sorry!)

The problem is actually embedded in MdePkg and MdeModulePkg.

- In DxeMain() (and in functions called by DxeMain()), we call 
DebugLib APIs *before* reaching ProcessLibraryConstructorList().


- In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to 
BaseDebugLibSerialPort (from MdePkg).


- BaseDebugLibSerialPort has a constructor function 
(BaseDebugLibSerialPortConstructor()).


These already suffice for broken DebugLib behavior. APIs of a library 
class should not be called because the library instance has a chance 
to initialize.


The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor 
calls SerialPortInitialize, but our SerialPortInitialize (in 
FdtPL011SerialPortLib) does nothing. Well, the latter doesn't need to 
do anything, because FdtPL011SerialPortLib has its own constructor 
(FdtPL011SerialPortLibInitialize), thus, if constructors were called 
properly, then BaseDebugLibSerialPort + FdtPL011SerialPortLib would 
work properly together, regardless of SerialPortInitialize being 
empty in the latter.


Basically the DXE Core has a hidden requirement -- it can only use 
such DebugLib instances that need no explicit initialization.


The proposed patch works around the problem by satisfying that hidden 
requirement one level lower down: in the SerialPortLib instance. The 
initialization of BaseDebugLibSerialPort is still busted (its 
constructor is not called, so it cannot call SerialPortInitialize 
either), but now it is masked, because EarlyFdtPL011SerialPortLib 
works withouth *both* SerialPortInitialize and construction.


The real fix would be to make the DXE Core requirement explicit, by 
introducing separate (dedicated) DebugLib and SerialPortLib *classes* 
(whose APIs are guaranteed to work without initialization).


Laszlo


Thanks for the comprehensive breakdown! :). I completely agree that
fixing this at the upper level (and ideally documenting this
requirement) is the better move.

I can drop this patch and take a crack at that. I'm in the last few
weeks leading up to an extended parental leave, so we'll see if I can
squeeze it in prior to then :).

Oliver








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

Re: [edk2-devel] VT100 terminal for UEFI shell

[Andrew Fish via groups.io]

> On Sep 7, 2023, at 8:00 AM, Marcin Juszkiewicz 
>  wrote:
> 
> Is there a way to have VT100 (or any other black/white, non-ANSI) terminal 
> for UEFI Shell?
> 
> I do many runs of QEMU/sbsa-ref with logging and all those ANSI colour codes 
> only make problems.
> 

These are the supported consoles [1]

EFI_GUID  *mTerminalType[] = {
  ,
  ,
  ,
  ,
  ,
  ,
  ,
  ,
  
};

CHAR16  *mSerialConsoleNames[] = {
  L"PC-ANSI Serial Console",
  L"VT-100 Serial Console",
  L"VT-100+ Serial Console",
  L"VT-UTF8 Serial Console",
  L"Tty Terminal Serial Console",
  L"Linux Terminal Serial Console",
  L"Xterm R6 Serial Console",
  L"VT-400 Serial Console",
  L"SCO Terminal Serial Console"
};

I think the OVMF default is coming from here [2]
VENDOR_DEVICE_PATHgTerminalTypeDeviceNode= gVtUtf8Terminal;

If you look up gVtUtf8Terminal you see [3]
#define gVtUtf8Terminal \
  { \
{ \
  MESSAGING_DEVICE_PATH, \
  MSG_VENDOR_DP, \
  { \
(UINT8) (sizeof (VENDOR_DEVICE_PATH)), \
(UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) \
  } \
}, \
DEVICE_PATH_MESSAGING_VT_UTF8 \
  }

Thus we get to the TL;DR part… The console NVRAM variables that point to the 
UART has a MESSAGING_DEVICE_PATH MSG_VENDOR_DP node after the UART definition 
that has a UUID (EFI_GUID) that defines the type of terminal emulation to use. 

The definition of the ConIn and ConOut variables is here [4]

[1] 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c#L24
[2] 
https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c#L50
[3] 
https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h#L129
[4] 
https://uefi.org/specs/UEFI/2.10/03_Boot_Manager.html#globally-defined-variables

Thanks,

Andrew Fish

> 
> 
> 
> 



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

Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Maintainers.txt: Update maintainers for Ampere platforms

[Leif Lindholm]

On Thu, Sep 07, 2023 at 22:46:43 +0700, Nhi Pham via groups.io wrote:
> This adds Rebecca Cran as reviewer, updates myself as maintainer, and
> Leif as reviewer. Also, removes Vu Nguyen and Thang Nguyen due to no
> longer working on EDK2.
> 
> Signed-off-by: Nhi Pham 

Reviewed-by: Leif Lindholm 

> ---
>  Maintainers.txt | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index d1d7613ef48d..d77f0dd35ac4 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -99,11 +99,10 @@ M: Leif Lindholm 
>  Ampere Computing
>  F: Platform/Ampere
>  F: Silicon/Ampere
> -R: Nhi Pham 
> -R: Vu Nguyen 
> -R: Thang Nguyen 
> +M: Nhi Pham 
>  R: Chuong Tran 
> -M: Leif Lindholm 
> +R: Leif Lindholm 
> +R: Rebecca Cran 
>  
>  ARM
>  F: Platform/ARM/
> -- 
> 2.25.1
> 
> 
> 
> 
> 
> 


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

Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Maintainers.txt: Update maintainers for Ampere platforms

[Rebecca Cran via groups.io]

On 9/7/2023 9:46 AM, Nhi Pham wrote:

This adds Rebecca Cran as reviewer, updates myself as maintainer, and
Leif as reviewer. Also, removes Vu Nguyen and Thang Nguyen due to no
longer working on EDK2.

Signed-off-by: Nhi Pham 
---
  Maintainers.txt | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index d1d7613ef48d..d77f0dd35ac4 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -99,11 +99,10 @@ M: Leif Lindholm 
  Ampere Computing
  F: Platform/Ampere
  F: Silicon/Ampere
-R: Nhi Pham 
-R: Vu Nguyen 
-R: Thang Nguyen 
+M: Nhi Pham 
  R: Chuong Tran 
-M: Leif Lindholm 
+R: Leif Lindholm 
+R: Rebecca Cran 
  
  ARM

  F: Platform/ARM/


Reviewed-by: Rebecca Cran 


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

[edk2-devel] [edk2-platforms][PATCH 1/1] Maintainers.txt: Update maintainers for Ampere platforms

[Nhi Pham via groups.io]

This adds Rebecca Cran as reviewer, updates myself as maintainer, and
Leif as reviewer. Also, removes Vu Nguyen and Thang Nguyen due to no
longer working on EDK2.

Signed-off-by: Nhi Pham 
---
 Maintainers.txt | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index d1d7613ef48d..d77f0dd35ac4 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -99,11 +99,10 @@ M: Leif Lindholm 
 Ampere Computing
 F: Platform/Ampere
 F: Silicon/Ampere
-R: Nhi Pham 
-R: Vu Nguyen 
-R: Thang Nguyen 
+M: Nhi Pham 
 R: Chuong Tran 
-M: Leif Lindholm 
+R: Leif Lindholm 
+R: Rebecca Cran 
 
 ARM
 F: Platform/ARM/
-- 
2.25.1



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

Re: [edk2-devel] [PATCH v1 2/2] StandaloneMmPkg: Fix HOB space and heap space conflicted issue

[Nhi Pham via groups.io]

On 9/6/2023 11:22 PM, Oliver Smith-Denny wrote:

On 9/6/2023 1:50 AM, Ard Biesheuvel wrote:
On Wed, 6 Sept 2023 at 09:56, Nhi Pham 
 wrote:


On 9/6/2023 1:33 PM, Ni, Ray wrote:

[EXTERNAL EMAIL NOTICE: This email originated from an external sender.
Please be mindful of safe email handling and proprietary information
protection practices.]

I am a bit confused.

The HOB list in standalone MM is read-only. Why could any module call
BuildGuidHob() to modify the HOB.

I saw Oliver mentioned something about StMM. I don't know what that 
is.

But it seems that's ARM specific. Then, I don't think it's proper to
modify code here for a specific arch ARM.


The HOB creation is available in the
StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf. If
other architectures also use that instance, I think the issue is not
specific to ARM.



The question here is whether the implementation follows the PI spec,
and whether HOB creation should be supported to begin with.



My reading of the PI spec is that this implementation does not follow
it. However, the PI spec is not very explicit about Standalone MM in
general, but particularly in relation to HOBs.

However, in the generic HOB section of PI spec v1.7, Vol. 3, section 4
(entitled HOB Design Discussion) it explicitly lays out that there are
HOB producer phases and HOB consumer phases. It uses PEI as a HOB
producer phase and DXE as a HOB consumer phase and explicitly says
that the HOB consumer phase must treat HOBs as read-only memory, per
Ray's comment.

In vol. 4, section 2.2, in discussing the Standalone MM entry point,
the document talks about the HOB list being passed to Standalone MM
to consume, which per the reading of the above section would classify
Standalone MM as a HOB consumer phase, where HOBs should then be
read-only.

So, I believe that we should not support HOB creation in Standalone MM
and instead rely on other mechanisms to pass information within the
phase. Per Nhi's other email in this thread, we should have the
discussion on how to solve that specific problem and that may well
lead to a discussion on whether HOBs are in fact the right mechanism
here, but I tend to lean towards leaving something as architectural as
HOBs to what the PI spec defines and using different mechanisms to
accomplish in-phase communication.
Thanks Oliver so much for that. I agree. We should focus on my specific 
problem with UEFI Variable Flash Info in StandaloneMM in another thread.


Does this reading of the spec align with others' expectations? As I
mentioned to Ray in another thread, Standalone MM feels like it could
have extra clarification in a few areas in the PI spec.


Thanks again. The HOB library should be updated to remove the HOB 
creation once we have the clarification.


Regards,

Nhi



Thanks,
Oliver



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

Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Oliver Smith-Denny]

On 9/7/2023 6:10 AM, Laszlo Ersek wrote:

(replying on the webui... sorry!)

The problem is actually embedded in MdePkg and MdeModulePkg.

- In DxeMain() (and in functions called by DxeMain()), we call DebugLib 
APIs *before* reaching ProcessLibraryConstructorList().


- In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to 
BaseDebugLibSerialPort (from MdePkg).


- BaseDebugLibSerialPort has a constructor function 
(BaseDebugLibSerialPortConstructor()).


These already suffice for broken DebugLib behavior. APIs of a library 
class should not be called because the library instance has a chance to 
initialize.


The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor 
calls SerialPortInitialize, but our SerialPortInitialize (in 
FdtPL011SerialPortLib) does nothing. Well, the latter doesn't need to do 
anything, because FdtPL011SerialPortLib has its own constructor 
(FdtPL011SerialPortLibInitialize), thus, if constructors were called 
properly, then BaseDebugLibSerialPort + FdtPL011SerialPortLib would work 
properly together, regardless of SerialPortInitialize being empty in the 
latter.


Basically the DXE Core has a hidden requirement -- it can only use such 
DebugLib instances that need no explicit initialization.


The proposed patch works around the problem by satisfying that hidden 
requirement one level lower down: in the SerialPortLib instance. The 
initialization of BaseDebugLibSerialPort is still busted (its 
constructor is not called, so it cannot call SerialPortInitialize 
either), but now it is masked, because EarlyFdtPL011SerialPortLib works 
withouth *both* SerialPortInitialize and construction.


The real fix would be to make the DXE Core requirement explicit, by 
introducing separate (dedicated) DebugLib and SerialPortLib *classes* 
(whose APIs are guaranteed to work without initialization).


Laszlo


Thanks for the comprehensive breakdown! :). I completely agree that
fixing this at the upper level (and ideally documenting this
requirement) is the better move.

I can drop this patch and take a crack at that. I'm in the last few
weeks leading up to an extended parental leave, so we'll see if I can
squeeze it in prior to then :).

Oliver


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

Re: [edk2-devel] [edk2][RFC] OvmfPkg/AcpiPlatformDxe: patch FADT PSCI bits if FDT advertises it

[Ard Biesheuvel]

On Thu, 7 Sept 2023 at 16:51, Laszlo Ersek  wrote:
>
> On 9/7/23 16:27, Ard Biesheuvel wrote:
> > On Mon, 16 Jan 2023 at 12:39, Evgeny Iakovlev
> >  wrote:
> >>
> >> EL3 firmware may implement PSCI interface on aarch64 platforms,
> >> including qemu-tcg-aarch64. However, EL3 firmware does not usually own
> >> pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
> >> EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
> >> firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
> >> EL3 firmware is present.
> >>
> >> PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
> >> expect to see all information published in ACPI. To better support
> >> running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
> >> and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
> >> bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
> >> advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
> >> FDT on arm, so it is ideally poised to handle this.
> >>
> >> This change illustrates how it could potentially be done. I am looking
> >> for comments on overall validity of the idea to patch FADT and whether
> >> or not this particular approach of handling it in AcpiPlatformDxe is the
> >> way to do it or maybe it is better to handle it via
> >> gQemuAcpiTableNotifyProtocolGuid somehow.
> >>
> >> Signed-off-by: Evgeny Iakovlev 
> >
> > Thanks for the patch, and apologies for the lack of response.
> >
> > First of all, I suspect this patch breaks non-ARM users of this
> > driver, so the patch is problematic as is. (It makes
> > gFdtClientProtocolGuid mandatory, right?)
> >
> > Then, I'd like to hear from other folks on cc what they think about
> > this. Perhaps it is simply a matter of tweaking QEMU so it exposes the
> > correct PSCI setting in the FADT when it emulates secure world.
> > Patching it like this feels like a last resort to me, rather than a
> > well designed interface.
>
> Thanks for the CC; both of your concerns are valid.
>
> The FDT client proto GUID has no reason to exist in (e.g.) an X64 OVMF
> build.
>
> Second, and more importantly, this is a total layering violation for
> AcpiPlatformDxe. QEMU is the single source of truth for AcpiPlatformDxe,
> and AcpiPlatformDxe must remain as blind as possible to the actual ACPI
> content.
>
> In the situation described by the commit message, the ACPI content
> exposed by QEMU is simply invalid. That's what should be fixed in QEMU
> (and not papered over in edk2). Something somewhere is responsible for
> setting the property value in question to "hvc"; that something
> precisely is responsible (directly or indirectly) for making QEMU expose
> the proper FADT.
>
> I've now grepped the QEMU source tree for '"hvc"'; the relevant hit
> seems to be in "hw/arm/boot.c", function fdt_add_psci_node(), under case
> label QEMU_PSCI_CONDUIT_HVC. So, whatever sets psci-conduit to
> QEMU_PSCI_CONDUIT_HVC should also make sure the FADT matches it.
>
> Taking one step back, in "hw/arm/virt.c" we have:
>
> if (vms->secure && firmware_loaded) {
> vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
> } else if (vms->virt) {
> vms->psci_conduit = QEMU_PSCI_CONDUIT_SMC;
> } else {
> vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC;
> }
>

The problem here is that QEMU does not know whether the EL3 firmware
running in the guest implements PSCI or not.

> So I figure the ACPI generator should be steered off the same information.
>
> BTW... I see the following in "hw/arm/virt-acpi-build.c", function
> build_fadt_rev6():
>
> case QEMU_PSCI_CONDUIT_HVC:
> fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
>  ACPI_FADT_ARM_PSCI_USE_HVC;
> break;
>
> That dates back minimally as far as commit 79e993a0a804
> ("hw/arm/virt-acpi-build: use SMC if booting in EL2", 2017-01-20).
>
> So why is it not taking effect? Patching edk2 should not be necessary at
> all, QEMU should already be doing the right thing.
>
> The commit message states, "Qemu itself also won't advertise PSCI in
> [...] ACPI if EL3 firmware is present"; if that's correct (I can't
> tell), then it may be the problem.
>

Exactly.

When not emulating EL2 or EL3 (which is equivalent to the KVM case),
PSCI calls are made using HVC instructions, which are handled by QEMU
directly.

When EL2 emulation is enabled, PSCI calls are made using SMC
instructions but using the same handling in QEMU.

When EL3 emulation is enabled, QEMU can no longer 'overrule' the side
effects of SMC instructions but has to deliver them to the firmware
that occupies EL3. Whether or not that firmware implements PSCI is not
known to QEMU, and so it assumes it is not, and populates the FADT
fields accordingly.

IIRC QEMU has some patching logic for ACPI tables. Could we make use
of that here?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to 

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt/README.md: bring your own OpenSBI

[Laszlo Ersek]

On 9/7/23 16:58, Laszlo Ersek wrote:
> Explain how users can compose their pre-OS environment purely from
> binaries they've built themselves.
> 
> Cc: Andrei Warkentin 
> Cc: Ard Biesheuvel 
> Cc: Gerd Hoffmann 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Sunil V L 
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> rendered version:
> 
> 
> https://github.com/lersek/edk2/tree/bring-your-own-opensbi/OvmfPkg/RiscVVirt#test-with-your-own-opensbi-binary
> 
>  OvmfPkg/RiscVVirt/README.md | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/OvmfPkg/RiscVVirt/README.md b/OvmfPkg/RiscVVirt/README.md
> index 8c3ac37b802a..dbb40bbe89b0 100644
> --- a/OvmfPkg/RiscVVirt/README.md
> +++ b/OvmfPkg/RiscVVirt/README.md
> @@ -69,3 +69,20 @@ Below example shows how to boot openSUSE Tumbleweed E20.
>  -device virtio-net-pci,netdev=net0 \
>  -device virtio-blk-device,drive=hd0 \
>  -drive 
> file=openSUSE-Tumbleweed-RISC-V-E20-efi.riscv64.raw,format=raw,id=hd0
> +
> +## Test with your own OpenSBI binary
> +Using the above QEMU command line, **RISCV_VIRT_CODE.fd** is launched by the
> +OpenSBI binary that is bundled with QEMU. You can build your own OpenSBI 
> binary
> +as well:
> +
> +OPENSBI_DIR=...
> +git clone https://github.com/riscv/opensbi.git $OPENSBI_DIR
> +make -C $OPENSBI_DIR \
> +-j $(getconf _NPROCESSORS_ONLN) \
> +CROSS_COMPILE=riscv64-linux-gnu- \
> +PLATFORM=generic
> +
> +then specify that binary for QEMU, with the following additional command line
> +option:
> +
> +-bios $OPENSBI_DIR/build/platform/generic/firmware/fw_dynamic.bin

Further tweaks on the horizon (for v2, or on top), highlighted by Drew
Jones:

- explain -kernel/-initrd/-append (after testing it, of course! I'm
unsure if fw_cfg kernel boot actually works with RiscVVirt!)

- point out that -bios (OpenSBI) makes no difference under KVM, as KVM
services those SBI ecalls

- explain "acpi=off" (in the context, not in the added text): Linux
support (i.e. the ACPI *consumer* side) is still under development /
work in progress.

Laszlo



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

Re: [edk2-devel] [Patch V2 1/5] MdeModulePkg: add MpService2Ppi field in SMM_S3_RESUME_STATE

[Wang, Jian J]

Reviewed-by: Jian J Wang 

Regards,
Jian



> -Original Message-
> From: Tan, Dun 
> Sent: Wednesday, September 06, 2023 11:22 AM
> To: devel@edk2.groups.io; Tan, Dun 
> Cc: Ni, Ray ; Wang, Jian J ; Gao,
> Liming 
> Subject: RE: [edk2-devel] [Patch V2 1/5] MdeModulePkg: add MpService2Ppi
> field in SMM_S3_RESUME_STATE
> 
> Hi Jian and Liming,
> 
> Could you please help to review this patch?
> 
> Thanks,
> Dun
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of duntan
> Sent: Monday, August 21, 2023 10:10 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Wang, Jian J ; Gao,
> Liming 
> Subject: [edk2-devel] [Patch V2 1/5] MdeModulePkg: add MpService2Ppi field in
> SMM_S3_RESUME_STATE
> 
> Add MpService2Ppi field in SMM_S3_RESUME_STATE of AcpiS3Context.h. It will
> be used to wakeup AP to do the CPU initialization during smm s3 boot flow in
> following patches.
> With this field, we can avoid sending InitSipiSipi to wakeup AP.
> 
> Signed-off-by: Dun Tan 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> ---
>  MdeModulePkg/Include/Guid/AcpiS3Context.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> index 645496d191..72d173c4fd 100644
> --- a/MdeModulePkg/Include/Guid/AcpiS3Context.h
> +++ b/MdeModulePkg/Include/Guid/AcpiS3Context.h
> @@ -1,7 +1,7 @@
>  /** @file
>Definitions for data structures used in S3 resume.
> 
> -Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -30,6 +30,7 @@ typedef struct {
>EFI_PHYSICAL_ADDRESSReturnContext1;
>EFI_PHYSICAL_ADDRESSReturnContext2;
>EFI_PHYSICAL_ADDRESSReturnStackPointer;
> +  EFI_PHYSICAL_ADDRESSMpService2Ppi;
>EFI_PHYSICAL_ADDRESSSmst;
>  } SMM_S3_RESUME_STATE;
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 



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

[edk2-devel] VT100 terminal for UEFI shell

[Marcin Juszkiewicz]

Is there a way to have VT100 (or any other black/white, non-ANSI) 
terminal for UEFI Shell?


I do many runs of QEMU/sbsa-ref with logging and all those ANSI colour 
codes only make problems.



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

[edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt/README.md: bring your own OpenSBI

[Laszlo Ersek]

Explain how users can compose their pre-OS environment purely from
binaries they've built themselves.

Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Sunil V L 
Signed-off-by: Laszlo Ersek 
---

Notes:
rendered version:


https://github.com/lersek/edk2/tree/bring-your-own-opensbi/OvmfPkg/RiscVVirt#test-with-your-own-opensbi-binary

 OvmfPkg/RiscVVirt/README.md | 17 +
 1 file changed, 17 insertions(+)

diff --git a/OvmfPkg/RiscVVirt/README.md b/OvmfPkg/RiscVVirt/README.md
index 8c3ac37b802a..dbb40bbe89b0 100644
--- a/OvmfPkg/RiscVVirt/README.md
+++ b/OvmfPkg/RiscVVirt/README.md
@@ -69,3 +69,20 @@ Below example shows how to boot openSUSE Tumbleweed E20.
 -device virtio-net-pci,netdev=net0 \
 -device virtio-blk-device,drive=hd0 \
 -drive 
file=openSUSE-Tumbleweed-RISC-V-E20-efi.riscv64.raw,format=raw,id=hd0
+
+## Test with your own OpenSBI binary
+Using the above QEMU command line, **RISCV_VIRT_CODE.fd** is launched by the
+OpenSBI binary that is bundled with QEMU. You can build your own OpenSBI binary
+as well:
+
+OPENSBI_DIR=...
+git clone https://github.com/riscv/opensbi.git $OPENSBI_DIR
+make -C $OPENSBI_DIR \
+-j $(getconf _NPROCESSORS_ONLN) \
+CROSS_COMPILE=riscv64-linux-gnu- \
+PLATFORM=generic
+
+then specify that binary for QEMU, with the following additional command line
+option:
+
+-bios $OPENSBI_DIR/build/platform/generic/firmware/fw_dynamic.bin


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

Re: [edk2-devel] [edk2][RFC] OvmfPkg/AcpiPlatformDxe: patch FADT PSCI bits if FDT advertises it

[Laszlo Ersek]

On 9/7/23 16:27, Ard Biesheuvel wrote:
> On Mon, 16 Jan 2023 at 12:39, Evgeny Iakovlev
>  wrote:
>>
>> EL3 firmware may implement PSCI interface on aarch64 platforms,
>> including qemu-tcg-aarch64. However, EL3 firmware does not usually own
>> pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
>> EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
>> firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
>> EL3 firmware is present.
>>
>> PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
>> expect to see all information published in ACPI. To better support
>> running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
>> and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
>> bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
>> advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
>> FDT on arm, so it is ideally poised to handle this.
>>
>> This change illustrates how it could potentially be done. I am looking
>> for comments on overall validity of the idea to patch FADT and whether
>> or not this particular approach of handling it in AcpiPlatformDxe is the
>> way to do it or maybe it is better to handle it via
>> gQemuAcpiTableNotifyProtocolGuid somehow.
>>
>> Signed-off-by: Evgeny Iakovlev 
> 
> Thanks for the patch, and apologies for the lack of response.
> 
> First of all, I suspect this patch breaks non-ARM users of this
> driver, so the patch is problematic as is. (It makes
> gFdtClientProtocolGuid mandatory, right?)
> 
> Then, I'd like to hear from other folks on cc what they think about
> this. Perhaps it is simply a matter of tweaking QEMU so it exposes the
> correct PSCI setting in the FADT when it emulates secure world.
> Patching it like this feels like a last resort to me, rather than a
> well designed interface.

Thanks for the CC; both of your concerns are valid.

The FDT client proto GUID has no reason to exist in (e.g.) an X64 OVMF
build.

Second, and more importantly, this is a total layering violation for
AcpiPlatformDxe. QEMU is the single source of truth for AcpiPlatformDxe,
and AcpiPlatformDxe must remain as blind as possible to the actual ACPI
content.

In the situation described by the commit message, the ACPI content
exposed by QEMU is simply invalid. That's what should be fixed in QEMU
(and not papered over in edk2). Something somewhere is responsible for
setting the property value in question to "hvc"; that something
precisely is responsible (directly or indirectly) for making QEMU expose
the proper FADT.

I've now grepped the QEMU source tree for '"hvc"'; the relevant hit
seems to be in "hw/arm/boot.c", function fdt_add_psci_node(), under case
label QEMU_PSCI_CONDUIT_HVC. So, whatever sets psci-conduit to
QEMU_PSCI_CONDUIT_HVC should also make sure the FADT matches it.

Taking one step back, in "hw/arm/virt.c" we have:

if (vms->secure && firmware_loaded) {
vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
} else if (vms->virt) {
vms->psci_conduit = QEMU_PSCI_CONDUIT_SMC;
} else {
vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC;
}

So I figure the ACPI generator should be steered off the same information.

BTW... I see the following in "hw/arm/virt-acpi-build.c", function
build_fadt_rev6():

case QEMU_PSCI_CONDUIT_HVC:
fadt.arm_boot_arch = ACPI_FADT_ARM_PSCI_COMPLIANT |
 ACPI_FADT_ARM_PSCI_USE_HVC;
break;

That dates back minimally as far as commit 79e993a0a804
("hw/arm/virt-acpi-build: use SMC if booting in EL2", 2017-01-20).

So why is it not taking effect? Patching edk2 should not be necessary at
all, QEMU should already be doing the right thing.

The commit message states, "Qemu itself also won't advertise PSCI in
[...] ACPI if EL3 firmware is present"; if that's correct (I can't
tell), then it may be the problem.

Thanks
Laszlo

> 
> 
>> ---
>> Resending the message because i wasn't subscribed to the rfc group the
>> last time, sorry.
>> ---
>>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |   4 +-
>>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 108 
>>  2 files changed, 111 insertions(+), 1 deletion(-)
>>
>> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
>> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> index 8939dde425..a011a43743 100644
>> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> @@ -30,6 +30,7 @@
>>QemuFwCfgAcpi.c
>>
>>  [Packages]
>> +  EmbeddedPkg/EmbeddedPkg.dec
>>MdeModulePkg/MdeModulePkg.dec
>>MdePkg/MdePkg.dec
>>OvmfPkg/OvmfPkg.dec
>> @@ -50,6 +51,7 @@
>>  [Protocols]
>>gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>>gEfiPciIoProtocolGuid # PROTOCOL 
>> SOMETIMES_CONSUMED
>> +  gFdtClientProtocolGuid# PROTOCOL 
>> SOMETIMES_CONSUMED
>>

Re: [edk2-devel] [edk2][RFC] OvmfPkg/AcpiPlatformDxe: patch FADT PSCI bits if FDT advertises it

[Ard Biesheuvel]

On Mon, 16 Jan 2023 at 12:39, Evgeny Iakovlev
 wrote:
>
> EL3 firmware may implement PSCI interface on aarch64 platforms,
> including qemu-tcg-aarch64. However, EL3 firmware does not usually own
> pulling and deploying ACPI tables from qemu fw_cfg. Thus the only way
> EL3 can advertise PSCI on qemu is in FDT. One such EL3 fw is ARM trusted
> firmware. Qemu itself also won't advertise PSCI in either FDT or ACPI if
> EL3 firmware is present.
>
> PSCI can be advertised in both FDT and ACPI, and Hyper-V/NT kernel
> expect to see all information published in ACPI. To better support
> running Hyper-V/NT on qemu-tcg-aarch64 with EDK2 as UEFI implementation
> and ARM trusted firmware as EL3 PSCI implementation we can patch in PSCI
> bits in ACPI FADT when pulling tables from fw_cfg if PSCI node is
> advertised in FDT. EDK2 owns ACPI table publishing and is also aware of
> FDT on arm, so it is ideally poised to handle this.
>
> This change illustrates how it could potentially be done. I am looking
> for comments on overall validity of the idea to patch FADT and whether
> or not this particular approach of handling it in AcpiPlatformDxe is the
> way to do it or maybe it is better to handle it via
> gQemuAcpiTableNotifyProtocolGuid somehow.
>
> Signed-off-by: Evgeny Iakovlev 

Thanks for the patch, and apologies for the lack of response.

First of all, I suspect this patch breaks non-ARM users of this
driver, so the patch is problematic as is. (It makes
gFdtClientProtocolGuid mandatory, right?)

Then, I'd like to hear from other folks on cc what they think about
this. Perhaps it is simply a matter of tweaking QEMU so it exposes the
correct PSCI setting in the FADT when it emulates secure world.
Patching it like this feels like a last resort to me, rather than a
well designed interface.


> ---
> Resending the message because i wasn't subscribed to the rfc group the
> last time, sorry.
> ---
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |   4 +-
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 108 
>  2 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 8939dde425..a011a43743 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -30,6 +30,7 @@
>QemuFwCfgAcpi.c
>
>  [Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
>MdeModulePkg/MdeModulePkg.dec
>MdePkg/MdePkg.dec
>OvmfPkg/OvmfPkg.dec
> @@ -50,6 +51,7 @@
>  [Protocols]
>gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
> +  gFdtClientProtocolGuid# PROTOCOL SOMETIMES_CONSUMED
>gQemuAcpiTableNotifyProtocolGuid  # PROTOCOL PRODUCES
>
>  [Guids]
> @@ -64,4 +66,4 @@
>gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
>
>  [Depex]
> -  gEfiAcpiTableProtocolGuid
> +  gEfiAcpiTableProtocolGuid AND gFdtClientProtocolGuid
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c 
> b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index c8dee17c13..31d8665d2d 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -9,6 +9,7 @@
>  **/
>
>  #include // EFI_ACPI_DESCRIPTION_HEADER
> +#include   // 
> EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE
>  #include   // QEMU_LOADER_FNAME_SIZE
>  #include   // AsciiStrCmp()
>  #include // CopyMem()
> @@ -20,6 +21,7 @@
>  #include  // gBS
>
>  #include 
> +#include 
>  #include "AcpiPlatform.h"
>  EFI_HANDLE   mQemuAcpiHandle = NULL;
>  QEMU_ACPI_TABLE_NOTIFY_PROTOCOL  mAcpiNotifyProtocol;
> @@ -815,6 +817,92 @@ UndoCmdWritePointer (
>  ));
>  }
>
> +/**
> +  Locate a PSCI node in DTB published by EL3 firmware that implemented it
> +  and use information in it to patch ACPI FADT PSCI bits.
> +
> +  The reason for it is that EL3 PSCI implementation, which is not EDK2,
> +  doesn't own ACPI tables and cannot advertise it there itself, it is using 
> DTB instead.
> +
> +  Qemu also won't advertise PSCI if EL3 firmware is present.
> +  A real example of that is running ARM trusted firmware in EL3 with PSCI 
> enabled.
> +
> +  @param[in] PonterValueFADT base address
> +
> +  @param[in] TableSize  Size of table in bytes
> +
> +  @return   EFI_SUCCESS if FAST was patched or if patching 
> was not needed,
> +error status returned by FDT client protocol 
> otherwise.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PatchFadtPsciBits (
> +  IN UINT64 PointerValue,
> +  IN UINTN  TableSize
> +  )
> +{
> +  EFI_STATUSStatus;
> +  FDT_CLIENT_PROTOCOL   *FdtClient;
> +  CONST VOID*Prop;
> +  

Re: [edk2-devel] [PATCH] ArmPkg/ArmPsciMpServices Add EFI_NOT_READY return

[Ard Biesheuvel]

On Thu, 29 Jun 2023 at 22:47, Jeff Brasen via groups.io
 wrote:
>
> Add EFI_NOT_READY return if the CPU can not be enabled if the
> processor is already on.
>
> This can occur in normal use if the CPU is still being turned off from
> a previous call when this is called again.
>
> Signed-off-by: Jeff Brasen 

Acked-by: Ard Biesheuvel 

I'll queue this up - thanks.

> ---
>  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c 
> b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> index f822a9877c..e7f4223513 100644
> --- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> +++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> @@ -103,7 +103,9 @@ DispatchCpu (
>
>ArmCallSmc ();
>
> -  if (Args.Arg0 != ARM_SMC_PSCI_RET_SUCCESS) {
> +  if (Args.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) {
> +Status = EFI_NOT_READY;
> +  } else if (Args.Arg0 != ARM_SMC_PSCI_RET_SUCCESS) {
>  DEBUG ((DEBUG_ERROR, "PSCI_CPU_ON call failed: %d\n", Args.Arg0));
>  Status = EFI_DEVICE_ERROR;
>}
> --
> 2.25.1
>
>
> 
>
>


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

[edk2-devel] Now: TianoCore edk2-test Bug Triage Meeting - Thursday, September 7, 2023 #cal-notice

[Group Notification]

*TianoCore edk2-test Bug Triage Meeting*

*When:*
Thursday, September 7, 2023
10:00pm to 11:00pm
(UTC+08:00) Asia/Shanghai

*Where:*
https://armltd.zoom.us/j/91247522013?pwd=ei9nUndTbG9oWEROS2M1aVREZkpiQT09=addon

*Organizer:* Edhaya Chandran edhaya.chand...@arm.com ( 
edhaya.chand...@arm.com?subject=Re:%20Event:%20TianoCore%20edk2-test%20Bug%20Triage%20Meeting
 )

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

*Description:*


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

Re: [edk2-devel] [PATCH v3 1/1] ArmVirtPkg: use PcdTerminalTypeGuidBuffer for VirtioSerial console

[Ard Biesheuvel]

On Thu, 6 Jul 2023 at 14:30, Gerd Hoffmann  wrote:
>
> Be consistent with pl011-based serial console setup.
>
> Suggested-by: Laszlo Ersek 
> Signed-off-by: Gerd Hoffmann 

Acked-by: Ard Biesheuvel 

I'll queue this one up too
> ---
>  ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c 
> b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index b92a916f7eec..85c01351b09d 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -606,7 +606,7 @@ SetupVirtioSerial (
>  1
>};
>
> -  STATIC CONST VENDOR_DEVICE_PATH  TerminalNode = {
> +  STATIC VENDOR_DEVICE_PATH  TerminalNode = {
>  {
>MESSAGING_DEVICE_PATH,
>MSG_VENDOR_DP,
> @@ -615,7 +615,7 @@ SetupVirtioSerial (
>  (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
>},
>  },
> -DEVICE_PATH_MESSAGING_VT_UTF8
> +// copy from PcdTerminalTypeGuidBuffer
>};
>
>EFI_STATUSStatus;
> @@ -634,6 +634,11 @@ SetupVirtioSerial (
>  return;
>}
>
> +  CopyGuid (
> +,
> +PcdGetPtr (PcdTerminalTypeGuidBuffer)
> +);
> +
>DevicePath = AppendDevicePathNode (
>   DevicePath,
>   
> --
> 2.41.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108392): https://edk2.groups.io/g/devel/message/108392
Mute This Topic: https://groups.io/mt/99984422/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] OvmfPkg/Include: remove QemuSmramSaveStateMap.h

[Ard Biesheuvel]

On Tue, 4 Jul 2023 at 11:52, Gerd Hoffmann  wrote:
>
> The qemu/kvm SMM emulation uses the AMD SaveState layout.
>
> So, now that we have AMD SaveState support merged we can just use
> Amd/SmramSaveStateMap.h, QemuSmramSaveStateMap.h is not needed any more.
>
> Signed-off-by: Gerd Hoffmann 

Acked-by: Ard Biesheuvel 

I'll queue this one up as well.

> ---
>  .../Include/Register/QemuSmramSaveStateMap.h  | 178 --
>  .../PeiDxeMemEncryptSevLibInternal.c  |   4 +-
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c |  21 +--
>  3 files changed, 12 insertions(+), 191 deletions(-)
>  delete mode 100644 OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
>
> diff --git a/OvmfPkg/Include/Register/QemuSmramSaveStateMap.h 
> b/OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
> deleted file mode 100644
> index 8ffde0548c4c..
> --- a/OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
> +++ /dev/null
> @@ -1,178 +0,0 @@
> -/** @file
> -SMRAM Save State Map Definitions.
> -
> -SMRAM Save State Map definitions based on contents of the
> -Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> -  Volume 3C, Section 34.4 SMRAM
> -  Volume 3C, Section 34.5 SMI Handler Execution Environment
> -  Volume 3C, Section 34.7 Managing Synchronous and Asynchronous SMIs
> -
> -and the AMD64 Architecture Programmer's Manual
> -  Volume 2, Section 10.2 SMM Resources
> -
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> -Copyright (c) 2015, Red Hat, Inc.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef __QEMU_SMRAM_SAVE_STATE_MAP_H__
> -#define __QEMU_SMRAM_SAVE_STATE_MAP_H__
> -
> -#pragma pack (1)
> -
> -///
> -/// 32-bit SMRAM Save State Map
> -///
> -typedef struct {
> -  UINT8 Reserved0[0x200]; // 7c00h
> -  UINT8 Reserved1[0xf8];  // 7e00h
> -  UINT32SMBASE;   // 7ef8h
> -  UINT32SMMRevId; // 7efch
> -  UINT16IORestart;// 7f00h
> -  UINT16AutoHALTRestart;  // 7f02h
> -  UINT8 Reserved2[0x9C];  // 7f08h
> -  UINT32IOMemAddr;// 7fa0h
> -  UINT32IOMisc;   // 7fa4h
> -  UINT32_ES;  // 7fa8h
> -  UINT32_CS;  // 7fach
> -  UINT32_SS;  // 7fb0h
> -  UINT32_DS;  // 7fb4h
> -  UINT32_FS;  // 7fb8h
> -  UINT32_GS;  // 7fbch
> -  UINT32Reserved3;// 7fc0h
> -  UINT32_TR;  // 7fc4h
> -  UINT32_DR7; // 7fc8h
> -  UINT32_DR6; // 7fcch
> -  UINT32_EAX; // 7fd0h
> -  UINT32_ECX; // 7fd4h
> -  UINT32_EDX; // 7fd8h
> -  UINT32_EBX; // 7fdch
> -  UINT32_ESP; // 7fe0h
> -  UINT32_EBP; // 7fe4h
> -  UINT32_ESI; // 7fe8h
> -  UINT32_EDI; // 7fech
> -  UINT32_EIP; // 7ff0h
> -  UINT32_EFLAGS;  // 7ff4h
> -  UINT32_CR3; // 7ff8h
> -  UINT32_CR0; // 7ffch
> -} QEMU_SMRAM_SAVE_STATE_MAP32;
> -
> -///
> -/// 64-bit SMRAM Save State Map
> -///
> -typedef struct {
> -  UINT8 Reserved0[0x200]; // 7c00h
> -
> -  UINT16_ES; // 7e00h
> -  UINT16_ESAccessRights; // 7e02h
> -  UINT32_ESLimit;// 7e04h
> -  UINT64_ESBase; // 7e08h
> -
> -  UINT16_CS; // 7e10h
> -  UINT16_CSAccessRights; // 7e12h
> -  UINT32_CSLimit;// 7e14h
> -  UINT64_CSBase; // 7e18h
> -
> -  UINT16_SS; // 7e20h
> -  UINT16_SSAccessRights; // 7e22h
> -  UINT32_SSLimit;// 7e24h
> -  UINT64_SSBase; // 7e28h
> -
> -  UINT16_DS; // 7e30h
> -  UINT16_DSAccessRights; // 7e32h
> -  UINT32_DSLimit;// 7e34h
> -  UINT64_DSBase; // 7e38h
> -
> -  UINT16_FS; // 7e40h
> -  UINT16_FSAccessRights; // 7e42h
> -  UINT32_FSLimit;// 7e44h
> -  UINT64_FSBase; // 7e48h
> -
> -  UINT16_GS; // 7e50h
> -  UINT16_GSAccessRights; // 7e52h
> -  UINT32_GSLimit;// 7e54h
> -  UINT64_GSBase; // 7e58h
> -
> -  UINT32_GDTRReserved1;  // 7e60h
> -  UINT16_GDTRLimit;  // 7e64h
> -  UINT16_GDTRReserved2;  // 7e66h
> -  UINT64_GDTRBase;   // 7e68h
> -
> -  UINT16_LDTR; // 7e70h
> -  UINT16_LDTRAccessRights; // 7e72h
> -  UINT32_LDTRLimit;// 7e74h
> -  UINT64_LDTRBase; // 7e78h
> -
> -  UINT32_IDTRReserved1;  // 7e80h
> -  UINT16_IDTRLimit;  // 7e84h
> -  UINT16_IDTRReserved2;  // 7e86h
> -  UINT64_IDTRBase;   // 7e88h
> -
> -  UINT16_TR; // 7e90h
> -  UINT16_TRAccessRights; // 7e92h
> -  UINT32_TRLimit;// 7e94h
> -  UINT64_TRBase; // 7e98h
> -
> -  UINT64IO_RIP;  // 7ea0h
> -  UINT64IO_RCX;  // 7ea8h
> -  UINT64IO_RSI;   

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/PlatformBootManagerLib: setup virtio-mmio devices.

[Ard Biesheuvel]

On Thu, 6 Jul 2023 at 17:31, Gerd Hoffmann  wrote:
>
> Add DetectAndPreparePlatformVirtioDevicePath() helper function
> to setup virtio-mmio devices.  Start with virtio-serial support.
>
> This makes virtio console usable with microvm.
>
> Signed-off-by: Gerd Hoffmann 

Acked-by: Ard Biesheuvel 

I'll queue this up

> ---
>  .../PlatformBootManagerLib.inf|  1 +
>  .../PlatformBootManagerLib/BdsPlatform.c  | 31 +++
>  2 files changed, 32 insertions(+)
>
> diff --git 
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 6b396eac7daf..c6ffc1ed9ed5 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -75,6 +75,7 @@ [Pcd.IA32, Pcd.X64]
>  [Protocols]
>gEfiDecompressProtocolGuid
>gEfiPciRootBridgeIoProtocolGuid
> +  gVirtioDeviceProtocolGuid
>gEfiS3SaveStateProtocolGuid   # PROTOCOL SOMETIMES_CONSUMED
>gEfiDxeSmmReadyToLockProtocolGuid # PROTOCOL SOMETIMES_PRODUCED
>gEfiLoadedImageProtocolGuid   # PROTOCOL SOMETIMES_PRODUCED
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 8dc2bbf97371..88c39df4aea9 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1236,6 +1237,30 @@ DetectAndPreparePlatformPciDevicePath (
>return Status;
>  }
>
> +EFI_STATUS
> +EFIAPI
> +DetectAndPreparePlatformVirtioDevicePath (
> +  IN EFI_HANDLE  Handle,
> +  IN VOID*Instance,
> +  IN VOID*Context
> +  )
> +{
> +  VIRTIO_DEVICE_PROTOCOL  *VirtIo = (VIRTIO_DEVICE_PROTOCOL *)Instance;
> +
> +  DEBUG ((DEBUG_INFO, "%a:%d: id %d\n", __func__, __LINE__, 
> VirtIo->SubSystemDeviceId));
> +
> +  switch (VirtIo->SubSystemDeviceId) {
> +case 3:
> +  PrepareVirtioSerialDevicePath (Handle);
> +  break;
> +default:
> +  /* nothing */
> +  break;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>Connect the predefined platform default console device.
>
> @@ -1256,6 +1281,12 @@ PlatformInitializeConsole (
>//
>VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);
>
> +  VisitAllInstancesOfProtocol (
> +,
> +DetectAndPreparePlatformVirtioDevicePath,
> +NULL
> +);
> +
>PrepareMicrovmDevicePath ();
>
>//
> --
> 2.41.0
>


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

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/README: Document Secure Boot

[Ard Biesheuvel]

On Tue, 4 Jul 2023 at 09:28, Gerd Hoffmann  wrote:
>
> On Fri, Jun 30, 2023 at 02:26:03AM +0400, Joursoir wrote:
> > Add the new section for Secure Boot.
> >
> > Signed-off-by: Alexander Goncharov 
>
> Acked-by: Gerd Hoffmann 
>

Thanks all

Queued as #4800


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

[edk2-devel] Event: TianoCore edk2-test Bug Triage Meeting - Thursday, September 7, 2023 #cal-reminder

[Group Notification]

*Reminder: TianoCore edk2-test Bug Triage Meeting*

*When:*
Thursday, September 7, 2023
10:00pm to 11:00pm
(UTC+08:00) Asia/Shanghai

*Where:*
https://armltd.zoom.us/j/91247522013?pwd=ei9nUndTbG9oWEROS2M1aVREZkpiQT09=addon

*Organizer:* Edhaya Chandran edhaya.chand...@arm.com ( 
edhaya.chand...@arm.com?subject=Re:%20Event:%20TianoCore%20edk2-test%20Bug%20Triage%20Meeting
 )

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

*Description:*


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

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info

[Oliver Steffen]

Quoting Yao, Jiewen (2023-09-07 14:26:31)
> I don't think using ASSERT is a good idea here.
>

Some context:

We observed that EDK2 hits an ASSERT (Out of Resources) when booting
with a full variable store. The message provided in this case is not
helpful for non-experts.
I think the goal here is to emit a more user friendly message.

My suggestion is to wrap the newly added DEBUG statement with an if:

   if (OptionIndex == -1) {
 Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
+if (Status == EFI_OUT_OF_RESOURCES) {
+  DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
+}
 ASSERT_EFI_ERROR (Status);
   }

The ASSERT_EFI_ERROR was already there. It will halt the execution in
case Status contains an error. Aparently the firmware is not able to
continue with a full variable store.

> Why not return ERROR?
>

This is in VOID PlatformRegisterFvBootOption (), returning ERROR is not
possible.

Thanks,
 Oliver

>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Zhenyu
> > Zhang
> > Sent: Thursday, September 7, 2023 7:11 PM
> > To: devel@edk2.groups.io
> > Cc: zheny...@redhat.com; ostef...@redhat.com; kra...@redhat.com;
> > marcandre.lur...@redhat.com; stef...@linux.ibm.com;
> > anthony.per...@citrix.com; jul...@xen.org
> > Subject: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full 
> > debug
> > info
> >
> > From: "Zhenyu Zhang" 
> >
> > When the variable store is full, edk2 will directly assert.
> > Add debug information to help users understand what caused
> > the assertion.
> >
> >  Actual results:
> >  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
> >  48BCD90AD31A - 0x0003 - 0x7E
> >  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize =
> >  0x3FF98
> >  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
> >  48BCD90AD31A - 0x0003 - 0x92
> >  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = 0x3FF98
> >
> >  Synchronous Exception at 0x00023CA60374
> >  ..
> >  ASSERT_EFI_ERROR (Status = Out of Resources)
> >  ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/
> >  PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_
> >  STATUS)(Status)) < 0)
> >
> > Cc: Oliver Steffen 
> > Cc: Gerd Hoffmann 
> > Cc: Marc-André Lureau 
> > Cc: Stefan Berger 
> > Cc: Anthony Perard 
> > Cc: Julien Grall 
> > Signed-off-by: Zhenyu Zhang 
> > ---
> >  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > index 8dc2bbf97371..c95c7931a3f5 100644
> > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > @@ -139,6 +139,7 @@ PlatformRegisterFvBootOption (
> >
> >if (OptionIndex == -1) {
> >  Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> > +DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
> >  ASSERT_EFI_ERROR (Status);
> >}
> >
> > --
> > 2.39.3
> >



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

Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Laszlo Ersek]

(replying on the webui... sorry!)

The problem is actually embedded in MdePkg and MdeModulePkg.

- In DxeMain() (and in functions called by DxeMain()), we call DebugLib APIs 
*before* reaching ProcessLibraryConstructorList().

- In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to 
BaseDebugLibSerialPort (from MdePkg).

- BaseDebugLibSerialPort has a constructor function 
(BaseDebugLibSerialPortConstructor()).

These already suffice for broken DebugLib behavior. APIs of a library class 
should not be called because the library instance has a chance to initialize.

The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor calls 
SerialPortInitialize, but our SerialPortInitialize (in FdtPL011SerialPortLib) 
does nothing. Well, the latter doesn't need to do anything, because 
FdtPL011SerialPortLib has its own constructor 
(FdtPL011SerialPortLibInitialize), thus, if constructors were called properly, 
then BaseDebugLibSerialPort + FdtPL011SerialPortLib would work properly 
together, regardless of SerialPortInitialize being empty in the latter.

Basically the DXE Core has a hidden requirement -- it can only use such 
DebugLib instances that need no explicit initialization.

The proposed patch works around the problem by satisfying that hidden 
requirement one level lower down: in the SerialPortLib instance. The 
initialization of BaseDebugLibSerialPort is still busted (its constructor is 
not called, so it cannot call SerialPortInitialize either), but now it is 
masked, because EarlyFdtPL011SerialPortLib works withouth *both* 
SerialPortInitialize and construction.

The real fix would be to make the DXE Core requirement explicit, by introducing 
separate (dedicated) DebugLib and SerialPortLib *classes* (whose APIs are 
guaranteed to work without initialization).

Laszlo


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

[edk2-devel] [PATCH v3 7/8] Sophgo/SG2042Pkg: Add SG2042Pkg.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

Add SG2042Pkg for Sophgo SG2042 platform. Provides PCD tokens.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec  | 35 +
 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni  | 13 
 Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni | 12 +++
 3 files changed, 60 insertions(+)
 create mode 100644 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
 create mode 100644 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni
 create mode 100644 Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni

diff --git a/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec 
b/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
new file mode 100644
index ..ed242de3651d
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
@@ -0,0 +1,35 @@
+## @file  SG2042Pkg.dec
+# This Package provides modules and libraries.for Sophgo SG2042 platform.
+#
+# Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  DEC_SPECIFICATION  = 0x0001001b
+  PACKAGE_NAME   = SG2042Pkg
+  PACKAGE_UNI_FILE   = SG2042Pkg.uni
+  PACKAGE_GUID   = A10E7DF0-B8AB-4DD0-B383-46358139D313
+  PACKAGE_VERSION= 1.0
+
+[Includes]
+  Include
+
+[Protocols]
+  gSophgoMmcHostProtocolGuid = { 0x3E591C00, 0x9E4A, 0x11DF, {0x92, 0x44, 
0x00, 0x02, 0xA5, 0xF5, 0xF5, 0x1B } }
+
+[Guids]
+  gSophgoSG2042PlatformPkgTokenSpaceGuid  = {0x779E9346, 0x3C24, 0x478C, { 
0xB1, 0x60, 0xB6, 0x09, 0xFC, 0xED, 0xA0, 0x72 }}
+
+[PcdsFixedAtBuild]
+  
gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdSG2042SDIOBase|0x0|UINT64|0x1001
+  gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdForceNoMMU|0x0|BOOLEAN|0x1002
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x0|UINT64|0x1004
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|0x0|UINT64|0x1005
+  gHisiTokenSpaceGuid.PcdSerialPortSendDelay|0x0|UINT32|0x1006
+  gHisiTokenSpaceGuid.PcdUartClkInHz|0x0|UINT32|0x1007
+
+[UserExtensions.TianoCore."ExtraFiles"]
+  SG2042PkgExtra.uni
diff --git a/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni 
b/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni
new file mode 100644
index ..c7a606cbb2f3
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni
@@ -0,0 +1,13 @@
+// /** @file
+// Sophgo SG2042 Package Localized Strings and Content.
+//
+// Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_PACKAGE_ABSTRACT#language en-US "Provides Sophgo 
RISC-V SG2042 platform modules and libraries"
+
+#string STR_PACKAGE_DESCRIPTION #language en-US "This Package Sophgo 
RISC-V SG2042 platform modules and libraries."
diff --git a/Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni 
b/Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni
new file mode 100644
index ..b14545b214eb
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni
@@ -0,0 +1,12 @@
+// /** @file
+// SiFive U5 Series Package Localized Strings and Content.
+//
+// Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All rights 
reserved.
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_PROPERTIES_PACKAGE_NAME
+#language en-US
+"SG2042 platform package"
-- 
2.34.1



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

[edk2-devel] [PATCH v3 1/8] Sophgo/SG2042Pkg: Add SmbiosPlatformDxe module.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

This driver installs SMBIOS information for SG2042. Install hardware
information by creating an SMBIOS table which includes BIOS version,
system manufacturer, product name, processor, memory, slots, storage,
and other.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |  39 +
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 805 ++
 2 files changed, 844 insertions(+)
 create mode 100644 
Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
 create mode 100644 
Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c

diff --git 
a/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf 
b/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
new file mode 100644
index ..61319d092a46
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -0,0 +1,39 @@
+#/** @file
+#  SMBIOS Table for RISC-V Sophgo SG2042 platform
+#
+#  Copyright (c) 2013, Linaro Ltd. All rights reserved.
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION= 0x0001001B
+  BASE_NAME  = SmbiosPlatformDxe
+  FILE_GUID  = 1CAFAAC3-C386-BF0B-7DD1-7EEE514A91B1
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= SmbiosPlatformDriverEntryPoint
+
+[Sources]
+  SmbiosPlatformDxe.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  HobLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiSmbiosProtocolGuid  # PROTOCOL ALWAYS_CONSUMED
+
+[Depex]
+  gEfiSmbiosProtocolGuid
diff --git 
a/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c 
b/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
new file mode 100644
index ..5129dc236f66
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -0,0 +1,805 @@
+/** @file
+  This driver installs SMBIOS information for Sophgo SG2042EVB platforms
+
+  Copyright (c) 2015-2020, Arm Limited. All rights reserved.
+  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+//
+// SMBIOS tables often reference each other using
+// fixed constants, define a list of these constants
+// for our hardcoded tables
+//
+enum SMBIOS_REFRENCE_HANDLES {
+  SMBIOS_HANDLE_L1I = 0x1000,
+  SMBIOS_HANDLE_L1D,
+  SMBIOS_HANDLE_L2,
+  SMBIOS_HANDLE_L3,
+  SMBIOS_HANDLE_MOTHERBOARD,
+  SMBIOS_HANDLE_CHASSIS,
+  SMBIOS_HANDLE_CLUSTER,
+  SMBIOS_HANDLE_MEMORY,
+  SMBIOS_HANDLE_DIMM
+};
+
+//
+// Type definition and contents of the default SMBIOS table.
+// This table covers only the minimum structures required by
+// the SMBIOS specification (section 6.2, version 3.0)
+//
+
+// BIOS information (section 7.1)
+STATIC SMBIOS_TABLE_TYPE0 mSG2042EVBType0 = {
+  { // SMBIOS_STRUCTURE Hdr
+EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
+sizeof (SMBIOS_TABLE_TYPE0),  // UINT8 Length
+SMBIOS_HANDLE_PI_RESERVED,
+  },
+  1,  // SMBIOS_TABLE_STRING   Vendor
+  2,  // SMBIOS_TABLE_STRING   BiosVersion
+  0xE800, // UINT16BiosSegment
+  3,  // SMBIOS_TABLE_STRING   
BiosReleaseDate
+  0,  // UINT8 BiosSize
+  {   // BiosCharacteristics
+0,// Reserved  :2
+0,// Unknown   :1
+0,// BiosCharacteristicsNotSupported   :1
+0,// IsaIsSupported:1
+0,// McaIsSupported:1
+0,// EisaIsSupported   :1
+1,// PciIsSupported:1
+0,// PcmciaIsSupported :1
+0,// PlugAndPlayIsSupported:1
+0,// ApmIsSupported:1
+1,

[edk2-devel] [PATCH v3 4/8] Sophgo/SG2042Pkg: Add base MMC driver.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

This driver implements the MMC Host protocol, which is used by SD
interface driver that the Sophgo SG2042 EVB supports. Add this driver
in Sophgo/SG2042Pkg leveraging the one form Embedded Package.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 .../SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf   |  46 ++
 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h | 513 +
 Silicon/Sophgo/SG2042Pkg/Include/MmcHost.h| 225 ++
 .../SG2042Pkg/Drivers/MmcDxe/ComponentName.c  | 156 
 .../SG2042Pkg/Drivers/MmcDxe/Diagnostics.c| 323 
 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.c | 527 +
 .../SG2042Pkg/Drivers/MmcDxe/MmcBlockIo.c | 643 
 .../SG2042Pkg/Drivers/MmcDxe/MmcDebug.c   | 194 +
 .../Drivers/MmcDxe/MmcIdentification.c| 719 ++
 9 files changed, 3346 insertions(+)
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Include/MmcHost.h
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/ComponentName.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Diagnostics.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcBlockIo.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcDebug.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcIdentification.c

diff --git a/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf 
b/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf
new file mode 100644
index ..0565fd229ad1
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf
@@ -0,0 +1,46 @@
+## @file
+#  Component description file for the MMC DXE driver module.
+#
+#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = MmcDxe
+  FILE_GUID  = B5A53998-42AD-4C66-8D2D-1C5FBD175F25
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= MmcDxeInitialize
+
+[Sources.common]
+  ComponentName.c
+  Mmc.h
+  Mmc.c
+  MmcBlockIo.c
+  MmcIdentification.c
+  MmcDebug.c
+  Diagnostics.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
+
+[LibraryClasses]
+  BaseLib
+  UefiLib
+  UefiDriverEntryPoint
+  BaseMemoryLib
+
+[Protocols]
+  gEfiDiskIoProtocolGuid## CONSUMES
+  gEfiBlockIoProtocolGuid   ## PRODUCES
+  gEfiDevicePathProtocolGuid## PRODUCES
+  gEfiDriverDiagnostics2ProtocolGuid## SOMETIMES_PRODUCES
+  gSophgoMmcHostProtocolGuid## CONSUMES
+
+[Depex]
+  TRUE
diff --git a/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h 
b/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h
new file mode 100644
index ..6ac59baa82ef
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h
@@ -0,0 +1,513 @@
+/** @file
+  Main Header file for the MMC DXE driver
+
+  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __MMC_H
+#define __MMC_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define BIT_32(nr)  (1U << (nr))
+#define BIT_64(nr)  (1ULL << (nr))
+#define UINT64_C(c) (c ## UL)
+#define GENMASK_64(h,l) (((~UINT64_C(0)) << (l)) & (~UINT64_C(0) >> (64 - 
1 - (h
+#define GENMASK(h,l)GENMASK_64(h,l)
+
+#define MMC_TRACE(txt)  DEBUG((DEBUG_BLKIO, "MMC: " txt "\n"))
+
+#define MMC_IOBLOCKS_READ   0
+#define MMC_IOBLOCKS_WRITE  1
+
+/* Value randomly chosen for eMMC RCA, it should be > 1 */
+#define MMC_FIX_RCA 6
+#define RCA_SHIFT_OFFSET16
+
+#define MMC_OCR_POWERUPBIT31
+#define MMC_OCR_ACCESS_MASK0x3 /* bit[30-29] */
+#define MMC_OCR_ACCESS_BYTE0x1 /* bit[29] */
+#define MMC_OCR_ACCESS_SECTOR  0x2 /* bit[30] */
+#define OCR_HCSBIT30
+#define OCR_BYTE_MODE  (0U << 29)
+#define OCR_SECTOR_MODE(2U << 29)
+#define OCR_ACCESS_MODE_MASK   (3U << 29)
+#define OCR_VDD_MIN_2V7GENMASK(23, 15)
+#define OCR_VDD_MIN_2V0GENMASK(14, 8)
+#define OCR_VDD_MIN_1V7BIT7
+
+/* Value randomly chosen for eMMC RCA, it should be > 1 */
+#define MMC_FIX_RCA  6
+#define 

Re: [edk2-devel] [PATCH v2 0/8] EDK2 on RISC-V Sophgo SG2042 platform

[caiyuqing_hz]

Hi, Leif
Thanks for your comments. We have conducted some testing and completed the 
third version of the patch.

1) Blurb
We have added some descriptions in Blurb to explain the current status, testing 
situation, and limitations of the project.

2) Milk-V Pioneer board
Our team does not have a Milk V Pioneer board. But we asked the help of other 
cooperative teams to test it, the test results indicate that EVB and Pioneer 
boards are not fully compatible. The SD card driver cannot correctly recognize 
all partitions and start the Linux operating system. Sorry, our team has no 
plans to add support for Pioneer board.

3) Layout
Based on your comments, we have modified the code layout of the port and moved 
most of the
Platform/Sophgo/SG2042Pkg/
to Silicon/Sophgo/SG2042Pkg/
The specific changes can be found in the submitted third version of the patch 
or in the link of the public git tree provided in Blurb.

4) Clang toolchain support
I‘m trying to use the CLANGDWARF toolchain (clang version 18.0.0) to build 
ports. It is able to build successfully but the compiled binary was not fully 
work. It's strange that the SD card driver got stuck in a loop while reading a 
data block. Adding multiple lines of printed output to the read data block 
function of the SD card can run normally and successfully start the Linux OS.

Thanks,
Yuqing Cai


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

[edk2-devel] [PATCH v3 6/8] Sophgo/SG2042_EVB_Board: Add Sophgo SG2042 platform.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

Add infrastructure files to build edk2-platforms for Sophgo SG2042
platform. It follows PEI less design.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 Platform/Sophgo/SG2042_EVB_Board/SG2042.dec   |  19 +
 Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc   | 548 ++
 Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf   | 248 
 .../Sophgo/SG2042_EVB_Board/SG2042.fdf.inc|  62 ++
 .../Sophgo/SG2042_EVB_Board/VarStore.fdf.inc  |  77 +++
 5 files changed, 954 insertions(+)
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.dec
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf.inc
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/VarStore.fdf.inc

diff --git a/Platform/Sophgo/SG2042_EVB_Board/SG2042.dec 
b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dec
new file mode 100644
index ..b0ea250a997e
--- /dev/null
+++ b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dec
@@ -0,0 +1,19 @@
+## @file  SG2042.dec
+# This Package provides Sophgo SG2042 modules and libraries.
+#
+# Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  DEC_SPECIFICATION  = 0x0001001b
+  PACKAGE_NAME   = SG2042
+  PACKAGE_GUID   = BBF86176-C58B-4EC5-8D76-B8D458A2548E
+  PACKAGE_VERSION= 1.0
+
+[LibraryClasses]
+
+[Guids]
+  gUefiRiscVPlatformSG2042PkgTokenSpaceGuid = {0xD0D80952, 0x8371, 0x4D8D, { 
0x8E, 0x65, 0x27, 0x52, 0x63, 0xF3, 0xD9, 0x27 }}
diff --git a/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc 
b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc
new file mode 100644
index ..34f3a4f3576b
--- /dev/null
+++ b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc
@@ -0,0 +1,548 @@
+## @file
+#  RISC-V EFI on Sophgo SG2042 EVB RISC-V platform
+#
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = SG2042_EVB
+  PLATFORM_GUID  = 8014637B-6999-4110-9762-464BE11E935F
+  PLATFORM_VERSION   = 0.1
+  DSC_SPECIFICATION  = 0x0001001c
+  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
+  SUPPORTED_ARCHITECTURES= RISCV64
+  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf
+
+  #
+  # Enable below options may cause build error or may not work on
+  # the initial version of RISC-V package
+  # Defines for default states.  These can be changed on the command line.
+  # -D FLAG=VALUE
+  #
+  DEFINE SECURE_BOOT_ENABLE  = FALSE
+  DEFINE DEBUG_ON_SERIAL_PORT= TRUE
+
+  #
+  # Network definition
+  #
+  DEFINE NETWORK_SNP_ENABLE   = FALSE
+  DEFINE NETWORK_IP6_ENABLE   = FALSE
+  DEFINE NETWORK_TLS_ENABLE   = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE
+  DEFINE NETWORK_ISCSI_ENABLE = FALSE
+
+[BuildOptions]
+  GCC:RELEASE_*_*_CC_FLAGS   = -DMDEPKG_NDEBUG
+!ifdef $(SOURCE_DEBUG_ENABLE)
+  GCC:*_*_RISCV64_GENFW_FLAGS= --keepexceptiontable
+!endif
+
+
+#
+# SKU Identification section - list of all SKU IDs supported by this Platform.
+#
+
+[SkuIds]
+  0|DEFAULT
+
+
+#
+# Library Class section - list of all Library Classes needed by this Platform.
+#
+
+
+!include MdePkg/MdeLibs.dsc.inc
+
+[LibraryClasses]
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+  
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
+  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  
PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  

[edk2-devel] [PATCH v3 3/8] Sophgo/SG2042Pkg: Add Sophgo SDHCI driver.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

This driver implements Sophgo SDHCI controller, which provides
the necessary interfaces for handling communication and data
transfer with SD cards.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 .../SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf |  47 +
 .../SG2042Pkg/Drivers/SdHostDxe/SdHci.h   | 309 ++
 .../SG2042Pkg/Drivers/SdHostDxe/SdHci.c   | 929 ++
 .../SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.c   | 450 +
 4 files changed, 1735 insertions(+)
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHci.h
 create mode 100755 Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHci.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.c

diff --git a/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf 
b/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf
new file mode 100644
index ..f4f51d8fde74
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf
@@ -0,0 +1,47 @@
+## @file
+#  Component description file for the SD Host Controller DXE driver module.
+#
+#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2017, Andrei Warkentin 
+#  Copyright (c) Microsoft Corporation. All rights reserved.
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = SdHostDxe
+  FILE_GUID  = 11322596-DD4F-47FA-9E6C-CE787E11E4B1
+  MODULE_TYPE= UEFI_DRIVER
+  VERSION_STRING = 1.0
+  ENTRY_POINT= SdHostInitialize
+
+[Sources]
+  SdHci.c
+  SdHci.h
+  SdHostDxe.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  IoLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+  UefiRuntimeServicesTableLib
+
+[Protocols]
+  gSophgoMmcHostProtocolGuid## PRODUCES
+
+[FixedPcd]
+  gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdSG2042SDIOBase## CONSUMES
+  gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdForceNoMMU## CONSUMES
diff --git a/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHci.h 
b/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHci.h
new file mode 100644
index ..d9a9c88674e6
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHci.h
@@ -0,0 +1,309 @@
+/** @file
+  The header file that provides definitions and function declarations
+  related to the SD Host Controller Interface (SDHCI) for SD card host 
controllers.
+
+  Copyright (c) 2016-2017, ARM Limited and Contributors. All rights reserved.
+  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+  SPDX-License-Identifier: BSD-3-Clause
+
+**/
+
+#ifndef _SD_HCI_H_
+#define _SD_HCI_H_
+
+#define SDIO_BASE   (FixedPcdGet64(PcdSG2042SDIOBase))
+#define SDHCI_DMA_ADDRESS   0x00
+#define SDHCI_BLOCK_SIZE0x04
+#define SDHCI_MAKE_BLKSZ(dma, blksz)dma) & 0x7) << 12) | ((blksz) & 
0xFFF))
+#define SDHCI_BLOCK_COUNT   0x06
+#define SDHCI_ARGUMENT  0x08
+#define SDHCI_TRANSFER_MODE 0x0C
+#define SDHCI_TRNS_DMA  BIT0
+#define SDHCI_TRNS_BLK_CNT_EN   BIT1
+#define SDHCI_TRNS_ACMD12   BIT2
+#define SDHCI_TRNS_READ BIT4
+#define SDHCI_TRNS_MULTIBIT5
+#define SDHCI_TRNS_RESP_INT BIT8
+#define SDHCI_COMMAND   0x0E
+#define SDHCI_CMD_RESP_MASK 0x03
+#define SDHCI_CMD_CRC   0x08
+#define SDHCI_CMD_INDEX 0x10
+#define SDHCI_CMD_DATA  0x20
+#define SDHCI_CMD_ABORTCMD  0xC0
+#define SDHCI_CMD_RESP_NONE 0x00
+#define SDHCI_CMD_RESP_LONG 0x01
+#define SDHCI_CMD_RESP_SHORT0x02
+#define SDHCI_CMD_RESP_SHORT_BUSY   0x03
+#define SDHCI_MAKE_CMD(c, f)c) & 0xff) << 8) | ((f) & 0xff))
+#define SDHCI_RESPONSE_01   0x10
+#define SDHCI_RESPONSE_23   0x14
+#define SDHCI_RESPONSE_45   0x18
+#define SDHCI_RESPONSE_67   0x1C
+#define SDHCI_PSTATE0x24
+#define SDHCI_CMD_INHIBIT   BIT0
+#define SDHCI_CMD_INHIBIT_DAT   BIT1
+#define SDHCI_BUF_WR_ENABLE BIT10
+#define SDHCI_BUF_RD_ENABLE BIT11
+#define SDHCI_CARD_INSERTED BIT16
+#define SDHCI_HOST_CONTROL  0x28

[edk2-devel] [PATCH v3 5/8] Sophgo/SG2042Pkg: Add SEC module.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

This module supports Sophgo SG2042 EVB platform. It uses the
PEI less design. Add this module in SG2042Pkg leveraging the
one from OvmfPkg/RiscVVirt.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf |  68 +
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h   | 104 +++
 Silicon/Sophgo/SG2042Pkg/Sec/Cpu.c   |  29 ++
 Silicon/Sophgo/SG2042Pkg/Sec/Memory.c| 347 +++
 Silicon/Sophgo/SG2042Pkg/Sec/Platform.c  | 130 +
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.c   | 115 
 Silicon/Sophgo/SG2042Pkg/Sec/SecEntry.S  |  18 ++
 7 files changed, 811 insertions(+)
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/Cpu.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/Memory.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/Platform.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.c
 create mode 100644 Silicon/Sophgo/SG2042Pkg/Sec/SecEntry.S

diff --git a/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf 
b/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf
new file mode 100644
index ..3b4d6d6b86bc
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf
@@ -0,0 +1,68 @@
+## @file
+#  SEC Driver for RISC-V
+#
+#  Copyright (c) 2022, Ventana Micro Systems Inc. All rights reserved.
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001B
+  BASE_NAME  = SecMainRiscV64
+  FILE_GUID  = 125E1236-9D4F-457B-BF7E-6311C88A1621
+  MODULE_TYPE= SEC
+  VERSION_STRING = 1.0
+  ENTRY_POINT= SecMain
+
+#
+# The following information is for reference only and not required by the 
build tools.
+#
+#  VALID_ARCHITECTURES   = RISCV64
+#
+
+[Sources]
+  SecEntry.S
+  SecMain.c
+  SecMain.h
+  Cpu.c
+  Memory.c
+  Platform.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+  Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
+  Platform/RISC-V/PlatformPkg/RiscVPlatformPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  IoLib
+  PeCoffLib
+  LzmaDecompressLib
+  RiscVSbiLib
+  PrePiLib
+  FdtLib
+  MemoryAllocationLib
+  HobLib
+  SerialPortLib
+
+[FixedPcd]
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVDxeFvBase
 ## CONSUMES
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdRiscVDxeFvSize
 ## CONSUMES
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdVariableFirmwareRegionBaseAddress 
 ## CONSUMES
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdVariableFirmwareRegionSize
 ## CONSUMES
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdTemporaryRamBase  
 ## CONSUMES
+  gUefiRiscVPlatformPkgTokenSpaceGuid.PcdTemporaryRamSize  
 ## CONSUMES
+
+[Guids]
+  gFdtHobGuid  ## PRODUCES
+
+[BuildOptions]
+  GCC:*_*_*_PP_FLAGS = -D__ASSEMBLY__
+
diff --git a/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h 
b/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h
new file mode 100644
index ..9d615e9fa6a1
--- /dev/null
+++ b/Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h
@@ -0,0 +1,104 @@
+/** @file
+  Master header file for SecCore.
+
+  Copyright (c) 2022, Ventana Micro Systems Inc. All rights reserved.
+  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SEC_MAIN_H_
+#define SEC_MAIN_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Entry point to the C language phase of SEC. After the SEC assembly
+  code has initialized some temporary memory and set up the stack,
+  the control is transferred to this function.
+
+  @param SizeOfRam   Size of the temporary memory available for use.
+  @param TempRamBase Base address of temporary ram
+  @param BootFirmwareVolume  Base address of the Boot Firmware Volume.
+**/
+VOID
+NORETURN
+EFIAPI
+SecStartup (
+  IN  UINTN  BootHartId,
+  IN  VOID   *DeviceTreeAddress
+  );
+
+/**
+  Auto-generated function that calls the library constructors for all of the 
module's
+  dependent libraries.  This function must be called by the SEC Core once a 
stack has
+  been established.
+
+**/
+VOID
+EFIAPI
+ProcessLibraryConstructorList (
+  VOID
+  );
+
+/**
+  Perform Platform PEIM initialization.
+
+  @return 

[edk2-devel] [PATCH v3 0/8] EDK2 on RISC-V Sophgo SG2042 platform

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

Description:
  Deploy EDK2 to run on 64-core CPU under RISC-V architecture, and successfully 
boot to OS.
  Implementation can be found on 
https://github.com/AII-SDU/edk2-platforms/tree/devel-Sophgo/SG2042Pkg/Platform/Sophgo.

Current progress and status:
  1.Adopted the scheme of separating OpenSBI and EDK2. It follows PEI less 
design.
  2.The startup process is roughly: ZSBL + FSBL + OpenSBI + EDK2 + GRUB + Linux 
OS.
The boot medium is SD card, where ZSBL is loaded and executed by an 
auxiliary MCU, performing initial initialization
operations such as DIMM initialization. FSBL then continues with further 
initialization tasks. OpenSBI is used to
initialize the hardware platform and kernel. Additionally, we have upgraded 
the OpenSBI from version V0.9 to V1.2 to
align with community progress. EDK2, running in S-mode, handles 
configuration operations, primarily completing the
SD card driver in the DXE stage, while PCIe driver testing and follow-up 
work are planned. GRUB2 is used to load the
operating system, facilitating subsequent operations.

Testing:
  1.Test the project on Sophgo SG2042 EVB and be able to launch Linux OS
  2.Test the project on Milk V Pioneer board and is able to boot into the UEFI 
shell.
However, the SD card driver could not correctly recognize all partitions 
and start Linux OS.

Current limitation:
  1.PCIE driver is not currently supported.
  2.MMU support
SG2042 (Xuantie C920) MMU can be enabled in SV39 mode. Introduce the PCD 
variable
PcdForceNoMMU to disable MMU configuration. Currently, enabling MMU results 
in a
timeout for reading data blocks from the SD card, So MMU is disabled by 
default.
  3.Clang toolchain support
Build the port using the CLANGDWARF toolchain (clang version 18.0.0). It is 
able
to build successfully but the compiled binary was not fully work.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 

caiyuqing379 (8):
  Sophgo/SG2042Pkg: Add SmbiosPlatformDxe module.
  Sophgo/SG2042Pkg: Add PlatformUpdateMmuDxe module.
  Sophgo/SG2042Pkg: Add Sophgo SDHCI driver.
  Sophgo/SG2042Pkg: Add base MMC driver.
  Sophgo/SG2042Pkg: Add SEC module.
  Sophgo/SG2042_EVB_Board: Add Sophgo SG2042 platform.
  Sophgo/SG2042Pkg: Add SG2042Pkg.
  Sophgo/SG2042Pkg: Add platform readme and document.

 Platform/Sophgo/SG2042_EVB_Board/SG2042.dec   |  19 +
 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec|  35 +
 Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc   | 548 +++
 Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf   | 248 +
 .../SG2042Pkg/Drivers/MmcDxe/MmcDxe.inf   |  46 +
 .../PlatformUpdateMmuDxe.inf  |  34 +
 .../SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf |  47 +
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |  39 +
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.inf  |  68 ++
 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.h | 513 ++
 .../SG2042Pkg/Drivers/SdHostDxe/SdHci.h   | 309 ++
 Silicon/Sophgo/SG2042Pkg/Include/MmcHost.h| 225 +
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.h| 104 ++
 .../SG2042Pkg/Drivers/MmcDxe/ComponentName.c  | 156 +++
 .../SG2042Pkg/Drivers/MmcDxe/Diagnostics.c| 323 ++
 Silicon/Sophgo/SG2042Pkg/Drivers/MmcDxe/Mmc.c | 527 ++
 .../SG2042Pkg/Drivers/MmcDxe/MmcBlockIo.c | 643 
 .../SG2042Pkg/Drivers/MmcDxe/MmcDebug.c   | 194 
 .../Drivers/MmcDxe/MmcIdentification.c| 719 ++
 .../PlatformUpdateMmuDxe.c| 593 +++
 .../SG2042Pkg/Drivers/SdHostDxe/SdHci.c   | 929 ++
 .../SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.c   | 450 +
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 805 +++
 Silicon/Sophgo/SG2042Pkg/Sec/Cpu.c|  29 +
 Silicon/Sophgo/SG2042Pkg/Sec/Memory.c | 347 +++
 Silicon/Sophgo/SG2042Pkg/Sec/Platform.c   | 130 +++
 Silicon/Sophgo/SG2042Pkg/Sec/SecMain.c| 115 +++
 Platform/Sophgo/About_Sophgo_platform.md  |  39 +
 .../Documents/Media/EDK2_SDU_Programme.png| Bin 0 -> 59830 bytes
 .../Sophgo/Documents/Media/SG2042_CPU.png | Bin 0 -> 806062 bytes
 .../Documents/Media/Sophgo_SG2042_EVB.png | Bin 0 -> 1445528 bytes
 Platform/Sophgo/Maintainers.md| 107 ++
 Platform/Sophgo/SG2042_EVB_Board/Readme.md| 100 ++
 .../Sophgo/SG2042_EVB_Board/SG2042.fdf.inc|  62 ++
 .../Sophgo/SG2042_EVB_Board/VarStore.fdf.inc  |  77 ++
 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.uni|  13 +
 Silicon/Sophgo/SG2042Pkg/SG2042PkgExtra.uni   |  12 +
 Silicon/Sophgo/SG2042Pkg/Sec/SecEntry.S   |  18 +
 38 files changed, 8623 insertions(+)
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.dec
 create mode 100644 Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
 create mode 100644 Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc
 create 

[edk2-devel] [PATCH v3 2/8] Sophgo/SG2042Pkg: Add PlatformUpdateMmuDxe module.

[caiyuqing_hz]

From: caiyuqing379 <202235...@mail.sdu.edu.cn>

SG2042 (Xuantie C920) MMU can be enabled in SV39 mode. C920 has five
customizable page properties that control whether the page is Strong
order(SO), Cacheable(C), Bufferable(B), Shareable(SH), Trustable(Sec).
This driver modifies the page table attributes to avoid exceptions
based on the memory attributes of the C920.

Introduces a PCD variable PcdForceNoMMU to disable MMU configuration.
Currently, enabling MMU results in a timeout for reading data blocks
from the SD card.

Signed-off-by: caiyuqing379 <202235...@mail.sdu.edu.cn>
Co-authored-by: USER0FISH 
Cc: dahogn 
Cc: meng-cz 
Cc: yli147 
Cc: ChaiEvan 
Cc: Sunil V L 
Cc: Leif Lindholm 
---
 .../PlatformUpdateMmuDxe.inf  |  34 +
 .../PlatformUpdateMmuDxe.c| 593 ++
 2 files changed, 627 insertions(+)
 create mode 100644 
Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.inf
 create mode 100644 
Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.c

diff --git 
a/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.inf
 
b/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.inf
new file mode 100644
index ..359bfa08a6c4
--- /dev/null
+++ 
b/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.inf
@@ -0,0 +1,34 @@
+## @file
+#  This driver modifies the page table attribute based on the memory attribute 
of the C920.
+#
+#  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION = 0x0001001b
+  BASE_NAME   = PlatformUpdateMmuDxe
+  FILE_GUID   = 9d1dd27f-6d7f-427b-aec4-b62f6279c2f1
+  MODULE_TYPE = UEFI_DRIVER
+  VERSION_STRING  = 1.0
+  ENTRY_POINT = PlatformUpdateMmu
+
+[Sources]
+  PlatformUpdateMmuDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+  Silicon/Sophgo/SG2042Pkg/SG2042Pkg.dec
+
+[LibraryClasses]
+  BaseLib
+  UefiLib
+  UefiDriverEntryPoint
+  RiscVMmuLib
+  DxeServicesTableLib
+
+[FixedPcd]
+  gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdForceNoMMU## CONSUMES
diff --git 
a/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.c 
b/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.c
new file mode 100644
index ..5ef6e7a2af1f
--- /dev/null
+++ 
b/Silicon/Sophgo/SG2042Pkg/Drivers/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.c
@@ -0,0 +1,593 @@
+/** @file
+  This driver modifies the page table attribute based on the memory attribute 
of the C920.
+  C920 has five customizable page properties that control whether the page is 
Strong order,
+  Cacheable, Bufferable, Shareable,Trustable.
+
+  Copyright (c) 2011-2020, ARM Limited. All rights reserved.
+  Copyright (c) 2016, Linaro Limited. All rights reserved.
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2023, Ventana Micro Systems Inc. All Rights Reserved.
+  Copyright (c) 2023, Academy of Intelligent Innovation, Shandong Universiy, 
China.P.R. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RISCV_PG_V   BIT0
+#define RISCV_PG_R   BIT1
+#define RISCV_PG_W   BIT2
+#define RISCV_PG_X   BIT3
+#define RISCV_PG_G   BIT5
+#define RISCV_PG_A   BIT6
+#define RISCV_PG_D   BIT7
+#define RISCV_PG_SH  BIT60
+#define RISCV_PG_B   BIT61
+#define RISCV_PG_C   BIT62
+#define RISCV_PG_SO  BIT63
+#define PTE_ATTRIBUTES_MASK  0xE
+
+#define PTE_PPN_MASK  0x3FFC00ULL
+#define PTE_PPN_SHIFT 10
+#define RISCV_MMU_PAGE_SHIFT  12
+
+STATIC UINTN  mMaxRootTableLevel;
+STATIC UINTN  mBitPerLevel;
+STATIC UINTN  mTableEntryCount;
+
+/**
+  Determine if the MMU enabled or not.
+
+  @retval TRUE  The MMU already enabled.
+  @retval FALSE The MMU not enabled.
+
+**/
+STATIC
+VOID
+RiscVMmuDisabled (
+  VOID
+  )
+{
+  RiscVSetSupervisorAddressTranslationRegister (SATP_MODE_OFF << 
SATP64_MODE_SHIFT);
+}
+
+/**
+  Determine if the MMU enabled or not.
+
+  @retval TRUE  The MMU already enabled.
+  @retval FALSE The MMU not enabled.
+
+**/
+STATIC
+BOOLEAN
+RiscVMmuEnabled (
+  VOID
+  )
+{
+  return ((RiscVGetSupervisorAddressTranslationRegister () &
+   SATP64_MODE) != (SATP_MODE_OFF << SATP64_MODE_SHIFT));
+}
+
+/**
+  Retrieve the root translate table.
+
+  @return The root translate table.
+
+**/
+STATIC
+UINTN
+RiscVGetRootTranslateTable (
+  VOID
+  )
+{
+  return (RiscVGetSupervisorAddressTranslationRegister () & SATP64_PPN) <<
+ RISCV_MMU_PAGE_SHIFT;
+}
+
+/**
+  Determine if an entry is valid pte.
+
+  

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info

[Yao, Jiewen]

I don't think using ASSERT is a good idea here.

Why not return ERROR?



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhenyu
> Zhang
> Sent: Thursday, September 7, 2023 7:11 PM
> To: devel@edk2.groups.io
> Cc: zheny...@redhat.com; ostef...@redhat.com; kra...@redhat.com;
> marcandre.lur...@redhat.com; stef...@linux.ibm.com;
> anthony.per...@citrix.com; jul...@xen.org
> Subject: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug
> info
> 
> From: "Zhenyu Zhang" 
> 
> When the variable store is full, edk2 will directly assert.
> Add debug information to help users understand what caused
> the assertion.
> 
>  Actual results:
>  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
>  48BCD90AD31A - 0x0003 - 0x7E
>  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize =
>  0x3FF98
>  RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
>  48BCD90AD31A - 0x0003 - 0x92
>  CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = 0x3FF98
> 
>  Synchronous Exception at 0x00023CA60374
>  ..
>  ASSERT_EFI_ERROR (Status = Out of Resources)
>  ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/
>  PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_
>  STATUS)(Status)) < 0)
> 
> Cc: Oliver Steffen 
> Cc: Gerd Hoffmann 
> Cc: Marc-André Lureau 
> Cc: Stefan Berger 
> Cc: Anthony Perard 
> Cc: Julien Grall 
> Signed-off-by: Zhenyu Zhang 
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 8dc2bbf97371..c95c7931a3f5 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -139,6 +139,7 @@ PlatformRegisterFvBootOption (
> 
>if (OptionIndex == -1) {
>  Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> +DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
>  ASSERT_EFI_ERROR (Status);
>}
> 
> --
> 2.39.3
> 
> 
> 
> 
> 



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

[edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info

[Zhenyu Zhang]

From: "Zhenyu Zhang" 

When the variable store is full, edk2 will directly assert.
Add debug information to help users understand what caused
the assertion.

 Actual results:
 RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
 48BCD90AD31A - 0x0003 - 0x7E
 CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize =
 0x3FF98
 RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929-
 48BCD90AD31A - 0x0003 - 0x92
 CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = 0x3FF98

 Synchronous Exception at 0x00023CA60374
 ..
 ASSERT_EFI_ERROR (Status = Out of Resources)
 ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/
 PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_
 STATUS)(Status)) < 0)

Cc: Oliver Steffen 
Cc: Gerd Hoffmann   
Cc: Marc-André Lureau 
Cc: Stefan Berger 
Cc: Anthony Perard 
Cc: Julien Grall 
Signed-off-by: Zhenyu Zhang 
---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 8dc2bbf97371..c95c7931a3f5 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -139,6 +139,7 @@ PlatformRegisterFvBootOption (
 
   if (OptionIndex == -1) {
 Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
+DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
 ASSERT_EFI_ERROR (Status);
   }
 
-- 
2.39.3



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

Re: [edk2-devel] [PATCH v5] MdeModulePkg/HiiDatabase: Fix incorrect AllocateCopyPool size

[Ard Biesheuvel]

On Thu, 7 Sept 2023 at 05:35, Mike Beaton  wrote:
>
> The immediately preceding call, GetBestLanguage, plus the implementation of
> HiiGetString, which is called immediately afterwards, make it clear that
> BestLanguage is a null-terminated ASCII string, and not just a five byte,
> non-null terminated buffer.
>
> Therefore AsciiStrLen is one byte too short, meaning that whether the space
> allocated is really sufficient and whether the resultant string is really
> null-terminated becomes implementation-dependent. Rather than switching to
> AsciiStrSize, we use an explicitly compile-time string length calculation
> (both compile-time and run-time approaches are currently used elsewhere in
> the codebase for copying static strings).
>
> Signed-off-by: Mike Beaton 

Thanks for the fix.

Reviewed-by: Ard Biesheuvel 

> ---
>  MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c 
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> index 96e05d4cf9..6e791783a6 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
> @@ -1987,7 +1987,7 @@ GetNameFromId (
> NULL
> );
>if (BestLanguage == NULL) {
> -BestLanguage = AllocateCopyPool (AsciiStrLen ("en-US"), "en-US");
> +BestLanguage = AllocateCopyPool (sizeof ("en-US"), "en-US");
>  ASSERT (BestLanguage != NULL);
>}
>
> --
> 2.41.0
>
>
>
> 
>
>


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

Re: [edk2-devel][PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore

[Ard Biesheuvel]

On Wed, 6 Sept 2023 at 23:49, Oliver Smith-Denny
 wrote:
>
> Currently, ArmVirtPkg does not provide a serial library for DxeCore,
> so any early prints are missed. These prints are extremely valuable
> for debugging.
>
> The early serial port lib used by PeiCore and PEIMs is also
> applicable to DxeCore and in testing works to print debug prints
> from DxeCore throughout its lifecycle.
>
> This patchset adds the indicated support for DXE_CORE to
> EarlyFdtPL011SerialPortLib and adds this as the serial port
> instance for DxeCore in ArmVirtPkg.
>
> Github PR: https://github.com/tianocore/edk2/pull/4793
>
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Gerd Hoffmann 
>
> Signed-off-by: Oliver Smith-Denny 

Thanks for the patch. I agree that omitting early DXE core DEBUG
output is not great.

However, I'd like to understand a bit better why this happens.

We have the PlatformPeim that discovers the UART base address and
records it in a GUIDed HOB 'gEarlyPL011BaseAddressGuid'. So when DXE
core launches, we already have the information we need stored
somewhere, and using the 'early' flavor of the PL011 serialportlib
that parses the DT for every line (or character?) printed seems
unnecessary. Maybe it is a matter of tweaking the logic in
ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.c (for
DEBUG builds only) to take the HOB into account even before the
constructor has been called?



> ---
>  ArmVirtPkg/ArmVirt.dsc.inc  | 1 +
>  ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf | 2 
> +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 2443e8351c99..cf352619fd6e 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -225,6 +225,7 @@ [LibraryClasses.common.DXE_CORE]
>DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>
> ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
>
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> +  
> SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
>
>  [LibraryClasses.common.DXE_DRIVER]
>
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> diff --git 
> a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf 
> b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
> index 32b2d337d412..2c22ab088033 100644
> --- a/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
> +++ b/ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf
> @@ -14,7 +14,7 @@ [Defines]
>FILE_GUID  = 0983616A-49BC-4732-B531-4AF98D2056F0
>MODULE_TYPE= BASE
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = SerialPortLib|SEC PEI_CORE PEIM
> +  LIBRARY_CLASS  = SerialPortLib|SEC PEI_CORE PEIM DXE_CORE
>
>  [Sources.common]
>EarlyFdtPL011SerialPortLib.c
> --
> 2.40.1
>


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: don't exit early if RSDP is not found in memory

[Ard Biesheuvel]

On Thu, 7 Sept 2023 at 10:34, Corvin Köhne  wrote:
>
> If OVMF fails to find the RSDP in memory, it should fall back installing
> the statically provided ACPI tables.
>
> Signed-off-by: Corvin Köhne 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Rebecca Cran 
> Cc: Peter Grehan 

Nit: please cc the cover letter to the same group of people as the
actual patches.

Typically, I add the cc's to the cover letter only, and use --cc-cover
with git send-email.

That way, the cc's don't pollute the commit log either.

The patch looks fine to me but i'd like someone with a clue about
bhyve to review/ack it as well.

Thanks,
Ard.


> ---
>  OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c | 19 ---
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c 
> b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> index fb926a8bd803..57b1e7a99666 100644
> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> @@ -259,19 +259,17 @@ InstallAcpiTables (
>   BHYVE_BIOS_PHYSICAL_END,
>   
>   );
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -
> -  Status = InstallAcpiTablesFromRsdp (
> - AcpiTable,
> - Rsdp
> - );
>if (!EFI_ERROR (Status)) {
> -return EFI_SUCCESS;
> +Status = InstallAcpiTablesFromRsdp (
> +   AcpiTable,
> +   Rsdp
> +   );
> +if (!EFI_ERROR (Status)) {
> +  return EFI_SUCCESS;
> +}
>}
>
> -  if (Status != EFI_NOT_FOUND) {
> +  if (EFI_ERROR (Status)) {
>  DEBUG (
>(
> DEBUG_WARN,
> @@ -280,7 +278,6 @@ InstallAcpiTables (
> Status
>)
>);
> -return Status;
>}
>
>Status = InstallOvmfFvTables (AcpiTable);
> --
> 2.42.0
>
>
>
> 
>
>


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

Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version

[Lien, HoraceX]

Hi Mike,

Could you please reply for me?
If you want to filter range 0-9, then I will send PR again.

Thanks,
Horace Lien

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Lien, HoraceX
Sent: Friday, September 1, 2023 3:06 PM
To: Kinney, Michael D ; devel@edk2.groups.io
Cc: Liu, Zhiguang ; Bi, Dandan ; 
Zeng, Star ; Gao, Zhichao 
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is 
not match with SMBIOS version

Hi Mike,

I have change code to
EntryPointStructureData.SmbiosBcdRevision = ((mPrivateData.Smbios.MajorVersion 
& 0x0f) << 4) | (mPrivateData.Smbios.MinorVersion & 0x0f); Add &0x0F to mask 
upper nibble bit, do we still need to guarantee that range is between 0-9? 
Because the old code only filtered 4 bits, instead of accurately filtering the 
number range 0-9.

Thanks,
Horace Lien

-Original Message-
From: Kinney, Michael D 
Sent: Thursday, August 31, 2023 11:56 PM
To: devel@edk2.groups.io; Lien, HoraceX 
Cc: Liu, Zhiguang ; Bi, Dandan ; 
Zeng, Star ; Gao, Zhichao ; Kinney, 
Michael D 
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is 
not match with SMBIOS version

Are mPrivateData.Smbios.MajorVersion and mPrivateData.Smbios.MinorVersion 
guaranteed to be in range 0..9?

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of 
> horacex.l...@intel.com
> Sent: Wednesday, August 30, 2023 2:13 AM
> To: devel@edk2.groups.io
> Cc: Lien, HoraceX ; Liu, Zhiguang 
> ; Bi, Dandan ; Zeng, Star 
> ; Gao, Zhichao 
> Subject: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision 
> is not match with SMBIOS version
> 
> From: HoraceX Lien 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4544
> 
> These value of Major/Minor version are updated from SMBIOS memory 
> data, but BCD Revision is updated from PCD PcdSmbiosVersion.
> We should also update PCD PcdSmbiosVersion from SMBIOS memory data, to 
> ensure that get consistent version value.
> 
> Cc: Zhiguang Liu 
> Cc: Dandan Bi 
> Cc: Star Zeng 
> Cc: Zhichao Gao 
> Signed-off-by: HoraceX Lien 
> ---
>  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> index 1a86e69d3c..e3f6215033 100644
> --- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> +++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> @@ -1072,7 +1072,7 @@ SmbiosCreateTable (
>  DEBUG ((DEBUG_INFO, "SmbiosCreateTable: Initialize 32-bit entry 
> point structure\n"));
> 
>  EntryPointStructureData.MajorVersion  =
> mPrivateData.Smbios.MajorVersion;
> 
>  EntryPointStructureData.MinorVersion  =
> mPrivateData.Smbios.MinorVersion;
> 
> -EntryPointStructureData.SmbiosBcdRevision = (UINT8)((PcdGet16
> (PcdSmbiosVersion) >> 4) & 0xf0) | (UINT8)(PcdGet16 (PcdSmbiosVersion) 
> & 0x0f);
> 
> +EntryPointStructureData.SmbiosBcdRevision =
> (mPrivateData.Smbios.MajorVersion << 4) | 
> mPrivateData.Smbios.MinorVersion;
> 
>  PhysicalAddress   = 0x;
> 
>  Status= gBS->AllocatePages (
> 
> 
> AllocateMaxAddress,
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#108150): 
> https://edk2.groups.io/g/devel/message/108150
> Mute This Topic: https://groups.io/mt/101057293/1643496
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [michael.d.kin...@intel.com]
> -=-=-=-=-=-=
> 








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

[edk2-devel] [PATCH v2] Pyrite support - Secure erase is only available if encryption is supported.

[Linus Liu]

From: Linus Liu 

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

Cc: Qi Zhang
Cc: Rahul Kumar 
Cc: Jiewen Yao  
Cc: Tina Chen   
Cc: Xiao X Chen 
---
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index e2e77cbc24..ba9fa66c60 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -87,7 +87,11 @@ OpalSupportGetAvailableActions (
 // Secure erase is performed by generating a new encryption key
 // this is only available if encryption is supported
 //
-AvalDiskActions->SecureErase = 1;
+if (SupportedAttributes->MediaEncryption) {
+  AvalDiskActions->SecureErase = 1;
+} else {
+  AvalDiskActions->SecureErase = 0;
+}
   } else {
 AvalDiskActions->PsidRevert  = 0;
 AvalDiskActions->SecureErase = 0;
-- 
2.39.2.windows.1



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

[edk2-devel] [PATCH v1 0/1] OvmfPkg/Bhyve: properly fall back to static ACPI tables

[Corvin Köhne]

CI: https://github.com/tianocore/edk2/pull/4799

Corvin Köhne (1):
  OvmfPkg/Bhyve: don't exit early if RSDP is not found in memory

 OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

-- 
2.42.0



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

[edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: don't exit early if RSDP is not found in memory

[Corvin Köhne]

If OVMF fails to find the RSDP in memory, it should fall back installing
the statically provided ACPI tables.

Signed-off-by: Corvin Köhne 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
Cc: Peter Grehan 
---
 OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c 
b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
index fb926a8bd803..57b1e7a99666 100644
--- a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
+++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
@@ -259,19 +259,17 @@ InstallAcpiTables (
  BHYVE_BIOS_PHYSICAL_END,
  
  );
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
-  Status = InstallAcpiTablesFromRsdp (
- AcpiTable,
- Rsdp
- );
   if (!EFI_ERROR (Status)) {
-return EFI_SUCCESS;
+Status = InstallAcpiTablesFromRsdp (
+   AcpiTable,
+   Rsdp
+   );
+if (!EFI_ERROR (Status)) {
+  return EFI_SUCCESS;
+}
   }
 
-  if (Status != EFI_NOT_FOUND) {
+  if (EFI_ERROR (Status)) {
 DEBUG (
   (
DEBUG_WARN,
@@ -280,7 +278,6 @@ InstallAcpiTables (
Status
   )
   );
-return Status;
   }
 
   Status = InstallOvmfFvTables (AcpiTable);
-- 
2.42.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108367): https://edk2.groups.io/g/devel/message/108367
Mute This Topic: https://groups.io/mt/101210702/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] Pyrite support - Secure erase is only available if encryption is supported.

[Yao, Jiewen]

Thanks.

1) I think we need an else branch for " SupportedAttributes->MediaEncryption ", 
to assign " AvalDiskActions->SecureErase = 0;"


> -AvalDiskActions->SecureErase = 1;
> 
> +if (SupportedAttributes->MediaEncryption) {
> 
> +  AvalDiskActions->SecureErase = 1;
> 
> +}
> 
>} else {
> 
>  AvalDiskActions->PsidRevert  = 0;
> 
>  AvalDiskActions->SecureErase = 0;


2) May I know what test you have done?





> -Original Message-
> From: Liu, Linus 
> Sent: Monday, September 4, 2023 4:38 PM
> To: devel@edk2.groups.io
> Cc: Liu, Linus ; Zhang, Qi1 ; Kumar,
> Rahul R ; Yao, Jiewen 
> Subject: [PATCH v1] Pyrite support - Secure erase is only available if 
> encryption is
> supported.
> 
> From: Linus Liu 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3004
> 
> Cc: Qi Zhang
> Cc: Rahul Kumar 
> Cc: Jiewen Yao  
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index e2e77cbc24..88650a28dc 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -87,7 +87,9 @@ OpalSupportGetAvailableActions (
>  // Secure erase is performed by generating a new encryption key
> 
>  // this is only available if encryption is supported
> 
>  //
> 
> -AvalDiskActions->SecureErase = 1;
> 
> +if (SupportedAttributes->MediaEncryption) {
> 
> +  AvalDiskActions->SecureErase = 1;
> 
> +}
> 
>} else {
> 
>  AvalDiskActions->PsidRevert  = 0;
> 
>  AvalDiskActions->SecureErase = 0;
> 
> --
> 2.39.2.windows.1



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

[Wu, Hao A]

Speaking of PcAtChipsetPkg/Library/AcpiTimerLib, the implementation of below 
instances:
* PeiAcpiTimerLib.inf
* DxeAcpiTimerLib.inf
* StandaloneMmAcpiTimerLib.inf
will not calculate the TSC frequency upon every call of 
GetPerformanceCounterProperties().

Best Regards,
Hao Wu

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Wu, Hao
> A
> Sent: Thursday, September 7, 2023 3:48 PM
> To: Mike Maslenkin ; devel@edk2.groups.io
> Cc: Henz, Patrick 
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> Performance Timer for XHCI Timeouts
> 
> I do not have strong opinion on this considering it is an IO driver.
> The call of GetTimeInNanoSecond() is also likely to invoke
> GetPerformanceCounterProperties() call.
> 
> I will let the patch owner to decide on the open raised below.
> 
> Best Regards,
> Hao Wu
> 
> > -Original Message-
> > From: Mike Maslenkin 
> > Sent: Thursday, September 7, 2023 3:36 PM
> > To: devel@edk2.groups.io; Wu, Hao A 
> > Cc: Henz, Patrick 
> > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > Performance Timer for XHCI Timeouts
> >
> > Hello Hao Wu,
> >
> > Isn't GetPerformanceCounterProperties (, ) call
> > too "heavy" for XHCI paths?
> > May be it's better to cache timer direction once?
> > I'm asking in a context of PcAtChipsetPkg/Library/AcpiTimerLib
> > instance used by a number of Intel's platform in edk-platforms.
> > For this  library GetPerformanceCounterProperties finally calls
> > InternalCalculateTscFrequency, that isn't simple.
> >
> > Regards,
> > Mike
> >
> > On Thu, Sep 7, 2023 at 4:27 AM Wu, Hao A  wrote:
> > >
> > > Sorry for the late response.
> > > Reviewed-by: Hao A Wu 
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Henz, Patrick
> > > > Sent: Thursday, September 7, 2023 4:37 AM
> > > > To: devel@edk2.groups.io
> > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > > Performance Timer for XHCI Timeouts
> > > >
> > > > I sent this patch out a few weeks ago now but haven't seen a
> > > > reply, just checking in to make sure it didn't get missed.
> > > >
> > > > Thanks,
> > > > Patrick Henz
> > > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Henz, Patrick
> > > > Sent: Monday, August 14, 2023 10:52 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Henz, Patrick 
> > > > Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > > Performance Timer for XHCI Timeouts
> > > >
> > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948
> > > >
> > > > XhciDxe uses the timer functionality provided by the boot services
> > > > table to detect timeout conditions. This breaks the driver's
> > > > ExitBootServices call back, as CoreExitBootServices halts the
> > > > timer before signaling the ExitBootServices event. If the host
> > > > controller fails to halt in the call back, the timeout condition
> > > > will never occur and the boot gets stuck in an indefinite spin
> > > > loop. Use the free running timer provided by TimerLib to calculate
> > > > timeouts, avoiding
> > the potential hang.
> > > >
> > > > Signed-off-by: Patrick Henz 
> > > > Reviewed-by:
> > > > ---
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +--
> > > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
> > > > +---
> > > >  5 files changed, 129 insertions(+), 86 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > index 62682dd27c..1dcbe20512 100644
> > > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > > @@ -1,6 +1,7 @@
> > > >  /** @file
> > > >The XHCI controller driver.
> > > >
> > > > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> > > >  Copyright (c) 2011 - 2022, Intel Corporation. All rights
> > > > reserved.
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
> > > >
> > > >return EFI_SUCCESS;
> > > >  }
> > > > +
> > > > +/**
> > > > +  Computes and returns the elapsed time in nanoseconds since
> > > > +  PreviousTick. The value of PreviousTick is overwritten with the
> > > > +  current performance counter value.
> > > > +
> > > > +  @param  PreviousTickPointer to PreviousTick count.
> > > > +  @return The elapsed time in nanoseconds since PreviousCount.
> > > > +  PreviousCount is overwritten with the current performance
> > > > +  counter value.
> > > > +**/
> > > > +UINT64
> > > > +XhcGetElapsedTime (
> > > > +  IN OUT UINT64  *PreviousTick
> > > > +  )
> > > > +{
> > > > +  UINT64  StartValue;
> 

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

[Wu, Hao A]

I do not have strong opinion on this considering it is an IO driver.
The call of GetTimeInNanoSecond() is also likely to invoke 
GetPerformanceCounterProperties() call.

I will let the patch owner to decide on the open raised below.

Best Regards,
Hao Wu

> -Original Message-
> From: Mike Maslenkin 
> Sent: Thursday, September 7, 2023 3:36 PM
> To: devel@edk2.groups.io; Wu, Hao A 
> Cc: Henz, Patrick 
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> Performance Timer for XHCI Timeouts
> 
> Hello Hao Wu,
> 
> Isn't GetPerformanceCounterProperties (, ) call too
> "heavy" for XHCI paths?
> May be it's better to cache timer direction once?
> I'm asking in a context of PcAtChipsetPkg/Library/AcpiTimerLib
> instance used by a number of Intel's platform in edk-platforms.
> For this  library GetPerformanceCounterProperties finally calls
> InternalCalculateTscFrequency, that isn't simple.
> 
> Regards,
> Mike
> 
> On Thu, Sep 7, 2023 at 4:27 AM Wu, Hao A  wrote:
> >
> > Sorry for the late response.
> > Reviewed-by: Hao A Wu 
> >
> > Best Regards,
> > Hao Wu
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Henz,
> > > Patrick
> > > Sent: Thursday, September 7, 2023 4:37 AM
> > > To: devel@edk2.groups.io
> > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > Performance Timer for XHCI Timeouts
> > >
> > > I sent this patch out a few weeks ago now but haven't seen a reply,
> > > just checking in to make sure it didn't get missed.
> > >
> > > Thanks,
> > > Patrick Henz
> > >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Henz,
> > > Patrick
> > > Sent: Monday, August 14, 2023 10:52 AM
> > > To: devel@edk2.groups.io
> > > Cc: Henz, Patrick 
> > > Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > > Performance Timer for XHCI Timeouts
> > >
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948
> > >
> > > XhciDxe uses the timer functionality provided by the boot services
> > > table to detect timeout conditions. This breaks the driver's
> > > ExitBootServices call back, as CoreExitBootServices halts the timer
> > > before signaling the ExitBootServices event. If the host controller
> > > fails to halt in the call back, the timeout condition will never
> > > occur and the boot gets stuck in an indefinite spin loop. Use the
> > > free running timer provided by TimerLib to calculate timeouts, avoiding
> the potential hang.
> > >
> > > Signed-off-by: Patrick Henz 
> > > Reviewed-by:
> > > ---
> > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> > >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +--
> > >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68
> > > +---
> > >  5 files changed, 129 insertions(+), 86 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > index 62682dd27c..1dcbe20512 100644
> > > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > > @@ -1,6 +1,7 @@
> > >  /** @file
> > >The XHCI controller driver.
> > >
> > > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> > >  Copyright (c) 2011 - 2022, Intel Corporation. All rights
> > > reserved.
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
> > >
> > >return EFI_SUCCESS;
> > >  }
> > > +
> > > +/**
> > > +  Computes and returns the elapsed time in nanoseconds since
> > > +  PreviousTick. The value of PreviousTick is overwritten with the
> > > +  current performance counter value.
> > > +
> > > +  @param  PreviousTickPointer to PreviousTick count.
> > > +  @return The elapsed time in nanoseconds since PreviousCount.
> > > +  PreviousCount is overwritten with the current performance
> > > +  counter value.
> > > +**/
> > > +UINT64
> > > +XhcGetElapsedTime (
> > > +  IN OUT UINT64  *PreviousTick
> > > +  )
> > > +{
> > > +  UINT64  StartValue;
> > > +  UINT64  EndValue;
> > > +  UINT64  CurrentTick;
> > > +  UINT64  Delta;
> > > +
> > > +  GetPerformanceCounterProperties (, );
> > > +
> > > +  CurrentTick = GetPerformanceCounter ();
> > > +
> > > +  //
> > > +  // Determine if the counter is counting up or down  //  if
> > > + (StartValue < EndValue) {
> > > +//
> > > +// Counter counts upwards, check for an overflow condition
> > > +//
> > > +if (*PreviousTick > CurrentTick) {
> > > +  Delta = (EndValue - *PreviousTick) + CurrentTick;
> > > +} else {
> > > +  Delta = CurrentTick - *PreviousTick;
> > > +}
> > > +  } else {
> > > +//
> > > +// Counter counts downwards, check for an underflow condition
> > > +//
> > > +if (*PreviousTick < CurrentTick) {
> > > +  Delta = (StartValue - CurrentTick) 

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

[Mike Maslenkin]

Hello Hao Wu,

Isn't GetPerformanceCounterProperties (, ) call
too "heavy" for XHCI paths?
May be it's better to cache timer direction once?
I'm asking in a context of PcAtChipsetPkg/Library/AcpiTimerLib
instance used by a number of Intel's platform in edk-platforms.
For this  library GetPerformanceCounterProperties finally calls
InternalCalculateTscFrequency, that isn't simple.

Regards,
Mike

On Thu, Sep 7, 2023 at 4:27 AM Wu, Hao A  wrote:
>
> Sorry for the late response.
> Reviewed-by: Hao A Wu 
>
> Best Regards,
> Hao Wu
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Henz,
> > Patrick
> > Sent: Thursday, September 7, 2023 4:37 AM
> > To: devel@edk2.groups.io
> > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use
> > Performance Timer for XHCI Timeouts
> >
> > I sent this patch out a few weeks ago now but haven't seen a reply, just
> > checking in to make sure it didn't get missed.
> >
> > Thanks,
> > Patrick Henz
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Henz,
> > Patrick
> > Sent: Monday, August 14, 2023 10:52 AM
> > To: devel@edk2.groups.io
> > Cc: Henz, Patrick 
> > Subject: [edk2-devel] [PATCH v2] MdeModulePkg/XhciDxe: Use Performance
> > Timer for XHCI Timeouts
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2948
> >
> > XhciDxe uses the timer functionality provided by the boot services table to
> > detect timeout conditions. This breaks the driver's ExitBootServices call 
> > back,
> > as CoreExitBootServices halts the timer before signaling the 
> > ExitBootServices
> > event. If the host controller fails to halt in the call back, the timeout 
> > condition
> > will never occur and the boot gets stuck in an indefinite spin loop. Use the
> > free running timer provided by TimerLib to calculate timeouts, avoiding the
> > potential hang.
> >
> > Signed-off-by: Patrick Henz 
> > Reviewed-by:
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 56 +++
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h  | 22 
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf |  2 +
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 67 +--
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 68 +---
> >  5 files changed, 129 insertions(+), 86 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > index 62682dd27c..1dcbe20512 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > @@ -1,6 +1,7 @@
> >  /** @file
> >The XHCI controller driver.
> >
> > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> >  Copyright (c) 2011 - 2022, Intel Corporation. All rights reserved.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -2294,3 +2295,58 @@ XhcDriverBindingStop (
> >
> >return EFI_SUCCESS;
> >  }
> > +
> > +/**
> > +  Computes and returns the elapsed time in nanoseconds since
> > +  PreviousTick. The value of PreviousTick is overwritten with the
> > +  current performance counter value.
> > +
> > +  @param  PreviousTickPointer to PreviousTick count.
> > +  @return The elapsed time in nanoseconds since PreviousCount.
> > +  PreviousCount is overwritten with the current performance
> > +  counter value.
> > +**/
> > +UINT64
> > +XhcGetElapsedTime (
> > +  IN OUT UINT64  *PreviousTick
> > +  )
> > +{
> > +  UINT64  StartValue;
> > +  UINT64  EndValue;
> > +  UINT64  CurrentTick;
> > +  UINT64  Delta;
> > +
> > +  GetPerformanceCounterProperties (, );
> > +
> > +  CurrentTick = GetPerformanceCounter ();
> > +
> > +  //
> > +  // Determine if the counter is counting up or down  //  if
> > + (StartValue < EndValue) {
> > +//
> > +// Counter counts upwards, check for an overflow condition
> > +//
> > +if (*PreviousTick > CurrentTick) {
> > +  Delta = (EndValue - *PreviousTick) + CurrentTick;
> > +} else {
> > +  Delta = CurrentTick - *PreviousTick;
> > +}
> > +  } else {
> > +//
> > +// Counter counts downwards, check for an underflow condition
> > +//
> > +if (*PreviousTick < CurrentTick) {
> > +  Delta = (StartValue - CurrentTick) + *PreviousTick;
> > +} else {
> > +  Delta = *PreviousTick - CurrentTick;
> > +}
> > +  }
> > +
> > +  //
> > +  // Set PreviousTick to CurrentTick
> > +  //
> > +  *PreviousTick = CurrentTick;
> > +
> > +  return GetTimeInNanoSecond (Delta);
> > +}
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> > index ca223bd20c..77feb66647 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> > @@ -2,6 +2,7 @@
> >
> >Provides some data structure definitions used by the XHCI host controller
> > driver.
> >
> > +(C) Copyright 2023 Hewlett Packard Enterprise Development LP
> >  Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
> > Copyright 

[edk2-devel] [PATCH] MdeModulePkg: Support customized FV Migration Information

[Wang Fan]

From: Wang Fan 

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

There are use cases which not all FVs need be migrated from TempRam to
permanent memory before TempRam tears down. This new guid is introduced
to avoid unnecessary FV migration to improve boot performance. Platform
can publish hob with this guid to customize FV migration info, and PEI
Core will only migrate FVs indicated by this Hob info.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Guomin Jiang 
Cc: Dandan Bi 
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 82 +--
 MdeModulePkg/Core/Pei/PeiMain.inf |  1 +
 MdeModulePkg/Include/Guid/MigratedFvInfo.h| 19 +
 MdeModulePkg/MdeModulePkg.dec |  3 +-
 4 files changed, 79 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c 
b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 5f32ebb560..a9e4f8fcca 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1184,7 +1184,11 @@ EvacuateTempRam (
 
   PEI_CORE_FV_HANDLEPeiCoreFvHandle;
   EFI_PEI_CORE_FV_LOCATION_PPI  *PeiCoreFvLocationPpi;
+  EDKII_TO_MIGRATE_FV_INFO  *ToMigrateFvInfo;
   EDKII_MIGRATED_FV_INFOMigratedFvInfo;
+  EFI_PEI_HOB_POINTERS  Hob;
+  BOOLEAN   MigrateAllFvs;
+  UINT32MigrationFlags;
 
   ASSERT (Private->PeiMemoryInstalled);
 
@@ -1211,6 +1215,17 @@ EvacuateTempRam (
 
   ConvertPeiCorePpiPointers (Private, );
 
+  //
+  // Check if platform defined hobs to indicate which FVs are expected to 
migrate or keep raw data.
+  // If ToMigrateFvInfo hobs exists, only migrate FVs recorded by hobs.
+  // If ToMigrateFvInfo hobs not exists, migrate all FVs to permanant memory.
+  //
+  MigrateAllFvs = TRUE;
+  Hob.Raw   = GetFirstGuidHob ();
+  if (Hob.Raw != NULL) {
+MigrateAllFvs = FALSE;
+  }
+
   for (FvIndex = 0; FvIndex < Private->FvCount; FvIndex++) {
 FvHeader = Private->Fv[FvIndex].FvHeader;
 ASSERT (FvHeader != NULL);
@@ -1224,6 +1239,25 @@ EvacuateTempRam (
   )
 )
 {
+  if (MigrateAllFvs) {
+MigrationFlags = 0;
+  } else {
+MigrationFlags = FLAGS_SKIP_FV_MIGRATION | FLAGS_SKIP_FV_RAW_DATA_COPY;
+Hob.Raw = GetFirstGuidHob ();
+while (Hob.Raw != NULL) {
+  ToMigrateFvInfo = GET_GUID_HOB_DATA (Hob);
+  if (ToMigrateFvInfo->FvOrgBase == (UINT32)(UINTN)FvHeader) {
+MigrationFlags = ToMigrateFvInfo->MigrationFlags;
+break;
+  }
+  Hob.Raw = GET_NEXT_HOB (Hob);
+  Hob.Raw = GetNextGuidHob (, Hob.Raw);
+}
+  }
+  if (MigrationFlags & FLAGS_SKIP_FV_MIGRATION) {
+continue;
+  }
+
   //
   // Allocate page to save the rebased PEIMs, the PEIMs will get 
dispatched later.
   //
@@ -1234,18 +1268,7 @@ EvacuateTempRam (
   );
   ASSERT_EFI_ERROR (Status);
   MigratedFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvHeaderAddress;
-
-  //
-  // Allocate pool to save the raw PEIMs, which is used to keep consistent 
context across
-  // multiple boot and PCR0 will keep the same no matter if the address of 
allocated page is changed.
-  //
-  Status =  PeiServicesAllocatePages (
-  EfiBootServicesCode,
-  EFI_SIZE_TO_PAGES ((UINTN)FvHeader->FvLength),
-  
-  );
-  ASSERT_EFI_ERROR (Status);
-  RawDataFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvHeaderAddress;
+  CopyMem (MigratedFvHeader, FvHeader, (UINTN)FvHeader->FvLength);
 
   DEBUG ((
 DEBUG_VERBOSE,
@@ -1256,18 +1279,29 @@ EvacuateTempRam (
 ));
 
   //
-  // Copy the context to the rebased pages and raw pages, and create hob 
to save the
-  // information. The MigratedFvInfo HOB will never be produced when
-  // PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD 
control the
-  // feature.
+  // Copy the context to the raw pages, and create hob to save the 
information. The MigratedFvInfo
+  // HOB will never be produced when PcdMigrateTemporaryRamFirmwareVolumes 
is FALSE, because the PCD
+  // controls the feature.
   //
-  CopyMem (MigratedFvHeader, FvHeader, (UINTN)FvHeader->FvLength);
-  CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN)FvHeader->FvLength);
-  MigratedFvInfo.FvOrgBase  = (UINT32)(UINTN)FvHeader;
-  MigratedFvInfo.FvNewBase  = (UINT32)(UINTN)MigratedFvHeader;
-  MigratedFvInfo.FvDataBase = (UINT32)(UINTN)RawDataFvHeader;
-  MigratedFvInfo.FvLength   = (UINT32)(UINTN)FvHeader->FvLength;
-  BuildGuidDataHob (, , sizeof 
(MigratedFvInfo));
+  if ((MigrationFlags & FLAGS_SKIP_FV_RAW_DATA_COPY) != 
FLAGS_SKIP_FV_RAW_DATA_COPY) {
+//
+// Allocate pool to save the raw PEIMs, which is used to keep 

Re: [edk2-devel] [PATCH V9 0/2] Support RSA4096 and RSA3072

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

Merged https://github.com/tianocore/edk2/pull/4798

> -Original Message-
> From: Sheng, W 
> Sent: Thursday, September 7, 2023 9:57 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J ;
> Xu, Min M ; Chen, Zeyi ; Wang,
> Fiona ; Lu, Xiaoyu1 ; Jiang,
> Guomin ; Kinney, Michael D
> 
> Subject: [PATCH V9 0/2] Support RSA4096 and RSA3072
> 
> Patch V9:
> Refine coding format for file AuthService.c
> 
> Patch V8:
> Update the patch comments for CryptoPkg.
> Comment should be <76 characters in each line.
> Refine coding format.
> 
> Patch V7:
> Drop raw RSA3072 and RSA4096. Only use gEfiCertX509Guid for RSA3072 and
> RSA4096
> Do the positive tests and the negative tests below. And got all the expected
> results.
> 
> Patch V6:
> Remove the changes in MdePkg.
> The changes of patch v6 are in CryptoPkg and SecurityPkg.
> Set signature type to gEfiCertX509Guid when enroll RSA3072/RSA4096 KEK.
> This signature type is used to check the supported signature and show the 
> strings.
> 
> Patch V5:
> Using define KEY_TYPE_RSASSA to replace the magic number.
> 
> Patch V4:
> Determine the RSA algorithm by a supported algorithm list.
> 
> Patch V3:
> Select SHA algorithm automaticly for a unsigned efi image.
> 
> Patch V2:
> Determine the SHA algorithm by a supported algorithm list.
> Create SHA context for each algorithm.
> 
> Test Case:
> 1. Enroll a RSA4096 Cert, and execute an RSA4096 signed efi image under UEFI
> shell.
> 2. Enroll a RSA3072 Cert, and execute an RSA3072 signed efi image under UEFI
> shell.
> 3. Enroll a RSA2048 Cert, and execute an RSA2048 signed efi image under UEFI
> shell.
> 4. Enroll an unsigned efi image, execute the unsigned efi image under UEFI 
> shell
> 
> Test Result:
> Pass
> 
> Negative Test Case:
> 1) Enroll a RSA2048 Cert, execute an unsigned efi image.
> 2) Enroll a RSA2048 Cert, execute a RSA4096 signed efi image.
> 3) Enroll a RSA4096 Cert, execute a RSA3072 signed efi image.
> 4) Enroll a RSA4096 Cert to both DB and DBX, execute the RSA4096 signed efi
> image.
> 
> Test Result:
> Get "Access Denied" when try to execute the efi image.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Min Xu 
> Cc: Zeyi Chen 
> Cc: Fiona Wang 
> Cc: Xiaoyu Lu 
> Cc: Guomin Jiang 
> Cc: Michael D Kinney 
> 
> Sheng Wei (2):
>   CryptoPkg/BaseCryptLib: add sha384 and sha512 to ImageTimestampVerify
>   SecurityPkg/SecureBoot: Support RSA4096 and RSA3072
> 
>  CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c   |   3 +-
>  .../Library/AuthVariableLib/AuthService.c | 225 +++---
>  .../AuthVariableLib/AuthServiceInternal.h |   4 +-
>  .../Library/AuthVariableLib/AuthVariableLib.c |  42 ++--
>  .../DxeImageVerificationLib.c |  74 +++---
>  .../SecureBootConfigDxe.inf   |   8 +
>  .../SecureBootConfigImpl.c|  52 +++-
>  .../SecureBootConfigImpl.h|   7 +
>  .../SecureBootConfigStrings.uni   |   2 +
>  9 files changed, 331 insertions(+), 86 deletions(-)
> 
> --
> 2.26.2.windows.1



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