date:20221128

[edk2-devel] [PATCH] ShellPkg: Multiple Coverity issues were found from EDK2 ShellPkg

2022-11-28 Thread Kalaivani P via groups.io

Attached is the report for Coverity issues identified in ShellPkg based
on edk2-stable202205.

Cc: Vasudevan Sambandan 
Cc: Sundaresan Selvaraj 
Cc: Arun k 
Cc: Sainadh N 
Signed-off-by: Kalaivani P 
---
 ShellPkg/Application/Shell/ShellManParser.c   |  6 +-
 ShellPkg/Application/Shell/ShellProtocol.c|  3 ++-
 .../Library/UefiShellDebug1CommandsLib/Dblk.c |  6 +++---
 .../HexEdit/BufferImage.c | 10 ++---
 .../HexEdit/FileImage.c   |  6 +-
 .../UefiShellDriver1CommandsLib/DrvCfg.c  |  3 ++-
 .../Library/UefiShellLevel1CommandsLib/For.c  |  4 
 .../Library/UefiShellLevel1CommandsLib/If.c   |  4 
 .../Library/UefiShellLevel2CommandsLib/Vol.c  |  5 -
 .../Library/UefiShellLevel3CommandsLib/Help.c |  4 
 ShellPkg/Library/UefiShellLib/UefiShellLib.c  | 21 +++
 11 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/ShellPkg/Application/Shell/ShellManParser.c 
b/ShellPkg/Application/Shell/ShellManParser.c
index 5c823cd7f5..716eb17a1d 100644
--- a/ShellPkg/Application/Shell/ShellManParser.c
+++ b/ShellPkg/Application/Shell/ShellManParser.c
@@ -2,6 +2,7 @@
   Provides interface to shell MAN file parser.



   Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.

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

   Copyright 2015 Dell Inc.

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



