Re: [edk2-devel] Memory Attribute for depex section

2024-01-17 Thread Nhi Pham via groups.io

Hi Laszlo,

On 1/16/2024 2:00 AM, Laszlo Ersek wrote:

On 1/15/24 15:04, Ard Biesheuvel wrote:

On Mon, 15 Jan 2024 at 14:07, Nhi Pham  wrote:


On 1/12/2024 4:45 PM, Laszlo Ersek wrote:

(Independently: I think that's a valid thing to do for *SMM* drivers,
because the entry point functions of those drivers are permitted to use
both SMM and DXE/UEFI protocols. But whether the same is valid for the
*standalone* MM drivers -- that looks questionable. Standalone MM
drivers should not depend on UEFI/DXE protocols ever, IIUC.)


3) The issue is patching the grammar in place, why can’t we just make a
copy for the dispatcher grammer, and operate on the copy. Maybe via a
copy on 1st update strategy?


Yes, copying the depex to the heap, and patching it there, was Nhi's #1
fix proposal. I think that could be made work. But I'm not sure if the
perf savings are worth the additional complexity. The heap allocation
(where the writeable depex would exist) would have to be permanently
associated with the loaded PE image -- because the dispatcher might need
to reevaluate the depex across multiple rounds of dispatching. So that's
a new field in some image-related structure, it also needs to be freed
upon unload (?), what if the memory allocation fails during depex eval
(just consider the depex to eval to FALSE?), etc. Doable, but hairy; not
sure if the perf is worth that effort.



Thanks so much, Laszlo for your valuable insights.

The approach #1 works for me. I will do further check for your concerns
above.

I'm trying your suggested patch and investigating the performance being
discussed here.



Not sure what approach #1 means,


(copying the depex to the heap, and maintaining it there, so that it can
be patched)


Thanks!




but I'd prefer to just remove this
optimization from standalone MM, given that not only a) it shouldn't
have to deal with a large number of protocol GUIDs, but also b) the
driver dispatch is much more straight-forward. (Typically, StMM
drivers can be dispatched in the order they appear in the firmware
volume, in which case each DEPEX is evaluated only once anyway)


Sounds like a promising basis for removing the optimization indeed!



Your patch suggested earlier works for me. And I don't see significant 
performance reduction compared with keeping optimization.


I don't have strong reason on removing the optimization, but I think it 
would be simply good for now. Could you post your patch to edk2-devel 
for review and merge?


Thanks,
Nhi


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




Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/CpuMpPei: Don't write CR3 in ConvertMemoryPageToNotPresent

