Re: [edk2-devel] [PATCH v1 07/17] Platform/Loongson/LoongArchQemuPkg: Add ImagePropertiesRecordLib Instance

2023-12-01 Thread xianglai

Reviewed-by:  Xianglai li 


在 2023/12/1 上午8:34, Taylor Beebe 写道:

ImagePropertiesRecordLib is used by DxeMain and PiSmmCore, so it
needs to be added to most platforms.

Signed-off-by: Taylor Beebe 
Cc: Bibo Mao 
Cc: Xianglai li 
Cc: Chao Li 
---
  Platform/Loongson/LoongArchQemuPkg/Loongson.dsc | 1 +
  1 file changed, 1 insertion(+)

diff --git a/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc 
b/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
index 00b28cfa500f..d547d86824b7 100644
--- a/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
+++ b/Platform/Loongson/LoongArchQemuPkg/Loongson.dsc
@@ -155,6 +155,7 @@
PciHostBridgeUtilityLib  | 
OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
MmuLib   | 
Platform/Loongson/LoongArchQemuPkg/Library/MmuLib/MmuBaseLib.inf
FileExplorerLib  | 
MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+  ImagePropertiesRecordLib | 
MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
  
  !if $(HTTP_BOOT_ENABLE) == TRUE

HttpLib  | 
MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111973): https://edk2.groups.io/g/devel/message/111973
Mute This Topic: https://groups.io/mt/102906512/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 00/17] Add ImagePropertiesRecordLib Instances

2023-12-01 Thread Ard Biesheuvel
On Fri, 1 Dec 2023 at 01:34, Taylor Beebe  wrote:
>
> ImagePropertiesRecordLib is used by DxeMain and PiSmmCore, so it
> needs to be added to most platforms.
>
> Taylor Beebe (17):
>   Platform/AMD/OverdriveBoard: Add ImagePropertiesRecordLib Instance
>   Platform/ARM/Morello: Add ImagePropertiesRecordLib Instance
>   Platform/BeagleBoard/BeagleBoardPkg: Add ImagePropertiesRecordLib
> Instance
>   Platform/Intel/Vlv2TbltDevicePkg: Add ImagePropertiesRecordLib
> Instance
>   Platform/LeMaker/CelloBoard: Add ImagePropertiesRecordLib Instance
>   Platform/RaspberryPi: Add ImagePropertiesRecordLib Instance
>   Platform/Loongson/LoongArchQemuPkg: Add ImagePropertiesRecordLib
> Instance
>   Platform/Qemu/SbsaQemu: Add ImagePropertiesRecordLib Instance
>   Platform/SiFive/U5SeriesPkg: Add ImagePropertiesRecordLib Instance
>   Platform/Socionext: Add ImagePropertiesRecordLib Instance
>   Platform/SoftIron/Overdrive1000Board: Add ImagePropertiesRecordLib
> Instance
>   Silicon/NXP: Add ImagePropertiesRecordLib Instance
>   Silicon/Hisilicon: Add ImagePropertiesRecordLib Instance
>   Silicon/Marvell/Armada7k8k: Add ImagePropertiesRecordLib Instance
>   Platform/Sophgo/SG2042_EVB_Board: Add ImagePropertiesRecordLib
> Instance
>   Silicon/Ampere/AmpereAltraPkg: Add ImagePropertiesRecordLib Instance
>   Silicon/Phytium/PhytiumCommonPkg: Add ImagePropertiesRecordLib
> Instance
>

Thanks for following up.

Pushed as 10beadf30a3b..674d6cf4105e


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




Re: 回复: [edk2-devel] [PATCH v3 09/39] MdePkg: Add a new library named PeiServicesTablePointerLibReg

2023-12-01 Thread Chao Li

Hi Liming,

Ok, I see. In V4 I will add the PeiServicesTablePointerLib only for 
LoongArch, and it probably be named PeiServicesTablePointerLibKs0.



Thanks,
Chao
On 2023/12/1 08:32, gaoliming wrote:


Chao:

 I agree with Laszlo. I want to highlight that current design has meet 
with your requirement. So, I don’t think we need to add new APIs in 
PeiServicesTablePointerLib header file. Loong Arch can implement its 
PeiServicesTablePointerLib library instance base on its specific 
register.


Thanks

Liming

*发件人:*Chao Li 
*发送时间:*2023年11月27日11:28
*收件人:*Laszlo Ersek ; devel@edk2.groups.io
*抄送:*Michael D Kinney ; Liming Gao 
; Zhiguang Liu ; 
Leif Lindholm ; Ard Biesheuvel 
; Sami Mujawar ; 
Sunil V L 
*主题:*Re: [edk2-devel] [PATCH v3 09/39] MdePkg: Add a new library named 
PeiServicesTablePointerLibReg


Hi Mike and Liming,

You opinion is very important, it will decide the direction. I will 
send the V4 this week, so can you please review the new patch of 
MdePkg for this series?


Thanks,
Chao

On 2023/11/24 19:35, Laszlo Ersek wrote:

On 11/22/23 02:47, Chao Li wrote:

Hi Laszlo,

Thanks,

Chao

On 2023/11/21 22:37, Laszlo Ersek wrote:

On 11/17/23 10:59, Chao Li wrote:

Since some ARCH or platform not require execute code
on memory during

PEI phase, some values may transferred via CPU registers.

Adding PeiServcieTablePointerLibReg to allow set and
get the PEI service

table pointer depend by a CPU register, this library
can accommodate lot

of platforms who not require execte code on memory
during PEI phase.

Adding PeiServiceTablePointerLibReg to allows setting
and getting the

PEI service table pointer via CPU registers, and the
library can

accommodate many platforms that do not need to execute
code on memory

during the PEI phase.

The idea of this library is derived from

ArmPkg/Library/PeiServicesTablePointerLib/

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

Cc: Michael D Kinney 


Cc: Liming Gao 


Cc: Zhiguang Liu 


Cc: Leif Lindholm 


Cc: Ard Biesheuvel 


Cc: Sami Mujawar 


Cc: Laszlo Ersek 


Cc: Sunil V L 


Signed-off-by: Chao Li 


