date:20231113

Re: [edk2-devel] [PATCH 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow

2023-11-13 Thread Chang, Abner via groups.io

[AMD Official Use Only - General]

Hi Igor,
Only one comment below,

> -Original Message-
> From: Igor Kulchytskyy 
> Sent: Tuesday, November 14, 2023 1:24 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Nickle Wang
> 
> Subject: [PATCH 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish
> Discover flow
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Filter out the network interfaces which are not supported by
> Redfish Host Interface.
>
> Cc: Abner Chang 
> Cc: Nickle Wang 
> Signed-off-by: Igor Kulchytskyy 
> ---
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 169 +-
>  1 file changed, 120 insertions(+), 49 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 0f622e05a9..3d514d7e4c 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -13,6 +13,12 @@
>
>  #include "RedfishDiscoverInternal.h"
>
> +#define MAC_COMPARE(ThisNetworkInterface, TargetNetworkInterface)
> (CompareMem ((VOID *)>MacAddress,
> >MacAddress, ThisNetworkInterface-
> >HwAddressSize))
> +#define VALID_TCP6(TargetNetworkInterface, ThisNetworkInterface)
> (TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp6))
> +#define VALID_TCP4(TargetNetworkInterface, ThisNetworkInterface)
> (!TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp4))
> +#define IS_TCP4_MATCH(IpType)
> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) && (IpType !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4))
> +#define IS_TCP6_MATCH(IpType)
> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) && (IpType !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6))

Let's move these macro to RedfishDicoverInternal.h

Abner