2024-01-17 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, January 17, 2024 6:24 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Ni, Ray ; Kumar, Rahul R ;
> Gerd Hoffmann 
> Subject: Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/CpuMpPei: Don't write
> CR3 in ConvertMemoryPageToNotPresent
> 
> On 1/17/24 07:22, Zhiguang Liu wrote:
> > The purpose of writing CR3 in ConvertMemoryPageToNotPresent is just
> > to flush TLB, because CR3 won't be changed in function
> > ConvertMemoryPageToNotPresent.
> > After ConvertMemoryPageToNotPresent, there is always a flush TLB
> > function. Also, because ConvertMemoryPageToNotPresent in called in a
> > loop, to improve performance, there is no need to flush TLB
> > inside ConvertMemoryPageToNotPresent. Just flushing TLB after the loop
> > is enough.
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Cc: Gerd Hoffmann 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  UefiCpuPkg/CpuMpPei/CpuPaging.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > index 15c7015fb8..b923d9b502 100644
> > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> > @@ -76,7 +76,8 @@ AllocatePageTableMemory (
> >
> >  /**
> >This function modifies the page attributes for the memory region
> specified
> > -  by BaseAddress and Length to not present.
> > +  by BaseAddress and Length to not present. This function only change
> page
> > +  table, but not flush TLB. Caller have the responsbility to flush TLB.
> >
> >Caller should make sure BaseAddress and Length is at page boundary.
> >
> > @@ -167,7 +168,6 @@ ConvertMemoryPageToNotPresent (
> >}
> >
> >ASSERT_EFI_ERROR (Status);
> > -  AsmWriteCr3 (PageTable);
> >return Status;
> >  }
> >
> 
> Reviewed-by: Laszlo Ersek 



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




Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/CpuPageTableLib: Enhance function header for PageTableMap()

2024-01-17 Thread Ni, Ray
Reviewed-by: Ray Ni 

Thanks,
Ray
> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, January 17, 2024 6:23 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Ni, Ray ; Kumar, Rahul R ;
> Gerd Hoffmann 
> Subject: Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/CpuPageTableLib:
> Enhance function header for PageTableMap()
> 
> On 1/17/24 07:21, Zhiguang Liu wrote:
> > PageTableMap() only modifies the PageTable root pointer when creating
> from zero.
> > Explicitly explain it in function header.
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Cc: Gerd Hoffmann 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  UefiCpuPkg/Include/Library/CpuPageTableLib.h | 1 +
> >  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> > index 78aa83b8de..755c8ab084 100644
> > --- a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> > +++ b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> > @@ -61,6 +61,7 @@ typedef enum {
> >Create or update page table to map [LinearAddress, LinearAddress +
> Length) with specified attribute.
> >
> >@param[in, out] PageTable  The pointer to the page table to
> update, or pointer to NULL if a new page table is to be created.
> > + If not pointer to NULL, the value
> it points to won't be changed in this function.
> >@param[in]  PagingMode The paging mode.
> >@param[in]  Buffer The free buffer to be used for page
> table creation/updating.
> >@param[in, out] BufferSize The buffer size.
> > diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > index 36b2c4e6a3..25bd9fc8d8 100644
> > --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > @@ -620,6 +620,7 @@ PageTableLibMapInLevel (
> >Create or update page table to map [LinearAddress, LinearAddress +
> Length) with specified attribute.
> >
> >@param[in, out] PageTable  The pointer to the page table to
> update, or pointer to NULL if a new page table is to be created.
> > + If not pointer to NULL, the value
> it points to won't be changed in this function.
> >@param[in]  PagingMode The paging mode.
> >@param[in]  Buffer The free buffer to be used for page
> table creation/updating.
> >@param[in, out] BufferSize The buffer size.
> 
> Reviewed-by: Laszlo Ersek 
> 
> (applying this is going to be difficult, because the patch emails were
> not sent in response to a common cover letter, or in response to each other)



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




[edk2-devel] [PATCH RESEND v2 2/2] UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu topology

2024-01-17 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

This patch adds support for AMD's new extended topology.
If processor supports CPUID 8026 leaf then obtain
the topology information using new method.

Algorithm:
  if CPUID is AMD:
then
 check for AMD's extended cpu tology leaf.
 if yes
   then extract cpu tology based on
   AMD programmer manual's instruction.
 else
   then fallback to existing topology function.
endif
  endif

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Abdul Lateef Attar 
---
 .../Library/BaseXApicLib/BaseXApicLib.c   | 126 +-
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 126 +-
 2 files changed, 250 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c 
b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index efb9d71ca1..c4457d98b3 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -4,7 +4,7 @@
   This local APIC library instance supports xAPIC mode only.
 
   Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
-  Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.
+  Copyright (c) 2017 - 2024, AMD Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1157,6 +1157,125 @@ GetProcessorLocationByApicId (
   }
 }
 
+/**
+  Get Package ID/Die ID/Module ID/Core ID/Thread ID of a AMD processor family.
+
+  The algorithm assumes the target system has symmetry across physical
+  package boundaries with respect to the number of threads per core, number of
+  cores per module, number of modules per die, number
+  of dies per package.
+
+  @param[in]   InitialApicId Initial APIC ID of the target logical processor.
+  @param[out]  Package   Returns the processor package ID.
+  @param[out]  Die   Returns the processor die ID.
+  @param[out]  Tile  Returns zero.
+  @param[out]  ModuleReturns the processor module ID.
+  @param[out]  Core  Returns the processor core ID.
+  @param[out]  ThreadReturns the processor thread ID.
+**/
+VOID
+AmdGetProcessorLocation2ByApicId (
+  IN  UINT32  InitialApicId,
+  OUT UINT32  *Package  OPTIONAL,
+  OUT UINT32  *Die  OPTIONAL,
+  OUT UINT32  *Tile OPTIONAL,
+  OUT UINT32  *Module   OPTIONAL,
+  OUT UINT32  *Core OPTIONAL,
+  OUT UINT32  *Thread   OPTIONAL
+  )
+{
+  CPUID_EXTENDED_TOPOLOGY_EAX  ExtendedTopologyEax;
+  CPUID_EXTENDED_TOPOLOGY_EBX  ExtendedTopologyEbx;
+  CPUID_EXTENDED_TOPOLOGY_ECX  ExtendedTopologyEcx;
+  UINT32   MaxExtendedCpuIdIndex;
+  UINT32   TopologyLevel;
+  UINT32   PreviousLevel;
+  UINT32   Data;
+
+  if (Die != NULL) {
+*Die = 0;
+  }
+
+  if (Tile != NULL) {
+*Tile = 0;
+  }
+
+  if (Module != NULL) {
+*Module = 0;
+  }
+
+  PreviousLevel = 0;
+  TopologyLevel = 0;
+
+  /// Check if extended toplogy supported
+  AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, NULL);
+  if (MaxExtendedCpuIdIndex >= AMD_CPUID_EXTENDED_TOPOLOGY) {
+do {
+  AsmCpuidEx (
+AMD_CPUID_EXTENDED_TOPOLOGY,
+TopologyLevel,
+,
+,
+,
+NULL
+);
+
+  if (ExtendedTopologyEbx.Bits.LogicalProcessors == 
CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID) {
+/// if this fails at first level
+/// then will fall back to non-extended topology
+break;
+  }
+
+  Data  = InitialApicId >> PreviousLevel;
+  Data &= (1 << (ExtendedTopologyEax.Bits.ApicIdShift - PreviousLevel)) - 
1;
+
+  switch (ExtendedTopologyEcx.Bits.LevelType) {
+case CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT:
+  if (Thread != NULL) {
+*Thread = Data;
+  }
+
+  break;
+case CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_CORE:
+  if (Core != NULL) {
+*Core = Data;
+  }
+
+  break;
+case CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_MODULE:
+  if (Module != NULL) {
+*Module = Data;
+  }
+
+  break;
+case CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_TILE:
+  if (Die != NULL) {
+*Die = Data;
+  }
+
+  break;
+default:
+  break;
+  }
+
+  TopologyLevel++;
+  PreviousLevel = ExtendedTopologyEax.Bits.ApicIdShift;
+} while (ExtendedTopologyEbx.Bits.LogicalProcessors != 
CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID);
+
+if (Package != NULL) {
+  *Package = InitialApicId >> PreviousLevel;
+}
+  }
+
+  /// If extended topology CPUID is not supported
+  /// OR, execution of AMD_CPUID_EXTENDED_TOPOLOGY at level 0 fails(return 0).
+  if (TopologyLevel == 0) {
+GetProcessorLocationByApicId (InitialApicId, Package, Core, Thread);
+  }
+
+  return;
+}
+
 /**
   Get Package ID/Die ID/Tile ID/Module ID/Core ID/Thread ID of a 

[edk2-devel] [PATCH RESEND v2 0/2] AMD CPU extended topology

2024-01-17 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

Resending the patch with updated Cc list.

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

V2: delta changes
 - Added additional check apart from supported CPUID extneded number.
 - removed EFIAPI

Implements AMD extended toplogy.
Adds CPUID macro
update APIC library to use new exteded cpuid.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Abdul Lateef Attar 

Abdul Lateef Attar (2):
  MdePkg: Adds AMD Extended CPU topology CPUID
  UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu topology

 MdePkg/Include/Register/Amd/Cpuid.h   |  23 +++-
 .../Library/BaseXApicLib/BaseXApicLib.c   | 126 +-
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 126 +-
 3 files changed, 272 insertions(+), 3 deletions(-)

-- 
2.34.1



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




[edk2-devel] [PATCH RESEND v2 1/2] MdePkg: Adds AMD Extended CPU topology CPUID

2024-01-17 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

Adds cpuid macro for AMD extended CPU topology.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Abdul Lateef Attar 
---
 MdePkg/Include/Register/Amd/Cpuid.h | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Register/Amd/Cpuid.h 
b/MdePkg/Include/Register/Amd/Cpuid.h
index 44394fc7a4..add43c40aa 100644
--- a/MdePkg/Include/Register/Amd/Cpuid.h
+++ b/MdePkg/Include/Register/Amd/Cpuid.h
@@ -6,7 +6,7 @@
   If a register returned is a single 32-bit value, then a data structure is
   not provided for that register.
 
-  Copyright (c) 2017, Advanced Micro Devices. All rights reserved.
+  Copyright (c) 2017 - 2024, Advanced Micro Devices. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -42,6 +42,27 @@ CPUID Signature Information
 /// @}
 ///
 
+/**
+  CPUID Extended Topology Enumeration
+
+  @note
+  Reference: AMD64 Architecture Programmer’s Manual Volume 3: General-Purpose 
and System Instructions,
+ Revision 3.35 Appendix E,
+  E.4.24 Function 8000_0026—Extended CPU Topology:
+CPUID Fn8000_0026 reports extended topology information for logical 
processors, including
+asymmetric and heterogenous topology descriptions. Individual logical 
processors may report
+different values in systems with asynchronous and heterogeneous topologies.
+The topology level is selected by the value passed to the instruction in 
ECX. To discover the topology
+of a system, software should execute CPUID Fn8000_0026 with increasing ECX 
values, starting with
+a value of zero, until the returned hierarchy level type (CPUID 
Fn8000_0026_ECX[LevelType]) is
+equal to zero. It is not guaranteed that all topology level types are 
present in the system
+
+  @param   EAX  AMD_CPUID_EXTENDED_TOPOLOGY   (0x8026)
+  @param   ECX  Level number
+
+**/
+#define AMD_CPUID_EXTENDED_TOPOLOGY  0x8026
+
 /**
   CPUID Extended Processor Signature and Features
 
-- 
2.34.1



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




Re: [edk2-devel] [PATCH 0/3] DxeTpm and DxeTpm2MeasureBootLib symbol rename

2024-01-17 Thread Yao, Jiewen
Thank you Doug for the prompt response.

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Douglas Flick [MSFT] 
> Sent: Thursday, January 18, 2024 6:47 AM
> To: devel@edk2.groups.io
> Cc: Douglas Flick [MSFT] ; Yao, Jiewen
> ; Kumar, Rahul R 
> Subject: [PATCH 0/3] DxeTpm and DxeTpm2MeasureBootLib symbol rename
> 
> OVMF is failing because it includes both DxeTpm2MeasureBootLib and
> DxeTpm2MeasureBootLib which makes the symbols collide. This patch
> renames the function names to be unique to avoid symbol collision.
> 
> Cc: Jiewen Yao 
> Cc: Rahul Kumar 
> 
> Signed-off-by: Doug Flick [MSFT] 
> 
> Douglas Flick [MSFT] (3):
>   SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117/4118 symbol
> rename
>   SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117/4118 symbol
> rename
>   SecurityPkg: : Updating SecurityFixes.yaml after symbol rename
> 
>  .../DxeTpm2MeasureBootLibSanitization.h   |  8 +++---
>  .../DxeTpmMeasureBootLibSanitization.h|  8 +++---
>  .../DxeTpm2MeasureBootLib.c   |  8 +++---
>  .../DxeTpm2MeasureBootLibSanitization.c   |  8 +++---
>  .../DxeTpm2MeasureBootLibSanitizationTest.c   | 26 -
>  .../DxeTpmMeasureBootLib.c|  8 +++---
>  .../DxeTpmMeasureBootLibSanitization.c| 10 +++
>  .../DxeTpmMeasureBootLibSanitizationTest.c| 26 -
>  SecurityPkg/SecurityFixes.yaml| 28 +++
>  9 files changed, 68 insertions(+), 62 deletions(-)
> 
> --
> 2.43.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113980): https://edk2.groups.io/g/devel/message/113980
Mute This Topic: https://groups.io/mt/103797461/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] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-17 Thread Zhiguang Liu
Thanks Laszlo for the comment, I will send a new version of patch to fix this.

Also include Pedro to see if Pedro have more comments.

Thanks
Zhiguang

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: Wednesday, January 17, 2024 6:51 PM
> To: devel@edk2.groups.io; Liu, Zhiguang 
> Cc: Ni, Ray ; Kumar, Rahul R ;
> Gerd Hoffmann ; Lee, Crystal 
> Subject: Re: [edk2-devel] [PATCH v2] UefiCpuPkg: Fix issue that IsModified is
> wrongly set in PageTableMap
> 
> On 1/17/24 09:09, Zhiguang Liu wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614
> >
> > About the IsModified, current function doesn't consider that hardware
> > also may change the pagetable. The issue is that in the first call of
> > internal function PageTableLibMapInLevel, the function assume page
> > table is not changed, and add ASSERT to check. But hardware may change
> > the page table, which cause the ASSERT happens.
> > Fix the issue by considering the hardware may also change page table,
> > and document the detail in function header.
> >
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Cc: Rahul Kumar 
> > Cc: Gerd Hoffmann 
> > Cc: Crystal Lee 
> > Signed-off-by: Zhiguang Liu 
> > ---
> >  .../Library/CpuPageTableLib/CpuPageTableMap.c| 16 
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > index 36b2c4e6a3..a3076ff2f6 100644
> > --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> > @@ -274,7 +274,7 @@ IsAttributesAndMaskValidForNonPresentEntry (
> >  Page table entries that map the linear 
> > address range are
> reset to 0 before set to the new attribute
> >  when a new physical base address is 
> > set.
> >@param[in]  Mask  The mask used for attribute. The 
> > corresponding
> field in Attribute is ignored if that in Mask is 0.
> > -  @param[out] IsModifiedTRUE means page table is modified. 
> > FALSE
> means page table is not modified.
> > +  @param[in, out] IsModifiedChange IsModified to True if page 
> > table is
> modified and input parameter Modify is TRUE.
> >
> >@retval RETURN_INVALID_PARAMETER  For non-present range, Mask-
> >Bits.Present is 0 but some other attributes are provided.
> >@retval RETURN_INVALID_PARAMETER  For non-present range, Mask-
> >Bits.Present is 1, Attribute->Bits.Present is 1 but some other attributes are
> not provided.
> > @@ -567,7 +567,10 @@ PageTableLibMapInLevel (
> >  OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
> >  PageTableLibSetPle (Level, CurrentPagingEntry, Offset,
> > Attribute, );
> >
> > -if (OriginalCurrentPagingEntry.Uint64 != 
> > CurrentPagingEntry->Uint64) {
> > +if (Modify && (OriginalCurrentPagingEntry.Uint64 !=
> CurrentPagingEntry->Uint64)) {
> > +  //
> > +  // The page table entry can be changed by this function only when
> Modify is true.
> > +  //
> >*IsModified = TRUE;
> >  }
> >}
> > @@ -609,7 +612,10 @@ PageTableLibMapInLevel (
> >// Check if ParentPagingEntry entry is modified here is enough. Except 
> > the
> changes happen in leaf PagingEntry during
> >// the while loop, if there is any other change happens in page table, 
> > the
> ParentPagingEntry must has been modified.
> >//
> > -  if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64)
> > {
> > +  if (Modify && (OriginalParentPagingEntry.Uint64 != ParentPagingEntry-
> >Uint64)) {
> > +//
> > +// The page table entry can be changed by this function only when
> Modify is true.
> > +//
> >  *IsModified = TRUE;
> >}
> >
> > @@ -633,7 +639,9 @@ PageTableLibMapInLevel (
> >   Page table entries that map the linear 
> > address range are
> reset to 0 before set to the new attribute
> >   when a new physical base address is set.
> >@param[in]  Mask   The mask used for attribute. The 
> > corresponding
> field in Attribute is ignored if that in Mask is 0.
> > -  @param[out] IsModified TRUE means page table is modified. FALSE
> means page table is not modified.
> > +  @param[out] IsModified TRUE means page table is modified by
> software or hardware. FALSE means page table is not modified by software.
> > + If the output IsModified is FALSE, there 
> > is possibility that
> the page table is changed by hardware. It is ok
> > + because page table can be changed by 
> > hardware anytime,
> and caller don't need to Flush TLB.
> >
> >@retval RETURN_UNSUPPORTEDPagingMode is not supported.
> >@retval 

[edk2-devel] [edk2-platforms PATCH v3 5/7] Silicon/Marvell: Driver to publish device tree

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds driver that can provide device tree to OS if user so
wishes. PCD's are used to enable this, default is not to take this
action.

Signed-off-by: Narinder Dhillon 
---
 .../Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c  | 259 ++
 .../Fdt/FdtPlatformDxe/FdtPlatformDxe.inf |  49 
 .../MarvellSiliconPkg/MarvellSiliconPkg.dec   |  20 ++
 3 files changed, 328 insertions(+)
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c
 create mode 100644 
Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatformDxe.inf

diff --git a/Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c 
b/Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c
new file mode 100644
index 00..cc3b853dff
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c
@@ -0,0 +1,259 @@
+/** @file
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+  https://spdx.org/licenses
+
+  Copyright (C) 2023 Marvell
+
+  Copyright (c) 2015, ARM Ltd. All rights reserved.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+//
+// Internal variables
+//
+
+VOID   *mFdtBlobBase;
+
+EFI_STATUS
+DeleteFdtNode (
+  IN  VOID*FdtAddr,
+  CONST CHAR8 *NodePath,
+  CONST CHAR8 *Compatible
+)
+{
+  INTNOffset = -1;
+  INTNReturn;
+
+  if ((NodePath != NULL) && (Compatible != NULL)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (NodePath != NULL) {
+Offset = fdt_path_offset (FdtAddr, NodePath);
+
+DEBUG ((DEBUG_INFO, "Offset: %d\n", Offset));
+
+if (Offset < 0) {
+  DEBUG ((DEBUG_ERROR, "Error getting the device node %a offset: %a\n",
+  NodePath, fdt_strerror (Offset)));
+  return EFI_NOT_FOUND;
+}
+  }
+
+  if (Compatible != NULL) {
+Offset = fdt_node_offset_by_compatible (FdtAddr, -1, Compatible);
+
+DEBUG ((DEBUG_INFO, "Offset: %d\n", Offset));
+
+if (Offset < 0) {
+  DEBUG ((DEBUG_ERROR, "Error getting the device node for %a offset: %a\n",
+  Compatible, fdt_strerror (Offset)));
+  return EFI_NOT_FOUND;
+}
+  }
+
+  if (Offset >= 0) {
+Return = fdt_del_node (FdtAddr, Offset);
+
+DEBUG ((DEBUG_INFO, "Return: %d\n", Return));
+
+if (Return < 0) {
+  DEBUG ((DEBUG_ERROR, "Error deleting the device node %a: %a\n",
+  NodePath, fdt_strerror (Return)));
+  return EFI_NOT_FOUND;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+DeleteRtcNode (
+  IN  VOID*FdtAddr
+  )
+{
+  INT32 Offset, NameLen, Return;
+  BOOLEAN Found;
+  CONST CHAR8 *Name;
+
+  Found = FALSE;
+  for (Offset = fdt_next_node(FdtAddr, 0, NULL);
+Offset >= 0;
+Offset = fdt_next_node(FdtAddr, Offset, NULL)) {
+
+Name = fdt_get_name(FdtAddr, Offset, );
+if (!Name) {
+  continue;
+}
+
+if ((Name[0] == 'r') && (Name[1] == 't') && (Name[2] == 'c')) {
+  Found = TRUE;
+  break;
+}
+  }
+
+  if (Found == TRUE) {
+Return = fdt_del_node (FdtAddr, Offset);
+
+if (Return < 0) {
+  DEBUG ((DEBUG_ERROR, "Error deleting the device node %a\n", Name));
+  return EFI_NOT_FOUND;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FdtFixup(
+  IN VOID *FdtAddr
+  )
+{
+  EFI_STATUS Status = EFI_SUCCESS;
+
+  if (FeaturePcdGet(PcdFixupFdt)) {
+Status |= DeleteFdtNode (FdtAddr, (CHAR8*)PcdGetPtr 
(PcdFdtConfigRootNode), NULL);
+
+// Hide the RTC
+Status |= DeleteRtcNode (FdtAddr);
+  }
+
+  if (!EFI_ERROR(Status)) {
+fdt_pack(FdtAddr);
+  }
+
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Install the FDT specified by its device path in text form.
+
+  @retval  EFI_SUCCESSThe FDT was installed.
+  @retval  EFI_NOT_FOUND  Failed to locate a protocol or a file.
+  @retval  EFI_INVALID_PARAMETER  Invalid device path.
+  @retval  EFI_UNSUPPORTEDDevice path not supported.
+  @retval  EFI_OUT_OF_RESOURCES   An allocation failed.
+**/
+STATIC
+EFI_STATUS
+InstallFdt (
+VOID
+)
+{
+  EFI_STATUS  Status;
+  UINTN   FdtBlobSize;
+  VOID*FdtConfigurationTableBase;
+  VOID*HobList;
+  EFI_HOB_GUID_TYPE   *GuidHob;
+
+  //
+  // Get the HOB list.  If it is not present, then ASSERT.
+  //
+  HobList = GetHobList ();
+  ASSERT (HobList != NULL);
+
+  //
+  // Search for FDT GUID HOB.  If it is not present, then
+  // there's nothing we can do. It may not exist on the update path.
+  //
+  GuidHob = GetNextGuidHob (, HobList);
+  if (GuidHob != NULL) {
+mFdtBlobBase = (VOID *)*(UINT64 *)(GET_GUID_HOB_DATA (GuidHob));
+FdtBlobSize = fdt_totalsize((VOID *)mFdtBlobBase);
+
+//
+// Ensure that the FDT header is valid and that the Size of the Device Tree
+// is smaller than the size of the read file
+//
+if (fdt_check_header (mFdtBlobBase)) {
+ 

[edk2-devel] [edk2-platforms PATCH v3 7/7] Silicon/Marvell: Odyssey project description files

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds Odyssey SoC project description and flash description
files.

Signed-off-by: Narinder Dhillon 
---
 Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc| 219 ++
 Platform/Marvell/OdysseyPkg/OdysseyPkg.fdf| 304 +
 Silicon/Marvell/OdysseyPkg/OdysseyPkg.dsc.inc | 399 ++
 3 files changed, 922 insertions(+)
 create mode 100644 Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc
 create mode 100644 Platform/Marvell/OdysseyPkg/OdysseyPkg.fdf
 create mode 100644 Silicon/Marvell/OdysseyPkg/OdysseyPkg.dsc.inc

diff --git a/Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc 
b/Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc
new file mode 100644
index 00..a9dad7b029
--- /dev/null
+++ b/Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc
@@ -0,0 +1,219 @@
+#/** @file
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#  https://spdx.org/licenses
+#
+#  Copyright (C) 2023 Marvell
+#
+#  The main build description file for OdysseyPkg.
+#**/
+
+
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = OdysseyPkg  # PLAT=ody
+  PLATFORM_GUID  = 7E7000DE-F50F-46AE-9B2C-903225F72B13
+  PLATFORM_VERSION   = 0.1
+  DSC_SPECIFICATION  = 0x00010005
+!ifdef $(EDK2_OUT_DIR) # Custom output directory, e.g. -D 
EDK2_OUT_DIR=Build/XYZ
+  OUTPUT_DIRECTORY   = $(EDK2_OUT_DIR)
+!else
+  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
+!endif
+  SUPPORTED_ARCHITECTURES= AARCH64
+  BUILD_TARGETS  = DEBUG|RELEASE
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = 
Platform/Marvell/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
+
+# dsc.inc file can be used in case there are different variants/boards of 
Odyssey family.
+# Per-board additional components shall be defined in exclusive dsc.inc files.
+!include Silicon/Marvell/$(PLATFORM_NAME)/$(PLATFORM_NAME).dsc.inc
+
+[LibraryClasses]
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf   # used by PlatformSmbiosDxe
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf # used by SmcLib
+
+  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf # used by 
SpiNorDxe
+
+  # USB Requirements
+  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf # used by UsbKbDxe
+  
RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
+
+[LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION, 
LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.DXE_DRIVER]
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
+  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf # used by 
BaseBmpSupportLib
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf
+!endif
+# ShellPkg/Application/Shell/Shell.inf -> UefiShellCommandLib -> 
OrderedCollectionLib
+  
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+  BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf # 
used by CapsuleApp
+  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+
+[BuildOptions]
+# GCC will generate code that runs on processors as idicated by -march
+# Single = (append) allows flags appendixes coming from [BuildOptions] defined 
in specific INFs.
+  GCC:*_*_AARCH64_PLATFORM_FLAGS = -DPLAT=0xBF -march=armv8.2-a 
-fdiagnostics-color -fno-diagnostics-show-caret
+
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+
+
+[PcdsFixedAtBuild.common]
+
+  # Generic Watchdog
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x802A
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x802B
+  gArmTokenSpaceGuid.PcdGenericWatchdogEl2IntrNum|0x1A
+
+  #  BIT0  - Initialization message.
+  #  BIT1  - Warning message.
+  #  BIT2  - Load Event message.
+  #  BIT3  - File System message.
+  #  BIT6  - Information message.
+  #  DEBUG_ERROR 0x8000  // Error
+  # NOTE: Adjust according to needs. See MdePkg.dec for bits definition.
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x804F
+
+  # The size of volatile buffer. This buffer is used to store VOLATILE 
attribute variables.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x0004
+
+  gArmTokenSpaceGuid.PcdVFPEnabled|1
+
+  # Set ARM PCD: Odyssey: up to 80 Neoverse V2 cores (code named Demeter)

[edk2-devel] [edk2-platforms PATCH v3 6/7] Silicon/Marvell: Command to dump device tree

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds an EDK2 shell command to dump configuration device tree.

Signed-off-by: Narinder Dhillon 
---
 .../Marvell/Applications/DumpFdt/DumpFdt.c| 344 ++
 .../Marvell/Applications/DumpFdt/DumpFdt.inf  |  52 +++
 .../Marvell/Applications/DumpFdt/DumpFdt.uni  |  35 ++
 3 files changed, 431 insertions(+)
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.c
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.inf
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.uni

diff --git a/Silicon/Marvell/Applications/DumpFdt/DumpFdt.c 
b/Silicon/Marvell/Applications/DumpFdt/DumpFdt.c
new file mode 100644
index 00..7abfb62a2c
--- /dev/null
+++ b/Silicon/Marvell/Applications/DumpFdt/DumpFdt.c
@@ -0,0 +1,344 @@
+/** @file
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+  https://spdx.org/licenses
+
+  Copyright (C) 2023 Marvell
+
+  Copyright (c) 2015, ARM Ltd. All rights reserved.
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CMD_NAME_STRING   L"dumpfdt"
+
+STATIC EFI_HII_HANDLE gShellDumpFdtHiiHandle = NULL;
+STATIC VOID   *mFdtBlobBase = NULL;
+STATIC CONST CHAR16 gShellDumpFdtFileName[] = L"ShellCommands";
+
+#define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1))
+#define PALIGN(p, a)((void *)(ALIGN ((unsigned long)(p), (a
+#define GET_CELL(p) (p += 4, *((const uint32_t *)(p-4)))
+
+STATIC
+UINTN
+IsPrintableString (
+  IN CONST VOID* data,
+  IN UINTN len
+  )
+{
+  CONST CHAR8 *s = data;
+  CONST CHAR8 *ss;
+
+  // Zero length is not
+  if (len == 0) {
+return 0;
+  }
+
+  // Must terminate with zero
+  if (s[len - 1] != '\0') {
+return 0;
+  }
+
+  ss = s;
+  while (*s/* && isprint (*s)*/) {
+s++;
+  }
+
+  // Not zero, or not done yet
+  if (*s != '\0' || (s + 1 - ss) < len) {
+return 0;
+  }
+
+  return 1;
+}
+
+STATIC
+VOID
+PrintData (
+  IN CONST CHAR8* data,
+  IN UINTN len
+  )
+{
+  UINTN i;
+  CONST CHAR8 *p = data;
+
+  // No data, don't print
+  if (len == 0)
+return;
+
+  if (IsPrintableString (data, len)) {
+Print (L" = \"%a\"", (const char *)data);
+  } else if ((len % 4) == 0) {
+Print (L" = <");
+for (i = 0; i < len; i += 4) {
+  Print (L"0x%08x%a", fdt32_to_cpu (GET_CELL (p)), i < (len - 4) ? " " : 
"");
+}
+Print (L">");
+  } else {
+Print (L" = [");
+for (i = 0; i < len; i++)
+  Print (L"%02x%a", *p++, i < len - 1 ? " " : "");
+Print (L"]");
+  }
+}
+
+STATIC
+VOID
+DumpFdt (
+  IN VOID*FdtBlob
+  )
+{
+  struct fdt_header *bph;
+  UINT32 off_dt;
+  UINT32 off_str;
+  CONST CHAR8* p_struct;
+  CONST CHAR8* p_strings;
+  CONST CHAR8* p;
+  CONST CHAR8* s;
+  CONST CHAR8* t;
+  UINT32 tag;
+  UINTN sz;
+  UINTN depth;
+  UINTN shift;
+  UINT32 version;
+
+  {
+// Can 'memreserve' be printed by below code?
+INTN num = fdt_num_mem_rsv (FdtBlob);
+INTN i, err;
+UINT64 addr = 0, size = 0;
+
+for (i = 0; i < num; i++) {
+  err = fdt_get_mem_rsv (FdtBlob, i, , );
+  if (err) {
+DEBUG ((DEBUG_ERROR, "Error (%d) : Cannot get memreserve section 
(%d)\n", err, i));
+  }
+  else {
+Print (L"/memreserve/ \t0x%lx \t0x%lx;\n", addr, size);
+  }
+}
+  }
+
+  depth = 0;
+  shift = 4;
+
+  bph = FdtBlob;
+  off_dt = fdt32_to_cpu (bph->off_dt_struct);
+  off_str = fdt32_to_cpu (bph->off_dt_strings);
+  p_struct = (CONST CHAR8*)FdtBlob + off_dt;
+  p_strings = (CONST CHAR8*)FdtBlob + off_str;
+  version = fdt32_to_cpu (bph->version);
+
+  p = p_struct;
+  while ((tag = fdt32_to_cpu (GET_CELL (p))) != FDT_END) {
+if (tag == FDT_BEGIN_NODE) {
+  s = p;
+  p = PALIGN (p + AsciiStrLen (s) + 1, 4);
+
+  if (*s == '\0')
+  s = "/";
+
+  Print (L"%*s%a {\n", depth * shift, L" ", s);
+
+  depth++;
+  continue;
+}
+
+if (tag == FDT_END_NODE) {
+  depth--;
+
+  Print (L"%*s};\n", depth * shift, L" ");
+  continue;
+}
+
+if (tag == FDT_NOP) {
+  /* Print (L"%*s// [NOP]\n", depth * shift, L" "); */
+  continue;
+}
+
+if (tag != FDT_PROP) {
+  Print (L"%*s ** Unknown tag 0x%08x\n", depth * shift, L" ", tag);
+  break;
+}
+sz = fdt32_to_cpu (GET_CELL (p));
+s = p_strings + fdt32_to_cpu (GET_CELL (p));
+if (version < 16 && sz >= 8)
+p = PALIGN (p, 8);
+t = p;
+
+p = PALIGN (p + sz, 4);
+
+Print (L"%*s%a", depth * shift, L" ", s);
+PrintData (t, sz);
+Print (L";\n");
+  }
+}
+
+STATIC
+SHELL_STATUS
+EFIAPI
+ShellCommandRunDumpFdt (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  SHELL_STATUS  ShellStatus;
+  EFI_STATUSStatus;
+
+  ShellStatus  = SHELL_SUCCESS;
+
+  Status = ShellInitialize ();
+  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return SHELL_ABORTED;
+  

[edk2-devel] [edk2-platforms PATCH v3 4/7] Silicon/Marvell: Device tree driver

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds a device tree driver that is used to read board
configuration information from a device tree.

Signed-off-by: Narinder Dhillon 
---
 .../Drivers/Fdt/FdtClientDxe/FdtClientDxe.c   | 382 ++
 .../Drivers/Fdt/FdtClientDxe/FdtClientDxe.inf |  43 ++
 .../Include/Protocol/FdtClient.h  | 180 +
 3 files changed, 605 insertions(+)
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.c
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.inf
 create mode 100644 
Silicon/Marvell/MarvellSiliconPkg/Include/Protocol/FdtClient.h

diff --git a/Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.c 
b/Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.c
new file mode 100644
index 00..8741a41e46
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.c
@@ -0,0 +1,382 @@
+/** @file
+*  FDT client driver
+*
+*  Copyright (c) 2016, Cavium Inc. All rights reserved.
+*  Copyright (c) 2016, Linaro Ltd. All rights reserved.
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+STATIC VOID  *mDeviceTreeBase;
+
+STATIC
+EFI_STATUS
+GetNodeProperty (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  FDT_HANDLENode,
+  IN  CONST CHAR8   *PropertyName,
+  OUT CONST VOID**Prop,
+  OUT UINT32*PropSize OPTIONAL
+  )
+{
+  INT32 Len;
+
+  ASSERT (mDeviceTreeBase != NULL);
+  ASSERT (Prop != NULL);
+
+  *Prop = fdt_getprop (mDeviceTreeBase, Node, PropertyName, );
+  if (*Prop == NULL) {
+return EFI_NOT_FOUND;
+  }
+
+  if (PropSize != NULL) {
+*PropSize = Len;
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+SetNodeProperty (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  FDT_HANDLENode,
+  IN  CONST CHAR8   *PropertyName,
+  IN  CONST VOID*Prop,
+  IN  UINT32PropSize
+  )
+{
+  INT32 Ret;
+
+  ASSERT (mDeviceTreeBase != NULL);
+
+  Ret = fdt_setprop (mDeviceTreeBase, Node, PropertyName, Prop, PropSize);
+  if (Ret != 0) {
+return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+FindCompatibleNode (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  CONST CHAR8   *CompatibleString,
+  IN  FDT_HANDLEPrevNode,
+  OUT FDT_HANDLE*Node
+  )
+{
+  FDT_HANDLE  Offset;
+
+  ASSERT (mDeviceTreeBase != NULL);
+  ASSERT (Node != NULL);
+
+  Offset = fdt_node_offset_by_compatible (mDeviceTreeBase, PrevNode, 
CompatibleString);
+
+  if (Offset < 0) {
+return EFI_NOT_FOUND;
+  }
+
+  *Node = Offset;
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetOrInsertChosenNode (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  OUT INT32 *Node
+  )
+{
+  INT32 NewNode;
+
+  ASSERT (mDeviceTreeBase != NULL);
+  ASSERT (Node != NULL);
+
+  NewNode = fdt_path_offset (mDeviceTreeBase, "/chosen");
+
+  if (NewNode < 0) {
+NewNode = fdt_add_subnode (mDeviceTreeBase, 0, "/chosen");
+  }
+
+  if (NewNode < 0) {
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  *Node = NewNode;
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetNodeDepth (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  FDT_HANDLENode,
+  OUT INT32 *Depth
+)
+{
+  *Depth = fdt_node_depth (mDeviceTreeBase, Node);
+
+  if (*Depth < 0) {
+return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetParentNode (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  FDT_HANDLENode,
+  OUT FDT_HANDLE*Parent
+)
+{
+  *Parent = fdt_parent_offset (mDeviceTreeBase, Node);
+
+  if (*Parent < 0) {
+return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetNode (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  CONST CHAR8   *Path,
+  OUT FDT_HANDLE*Node
+)
+{
+  *Node = fdt_path_offset (mDeviceTreeBase, Path);
+
+  if (*Node < 0) {
+return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetNodePath (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  CONST FDT_HANDLE  Node,
+  OUT CHAR8 *Path,
+  IN  INT32 Size
+)
+{
+  INT32 Result;
+
+  Result = fdt_get_path (mDeviceTreeBase, Node, Path, Size);
+
+  if (Result < 0) {
+return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+GetNodeByPropertyValue (
+  IN  MRVL_FDT_CLIENT_PROTOCOL  *This,
+  IN  CONST FDT_HANDLE  StartNode,
+  IN  CHAR8 *Property,
+  IN  VOID  *Value,
+  IN  INT32 Size,
+  OUT FDT_HANDLE*Node
+)
+{
+  INT32  Offset;
+
+  ASSERT (mDeviceTreeBase != NULL);
+  ASSERT (Node != NULL);
+
+  Offset = fdt_node_offset_by_prop_value (mDeviceTreeBase, 

[edk2-devel] [edk2-platforms PATCH v3 3/7] Silicon/Marvell: Odyssey watchdog driver

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds watchdog driver for Odyssey SoC.

Signed-off-by: Narinder Dhillon 
---
 .../Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c  | 408 ++
 .../Wdt/GtiWatchdogDxe/GtiWatchdogDxe.inf |  45 ++
 2 files changed, 453 insertions(+)
 create mode 100644 Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c
 create mode 100644 
Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdogDxe.inf

diff --git a/Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c 
b/Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c
new file mode 100644
index 00..c8f2888423
--- /dev/null
+++ b/Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c
@@ -0,0 +1,408 @@
+/** @file
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+* https://spdx.org/licenses
+*
+* Copyright (C) 2022 Marvell
+*
+* Source file for Marvell Watchdog driver
+*
+**/
+
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define GTI_CWD_WDOG(Core)(FixedPcdGet64(PcdGtiWatchdogBase64) + 0x4 + 
Core * 0x8)
+#define GTI_CWD_POKE(Core)(FixedPcdGet64(PcdGtiWatchdogBase64) + 0x5 + 
Core * 0x8)
+
+typedef union _GTI_CWD_WDOG_UNION {
+  UINT64  U64;
+  struct {
+UINTN Mode   : 2;
+UINTN State  : 2;
+UINTN Len: 16;
+UINTN Cnt: 24;
+UINTN DStop  : 1;
+UINTN GStop  : 1;
+UINTN Rsvd   : 18;
+  } PACKED S;
+} GTI_CWD_WDOG_UNION;
+
+#define CWD_WDOG_MODE_RST (BIT1 | BIT0)
+
+#define RST_BOOT_PNR_MUL(Val)  ((Val >> 33) & 0x1F)
+
+EFI_EVENT mGtiExitBootServicesEvent = (EFI_EVENT)NULL;
+UINT32  mSclk = 0;
+BOOLEAN mHardwarePlatform = TRUE;
+
+/**
+  Stop the GTI watchdog timer from counting down by disabling interrupts.
+**/
+STATIC
+VOID
+GtiWdtStop (
+  VOID
+  )
+{
+  GTI_CWD_WDOG_UNION Wdog;
+
+  MmioWrite64(GTI_CWD_POKE(0), 0);
+
+  Wdog.U64 = MmioRead64(GTI_CWD_WDOG(0));
+
+  // Disable WDT
+  if (Wdog.S.Mode != 0) {
+Wdog.S.Len = 1;
+Wdog.S.Mode = 0;
+MmioWrite64 (GTI_CWD_WDOG(0), Wdog.U64);
+  }
+}
+
+/**
+  Starts the GTI WDT countdown by enabling interrupts.
+  The count down will start from the value stored in the Load register,
+  not from the value where it was previously stopped.
+**/
+STATIC
+VOID
+GtiWdtStart (
+  VOID
+  )
+{
+  GTI_CWD_WDOG_UNION Wdog;
+
+  // Reset the WDT
+  MmioWrite64 (GTI_CWD_POKE(0), 0);
+
+  Wdog.U64 = MmioRead64 (GTI_CWD_WDOG(0));
+
+  // Enable countdown
+  if (Wdog.S.Mode == 0) {
+Wdog.S.Mode = CWD_WDOG_MODE_RST;
+MmioWrite64 (GTI_CWD_WDOG(0), Wdog.U64);
+  }
+}
+
+/**
+On exiting boot services we must make sure the SP805 Watchdog Timer
+is stopped.
+**/
+VOID
+EFIAPI
+GtiExitBootServices (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  MmioWrite64 (GTI_CWD_POKE(0), 0);
+  GtiWdtStop ();
+}
+
+/**
+  This function registers the handler NotifyFunction so it is called every time
+  the watchdog timer expires.  It also passes the amount of time since the last
+  handler call to the NotifyFunction.
+  If NotifyFunction is not NULL and a handler is not already registered,
+  then the new handler is registered and EFI_SUCCESS is returned.
+  If NotifyFunction is NULL, and a handler is already registered,
+  then that handler is unregistered.
+  If an attempt is made to register a handler when a handler is already 
registered,
+  then EFI_ALREADY_STARTED is returned.
+  If an attempt is made to unregister a handler when a handler is not 
registered,
+  then EFI_INVALID_PARAMETER is returned.
+
+  @param  This The EFI_TIMER_ARCH_PROTOCOL instance.
+  @param  NotifyFunction   The function to call when a timer interrupt fires. 
This
+   function executes at TPL_HIGH_LEVEL. The DXE Core 
will
+   register a handler for the timer interrupt, so it 
can know
+   how much time has passed. This information is used 
to
+   signal timer based events. NULL will unregister the 
handler.
+
+  @retval EFI_SUCCESS   The watchdog timer handler was registered.
+  @retval EFI_ALREADY_STARTED   NotifyFunction is not NULL, and a handler is 
already
+registered.
+  @retval EFI_INVALID_PARAMETER NotifyFunction is NULL, and a handler was not
+previously registered.
+  @retval EFI_UNSUPPORTED   HW does not support this functionality.
+
+**/
+EFI_STATUS
+EFIAPI
+GtiWdtRegisterHandler (
+  IN CONST EFI_WATCHDOG_TIMER_ARCH_PROTOCOL   *This,
+  IN EFI_WATCHDOG_TIMER_NOTIFYNotifyFunction
+  )
+{
+  // UNSUPPORTED - The hardware watchdog will reset the board
+  return EFI_UNSUPPORTED;
+}
+
+/**
+
+  This function adjusts the period of timer interrupts to the value specified
+  by TimerPeriod.  If the timer period is updated, then the selected timer
+  period is stored in EFI_TIMER.TimerPeriod, and EFI_SUCCESS is returned.  If
+  the timer hardware is 

[edk2-devel] [edk2-platforms PATCH v3 0/7] Silicon/Marvell/OdysseyPkg

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

New Marvell Odyssey SoC

This patchset contains only the very basic elements needed to boot to 
EDK2 UiApp on Marvell Odyssey SoC
- ARM BL31 firmware component copies EDK2 image into memory, so it is
  always executing from memory
- There is a SMC library to get system information from BL31
- There are drivers to get board configuration details from a device
  tree
- Emulated variable storage is used for now

v3:
-Added a helper library instead of overriding ArmPlatformPkg
-Use virtual RTC instead of adding a dummy RTC
-Put shell command in separate commit
-More specific names

v2:
-Split patch into 8 commits

v1:
-Original patch in single commit

Narinder Dhillon (7):
  Silicon/Marvell: New Marvell Odyssey processor
  Silicon/Marvell: Odyssey SmcLib
  Silicon/Marvell: Odyssey watchdog driver
  Silicon/Marvell: Device tree driver
  Silicon/Marvell: Driver to publish device tree
  Silicon/Marvell: Command to dump device tree
  Silicon/Marvell: Odyssey project description files

 Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc| 219 ++
 Platform/Marvell/OdysseyPkg/OdysseyPkg.fdf| 304 +
 .../Marvell/Applications/DumpFdt/DumpFdt.c| 344 +++
 .../Marvell/Applications/DumpFdt/DumpFdt.inf  |  52 +++
 .../Marvell/Applications/DumpFdt/DumpFdt.uni  |  35 ++
 .../Drivers/Fdt/FdtClientDxe/FdtClientDxe.c   | 382 
 .../Drivers/Fdt/FdtClientDxe/FdtClientDxe.inf |  43 ++
 .../Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c  | 259 +++
 .../Fdt/FdtPlatformDxe/FdtPlatformDxe.inf |  49 +++
 .../Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c  | 408 ++
 .../Wdt/GtiWatchdogDxe/GtiWatchdogDxe.inf |  45 ++
 Silicon/Marvell/Library/SmcLib/SmcLib.c   |  24 ++
 Silicon/Marvell/Library/SmcLib/SmcLib.inf |  29 ++
 .../Include/IndustryStandard/SmcLib.h |  28 ++
 .../Include/Protocol/FdtClient.h  | 180 
 .../MarvellSiliconPkg/MarvellSiliconPkg.dec   |  20 +
 .../OdysseyLib/AArch64/ArmPlatformHelper.S|  97 +
 .../Library/OdysseyLib/OdysseyLib.c   |  79 
 .../Library/OdysseyLib/OdysseyLib.inf |  60 +++
 .../Library/OdysseyLib/OdysseyLibMem.c| 142 ++
 Silicon/Marvell/OdysseyPkg/OdysseyPkg.dsc.inc | 399 +
 21 files changed, 3198 insertions(+)
 create mode 100644 Platform/Marvell/OdysseyPkg/OdysseyPkg.dsc
 create mode 100644 Platform/Marvell/OdysseyPkg/OdysseyPkg.fdf
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.c
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.inf
 create mode 100644 Silicon/Marvell/Applications/DumpFdt/DumpFdt.uni
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.c
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtClientDxe/FdtClientDxe.inf
 create mode 100644 Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatform.c
 create mode 100644 
Silicon/Marvell/Drivers/Fdt/FdtPlatformDxe/FdtPlatformDxe.inf
 create mode 100644 Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdog.c
 create mode 100644 
Silicon/Marvell/Drivers/Wdt/GtiWatchdogDxe/GtiWatchdogDxe.inf
 create mode 100644 Silicon/Marvell/Library/SmcLib/SmcLib.c
 create mode 100644 Silicon/Marvell/Library/SmcLib/SmcLib.inf
 create mode 100644 
Silicon/Marvell/MarvellSiliconPkg/Include/IndustryStandard/SmcLib.h
 create mode 100644 
Silicon/Marvell/MarvellSiliconPkg/Include/Protocol/FdtClient.h
 create mode 100644 
Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/AArch64/ArmPlatformHelper.S
 create mode 100644 Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.c
 create mode 100644 Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.inf
 create mode 100644 
Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLibMem.c
 create mode 100644 Silicon/Marvell/OdysseyPkg/OdysseyPkg.dsc.inc


base-commit: 3ebd07c49d498bf44ee7807300344b259d9caaa6
-- 
2.34.1



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




[edk2-devel] [edk2-platforms PATCH v3 1/7] Silicon/Marvell: New Marvell Odyssey processor

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch adds helper library to initialize Odyssey SoC.

Signed-off-by: Narinder Dhillon 
---
 .../OdysseyLib/AArch64/ArmPlatformHelper.S|  97 
 .../Library/OdysseyLib/OdysseyLib.c   |  79 ++
 .../Library/OdysseyLib/OdysseyLib.inf |  60 
 .../Library/OdysseyLib/OdysseyLibMem.c| 142 ++
 4 files changed, 378 insertions(+)
 create mode 100644 
Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/AArch64/ArmPlatformHelper.S
 create mode 100644 Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.c
 create mode 100644 Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.inf
 create mode 100644 
Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLibMem.c

diff --git 
a/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/AArch64/ArmPlatformHelper.S 
b/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/AArch64/ArmPlatformHelper.S
new file mode 100644
index 00..e816e6bd5a
--- /dev/null
+++ b/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/AArch64/ArmPlatformHelper.S
@@ -0,0 +1,97 @@
+/** @file
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+* https://spdx.org/licenses
+*
+* Copyright (C) 2023 Marvell
+*
+* Source file for Marvell ARM Platform library
+* Based on ArmPlatformPkg/Library/ArmPlatformLibNull
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* x1 - node number
+ */
+
+.text
+.align 2
+
+GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
+GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
+GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
+GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
+GCC_ASM_EXPORT(ArmGetCpuCountPerCluster)
+
+GCC_ASM_IMPORT(mDeviceTreeBaseAddress)
+GCC_ASM_IMPORT(mSystemMemoryEnd)
+
+ASM_FUNC(ArmPlatformPeiBootAction)
+  // Save the boot parameter to a global variable
+  adr   x10, mDeviceTreeBaseAddress
+  str   x1, [x10]
+
+  adr   x1, PrimaryCoreMpid
+  str   w0, [x1]
+  ldr   x0, =MV_SMC_ID_DRAM_SIZE
+  mov   x1, xzr
+  smc   #0
+  sub   x0, x0, #1   // Last valid address
+  // if mSystemMemoryEnd wasn't gethered from SMC call, get it from PCDs
+  cmp   x0, #0x
+  bne   done
+  // if mSystemMemoryEnd wasn't gethered from SMC call, get it from PCDs
+  MOV64 (x0, FixedPcdGet64(PcdSystemMemoryBase) + 
FixedPcdGet64(PcdSystemMemorySize) - 1)
+done:
+  adr   x1, mSystemMemoryEnd
+  str   x0, [x1] // Set mSystemMemoryEnd
+
+  ret
+
+
+//UINTN
+//ArmPlatformGetPrimaryCoreMpId (
+//  VOID
+//  );
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  MOV32(w0, FixedPcdGet32(PcdArmPrimaryCore))
+  ret
+
+//UINTN
+//ArmPlatformIsPrimaryCore (
+//  IN UINTN MpId
+//  );
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
+  and   x0, x0, x1
+  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCore))
+  cmp   w0, w1
+  mov   x0, #1
+  mov   x1, #0
+  csel  x0, x0, x1, eq
+  ret
+
+//UINTN
+//ArmPlatformGetCorePosition (
+//  IN UINTN MpId
+//  );
+ASM_FUNC(ArmPlatformGetCorePosition)
+/*
+  Affinity Level 0: single thread 0
+  Affinity Level 1: clustering 0(
+  Affinity Level 2: number of clusters up to 64 (CN10K)/ 80 (Odyssey)/ 16 
(Iliad)
+  Affinity Level 3: number of chip 0
+  LinearId = Aff2
+*/
+  and   x0, x0, #ARM_CORE_AFF2
+  lsr   x0, x0, #16
+  ret
+
+ASM_FUNCTION_REMOVE_IF_UNREFERENCED
+
+PrimaryCoreMpid:  .word0x0
diff --git a/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.c 
b/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.c
new file mode 100644
index 00..ed48a00950
--- /dev/null
+++ b/Silicon/Marvell/OdysseyPkg/Library/OdysseyLib/OdysseyLib.c
@@ -0,0 +1,79 @@
+/** @file
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+* https://spdx.org/licenses
+*
+* Copyright (C) 2022 Marvell
+*
+* Source file for Marvell ARM Platform library
+* Based on ArmPlatformPkg/Library/ArmPlatformLibNull
+**/
+
+#include 
+#include   // EFI_BOOT_MODE
+#include // EFI_PEI_PPI_DESCRIPTOR
+#include// ASSERT
+#include  // ArmPlatformIsPrimaryCore
+#include   // ARM_MP_CORE_INFO_PPI
+
+/**
+  Return the current Boot Mode
+
+  This function returns the boot reason on the platform
+
+  @return   Return the current Boot Mode of the platform
+
+**/
+EFI_BOOT_MODE
+ArmPlatformGetBootMode (
+  VOID
+  )
+{
+  return BOOT_WITH_FULL_CONFIGURATION;
+}
+
+/**
+  Initialize controllers that must setup in the normal world
+
+  This function is called by the ArmPlatformPkg/PrePei or 
ArmPlatformPkg/Pei/PlatformPeim
+  in the PEI phase.
+
+**/
+RETURN_STATUS
+ArmPlatformInitialize (
+  IN  UINTN MpId
+  )
+{
+  ASSERT(ArmPlatformIsPrimaryCore (MpId));
+
+  return RETURN_SUCCESS;
+}
+
+EFI_STATUS
+PrePeiCoreGetMpCoreInfo (
+  OUT UINTN   *CoreCount,
+  OUT ARM_CORE_INFO   **ArmCoreTable
+  )
+{
+return EFI_UNSUPPORTED;
+}
+
+ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
+
+EFI_PEI_PPI_DESCRIPTOR  gPlatformPpiTable[] = {
+  {
+

[edk2-devel] [edk2-platforms PATCH v3 2/7] Silicon/Marvell: Odyssey SmcLib

2024-01-17 Thread Narinder Dhillon
From: Narinder Dhillon 

This patch provides SMC call needed by Odyssey to determine size
of available memory.

Signed-off-by: Narinder Dhillon 
---
 Silicon/Marvell/Library/SmcLib/SmcLib.c   | 24 +++
 Silicon/Marvell/Library/SmcLib/SmcLib.inf | 29 +++
 .../Include/IndustryStandard/SmcLib.h | 28 ++
 3 files changed, 81 insertions(+)
 create mode 100644 Silicon/Marvell/Library/SmcLib/SmcLib.c
 create mode 100644 Silicon/Marvell/Library/SmcLib/SmcLib.inf
 create mode 100644 
Silicon/Marvell/MarvellSiliconPkg/Include/IndustryStandard/SmcLib.h

diff --git a/Silicon/Marvell/Library/SmcLib/SmcLib.c 
b/Silicon/Marvell/Library/SmcLib/SmcLib.c
new file mode 100644
index 00..0280983dd0
--- /dev/null
+++ b/Silicon/Marvell/Library/SmcLib/SmcLib.c
@@ -0,0 +1,24 @@
+/** @file
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+* https://spdx.org/licenses
+*
+* Copyright (C) 2023 Marvell
+*
+* Source file for Marvell SMC Interface
+*
+**/
+
+#include 
+#include   // ArmCallSmc
+
+UINTN SmcGetRamSize ( IN UINTN Node )
+{
+  ARM_SMC_ARGS ArmSmcArgs;
+
+  ArmSmcArgs.Arg0 = MV_SMC_ID_DRAM_SIZE;
+  ArmSmcArgs.Arg1 = Node;
+  ArmCallSmc ();
+
+  return ArmSmcArgs.Arg0;
+}
diff --git a/Silicon/Marvell/Library/SmcLib/SmcLib.inf 
b/Silicon/Marvell/Library/SmcLib/SmcLib.inf
new file mode 100644
index 00..7fc1085b85
--- /dev/null
+++ b/Silicon/Marvell/Library/SmcLib/SmcLib.inf
@@ -0,0 +1,29 @@
+#/** @file
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+# https://spdx.org/licenses
+#
+# Copyright (C) 2023 Marvell
+#
+# Marvell SMC Interface library
+#
+#**/
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SmcLib
+  FILE_GUID  = fee427a7-816a-4636-bb81-a640c8288f28
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = SmcLib
+
+[Sources]
+  SmcLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/MarvellSiliconPkg/MarvellSiliconPkg.dec
+
+[LibraryClasses]
+  ArmSmcLib
diff --git 
a/Silicon/Marvell/MarvellSiliconPkg/Include/IndustryStandard/SmcLib.h 
b/Silicon/Marvell/MarvellSiliconPkg/Include/IndustryStandard/SmcLib.h
new file mode 100644
index 00..f2d0bed356
--- /dev/null
+++ b/Silicon/Marvell/MarvellSiliconPkg/Include/IndustryStandard/SmcLib.h
@@ -0,0 +1,28 @@
+/** @file
+*
+* SPDX-License-Identifier: BSD-2-Clause-Patent
+* https://spdx.org/licenses
+*
+* Copyright (C) 2023 Marvell
+*
+* Header file for for Marvell SMC Interface
+*
+**/
+
+#ifndef __SMCLIB_H__
+#define __SMCLIB_H__
+
+/* SMC function IDs for Marvell Service queries */
+
+#define MV_SMC_ID_CALL_COUNT  0xc200ff00
+#define MV_SMC_ID_UID 0xc200ff01
+
+#define MV_SMC_ID_VERSION 0xc200ff03
+
+/* x1 - node number */
+#define MV_SMC_ID_DRAM_SIZE   0xc2000301
+
+
+UINTN SmcGetRamSize (IN UINTN Node);
+
+#endif
-- 
2.34.1



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




Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Doug Flick via groups.io
Linking this here

https://edk2.groups.io/g/devel/message/113966


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




[edk2-devel] [PATCH 1/3] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117/4118 symbol rename

2024-01-17 Thread Doug Flick via groups.io
Updates the sanitation function names to be lib unique names

Cc: Jiewen Yao 
Cc: Rahul Kumar 

Signed-off-by: Doug Flick [MSFT] 
---
 .../DxeTpm2MeasureBootLibSanitization.h   |  8 +++---
 .../DxeTpm2MeasureBootLib.c   |  8 +++---
 .../DxeTpm2MeasureBootLibSanitization.c   |  8 +++---
 .../DxeTpm2MeasureBootLibSanitizationTest.c   | 26 +--
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
index 8f72ba42401f..8526bc7537d5 100644
--- 
a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
+++ 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
@@ -54,7 +54,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+Tpm2SanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL   *BlockIo
   );
@@ -78,7 +78,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+Tpm2SanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32   *AllocationSize
   );
@@ -107,7 +107,7 @@ SanitizePrimaryHeaderAllocationSize (
 One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePrimaryHeaderGptEventSize (
+Tpm2SanitizePrimaryHeaderGptEventSize (
   IN  CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN  UINTN NumberOfPartition,
   OUT UINT32*EventSize
@@ -131,7 +131,7 @@ SanitizePrimaryHeaderGptEventSize (
 One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePeImageEventSize (
+Tpm2SanitizePeImageEventSize (
   IN  UINT32  FilePathSize,
   OUT UINT32  *EventSize
   );
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index 714cc8e03e80..73719f3b96ed 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -200,7 +200,7 @@ Tcg2MeasureGptTable (
  BlockIo->Media->BlockSize,
  (UINT8 *)PrimaryHeader
  );
-  if (EFI_ERROR (Status) || EFI_ERROR (SanitizeEfiPartitionTableHeader 
(PrimaryHeader, BlockIo))) {
+  if (EFI_ERROR (Status) || EFI_ERROR (Tpm2SanitizeEfiPartitionTableHeader 
(PrimaryHeader, BlockIo))) {
 DEBUG ((DEBUG_ERROR, "Failed to read Partition Table Header or invalid 
Partition Table Header!\n"));
 FreePool (PrimaryHeader);
 return EFI_DEVICE_ERROR;
@@ -209,7 +209,7 @@ Tcg2MeasureGptTable (
   //
   // Read the partition entry.
   //
-  Status = SanitizePrimaryHeaderAllocationSize (PrimaryHeader, );
+  Status = Tpm2SanitizePrimaryHeaderAllocationSize (PrimaryHeader, );
   if (EFI_ERROR (Status)) {
 FreePool (PrimaryHeader);
 return EFI_BAD_BUFFER_SIZE;
@@ -250,7 +250,7 @@ Tcg2MeasureGptTable (
   //
   // Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
   //
-  Status = SanitizePrimaryHeaderGptEventSize (PrimaryHeader, 
NumberOfPartition, );
+  Status = Tpm2SanitizePrimaryHeaderGptEventSize (PrimaryHeader, 
NumberOfPartition, );
   if (EFI_ERROR (Status)) {
 FreePool (PrimaryHeader);
 FreePool (EntryPtr);
@@ -420,7 +420,7 @@ Tcg2MeasurePeImage (
   }
 
   FilePathSize = (UINT32)GetDevicePathSize (FilePath);
-  Status   = SanitizePeImageEventSize (FilePathSize, );
+  Status   = Tpm2SanitizePeImageEventSize (FilePathSize, );
   if (EFI_ERROR (Status)) {
 return EFI_UNSUPPORTED;
   }
diff --git 
a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
index 2a4d52c6d5cf..809a3bfd892e 100644
--- 
a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
+++ 
b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
@@ -63,7 +63,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+Tpm2SanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL   *BlockIo
   )
@@ -169,7 +169,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+Tpm2SanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32   *AllocationSize
   )
@@ -221,7 +221,7 @@ SanitizePrimaryHeaderAllocationSize (
 One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePrimaryHeaderGptEventSize (
+Tpm2SanitizePrimaryHeaderGptEventSize (
   IN  CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN  UINTN NumberOfPartition,
  

[edk2-devel] [PATCH 3/3] SecurityPkg: : Updating SecurityFixes.yaml after symbol rename

2024-01-17 Thread Doug Flick via groups.io
Adding the new commit titles for the symbol renames

Cc: Jiewen Yao 
Cc: Rahul Kumar 

Signed-off-by: Doug Flick [MSFT] 
---
 SecurityPkg/SecurityFixes.yaml | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
index 833fb827a96c..b4006b42b89e 100644
--- a/SecurityPkg/SecurityFixes.yaml
+++ b/SecurityPkg/SecurityFixes.yaml
@@ -9,28 +9,34 @@ CVE_2022_36763:
 - "SecurityPkg: DxeTpm2Measurement: SECURITY PATCH 4117 - CVE 2022-36763"
 - "SecurityPkg: DxeTpmMeasurement: SECURITY PATCH 4117 - CVE 2022-36763"
 - "SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml"
+- "SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117/4118 symbol 
rename"
+- "SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117/4118 symbol 
rename"
+- "SecurityPkg: : Updating SecurityFixes.yaml after symbol rename"
   cve: CVE-2022-36763
   date_reported: 2022-10-25 11:31 UTC
   description: (CVE-2022-36763) - Heap Buffer Overflow in Tcg2MeasureGptTable()
   note: This patch is related to and supersedes TCBZ2168
   files_impacted:
-  - Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
-  - Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
+- Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
+- Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
   links:
-  - https://bugzilla.tianocore.org/show_bug.cgi?id=4117
-  - https://bugzilla.tianocore.org/show_bug.cgi?id=2168
-  - https://bugzilla.tianocore.org/show_bug.cgi?id=1990
+- https://bugzilla.tianocore.org/show_bug.cgi?id=4117
+- https://bugzilla.tianocore.org/show_bug.cgi?id=2168
+- https://bugzilla.tianocore.org/show_bug.cgi?id=1990
 CVE_2022_36764:
   commit_titles:
- - "SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 
2022-36764"
- - "SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE 
2022-36764"
- - "SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml"
+- "SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 
2022-36764"
+- "SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764"
+- "SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml"
+- "SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117/4118 symbol 
rename"
+- "SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117/4118 symbol 
rename"
+- "SecurityPkg: : Updating SecurityFixes.yaml after symbol rename"
   cve: CVE-2022-36764
   date_reported: 2022-10-25 12:23 UTC
   description: Heap Buffer Overflow in Tcg2MeasurePeImage()
   note:
   files_impacted:
-  - Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
-  - Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
+- Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
+- Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
   links:
-  - https://bugzilla.tianocore.org/show_bug.cgi?id=4118
+- https://bugzilla.tianocore.org/show_bug.cgi?id=4118
-- 
2.43.0



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




[edk2-devel] [PATCH 2/3] SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117/4118 symbol rename

2024-01-17 Thread Doug Flick via groups.io
Updates the sanitation function names to be lib unique names

Cc: Jiewen Yao 
Cc: Rahul Kumar 

Signed-off-by: Doug Flick [MSFT] 
---
 .../DxeTpmMeasureBootLibSanitization.h|  8 +++---
 .../DxeTpmMeasureBootLib.c|  8 +++---
 .../DxeTpmMeasureBootLibSanitization.c| 10 +++
 .../DxeTpmMeasureBootLibSanitizationTest.c| 26 +--
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
index 2248495813b5..db6e9c3752d6 100644
--- 
a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
+++ 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
@@ -53,7 +53,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+TpmSanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL   *BlockIo
   );
@@ -77,7 +77,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+TpmSanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32   *AllocationSize
   );
@@ -105,7 +105,7 @@ SanitizePrimaryHeaderAllocationSize (
 One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePrimaryHeaderGptEventSize (
+TpmSanitizePrimaryHeaderGptEventSize (
   IN  CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN  UINTN NumberOfPartition,
   OUT UINT32*EventSize
@@ -129,7 +129,7 @@ SanitizePrimaryHeaderGptEventSize (
 One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePeImageEventSize (
+TpmSanitizePeImageEventSize (
   IN  UINT32  FilePathSize,
   OUT UINT32  *EventSize
   );
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index a9fc440a091e..ac855b8fbbf4 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -174,7 +174,7 @@ TcgMeasureGptTable (
  BlockIo->Media->BlockSize,
  (UINT8 *)PrimaryHeader
  );
-  if (EFI_ERROR (Status) || EFI_ERROR (SanitizeEfiPartitionTableHeader 
(PrimaryHeader, BlockIo))) {
+  if (EFI_ERROR (Status) || EFI_ERROR (TpmSanitizeEfiPartitionTableHeader 
(PrimaryHeader, BlockIo))) {
 DEBUG ((DEBUG_ERROR, "Failed to read Partition Table Header or invalid 
Partition Table Header!\n"));
 FreePool (PrimaryHeader);
 return EFI_DEVICE_ERROR;
@@ -183,7 +183,7 @@ TcgMeasureGptTable (
   //
   // Read the partition entry.
   //
-  Status = SanitizePrimaryHeaderAllocationSize (PrimaryHeader, );
+  Status = TpmSanitizePrimaryHeaderAllocationSize (PrimaryHeader, );
   if (EFI_ERROR (Status)) {
 FreePool (PrimaryHeader);
 return EFI_DEVICE_ERROR;
@@ -224,7 +224,7 @@ TcgMeasureGptTable (
   //
   // Prepare Data for Measurement
   //
-  Status   = SanitizePrimaryHeaderGptEventSize (PrimaryHeader, 
NumberOfPartition, );
+  Status   = TpmSanitizePrimaryHeaderGptEventSize (PrimaryHeader, 
NumberOfPartition, );
   TcgEvent = (TCG_PCR_EVENT *)AllocateZeroPool (EventSize);
   if (TcgEvent == NULL) {
 FreePool (PrimaryHeader);
@@ -351,7 +351,7 @@ TcgMeasurePeImage (
 
   // Determine destination PCR by BootPolicy
   //
-  Status = SanitizePeImageEventSize (FilePathSize, );
+  Status = TpmSanitizePeImageEventSize (FilePathSize, );
   if (EFI_ERROR (Status)) {
 return EFI_UNSUPPORTED;
   }
diff --git 
a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
index c989851cec2d..070e4a2c1cab 100644
--- 
a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
+++ 
b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
@@ -1,5 +1,5 @@
 /** @file
-  The library instance provides security service of TPM2 measure boot and
+  The library instance provides security service of TPM measure boot and
   Confidential Computing (CC) measure boot.
 
   Caution: This file requires additional review when modified.
@@ -63,7 +63,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+TpmSanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL   *BlockIo
   )
@@ -145,7 +145,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+TpmSanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32   *AllocationSize
   )
@@ -194,7 +194,7 @@ SanitizePrimaryHeaderAllocationSize (
 One of the passed 

[edk2-devel] [PATCH 0/3] DxeTpm and DxeTpm2MeasureBootLib symbol rename

2024-01-17 Thread Doug Flick via groups.io
OVMF is failing because it includes both DxeTpm2MeasureBootLib and
DxeTpm2MeasureBootLib which makes the symbols collide. This patch
renames the function names to be unique to avoid symbol collision.

Cc: Jiewen Yao 
Cc: Rahul Kumar 

Signed-off-by: Doug Flick [MSFT] 

Douglas Flick [MSFT] (3):
  SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117/4118 symbol
rename
  SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117/4118 symbol
rename
  SecurityPkg: : Updating SecurityFixes.yaml after symbol rename

 .../DxeTpm2MeasureBootLibSanitization.h   |  8 +++---
 .../DxeTpmMeasureBootLibSanitization.h|  8 +++---
 .../DxeTpm2MeasureBootLib.c   |  8 +++---
 .../DxeTpm2MeasureBootLibSanitization.c   |  8 +++---
 .../DxeTpm2MeasureBootLibSanitizationTest.c   | 26 -
 .../DxeTpmMeasureBootLib.c|  8 +++---
 .../DxeTpmMeasureBootLibSanitization.c| 10 +++
 .../DxeTpmMeasureBootLibSanitizationTest.c| 26 -
 SecurityPkg/SecurityFixes.yaml| 28 +++
 9 files changed, 68 insertions(+), 62 deletions(-)

-- 
2.43.0


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




[edk2-devel] [PATCH v2 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

2024-01-17 Thread Jeremy Linton
Its a complete tossup whether removing the mailbox call after we have
set up the XHCI works for a given kernel+distro in DT mode. So lets
give users which want to try DT the option of flipping this on/off.

Users that don't want to have to deal with DT, can use ACPI.

Signed-off-by: Jeremy Linton 
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 15 +++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  |  4 
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf|  1 +
 Platform/RaspberryPi/RPi3/RPi3.dsc|  6 ++
 Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
 Platform/RaspberryPi/RaspberryPi.dec  |  1 +
 9 files changed, 50 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3dcf2bac0d..2484787982 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -298,6 +298,16 @@ SetupVariables (
 Status = PcdSet32S (PcdXhciPci, 1);
 ASSERT_EFI_ERROR (Status);
   }
+
+  Size = sizeof (UINT32);
+  Status = gRT->GetVariable (L"XhciReload",
+ ,
+ NULL, , );
+  if (EFI_ERROR (Status)) {
+Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
+ASSERT_EFI_ERROR (Status);
+  }
+
 }
   } else {
 /*
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 6f6e8f42ac..475e645537 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 5ec17072c3..8130638876 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -62,6 +62,11 @@
 #string STR_ADVANCED_XHCIPCI_XHCI #language en-US "XHCI"
 #string STR_ADVANCED_XHCIPCI_PCIE #language en-US "PCIe"
 
+#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI 
firmware"
+#string STR_ADVANCED_XHCIRELOAD_HELP#language en-US "OS should reload XHCI 
firmware on reset"
+#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset 
Tag"
 
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f668b7a553..f13b70711d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -61,6 +61,11 @@ formset
   name  = XhciPci,
   guid  = CONFIGDXE_FORM_SET_GUID;
 
+efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
+  attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  name  = XhciReload,
+  guid  = CONFIGDXE_FORM_SET_GUID;
+
 efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
   attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
   name  = SystemTableMode,
@@ -228,6 +233,16 @@ formset
   option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 
1, flags = 0;
 endoneof;
   endif;
+
+  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+oneof varid = XhciReload.Value,
+  prompt  = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
+  help= STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
+  flags   = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), 
value = 0, flags = DEFAULT;
+  option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), 
value = 1, flags = 0;
+endoneof;
+  endif;
 endif;
 #endif
 string varid = AssetTag.AssetTag,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cbbc2ad30d..dd4fc0a05e 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,10 @@ SyncPcie (
 return 

[edk2-devel] [PATCH v2 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT

2024-01-17 Thread Jeremy Linton
Since we are updating the DT memory map and telling it how
we have configured the PCIe, there isn't a reason for moving the
MMIO window. In fact this appears to fix OpenBSD+DT as well as
it makes the linux XHCI reset sequence happier.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  | 22 +++
 .../RaspberryPi/Drivers/FdtDxe/FdtDxe.inf |  6 +
 2 files changed, 28 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index dd4fc0a05e..a247aa36ed 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,28 @@ SyncPcie (
 return EFI_NOT_FOUND;
   }
 
+  // move the MMIO window too
+  DmaRanges[0] = cpu_to_fdt32 (0x0200); //non prefech 32-bit
+  DmaRanges[1] = cpu_to_fdt32 (FixedPcdGet64 (PcdBcm27xxPciBusMmioAdr) >> 32); 
//bus addr @ 0x0f800
+  DmaRanges[2] = cpu_to_fdt32 (FixedPcdGet64 (PcdBcm27xxPciBusMmioAdr) & 
MAX_UINT32);
+  DmaRanges[3] = cpu_to_fdt32 (FixedPcdGet64 (PcdBcm27xxPciCpuMmioAdr) >> 32); 
//cpu addr @ 0x6
+  DmaRanges[4] = cpu_to_fdt32 (FixedPcdGet64 (PcdBcm27xxPciCpuMmioAdr) & 
MAX_UINT32);
+  DmaRanges[5] = cpu_to_fdt32 (0x);
+  DmaRanges[6] = cpu_to_fdt32 (FixedPcdGet32 (PcdBcm27xxPciBusMmioLen) + 1); 
// len = 0x4000 
+
+  DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n",  __func__));
+
+  /*
+   * Match ranges (BAR/MMIO) with the EDK2+ACPI setup we are using.
+   */
+  Retval = fdt_setprop (mFdtImage, Node, "ranges",
+DmaRanges,  sizeof DmaRanges);
+  if (Retval != 0) {
+DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property 
(%d)\n",
+  __func__, Retval));
+return EFI_NOT_FOUND;
+  }
+
   if (PcdGet32 (PcdXhciReload) != 1) {
 return EFI_SUCCESS;
   }
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf 
b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index d9fb6ee480..90e138af7c 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -23,6 +23,7 @@
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   Platform/RaspberryPi/RaspberryPi.dec
+  Silicon/Broadcom/Bcm27xx/Bcm27xx.dec
 
 [LibraryClasses]
   BaseLib
@@ -44,6 +45,11 @@
 
 [FixedPcd]
   gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress
+  gBcm27xxTokenSpaceGuid.PcdBcm27xxPciRegBase
+  gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioAdr
+  gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen
+  gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr
+
 
 [Pcd]
   gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
-- 
2.43.0



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




[edk2-devel] [PATCH v2 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings

2024-01-17 Thread Jeremy Linton
Some recent GCC revisions will throw warnings about values being used
before being initialized. But in the case where the lack of initialization
is the result of the called function returning error status the EFI_ERROR()
macro/error seems to confuse the compiler about the fact that the value
is then never used.

So, while the code appears to be fine, lets just zero the variables
anyway to make the compiler happy.

Signed-off-by: Jeremy Linton 
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c| 2 ++
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
index 9e5d30fafd..2d5f70170e 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
@@ -381,6 +381,8 @@ GenericPhyUpdateConfig (
   BOOLEAN LinkUp;
 
   Status = GenericPhyGetLinkStatus (Phy);
+  Speed = 0;
+  Duplex = 0;
   LinkUp = EFI_ERROR (Status) ? FALSE : TRUE;
 
   if (Phy->LinkUp != LinkUp) {
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c 
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 3b51a86d65..7a7c398b1f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -731,6 +731,9 @@ GenetSimpleNetworkReceive (
   UINT8   *Frame;
   UINTN   FrameLength;
 
+  DescIndex = 0;
+  FrameLength = 0;
+
   if (This == NULL || Buffer == NULL) {
 DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (missing handle or buffer)\n",
   __FUNCTION__));
-- 
2.43.0



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




[edk2-devel] [PATCH v2 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

2024-01-17 Thread Jeremy Linton
The rpi's config.txt controls which uart (pl011, or miniuart) is
selected as the console. TFA and edk2 follow its lead, but if the
miniuart is selected as the primary and the machine is booted in ACPI
mode the baud/etc is never configured for the pl011. The linux kernel
won't reconfigure it either as its listed as a "SBSA" uart, so it
simply won't work.

This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
to work again.

Signed-off-by: Jeremy Linton 
---
 .../DualSerialPortLib/DualSerialPortLib.c | 44 ---
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c 
b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index d2f983bf0a..09d3e33c00 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -76,6 +76,8 @@ SerialPortInitialize (
   EFI_PARITY_TYPE Parity;
   UINT8   DataBits;
   EFI_STOP_BITS_TYPE  StopBits;
+  RETURN_STATUS   Ret;
+  UINTN   Timeout;
 
   //
   // First thing we need to do is determine which of PL011 or miniUART is 
selected
@@ -85,23 +87,34 @@ SerialPortInitialize (
 UsePl011UartSet = TRUE;
   }
 
-  if (UsePl011Uart) {
-BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+  // always init the pl011 on the RPi4, linux expects a SBSA uart to be at 
115200
+  // this means we need to set the baud/etc even if we aren't using it as a 
console
+  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
 ReceiveFifoDepth = 0; // Use default FIFO depth
+if (!UsePl011Uart)
+{
+  BaudRate = 115200;
+}
+else
+{
+  BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+}
 Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
 DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
 StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
 
-return PL011UartInitializePort (
- PL011_UART_REGISTER_BASE,
- PL011UartClockGetFreq(),
- ,
- ,
- ,
- ,
- 
- );
-  } else {
+Ret = PL011UartInitializePort (
+   PL011_UART_REGISTER_BASE,
+   PL011UartClockGetFreq(),
+   ,
+   ,
+   ,
+   ,
+   
+   );
+  }
+
+  if (!UsePl011Uart) {
 SerialRegisterBase = MINI_UART_REGISTER_BASE;
 Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
 
@@ -127,7 +140,8 @@ SerialPortInitialize (
 // Wait for the serial port to be ready.
 // Verify that both the transmit FIFO and the shift register are empty.
 //
-while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+Timeout = 1000;
+while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) 
&& (Timeout--));
 
 //
 // Configure baud rate
@@ -158,9 +172,9 @@ SerialPortInitialize (
 // Put Modem Control Register(MCR) into its reset state of 0x00.
 //
 SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-
-return RETURN_SUCCESS;
+Ret = RETURN_SUCCESS;
   }
+  return Ret;
 }
 
 /**
-- 
2.43.0



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




[edk2-devel] [PATCH v2 3/5] Platform/RaspberryPi: Cleanup menu visibility

2024-01-17 Thread Jeremy Linton
Lets allow some of these options to change when the
system is in ACPI+DT mode. Plus the fan temp should
be disabled when ACPI isn't enabled.

Signed-off-by: Jeremy Linton 
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index e8bf16312d..f668b7a553 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -196,7 +196,7 @@ formset
 endoneof;
 
 #if (RPI_MODEL == 4)
-grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
   oneof varid = FanOnGpio.Enabled,
   prompt  = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
@@ -207,7 +207,7 @@ formset
   endoneof;
 endif;
 
-grayoutif ideqval FanOnGpio.Enabled == 0;
+grayoutif ideqval FanOnGpio.Enabled == 0 OR ideqval 
SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
   numeric varid = FanTemp.Value,
   prompt  = STRING_TOKEN(STR_ADVANCED_FANTEMP_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_FANTEMP_HELP),
@@ -219,7 +219,7 @@ formset
 endif;
 
 suppressif ideqval XhciPci.Value == 2;
-  grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+  grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
 oneof varid = XhciPci.Value,
   prompt  = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PROMPT),
   help= STRING_TOKEN(STR_ADVANCED_XHCIPCI_HELP),
-- 
2.43.0



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




[edk2-devel] [PATCH v2 0/5] Platform/RaspberryPi: Various minor fixes

2024-01-17 Thread Jeremy Linton
This includes a change to always initialize the PL011 to the
configured baud (which should be 115200 for the SBSA UART), which
fixes linux's assumption that SBSA UARTs are pre-programmed for
115200. This in turn (re)enables the PL011 when the console is on the
miniuart per the config.txt file.

Also included is another spin with the DT/XHCI reset patch which puts
removal of the DT node that causes linux to reset the XHCI controller,
as well as an additional patch that updates the DT to match the PCIe
MMIO window we have programmed. This cures much of the problem with
the PCIe/XHCI configuration when booted in DT mode on linux.

There is also a few menu visibility/section tweaks to assure ACPI/DT
specific settings show up at the appropriate time.

As well as a minor fix to work around a bogus compiler warning.

v1->v2: Ard's review comments.
Convert PCIe MMIO address to use the fixedPCDs defined elsewhere
Hardcode 115200 for the SBSA UART

Jeremy Linton (5):
  Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
  Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings
  Platform/RaspberryPi: Cleanup menu visibility
  Platform/RaspberryPi: Give the user control over the XHCI mailbox
  Platform/RaspberryPi: Update PCIe MMIO window for DT

 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 +
 .../Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni|  5 +++
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr| 21 +++--
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  | 26 +++
 .../RaspberryPi/Drivers/FdtDxe/FdtDxe.inf |  7 +++
 .../DualSerialPortLib/DualSerialPortLib.c | 44 ---
 Platform/RaspberryPi/RPi3/RPi3.dsc|  6 +++
 Platform/RaspberryPi/RPi4/RPi4.dsc|  7 +++
 Platform/RaspberryPi/RaspberryPi.dec  |  1 +
 .../Drivers/Net/BcmGenetDxe/GenericPhy.c  |  2 +
 .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c   |  3 ++
 12 files changed, 115 insertions(+), 18 deletions(-)

-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113960): https://edk2.groups.io/g/devel/message/113960
Mute This Topic: https://groups.io/mt/103796306/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] UefiCpuPkg/MpInitLib: Use AsmCpuidEx() for CPUID_EXTENDED_TOPOLOGY leaf

2024-01-17 Thread Lendacky, Thomas via groups.io

On 11/28/23 08:35, Lendacky, Thomas via groups.io wrote:

On 11/6/23 17:15, Tom Lendacky wrote:

On 11/6/23 16:45, Lendacky, Thomas via groups.io wrote:

The CPUID_EXTENDED_TOPOLOGY CPUID leaf takes a subleaf as input when
returning CPUID information. However, the AsmCpuid() function does not
zero out ECX before the CPUID instruction, so the input leaf is used as
the sub-leaf for the CPUID request and returns erroneous/invalid CPUID
data, since the intent of the request was to get data related to sub-leaf
0. Instead, use AsmCpuidEx() for the CPUID_EXTENDED_TOPOLOGY leaf.


Alternatively, the AsmCpuid() function could be changed to XOR ECX 
before invoking the CPUID instruction. This would ensure that the 0 
sub-leaf is returned for any CPUID leaves that support sub-leaves. 
Thoughts?


Adding some additional maintainers for their thoughts, too.


Any thoughts on this approach (as a separate, unrelated patch) to 
eliminate future issues that could pop up?


Seems like zeroing out ECX before calling CPUID would be an appropriate 
thing to do, but I'm not sure if that will have any impact on the existing 
code base... it shouldn't, but you never know.


Just a re-ping for thoughts on this.

Thanks,
Tom



Thanks,
Tom



Thanks,
Tom



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




Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Doug Flick via groups.io
I'll propose a patch to correct this. Building against Ovmf now to confirm it 
corrects the issue.


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




[edk2-devel] [PATCH edk2-platforms 1/1] IpmiFeaturePkg/ServerManagementLib: Fix a GCC compile error

2024-01-17 Thread Xu, Wei6
The source file definition in INF file is ServerManagementELog.c,
while the actual file name is ServerManagementElog.c. The case is
mismatched. Correct the definition in INF file to fix this issue.

Cc: Abner Chang 
Cc: Nate DeSimone 
Signed-off-by: Wei6 Xu 
---
 .../Library/ServerManagementLib/ServerManagementLib.inf   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
index 6e7702e0db33..5c5e8f6d48af 100644
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
+++ 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/ServerManagementLib/ServerManagementLib.inf
@@ -1,6 +1,6 @@
 ### @file
 #
-# Copyright (c) 2023, Intel Corporation. All rights reserved.
+# Copyright (c) 2023 - 2024, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -17,7 +17,7 @@
   LIBRARY_CLASS  = ServerManagementLib
 
 [Sources]
-  ServerManagementELog.c
+  ServerManagementElog.c
   ServerManagementTime.c
 
 [Packages]
-- 
2.29.2.windows.2



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




Re: [edk2-devel] [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

2024-01-17 Thread Saloni Kasbekar
Liming, Mike,

Could you please help merge this PR?

Thanks,
Saloni

From: Ashish Singhal 
Sent: Wednesday, January 17, 2024 6:08 AM
To: Kasbekar, Saloni ; devel@edk2.groups.io; 
Clark-williams, Zachary ; Jeff Brasen 

Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Hello,

Checking back for an update on when this PR can be merged or if there are any 
other changes you recommend.

Thanks
Ashish


From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Saturday, January 6, 2024 5:53 AM
To: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Thanks Saloni. PR for getting this merged is available at 
https://github.com/tianocore/edk2/pull/5150

Thanks
Ashish


From: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>
Sent: Saturday, January 6, 2024 1:31 AM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments


Yes, SetData does reset the previous configuration.



Reviewed-by: Saloni Kasbekar 
mailto:saloni.kasbe...@intel.com>>



Thanks,

Saloni



From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Friday, January 5, 2024 2:34 AM
To: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; 
devel@edk2.groups.io; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



I do not recommend doing that. Setting policy via SetData does enough to wipe 
out any previous manual configuration and that is the goal for reset to default.



From: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>
Sent: Friday, January 5, 2024 2:30 AM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



External email: Use caution opening links or attachments



Makes sense. Should we also set IfrNvData->DhcpEnable = TRUE when updating the 
Policy then?



From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Wednesday, January 3, 2024 8:52 AM
To: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; 
devel@edk2.groups.io; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



Hello Saloni,



Thanks for the feedback. After the reset, or when we disable configure from 
menu, GetData returns policy to static as the enum value is 0. However, setting 
value as static does not have any benefit as it forces to reuse the old network 
settings. Using DHCP really mimics the reset behavior that we see without any 
configuration done manually.



Thanks

Ashish





From: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>
Sent: Tuesday, January 2, 2024 1:47 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



External email: Use caution opening links or attachments



Hi Ashish,



+Ip4NvData->Policy = Ip4Config2PolicyDhcp;

+Status= Ip4Cfg2->SetData (

+   Ip4Cfg2,

+   Ip4Config2DataTypePolicy,

+   sizeof (EFI_IP4_CONFIG2_POLICY),

+   >Policy

+   );



Here we're assuming IfrFormNvData->DhcpEnable is TRUE. Should we check it 
before setting the policy and calling SetData()?



Thanks,

Saloni





From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Monday, January 1, 2024 8:48 AM
To: devel@edk2.groups.io; Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; Clark-williams, 
Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



Hello,



Checking again for some feedback on this.



Thanks

Ashish




Re: [edk2-devel] [PATCH v2 1/2] MdePkg: Adds AMD Extended CPU topology CPUID

2024-01-17 Thread Gerd Hoffmann
On Wed, Jan 17, 2024 at 03:25:37PM +0530, Abdul Lateef Attar via groups.io 
wrote:

Hmm?  Empty patch.  Forgot "git add $file"?

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu topology

2024-01-17 Thread Gerd Hoffmann
> +  /// Check if extended toplogy supported
> +  AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, 
> NULL);
> +  if (MaxExtendedCpuIdIndex >= AMD_CPUID_EXTENDED_TOPOLOGY) {
> +do {
> +  AsmCpuidEx (
> +AMD_CPUID_EXTENDED_TOPOLOGY,
> +TopologyLevel,
> +,
> +,
> +,
> +NULL
> +);
> +
> +  if (ExtendedTopologyEbx.Bits.LogicalProcessors == 
> CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID) {

Comparing LogicalProcessors with CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_*
looks wrong, even though it probably works due to
CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID being zero ...

> +/// if this fails at first level
> +/// then will fall back to non-extended topology
> +break;
> +  }

> +} while (ExtendedTopologyEbx.Bits.LogicalProcessors != 
> CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID);

Same here.  Also this will never be false because the check above
already quits the loop in that case, so you can simplify this to
"while (true)".

take care,
  Gerd



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




Re: [edk2-devel] [PATCH 0/2] OvmfPkg: drop support for TPM 1.2

2024-01-17 Thread Gerd Hoffmann
On Tue, Jan 16, 2024 at 11:38:12PM +, Yao, Jiewen wrote:
> Gerd
> I am OK with the patch.
> 
> Quick question: Have you validated that the TPM2 is still working?

TPM1_ENABLE=FALSE is known to work and this series should give identical
results.  See other replies for more details.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Yao, Jiewen
Hi Marc
I notice you are reviewer for TPM module in OvmfPkg.

Would you please help to test the TPM2.0 feature with patch from Gerd?

Thank you
Yao, Jiewen

> -Original Message-
> From: Gerd Hoffmann 
> Sent: Wednesday, January 17, 2024 10:06 PM
> To: devel@edk2.groups.io; Yao, Jiewen 
> Cc: Li, Yi1 ; dougfl...@microsoft.com; Douglas Flick [MSFT]
> 
> Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
> 
> On Wed, Jan 17, 2024 at 08:23:19AM +, Yao, Jiewen wrote:
> > That is weird.
> > It seems we need to merge Gerd's patch soon -
> https://github.com/tianocore/edk2/pull/5265 to unblock CI.
> >
> > Hi Gerd
> > Would you please confirm what test you have done for removing TPM1.2?
> > Does TPM2.0 in OvmfPkg still work?
> 
> For RHEL we build OVMF with TPM1_ENABLE=FALSE for quite a while without
> seeing any problems, removing the TPM1_ENABLE option altogether should
> give in identical results.  I have to admit that I didn't actually test
> that though.
> 
> take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113952): https://edk2.groups.io/g/devel/message/113952
Mute This Topic: https://groups.io/mt/103675434/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/2] OvmfPkg/Tcg2Config: remove unused TPM 1.2 support

2024-01-17 Thread Gerd Hoffmann
> This patch is good:
> 
> Reviewed-by: Laszlo Ersek 
> 
> but the series shouldn't stop here. In "OvmfPkg/Tcg/Tcg2Config", we're
> left with an INF file (Tcg2ConfigPei.inf) that still references
> "Tpm12Support.h", and the common C source file "Tcg2ConfigPeim.c" still
> calls the one API -- InternalTpm12Detect() -- declared in that header
> file. The only remaining implementation of InternalTpm12Detect() is now
> in "Tpm12SupportNull.c", and all it does is "return EFI_UNSUPPORTED".
> 
> Therefore, in a subsequent patch, "Tpm12SupportNull.c" and
> "Tpm12Support.h" should be removed, both from the tree, and from the
> remaining INF file. Furthermore, the InternalTpm12Detect() call in
> "Tcg2ConfigPeim.c", and everything that depends on the success of that
> call, now counts as dead code, and should be removed.
> 
> And *that* in turn means that we should also remove
> "gEfiTpmDeviceInstanceTpm12Guid" from the [Guids] section of the
> remaining INF file.

Yes, I noticed there is more to cleanup, but I wanted have something
quick and proven (== identical to setting TPM1_ENABLE=FALSE) out of
the door to fix the build / CI problems.

Touching the code for additional cleanups requires a bit more attention
and testing, I'll keep that on my radar though.

thanks & take care,
  Gerd



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




Re: [edk2-devel] [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

2024-01-17 Thread Ashish Singhal via groups.io
Hello,

Checking back for an update on when this PR can be merged or if there are any 
other changes you recommend.

Thanks
Ashish


From: Ashish Singhal 
Sent: Saturday, January 6, 2024 5:53 AM
To: Kasbekar, Saloni ; devel@edk2.groups.io 
; Clark-williams, Zachary 
; Jeff Brasen 
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Thanks Saloni. PR for getting this merged is available at 
https://github.com/tianocore/edk2/pull/5150

Thanks
Ashish


From: Kasbekar, Saloni 
Sent: Saturday, January 6, 2024 1:31 AM
To: Ashish Singhal ; devel@edk2.groups.io 
; Clark-williams, Zachary 
; Jeff Brasen 
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments


Yes, SetData does reset the previous configuration.



Reviewed-by: Saloni Kasbekar 



Thanks,

Saloni



From: Ashish Singhal 
Sent: Friday, January 5, 2024 2:34 AM
To: Kasbekar, Saloni ; devel@edk2.groups.io; 
Clark-williams, Zachary ; Jeff Brasen 

Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



I do not recommend doing that. Setting policy via SetData does enough to wipe 
out any previous manual configuration and that is the goal for reset to default.



From: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>
Sent: Friday, January 5, 2024 2:30 AM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



External email: Use caution opening links or attachments



Makes sense. Should we also set IfrNvData->DhcpEnable = TRUE when updating the 
Policy then?



From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Wednesday, January 3, 2024 8:52 AM
To: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; 
devel@edk2.groups.io; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



Hello Saloni,



Thanks for the feedback. After the reset, or when we disable configure from 
menu, GetData returns policy to static as the enum value is 0. However, setting 
value as static does not have any benefit as it forces to reuse the old network 
settings. Using DHCP really mimics the reset behavior that we see without any 
configuration done manually.



Thanks

Ashish





From: Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>
Sent: Tuesday, January 2, 2024 1:47 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Clark-williams, Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



External email: Use caution opening links or attachments



Hi Ashish,



+Ip4NvData->Policy = Ip4Config2PolicyDhcp;

+Status= Ip4Cfg2->SetData (

+   Ip4Cfg2,

+   Ip4Config2DataTypePolicy,

+   sizeof (EFI_IP4_CONFIG2_POLICY),

+   >Policy

+   );



Here we’re assuming IfrFormNvData->DhcpEnable is TRUE. Should we check it 
before setting the policy and calling SetData()?



Thanks,

Saloni





From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Monday, January 1, 2024 8:48 AM
To: devel@edk2.groups.io; Kasbekar, Saloni 
mailto:saloni.kasbe...@intel.com>>; Clark-williams, 
Zachary 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



Hello,



Checking again for some feedback on this.



Thanks

Ashish





From: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Sent: Thursday, December 14, 2023 4:42 PM
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; 
saloni.kasbe...@intel.com 
mailto:saloni.kasbe...@intel.com>>; 
zachary.clark-willi...@intel.com 
mailto:zachary.clark-willi...@intel.com>>; 
Jeff Brasen mailto:jbra...@nvidia.com>>
Cc: Ashish Singhal mailto:ashishsin...@nvidia.com>>
Subject: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default



Exercising reset to default does not reset the settings.
Add handler code for the case where configuration is
disabled.

Signed-off-by: Ashish Singhal 
mailto:ashishsin...@nvidia.com>>
---
 NetworkPkg/Ip4Dxe/Ip4Config2Nv.c | 25 +
 1 file changed, 25 insertions(+)

diff --git 

Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Gerd Hoffmann
On Wed, Jan 17, 2024 at 08:23:19AM +, Yao, Jiewen wrote:
> That is weird.
> It seems we need to merge Gerd's patch soon - 
> https://github.com/tianocore/edk2/pull/5265 to unblock CI.
> 
> Hi Gerd
> Would you please confirm what test you have done for removing TPM1.2?
> Does TPM2.0 in OvmfPkg still work?

For RHEL we build OVMF with TPM1_ENABLE=FALSE for quite a while without
seeing any problems, removing the TPM1_ENABLE option altogether should
give in identical results.  I have to admit that I didn't actually test
that though.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113949): https://edk2.groups.io/g/devel/message/113949
Mute This Topic: https://groups.io/mt/103675434/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] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-17 Thread Laszlo Ersek
On 1/17/24 09:09, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614
> 
> About the IsModified, current function doesn't consider that hardware
> also may change the pagetable. The issue is that in the first call of
> internal function PageTableLibMapInLevel, the function assume page
> table is not changed, and add ASSERT to check. But hardware may change
> the page table, which cause the ASSERT happens.
> Fix the issue by considering the hardware may also change page table,
> and document the detail in function header.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Crystal Lee 
> Signed-off-by: Zhiguang Liu 
> ---
>  .../Library/CpuPageTableLib/CpuPageTableMap.c| 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c 
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 36b2c4e6a3..a3076ff2f6 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -274,7 +274,7 @@ IsAttributesAndMaskValidForNonPresentEntry (
>  Page table entries that map the linear 
> address range are reset to 0 before set to the new attribute
>  when a new physical base address is set.
>@param[in]  Mask  The mask used for attribute. The 
> corresponding field in Attribute is ignored if that in Mask is 0.
> -  @param[out] IsModifiedTRUE means page table is modified. FALSE 
> means page table is not modified.
> +  @param[in, out] IsModifiedChange IsModified to True if page table 
> is modified and input parameter Modify is TRUE.
>  
>@retval RETURN_INVALID_PARAMETER  For non-present range, 
> Mask->Bits.Present is 0 but some other attributes are provided.
>@retval RETURN_INVALID_PARAMETER  For non-present range, 
> Mask->Bits.Present is 1, Attribute->Bits.Present is 1 but some other 
> attributes are not provided.
> @@ -567,7 +567,10 @@ PageTableLibMapInLevel (
>  OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
>  PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, 
> );
>  
> -if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) 
> {
> +if (Modify && (OriginalCurrentPagingEntry.Uint64 != 
> CurrentPagingEntry->Uint64)) {
> +  //
> +  // The page table entry can be changed by this function only when 
> Modify is true.
> +  //
>*IsModified = TRUE;
>  }
>}
> @@ -609,7 +612,10 @@ PageTableLibMapInLevel (
>// Check if ParentPagingEntry entry is modified here is enough. Except the 
> changes happen in leaf PagingEntry during
>// the while loop, if there is any other change happens in page table, the 
> ParentPagingEntry must has been modified.
>//
> -  if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) {
> +  if (Modify && (OriginalParentPagingEntry.Uint64 != 
> ParentPagingEntry->Uint64)) {
> +//
> +// The page table entry can be changed by this function only when Modify 
> is true.
> +//
>  *IsModified = TRUE;
>}
>  
> @@ -633,7 +639,9 @@ PageTableLibMapInLevel (
>   Page table entries that map the linear 
> address range are reset to 0 before set to the new attribute
>   when a new physical base address is set.
>@param[in]  Mask   The mask used for attribute. The 
> corresponding field in Attribute is ignored if that in Mask is 0.
> -  @param[out] IsModified TRUE means page table is modified. FALSE 
> means page table is not modified.
> +  @param[out] IsModified TRUE means page table is modified by 
> software or hardware. FALSE means page table is not modified by software.
> + If the output IsModified is FALSE, there is 
> possibility that the page table is changed by hardware. It is ok
> + because page table can be changed by 
> hardware anytime, and caller don't need to Flush TLB.
>  
>@retval RETURN_UNSUPPORTEDPagingMode is not supported.
>@retval RETURN_INVALID_PARAMETER  PageTable, BufferSize, Attribute or Mask 
> is NULL.

This patch looks good to me, thanks, except for one small wart: in the
documentation section of PageTableLibMapInLevel(), you change IsModified
from "@param[out]" to "@param[in, out]", which is correct, *but* a
similar change has been omitted for the actual parameter in the
parameter list:

  OUTBOOLEAN *IsModified

This should also become "IN OUT".

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113948): https://edk2.groups.io/g/devel/message/113948
Mute This Topic: https://groups.io/mt/103781942/21656
Group Owner: 

Re: [edk2-devel] [PATCH 1/6] UefiCpuPkg/LocalApicTimerDxe: Duplicate OvmfPkg/LocalApicTimerDxe driver

2024-01-17 Thread Michael Brown

On 17/01/2024 07:11, Ni, Ray wrote:

The above flow shows endless re-entrance of timer interrupt handler.

But, my question is: above flow only can happen in real platform when the below 
4 steps occupies more time than the timer period (usually 10ms).
 [Timer Interrupt #2]1. RaiseTPL (HIGH) from NOTIFY causing CPU 
interrupt be disabled.
 [Timer Interrupt #2]2. Send APIC EOI (ACK the interrupt received 
so APIC can continue generate interrupts)
 [Timer Interrupt #2]3. Call DxeCore::CoreTimerTick()
 [Timer Interrupt #2]4. RestoreTPL (NOTIFY) from HIGH. No callback 
runs as no callback can be registered at TPL > NOTIFY. In the end of 
RestoreTPL(), CPU interrupt is enabled.

But, in my opinion, it's impossible.


As is thoroughly documented in NestedInterruptRestoreTpl(), the 
potential for unbounded stack consumption arises when an interrupt 
occurs after the point that RestoreTPL() completes dispatching all 
notifications but before the IRET (or equivalent) instruction pops the 
original stack frame.


Since dispatching notifications can take an unbounded amount of time, 
there is absolutely no guarantee that this will be less than 10ms after 
the previous interrupt.  It could easily be 30 seconds later.


The problematic flow is a subtle variation on what you described:

  [IRQ#1] timer interrupt at TPL_APPLICATION
[ISR#1] RaiseTPL from TPL_APPLICATION -> TPL_HIGH_LEVEL
[ISR#1] Send APIC EOI
[ISR#1] Call CoreTimerTick()
[ISR#1] RestoreTPL from TPL_HIGH_LEVEL -> TPL_APPLICATION
  [ISR#1] Callbacks for TPL_NOTIFY are run
  [ISR#1] Callbacks for TPL_CALLBACK are run
  ... these may take several *seconds* to complete, during
  which further interrupts are raised, the details of
  which are not shown here...
  [ISR#1] TPL is now restored to TPL_APPLICATION
  [IRQ#N] timer interrupt at TPL_APPLICATION
[ISR#N] RaiseTPL from TPL_APPLICATION -> TPL_HIGH_LEVEL
... continues ...

The root cause is that the ISR reaches a state in which:

  a) an arbitrary amount of time has elapsed since the triggering
 interrupt (due to unknown callbacks being invoked, which may
 themselves wait for further timer interrupts), and

  b) the TPL has been fully restored back to the TPL at the point
 the triggering interrupt occurred (i.e. TPL_APPLICATION in
 this example), and

  c) the timer interrupt source is enabled, and

  d) CPU interrupts are enabled

At this point, there is nothing preventing another interrupt from 
occurring.  It will occur at TPL_APPLICATION and it will be one stack 
frame deeper than the previous interrupt at TPL_APPLICATION.


Rinse and repeat, and you have unbounded stack consumption.

Hence the requirement for NestedInterruptTplLib, even on physical hardware.

Michael



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




Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/CpuMpPei: Don't write CR3 in ConvertMemoryPageToNotPresent

2024-01-17 Thread Laszlo Ersek
On 1/17/24 07:22, Zhiguang Liu wrote:
> The purpose of writing CR3 in ConvertMemoryPageToNotPresent is just
> to flush TLB, because CR3 won't be changed in function
> ConvertMemoryPageToNotPresent.
> After ConvertMemoryPageToNotPresent, there is always a flush TLB
> function. Also, because ConvertMemoryPageToNotPresent in called in a
> loop, to improve performance, there is no need to flush TLB
> inside ConvertMemoryPageToNotPresent. Just flushing TLB after the loop
> is enough.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/CpuMpPei/CpuPaging.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 15c7015fb8..b923d9b502 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -76,7 +76,8 @@ AllocatePageTableMemory (
>  
>  /**
>This function modifies the page attributes for the memory region specified
> -  by BaseAddress and Length to not present.
> +  by BaseAddress and Length to not present. This function only change page
> +  table, but not flush TLB. Caller have the responsbility to flush TLB.
>  
>Caller should make sure BaseAddress and Length is at page boundary.
>  
> @@ -167,7 +168,6 @@ ConvertMemoryPageToNotPresent (
>}
>  
>ASSERT_EFI_ERROR (Status);
> -  AsmWriteCr3 (PageTable);
>return Status;
>  }
>  

Reviewed-by: Laszlo Ersek 



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




Re: [edk2-devel] [PATCH v2 1/2] UefiCpuPkg/CpuPageTableLib: Enhance function header for PageTableMap()

2024-01-17 Thread Laszlo Ersek
On 1/17/24 07:21, Zhiguang Liu wrote:
> PageTableMap() only modifies the PageTable root pointer when creating from 
> zero.
> Explicitly explain it in function header.
> 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Signed-off-by: Zhiguang Liu 
> ---
>  UefiCpuPkg/Include/Library/CpuPageTableLib.h | 1 +
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/UefiCpuPkg/Include/Library/CpuPageTableLib.h 
> b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> index 78aa83b8de..755c8ab084 100644
> --- a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> +++ b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> @@ -61,6 +61,7 @@ typedef enum {
>Create or update page table to map [LinearAddress, LinearAddress + Length) 
> with specified attribute.
>  
>@param[in, out] PageTable  The pointer to the page table to update, or 
> pointer to NULL if a new page table is to be created.
> + If not pointer to NULL, the value it points 
> to won't be changed in this function.
>@param[in]  PagingMode The paging mode.
>@param[in]  Buffer The free buffer to be used for page table 
> creation/updating.
>@param[in, out] BufferSize The buffer size.
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c 
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 36b2c4e6a3..25bd9fc8d8 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -620,6 +620,7 @@ PageTableLibMapInLevel (
>Create or update page table to map [LinearAddress, LinearAddress + Length) 
> with specified attribute.
>  
>@param[in, out] PageTable  The pointer to the page table to update, or 
> pointer to NULL if a new page table is to be created.
> + If not pointer to NULL, the value it points 
> to won't be changed in this function.
>@param[in]  PagingMode The paging mode.
>@param[in]  Buffer The free buffer to be used for page table 
> creation/updating.
>@param[in, out] BufferSize The buffer size.

Reviewed-by: Laszlo Ersek 

(applying this is going to be difficult, because the patch emails were
not sent in response to a common cover letter, or in response to each other)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113945): https://edk2.groups.io/g/devel/message/113945
Mute This Topic: https://groups.io/mt/103781121/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 v3 6/6] OvmfPkg/VirtNorFlashDxe: move DoErase code block into new function

2024-01-17 Thread Laszlo Ersek
On 1/16/24 18:11, Gerd Hoffmann wrote:
> Move the DoErase code block into a separate function, call the function
> instead of jumping around with goto.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c | 76 ++
>  1 file changed, 52 insertions(+), 24 deletions(-)
> 
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c 
> b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> index 3d1d20daa1e5..e6aaed27ceba 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> @@ -502,6 +502,38 @@ NorFlashRead (
>return EFI_SUCCESS;
>  }
>  
> +STATIC
> +EFI_STATUS
> +NorFlashWriteSingleBlockWithErase (
> +  INNOR_FLASH_INSTANCE  *Instance,
> +  INEFI_LBA Lba,
> +  INUINTN   Offset,
> +  IN OUTUINTN   *NumBytes,
> +  INUINT8   *Buffer
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  // Read NOR Flash data into shadow buffer
> +  Status = NorFlashReadBlocks (Instance, Lba, Instance->BlockSize, 
> Instance->ShadowBuffer);
> +  if (EFI_ERROR (Status)) {
> +// Return one of the pre-approved error statuses
> +return EFI_DEVICE_ERROR;
> +  }
> +
> +  // Put the data at the appropriate location inside the buffer area
> +  CopyMem ((VOID *)((UINTN)Instance->ShadowBuffer + Offset), Buffer, 
> *NumBytes);
> +
> +  // Write the modified buffer back to the NorFlash
> +  Status = NorFlashWriteBlocks (Instance, Lba, Instance->BlockSize, 
> Instance->ShadowBuffer);
> +  if (EFI_ERROR (Status)) {
> +// Return one of the pre-approved error statuses
> +return EFI_DEVICE_ERROR;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /*
>Write a full or portion of a block. It must not span block boundaries; 
> that is,
>Offset + *NumBytes <= Instance->BlockSize.
> @@ -607,7 +639,14 @@ NorFlashWriteSingleBlock (
>  // that we want to set. In that case, we will need to erase the block 
> first.
>  for (CurOffset = 0; CurOffset < *NumBytes; CurOffset++) {
>if (~(UINT32)OrigData[CurOffset] & (UINT32)Buffer[CurOffset]) {
> -goto DoErase;
> +Status = NorFlashWriteSingleBlockWithErase (
> +   Instance,
> +   Lba,
> +   Offset,
> +   NumBytes,
> +   Buffer
> +   );
> +return Status;
>}
>  
>OrigData[CurOffset] = Buffer[CurOffset];
> @@ -636,33 +675,22 @@ NorFlashWriteSingleBlock (
>  goto Exit;
>}
>  }
> -
> -Exit:
> -// Put device back into Read Array mode
> -SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
> -
> +  } else {
> +Status = NorFlashWriteSingleBlockWithErase (
> +   Instance,
> +   Lba,
> +   Offset,
> +   NumBytes,
> +   Buffer
> +   );
>  return Status;
>}
>  
> -DoErase:
> -  // Read NOR Flash data into shadow buffer
> -  Status = NorFlashReadBlocks (Instance, Lba, BlockSize, 
> Instance->ShadowBuffer);
> -  if (EFI_ERROR (Status)) {
> -// Return one of the pre-approved error statuses
> -return EFI_DEVICE_ERROR;
> -  }
> +Exit:
> +  // Put device back into Read Array mode
> +  SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
>  
> -  // Put the data at the appropriate location inside the buffer area
> -  CopyMem ((VOID *)((UINTN)Instance->ShadowBuffer + Offset), Buffer, 
> *NumBytes);
> -
> -  // Write the modified buffer back to the NorFlash
> -  Status = NorFlashWriteBlocks (Instance, Lba, BlockSize, 
> Instance->ShadowBuffer);
> -  if (EFI_ERROR (Status)) {
> -// Return one of the pre-approved error statuses
> -return EFI_DEVICE_ERROR;
> -  }
> -
> -  return EFI_SUCCESS;
> +  return Status;
>  }
>  
>  EFI_STATUS

Reviewed-by: Laszlo Ersek 

This series is now ready to be merged, but I'm unsure what the CI status
is, after the recent security fixes seem to have caused breakage with
multiple external definitions of the same symbol. For now I'll keep this
tagged, as a reminder for myself.

Laszlo



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




[edk2-devel] [PATCH v2 2/2] UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu topology

2024-01-17 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

This patch adds support for AMD's new extended topology.
If processor supports CPUID 8026 leaf then obtain
the topology information using new method.

Algorithm:
  if CPUID is AMD:
then
 check for AMD's extended cpu tology leaf.
 if yes
   then extract cpu tology based on
   AMD programmer manual's instruction.
 else
   then fallback to existing topology function.
endif
  endif

Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Abdul Lateef Attar 
---
 .../Library/BaseXApicLib/BaseXApicLib.c   | 126 +-
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 126 +-
 2 files changed, 250 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c 
b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index efb9d71ca1..c4457d98b3 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -4,7 +4,7 @@
   This local APIC library instance supports xAPIC mode only.
 
   Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
-  Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.
+  Copyright (c) 2017 - 2024, AMD Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1157,6 +1157,125 @@ GetProcessorLocationByApicId (
   }
 }
 
+/**
+  Get Package ID/Die ID/Module ID/Core ID/Thread ID of a AMD processor family.
+
+  The algorithm assumes the target system has symmetry across physical
+  package boundaries with respect to the number of threads per core, number of
+  cores per module, number of modules per die, number
+  of dies per package.
+
+  @param[in]   InitialApicId Initial APIC ID of the target logical processor.
+  @param[out]  Package   Returns the processor package ID.
+  @param[out]  Die   Returns the processor die ID.
+  @param[out]  Tile  Returns zero.
+  @param[out]  ModuleReturns the processor module ID.
+  @param[out]  Core  Returns the processor core ID.
+  @param[out]  ThreadReturns the processor thread ID.
+**/
+VOID
+AmdGetProcessorLocation2ByApicId (
+  IN  UINT32  InitialApicId,
+  OUT UINT32  *Package  OPTIONAL,
+  OUT UINT32  *Die  OPTIONAL,
+  OUT UINT32  *Tile OPTIONAL,
+  OUT UINT32  *Module   OPTIONAL,
+  OUT UINT32  *Core OPTIONAL,
+  OUT UINT32  *Thread   OPTIONAL
+  )
+{
+  CPUID_EXTENDED_TOPOLOGY_EAX  ExtendedTopologyEax;
+  CPUID_EXTENDED_TOPOLOGY_EBX  ExtendedTopologyEbx;
+  CPUID_EXTENDED_TOPOLOGY_ECX  ExtendedTopologyEcx;
+  UINT32   MaxExtendedCpuIdIndex;
+  UINT32   TopologyLevel;
+  UINT32   PreviousLevel;
+  UINT32   Data;
+
+  if (Die != NULL) {
+*Die = 0;
+  }
+
+  if (Tile != NULL) {
+*Tile = 0;
+  }
+
+  if (Module != NULL) {
+*Module = 0;
+  }
+
+  PreviousLevel = 0;
+  TopologyLevel = 0;
+
+  /// Check if extended toplogy supported
+  AsmCpuid (CPUID_EXTENDED_FUNCTION, , NULL, NULL, NULL);
+  if (MaxExtendedCpuIdIndex >= AMD_CPUID_EXTENDED_TOPOLOGY) {
+do {
+  AsmCpuidEx (
+AMD_CPUID_EXTENDED_TOPOLOGY,
+TopologyLevel,
+,
+,
+,
+NULL
+);
+
+  if (ExtendedTopologyEbx.Bits.LogicalProcessors == 
CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID) {
+/// if this fails at first level
+/// then will fall back to non-extended topology
+break;
+  }
+
+  Data  = InitialApicId >> PreviousLevel;
+  Data &= (1 << (ExtendedTopologyEax.Bits.ApicIdShift - PreviousLevel)) - 
1;
+
+  switch (ExtendedTopologyEcx.Bits.LevelType) {
+case CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_SMT:
+  if (Thread != NULL) {
+*Thread = Data;
+  }
+
+  break;
+case CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_CORE:
+  if (Core != NULL) {
+*Core = Data;
+  }
+
+  break;
+case CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_MODULE:
+  if (Module != NULL) {
+*Module = Data;
+  }
+
+  break;
+case CPUID_V2_EXTENDED_TOPOLOGY_LEVEL_TYPE_TILE:
+  if (Die != NULL) {
+*Die = Data;
+  }
+
+  break;
+default:
+  break;
+  }
+
+  TopologyLevel++;
+  PreviousLevel = ExtendedTopologyEax.Bits.ApicIdShift;
+} while (ExtendedTopologyEbx.Bits.LogicalProcessors != 
CPUID_EXTENDED_TOPOLOGY_LEVEL_TYPE_INVALID);
+
+if (Package != NULL) {
+  *Package = InitialApicId >> PreviousLevel;
+}
+  }
+
+  /// If extended topology CPUID is not supported
+  /// OR, execution of AMD_CPUID_EXTENDED_TOPOLOGY at level 0 fails(return 0).
+  if (TopologyLevel == 0) {
+GetProcessorLocationByApicId (InitialApicId, Package, Core, Thread);
+  }
+
+  return;
+}
+
 /**
   Get Package ID/Die ID/Tile ID/Module ID/Core ID/Thread ID of a processor.
 
@@ -1194,6 +1313,11 @@ 

[edk2-devel] [PATCH v2 1/2] MdePkg: Adds AMD Extended CPU topology CPUID

2024-01-17 Thread Abdul Lateef Attar via groups.io



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




[edk2-devel] [PATCH v2 0/2] AMD CPU extended topology

2024-01-17 Thread Abdul Lateef Attar via groups.io
From: Abdul Lateef Attar 

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

V2: delta changes
 - Added additional check apart from supported CPUID extneded number.
 - removed EFIAPI

Implements AMD extended toplogy.
Adds CPUID macro
update APIC library to use new exteded cpuid.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Abdul Lateef Attar 

Abdul Lateef Attar (2):
  MdePkg: Adds AMD Extended CPU topology CPUID
  UefiCpuPkg/BaseXApic[X2]ApicLib: Implements AMD extended cpu topology

 MdePkg/Include/Register/Amd/Cpuid.h   |  23 +++-
 .../Library/BaseXApicLib/BaseXApicLib.c   | 126 +-
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 126 +-
 3 files changed, 272 insertions(+), 3 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113941): https://edk2.groups.io/g/devel/message/113941
Mute This Topic: https://groups.io/mt/103782765/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/2] OvmfPkg/Tcg2Config: remove unused TPM 1.2 support

2024-01-17 Thread Laszlo Ersek
On 1/16/24 16:42, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf | 56 ---
>  OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 83 ---
>  2 files changed, 139 deletions(-)
>  delete mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
>  delete mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> 
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf 
> b/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
> deleted file mode 100644
> index e8e0b88e6058..
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -## @file
> -# Set TPM device type - supports TPM 1.2 and 2.0
> -#
> -# In SecurityPkg, this module initializes the TPM device type based on a UEFI
> -# variable and/or hardware detection. In OvmfPkg, the module only performs 
> TPM
> -# hardware detection.
> -#
> -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> -# Copyright (C) 2018, Red Hat, Inc.
> -#
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> -##
> -
> -[Defines]
> -  INF_VERSION= 0x00010005
> -  BASE_NAME  = Tcg2ConfigPei
> -  FILE_GUID  = 8AD3148F-945F-46B4-8ACD-71469EA73945
> -  MODULE_TYPE= PEIM
> -  VERSION_STRING = 1.0
> -  ENTRY_POINT= Tcg2ConfigPeimEntryPoint
> -
> -[Sources]
> -  Tcg2ConfigPeim.c
> -  Tpm12Support.h
> -  Tpm12Support.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
> -  OvmfPkg/OvmfPkg.dec
> -  SecurityPkg/SecurityPkg.dec
> -
> -[LibraryClasses]
> -  PeimEntryPoint
> -  DebugLib
> -  PeiServicesLib
> -  Tpm2DeviceLib
> -  BaseLib
> -  Tpm12DeviceLib
> -
> -[Guids]
> -  gEfiTpmDeviceSelectedGuid   ## PRODUCES ## GUID # Used as a PPI 
> GUID
> -  gEfiTpmDeviceInstanceTpm20DtpmGuid  ## SOMETIMES_CONSUMES
> -  gEfiTpmDeviceInstanceTpm12Guid  ## SOMETIMES_CONSUMES
> -
> -[Ppis]
> -  gPeiTpmInitializationDonePpiGuid## SOMETIMES_PRODUCES
> -
> -[Pcd]
> -  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## 
> PRODUCES
> -
> -[Depex.IA32, Depex.X64]
> -  gOvmfTpmMmioAccessiblePpiGuid
> -
> -[Depex.ARM, Depex.AARCH64]
> -  gOvmfTpmDiscoveredPpiGuid
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c 
> b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> deleted file mode 100644
> index c88da5758b44..
> --- a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> +++ /dev/null
> @@ -1,83 +0,0 @@
> -/** @file
> -  Implement the InternalTpm12Detect() function on top of the Tpm12DeviceLib
> -  class.
> -
> -  Copyright (C) 2020, Red Hat, Inc.
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include 
> -#include 
> -
> -#include "Tpm12Support.h"
> -
> -#pragma pack (1)
> -typedef struct {
> -  TPM_RSP_COMMAND_HDRHdr;
> -  TPM_CURRENT_TICKS  CurrentTicks;
> -} TPM_RSP_GET_TICKS;
> -#pragma pack ()
> -
> -/**
> -  Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
> -
> -  Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> -  header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> -
> -  @retval EFI_SUCCESS  TPM version 1.2 probing successful.
> -
> -  @return  Error codes propagated from Tpm12SubmitCommand().
> -**/
> -STATIC
> -EFI_STATUS
> -TestTpm12 (
> -  )
> -{
> -  EFI_STATUS   Status;
> -  TPM_RQU_COMMAND_HDR  Command;
> -  TPM_RSP_GET_TICKSResponse;
> -  UINT32   Length;
> -
> -  Command.tag   = SwapBytes16 (TPM_TAG_RQU_COMMAND);
> -  Command.paramSize = SwapBytes32 (sizeof (Command));
> -  Command.ordinal   = SwapBytes32 (TPM_ORD_GetTicks);
> -
> -  Length = sizeof (Response);
> -  Status = Tpm12SubmitCommand (
> - sizeof (Command),
> - (UINT8 *),
> - ,
> - (UINT8 *)
> - );
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Detect the presence of a TPM with interface version 1.2.
> -
> -  @retval EFI_SUCCESS  TPM-1.2 available. The Tpm12RequestUseTpm() and
> -   Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
> -   (from the Tpm12DeviceLib class) have succeeded.
> -
> -  @return  Error codes propagated from Tpm12RequestUseTpm() 
> and
> -   Tpm12SubmitCommand().
> -**/
> -EFI_STATUS
> -InternalTpm12Detect (
> -  VOID
> -  )
> -{
> -  EFI_STATUS  Status;
> -
> -  Status = Tpm12RequestUseTpm ();
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -
> -  return TestTpm12 ();
> -}

This patch is good:

Reviewed-by: Laszlo Ersek 

but the series shouldn't stop here. In "OvmfPkg/Tcg/Tcg2Config", we're
left with an INF file (Tcg2ConfigPei.inf) that still references
"Tpm12Support.h", and the common C source file "Tcg2ConfigPeim.c" still
calls the one API -- InternalTpm12Detect() -- declared in that header
file. The only 

Re: [edk2-devel] [PATCH 1/2] OvmfPkg: remove TPM1_ENABLE build option

2024-01-17 Thread Laszlo Ersek
On 1/16/24 16:42, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc | 6 --
>  OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc | 5 -
>  OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc   | 3 ---
>  OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc  | 9 -
>  OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc  | 6 --
>  OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc   | 3 ---
>  OvmfPkg/Include/Fdf/OvmfTpmPei.fdf.inc   | 5 -
>  OvmfPkg/PlatformCI/ReadMe.md | 2 +-
>  8 files changed, 1 insertion(+), 38 deletions(-)
> 
> diff --git a/OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc 
> b/OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc
> index 75ae09571e8c..eef20b77149a 100644
> --- a/OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/OvmfTpmComponentsDxe.dsc.inc
> @@ -15,12 +15,6 @@
>NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>}
>SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> -!if $(TPM1_ENABLE) == TRUE
> -  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {
> -
> -  
> Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> -  }
> -!endif
>SecurityPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf {
>  
>
> TpmPlatformHierarchyLib|SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf
> diff --git a/OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc 
> b/OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc
> index fa486eed82d2..b91f29e5a64b 100644
> --- a/OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/OvmfTpmComponentsPei.dsc.inc
> @@ -4,12 +4,7 @@
>  
>  !if $(TPM2_ENABLE) == TRUE
>OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> -!if $(TPM1_ENABLE) == TRUE
> -  OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
> -  SecurityPkg/Tcg/TcgPei/TcgPei.inf
> -!else
>OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> -!endif
>SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
>  
>
> HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> diff --git a/OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc 
> b/OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc
> index a65564d8d9d2..ad3740a4737a 100644
> --- a/OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc
> @@ -3,6 +3,3 @@
>  ##
>  
>DEFINE TPM2_ENABLE = FALSE
> -
> -  # has no effect unless TPM2_ENABLE == TRUE
> -  DEFINE TPM1_ENABLE = TRUE
> diff --git a/OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc 
> b/OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> index b97244695b52..e02a5d02d1a5 100644
> --- a/OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/OvmfTpmLibs.dsc.inc
> @@ -4,9 +4,6 @@
>  
>  [LibraryClasses]
>  !if $(TPM2_ENABLE) == TRUE
> -!if $(TPM1_ENABLE) == TRUE
> -  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
> -!endif
>Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
>
> Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
>
> Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> @@ -27,16 +24,10 @@ [LibraryClasses]
>  [LibraryClasses.common.PEIM]
>  !if $(TPM2_ENABLE) == TRUE
>BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> -!if $(TPM1_ENABLE) == TRUE
> -  
> Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> -!endif
>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
>  !endif
>  
>  [LibraryClasses.common.DXE_DRIVER]
>  !if $(TPM2_ENABLE) == TRUE
> -!if $(TPM1_ENABLE) == TRUE
> -  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
> -!endif
>Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
>  !endif
> diff --git a/OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc 
> b/OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc
> index 89455feca4d9..c40d6b0a0e78 100644
> --- a/OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc
> +++ b/OvmfPkg/Include/Dsc/OvmfTpmSecurityStub.dsc.inc
> @@ -2,12 +2,6 @@
>  #SPDX-License-Identifier: BSD-2-Clause-Patent
>  ##
>  
> -!if $(TPM2_ENABLE) == TRUE
> -!if $(TPM1_ENABLE) == TRUE
> -  NULL|SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf
> -!endif
> -!endif
> -
>  !if $(TPM2_ENABLE) == TRUE || $(CC_MEASUREMENT_ENABLE) == TRUE
>#
># DxeTpm2MeasureBootLib provides security service of TPM2 measure boot 
> and
> diff --git a/OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc 
> b/OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
> index 7fc2bf8590a4..bd0be8fedbd5 100644
> --- a/OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
> +++ b/OvmfPkg/Include/Fdf/OvmfTpmDxe.fdf.inc
> @@ -3,9 +3,6 @@
>  ##
>  
>  !if $(TPM2_ENABLE) == TRUE
> -!if $(TPM1_ENABLE) == TRUE
> -INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf
> -!endif
>  INF  

Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Yao, Jiewen
That is weird.
It seems we need to merge Gerd's patch soon - 
https://github.com/tianocore/edk2/pull/5265 to unblock CI.

Hi Gerd
Would you please confirm what test you have done for removing TPM1.2?
Does TPM2.0 in OvmfPkg still work?

Hi Doug
I cannot tell why CI passed before but failed now.
But it does seems a big issue now. Would you please propose a patch to resolve 
it? Just rename the symbol.

Thank you
Yao, Jiewen

> -Original Message-
> From: Li, Yi1 
> Sent: Wednesday, January 17, 2024 4:15 PM
> To: Yao, Jiewen ; devel@edk2.groups.io; Gerd Hoffmann
> 
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
> 
> Hi Jiewen,
> 
> Sounds strange, but new PRs in today all broken due to this issue, e.g.:
> https://github.com/tianocore/edk2/pull/5210
> https://github.com/tianocore/edk2/pull/5268
> 
> 
> I checked build log, it matched the description from Gerd:
> https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-
> 7def1f19d478/_apis/build/builds/114097/logs/350
> 2024-01-17T04:09:52.5996237Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6010570Z INFO - (.text+0x0): multiple definition of
> `SanitizeEfiPartitionTableHeader'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6020435Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6030987Z INFO - (.text+0x0): multiple definition of
> `SanitizePrimaryHeaderAllocationSize'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6040167Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6050625Z INFO - (.text+0x0): multiple definition of
> `SanitizePrimaryHeaderGptEventSize'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6061966Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6072661Z INFO - (.text+0x0): multiple definition of
> `SanitizePeImageEventSize'; DxeTpmMeasureBootLibSanitization.obj (symbol
> from plugin):(.text+0x0): first defined here
> 2024-01-17T04:10:12.9532147Z INFO - build.py...
> 2024-01-17T04:10:12.9593220Z INFO -  : error 7000: Failed to execute command
> 2024-01-17T04:10:23.2054653Z INFO - build.py...
> 2024-01-17T04:10:23.2055014Z INFO -  : error F002: Failed to build module
> 2024-01-17T04:10:23.2055379Z INFO -
>   /__w/1/s/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.i
> nf [X64, GCC5, DEBUG]
> 
> -Original Message-
> From: Yao, Jiewen 
> Sent: Wednesday, January 17, 2024 4:09 PM
> To: Li, Yi1 ; devel@edk2.groups.io; Gerd Hoffmann
> 
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
> 
> Please check https://github.com/tianocore/edk2/pull/5264. It is merged after
> pass CI.
> 
> May I know where you see PR CI builds are broken?
> 
> Thank you
> Yao, Jiewen
> 
> > -Original Message-
> > From: Li, Yi1 
> > Sent: Wednesday, January 17, 2024 3:21 PM
> > To: devel@edk2.groups.io; Yao, Jiewen ; Gerd
> > Hoffmann 
> > Cc: dougfl...@microsoft.com; Douglas Flick [MSFT]
> > 
> > Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > Hi Jiewen,
> >
> > All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> > Maybe we didn't have enough time to wait feedback and should fix the
> > CI issue first.
> >
> > Regards,
> > Yi
> >
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Yao,
> > Jiewen
> > Sent: Tuesday, January 16, 2024 10:38 PM
> > To: Gerd Hoffmann ; devel@edk2.groups.io
> > Cc: dougfl...@microsoft.com; Douglas Flick [MSFT]
> > 
> > Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > Sure. Let's start from OVMF.
> >
> > We have leaf enough time for feedback, but I see no comment from other
> people.
> >
> >
> > > -Original Message-
> > > From: Gerd Hoffmann 
> > > Sent: Tuesday, January 16, 2024 10:35 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen 
> > > Cc: dougfl...@microsoft.com; Douglas Flick [MSFT]
> > > 
> > > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117
> > > &
> > > TCBZ4118
> > >
> > > On Tue, Jan 16, 2024 at 01:30:43PM +, Yao, Jiewen wrote:
> > > > Gerd
> > > > I have merged this patch set today.
> > > >
> > > > I am fine to remove TPM1.2 in OVMF because of the known security
> > limitation.
> > >
> > > I was thinking about the complete edk2 code base not only OVMF.
> > >
> > > But I can surely 

Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Li, Yi
Hi Jiewen,

Sounds strange, but new PRs in today all broken due to this issue, e.g.:
https://github.com/tianocore/edk2/pull/5210
https://github.com/tianocore/edk2/pull/5268


I checked build log, it matched the description from Gerd:
https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-7def1f19d478/_apis/build/builds/114097/logs/350
2024-01-17T04:09:52.5996237Z INFO - /usr/bin/ld: 
DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function 
`SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6010570Z INFO - (.text+0x0): multiple definition of 
`SanitizeEfiPartitionTableHeader'; DxeTpmMeasureBootLibSanitization.obj (symbol 
from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6020435Z INFO - /usr/bin/ld: 
DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function 
`SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6030987Z INFO - (.text+0x0): multiple definition of 
`SanitizePrimaryHeaderAllocationSize'; DxeTpmMeasureBootLibSanitization.obj 
(symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6040167Z INFO - /usr/bin/ld: 
DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function 
`SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6050625Z INFO - (.text+0x0): multiple definition of 
`SanitizePrimaryHeaderGptEventSize'; DxeTpmMeasureBootLibSanitization.obj 
(symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6061966Z INFO - /usr/bin/ld: 
DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function 
`SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6072661Z INFO - (.text+0x0): multiple definition of 
`SanitizePeImageEventSize'; DxeTpmMeasureBootLibSanitization.obj (symbol from 
plugin):(.text+0x0): first defined here
2024-01-17T04:10:12.9532147Z INFO - build.py...
2024-01-17T04:10:12.9593220Z INFO -  : error 7000: Failed to execute command
2024-01-17T04:10:23.2054653Z INFO - build.py...
2024-01-17T04:10:23.2055014Z INFO -  : error F002: Failed to build module
2024-01-17T04:10:23.2055379Z INFO - 
/__w/1/s/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf [X64, GCC5, 
DEBUG]

-Original Message-
From: Yao, Jiewen  
Sent: Wednesday, January 17, 2024 4:09 PM
To: Li, Yi1 ; devel@edk2.groups.io; Gerd Hoffmann 

Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

Please check https://github.com/tianocore/edk2/pull/5264. It is merged after 
pass CI.

May I know where you see PR CI builds are broken?

Thank you
Yao, Jiewen

> -Original Message-
> From: Li, Yi1 
> Sent: Wednesday, January 17, 2024 3:21 PM
> To: devel@edk2.groups.io; Yao, Jiewen ; Gerd 
> Hoffmann 
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> 
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & 
> TCBZ4118
> 
> Hi Jiewen,
> 
> All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> Maybe we didn't have enough time to wait feedback and should fix the 
> CI issue first.
> 
> Regards,
> Yi
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Yao, 
> Jiewen
> Sent: Tuesday, January 16, 2024 10:38 PM
> To: Gerd Hoffmann ; devel@edk2.groups.io
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> 
> Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & 
> TCBZ4118
> 
> Sure. Let's start from OVMF.
> 
> We have leaf enough time for feedback, but I see no comment from other people.
> 
> 
> > -Original Message-
> > From: Gerd Hoffmann 
> > Sent: Tuesday, January 16, 2024 10:35 PM
> > To: devel@edk2.groups.io; Yao, Jiewen 
> > Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> > 
> > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 
> > &
> > TCBZ4118
> >
> > On Tue, Jan 16, 2024 at 01:30:43PM +, Yao, Jiewen wrote:
> > > Gerd
> > > I have merged this patch set today.
> > >
> > > I am fine to remove TPM1.2 in OVMF because of the known security
> limitation.
> >
> > I was thinking about the complete edk2 code base not only OVMF.
> >
> > But I can surely start with OVMF.  Maybe it is the only platform 
> > affected because on physical hardware you usually know whenever TPM
> > 1.2 or TPM 2.0 is present so there is no need to include both.
> >
> > take care,
> >   Gerd
> 
> 
> 
> 
> 



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




[edk2-devel] [PATCH v2] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap

2024-01-17 Thread Zhiguang Liu
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614

About the IsModified, current function doesn't consider that hardware
also may change the pagetable. The issue is that in the first call of
internal function PageTableLibMapInLevel, the function assume page
table is not changed, and add ASSERT to check. But hardware may change
the page table, which cause the ASSERT happens.
Fix the issue by considering the hardware may also change page table,
and document the detail in function header.

Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Crystal Lee 
Signed-off-by: Zhiguang Liu 
---
 .../Library/CpuPageTableLib/CpuPageTableMap.c| 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c 
b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
index 36b2c4e6a3..a3076ff2f6 100644
--- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
+++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
@@ -274,7 +274,7 @@ IsAttributesAndMaskValidForNonPresentEntry (
 Page table entries that map the linear 
address range are reset to 0 before set to the new attribute
 when a new physical base address is set.
   @param[in]  Mask  The mask used for attribute. The 
corresponding field in Attribute is ignored if that in Mask is 0.
-  @param[out] IsModifiedTRUE means page table is modified. FALSE 
means page table is not modified.
+  @param[in, out] IsModifiedChange IsModified to True if page table is 
modified and input parameter Modify is TRUE.
 
   @retval RETURN_INVALID_PARAMETER  For non-present range, Mask->Bits.Present 
is 0 but some other attributes are provided.
   @retval RETURN_INVALID_PARAMETER  For non-present range, Mask->Bits.Present 
is 1, Attribute->Bits.Present is 1 but some other attributes are not provided.
@@ -567,7 +567,10 @@ PageTableLibMapInLevel (
 OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
 PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, 
);
 
-if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) {
+if (Modify && (OriginalCurrentPagingEntry.Uint64 != 
CurrentPagingEntry->Uint64)) {
+  //
+  // The page table entry can be changed by this function only when 
Modify is true.
+  //
   *IsModified = TRUE;
 }
   }
@@ -609,7 +612,10 @@ PageTableLibMapInLevel (
   // Check if ParentPagingEntry entry is modified here is enough. Except the 
changes happen in leaf PagingEntry during
   // the while loop, if there is any other change happens in page table, the 
ParentPagingEntry must has been modified.
   //
-  if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) {
+  if (Modify && (OriginalParentPagingEntry.Uint64 != 
ParentPagingEntry->Uint64)) {
+//
+// The page table entry can be changed by this function only when Modify 
is true.
+//
 *IsModified = TRUE;
   }
 
@@ -633,7 +639,9 @@ PageTableLibMapInLevel (
  Page table entries that map the linear 
address range are reset to 0 before set to the new attribute
  when a new physical base address is set.
   @param[in]  Mask   The mask used for attribute. The 
corresponding field in Attribute is ignored if that in Mask is 0.
-  @param[out] IsModified TRUE means page table is modified. FALSE 
means page table is not modified.
+  @param[out] IsModified TRUE means page table is modified by software 
or hardware. FALSE means page table is not modified by software.
+ If the output IsModified is FALSE, there is 
possibility that the page table is changed by hardware. It is ok
+ because page table can be changed by hardware 
anytime, and caller don't need to Flush TLB.
 
   @retval RETURN_UNSUPPORTEDPagingMode is not supported.
   @retval RETURN_INVALID_PARAMETER  PageTable, BufferSize, Attribute or Mask 
is NULL.
-- 
2.31.1.windows.1



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




Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118

2024-01-17 Thread Yao, Jiewen
Please check https://github.com/tianocore/edk2/pull/5264. It is merged after 
pass CI.

May I know where you see PR CI builds are broken?

Thank you
Yao, Jiewen

> -Original Message-
> From: Li, Yi1 
> Sent: Wednesday, January 17, 2024 3:21 PM
> To: devel@edk2.groups.io; Yao, Jiewen ; Gerd Hoffmann
> 
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
> 
> Hi Jiewen,
> 
> All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> Maybe we didn't have enough time to wait feedback and should fix the CI issue
> first.
> 
> Regards,
> Yi
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Yao, Jiewen
> Sent: Tuesday, January 16, 2024 10:38 PM
> To: Gerd Hoffmann ; devel@edk2.groups.io
> Cc: dougfl...@microsoft.com; Douglas Flick [MSFT] 
> Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
> 
> Sure. Let's start from OVMF.
> 
> We have leaf enough time for feedback, but I see no comment from other people.
> 
> 
> > -Original Message-
> > From: Gerd Hoffmann 
> > Sent: Tuesday, January 16, 2024 10:35 PM
> > To: devel@edk2.groups.io; Yao, Jiewen 
> > Cc: dougfl...@microsoft.com; Douglas Flick [MSFT]
> > 
> > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > On Tue, Jan 16, 2024 at 01:30:43PM +, Yao, Jiewen wrote:
> > > Gerd
> > > I have merged this patch set today.
> > >
> > > I am fine to remove TPM1.2 in OVMF because of the known security
> limitation.
> >
> > I was thinking about the complete edk2 code base not only OVMF.
> >
> > But I can surely start with OVMF.  Maybe it is the only platform
> > affected because on physical hardware you usually know whenever TPM
> > 1.2 or TPM 2.0 is present so there is no need to include both.
> >
> > take care,
> >   Gerd
> 
> 
> 
> 
> 



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