---

.../Library/PeiServicesTablePointerLib.h  | 37
+++-

.../PeiServicesTablePointer.c | 86
+++

.../PeiServicesTablePointerLib.uni    | 20 +

.../PeiServicesTablePointerLibReg.inf | 40
+

MdePkg/MdePkg.dsc |  1 +

5 files changed, 180 insertions(+), 4 deletions(-)

create mode 100644

MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c

create mode 100644

MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLib.uni

create mode 100644

MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLibReg.inf

In my opinion, the PeiServicesTablePointerLib class header
should not be

extended with new interfaces. I understand that the
generality is

attractive, but it is not put to use; only the loongarch
architecture

applies the new interfaces (in the subsequent patch), and
for example

the ARM code (ArmPkg/Library/PeiServicesTablePointerLib)
is not reworked

in terms of these new interfaces.

This libarary have ability of accommodate more ARCH why not? I
checked

the PI SPEC, all ARCH except IA32 and X64 using the register
mechanism,

if this library can be approved, all of them can moved into this

libraryso that code con be reused more, I think this library
is fine.

The library may be fine from a design point of view, but without

actually putting the extra generality to use, it's a waste. It's a

maintenance burden. There's a name for this anti-pattern:

Re: [edk2-devel] [PATCH] UefiCpuPkg/ResetVector: Option for 1G Page Table Size increase

2023-12-01 Thread Ni, Ray
Ashraf,
If any code wants to map above 512GB, that code can create a new mapping in 
memory or temporary ram.

I agree your patch adds flexibility to reset vector. But I just do not want the 
flexibility hurts the maintainability.


Thanks,
Ray
> -Original Message-
> From: S, Ashraf Ali 
> Sent: Friday, December 1, 2023 2:26 PM
> To: Ni, Ray ; devel@edk2.groups.io
> Cc: Kumar, Rahul R ; West, Catharine
> ; V, Sangeetha 
> Subject: RE: [PATCH] UefiCpuPkg/ResetVector: Option for 1G Page Table Size
> increase
> 
> Hi., Ray
> 
> Yes, I agree. But if any device which needs to map above 512GB in Pre
> Memory (Early PreMem).
> 
> if any device mapped any device below 512GB. And if the DRAM has 512GB
> then it would create a conflict.
> 
> So we are not changing the default page table size, keeping 512GB only. So
> that it will give room to decide the reset vector size based on the platform
> needs.
> 
> Sizes mentioned below:
> 512GB - 16KB
> 1TB  - 20KB
> 2TB  - 28KB
> 
> Thanks.,
> S, Ashraf Ali
> 
> -Original Message-
> From: Ni, Ray 
> Sent: Friday, December 1, 2023 10:58 AM
> To: S, Ashraf Ali ; devel@edk2.groups.io
> Cc: Kumar, Rahul R ; West, Catharine
> ; V, Sangeetha 
> Subject: RE: [PATCH] UefiCpuPkg/ResetVector: Option for 1G Page Table Size
> increase
> 
> Ashraf,
> When we implement the reset vector, we assume that 512GB page table
> occupies minimal flash size and also sufficient for XIP code.
> Any code that wants more page table coverage should create a new page
> table, either in the temporary ram (when physical mem is not ready) or in
> physical ram.
> 
> Why does this patch increase the default 512 GB coverage?
> 
> Thanks,
> Ray
> > -Original Message-
> > From: S, Ashraf Ali 
> > Sent: Friday, December 1, 2023 1:35 AM
> > To: devel@edk2.groups.io
> > Cc: S, Ashraf Ali ; Kumar, Rahul R
> > ; Ni, Ray ; West, Catharine
> > ; V, Sangeetha 
> > Subject: [PATCH] UefiCpuPkg/ResetVector: Option for 1G Page Table Size
> > increase
> >
> > Currently 1G Page table is restricted 512GB. this patch can help to
> > increase the page table size based on the input. default will be 512GB
> > build option PAGE_TABLE_1G_SIZE is used to increase the page table
> > size
> >
> > Cc: Rahul Kumar 
> > Cc: Ray Ni 
> > Cc: Catharine West 
> > Cc: V Sangeetha 
> > Signed-off-by: Ashraf Ali S 
> > ---
> >  UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt |  4 ++--
> >  UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf   |  2 ++
> >  UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm | 13 +++--
> >  3 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt
> > b/UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt
> > index 4fcb15c3b1..4d153fc1a7 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt
> > @@ -1,10 +1,10 @@
> >
> >  === HOW TO USE VTF0 ===
> >  Add this line to your DSC [Components.IA32] or [Components.X64] section:
> > -  UefiCpuPkg/ResetVector/Vtf0/ResetVector.inf
> > +  UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> >
> >  Add this line to your FDF FV section:
> > -  INF  RuleOverride=RESET_VECTOR
> > UefiCpuPkg/ResetVector/Vtf0/ResetVector.inf
> > +  INF  RuleOverride=RESET_VECTOR UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> >
> >  In your FDF FFS file rules sections add:
> >[Rule.Common.SEC.RESET_VECTOR]
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> > b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> > index 6b406163db..96106a4b11 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf
> > @@ -53,5 +53,7 @@
> >  #   -DARCH_X64, -DARCH_IA32
> >  #   * for using 1G page table:
> >  #   -DPAGE_TABLE_1G
> > +#   * for incresing the 1G page table size (Size in GBs):
> > +#   -DPAGE_TABLE_1G_SIZE=1024
> >  #   * for different debug channels:
> >  #   -DDEBUG_SERIAL, -DDEBUG_PORT80, or not specify any debug
> channel
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> > b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> > index 7960b141be..b080dc5296 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/X64/PageTables.asm
> > @@ -1,7 +1,7 @@
> >
> > ;-
> > -
> >  ; @file
> >  ; Emits Page Tables for 1:1 mapping.
> > -; If using 1G page table, map addresses 0 - 0x80 (512GB),
> > +; If using 1G page table, map addresses 0 - 0x80 (512GB, Size
> > +can
> > be increse via PAGE_TABLE_1G_SIZE),
> >  ; else, map addresses 0 - 0x1 (4GB)  ;  ; Copyright (c) 2021
> > - 2023, Intel Corporation. All rights reserved.
> > @@ -39,6 +39,15 @@ BITS64
> >  %define PAGE_PDPTE_1GB(x) ((x << 30) + PAGE_BLE_ATTR)  %define
> > PAGE_PDE_2MB(x) ((x << 21) + PAGE_BLE_ATTR)
> >
> > +%ifdef PAGE_TABLE_1G_SIZE
> > +  %define PAGE_PDPTE_1GB_SIZE PAGE_TABLE_1G_SIZE %else
> > +  ;
> > +  ; Default 512GB of

Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable before using it.