> +
>  LIST_ENTRY   mRedfishDiscoverList;
>  LIST_ENTRY   mRedfishInstanceList;
>  EFI_SMBIOS_PROTOCOL  *mSmbios = NULL;
> @@ -322,9 +328,16 @@ GetTargetNetworkInterfaceInternal (
>  {
>EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>
> +  if (IsListEmpty ()) {
> +return NULL;
> +  }
> +
>ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> ();
>while (TRUE) {
> -if (CompareMem ((VOID *)>MacAddress,
> >MacAddress, ThisNetworkInterface-
> >HwAddressSize) == 0) {
> +if ((MAC_COMPARE (ThisNetworkInterface, TargetNetworkInterface) == 0)
> &&
> +(VALID_TCP6 (TargetNetworkInterface, ThisNetworkInterface) ||
> + VALID_TCP4 (TargetNetworkInterface, ThisNetworkInterface)))
> +{
>return ThisNetworkInterface;
>  }
>
> @@ -354,6 +367,10 @@ GetTargetNetworkInterfaceInternalByController (
>  {
>EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>
> +  if (IsListEmpty ()) {
> +return NULL;
> +  }
> +
>ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> ();
>while (TRUE) {
>  if (ThisNetworkInterface->OpenDriverControllerHandle ==
> ControllerHandle) {
> @@ -476,6 +493,42 @@ CheckIsIpVersion6 (
>return FALSE;
>  }
>
> +/**
> +  This function returns the  IP type supported by the Host Interface.
> +
> +  @retval 00h is Unknown
> +  01h is Ipv4
> +  02h is Ipv6
> +
> +**/
> +UINT8
> +GetHiIpProtocolType (
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +  REDFISH_OVER_IP_PROTOCOL_DATA  *Data;
> +  REDFISH_INTERFACE_DATA *DeviceDescriptor;
> +
> +  Data = NULL;
> +  DeviceDescriptor = NULL;
> +  if (mSmbios == NULL) {
> +Status = gBS->LocateProtocol (, NULL, (VOID
> **));
> +if (EFI_ERROR (Status)) {
> +  return
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> +}
> +  }
> +
> +  Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> , ); // Search for SMBIOS type 42h
> +  if (!EFI_ERROR (Status) && (Data != NULL) &&
> +  (Data->HostIpAssignmentType == RedfishHostIpAssignmentStatic))
> +  {
> +return Data->HostIpAddressFormat;
> +  }
> +
> +  return
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> +}
> +
>  /**
>This function discover Redfish service through SMBIOS host interface.
>
> @@ -512,6 +565,18 @@ DiscoverRedfishHostInterface (
>
>Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> , ); // Search for SMBIOS type 42h
>if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> +if ((Instance->NetworkInterface->NetworkProtocolType ==
> ProtocolTypeTcp4) &&
> +(Data->HostIpAddressFormat !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) // IPv4 case
> +{
> +  DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv4, but Host
> Interface requires Ipv6\n", __func__));
> +  return

Re: [edk2-devel] [PATCH 1/2] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx

2023-11-13 Thread Chang, Abner via groups.io

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Igor Kulchytskyy 
> Sent: Tuesday, November 14, 2023 1:24 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Nickle Wang
> 
> Subject: [PATCH 1/2] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4
> installed after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Supported function of the driver changed to wait for all network
> interface to be installed.
>
> Cc: Abner Chang 
> Cc: Nickle Wang 
> Signed-off-by: Igor Kulchytskyy 
> ---
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 31 ++-
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 23da3b968f..0f622e05a9 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -1547,25 +1547,26 @@ TestForRequiredProtocols (
>  ControllerHandle,
>  EFI_OPEN_PROTOCOL_TEST_PROTOCOL
>  );
> +if (EFI_ERROR (Status)) {
> +  return EFI_UNSUPPORTED;
> +}
> +
> +Status = gBS->OpenProtocol (
> +ControllerHandle,
> +gRequiredProtocol[Index].DiscoveredProtocolGuid,
> +(VOID **),
> +This->DriverBindingHandle,
> +ControllerHandle,
> +EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +);
>  if (!EFI_ERROR (Status)) {
> -  Status = gBS->OpenProtocol (
> -  ControllerHandle,
> -  gRequiredProtocol[Index].DiscoveredProtocolGuid,
> -  (VOID **),
> -  This->DriverBindingHandle,
> -  ControllerHandle,
> -  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -  );
> -  if (EFI_ERROR (Status)) {
> -if (Index == ListCount - 1) {
> -  DEBUG ((DEBUG_INFO, "%a: all required protocols are found on this
> controller handle: %p.\n", __func__, ControllerHandle));
> -  return EFI_SUCCESS;
> -}
> -  }
> +  // Already installed
> +  return EFI_UNSUPPORTED;
>  }
>}
>
> -  return EFI_UNSUPPORTED;
> +  DEBUG ((DEBUG_MANAGEABILITY, "%a: all required protocols are found on
> this controller handle: %p.\n", __func__, ControllerHandle));
> +  return EFI_SUCCESS;
>  }
>
>  /**
> --
> 2.37.1.windows.1
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended
> to be read only by the individual or entity to whom it is addressed or by 
> their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by telephone
> at 770-246-8600, and then delete or destroy all copies of the transmission.


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

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

2023-11-13 Thread Group Notification

*Reminder: TianoCore Bug Triage - APAC / NAMO*

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

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

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

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

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

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

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

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

*Or call in (audio only)*

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

Phone Conference ID: 774 638 21#

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

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


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

Re: [edk2-devel] [PATCH v2 23/30] OvmfPkg/LoongArchVirt: Add PeiServiceTablePointerLib

2023-11-13 Thread Chao Li


Hi Laszlo,

I see, I will look into this PI SPEC and try to implement it in MdePkg 
first and then to update the SPEC.



Thanks,
Chao
On 2023/11/13 19:02, Laszlo Ersek wrote:

On 11/10/23 07:44, Chao Li wrote:

Hi Laszlo,

This is a good question, same as the previous email, some ARCH don't
require running code on memory during PEI phase or other reason can not
used the MdePkg version, so this pointer will be saved by some register,
I saw the ArmPkg version also uses a register to save this pointer.

If I move ArmPkg version PeiServiceTablePointer to MdePkg and the method
of getting and saving the pointer use a new library which related
architecture, other word, the API of PeiServiceTablePointer has not
changed, adding a new library that PeiServiceTablePointer depends on,
the real saving and reading method completed in the new library. Do you
think this way is better?

Right. I think you need a loongarch-specific library instance of
PeiServicesTablePointerLib under MdePkg. Presumably, if / when you
enable edk2 on *physical* loongarch platforms, you're going to need the
same library instance. And therefore it should not be under OvmfPkg, but
MdePkg.

In fact, now that I'm reading the comments in
"MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c",
it seems that the architecture bindings in the Platform Init
specification actually document how the PEI services table pointer is
supposed to be stored before system memory becomes available (IIUC).

The latest version of the PI spec that I have is 1.8.

In Volume I, there is a section titled "I-9.4 PEI Services Table
Retrieval". It considers the following architectures: X86, x64, Itanium,
ARM, AArch64, RISC-V.

I think that in the longer term, you should file a PIWG Mantis ticket
for extending this section, with an Engineering Change Request where you
describe how this mechanism should work on loongarch.

And then the implementation certainly belongs to MdePkg.

(Note that I'm *not* saying you should first update the specification
and then implement it in edk2 -- that's not my point. My point is that
*eventually*, this mechanism will become a part of the public loongarch
bindings, and therefore you can already start introducing it under
MdePkg, in my opinion anyway.)

Note that "prior art" is inconsistent here, regarding edk2 locations,
because, as you say, the arm/aarch64 implementation lives under ArmPkg
-- but it's quite unlikely IMO that a LoongArchPkg top-level directory
would be introduced. In the longer term, we might want to consolidate
all PeiServicesTablePointerLib instances under MdePkg.


Then your question is, IIUC, whether to reuse any portion of the ArmPkg
implementation. I don't think that's necessary; the library is so small,
there is effectively *nothing generic* in it. Put differently, all of it
is architecture specific. Thus I think you can just add a totally
self-contained loongarch instance.

Laszlo








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

[edk2-devel] [PATCH] UefiCpuPkg: Use NonSmm BSP as BSP if BSP election is not enabled.

2023-11-13 Thread Zhiguang Liu

Currently, if BSP election is not enabled, will use Core0 as SMM BSP,
however, Core0 does not always have the highest performance core.
So, we can used NonSmm BSP as default BSP.

Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Signed-off-by: Zhiguang Liu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 10 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 48 +++---
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 25d058c5b9..a4f83bb122 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -29,6 +29,8 @@ MM_COMPLETIONmSmmStartupThisApToken;
 //
 UINT32  *mPackageFirstThreadIndex = NULL;
 
+extern EFI_MP_SERVICES_PROTOCOL  *mMpServices;
+
 /**
   Performs an atomic compare exchange operation to get semaphore.
   The compare exchange operation must be performed using
@@ -1953,6 +1955,14 @@ InitializeMpSyncData (
   // Enable BSP election by setting BspIndex to -1
   //
   mSmmMpSyncData->BspIndex = (UINT32)-1;
+} else {
+  //
+  // Use NonSMM BSP as SMM BSP
+  //
+  ASSERT (mMpServices != NULL);
+  if (mMpServices != NULL) {
+mMpServices->WhoAmI (mMpServices, (UINTN *)>BspIndex);
+  }
 }
 
 mSmmMpSyncData->EffectiveSyncMode = mCpuSmmSyncMode;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 1d022a7051..18c77c59e6 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -128,7 +128,8 @@ SPIN_LOCK  *mConfigSmmCodeAccessCheckLock = NULL;
 EFI_SMRAM_DESCRIPTOR  *mSmmCpuSmramRanges;
 UINTN mSmmCpuSmramRangeCount;
 
-UINT8  mPhysicalAddressBits;
+UINT8 mPhysicalAddressBits;
+EFI_MP_SERVICES_PROTOCOL  *mMpServices;
 
 //
 // Control register contents saved for SMM S3 resume state initialization.
@@ -603,26 +604,25 @@ PiCpuSmmEntry (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUSStatus;
-  EFI_MP_SERVICES_PROTOCOL  *MpServices;
-  UINTN NumberOfEnabledProcessors;
-  UINTN Index;
-  VOID  *Buffer;
-  UINTN BufferPages;
-  UINTN TileCodeSize;
-  UINTN TileDataSize;
-  UINTN TileSize;
-  UINT8 *Stacks;
-  VOID  *Registration;
-  UINT32RegEax;
-  UINT32RegEbx;
-  UINT32RegEcx;
-  UINT32RegEdx;
-  UINTN FamilyId;
-  UINTN ModelId;
-  UINT32Cr3;
-  EFI_HOB_GUID_TYPE *GuidHob;
-  SMM_BASE_HOB_DATA *SmmBaseHobData;
+  EFI_STATUS Status;
+  UINTN  NumberOfEnabledProcessors;
+  UINTN  Index;
+  VOID   *Buffer;
+  UINTN  BufferPages;
+  UINTN  TileCodeSize;
+  UINTN  TileDataSize;
+  UINTN  TileSize;
+  UINT8  *Stacks;
+  VOID   *Registration;
+  UINT32 RegEax;
+  UINT32 RegEbx;
+  UINT32 RegEcx;
+  UINT32 RegEdx;
+  UINTN  FamilyId;
+  UINTN  ModelId;
+  UINT32 Cr3;
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  SMM_BASE_HOB_DATA  *SmmBaseHobData;
 
   GuidHob= NULL;
   SmmBaseHobData = NULL;
@@ -656,13 +656,13 @@ PiCpuSmmEntry (
   //
   // Get MP Services Protocol
   //
-  Status = SystemTable->BootServices->LocateProtocol 
(, NULL, (VOID **));
+  Status = SystemTable->BootServices->LocateProtocol 
(, NULL, (VOID **));
   ASSERT_EFI_ERROR (Status);
 
   //
   // Use MP Services Protocol to retrieve the number of processors and number 
of enabled processors
   //
-  Status = MpServices->GetNumberOfProcessors (MpServices, , 
);
+  Status = mMpServices->GetNumberOfProcessors (mMpServices, , 
);
   ASSERT_EFI_ERROR (Status);
   ASSERT (mNumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
 
@@ -945,7 +945,7 @@ PiCpuSmmEntry (
 gSmmCpuPrivate->Operation[Index]= SmmCpuNone;
 
 if (Index < mNumberOfCpus) {
-  Status = MpServices->GetProcessorInfo (MpServices, Index | 
CPU_V2_EXTENDED_TOPOLOGY, >ProcessorInfo[Index]);
+  Status = mMpServices->GetProcessorInfo (mMpServices, Index | 
CPU_V2_EXTENDED_TOPOLOGY, >ProcessorInfo[Index]);
   ASSERT_EFI_ERROR (Status);
   mCpuHotPlugData.ApicId[Index] = 
gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId;
 
-- 
2.31.1.windows.1



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

Re: [edk2-devel] [PATCH v2 11/30] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg

2023-11-13 Thread Chao Li


Hi Andrei,

I agree with you, I will move LoongArch-specific stuff from UefiCpuPkg 
to MdePkg/Include/ in V3.


And you are also saied record the idea of moving ArmMmuLib to CpuMmuLib, 
I would to say, this header is referenced from ArmPkg, and I plan to 
move the ArmMmuLib from ArmPkg to UefiCpuPkg after merging this patch 
set, and other BZ number will be used. The premise is that the 
maintainers of ArmPkg agrees move operation.



Thanks,
Chao
On 2023/11/9 09:17, Andrei Warkentin wrote:

Hi Chao,

I see that you're adding a header, CpuMmuLib.h, but this header is not generic - it 
includes a bunch of Loongson arch-specific "stuff".

I agree in principle that there's room for a generic CpuMmuLib, and I don't 
think you ought to be on the hook to refactor ArmMmuLib, but I suggest moving
the architecture-specific PTE #defines into MdePkg/Include similar to 
./MdePkg/Include/Register/RiscV64. (it would, though, be nice to at least 
record the idea of moving ArmMmuLib to CpuMmuLib raise a BZ for that or 
something!)

This will avoid the #ifdef MDE_CPU_LOONGARCH64 inside a generic Library header.

A


-Original Message-
From: Chao Li
Sent: Sunday, November 5, 2023 9:28 PM
To:devel@edk2.groups.io
Cc: Dong, Eric; Ni, Ray; Kumar,
Rahul R; Gerd Hoffmann;
Leif Lindholm; Ard Biesheuvel
; Sami Mujawar;
Sunil V L; Warkentin, Andrei

Subject: [PATCH v2 11/30] UefiCpuPkg: Add CpuMmuLib.h to UefiCpuPkg

Add a new header file CpuMmuLib.h, whitch is referenced from
ArmPkg/Include/Library/ArmMmuLib.h. Currently, only support for
LoongArch64 is added, and more architectures can be accommodated in the
future.

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

Cc: Eric Dong
Cc: Ray Ni
Cc: Rahul Kumar
Cc: Gerd Hoffmann
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
Cc: Sunil V L
Cc: Andrei Warkentin
Signed-off-by: Chao Li
---
  UefiCpuPkg/Include/Library/CpuMmuLib.h | 194
+
  UefiCpuPkg/UefiCpuPkg.dec  |   4 +
  2 files changed, 198 insertions(+)
  create mode 100644 UefiCpuPkg/Include/Library/CpuMmuLib.h

diff --git a/UefiCpuPkg/Include/Library/CpuMmuLib.h
b/UefiCpuPkg/Include/Library/CpuMmuLib.h
new file mode 100644
index 00..8f524d31d4
--- /dev/null
+++ b/UefiCpuPkg/Include/Library/CpuMmuLib.h
@@ -0,0 +1,194 @@
+/** @file
+
+  Copyright (c) 2023 Loongson Technology Corporation Limited. All
+ rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CPU_MMU_LIB_H_
+#define CPU_MMU_LIB_H_
+
+#include 
+
+#ifdef MDE_CPU_LOONGARCH64
+
+/* Page table property definitions  */
+#define PAGE_VALID_SHIFT   0
+#define PAGE_DIRTY_SHIFT   1
+#define PAGE_PLV_SHIFT 2  // 2~3, two bits
+#define CACHE_SHIFT4  // 4~5, two bits
+#define PAGE_GLOBAL_SHIFT  6
+#define PAGE_HUGE_SHIFT6  // HUGE is a PMD bit
+
+#define PAGE_HGLOBAL_SHIFT  12 // HGlobal is a PMD bit
+#define PAGE_PFN_SHIFT  12
+#define PAGE_PFN_END_SHIFT  48
+#define PAGE_NO_READ_SHIFT  61
+#define PAGE_NO_EXEC_SHIFT  62
+#define PAGE_RPLV_SHIFT 63
+
+/* Used by TLB hardware (placed in EntryLo*) */
+#define PAGE_VALID((UINTN)(1) << PAGE_VALID_SHIFT)
+#define PAGE_DIRTY((UINTN)(1) << PAGE_DIRTY_SHIFT)
+#define PAGE_PLV  ((UINTN)(3) << PAGE_PLV_SHIFT)
+#define PAGE_GLOBAL   ((UINTN)(1) << PAGE_GLOBAL_SHIFT)
+#define PAGE_HUGE ((UINTN)(1) << PAGE_HUGE_SHIFT)
+#define PAGE_HGLOBAL  ((UINTN)(1) << PAGE_HGLOBAL_SHIFT) #define
+PAGE_NO_READ  ((UINTN)(1) << PAGE_NO_READ_SHIFT) #define
PAGE_NO_EXEC
+((UINTN)(1) << PAGE_NO_EXEC_SHIFT)
+#define PAGE_RPLV ((UINTN)(1) << PAGE_RPLV_SHIFT)
+#define CACHE_MASK((UINTN)(3) << CACHE_SHIFT)
+#define PFN_SHIFT (EFI_PAGE_SHIFT - 12 + PAGE_PFN_SHIFT)
+
+#define PLV_KERNEL  0
+#define PLV_USER3
+
+#define PAGE_USER(PLV_USER << PAGE_PLV_SHIFT)
+#define PAGE_KERNEL  (PLV_KERN << PAGE_PLV_SHIFT)
+
+#define CACHE_SUC  (0 << CACHE_SHIFT) // Strong-ordered UnCached
+#define CACHE_CC   (1 << CACHE_SHIFT) // Coherent Cached
+#define CACHE_WUC  (2 << CACHE_SHIFT) // Weak-ordered UnCached
+
+#define EFI_MEMORY_CACHETYPE_MASK  (EFI_MEMORY_UC  | \
+EFI_MEMORY_WC  | \
+EFI_MEMORY_WT  | \
+EFI_MEMORY_WB  | \
+EFI_MEMORY_UCE   \
+)
+#endif
+
+typedef struct {
+  EFI_PHYSICAL_ADDRESSPhysicalBase;
+  EFI_VIRTUAL_ADDRESS VirtualBase;
+  UINTN   Length;
+  UINTN   Attributes;
+} MEMORY_REGION_DESCRIPTOR;
+
+/**
+  Converts EFI Attributes to corresponding architecture Attributes.
+
+  @param[in]  EfiAttributes Efi Attributes.
+
+  @retval  Corresponding architecture attributes.
+**/
+UINTN
+EfiAttributeConverse (
+  IN UINTN  EfiAttributes
+  );
+
+/**
+  Finds the length and memory properties of the memory region
corresponding to the specified base address.
+
+  @param[in]  BaseAddressTo

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Michael Kubacki


On 11/13/2023 3:37 PM, Rebecca Cran wrote:

On 11/13/2023 1:08 PM, Michael Kubacki wrote:
Yes. I just did it. It is relatively minor and impacts expected code 
areas.


https://github.com/tianocore/edk2/pull/5043/files



Could you update .git-blame-ignore-revs please?

https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame


Yes. This was discussed this in the Tianocore Tools & CI meeting. I will 
submit the series to the mailing list as this only impacts 5 files in 2 
packages.


I will follow up with a commmit to propose adding that file with all 
formatting changes including the original Uncrustify changes.



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

Re: [edk2-devel] [PATCH v2 16/30] ArmVirtPkg: Move PCD of FDT base address and FDT padding to OvmfPkg

2023-11-13 Thread Chao Li


Hi Sami,

Ok, I will fix them in V3 and follow your suggestions, thanks for the 
review.



Thanks,
Chao
On 2023/11/13 19:46, Sami Mujawar wrote:

Hi Chao,

Thank you for this patch.

I have a few suggestions marked inline as [SAMI].

Otherwise this patch looks good to me.

With those fixed,

Reviewed-by: Sami Mujawar 

Regards,

Sami Mujawar

On 06/11/2023 03:29 am, Chao Li wrote:

Moved PcdDeviceTreeInitialBaseAddress and PcdDeviceTreeAllocationPadding
to OvmfPkg for easier use by other architectures.

Build-tested only (with "ArmVirtQemu.dsc").

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

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Signed-off-by: Chao Li 
---
  ArmVirtPkg/ArmVirtCloudHv.dsc |  2 +-
  ArmVirtPkg/ArmVirtKvmTool.dsc |  2 +-
  ArmVirtPkg/ArmVirtPkg.dec | 14 --
  ArmVirtPkg/ArmVirtQemu.dsc    |  2 +-
  ArmVirtPkg/ArmVirtQemuKernel.dsc  |  2 +-
  ArmVirtPkg/ArmVirtXen.dsc |  2 +-
  .../ArmVirtPsciResetSystemPeiLib.inf  |  3 ++-
  .../CloudHvVirtMemInfoPeiLib.inf  |  3 ++-
  .../DebugLibFdtPL011UartFlash.inf |  3 ++-
  .../EarlyFdt16550SerialPortHookLib.inf    |  3 ++-
  .../EarlyFdtPL011SerialPortLib.inf    |  3 ++-
  .../KvmtoolPlatformPeiLib.inf |  5 +++--
  .../Library/PlatformPeiLib/PlatformPeiLib.inf | 10 +-
  .../QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf  |  3 ++-
  .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf  |  3 ++-
  OvmfPkg/OvmfPkg.dec   | 15 +++
  16 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc 
b/ArmVirtPkg/ArmVirtCloudHv.dsc

index 2cb89ce10c..76c0d28544 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -129,7 +129,7 @@
    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x4000
      # initial location of the device tree blob passed by Cloud 
Hypervisor -- base of DRAM

- gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x4000
+ gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x4000
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 
0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 
0xf4, 0x66, 0x23, 0x31 }
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc 
b/ArmVirtPkg/ArmVirtKvmTool.dsc

index f50d53bf15..cac4fe06d3 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -179,7 +179,7 @@
    # We are booting from RAM using the Linux kernel boot protocol,
    # x0 will point to the DTB image in memory.
    #
-  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0
+ gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0
      gArmTokenSpaceGuid.PcdFdBaseAddress|0x0
    gArmTokenSpaceGuid.PcdFvBaseAddress|0x0
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 0f2d787327..2451644844 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -42,20 +42,6 @@
gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|FALSE|BOOLEAN|0x0004
    [PcdsFixedAtBuild, PcdsPatchableInModule]
-  #
-  # This is the physical address where the device tree is expected 
to be stored
-  # upon first entry into UEFI. This needs to be a FixedAtBuild PCD, 
so that we
-  # can do a first pass over the device tree in the SEC phase to 
discover the

-  # UART base address.
-  #
- 
gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0|UINT64|0x0001

-
-  #
-  # Padding in bytes to add to the device tree allocation, so that 
the DTB can

-  # be modified in place (default: 256 bytes)
-  #
- 
gArmVirtTokenSpaceGuid.PcdDeviceTreeAllocationPadding|256|UINT32|0x0002

-
    #
    # Binary representation of the GUID that determines the terminal 
type. The

    # size must be exactly 16 bytes. The default value corresponds to
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 30e3cfc8b9..cf306cac08 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -201,7 +201,7 @@
    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x4000
      # initial location of the device tree blob passed by QEMU -- 
base of DRAM

- gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x4000
+ gUefiOvmfPkgTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x4000
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 
0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 
0xf4, 0x66, 0x23, 0x31 }
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc 
b/ArmVirtPkg/ArmVirtQemuKernel.dsc

index b50f8e84a3..c0d079e28d 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++

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

2023-11-13 Thread Group Notification

*Tools, CI, Code base construction meeting series*

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

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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

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

2023-11-13 Thread Group Notification

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

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

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

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

*Description:*

TianoCore community,

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

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

Anyone is welcome to join.

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

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


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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Pedro Falcato

On Mon, Nov 13, 2023 at 8:37 PM Rebecca Cran  wrote:
>
> On 11/13/2023 1:08 PM, Michael Kubacki wrote:
> > Yes. I just did it. It is relatively minor and impacts expected code
> > areas.
> >
> > https://github.com/tianocore/edk2/pull/5043/files
>
>
> Could you update .git-blame-ignore-revs please?

You can't do that until the merge is done, since we use
rebase-and-merge for tianocore (and rebases do not keep stable commit
hashes).
But I would plead that this should not get merged in general :/

-- 
Pedro


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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Michael D Kinney

Hi Michael,

Have you tried to upstream the edk2 specific elements to the
uncrustify project?  That would be a path to remove the fork
and eventually all the distros would catch up.

Mike

> -Original Message-
> From: Michael Kubacki 
> Sent: Monday, November 13, 2023 12:22 PM
> To: Pedro Falcato ; devel@edk2.groups.io;
> ler...@redhat.com
> Cc: Kinney, Michael D ; Andrew Fish
> ; Marcin Juszkiewicz ;
> Leif Lindholm (Quic) 
> Subject: Re: [edk2-devel] edk2 uncrustify update (73.0.8)?
> 
> On 11/13/2023 2:07 PM, Pedro Falcato wrote:
> > On Mon, Nov 13, 2023 at 11:58 AM Laszlo Ersek 
> wrote:
> >>
> >> Hi Michael,
> >>
> >> recently I encountered an uncrustify failure on github.
> >>
> >> The reason was that my local uncrustify was *more recent* (73.0.8)
> than
> >> the one we use in edk2 CI (which is 73.0.3, per the edk2 file
> >> ".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").
> >
> > Wait, you can use upstream uncrustify? I'm just using whatever
> > uncrustify version I took from the project-mu fork...
> >
> The fork version is needed for edk2 specific conventions. More details
> are here -
> https://dev.azure.com/projectmu/uncrustify?anchor=edk-ii-poc-details
> 
> >>
> >> Updating the version number in the YAML file (i.e., advancing edk2
> to
> >> version 73.0.8) seems easy enough, but:
> >>
> >> - Do you think 73.0.8 is mature enough for adoption in edk2?
> >>
> >>This upstream uncrustify release was tagged in April (and I
> can't see
> >>any more recent commits), so I assume it should be stable.
> >>
> >> - Would the version update require a whole-tree re-
> uncrustification?
> >
> > Please, no. I didn't mind doing an initial reformatting at first,
> but
> > doing this continuously is both 1) problem-prone 2) just amazing
> > amounts of churn.
> > Let's say I have version N, you have version N+1 - we may never get
> > any final, formatted output as your version formats it differently
> > from mine.
> >
> > I don't know how the CI is doing its thing atm (I haven't merged
> > anything myself to edk2), but the uncrustify check should be relaxed
> > to just a warning. There's nothing wrong with what my uncrustify
> > version is formatting to, there's nothing wrong with yours either,
> and
> > CI isn't necessarily wrong either.
> >
> > And, to be fair, I already find uncrustify a large pain in the butt
> to
> > use (requiring a custom fork really does not help), but I find the
> > benefits worth it *locally*, as my coding style is also quite
> > different from the NT-esque style.
> >
> It should be simple to update and ensure everyone is using the same
> version. This requires stuart commands to be used
> (https://github.com/tianocore/edk2-pytool-extensions). I know there's
> aversion to stuart but that's how these extensions plug into the edk2
> build process right now.
> 
> If you use it, as an end user, you just run "stuart_update -c
> .pytool/CISettings.py" and it will get the Uncrustify binary for your
> host OS with the version used by the project.
> 
> ---
> 
> The version pulled and the source feed used by stuart are defined in
> edk2 here:
> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/Uncrustif
> yCheck/uncrustify_ext_dep.yaml
> 
> That file and command are used locally, in CI, and the file is checked
> into edk2. At any given point in time, a user at a given point in edk2
> history should be using the same version and configuration.
> 
> More details, for those interested, are here
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-
> Formatting.
> That tries to cover some niche use cases so it may seem more
> overwhelming than it actually is to just get and use the executable.


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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Michael Kubacki


On 11/13/2023 2:14 PM, Rebecca Cran via groups.io wrote:

On 11/13/2023 5:29 AM, Marcin Juszkiewicz via groups.io wrote:

Still a fan of adding edk2-uncrustify to BaseTools. If we are expected 
to use it then let it get installed at same moment as "build" command is.


The issue with doing this is there's a push to remove all C/C++ code 
from BaseTools (including porting existing code to Python), and adding 
edk2-uncrustify would work against that.


That was a reason I did not add the code to BaseTools. I personally 
think it would bloat the edk2 repo and complicate its build process for 
something that rarely changes. Given binary dependencies are already 
used and managed for other incoming binaries such as iasl and nasm 
(https://github.com/tianocore/edk2/tree/master/BaseTools/Bin) and the 
effort to eliminate C/C++ code from BaseTools, I think it makes sense to 
keep it in a dedicated repository.


If there is another reason for the move, such as a preference to move 
the repo under the Tianocore organization (for whatever reason) or 
something like that, please let me know.



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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Rebecca Cran


On 11/13/2023 1:08 PM, Michael Kubacki wrote:
Yes. I just did it. It is relatively minor and impacts expected code 
areas.


https://github.com/tianocore/edk2/pull/5043/files



Could you update .git-blame-ignore-revs please?

https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame


--

Rebecca Cran



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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Michael Kubacki

On 11/13/2023 2:07 PM, Pedro Falcato wrote:

On Mon, Nov 13, 2023 at 11:58 AM Laszlo Ersek wrote:

Hi Michael,

recently I encountered an uncrustify failure on github.

The reason was that my local uncrustify was *more recent* (73.0.8) than
the one we use in edk2 CI (which is 73.0.3, per the edk2 file
".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").

Wait, you can use upstream uncrustify? I'm just using whatever
uncrustify version I took from the project-mu fork...

The fork version is needed for edk2 specific conventions. More details
are here -
https://dev.azure.com/projectmu/uncrustify?anchor=edk-ii-poc-details

Updating the version number in the YAML file (i.e., advancing edk2 to
version 73.0.8) seems easy enough, but:

- Do you think 73.0.8 is mature enough for adoption in edk2?

This upstream uncrustify release was tagged in April (and I can't see
any more recent commits), so I assume it should be stable.

- Would the version update require a whole-tree re-uncrustification?

Please, no. I didn't mind doing an initial reformatting at first, but
doing this continuously is both 1) problem-prone 2) just amazing
amounts of churn.
Let's say I have version N, you have version N+1 - we may never get
any final, formatted output as your version formats it differently
from mine.

I don't know how the CI is doing its thing atm (I haven't merged
anything myself to edk2), but the uncrustify check should be relaxed
to just a warning. There's nothing wrong with what my uncrustify
version is formatting to, there's nothing wrong with yours either, and
CI isn't necessarily wrong either.

And, to be fair, I already find uncrustify a large pain in the butt to
use (requiring a custom fork really does not help), but I find the
benefits worth it *locally*, as my coding style is also quite
different from the NT-esque style.

It should be simple to update and ensure everyone is using the same
version. This requires stuart commands to be used
(https://github.com/tianocore/edk2-pytool-extensions). I know there's
aversion to stuart but that's how these extensions plug into the edk2
build process right now.

If you use it, as an end user, you just run "stuart_update -c
.pytool/CISettings.py" and it will get the Uncrustify binary for your
host OS with the version used by the project.

---

The version pulled and the source feed used by stuart are defined in
edk2 here:
https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml

That file and command are used locally, in CI, and the file is checked
into edk2. At any given point in time, a user at a given point in edk2
history should be using the same version and configuration.

More details, for those interested, are here
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting.
That tries to cover some niche use cases so it may seem more
overwhelming than it actually is to just get and use the executable.

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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Michael Kubacki


On 11/13/2023 6:58 AM, Laszlo Ersek wrote:

Hi Michael,

recently I encountered an uncrustify failure on github.

The reason was that my local uncrustify was *more recent* (73.0.8) than
the one we use in edk2 CI (which is 73.0.3, per the edk2 file
".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").

Updating the version number in the YAML file (i.e., advancing edk2 to
version 73.0.8) seems easy enough, but:

- Do you think 73.0.8 is mature enough for adoption in edk2?

   This upstream uncrustify release was tagged in April (and I can't see
   any more recent commits), so I assume it should be stable.

Yes, it is stable. We've been using each new Uncrustify release against 
edk2 code in Project Mu during that time. I updated our code to include 
that change in September 2022 - 
https://github.com/microsoft/mu_basecore/commit/6932526bee9a2f5f3af7588923beae5e5d8fd128.


The changes since then have been additional build support for Linux and 
Windows Arm and macOS.


I originally did not bring this to edk2 right away to verify stability 
over time and reduce thrash if any other changes came in to consolidate 
overall disruption to edk2.



- Would the version update require a whole-tree re-uncrustification?


Yes. I just did it. It is relatively minor and impacts expected code areas.

https://github.com/tianocore/edk2/pull/5043/files

I'm happy to send that to the list if desired.


The reason I'm not just ignoring this topic is that 73.0.8 actually
produces *better output* than 73.0.3, at least in the one instance I
encountered. Compare:


diff --git 
a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c 
b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
index 434cdca84b23..3a6f75988220 100644
--- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
+++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
@@ -43,12 +43,12 @@ STATIC EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL
  STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  mMmio64Configuration = {
ACPI_ADDRESS_SPACE_DESCRIPTOR,   // Desc
(UINT16)(// Len
-   sizeof 
(EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
-   OFFSET_OF (
- 
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
- ResType
- )
-   ),
+sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+OFFSET_OF (
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+  ResType
+  )
+),
ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType
0,   // GenFlag
0,   // SpecificFlag
@@ -77,12 +77,12 @@ STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  
mMmio64Configuration = {
  STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  mOptionRomConfiguration =   {
ACPI_ADDRESS_SPACE_DESCRIPTOR,   // Desc
(UINT16)(// Len
-   sizeof 
(EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
-   OFFSET_OF (
- 
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
- ResType
- )
-   ),
+sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+OFFSET_OF (
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+  ResType
+  )
+),
ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType
0,   // GenFlag
0,   // Disable option roms 
SpecificFlag


Note that 73.0.3 indents the subexpression to the "//"  comment on the
previous line, while 73.0.8 ignores the comment -- which I think is
justified here.

I believe this improvement may come from uncrustify commit 239c4fad745b
("Prevent endless indentation scenario in struct assignment",
2022-07-29). I think it's worth having in edk2.

CC: stewards, Pedro (commit 6ded9f50c3aa), Marcin (traditionally a big
fan of uncrustify :))

Thanks
Laszlo



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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Rebecca Cran via groups.io


On 11/13/2023 5:29 AM, Marcin Juszkiewicz via groups.io wrote:

Still a fan of adding edk2-uncrustify to BaseTools. If we are expected 
to use it then let it get installed at same moment as "build" command is.


The issue with doing this is there's a push to remove all C/C++ code 
from BaseTools (including porting existing code to Python), and adding 
edk2-uncrustify would work against that.


--
Rebecca Cran




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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Pedro Falcato

On Mon, Nov 13, 2023 at 7:07 PM Pedro Falcato via groups.io
 wrote:
>
> On Mon, Nov 13, 2023 at 11:58 AM Laszlo Ersek  wrote:
> >
> > Hi Michael,
> >
> > recently I encountered an uncrustify failure on github.
> >
> > The reason was that my local uncrustify was *more recent* (73.0.8) than
> > the one we use in edk2 CI (which is 73.0.3, per the edk2 file
> > ".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").
>
> Wait, you can use upstream uncrustify? I'm just using whatever
> uncrustify version I took from the project-mu fork...

Scratch that, I see there's a 73.0.8 in mu's fork.

-- 
Pedro


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

Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

2023-11-13 Thread Pedro Falcato

On Mon, Nov 13, 2023 at 11:58 AM Laszlo Ersek  wrote:
>
> Hi Michael,
>
> recently I encountered an uncrustify failure on github.
>
> The reason was that my local uncrustify was *more recent* (73.0.8) than
> the one we use in edk2 CI (which is 73.0.3, per the edk2 file
> ".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").

Wait, you can use upstream uncrustify? I'm just using whatever
uncrustify version I took from the project-mu fork...

>
> Updating the version number in the YAML file (i.e., advancing edk2 to
> version 73.0.8) seems easy enough, but:
>
> - Do you think 73.0.8 is mature enough for adoption in edk2?
>
>   This upstream uncrustify release was tagged in April (and I can't see
>   any more recent commits), so I assume it should be stable.
>
> - Would the version update require a whole-tree re-uncrustification?

Please, no. I didn't mind doing an initial reformatting at first, but
doing this continuously is both 1) problem-prone 2) just amazing
amounts of churn.
Let's say I have version N, you have version N+1 - we may never get
any final, formatted output as your version formats it differently
from mine.

I don't know how the CI is doing its thing atm (I haven't merged
anything myself to edk2), but the uncrustify check should be relaxed
to just a warning. There's nothing wrong with what my uncrustify
version is formatting to, there's nothing wrong with yours either, and
CI isn't necessarily wrong either.

And, to be fair, I already find uncrustify a large pain in the butt to
use (requiring a custom fork really does not help), but I find the
benefits worth it *locally*, as my coding style is also quite
different from the NT-esque style.

-- 
Pedro

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

62 matches

Mail list logo