@@ -601,7 +602,10 @@ ProcessManFile (
   if (TempString != NULL) {

 FileHandle = ConvertEfiFileProtocolToShellHandle (CreateFileInterfaceMem 
(TRUE), NULL);

 HelpSize   = StrLen (TempString) * sizeof (CHAR16);

-ShellWriteFile (FileHandle, , TempString);

+Status = ShellWriteFile (FileHandle, , TempString);

+if (EFI_ERROR (Status)) {

+  return Status;

+}

 ShellSetFilePosition (FileHandle, 0);

 HelpSize  = 0;

 BriefSize = 0;

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c 
b/ShellPkg/Application/Shell/ShellProtocol.c
index 509eb60e40..fbe1d7e01f 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -5,6 +5,7 @@
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.

   (C) Copyright 2016 Hewlett Packard Enterprise Development LP

   Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.

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

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



 **/

@@ -2518,7 +2519,7 @@ ShellSearchHandle (
   EfiShellClose (ShellInfoNode->Handle);

   ShellInfoNode->Handle = NULL;

 }

-  } else if (!EFI_ERROR (Status)) {

+  } else if (!EFI_ERROR (Status) && (ShellInfoNode->FullName == NULL)) 
{

 //

 // should be a file

 //

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
index 97a4b57a93..08372d9fa4 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
@@ -3,6 +3,7 @@


   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.

   Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.

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

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



 **/

@@ -45,8 +46,7 @@ DisplayTheBlocks (
   if (EFI_ERROR (Status)) {

 return (SHELL_NOT_FOUND);

   }

-

-  BufferSize = BlockIo->Media->BlockSize * BlockCount;

+  BufferSize = BlockIo->Media->BlockSize * (UINTN)BlockCount;

   if (BlockIo->Media->IoAlign == 0) {

 BlockIo->Media->IoAlign = 1;

   }

@@ -55,7 +55,7 @@ DisplayTheBlocks (
 OriginalBuffer = AllocateZeroPool (BufferSize + BlockIo->Media->IoAlign);

 Buffer = ALIGN_POINTER (OriginalBuffer, BlockIo->Media->IoAlign);

   } else {

-ShellPrintEx (-1, -1, L"  BlockSize: 0x%08x, BlockCount: 0x%08x\r\n", 
BlockIo->Media->BlockSize, BlockCount);

+ShellPrintEx (-1, -1, L"  BlockSize: 0x%08x, BlockCount: 0x%08x\r\n", 
BlockIo->Media->BlockSize,(UINTN)BlockCount);

 OriginalBuffer = NULL;

 Buffer = NULL;

   }

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
index be77e31a40..e68ab7c21a 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
@@ -3,6 +3,7 @@
   as well as the event handlers for editing the file



   Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved. 

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

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



 **/

@@ -2036,7 +2037,8 @@ HBufferImageAddCharacterToBuffer (
   UINTN  OldPos;



   UINTN  NewPos;

-

+  EFI_STATUS   Status;

+

   Size = HBufferImageGetTotalSize ();

回复: [edk2-devel] [PATCH 1/1] BaseSynchronizationLib: Fix RISC-V helper name

2022-11-28 Thread gaoliming via groups.io

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Sunil V L
> 发送时间: 2022年11月11日 19:21
> 收件人: devel@edk2.groups.io
> 抄送: Abner Chang ; Zhihao Li
> ; Michael D Kinney ;
> Liming Gao ; Zhiguang Liu
> ; Daniel Schaefer 
> 主题: [edk2-devel] [PATCH 1/1] BaseSynchronizationLib: Fix RISC-V helper
> name
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4143
> 
> Fix the name of InternalSyncCompareExchange64() function.
> 
> Signed-off-by: Sunil V L 
> Reported-by: Zhihao Li 
> Tested-by: Zhihao Li 
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Daniel Schaefer 
> ---
>  .../Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
>  .../Library/BaseSynchronizationLib/RiscV64/Synchronization.S  | 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git
> a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> index dd66ec1d0370..88dfb880fea9 100755
> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> @@ -81,7 +81,7 @@ [Sources.AARCH64]
> 
>  [Sources.RISCV64]
>Synchronization.c
> -  RiscV64/Synchronization.S
> +  RiscV64/Synchronization.S | GCC
> 
>  [Sources.LOONGARCH64]
>Synchronization.c
> diff --git
> a/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.S
> b/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.S
> index bac80d687168..f287ef38f651 100644
> --- a/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.S
> +++ b/MdePkg/Library/BaseSynchronizationLib/RiscV64/Synchronization.S
> @@ -36,8 +36,6 @@ exit:
>  mva0, a3
>  ret
> 
> -.global ASM_PFX(InternalSyncCompareExchange64)
> -
>  //
>  // Compare and xchange a 64-bit value.
>  //
> @@ -45,7 +43,7 @@ exit:
>  // @param a1 : Compare value.
>  // @param a2 : Exchange value.
>  //
> -ASM_PFX (SyncCompareExchange64):
> +ASM_PFX (InternalSyncCompareExchange64):
>  lr.d  a3, (a0)   // Load the value from a0 and make
>   // the reservation of address.
>  bne   a3, a1, exit
> --
> 2.38.0
> 
> 
> 
> 
> 





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

Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] FW: [PATCH] ShellPkg: Displaying SMBIOS Type38 fields in formatted manner

2022-11-28 Thread Prakash K via groups.io

Hi Gaoliming,

Link for Pull Request -> https://github.com/tianocore/edk2/pull/3656

Thanks,
Prakash K


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

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, November 29, 2022 #cal-reminder

2022-11-28 Thread Group Notification

*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, November 29, 2022
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Liming Gao gaolim...@byosoft.com.cn ( 
gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

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

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions ( 
https://conf.intel.com/teams/?conf=1160620940=teams=conf.intel.com=test_call
 )

*Or call in (audio only)*

+1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, 
Sacramento

Phone Conference ID: 774 638 21#

Find a local number ( 
https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821
 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2=0=en-US
 )


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#9): https://edk2.groups.io/g/devel/message/9
Mute This Topic: https://groups.io/mt/95327022/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] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

2022-11-28 Thread Rebecca Cran

Thanks. RPi4 and Juno use EmbeddedPkg/MetronomeDxe 
(https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/MetronomeDxe/Metronome.c#L54) 
which uses the PCD PcdMetronomeTickPeriod.


JunoPkg overrides that to 1000, while RPi4 uses the default of 100. 
Given that the clocks run at 50MHz and 54MHz, I think the tick period 
should be 1 (1 100-ns unit, giving a clock frequency of 10MHz)?


--
Rebecca Cran



On 11/28/22 12:51, Andrew Fish via groups.io wrote:

Rebecca,

gBS->Stall() is built on top [1] of the Metronome Architectural Protocol 
[2]. You should look at how the platform implements the Metronome 
Architectural Protocol.


It looks like most platform implement a generic Metronome Driver[3] that 
just sits on top of the platforms TimerLib [4] implementation.


You can find the TimerLib implementations via:

$ git grep TimerLib -- \*.inf | grep LIBRARY_CLASS

ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf:14:  LIBRARY_CLASS
               = TimerLib


EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf:19:  
LIBRARY_CLASS                  = DebugAgentTimerLib|SEC BASE DXE_CORE

EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.inf:22:  
LIBRARY_CLASS                  = TimerLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION


EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.inf:22:  LIBRARY_CLASS  
             = TimerLib|DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER 
UEFI_APPLICATION


EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.inf:22:  LIBRARY_CLASS  
             = TimerLib|PEIM PEI_CORE SEC


MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf:23:  
LIBRARY_CLASS                  = TimerLib

MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf:30:  
LIBRARY_CLASS                  = TimerLib


OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf:17:  LIBRARY_CLASS  = 
TimerLib|PEI_CORE PEIM DXE_CORE


OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf:18:  
LIBRARY_CLASS  = TimerLib


OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf:16:  LIBRARY_CLASS  
= TimerLib|SEC


OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf:17:  LIBRARY_CLASS  = 
TimerLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER 
UEFI_APPLICATION SMM_CORE


PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf:21:  
LIBRARY_CLASS                  = TimerLib|SEC PEI_CORE PEIM


PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf:21:  
LIBRARY_CLASS                  = TimerLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE


PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf:21:  
LIBRARY_CLASS                  = TimerLib|PEI_CORE PEIM


PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmAcpiTimerLib.inf:23:  
LIBRARY_CLASS                  = TimerLib|MM_CORE_STANDALONE MM_STANDALONE


UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf:18:  LIBRARY_CLASS
               = TimerLib


UefiCpuPkg/Library/SecPeiDxeTimerLibUefiCpu/SecPeiDxeTimerLibUefiCpu.inf:30:  
LIBRARY_CLASS                  = TimerLib

UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf:15:  LIBRARY_CLASS  
                 = TimerLib



So I’d guess your platform is using the ArmArchTimerLi [5]. If that is 
the case I’d look at the value of PcdArmArchTimerFreqInHz if it is zero 
then I think you are running on the default ARM timer value.


If you are using the stock stuff and all your TimerLib instances are the 
same in all your drivers then the TimerLib is also abstraction the 
performance measure.


This is probably not helpful but the UEFI Spec defines that Stall() 
delays for at least the requested amount of time, with no upper bound on 
how long it can take.


[1] 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Misc/Stall.c#L49 
[2] 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Metronome.h#L25 
[3] 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Metronome/Metronome.c#L61 
[4] 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/TimerLib.h 
[5] 
https://github.com/tianocore/edk2/blob/master/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c#L18 


Thanks,

Andrew Fish



On Nov 28, 2022, at 11:02 AM, Rebecca Cran  wrote:

I've been doing some work on USB and ended up realizing that 
gBS->Stall(1) is taking much longer than it should: on my Juno R2 it's 
stalling for 100 us, while on my Raspberry Pi 4 it's 10 us.


This appears to be causing a USB bulk transfer request that asks for a

[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, November 28, 2022 #cal-notice

2022-11-28 Thread Group Notification

*Tools, CI, Code base construction meeting series*

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

*Where:*
https://github.com/tianocore/edk2/discussions/2614

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96664): https://edk2.groups.io/g/devel/message/96664
Mute This Topic: https://groups.io/mt/95325119/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 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

2022-11-28 Thread Rebecca Cran


On 11/28/22 15:59, Kun Qin wrote:

Hi Rebecca/Ard,

I was trying to reach out regarding the original patches earlier (see 
below) but it might fell off your stacks
due to high traffic on the mailing list. Could you please kindly review 
the questions when you have a chance?


In addition, I found another edge case of the MP service: when the AP 
routine hits a timeout, the metadata
will be left in the unfinished states. If the AP routine eventually 
completes and return, this AP will stay in "finished"
but never become "ready" during this boot. I tried to add below change, 
which seems to work. But please let me

know if you have other concerns:
https://github.com/kuqin12/mu_silicon_arm_tiano/commit/c76072b37018276f2fec2582d0c540be5b40d0f2


Thanks, I'll take a look and integrate the fix into the next revision of 
the patch series.




Lastly, do you plan to merge these patches in the near future? This will 
be a great add-on for ARM platforms.


The issue that's currently preventing them from being merged is a 
failure I noticed on the Neoverse N2 FVP: for some reason despite 
enabling the MMU and caches, manual cache flushes are still required for 
the data to be seen between CPUs. I don't know if that's a bug in the 
code or in the FVP model.


--
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96663): https://edk2.groups.io/g/devel/message/96663
Mute This Topic: https://groups.io/mt/93329492/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/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

2022-11-28 Thread Rebecca Cran


Sorry, I missed this originally. I've added my replies inline.][

On 9/29/22 12:45, Kun Qin wrote:

Hi Rebecca,

Thanks for sending this patch. I have a few questions inline "[KQ]". 
Could you please help me to

understand the patch better? Thanks in advance.

Regards,
Kun

On 8/29/2022 8:59 AM, Rebecca Cran wrote:



+#define GET_MPIDR_AFFINITY_BITS(x)  ((x) & 0xFF00FF)
+
+#define MPIDR_MT_BIT  BIT24
[KQ]: Is there any reason why this is not defined in a more formal 
place? I think that will allow the platform to use as well.


That's a good point. I can certainly move it somewhere more proper.


+  BOOLEAN IsBsp;
[KQ]: Can we add a check to see if BSP is found during this routine? I 
think failing to identify a BSP might cause other issues in the 
following services?


We could certainly log an error and abort if IsBsp is never set to TRUE.


+  for (Index = 0; Index < mCpuMpData.NumberOfProcessors; Index++) {
+    if (GET_MPIDR_AFFINITY_BITS (ArmReadMpidr ()) == 
CoreInfo[Index].Mpidr) {
[KQ]: Does this mean BSP should never set the `MPIDR_MT_BIT` in the 
gArmMpCoreInfoGuid HOB data? Otherwise, this logic will never be able to 
find the BSP?


No, here we assume we're running on the BSP, and the code is looking for 
the entry in the CoreInfo array that matches the BSP's affinity fields.



+  switch (GetApState (CpuData)) {
+    case CpuStateReady:
+  SetApProcedure (CpuData, Procedure, ProcedureArgument);
+  Status = DispatchCpu (Index);
+  if (EFI_ERROR (Status)) {
+    CpuData->State = CpuStateIdle;
+    Status = EFI_NOT_READY;
+    goto Done;
[KQ]: Is it really necessary to use "goto Done"? This might cause all 
the CPUs after this failing index to not reset the CPU state.
That way, all the subsequent "StartupAllAps" will fail due to 
"CheckAllCpusReady" returning FALSE on those "dirty cores". Please

let me know if that is by design or not.


Good point. I'll fix it.


--
Rebecca Cran


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

[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, November 28, 2022 #cal-reminder

2022-11-28 Thread Group Notification

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

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

*Where:*
https://github.com/tianocore/edk2/discussions/2614

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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

Re: [edk2-devel] [PATCH 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

2022-11-28 Thread Kun Qin


Hi Rebecca/Ard,

I was trying to reach out regarding the original patches earlier (see 
below) but it might fell off your stacks
due to high traffic on the mailing list. Could you please kindly review 
the questions when you have a chance?


In addition, I found another edge case of the MP service: when the AP 
routine hits a timeout, the metadata
will be left in the unfinished states. If the AP routine eventually 
completes and return, this AP will stay in "finished"
but never become "ready" during this boot. I tried to add below change, 
which seems to work. But please let me

know if you have other concerns:
https://github.com/kuqin12/mu_silicon_arm_tiano/commit/c76072b37018276f2fec2582d0c540be5b40d0f2

Lastly, do you plan to merge these patches in the near future? This will 
be a great add-on for ARM platforms.


Thanks,
Kun

On 9/29/2022 11:45 AM, Kun Qin wrote:

Hi Rebecca,

Thanks for sending this patch. I have a few questions inline "[KQ]". 
Could you please help me to

understand the patch better? Thanks in advance.

Regards,
Kun

On 8/29/2022 8:59 AM, Rebecca Cran wrote:

Add support for EFI_MP_SERVICES_PROTOCOL during the DXE phase under
AArch64.

PSCI_CPU_ON is called to power on the core, the supplied procedure is
executed and PSCI_CPU_OFF is called to power off the core.

Fixes contributed by Ard Biesheuvel.

Signed-off-by: Rebecca Cran 
---
  ArmPkg/ArmPkg.dsc |    1 +
  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   55 +
  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h |  351 
  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1774 


  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S |   57 +
  5 files changed, 2238 insertions(+)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 59fd8f295d4f..4716789402fc 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -125,6 +125,7 @@ [Components.common]
    ArmPkg/Drivers/CpuPei/CpuPei.inf
    ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
    ArmPkg/Drivers/ArmGic/ArmGicLib.inf
+  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf
    ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
    ArmPkg/Drivers/TimerDxe/TimerDxe.inf
  diff --git 
a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf 
b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf

new file mode 100644
index ..2b109c72e0a0
--- /dev/null
+++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf
@@ -0,0 +1,55 @@
+## @file
+#  ARM MP services protocol driver
+#
+#  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights 
reserved.

+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION    = 1.27
+  BASE_NAME  = ArmPsciMpServicesDxe
+  FILE_GUID  = 007ab472-dc4a-4df8-a5c2-abb4a327278c
+  MODULE_TYPE    = DXE_DRIVER
+  VERSION_STRING = 1.0
+
+  ENTRY_POINT    = ArmPsciMpServicesDxeInitialize
+
+[Sources.Common]
+  ArmPsciMpServicesDxe.c
+  MpFuncs.S
+  MpServicesInternal.h
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  ArmMmuLib
+  ArmSmcLib
+  BaseMemoryLib
+  CacheMaintenanceLib
+  DebugLib
+  HobLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiMpServiceProtocolGuid    ## PRODUCES
+  gEfiLoadedImageProtocolGuid  ## CONSUMES
+
+[Guids]
+  gArmMpCoreInfoGuid
+
+[Depex]
+  TRUE
+
+[BuildOptions]
+  GCC:*_*_*_CC_FLAGS = -mstrict-align
diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h 
b/ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h

new file mode 100644
index ..ee13816ef64c
--- /dev/null
+++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h
@@ -0,0 +1,351 @@
+/** @file
+
+Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights 
reserved.

+Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Portions copyright (c) 2011, Apple Inc. All rights reserved.
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MP_SERVICES_INTERNAL_H_
+#define MP_SERVICES_INTERNAL_H_
+
+#include 
+#include 
+
+#include 
+#include 
+
+#define AP_STACK_SIZE  0x1000
+
+//
+// Internal Data Structures
+//
+
+//
+// AP state
+//
+// The state transitions for an AP when it process a procedure are:
+//  Idle > Ready > Busy > Idle
+//   [BSP]   [AP]   [AP]
+//
+typedef enum {
+  CpuStateIdle,
+  CpuStateReady,
+  CpuStateBlocked,
+  CpuStateBusy,
+  CpuStateFinished,
+  CpuStateDisabled
+} CPU_STATE;
+
+//
+// Define Individual Processor Data block.
+//
+typedef struct {
+  EFI_PROCESSOR_INFORMATION    Info;
+  EFI_AP_PROCEDURE Procedure;
+  VOID *Parameter;
+  CPU_STATE    State;
+

Re: [edk2-devel] [PATCH v6 0/5] CI: Use Fedora 35 container for Linux jobs

2022-11-28 Thread Chris Fernald

After discussing this a bit more internally, I think I'm actually coming 
around to removing the compiler external dependencies. Leaving behind 
the unused external dependencies is likely to lead to them collecting 
dust, so it would probably be better left for consumers to define at the 
platform level if they need it.


One additional change I would like, but will not hold my review on is to 
remove the "gcc__linux" scopes from PlatformBuild.py and 
CISettings.py as they will no longer be used once the extdep files are 
removed.


Reviewed-by: Chris Fernald 

On 9/26/2022 9:31 AM, Oliver Steffen wrote:

Update CI, run all Linux (aka Ubuntu-GCC5) based jobs in custom
containers.

The container image provides the required compiler toolchains and Qemu
for the supported architectures. These are then no longer downloaded at
runtime, avoiding CI failures due to download errors. This approach also
makes it easier to switch to other or newer compilers. It makes the CI
setup independent from the default images that Azure DevOps provides.
It can also help debugging CI problems, because the CI environment
can be reproduced on a local machine.

The container images are hosted on ghcr.io and are automatically
generated using GitHub Actions. The Dockerfiles are maintained in the
Tianocore "containers" repository:
https://github.com/tianocore/containers.

The current image is based on Fedora 35, with gcc 11. Fedora was chosen
because of its fast release cycle which makes it easy to keep the
toolchains up-to-date.

Some further possible changes not included in this series:
- The Tianocore/containers repository provides stack of layered images.
   One image for general purpose (build+test) and build-only jobs.
   The build+test image is based on the build-only one and adds Qemu,
   for the testing job that involve Qemu. This reduces the total download
   size. This was suggested by Chris Fernald. The work in the image side
   is done, we just need to change the CI setup accordingly.
   This patch set uses the build+test images for all jobs.
- Further reduce the number of external dependencies that need to be
   downloaded at runtime. Candidates are iasl and nasm, which are already
   included in the image but not used yet.

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

v6:
- Include suggestions by Chris Fernald.
- Added a parameter for the container image to the job template, makes usage
   of containers optional.
- Added a parameter to configure the Python version to download. Allows
   using Python from the VM/container image also.
- Restructure the commits (no further functional changes).

v5:
- Update image

v4:
- Use the latest image from the tianocode/containers repository which
   - does not include acpica-tools
   - includes Pyhton 3.10

v3:
- Use the latest image from the tianocode/containers repository which
   pins down version numbers of gcc, iasl, and nasm in the Dockerfile.

v2:
- Images are now hosted under the Tianocore Organization
   https://github.com/tianocore/containers

v1:
- Thread: https://edk2.groups.io/g/devel/message/89058
- Images were hosted at https://github.com/osteffenrh/edk2-build-images

Signed-off-by: Oliver Steffen 
Acked-by: Ard Biesheuvel 
Acked-by: Gerd Hoffmann 

Oliver Steffen (5):
   CI: make Python version configurable
   CI: add ~/.local/bin to PATH (Linux only)
   CI: Allow running in a container.
   CI: Use Fedora 35 container (Linux only)
   BaseTools: Remove ext. gcc dependencies (Linux only)

  .azurepipelines/Ubuntu-GCC5.yml   |  2 ++
  .../templates/basetools-build-steps.yml   |  9 
  .../templates/platform-build-run-steps.yml| 12 +-
  .../templates/pr-gate-build-job.yml   |  6 +
  .azurepipelines/templates/pr-gate-steps.yml   | 12 --
  .../.azurepipelines/Ubuntu-GCC5.yml   |  7 +++---
  BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml  | 21 --
  BaseTools/Bin/gcc_arm_linux_ext_dep.yaml  | 21 --
  .../Bin/gcc_riscv64_unknown_ext_dep.yaml  | 22 ---
  .../.azurepipelines/Ubuntu-GCC5.yml   |  3 +++
  .../.azurepipelines/Ubuntu-GCC5.yml   |  7 +++---
  11 files changed, 38 insertions(+), 84 deletions(-)
  delete mode 100644 BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml
  delete mode 100644 BaseTools/Bin/gcc_arm_linux_ext_dep.yaml
  delete mode 100644 BaseTools/Bin/gcc_riscv64_unknown_ext_dep.yaml




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

Re: [edk2-devel] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

2022-11-28 Thread Andrew Fish via groups.io

Rebecca, 

gBS->Stall() is built on top [1] of the Metronome Architectural Protocol [2]. 
You should look at how the platform implements the Metronome Architectural 
Protocol. 

It looks like most platform implement a generic Metronome Driver[3] that just 
sits on top of the platforms TimerLib [4] implementation. 

You can find the TimerLib implementations via:
$ git grep TimerLib -- \*.inf | grep LIBRARY_CLASS
ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf:14:  LIBRARY_CLASS   
   = TimerLib
EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf:19:  
LIBRARY_CLASS  = DebugAgentTimerLib|SEC BASE DXE_CORE
EmulatorPkg/Library/DxeCoreTimerLib/DxeCoreTimerLib.inf:22:  LIBRARY_CLASS  
= TimerLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER 
UEFI_APPLICATION
EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.inf:22:  LIBRARY_CLASS  
= TimerLib|DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.inf:22:  LIBRARY_CLASS  
= TimerLib|PEIM PEI_CORE SEC
MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf:23:  
LIBRARY_CLASS  = TimerLib
MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf:30:  LIBRARY_CLASS 
 = TimerLib
OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf:17:  LIBRARY_CLASS  = 
TimerLib|PEI_CORE PEIM DXE_CORE
OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf:18:  LIBRARY_CLASS  = 
TimerLib
OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf:16:  LIBRARY_CLASS  = 
TimerLib|SEC
OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf:17:  LIBRARY_CLASS  = 
TimerLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER 
UEFI_APPLICATION SMM_CORE
PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf:21:  LIBRARY_CLASS 
 = TimerLib|SEC PEI_CORE PEIM
PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf:21:  LIBRARY_CLASS  
= TimerLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER 
UEFI_APPLICATION UEFI_DRIVER SMM_CORE
PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf:21:  LIBRARY_CLASS  
= TimerLib|PEI_CORE PEIM
PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmAcpiTimerLib.inf:23:  
LIBRARY_CLASS  = TimerLib|MM_CORE_STANDALONE MM_STANDALONE
UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf:18:  LIBRARY_CLASS   
   = TimerLib
UefiCpuPkg/Library/SecPeiDxeTimerLibUefiCpu/SecPeiDxeTimerLibUefiCpu.inf:30:  
LIBRARY_CLASS  = TimerLib
UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf:15:  LIBRARY_CLASS 
 = TimerLib

So I’d guess your platform is using the ArmArchTimerLi [5]. If that is the case 
I’d look at the value of PcdArmArchTimerFreqInHz if it is zero then I think you 
are running on the default ARM timer value. 

If you are using the stock stuff and all your TimerLib instances are the same 
in all your drivers then the TimerLib is also abstraction the performance 
measure.

This is probably not helpful but the UEFI Spec defines that Stall() delays for 
at least the requested amount of time, with no upper bound on how long it can 
take.  

[1] 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Misc/Stall.c#L49
[2] 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Metronome.h#L25
[3] 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Metronome/Metronome.c#L61
 
[4] 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/TimerLib.h 
[5] 
https://github.com/tianocore/edk2/blob/master/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.c#L18

Thanks,

Andrew Fish


> On Nov 28, 2022, at 11:02 AM, Rebecca Cran  wrote:
> 
> I've been doing some work on USB and ended up realizing that gBS->Stall(1) is 
> taking much longer than it should: on my Juno R2 it's stalling for 100 us, 
> while on my Raspberry Pi 4 it's 10 us.
> 
> This appears to be causing a USB bulk transfer request that asks for a 1ms 
> timeout taking 100ms on the Juno.
> 
> I'm measuring the delay with the following code:
> 
> 
> UINT64 First = GetPerformanceCounter ();
> 
> gBS->Stall (XHC_1_MICROSECOND);
> 
> UINT64 Second = GetPerformanceCounter ();
> 
> UINT64 FirstNs = GetTimeInNanoSecond (First);
> UINT64 SecondNs = GetTimeInNanoSecond (Second);
> 
> DEBUG ((DEBUG_INFO, "Stalled for %llu ns (%llu ms)\n", (SecondNs - FirstNs), 
> (SecondNs - FirstNs) / 1024 / 1024));
> 
> 
> 
> I see output such as:
> 
> Stalled for 10500 ns (0 ms)
> 
> 
> -- 
> Rebecca Cran
> 
> 
> 
> 
> 



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

[edk2-devel] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

2022-11-28 Thread Rebecca Cran

I've been doing some work on USB and ended up realizing that 
gBS->Stall(1) is taking much longer than it should: on my Juno R2 it's 
stalling for 100 us, while on my Raspberry Pi 4 it's 10 us.


This appears to be causing a USB bulk transfer request that asks for a 
1ms timeout taking 100ms on the Juno.


I'm measuring the delay with the following code:


UINT64 First = GetPerformanceCounter ();

gBS->Stall (XHC_1_MICROSECOND);

UINT64 Second = GetPerformanceCounter ();

UINT64 FirstNs = GetTimeInNanoSecond (First);
UINT64 SecondNs = GetTimeInNanoSecond (Second);

DEBUG ((DEBUG_INFO, "Stalled for %llu ns (%llu ms)\n", (SecondNs - 
FirstNs), (SecondNs - FirstNs) / 1024 / 1024));




I see output such as:

Stalled for 10500 ns (0 ms)


--
Rebecca Cran


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

Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable

2022-11-28 Thread Gerd Hoffmann

On Mon, Sep 26, 2022 at 10:24:58AM +0200, Ard Biesheuvel wrote:
> When the memory protections were implemented and enabled on ArmVirtQemu
> 5+ years ago, we had to work around the fact that GRUB at the time
> expected EFI_LOADER_DATA to be executable, as that is the memory type it
> allocates when loading its modules.
> 
> This has been fixed in GRUB in August 2017, so by now, we should be able
> to tighten this, and remove execute permissions from EFI_LOADER_DATA
> allocations.

Data point: https://bugzilla.redhat.com/show_bug.cgi?id=2149020
tl;dr: fedora 37 grub.efi is still broken.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

2022-11-28 Thread Michael D Kinney

Ard,

I agree it should be a strong recommendation for all of these reasons.

There is a patch review for this spec for use of 'static'.  Can you 
please provide feedback with your recommended content?

Thanks,

Mike

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, November 28, 2022 1:08 AM
> To: devel@edk2.groups.io; Kinney, Michael D 
> Cc: Chang, Abner ; Ni, Ray ; Gao, 
> Liming 
> Subject: Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 
> 5.4.2.2 STATIC
> 
> On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
>  wrote:
> >
> > Hi Abner,
> >
> > Removing that section 5.4.2.2 is required to close this bug.
> >
> > Meaning of 'static' is covered by the ANSI C standards.
> >
> > Use of 'static' for non-public variable/functions in EDK II
> > libraries/modules is recommended.
> >
> > However, it is not required.  It is recommended to reduce chances
> > of symbol conflicts at link time.  Current approach is if a link
> > failure occurs for multiply defined symbols and those are non-public
> > symbols, the 'static' attribute can be applied to the non-public
> > symbols in the components that generated the link failure.
> >
> > It may be good to mention this recommendation in the CSS.
> >
> > I will let you decide when this recommendation needs to be
> > added to CSS.
> >
> 
> 'static' is not just a tool to avoid symbol conflicts. It also avoids
> abuse of symbols that are assumed to have a private nature but can be
> linked to nonetheless (e.g., in static libraries). Ideally, any
> library should only export the symbols that it defines as part of its
> interface, although this is not currently feasible of a library
> consists of multiple object files.
> 
> Another thing to keep in mind is that static is used by the compiler
> to make inferences about the value. A static global variable can only
> be modified by the code in the same compilation unit, and so the
> compiler is free to optimize accesses or perform constant propagation
> and drop it entirely if it only observes reads and no writes from the
> variable.
> 
> I consider it good developer hygiene to always use static on global
> symbols (code and data) unless the symbol needs to be shared with
> other compilation units.


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

Re: [edk2-devel] [PATCH v6 5/5] BaseTools: Remove ext. gcc dependencies (Linux only)

2022-11-28 Thread Oliver Steffen

Quoting Chris Fernald (2022-11-28 06:20:36)
> Over-all I think this change is good to go. My only outstanding concern
> would be around deleting these yaml files for the compilers. I fear
> deleting them may break others work-flows and wonder if instead we
> should update the python files to first check for the presence of the
> prefix shell variable to dynamically determine if the extdep is needed
> or not similar to how the compiler plugins determine whether to use the
> extdep compiler or not.

Yes.  Deleting the ext_dep files is for sure not the right way.

I think we need an option to allow the user (be it human or CI script)
to choose between using the ext_dep and the tools already present on the
system (=distro tools).  This should affect gcc as well as all other
tools (iasl, nasm, ...).

I am not sure how to implement this in a good way.  I had also started a
discussion on github about that [0].  But I have to admit that I was very
busy recently and had little time to work on this (sorry).

Thanks,
Oliver

[0] https://github.com/tianocore/edk2-pytool-extensions/discussions/323

>
> Thanks,
>
> Chris
>
> On 9/26/2022 9:31 AM, Oliver Steffen wrote:
> > Remove BaseTools/Bin/gcc*_linux_ext_dep.yaml to stop
> > downloading gcc from external locations; use the
> > toolchains provided by the container image instead.
> >
> > The image needs to set the GCC5_*_PREFIX accordingly.
> >
> > Signed-off-by: Oliver Steffen 
> > ---
> >   BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml  | 21 --
> >   BaseTools/Bin/gcc_arm_linux_ext_dep.yaml  | 21 --
> >   .../Bin/gcc_riscv64_unknown_ext_dep.yaml  | 22 ---
> >   3 files changed, 64 deletions(-)
> >   delete mode 100644 BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml
> >   delete mode 100644 BaseTools/Bin/gcc_arm_linux_ext_dep.yaml
> >   delete mode 100644 BaseTools/Bin/gcc_riscv64_unknown_ext_dep.yaml
> >
> > diff --git a/BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml
> > b/BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml
> > deleted file mode 100644
> > index ff8a9e868100..
> > --- a/BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -## @file
> > -# Download GCC AARCH64 compiler from Arm's release site
> > -# Set shell variable GCC5_AARCH64_INSTALL to this folder
> > -#
> > -# This is only downloaded when a build activates scope gcc_aarch64_linux
> > -#
> > -# Copyright (c) Microsoft Corporation.
> > -# SPDX-License-Identifier: BSD-2-Clause-Patent
> > -##
> > -{
> > -  "scope": "gcc_aarch64_linux",
> > -  "type": "web",
> > -  "name": "gcc_aarch64_linux",
> > -  "source": 
> > "https://developer.arm.com/-/media/Files/downloads/gnu/11.2-2022.02/binrel/gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu.tar.xz;,
> > -  "version": "11.2-2022.02",
> > -  "sha256": 
> > "52dbac3eb71dbe0916f60a8c5ab9b7dc9b66b3ce513047baa09fae56234e53f3",
> > -  "compression_type": "tar",
> > -  "internal_path": "/gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu/",
> > -  "flags": ["set_shell_var", ],
> > -  "var_name": "GCC5_AARCH64_INSTALL"
> > -}
> > diff --git a/BaseTools/Bin/gcc_arm_linux_ext_dep.yaml
> > b/BaseTools/Bin/gcc_arm_linux_ext_dep.yaml
> > deleted file mode 100644
> > index 151cbfa4b532..
> > --- a/BaseTools/Bin/gcc_arm_linux_ext_dep.yaml
> > +++ /dev/null
> > @@ -1,21 +0,0 @@
> > -## @file
> > -# Download GCC ARM compiler from Arm's release site
> > -# Set shell variable GCC5_ARM_INSTALL to this folder
> > -#
> > -# This is only downloaded when a build activates scope gcc_arm_linux
> > -#
> > -# Copyright (c) Microsoft Corporation.
> > -# SPDX-License-Identifier: BSD-2-Clause-Patent
> > -##
> > -{
> > -  "scope": "gcc_arm_linux",
> > -  "type": "web",
> > -  "name": "gcc_arm_linux",
> > -  "source": 
> > "https://developer.arm.com/-/media/Files/downloads/gnu/11.2-2022.02/binrel/gcc-arm-11.2-2022.02-x86_64-arm-none-linux-gnueabihf.tar.xz;,
> > -  "version": "11.2-2022.02",
> > -  "sha256": 
> > "c254f7199261fe76c32ef42187502839bda7efad0a66646cf739d074eff45fad",
> > -  "compression_type": "tar",
> > -  "internal_path": 
> > "/gcc-arm-11.2-2022.02-x86_64-arm-none-linux-gnueabihf/",
> > -  "flags": ["set_shell_var", ],
> > -  "var_name": "GCC5_ARM_INSTALL"
> > -}
> > diff --git a/BaseTools/Bin/gcc_riscv64_unknown_ext_dep.yaml
> > b/BaseTools/Bin/gcc_riscv64_unknown_ext_dep.yaml
> > deleted file mode 100644
> > index 8abbcd7ba040..
> > --- a/BaseTools/Bin/gcc_riscv64_unknown_ext_dep.yaml
> > +++ /dev/null
> > @@ -1,22 +0,0 @@
> > -## @file
> > -# Download GCC RISCV64 compiler from RISC-V Organization release site
> > -# Set shell variable GCC5_RISCV64_INSTALL to this folder
> > -#
> > -# This is only downloaded when a build activates scope gcc_riscv64_unknown
> > -#
> > -# Copyright (c) Microsoft Corporation.
> > -# Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> > rights reserved.
> > -# SPDX-License-Identifier: BSD-2-Clause-Patent

Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for ArmTrngLib/RngDxe

2022-11-28 Thread PierreGondois


Hello Ard,

On 11/26/22 15:33, Ard Biesheuvel wrote:

On Thu, 24 Nov 2022 at 17:18,  wrote:


From: Pierre Gondois 

v1:
- https://edk2.groups.io/g/devel/message/96356
v2:
- https://edk2.groups.io/g/devel/message/96434
- Reformulate commit message.
- Do not warn if no algorithm is found as the message
   would be printed on non-Arm platforms.
v3:
- Add the following patches:
   1. ArmPkg/ArmTrngLib: Remove ASSERTs in ArmTrngLibConstructor()
  Requested by Ard.
  Cf https://edk2.groups.io/g/devel/message/96495
   2. SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL
  Do not install EFI_RNG_PROTOCOL if no RNG algorithm is available.
  Cf. https://edk2.groups.io/g/devel/message/96494
   3. SecurityPkg/RngDxe: Fix Rng algo selection for Arm
  Coming from v2 patch being split.

Some issues were found by Ard/Sami on the RngDxe/ArmTrngLib after
recent patches were merged. This patch serie intends to fix them.

Pierre Gondois (4):
   ArmPkg/ArmTrngLib: Remove ASSERTs in ArmTrngLibConstructor()


Thanks for the fixed

Reviewed-by: Ard Biesheuvel 

I pushed this one as #3663 (pending CI verification atm)


   SecurityPkg/RngDxe: Correctly update mAvailableAlgoArrayCount
   SecurityPkg/RngDxe: Conditionally install EFI_RNG_PROTOCOL
   SecurityPkg/RngDxe: Fix Rng algo selection for Arm



The remaining code still looks a bit clunky to me. Can't we just
return an error from the library constructor of the library cannot
initialize due to a missing prerequisite?


In RngDriverEntry(), GetAvailableAlgorithms() probe the available
RNG algorithms. If there are none, then we return an error code.
Otherwise we install EFI_RNG_PROTOCOL.

I assume the prerequisite you a referring to is 'checking there is
at least one available RNG algorithm that RngDxe can use', so if
ArmTrngLib's constructor fails, we should not prevent RngDxe to be
loaded (as the RngLib could be used for instance).

Does it answer your question, or did I understand it incorrectly ?

Regards,
Pierre





  ArmPkg/Library/ArmTrngLib/ArmTrngLib.c|  5 -
  .../RandomNumberGenerator/RngDxe/ArmRngDxe.c  | 18 +-
  .../RngDxe/Rand/RngDxe.c  |  9 -
  .../RandomNumberGenerator/RngDxe/RngDxe.c | 19 ++-
  4 files changed, 27 insertions(+), 24 deletions(-)

--
2.25.1




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

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] Silicon/ARM/NeoverseN1Soc: Update CCIX PNP ID

2022-11-28 Thread Leif Lindholm


Hi Sahil,

Please split the license update out into a separate patch.

/
Leif

On 2022-11-24 15:55, sa...@arm.com wrote:

There is no need for a separate ID for CCIX host bridge,
therefore reusing PCIe PNP ID for CCIX.
Also, updating the file's license to resolve error during
ECC checks.

Signed-off-by: sahil 
---
  Silicon/ARM/NeoverseN1Soc/Library/PciHostBridgeLib/PciHostBridgeLib.c | 17 
+
  1 file changed, 9 insertions(+), 8 deletions(-)

diff --git 
a/Silicon/ARM/NeoverseN1Soc/Library/PciHostBridgeLib/PciHostBridgeLib.c 
b/Silicon/ARM/NeoverseN1Soc/Library/PciHostBridgeLib/PciHostBridgeLib.c
index 1f38f654a8ce..4c1acf154ca0 100644
--- a/Silicon/ARM/NeoverseN1Soc/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/Silicon/ARM/NeoverseN1Soc/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -1,10 +1,11 @@
  /** @file

-*  PCI Host Bridge Library instance for ARM Neoverse N1 platform

-*

-*  Copyright (c) 2019 - 2022, ARM Limited. All rights reserved.

-*

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

-*

+

+  PCI Host Bridge Library instance for ARM Neoverse N1 platform

+

+  Copyright (c) 2019 - 2022, ARM Limited. All rights reserved.

+

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

+

  **/

  


  #include 

@@ -65,8 +66,8 @@ STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH 
mEfiPciRootBridgeDevicePath[ROOT_COMPLEX_
(UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8)

  }

},

-  EISA_PNP_ID(0x0A09), // CCIX

-  0

+  EISA_PNP_ID(0x0A08), // CCIX

+  1

  },

  {

END_DEVICE_PATH_TYPE,





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

Re: [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformCI: Add new JOB in .yml of OvmfPkg PlatformCI

2022-11-28 Thread duntan

Hi all,
Could you please help to review this patch? Thanks a lot!

Thanks,
Dun

-Original Message-
From: devel@edk2.groups.io  On Behalf Of duntan
Sent: Tuesday, November 22, 2022 7:48 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel ; Yao, Jiewen 
; Justen, Jordan L ; Gerd 
Hoffmann ; Ni, Ray 
Subject: [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformCI: Add new JOB in .yml of 
OvmfPkg PlatformCI

Add new job like OVMF_X64_DEBUG_UNIT_TEST in OvmfPkg PlatformCI .yml file. New 
parameter unit_test_list is used to specify Shell Unit Test list which needs to 
build and run. Format for this input should be:'-u 
ModulePath1:DscPath1,ModulePath2:DscPath2'
or '-u ModulePath1:DscPath1 -u ModulePath2:DscPath2'.
(Path is edk2 workspace relative)

Signed-off-by: Dun Tan 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
---
 OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml| 11 +++
 OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml | 11 +++
 2 files changed, 22 insertions(+)

diff --git a/OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml 
b/OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
index 7160d95f7e..2242ffebb5 100644
--- a/OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
+++ b/OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC5.yml
@@ -22,6 +22,7 @@ jobs:
   vm_image: 'ubuntu-18.04'
   should_run: true
   run_flags: "MAKE_STARTUP_NSH=TRUE QEMU_HEADLESS=TRUE"
+  unit_test_list: ''
 
 #Use matrix to speed up the build process
 strategy:
@@ -55,6 +56,15 @@ jobs:
 Build.Target: "DEBUG"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  OVMF_X64_DEBUG_UNIT_TEST:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: ""
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+# unit_test_list should be the format: '-u 
ModulePath1:DscPath1,ModulePath2:DscPath2' or '-u ModulePath1:DscPath1 -u 
ModulePath2:DscPath2'.(Path is workspace relative)
+unit_test_list: ''
   OVMF_X64_RELEASE:
 Build.File: "$(package)/PlatformCI/PlatformBuild.py"
 Build.Arch: "X64"
@@ -187,6 +197,7 @@ jobs:
 build_file: $(Build.File)
 build_flags: $(Build.Flags)
 run_flags: $(Run.Flags)
+unit_test_list: $(unit_test_list)
 extra_install_step:
 - bash: sudo apt-get install qemu
   displayName: Install qemu
diff --git a/OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml 
b/OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
index 7d6344d638..881db9eb27 100644
--- a/OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
+++ b/OvmfPkg/PlatformCI/.azurepipelines/Windows-VS2019.yml
@@ -21,6 +21,7 @@ jobs:
   vm_image: 'windows-2019'
   should_run: true
   run_flags: "MAKE_STARTUP_NSH=TRUE QEMU_HEADLESS=TRUE"
+  unit_test_list : ''
 
 #Use matrix to speed up the build process
 strategy:
@@ -54,6 +55,15 @@ jobs:
 Build.Target: "DEBUG"
 Run.Flags: $(run_flags)
 Run: $(should_run)
+  OVMF_X64_DEBUG_UNIT_TEST:
+Build.File: "$(package)/PlatformCI/PlatformBuild.py"
+Build.Arch: "X64"
+Build.Flags: ""
+Build.Target: "DEBUG"
+Run.Flags: $(run_flags)
+Run: $(should_run)
+# unit_test_list should be the format: '-u 
ModulePath1:DscPath1,ModulePath2:DscPath2' or '-u ModulePath1:DscPath1 -u 
ModulePath2:DscPath2'.(Path is workspace relative)
+unit_test_list: ''
   OVMF_X64_RELEASE:
 Build.File: "$(package)/PlatformCI/PlatformBuild.py"
 Build.Arch: "X64"
@@ -133,6 +143,7 @@ jobs:
 build_file: $(Build.File)
 build_flags: $(Build.Flags)
 run_flags: $(Run.Flags)
+unit_test_list: $(unit_test_list)
 extra_install_step:
 - powershell: choco install qemu --version=2021.5.5; Write-Host 
"##vso[task.prependpath]c:\Program Files\qemu"
   displayName: Install QEMU and Set QEMU on path # friendly name 
displayed in the UI
--
2.31.1.windows.1








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

Re: [edk2-devel] [PATCH 3/3] .azurepipelines: Expand PlatformCI template for Shell UnitTest

2022-11-28 Thread duntan

Hi all,
Could you please help to review this patch? Thanks a lot!

Thanks,
Dun

-Original Message-
From: devel@edk2.groups.io  On Behalf Of duntan
Sent: Tuesday, November 22, 2022 7:48 PM
To: devel@edk2.groups.io
Cc: Sean Brogan ; Michael Kubacki 
; Kinney, Michael D ; 
Gao, Liming ; Ni, Ray 
Subject: [edk2-devel] [PATCH 3/3] .azurepipelines: Expand PlatformCI template 
for Shell UnitTest

Expand PlatformCI build and run steps template for Shell UnitTest. Add a new 
parameter unit_test_list to support building and running specific Shell 
UnitTest modules.

In stuart_pr_eval step, if the unit_test_list passed from platform yml file is 
not null, it will select some packages from the packages which contain the 
module in unit_test_list and set them into a new variable pkgs_to_build base on 
its evaluation rule.
In stuart_build step, if unit_test_list is not null, '${{ 
parameters.unit_test_list}} -p $(pkgs_to_build)' will be added into the 
arguments to build specific UnitTest modules in pkgs_to_build.
In 'Run to shell' step, if unit_test_list is not null, all the UnitTest modules 
built in stuart_build step will runs in shell.

Signed-off-by: Dun Tan 
Cc: Sean Brogan 
Cc: Michael Kubacki 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Ray Ni 
---
 .azurepipelines/templates/platform-build-run-steps.yml | 21 
+
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/.azurepipelines/templates/platform-build-run-steps.yml 
b/.azurepipelines/templates/platform-build-run-steps.yml
index 40a31a509f..51503287c4 100644
--- a/.azurepipelines/templates/platform-build-run-steps.yml
+++ b/.azurepipelines/templates/platform-build-run-steps.yml
@@ -30,6 +30,9 @@ parameters:
 - name: run_flags
   type: string
   default: ''
+- name: unit_test_list
+  type: string
+  default: ''
 
 - name: extra_install_step
   type: stepList
@@ -49,7 +52,9 @@ steps:
   displayName: 'Install/Upgrade pip modules'
 
 # Set default
-- bash: echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
+- bash: |
+echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
+echo "##vso[task.setvariable variable=pkgs_to_build]${{ 'all' }}"
 
 # Fetch the target branch so that pr_eval can diff them.
 # Seems like azure pipelines/github changed checkout process in nov 2020.
@@ -62,7 +67,7 @@ steps:
   displayName: Check if ${{ parameters.build_pkg }} need testing
   inputs:
 filename: stuart_pr_eval
-arguments: -c ${{ parameters.build_file }} -t ${{ 
parameters.build_target}} -a ${{ parameters.build_arch}} --pr-target 
origin/$(System.PullRequest.targetBranch) --output-count-format-string 
"##vso[task.setvariable variable=pkg_count;isOutpout=true]{pkgcount}"
+arguments: -c ${{ parameters.build_file }} -t ${{ 
+ parameters.build_target}} -a ${{ parameters.build_arch}} --pr-target 
+ origin/$(System.PullRequest.targetBranch) --output-count-format-string 
+ "##vso[task.setvariable variable=pkg_count;isOutpout=true]{pkgcount}" 
+ --output-csv-format-string "##vso[task.setvariable 
+ variable=pkgs_to_build]{pkgcsv}" ${{ parameters.unit_test_list}}
   condition: eq(variables['Build.Reason'], 'PullRequest')
 
  # Setup repo
@@ -97,14 +102,22 @@ steps:
   inputs:
 filename: stuart_build
 arguments: -c ${{ parameters.build_file }} TOOL_CHAIN_TAG=${{ 
parameters.tool_chain_tag}} TARGET=${{ parameters.build_target}} -a ${{ 
parameters.build_arch}} ${{ parameters.build_flags}}
-  condition: and(gt(variables.pkg_count, 0), succeeded())
+  condition: and(and(gt(variables.pkg_count, 0), succeeded()), 
+ eq(variables.unit_test_list, ''))
+
+# Build specific pkg for UnitTest
+- task: CmdLine@1
+  displayName: Build UnitTest
+  inputs:
+filename: stuart_build
+arguments: ${{ parameters.unit_test_list}} -p $(pkgs_to_build) -c 
+${{ parameters.build_file }} TOOL_CHAIN_TAG=${{ 
+parameters.tool_chain_tag}} TARGET=${{ parameters.build_target}} -a ${{ 
+parameters.build_arch}} ${{ parameters.build_flags}}
+  condition: and(and(gt(variables.pkg_count, 0), succeeded()), 
+not(eq(variables.unit_test_list, '')))
 
 # Run
 - task: CmdLine@1
   displayName: Run to shell
   inputs:
 filename: stuart_build
-arguments: -c ${{ parameters.build_file }} TOOL_CHAIN_TAG=${{ 
parameters.tool_chain_tag}} TARGET=${{ parameters.build_target}} -a ${{ 
parameters.build_arch}} ${{ parameters.build_flags}} ${{ parameters.run_flags 
}} --FlashOnly
+arguments: -c ${{ parameters.build_file }} TOOL_CHAIN_TAG=${{ 
+ parameters.tool_chain_tag}} TARGET=${{ parameters.build_target}} -a 
+ ${{ parameters.build_arch}} ${{ parameters.build_flags}} ${{ 
+ parameters.run_flags }} --FlashOnly ${{ parameters.unit_test_list}}
   condition: and(and(gt(variables.pkg_count, 0), succeeded()), 
eq(variables['Run'], true))
   timeoutInMinutes: 1
 
--
2.31.1.windows.1








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

Re: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI for Shell UnitTest

2022-11-28 Thread duntan

Hi all,
Could you please help to review this patch? Thanks a lot!

Thanks,
Dun

-Original Message-
From: devel@edk2.groups.io  On Behalf Of duntan
Sent: Tuesday, November 22, 2022 7:48 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel ; Yao, Jiewen 
; Justen, Jordan L ; Gerd 
Hoffmann ; Ni, Ray 
Subject: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI 
for Shell UnitTest

Expand Ovmf PlatformBuild.py and PlatformBuildLib.py to support building and 
running specific Shell target UnitTest modules.
In the new CommonPlatform class:
It provides new class attributes and new methods to support build and run 
specific Shell Unit Test modules.

In the new SettingsManager class for stuart_pr_eval:
It calls new API in CommonPlatform to updates PackagesSupported based on -u 
ShellUnitTestList input from cmd. The package which contains the module in 
ShellUnitTestList will be added into PackagesSupported for further evaluation.

In the new PlatformBuilder class for stuart_build:
1.In PlatformPostBuild(), it conditionally calls new API in CommonPlatform to 
build -u ShellUnitTestList in -p PkgsToBuild and copy them to VirtualDrive 
folder. If no -p option, all the modules in -u ShellUnitTestList will be built.
2.In FlashRomImage(), it conditionally calls the new API in CommonPlatform to 
write all efi files name in VirtualDrive into startup.nsh and output UnitTest 
log into ShellUnitTestLog.
3. After the boot process, it conditionally calls new API in CommonPlatform to 
check the UnitTest boot log.

Signed-off-by: Dun Tan 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Ray Ni 
---
 OvmfPkg/PlatformCI/PlatformBuild.py| 112 

 OvmfPkg/PlatformCI/PlatformBuildLib.py |  51 
+--
 2 files changed, 157 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/PlatformCI/PlatformBuild.py 
b/OvmfPkg/PlatformCI/PlatformBuild.py
index 6c541cdea4..72cb7e0e9e 100644
--- a/OvmfPkg/PlatformCI/PlatformBuild.py
+++ b/OvmfPkg/PlatformCI/PlatformBuild.py
@@ -6,6 +6,11 @@
 ##
 import os
 import sys
+import shutil
+import logging
+import re
+from edk2toolext.environment import shell_environment from 
+edk2toolext.environment.multiple_workspace import MultipleWorkspace
 
 sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 from PlatformBuildLib import SettingsManager @@ -24,6 +29,10 @@ class 
CommonPlatform():
 Scopes = ('ovmf', 'edk2-build')
 WorkspaceRoot = os.path.realpath(os.path.join(
 os.path.dirname(os.path.abspath(__file__)), "..", ".."))
+# Support build and run Shell Unit Test modules
+UnitTestModuleList = {}
+RunShellUnitTest   = False
+ShellUnitTestLog   = ''
 
 @classmethod
 def GetDscName(cls, ArchCsv: str) -> str:
@@ -39,5 +48,108 @@ class CommonPlatform():
 dsc += ".dsc"
 return dsc
 
+@classmethod
+def UpdatePackagesSupported(cls, ShellUnitTestListArg):
+''' Update PackagesSupported by -u ShellUnitTestList from cmd line. '''
+UnitTestModuleListStr = ','.join(ShellUnitTestListArg)
+if not re.search(r'.+.inf:.+.dsc', UnitTestModuleListStr):
+raise Exception('No valid ModulePath:DscPath in the -u 
{}'.format(UnitTestModuleListStr))
+UnitTestModuleList = UnitTestModuleListStr.split(',')
+PackagesSupported = []
+for KeyValue in UnitTestModuleList:
+PkgName = KeyValue.split("Pkg")[0] + 'Pkg'
+if PkgName not in PackagesSupported:
+PackagesSupported.append(PkgName)
+cls.PackagesSupported = tuple(PackagesSupported)
+print('PackagesSupported for UnitTest is 
+ {}'.format(cls.PackagesSupported))
+
+@classmethod
+def UpdateUnitTestConfig(cls, args):
+''' Update UnitTest config by -u ShellUnitTestList and -p 
PkgsToBuildForUT from cmd line.
+ShellUnitTestList is in this format: {module1:dsc1, module2:dsc2, 
module3:dsc2...}.
+Only the modules which are in the PkgsToBuildForUT list are added 
into self.UnitTestModuleList.
+'''
+UnitTestModuleListStr = ','.join(args.ShellUnitTestList)
+if not re.search(r'.+.inf:.+.dsc', UnitTestModuleListStr):
+raise Exception('No valid ModulePath:DscPath in the -u 
{}'.format(args.ShellUnitTestList))
+UnitTestModuleList = UnitTestModuleListStr.split(',')
+if args.PkgsToBuildForUT is None or all(['Pkg' not in Pkg for Pkg in 
args.PkgsToBuildForUT]):
+# No invalid Pkgs from input. Build all modules in -u 
UnitTestModuleList.
+for KeyValue in UnitTestModuleList:
+UnitTestPath = os.path.normpath(KeyValue.split(":")[0])
+DscPath  = os.path.normpath(KeyValue.split(":")[1])
+cls.UnitTestModuleList[UnitTestPath] = DscPath
+else:
+

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

2022-11-28 Thread Ard Biesheuvel

On Tue, 22 Nov 2022 at 19:10, Michael D Kinney
 wrote:
>
> Hi Abner,
>
> Removing that section 5.4.2.2 is required to close this bug.
>
> Meaning of 'static' is covered by the ANSI C standards.
>
> Use of 'static' for non-public variable/functions in EDK II
> libraries/modules is recommended.
>
> However, it is not required.  It is recommended to reduce chances
> of symbol conflicts at link time.  Current approach is if a link
> failure occurs for multiply defined symbols and those are non-public
> symbols, the 'static' attribute can be applied to the non-public
> symbols in the components that generated the link failure.
>
> It may be good to mention this recommendation in the CSS.
>
> I will let you decide when this recommendation needs to be
> added to CSS.
>

'static' is not just a tool to avoid symbol conflicts. It also avoids
abuse of symbols that are assumed to have a private nature but can be
linked to nonetheless (e.g., in static libraries). Ideally, any
library should only export the symbols that it defines as part of its
interface, although this is not currently feasible of a library
consists of multiple object files.

Another thing to keep in mind is that static is used by the compiler
to make inferences about the value. A static global variable can only
be modified by the code in the same compilation unit, and so the
compiler is free to optimize accesses or perform constant propagation
and drop it entirely if it only observes reads and no writes from the
variable.

I consider it good developer hygiene to always use static on global
symbols (code and data) unless the symbol needs to be shared with
other compilation units.

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

Re: [edk2-devel] [PATCH v2 0/3] Enable support for terminal resize

2022-11-28 Thread Ard Biesheuvel

On Mon, 28 Nov 2022 at 05:35, Paweł Poławski  wrote:
>
> This is re-submission of original patches written by Laszlo Ersek.
> When mode will be changed in the EFI - xterm resolution will
> change too. Tested with xterm, Gnome terminal and XFCE4 terminal.
>

I think I know why this is a good thing, but can you explain anyway?

> Laszlo Ersek (3):
>   MdeModulePkg: TerminalDxe: set xterm resolution on mode change
>   OvmfPkg: take PcdResizeXterm from the QEMU command line
>   ArmVirtPkg: take PcdResizeXterm from the QEMU command line
>

Some of these arrived whitespace mangled in my inbox - can you please
double check you git send-email config?


>  MdeModulePkg/MdeModulePkg.dec|  4 ++
>  ArmVirtPkg/ArmVirtQemu.dsc   |  7 
> +++-
>  OvmfPkg/AmdSev/AmdSevX64.dsc |  1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc   |  1 +
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc |  1 +
>  OvmfPkg/Microvm/MicrovmX64.dsc   |  2 +-
>  OvmfPkg/OvmfPkgIa32.dsc  |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc   |  1 +
>  OvmfPkg/OvmfPkgX64.dsc   |  1 +
>  ArmVirtPkg/Library/TerminalPcdProducerLib/TerminalPcdProducerLib.inf | 33 
> 
>  MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf   |  2 +
>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>  ArmVirtPkg/Library/TerminalPcdProducerLib/TerminalPcdProducerLib.c   | 41 
> 
>  MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c  | 29 
> ++
>  OvmfPkg/PlatformPei/Platform.c   | 13 
> +++
>  15 files changed, 136 insertions(+), 2 deletions(-)
>  create mode 100644 
> ArmVirtPkg/Library/TerminalPcdProducerLib/TerminalPcdProducerLib.inf
>  create mode 100644 
> ArmVirtPkg/Library/TerminalPcdProducerLib/TerminalPcdProducerLib.c
>
> --
> 2.38.1
>
>
>
> 
>
>


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

[edk2-devel] [PATCH] ShellPkg: Multiple Coverity issues were found from EDK2 ShellPkg

回复: [edk2-devel] [PATCH 1/1] BaseSynchronizationLib: Fix RISC-V helper name

Re: [edk2-devel] 回复: [edk2-devel] 回复: [edk2-devel] FW: [PATCH] ShellPkg: Displaying SMBIOS Type38 fields in formatted manner

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, November 29, 2022 #cal-reminder

Re: [edk2-devel] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, November 28, 2022 #cal-notice

Re: [edk2-devel] [PATCH 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

Re: [edk2-devel] [PATCH 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, November 28, 2022 #cal-reminder

Re: [edk2-devel] [PATCH 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls

Re: [edk2-devel] [PATCH v6 0/5] CI: Use Fedora 35 container for Linux jobs

Re: [edk2-devel] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

[edk2-devel] On RPi4 and Juno, gBS->Stall(1) takes 10us and 100us respectively

Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

Re: [edk2-devel] [PATCH v6 5/5] BaseTools: Remove ext. gcc dependencies (Linux only)

Re: [edk2-devel] [PATCH v3 0/4] ArmPkg/SecurityPkg: Fixes for ArmTrngLib/RngDxe

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] Silicon/ARM/NeoverseN1Soc: Update CCIX PNP ID

Re: [edk2-devel] [PATCH 2/3] OvmfPkg/PlatformCI: Add new JOB in .yml of OvmfPkg PlatformCI

Re: [edk2-devel] [PATCH 3/3] .azurepipelines: Expand PlatformCI template for Shell UnitTest

Re: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI for Shell UnitTest

Re: [edk2-devel] [PATCH] edk II C Coding Standard: Remove section 5.4.2.2 STATIC

Re: [edk2-devel] [PATCH v2 0/3] Enable support for terminal resize

23 matches

Site Navigation

Mail list logo

Footer information