2023-12-01 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Liu, Zhiguang 
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
> ; Laszlo Ersek 
> Subject: [PATCH v2 1/3] UefiCpuPkg/CpuPageTableLib: Init local variable
> before using it.
> 
> The local variable OneOfPagingEntry is used before initialized, this
> may cause reserved bit in page table entry is set especially in PAE
> paging mode. The bug is random because it depends on the value in
> stack.
> 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Laszlo Ersek 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index eff02619fa..36b2c4e6a3 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -338,7 +338,7 @@ PageTableLibMapInLevel (
>ParentAttribute = &LocalParentAttribute;
> 
>OriginalParentPagingEntry.Uint64 = ParentPagingEntry->Uint64;
> -
> +  OneOfPagingEntry.Uint64  = 0;
>//
>// RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21)
> or 4K (1 << 12).
>//
> @@ -367,8 +367,6 @@ PageTableLibMapInLevel (
>if (RETURN_ERROR (Status)) {
>  return Status;
>}
> -
> -  OneOfPagingEntry.Pnle.Uint64 = 0;
>  } else {
>PageTableLibSetPle (Level, &OneOfPagingEntry, 0, &PleBAttribute,
> &AllOneMask);
>  }
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111977): https://edk2.groups.io/g/devel/message/111977
Mute This Topic: https://groups.io/mt/102889278/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 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test case for PAE paging.

2023-12-01 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Liu, Zhiguang 
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
> ; Laszlo Ersek 
> Subject: [PATCH v2 2/3] UefiCpuPkg/CpuPageTableLib/TestCase: Refine test
> case for PAE paging.
> 
> Refine test case:
> 1. Check PAE paging reserved bits is zero.
> 2. Set stack as random value.
> 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Laszlo Ersek 
> Signed-off-by: Zhiguang Liu 
> ---
>  .../CpuPageTableLib/UnitTest/RandomTest.c | 24 ++-
>  .../CpuPageTableLib/UnitTest/TestHelper.c | 14 ---
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> index f7a77d00e7..9ac3188be0 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> @@ -138,6 +138,23 @@ RandomBoolean (
>return ((Probability > ((UINT8)Random64 (0, 100))) ? TRUE : FALSE);
>  }
> 
> +/**
> +  Set 8K stack as random value.
> +**/
> +VOID
> +SetRandomStack (
> +  VOID
> +  )
> +{
> +  UINT64  Buffer[SIZE_1KB];
> +  UINTN   Index;
> +
> +  for (Index = 0; Index < SIZE_1KB; Index++) {
> +Buffer[Index] = Random64 (0, MAX_UINT64);
> +Buffer[Index] = Buffer[Index];
> +  }
> +}
> +
>  /**
>Check if the Page table entry is valid
> 
> @@ -670,6 +687,7 @@ SingleMapEntryTest (
>IsNotPresent  = FALSE;
>IsModified= FALSE;
> 
> +  SetRandomStack ();
>GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
>LastMapEntry = &MapEntrys->Maps[MapsIndex];
>Status   = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
> @@ -1039,7 +1057,11 @@ TestCaseforRandomTest (
>mSupportedBit.Bits.Pat= 1;
>mSupportedBit.Bits.Global = 1;
>mSupportedBit.Bits.ProtectionKey  = 0xF;
> -  mSupportedBit.Bits.Nx = 1;
> +  if (((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT *)Context)-
> >PagingMode == PagingPae) {
> +mSupportedBit.Bits.ProtectionKey = 0;
> +  }
> +
> +  mSupportedBit.Bits.Nx = 1;
> 
>mRandomOption = ((CPU_PAGE_TABLE_LIB_RANDOM_TEST_CONTEXT
> *)Context)->RandomOption;
>mNumberIndex  = 0;
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> index 67776255c2..d2c50a6c8a 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> @@ -9,6 +9,7 @@
>  #include "CpuPageTableLibUnitTest.h"
>  #include "../CpuPageTable.h"
> 
> +#define IA32_PAE_RESERVED_MASK  0x7FF0ull
>  //
>  // Global Data to validate if the page table is legal
>  // mValidMaskNoLeaf[0] is not used
> @@ -95,6 +96,7 @@ InitGlobalData (
>@param[in]   Level  the level of PagingEntry.
>@param[in]   MaxLeafLevel   Max leaf entry level.
>@param[in]   LinearAddress  The linear address verified.
> +  @param[in]   PagingMode The paging mode.
> 
>@retval  Leaf entry.
>  **/
> @@ -103,13 +105,18 @@ IsPageTableEntryValid (
>IN IA32_PAGING_ENTRY  *PagingEntry,
>IN UINTN  Level,
>IN UINTN  MaxLeafLevel,
> -  IN UINT64 Address
> +  IN UINT64 Address,
> +  IN PAGING_MODEPagingMode
>)
>  {
>UINT64 Index;
>IA32_PAGING_ENTRY  *ChildPageEntry;
>UNIT_TEST_STATUS   Status;
> 
> +  if (PagingMode == PagingPae) {
> +UT_ASSERT_EQUAL (PagingEntry->Uint64 & IA32_PAE_RESERVED_MASK,
> 0);
> +  }
> +
>if (PagingEntry->Pce.Present == 0) {
>  return UNIT_TEST_PASSED;
>}
> @@ -142,7 +149,7 @@ IsPageTableEntryValid (
> 
>ChildPageEntry = (IA32_PAGING_ENTRY
> *)(UINTN)(IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&PagingEntry->Pnle));
>for (Index = 0; Index < 512; Index++) {
> -Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1,
> MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)));
> +Status = IsPageTableEntryValid (&ChildPageEntry[Index], Level-1,
> MaxLeafLevel, Address + (Index<<(9*(Level-1) + 3)), PagingMode);
>  if (Status != UNIT_TEST_PASSED) {
>return Status;
>  }
> @@ -190,9 +197,10 @@ IsPageTableValid (
>  if (PagingMode == PagingPae) {
>UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero, 0);
>UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero2, 0);
> +  UT_ASSERT_EQUAL (PagingEntry[Index].PdptePae.Bits.MustBeZero3, 0);
>  }
> 
> -Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel,
> MaxLeafLevel, Index << (9 * MaxLevel + 3));
> +Status = IsPageTableEntryValid (&PagingEntry[Index], MaxLevel,
> MaxLeafLevel, Index << (9 * MaxLevel + 3), PagingMode);
>  if (Status != UNIT_TEST_PASSED) {
>retur

Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set memory attribute.

2023-12-01 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Liu, Zhiguang 
> Sent: Thursday, November 30, 2023 2:29 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang ; Ni, Ray ;
> Kumar, Rahul R ; Gerd Hoffmann
> ; Laszlo Ersek 
> Subject: [PATCH v2 3/3] UefiCpuPkg/CpuMpPei: Use CpuPageTableLib to set
> memory attribute.
> 
> Currently, there are code to set memory attribute in CpuMpPei module.
> However, the code doesn't handle the case of 5 level paging.
> Use the CpuPageTableLib to set memory attribute for two purpose:
> 1. Add 5 level paging support
> 2. Clean up code
> 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Laszlo Ersek 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/CpuMpPei/CpuPaging.c | 317 +++-
>  1 file changed, 69 insertions(+), 248 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index b7ddb0005b..2dd7237141 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -15,50 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
> 
>  #include "CpuMpPei.h"
> -
> -#define IA32_PG_P   BIT0
> -#define IA32_PG_RW  BIT1
> -#define IA32_PG_U   BIT2
> -#define IA32_PG_A   BIT5
> -#define IA32_PG_D   BIT6
> -#define IA32_PG_PS  BIT7
> -#define IA32_PG_NX  BIT63
> -
> -#define PAGE_ATTRIBUTE_BITS  (IA32_PG_RW | IA32_PG_P)
> -#define PAGE_PROGATE_BITS(IA32_PG_D | IA32_PG_A | IA32_PG_NX |
> IA32_PG_U | \
> -   PAGE_ATTRIBUTE_BITS)
> -
> -#define PAGING_PAE_INDEX_MASK0x1FF
> -#define PAGING_4K_ADDRESS_MASK_640x000FF000ull
> -#define PAGING_2M_ADDRESS_MASK_640x000FFFE0ull
> -#define PAGING_1G_ADDRESS_MASK_640x000FC000ull
> -#define PAGING_512G_ADDRESS_MASK_64  0x000FFF80ull
> -
> -typedef enum {
> -  PageNone = 0,
> -  PageMin  = 1,
> -  Page4K   = PageMin,
> -  Page2M   = 2,
> -  Page1G   = 3,
> -  Page512G = 4,
> -  PageMax  = Page512G
> -} PAGE_ATTRIBUTE;
> -
> -typedef struct {
> -  PAGE_ATTRIBUTEAttribute;
> -  UINT64Length;
> -  UINT64AddressMask;
> -  UINTN AddressBitOffset;
> -  UINTN AddressBitLength;
> -} PAGE_ATTRIBUTE_TABLE;
> -
> -PAGE_ATTRIBUTE_TABLE  mPageAttributeTable[] = {
> -  { PageNone, 0,  0,   0,  0 },
> -  { Page4K,   SIZE_4KB,   PAGING_4K_ADDRESS_MASK_64,   12, 9 },
> -  { Page2M,   SIZE_2MB,   PAGING_2M_ADDRESS_MASK_64,   21, 9 },
> -  { Page1G,   SIZE_1GB,   PAGING_1G_ADDRESS_MASK_64,   30, 9 },
> -  { Page512G, SIZE_512GB, PAGING_512G_ADDRESS_MASK_64, 39, 9 },
> -};
> +#define PAGING_4K_ADDRESS_MASK_64  0x000FF000ull
> 
>  EFI_PEI_NOTIFY_DESCRIPTOR  mPostMemNotifyList[] = {
>{
> @@ -117,237 +74,101 @@ AllocatePageTableMemory (
>return Address;
>  }
> 
> -/**
> -  Get the type of top level page table.
> -
> -  @retval Page512G  PML4 paging.
> -  @retval Page1GPAE paging.
> -
> -**/
> -PAGE_ATTRIBUTE
> -GetPageTableTopLevelType (
> -  VOID
> -  )
> -{
> -  MSR_IA32_EFER_REGISTER  MsrEfer;
> -
> -  MsrEfer.Uint64 = AsmReadMsr64 (MSR_CORE_IA32_EFER);
> -
> -  return (MsrEfer.Bits.LMA == 1) ? Page512G : Page1G;
> -}
> -
> -/**
> -  Return page table entry matching the address.
> -
> -  @param[in]   Address  The address to be checked.
> -  @param[out]  PageAttributes   The page attribute of the page entry.
> -
> -  @return The page entry.
> -**/
> -VOID *
> -GetPageTableEntry (
> -  IN  PHYSICAL_ADDRESS  Address,
> -  OUT PAGE_ATTRIBUTE*PageAttribute
> -  )
> -{
> -  INTNLevel;
> -  UINTN   Index;
> -  UINT64  *PageTable;
> -  UINT64  AddressEncMask;
> -
> -  AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
> -  PageTable  = (UINT64 *)(UINTN)(AsmReadCr3 () &
> PAGING_4K_ADDRESS_MASK_64);
> -  for (Level = (INTN)GetPageTableTopLevelType (); Level > 0; --Level) {
> -Index  = (UINTN)RShiftU64 (Address,
> mPageAttributeTable[Level].AddressBitOffset);
> -Index &= PAGING_PAE_INDEX_MASK;
> -
> -//
> -// No mapping?
> -//
> -if (PageTable[Index] == 0) {
> -  *PageAttribute = PageNone;
> -  return NULL;
> -}
> -
> -//
> -// Page memory?
> -//
> -if (((PageTable[Index] & IA32_PG_PS) != 0) || (Level == PageMin)) {
> -  *PageAttribute = (PAGE_ATTRIBUTE)Level;
> -  return &PageTable[Index];
> -}
> -
> -//
> -// Page directory or table
> -//
> -PageTable = (UINT64 *)(UINTN)(PageTable[Index] &
> -  ~AddressEncMask &
> -  PAGING_4K_ADDRESS_MASK_64);
> -  }
> -
> -  *PageAttribute = PageNone;
> -  return NULL;
> -}
> -
> -/**
> -  This function splits one page entry to smaller page entries.
> -
> -  @param[in]  PageEntryThe page entry to be splitted.
> -  @param[in]  PageAttributeThe page attribute of the page entry.
> -  @param[in]  SplitAttribute   How to split the page 

Re: [edk2-devel] [PATCH 1/1] MdePkg/IndustryStandard: Add _PSD/_CPC/Coord types definitions

2023-12-01 Thread Ni, Ray
> --- a/MdePkg/Include/IndustryStandard/Acpi50.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi50.h

> +#define EFI_ACPI_5_0_AML_PSD_REVISION  0 
> +#define EFI_ACPI_5_0_AML_CPC_REVISION  1


Do you think it's better to define EFI_ACPI_AML_PSD_REVISION and
EFI_ACPI_AML_CPC_REVISION in Acpi50.h?

So that Acpi51.h doesn't have to redefine a different macro to
the same value?

> 
> diff --git a/MdePkg/Include/IndustryStandard/Acpi51.h
> b/MdePkg/Include/IndustryStandard/Acpi51.h
> index 01ef544c3a29..19dd7b4f864c 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi51.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi51.h 

> +#define EFI_ACPI_5_1_AML_PSD_REVISION  0 
> +#define EFI_ACPI_5_1_AML_CPC_REVISION  1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111980): https://edk2.groups.io/g/devel/message/111980
Mute This Topic: https://groups.io/mt/102891569/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] RedfishPkg/HostInterfaceBmcUsbNic: Set default Redfish service port

2023-12-01 Thread Nickle Wang via groups.io


Reviewed-by: Nickle Wang 

Regards,
Nickle

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chang, Abner
> via groups.io
> Sent: Tuesday, November 28, 2023 1:26 PM
> To: devel@edk2.groups.io
> Cc: Nickle Wang ; Igor Kulchytskyy ;
> Mike Maslenkin 
> Subject: [edk2-devel] [PATCH] RedfishPkg/HostInterfaceBmcUsbNic: Set default
> Redfish service port
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> From: Abner Chang 
> 
> BZ #4607
> Create a PCD for the default Redfish service port.
> 
> Signed-off-by: Abner Chang 
> Cc: Nickle Wang 
> Cc: Igor Kulchytskyy 
> Cc: Mike Maslenkin 
> ---
>  RedfishPkg/RedfishPkg.dec | 8 ++--
>  .../PlatformHostInterfaceBmcUsbNicLib.inf | 1 +
>  .../PlatformHostInterfaceBmcUsbNicLib.c   | 2 +-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> index e40538247c2..3ea9ff3ef7f 100644
> --- a/RedfishPkg/RedfishPkg.dec
> +++ b/RedfishPkg/RedfishPkg.dec
> @@ -141,12 +141,16 @@
># specification for that.
>#
>gEfiRedfishPkgTokenSpaceGuid.PcdRedfishServiceUuid|L"--
> --"|VOID*|0x1006
> +  # Use PCD to declare the Redfish service port, default set to port 443.
> +  # Platform can overide this value in platform DSC file.
> +  #
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishServicePort|443|UINT16|0x1007
>#
># This PCD indicates that if BMC bootstrap credential service will be 
> disabled by
> BIOS or not.
>#
> -
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDisableBootstrapCredentialService|FA
> LSE|BOOLEAN|0x1007
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDisableBootstrapCredentialService|FA
> LSE|BOOLEAN|0x1008
>#
># The EFI_REST_EX_HTTP_CONFIG_DATA.SendReceiveTimeout value that
> RedfishDiscoverDxe driver
># set to EFI_REST_EX_PROTOCOL.
>#
> -
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishSendReceiveTimeout|5000|UINT32|0
> x1008
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishSendReceiveTimeout|5000|UINT32|0
> x1009
> diff --git
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.inf
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.inf
> index f2c7d7fec89..838a1721a7a 100644
> ---
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.inf
> +++
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.inf
> @@ -43,6 +43,7 @@
>  [Pcd]
>gEfiRedfishPkgTokenSpaceGuid.PcdRedfishHostName ## CONSUMED
>gEfiRedfishPkgTokenSpaceGuid.PcdRedfishServiceUuid  ## CONSUMED
> +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishServicePort  ## CONSUMED
> 
>  [Depex]
>gIpmiProtocolGuid
> diff --git
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.c
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.c
> index 2938d54da65..7f295fe7f1c 100644
> ---
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.c
> +++
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfac
> eBmcUsbNicLib.c
> @@ -244,7 +244,7 @@ RedfishPlatformHostInterfaceProtocolData (
>  );
> 
>// RedfishServiceIpPort
> -  RedfishOverIpData->RedfishServiceIpPort = 0;
> +  RedfishOverIpData->RedfishServiceIpPort = PcdGet16
> (PcdRedfishServicePort);
> 
>// RedfishServiceVlanId
>RedfishOverIpData->RedfishServiceVlanId = ThisInstance->VLanId;
> --
> 2.37.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111981): https://edk2.groups.io/g/devel/message/111981
Mute This Topic: https://groups.io/mt/102845949/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] MdePkg/IndustryStandard: Add _PSD/_CPC/Coord types definitions

2023-12-01 Thread PierreGondois

Hi Ray,
I followed the way revisions are defined for ACPI tables revisions,
like for the MADT:
EFI_ACPI_x_x_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION definition

For each ACPI spec. revision, there is a matching MADT revision number.
Sometimes the table has changed and the revision is upgraded, sometimes
not and the revision stays the same.
This also means having a macro definition for each ACPI spec. header
file. I think it should be ok do to the same thing for _PSD/_CPC
revisions,

Regards,
Pierre

On 12/1/23 11:22, Ni, Ray wrote:

--- a/MdePkg/Include/IndustryStandard/Acpi50.h
+++ b/MdePkg/Include/IndustryStandard/Acpi50.h



+#define EFI_ACPI_5_0_AML_PSD_REVISION  0
+#define EFI_ACPI_5_0_AML_CPC_REVISION  1



Do you think it's better to define EFI_ACPI_AML_PSD_REVISION and
EFI_ACPI_AML_CPC_REVISION in Acpi50.h?

So that Acpi51.h doesn't have to redefine a different macro to
the same value?



diff --git a/MdePkg/Include/IndustryStandard/Acpi51.h
b/MdePkg/Include/IndustryStandard/Acpi51.h
index 01ef544c3a29..19dd7b4f864c 100644
--- a/MdePkg/Include/IndustryStandard/Acpi51.h
+++ b/MdePkg/Include/IndustryStandard/Acpi51.h



+#define EFI_ACPI_5_1_AML_PSD_REVISION  0
+#define EFI_ACPI_5_1_AML_CPC_REVISION  1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111982): https://edk2.groups.io/g/devel/message/111982
Mute This Topic: https://groups.io/mt/102891569/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] UnitTestFrameworkPkg: Fix Google Test components with multiple files

2023-12-01 Thread Michael Kubacki

Hi Pedro,

Visual Studio NOOPT builds result in linker errors. I combined your 
patch series with the test instruction change in this PR - 
https://github.com/tianocore/edk2/pull/5096.


You can use a PR to test the VS build.

Thanks,
Michael

On 11/30/2023 5:42 PM, Pedro Falcato wrote:

Google Test hides test registration in global constructors on global
objects. Global constructors are traditionally implemented by placing
references to the global constructor's symbol in special sections
(traditionally named .ctors or .init_array). These sections are not
explicitly referenced by the linker, and libc only looks at special
start and end symbols (and calls them).

This works fine if you're linking a program manually using

 gcc a.o b.o c.o -o test_suite

but fails miserably when using static libraries (such as what EDK2
does), because traditional static archive symbol resolution rules don't
allow for object files to be pulled in to the link if there isn't an
undefined symbol reference to that .o elsewhere.

Fix it by passing --whole-archive (GCC) and /WHOLEARCHIVE (MSVC). These
options force the linker to pull in the entire static library, thus
including previously-unreferenced constructors and making sure
multi-file gtest EDK2 components work.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4610
Signed-off-by: Pedro Falcato 
Cc: Michael D Kinney 
Cc: Michael Kubacki 
Cc: Sean Brogan 
---
  Note: /WHOLEARCHIVE should work for VS2015 and newer. I haven't been able to
test it, so if someone could test on msft it would be much appreciated.
  Testing is as simple as taking this patch set
  (https://openfw.io/edk2-devel/20231130024611.67135-1-pedro.falc...@gmail.com/)
  and removing the #include "TestCheckSum.cpp" in TestBaseLibMain.cpp. If 
everything worked correctly,
  you should have 4 tests and not 0.

  UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc 
b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
index 5217de31e491..0f11706e7324 100644
--- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
+++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
@@ -37,7 +37,7 @@
# MSFT
#
MSFT:*_*_*_CC_FLAGS   = /EHsc
-  MSFT:*_*_*_DLINK_FLAGS== /out:"$(BIN_DIR)\$(MODULE_NAME_GUID).exe" 
/pdb:"$(BIN_DIR)\$(MODULE_NAME_GUID).pdb" /IGNORE:4001 /NOLOGO /SUBSYSTEM:CONSOLE /DEBUG 
/STACK:0x4,0x4 /NODEFAULTLIB:libcmt.lib libcmtd.lib
+  MSFT:*_*_*_DLINK_FLAGS== /out:"$(BIN_DIR)\$(MODULE_NAME_GUID).exe" 
/pdb:"$(BIN_DIR)\$(MODULE_NAME_GUID).pdb" /IGNORE:4001 /NOLOGO /SUBSYSTEM:CONSOLE /DEBUG 
/STACK:0x4,0x4 /NODEFAULTLIB:libcmt.lib libcmtd.lib /WHOLEARCHIVE
MSFT:*_*_IA32_DLINK_FLAGS = /MACHINE:I386
MSFT:*_*_X64_DLINK_FLAGS  = /MACHINE:AMD64
  
@@ -56,7 +56,12 @@

#
GCC:*_*_IA32_DLINK_FLAGS == -o $(BIN_DIR)/$(MODULE_NAME_GUID) -m32 -no-pie
GCC:*_*_X64_DLINK_FLAGS  == -o $(BIN_DIR)/$(MODULE_NAME_GUID) -m64 -no-pie
-  GCC:*_*_*_DLINK2_FLAGS   == -lgcov -lpthread -lstdc++ -lm
+  #
+  # Surround our static libraries with whole-archive, so constructor-based 
test registration works properly.
+  # Note that we need to --no-whole-archive before linking system libraries.
+  #
+  GCC:*_*_*_DLINK_FLAGS= -Wl,--whole-archive
+  GCC:*_*_*_DLINK2_FLAGS   == -Wl,--no-whole-archive -lgcov -lpthread -lstdc++ 
-lm
  
#

# Need to do this link via gcc and not ld as the pathing to libraries 
changes from OS version to OS version



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111983): https://edk2.groups.io/g/devel/message/111983
Mute This Topic: https://groups.io/mt/102904623/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] UnitTestFrameworkPkg: Fix Google Test components with multiple files

2023-12-01 Thread Pedro Falcato
On Fri, Dec 1, 2023 at 5:07 PM Michael Kubacki
 wrote:
>
> Hi Pedro,
>
> Visual Studio NOOPT builds result in linker errors. I combined your
> patch series with the test instruction change in this PR -
> https://github.com/tianocore/edk2/pull/5096.
>
> You can use a PR to test the VS build.

Thanks for the heads up, but I ended up booting Windows to expedite the process.

So, I noticed from the build logs that libcmtd.lib was having issues
doing a /WHOLEARCHIVE link (not unheard of, had the same problems with
Linux system libraries). Then I noticed in MSDN:
"The /WHOLEARCHIVE option forces the linker to include every object
file from either a specified static library, or if no library is
specified, from all static libraries specified to the LINK command"
Note the "from all static libraries specified to the LINK command". So
I noticed libcmtd.lib was being specified manually, and I simply
deleted

/NODEFAULTLIB:libcmt.lib libcmtd.lib

>From line 40 of UnitTestFrameworkPkgHost.dsc.inc.
So, before I submit a v2 of this, does anyone know why this was added
manually? Mike?

Note: I tried to add /MTd, but that seems to be a cl.exe option, not link.exe

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111985): https://edk2.groups.io/g/devel/message/111985
Mute This Topic: https://groups.io/mt/102904623/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] UnitTestFrameworkPkg: Fix Google Test components with multiple files

2023-12-01 Thread Pedro Falcato
On Fri, Dec 1, 2023 at 8:50 PM Pedro Falcato via groups.io
 wrote:
>
> On Fri, Dec 1, 2023 at 5:07 PM Michael Kubacki
>  wrote:
> >
> > Hi Pedro,
> >
> > Visual Studio NOOPT builds result in linker errors. I combined your
> > patch series with the test instruction change in this PR -
> > https://github.com/tianocore/edk2/pull/5096.
> >
> > You can use a PR to test the VS build.
>
> Thanks for the heads up, but I ended up booting Windows to expedite the 
> process.
>
> So, I noticed from the build logs that libcmtd.lib was having issues
> doing a /WHOLEARCHIVE link (not unheard of, had the same problems with
> Linux system libraries). Then I noticed in MSDN:
> "The /WHOLEARCHIVE option forces the linker to include every object
> file from either a specified static library, or if no library is
> specified, from all static libraries specified to the LINK command"
> Note the "from all static libraries specified to the LINK command". So
> I noticed libcmtd.lib was being specified manually, and I simply
> deleted
>
> /NODEFAULTLIB:libcmt.lib libcmtd.lib

... Forgot to mention that deleting this line allows the link to
complete and /WHOLEARCHIVE has the intended effect.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111986): https://edk2.groups.io/g/devel/message/111986
Mute This Topic: https://groups.io/mt/102904623/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] NetworkPkg: Triger regularly scan only if not connect to AP

2023-12-01 Thread Saloni Kasbekar
Reviewed-by: Kasbekar, Saloni 

Thanks,
Saloni

-Original Message-
From: Luo, Heng  
Sent: Monday, November 27, 2023 7:07 PM
To: devel@edk2.groups.io
Cc: Kasbekar, Saloni ; Clark-williams, Zachary 

Subject: [PATCH v2] NetworkPkg: Triger regularly scan only if not connect to AP

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

When UEFI Wi-Fi is in BSS connected state, the platform is considered as a 
static and Wi-Fi roaming support is not needed.
Wifi connection manager should not initiate Scan requests in this state affect 
BSS client connectivity and must be avoided.
Triger regularly scan only if not connect to AP.

Signed-off-by: Heng Luo 
Cc: Saloni Kasbekar 
Cc: Zachary Clark-williams 
---
 NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c 
b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
index d1182e52bd..4c5460b65c 100644
--- a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
+++ b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
@@ -1506,8 +1506,8 @@ WifiMgrOnTimerTick (
   }Nic->ScanTickTime++;-  if (((Nic->ScanTickTime > WIFI_SCAN_FREQUENCY) 
|| Nic->OneTimeScanRequest) &&-  (Nic->ScanState == WifiMgrScanFinished))+  
if Nic->ScanTickTime > WIFI_SCAN_FREQUENCY) && (Nic->ConnectState != 
WifiMgrConnectedToAp)) ||+   Nic->OneTimeScanRequest) && (Nic->ScanState == 
WifiMgrScanFinished))   { Nic->OneTimeScanRequest = FALSE; 
Nic->ScanTickTime   = 0;-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111987): https://edk2.groups.io/g/devel/message/111987
Mute This Topic: https://groups.io/mt/102844565/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] UnitTestFrameworkPkg: Fix Google Test components with multiple files

2023-12-01 Thread Michael D Kinney
Hi Pedro,

Thanks for the follow up and debug.

I suspect some of those flags were inherited from
EmulatorPkg builds and may not apply to GoogeTest
builds.  

I will investigate.

Very happy to see a solution for this issue.

Mike

> -Original Message-
> From: Pedro Falcato 
> Sent: Friday, December 1, 2023 12:53 PM
> To: devel@edk2.groups.io; pedro.falc...@gmail.com
> Cc: mikub...@linux.microsoft.com; Kinney, Michael D
> ; Sean Brogan 
> Subject: Re: [edk2-devel] [PATCH 1/2] UnitTestFrameworkPkg: Fix Google Test
> components with multiple files
> 
> On Fri, Dec 1, 2023 at 8:50 PM Pedro Falcato via groups.io
>  wrote:
> >
> > On Fri, Dec 1, 2023 at 5:07 PM Michael Kubacki
> >  wrote:
> > >
> > > Hi Pedro,
> > >
> > > Visual Studio NOOPT builds result in linker errors. I combined your
> > > patch series with the test instruction change in this PR -
> > > https://github.com/tianocore/edk2/pull/5096.
> > >
> > > You can use a PR to test the VS build.
> >
> > Thanks for the heads up, but I ended up booting Windows to expedite the
> process.
> >
> > So, I noticed from the build logs that libcmtd.lib was having issues
> > doing a /WHOLEARCHIVE link (not unheard of, had the same problems with
> > Linux system libraries). Then I noticed in MSDN:
> > "The /WHOLEARCHIVE option forces the linker to include every object
> > file from either a specified static library, or if no library is
> > specified, from all static libraries specified to the LINK command"
> > Note the "from all static libraries specified to the LINK command". So
> > I noticed libcmtd.lib was being specified manually, and I simply
> > deleted
> >
> > /NODEFAULTLIB:libcmt.lib libcmtd.lib
> 
> ... Forgot to mention that deleting this line allows the link to
> complete and /WHOLEARCHIVE has the intended effect.
> 
> --
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111988): https://edk2.groups.io/g/devel/message/111988
Mute This Topic: https://groups.io/mt/102904623/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 edk2-platforms v2 1/1] Platform/ARM: Fix the build failure DxeCore

2023-12-01 Thread nuno . lopes
Tested for Reference Design Platforms.

Tested-by: Nuno Lopes < nuno.lo...@arm.com ( nuno.lo...@arm.com ) >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111989): https://edk2.groups.io/g/devel/message/111989
Mute This Topic: https://groups.io/mt/102894400/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 1/9] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm

2023-12-01 Thread Igor Kulchytskyy via groups.io
Reviewed-by: Igor Kulchytskyy 

Regards,
Igor

-Original Message-
From: abner.ch...@amd.com 
Sent: Monday, November 27, 2023 12:31 AM
To: devel@edk2.groups.io
Cc: Mike Maslenkin ; Nickle Wang 
; Igor Kulchytskyy 
Subject: [EXTERNAL] [PATCH V3 1/9] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC 
searching algorithm


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

From: Abner Chang 

Update BMC USB NIC searching algorithm for IPv4 only.

Signed-off-by: Abner Chang 
Co-authored-by: Mike Maslenkin 
Cc: Nickle Wang 
Cc: Igor Kulchytskyy 
Cc: Mike Maslenkin 
---
 .../PlatformHostInterfaceBmcUsbNicLib.c   | 188 --
 1 file changed, 128 insertions(+), 60 deletions(-)

diff --git 
a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
 
b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
index 95900579118..e5bf70cfd58 100644
--- 
a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
+++ 
b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
@@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo (
 ));
   CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID 
*)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
   //
-  // According to UEFI spec, the IP address at BMC USB NIC host end is the 
IP address at BMC end minus 1.
+  // According to the design spec:
+  // 
https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+  // The IP address at BMC USB NIC host end is the IP address at BMC end 
minus 1.
   //
   CopyMem ((VOID *)&ThisInstance->HostIpAddressIpv4, (VOID 
*)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
   ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance->HostIpAddressIpv4) 
- 1] -= 1;
@@ -729,8 +731,10 @@ HostInterfaceIpmiCheckMacAddress (

   //
   // According to design spec in Readme file under RedfishPkg.
-  // Compare the first five MAC address and
-  // the 6th MAC address.
+  // 
https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+  // Compare the first five elements of MAC address and the 6th element of 
MAC address.
+  // The 6th element of MAC address must be the 6th element of
+  // IPMI channel MAC address minus 1.
   //
   if ((IpmiLanMacAddressSize != UsbNicInfo->MacAddressSize) ||
   (CompareMem (
@@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress (
  (VOID *)&IpmiLanChannelMacAddress.Addr,
  IpmiLanMacAddressSize - 1
  ) != 0) ||
-  (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=
-   *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1)
+  ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] - 1) !=
+   *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1))
   )
   {
 DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address is not 
matched.\n"));
@@ -962,6 +966,49 @@ UsbNicSearchUsbIo (
   return EFI_NOT_FOUND;
 }

+/**
+  This function identifies if the USB NIC has MAC address and internet
+  protocol device path installed. (Only support IPv4)
+
+  @param[in] UsbDevicePath USB device path.
+
+  @retval EFI_SUCCESS  Yes, this is IPv4 SNP handle
+  @retval EFI_NOT_FOUNDNo, this is not IPv4 SNP handle
+
+**/
+EFI_STATUS
+IdentifyNetworkMessageDevicePath (
+  IN EFI_DEVICE_PATH_PROTOCOL  *UsbDevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = UsbDevicePath;
+  while (TRUE) {
+DevicePath = NextDevicePathNode (DevicePath);
+if (IsDevicePathEnd (DevicePath)) {
+  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address device path is not 
found on this handle.\n"));
+  break;
+}
+
+if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == 
MSG_MAC_ADDR_DP)) {
+  DevicePath = NextDevicePathNode (DevicePath); // Advance to next device 
path protocol.
+  if (IsDevicePathEnd (DevicePath)) {
+DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not found 
on this handle.\n"));
+break;
+  }
+
+  if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType 
== MSG_IPv4_DP)) {
+return EFI_SUCCESS;
+  }
+
+  break;
+}
+  }
+
+  return EFI_NOT_FOUND;
+}
+
 /**
   This function identifies if the USB NIC is exposed by BMC as
   the host-BMC channel.
@@ -1025,7 +1072,7 @@ IdentifyUsbNicBmcChannel (
 (VOID *)&Snp->Mode->CurrentAddress,
 BmcUsbNic->MacAddressSize
 );
-  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address (in size %d) for this 
SNP instance:\n  ", BmcUsbNic->MacAddressSize));
+  DEBUG ((DEBUG_REDFISH_HOST_INT

[edk2-devel] Question regarding the PI timer architectural protocols

2023-12-01 Thread Pedro Falcato
Hi,

I'm trying to write an EFI application[1] to sample the program
counter, using a timer. I want to grab the EFI_SYSTEM_CONTEXT (for
sampling) on timer interrupts. However:

1) Regular event-based SetTimer does not pass me this information
2) EFI timer architectural protocols also do not seem to pass this.
EFI_TIMER_ARCH_PROTOCOL.RegisterHandler() takes a VOID
(EFIAPI *EFI_TIMER_NOTIFY) (
  IN UINT64  Time
  );

AKA not good.

Am I missing something? Is the only option to register my own
interrupt handler using EFI_CPU_ARCH_PROTOCOL, while using another
timer and praying that the firmware isn't using it itself?

[1] I would really prefer to keep it an EFI application, even while
using DXE protocols, as I'd like for this to be usable without needing
to rebuild firmware

-- 
Pedro


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