Re: [edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
> -Original Message-
> From: Jeff Brasen
> Sent: Friday, June 30, 2023 11:21 AM
> To: Ni, Ray ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Gao, Liming
> ; Wu, Hao A
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver
> binding
>
> Not sure why the patch failed to apply I'll see if there is something wrong
> with my
> gitconfig tomorrow. The path you suggested below is exactly what our current
> implementation does. However, I am trying to make our pcie controller driver
> do
> async initialization so that using a depex is less ideal as we may have to
> postpone
> driver load to after end of dxe instead of just the connection. It seemed
> that a
> driver binding type approach was a good approach for this.
I am curious how the pcie root (root complex) initialization is done in async
way?
Does it use a timer callback to poll the initialization status every certain
interval?
On the other hand, even you make PciHostBridge as a UEFI driver-model driver,
you still
require some code to initiate the "ConnectController()". How is that done?
Comparing the two paths (today's = my proposal, new way = your patch), both
require
some code to explicitly call "ConnectController()".
>
> On a less important implementation if the pieces that live under the library
> are
> driver binding we have to reject any stop requests as there is no driver
> linkage
> between the two layers.
In x86, root complex (pcie root) is almost the root of all peripherals. I
cannot see a value to Stop () the pcie root.
If you check the PciBus implementation, it supports Stop() but the Stop() only
succeeds when all upper layer drivers
succeed to Stop(). (usually it's not the case.)
And even PciBus.Stop() succeeds, the resource(MMIO/IO/BUS) allocation is not
un-done. It's only the PciIo
handles that are destroyed in software level.
Thanks,
Ray
>
> -Jeff
>
>
> -Original Message-
> From: Ni, Ray
> Sent: Thursday, June 29, 2023 8:29 PM
> To: Jeff Brasen ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Gao, Liming
> ; Wu, Hao A
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver
> binding
>
> External email: Use caution opening links or attachments
>
>
> I failed to apply the patch in my local tree.
>
> It seems you invented a new EdkiiRootBridgeIo protocol and a certain
> proprietary
> driver would produce this protocol instance.
> Then the open source PciHostBridge driver starts on that.
>
> Then, why not implement your own PciHostBridgeLib and let it depends on some
> "AllRootBridgeIoInformationIsReady" protocol.
> So that the PciHostBridge driver could still call PciHostBridgeLib and all
> your
> implementation in this patch can be in that lib.
>
> Thanks,
> Ray
>
> > -Original Message-
> > From: Jeff Brasen
> > Sent: Friday, June 30, 2023 4:54 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J ; Gao, Liming
> > ; Wu, Hao A ; Ni, Ray
> > ; Jeff Brasen
> > Subject: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver
> > binding
> >
> > If the platform does not support any PCIe devices using the library
> >
> > method allow devices to connect to host bridge via driver binding.
> >
> >
> >
> > Signed-off-by: Jeff Brasen
> >
> > ---
> >
> > .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 649
> > ++
> >
> > .../Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
> >
> > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 13 +
> >
> > .../Pci/PciHostBridgeDxe/PciRootBridgeIo.c| 24 +
> >
> > MdeModulePkg/MdeModulePkg.dec | 4 +
> >
> > 5 files changed, 562 insertions(+), 129 deletions(-)
> >
> >
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >
> > index d573e532ba..506c6660ae 100644
> >
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >
> > @@ -422,167 +422,320 @@ IoMmuProtocolCallback (
> >
> > }
> >
> >
> >
> > /**
> >
> > + PCI Root Bridge Memory setup.
> >
> >
> >
> > - Entry point of this driver.
> >
> > + @param RootBridgeRoot Bridge instance.
> >
> >
> >
> > - @param ImageHandle Image handle of this driver.
> >
> > - @param SystemTable Pointer to standard EFI system table.
> >
> > -
> >
> > - @retval EFI_SUCCESS Succeed.
> >
> > - @retval EFI_DEVICE_ERROR Fail to install PCI_ROOT_BRIDGE_IO protocol.
> >
> > + @retval EFI_SUCCESS Memory was setup correctly
> >
> > + @retval othersError in setup
> >
> >
> >
> > **/
> >
> > EFI_STATUS
> >
> > EFIAPI
> >
> > -InitializePciHostBridge (
> >
> > - IN EFI_HANDLEImageHandle,
> >
> > - IN EFI_SYSTEM_TABLE *SystemTable
> >
> > +PciRootBridgeMemorySetup (
> >
> > + IN PCI_ROOT_BRIDGE *RootBridge
> >
> >)
> >
> > {
> >
> >EFI_STATUSStatus;
> >
> > - PCI_HOST_BRIDGE_INSTANCE *HostBridge;
> >
Re: [edk2-devel] Subject: [PATCH] Modify IPMI_GET_SYSTEM_UUID_RESPONSE to use IPMI_GUID.
[AMD Official Use Only - General]
> -Original Message-
> From: Aaron Pop
> Sent: Friday, June 30, 2023 4:17 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; michael.d.kin...@intel.com
> Subject: Subject: [PATCH] Modify IPMI_GET_SYSTEM_UUID_RESPONSE to use
> IPMI_GUID.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> IPMI specification's Get Device Guid Command says that data is returned in
> "least signification byte first" and "this is the reverse of convention
> described in RFC4122". IPMI_GET_SYSTEM_UUID_RESPONSE is defined to use
> EFI_GUID, which is reverse format of Ipmi specification. Correcting
> IPMI_GET_SYSTEM_UUID_RESPONSE to use IPMI_GUID.
>
> Signed-off-by: Aaron Pop
> ---
> MdePkg/Include/IndustryStandard/IpmiNetFnApp.h | 18
> --
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h
> b/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h
> index b6bc91f46c..afdc2dc30b 100644
> --- a/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h
> +++ b/MdePkg/Include/IndustryStandard/IpmiNetFnApp.h
> @@ -14,6 +14,7 @@
> Copyright (c) 1999 - 2018, Intel Corporation. All rights reserved.
> Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
> Copyright (c) 2023, Ampere Computing LLC. All rights reserved.
> + Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> @@ -488,9 +489,22 @@ typedef struct {
> //
> #define IPMI_APP_GET_SYSTEM_GUID 0x37
>
> +//
> +// In IPMI, GUID are stored least-significant byte first.
> +// The order of fields are the reverse of convention described in
> [RFC4122].
> +// Note: This definition is used for both the IPMI_APP_GET_SYSTEM_GUID
> +// and IPMI_APP_GET_DEVICE_GUID.
> +//
> +typedef struct {
> + UINT8 Data4[8];
> + UINT16Data3;
> + UINT16Data2;
> + UINT32Data1;
> +} IPMI_GUID;
> +
> typedef struct {
> - UINT8 CompletionCode;
> - EFI_GUIDSystemUuid;
> + UINT8CompletionCode;
> + IPMI_GUIDSystemUuid;
> } IPMI_GET_SYSTEM_UUID_RESPONSE;
>
Hi Aaron,
Thanks for this contribution.
Due to the return value is in EFI_GUID format as the UEFI friendly
implementation, we have to convert IPMI_GUID to EFI_GUID as a return value.
Thanks
Abner
> //
> --
> 2.41.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106557): https://edk2.groups.io/g/devel/message/106557
Mute This Topic: https://groups.io/mt/99858608/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Does RedfishPkg support BMC Chip's lan over usb feature?
[AMD Official Use Only - General] Yes, edk2 Redfish does support BMC exposed USB NIC, as RedfishPkg relies on RestEx protocol for discovering Redfish service. The current RestEx implementation is network based, which means USB NIC would be used as the transport interface for Redfish communications between host and BMC. Abner From: devel@edk2.groups.io On Behalf Of Yoshinoya via groups.io Sent: Friday, June 30, 2023 11:13 AM To: devel@edk2.groups.io Subject: [edk2-devel] Does RedfishPkg support BMC Chip's lan over usb feature? Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. Hi, The communication between BIOS and BMC usually is IPMI over LPC interface. The communication also supports with IPMI over IP. BMC chip has ability to emulate itself as a usb network card, and connected with motherboard's usb physical port. So, current redfishpkg supports BMC chip's lan over usb feature? Thanks -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106556): https://edk2.groups.io/g/devel/message/106556 Mute This Topic: https://groups.io/mt/99866456/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
Not sure why the patch failed to apply I'll see if there is something wrong
with my gitconfig tomorrow. The path you suggested below is exactly what our
current implementation does. However, I am trying to make our pcie controller
driver do async initialization so that using a depex is less ideal as we may
have to postpone driver load to after end of dxe instead of just the
connection. It seemed that a driver binding type approach was a good approach
for this.
On a less important implementation if the pieces that live under the library
are driver binding we have to reject any stop requests as there is no driver
linkage between the two layers.
-Jeff
-Original Message-
From: Ni, Ray
Sent: Thursday, June 29, 2023 8:29 PM
To: Jeff Brasen ; devel@edk2.groups.io
Cc: Wang, Jian J ; Gao, Liming
; Wu, Hao A
Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
External email: Use caution opening links or attachments
I failed to apply the patch in my local tree.
It seems you invented a new EdkiiRootBridgeIo protocol and a certain
proprietary driver would produce this protocol instance.
Then the open source PciHostBridge driver starts on that.
Then, why not implement your own PciHostBridgeLib and let it depends on some
"AllRootBridgeIoInformationIsReady" protocol.
So that the PciHostBridge driver could still call PciHostBridgeLib and all your
implementation in this patch can be in that lib.
Thanks,
Ray
> -Original Message-
> From: Jeff Brasen
> Sent: Friday, June 30, 2023 4:54 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Gao, Liming
> ; Wu, Hao A ; Ni, Ray
> ; Jeff Brasen
> Subject: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver
> binding
>
> If the platform does not support any PCIe devices using the library
>
> method allow devices to connect to host bridge via driver binding.
>
>
>
> Signed-off-by: Jeff Brasen
>
> ---
>
> .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 649
> ++
>
> .../Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
>
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 13 +
>
> .../Pci/PciHostBridgeDxe/PciRootBridgeIo.c| 24 +
>
> MdeModulePkg/MdeModulePkg.dec | 4 +
>
> 5 files changed, 562 insertions(+), 129 deletions(-)
>
>
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> index d573e532ba..506c6660ae 100644
>
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> @@ -422,167 +422,320 @@ IoMmuProtocolCallback (
>
> }
>
>
>
> /**
>
> + PCI Root Bridge Memory setup.
>
>
>
> - Entry point of this driver.
>
> + @param RootBridgeRoot Bridge instance.
>
>
>
> - @param ImageHandle Image handle of this driver.
>
> - @param SystemTable Pointer to standard EFI system table.
>
> -
>
> - @retval EFI_SUCCESS Succeed.
>
> - @retval EFI_DEVICE_ERROR Fail to install PCI_ROOT_BRIDGE_IO protocol.
>
> + @retval EFI_SUCCESS Memory was setup correctly
>
> + @retval othersError in setup
>
>
>
> **/
>
> EFI_STATUS
>
> EFIAPI
>
> -InitializePciHostBridge (
>
> - IN EFI_HANDLEImageHandle,
>
> - IN EFI_SYSTEM_TABLE *SystemTable
>
> +PciRootBridgeMemorySetup (
>
> + IN PCI_ROOT_BRIDGE *RootBridge
>
>)
>
> {
>
>EFI_STATUSStatus;
>
> - PCI_HOST_BRIDGE_INSTANCE *HostBridge;
>
> - PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
>
> - PCI_ROOT_BRIDGE *RootBridges;
>
> - UINTN RootBridgeCount;
>
> - UINTN Index;
>
> + UINT64HostAddress;
>
>PCI_ROOT_BRIDGE_APERTURE *MemApertures[4];
>
>UINTN MemApertureIndex;
>
> - BOOLEAN ResourceAssigned;
>
> - LIST_ENTRY*Link;
>
> - UINT64HostAddress;
>
>
>
> - RootBridges = PciHostBridgeGetRootBridges ();
>
> - if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
>
> -return EFI_UNSUPPORTED;
>
> - }
>
> -
>
> - Status = gBS->LocateProtocol (, NULL, (VOID
> **));
>
> - ASSERT_EFI_ERROR (Status);
>
> -
>
> - //
>
> - // Most systems in the world including complex servers have only
> one Host Bridge.
>
> - //
>
> - HostBridge = AllocateZeroPool (sizeof (PCI_HOST_BRIDGE_INSTANCE));
>
> - ASSERT (HostBridge != NULL);
>
> -
>
> - HostBridge->Signature= PCI_HOST_BRIDGE_SIGNATURE;
>
> - HostBridge->CanRestarted = TRUE;
>
> - InitializeListHead (>RootBridges);
>
> - ResourceAssigned = FALSE;
>
> -
>
> - //
>
> - // Create Root Bridge Device Handle in this Host Bridge
>
> - //
>
> - for (Index = 0; Index < RootBridgeCount; Index++) {
>
> + if (RootBridge->Io.Base <= RootBridge->Io.Limit) {
>
> //
>
> -// Create Root Bridge Handle Instance
>
> +// Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
>
> +
[edk2-devel] Does RedfishPkg support BMC Chip's lan over usb feature?
Hi, The communication between BIOS and BMC usually is IPMI over LPC interface. The communication also supports with IPMI over IP. BMC chip has ability to emulate itself as a usb network card, and connected with motherboard's usb physical port. So, current redfishpkg supports BMC chip's lan over usb feature? Thanks -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106554): https://edk2.groups.io/g/devel/message/106554 Mute This Topic: https://groups.io/mt/99866456/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 v1 1/1] IntelSiliconPkg/SpiFvbService: Non-functional cleanup
From: Michael Kubacki
During a review of this driver a number of improvements were noted
such as strengthening function input validation, checking return
values, making debug print error levels consistent in certain code
blocks, etc.
These type of changes are made with no explicit change to driver
functionality.
Cc: Ray Ni
Cc: Rangasai V Chaganty
Cc: Isaac Oram
Cc: Ashraf Ali S
Signed-off-by: Michael Kubacki
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
| 35 ++-
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
| 248
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
| 54 +++--
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
| 31 ++-
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.h
| 6 +-
5 files changed, 239 insertions(+), 135 deletions(-)
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
index ab1cb2ef1622..ebbd9edaada3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
@@ -2,16 +2,17 @@
Defines data structure that is the volume header found.
These data is intent to decouple FVB driver with FV header.
-Copyright (c) 2017, Intel Corporation. All rights reserved.
-Copyright (c) Microsoft Corporation.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+ Copyright (c) 2017, Intel Corporation. All rights reserved.
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "SpiFvbServiceCommon.h"
-#define FIRMWARE_BLOCK_SIZE 0x1
+#define FIRMWARE_BLOCK_SIZE SIZE_64KB
#define FVB_MEDIA_BLOCK_SIZEFIRMWARE_BLOCK_SIZE
+
typedef struct {
EFI_PHYSICAL_ADDRESSBaseAddress;
EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
@@ -19,7 +20,7 @@ typedef struct {
} EFI_FVB2_MEDIA_INFO;
/**
- Returns FVB media information for NV variable storage.
+ Returns FVB media information for a firmware volume.
@return FvbMediaInfo A pointer to an instance of FVB media
info produced by this function.
The buffer is allocated internally to
this function and it is the caller's
@@ -100,8 +101,21 @@ FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
GenerateNvStorageFvbMediaInfo
};
+/**
+ Returns an empty firmware volume for the firmware volume at the given base
address.
+
+ @param[in]FvBaseAddress The base address of the firmware volume
requested.
+ @param[out] FvbInfo A pointer that will be set to a buffer for
the firmware volume header
+at the given base address. The buffer is a
pool allocation made in this function.
+
+ @retval EFI_SUCCESS The firmware volume was returned
successfully.
+ @retval EFI_INVALID_PARAMETER The FvbInfo pointer argument is NULL.
+ @retval EFI_NOT_FOUND The firmware volume was not found for the
given base address.
+ @retval EFI_OUT_OF_RESOURCES Insufficient memory to allocate a buffer
to the hold the firmware volume.
+
+**/
EFI_STATUS
-GetFvbInfo (
+GetGeneratedFvByAddress (
IN EFI_PHYSICAL_ADDRESS FvBaseAddress,
OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
)
@@ -111,11 +125,19 @@ GetFvbInfo (
UINTN Index;
EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ if (FvbInfo == NULL) {
+return EFI_INVALID_PARAMETER;
+ }
+
for (Index = 0; Index < ARRAY_SIZE (mFvbMediaInfoGenerators); Index++) {
Status = mFvbMediaInfoGenerators[Index]();
ASSERT_EFI_ERROR (Status);
if (!EFI_ERROR (Status) && (FvbMediaInfo.BaseAddress == FvBaseAddress)) {
FvHeader = AllocateCopyPool (FvbMediaInfo.FvbInfo.HeaderLength,
);
+ if (FvHeader == NULL) {
+ASSERT (FvHeader != NULL);
+return EFI_OUT_OF_RESOURCES;
+ }
//
// Update the checksum value of FV header.
@@ -136,5 +158,6 @@ GetFvbInfo (
return EFI_SUCCESS;
}
}
+
return EFI_NOT_FOUND;
}
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
index fcdc715263f2..e21113682f4f 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
@@ -2,9 +2,9 @@
Common driver source for several Serial Flash devices
which are compliant with the Intel(R) Serial Flash Interface Compatibility
Specification.
-Copyright (c) 2017, Intel Corporation. All rights reserved.
-Copyright (c) Microsoft Corporation.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+ Copyright
Re: [edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
I failed to apply the patch in my local tree.
It seems you invented a new EdkiiRootBridgeIo protocol and a certain
proprietary driver would produce this protocol instance.
Then the open source PciHostBridge driver starts on that.
Then, why not implement your own PciHostBridgeLib and let it depends on some
"AllRootBridgeIoInformationIsReady" protocol.
So that the PciHostBridge driver could still call PciHostBridgeLib and all your
implementation in this patch can be in that lib.
Thanks,
Ray
> -Original Message-
> From: Jeff Brasen
> Sent: Friday, June 30, 2023 4:54 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Gao, Liming
> ; Wu, Hao A ; Ni, Ray
> ; Jeff Brasen
> Subject: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
>
> If the platform does not support any PCIe devices using the library
>
> method allow devices to connect to host bridge via driver binding.
>
>
>
> Signed-off-by: Jeff Brasen
>
> ---
>
> .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 649 ++
>
> .../Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
>
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 13 +
>
> .../Pci/PciHostBridgeDxe/PciRootBridgeIo.c| 24 +
>
> MdeModulePkg/MdeModulePkg.dec | 4 +
>
> 5 files changed, 562 insertions(+), 129 deletions(-)
>
>
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> index d573e532ba..506c6660ae 100644
>
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>
> @@ -422,167 +422,320 @@ IoMmuProtocolCallback (
>
> }
>
>
>
> /**
>
> + PCI Root Bridge Memory setup.
>
>
>
> - Entry point of this driver.
>
> + @param RootBridgeRoot Bridge instance.
>
>
>
> - @param ImageHandle Image handle of this driver.
>
> - @param SystemTable Pointer to standard EFI system table.
>
> -
>
> - @retval EFI_SUCCESS Succeed.
>
> - @retval EFI_DEVICE_ERROR Fail to install PCI_ROOT_BRIDGE_IO protocol.
>
> + @retval EFI_SUCCESS Memory was setup correctly
>
> + @retval othersError in setup
>
>
>
> **/
>
> EFI_STATUS
>
> EFIAPI
>
> -InitializePciHostBridge (
>
> - IN EFI_HANDLEImageHandle,
>
> - IN EFI_SYSTEM_TABLE *SystemTable
>
> +PciRootBridgeMemorySetup (
>
> + IN PCI_ROOT_BRIDGE *RootBridge
>
>)
>
> {
>
>EFI_STATUSStatus;
>
> - PCI_HOST_BRIDGE_INSTANCE *HostBridge;
>
> - PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
>
> - PCI_ROOT_BRIDGE *RootBridges;
>
> - UINTN RootBridgeCount;
>
> - UINTN Index;
>
> + UINT64HostAddress;
>
>PCI_ROOT_BRIDGE_APERTURE *MemApertures[4];
>
>UINTN MemApertureIndex;
>
> - BOOLEAN ResourceAssigned;
>
> - LIST_ENTRY*Link;
>
> - UINT64HostAddress;
>
>
>
> - RootBridges = PciHostBridgeGetRootBridges ();
>
> - if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
>
> -return EFI_UNSUPPORTED;
>
> - }
>
> -
>
> - Status = gBS->LocateProtocol (, NULL, (VOID
> **));
>
> - ASSERT_EFI_ERROR (Status);
>
> -
>
> - //
>
> - // Most systems in the world including complex servers have only one Host
> Bridge.
>
> - //
>
> - HostBridge = AllocateZeroPool (sizeof (PCI_HOST_BRIDGE_INSTANCE));
>
> - ASSERT (HostBridge != NULL);
>
> -
>
> - HostBridge->Signature= PCI_HOST_BRIDGE_SIGNATURE;
>
> - HostBridge->CanRestarted = TRUE;
>
> - InitializeListHead (>RootBridges);
>
> - ResourceAssigned = FALSE;
>
> -
>
> - //
>
> - // Create Root Bridge Device Handle in this Host Bridge
>
> - //
>
> - for (Index = 0; Index < RootBridgeCount; Index++) {
>
> + if (RootBridge->Io.Base <= RootBridge->Io.Limit) {
>
> //
>
> -// Create Root Bridge Handle Instance
>
> +// Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
>
> +// For GCD resource manipulation, we need to use host address.
>
> //
>
> -RootBridge = CreateRootBridge ([Index]);
>
> -ASSERT (RootBridge != NULL);
>
> -if (RootBridge == NULL) {
>
> - continue;
>
> +HostAddress = TO_HOST_ADDRESS (
>
> +RootBridge->Io.Base,
>
> +RootBridge->Io.Translation
>
> +);
>
> +
>
> +Status = AddIoSpace (
>
> + HostAddress,
>
> + RootBridge->Io.Limit - RootBridge->Io.Base + 1
>
> + );
>
> +ASSERT_EFI_ERROR (Status);
>
> +if (EFI_ERROR (Status)) {
>
> + return Status;
>
> }
>
>
>
> -//
>
> -// Make sure all root bridges share the same ResourceAssigned value.
>
> -//
>
> -if (Index == 0) {
>
> - ResourceAssigned = RootBridges[Index].ResourceAssigned;
>
> -}
Re: [edk2-devel] [PATCH] RedfishPkg: Fix SortLib library class name typo.
[AMD Official Use Only - General] Hi Mike, I see there are CI errors for this PR, could you please check it? Thanks! Abner > -Original Message- > From: Mike Maslenkin > Sent: Friday, June 30, 2023 6:44 AM > To: Chang, Abner > Cc: devel@edk2.groups.io; nick...@nvidia.com; ig...@ami.com > Subject: Re: [PATCH] RedfishPkg: Fix SortLib library class name typo. > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > Sure! > https://github.com/tianocore/edk2-redfish-client/pull/43 > > Thanks, > Mike > > > On Tue, Jun 27, 2023 at 3:05 AM Chang, Abner > wrote: > > > > [AMD Official Use Only - General] > > > > Hi Mike, could you please create a PR for this change? Then I can merge it > once the change passed CI. > > > > Thanks > > Abner > > > > > -Original Message- > > > From: Chang, Abner > > > Sent: Wednesday, June 21, 2023 8:26 AM > > > To: Mike Maslenkin ; devel@edk2.groups.io > > > Cc: nick...@nvidia.com; ig...@ami.com > > > Subject: RE: [PATCH] RedfishPkg: Fix SortLib library class name typo. > > > > > > Reviewed-by: Abner Chang > > > > > > > -Original Message- > > > > From: Mike Maslenkin > > > > Sent: Wednesday, June 21, 2023 5:30 AM > > > > To: devel@edk2.groups.io > > > > Cc: Chang, Abner ; nick...@nvidia.com; > > > > ig...@ami.com; Mike Maslenkin > > > > Subject: [PATCH] RedfishPkg: Fix SortLib library class name typo. > > > > > > > > Caution: This message originated from an External Source. Use proper > > > caution > > > > when opening attachments, clicking links, or responding. > > > > > > > > > > > > BaseSortLib is the library instance name not the class name. > > > > > > > > Signed-off-by: Mike Maslenkin > > > > Cc: Abner Chang > > > > Cc: Nickle Wang > > > > Cc: Igor Kulchytskyy > > > > --- > > > > RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf | 2 +- > > > > RedfishPkg/RedfishLibs.dsc.inc| 2 +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > index 5c1c7cffa650..6ff5dba75c30 100644 > > > > --- a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > +++ b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > @@ -25,7 +25,7 @@ > > > > > > > > > > > > [LibraryClasses] > > > > > > > >BaseLib > > > > > > > > - BaseSortLib > > > > > > > > + SortLib > > > > > > > >DebugLib > > > > > > > >MemoryAllocationLib > > > > > > > >UefiRuntimeServicesTableLib > > > > > > > > diff --git a/RedfishPkg/RedfishLibs.dsc.inc > b/RedfishPkg/RedfishLibs.dsc.inc > > > > index 6e497899b556..03b7ef6f3289 100644 > > > > --- a/RedfishPkg/RedfishLibs.dsc.inc > > > > +++ b/RedfishPkg/RedfishLibs.dsc.inc > > > > @@ -14,7 +14,7 @@ > > > > !if $(REDFISH_ENABLE) == TRUE > > > > > > > >RestExLib|RedfishPkg/Library/DxeRestExLib/DxeRestExLib.inf > > > > > > > > > Ucs2Utf8Lib|RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf > > > > > > > > - BaseSortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf > > > > > > > > + SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf > > > > > > > > > RedfishCrtLib|RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > > > > >JsonLib|RedfishPkg/Library/JsonLib/JsonLib.inf > > > > > > > >RedfishLib|RedfishPkg/PrivateLibrary/RedfishLib/RedfishLib.inf > > > > > > > > -- > > > > 2.32.0 (Apple Git-132) > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106551): https://edk2.groups.io/g/devel/message/106551 Mute This Topic: https://groups.io/mt/99664815/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] 回复: [PATCH v1 00/24] Update Edk2-pytools to latest versions
Sean and Joey: Thanks for your detail information. Now, I understand the rule that require explicit opt-in for all the stuart based validation tools. I have no more comments for this patch set. Thanks Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Sean > 发送时间: 2023年6月30日 2:01 > 收件人: devel@edk2.groups.io; gaolim...@byosoft.com.cn; 'Joey Vagedes' > > 抄送: 'Abner Chang' ; 'Alexei Fedorov' > ; 'Ard Biesheuvel' ; > 'Ashraf Ali S' ; 'Chasel Chiu' > ; > 'Chen Gang C' ; 'Daniel Schaefer' > ; 'Duggapu Chinni B' ; > 'Eric Dong' ; 'Gerd Hoffmann' ; > 'Guo Dong' ; 'Guomin Jiang' ; > 'Hao A Wu' ; 'Igor Kulchytskyy' ; > 'James Lu' ; 'Jian J Wang' ; > 'Jiewen Yao' ; 'Leif Lindholm' > ; 'Maciej Rabeda' > ; 'Michael D Kinney' > ; 'Michael Kubacki' > ; 'Nate DeSimone' > ; 'Nickle Wang' ; > 'Pierre Gondois' ; 'Rahul Kumar' > ; 'Ray Han Lim Ng' ; > 'Ray Ni' ; 'Sami Mujawar' ; > 'Sean Brogan' ; 'Sean Rhodes' > ; 'Siyuan Fu' ; 'Star Zeng' > ; 'Susovan Mohapatra' > ; 'Ted Kuo' ; 'Wei6 Xu' > ; 'Xiaoyu Lu' ; 'Yi Li' > ; 'Zhichao Gao' ; 'Zhiguang Liu' > > 主题: Re: [edk2-devel] 回复: [PATCH v1 00/24] Update Edk2-pytools to latest > versions > > Liming, > > > In general, due to feedback on this list we have tried to require > explicit opt-in for all the stuart based validation tools. So could > that be done yes, but it goes against our current consistent approach of > requiring opt-in and then would put the burden on a platform/package > owner to opt-out if they didn't want this new functionality. > > > Thanks > > Sean > > > > On 6/26/2023 5:59 PM, gaoliming via groups.io wrote: > > Joey: > >So, this change is required for all package YAML. If yes, can PrEval be > > auto set to the package dsc if only one DSC is in Package directory? > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106550): https://edk2.groups.io/g/devel/message/106550 Mute This Topic: https://groups.io/mt/99865418/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] 回复: [PATCH 0/2] dp command without ACPI
Jeff: Performance has been stored as FPDT table. It can be directly mapped to the configuration table. So, this solution is good to me. Reviewed-by: Liming Gao Thanks Liming > -邮件原件- > 发件人: Jeff Brasen > 发送时间: 2023年6月30日 5:09 > 收件人: devel@edk2.groups.io > 抄送: jian.j.w...@intel.com; gaolim...@byosoft.com.cn; > dandan...@intel.com; zhichao@intel.com; Jeff Brasen > > 主题: [PATCH 0/2] dp command without ACPI > > Systems that do not boot with ACPI (system that use device tree for example) > can not use the shell dp command. This patch adds this to the configuration > table so that dp command can get this without the FPDT table. > > I am open to other ways for this to be passed if desired (Installed protocol, > handler of the status code, etc) but wanted to post this to at least get > thoughts > on this. > > -Jeff > > Jeff Brasen (2): > MdeModulePkg/DxeCorePerformanceLib: Install BPDT in config table > ShellPkg/Dp: Allow dp command to work without ACPI > > .../DxeCorePerformanceLib/DxeCorePerformanceLib.c | 2 ++ > ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 11 > --- > ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf| 1 + > .../DpDynamicCommand/DpDynamicCommand.inf | 1 + > 4 files changed, 12 insertions(+), 3 deletions(-) > > -- > 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106549): https://edk2.groups.io/g/devel/message/106549 Mute This Topic: https://groups.io/mt/99865222/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/1] Fixing RngDxe error for ARM/AARCH64
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491 This patch series is a follow-up of previous submission: https://edk2.groups.io/g/devel/message/91995 The main changes between v1 and v2 patches are: - Removed a fix which is a duplicate effort of https://edk2.groups.io/g/devel/message/104347 - Added reviewed-by tag This change is verified on QEMU based ARM SBSA system. Patch v2 branch: https://github.com/kuqin12/edk2/tree/fix_rng_edk2_v2 Cc: Jiewen Yao Cc: Jian J Wang Cc: Sami Mujawar Cc: Pierre Gondois Kun Qin (1): SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 2 +- SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.41.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106546): https://edk2.groups.io/g/devel/message/106546 Mute This Topic: https://groups.io/mt/99863880/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2 1/1] SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator
From: Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491
mAvailableAlgoArray is currently allocated for "RNG_AVAILABLE_ALGO_MAX"
number of bytes, whereas it was dereferenced as "EFI_RNG_ALGORITHM".
This change fixed the buffer allocation logic by allocating a proper size
of buffer before referencing.
Cc: Jiewen Yao
Cc: Jian J Wang
Cc: Sami Mujawar
Cc: Pierre Gondois
Signed-off-by: Kun Qin
Reviewed-by: Sami Mujawar
---
Notes:
v2:
- Added reviewed-by tag [Sami]
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 2 +-
SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..e7107a0b7039 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -33,7 +33,7 @@ GetAvailableAlgorithms (
UINT16 MinorRevision;
// Rng algorithms 2 times, one for the allocation, one to populate.
- mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
+ mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX * sizeof
(EFI_RNG_ALGORITHM));
if (mAvailableAlgoArray == NULL) {
return EFI_OUT_OF_RESOURCES;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
index 4b24f5c4a69b..5e621df601fb 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
@@ -32,7 +32,7 @@ GetAvailableAlgorithms (
UINT16 MinorRevision;
// Rng algorithms 2 times, one for the allocation, one to populate.
- mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
+ mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX * sizeof
(EFI_RNG_ALGORITHM));
if (mAvailableAlgoArray == NULL) {
return EFI_OUT_OF_RESOURCES;
}
--
2.41.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106547): https://edk2.groups.io/g/devel/message/106547
Mute This Topic: https://groups.io/mt/99863881/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
Hi Sami,
Your suggestion in https://edk2.groups.io/g/devel/message/106511 works
properly during my test.
But I think we still need to keep the `+ , ` change below
as a bug fix?
Thanks,
Kun
On 6/29/2023 3:28 AM, Sami Mujawar wrote:
Hi Pierre,
I think this patch would not be required if my suggestions for patch
6/8 are adopted.
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
The first element of mAvailableAlgoArray should be the default
algorithm to avoid going through a selection process at each
RngGetRNG() call.
Once all the available Rng algorithms have been probed, place
a safe Rng algorithm at the first position of mAvailableAlgoArray.
Signed-off-by: Pierre Gondois
---
.../RngDxe/AArch64/AArch64Algo.c | 48 ++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git
a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index a1ff7bd58fda..ed236b2e8141 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -17,6 +17,50 @@
// Maximum number of Rng algorithms.
#define RNG_AVAILABLE_ALGO_MAX 2
+/** mAvailableAlgoArray[0] should contain the default Rng algorithm.
+ The Rng algorithm at the first index might be unsafe.
+ If a safe algorithm is available, choose it as the default one.
+**/
+VOID
+EFIAPI
+RngFindDefaultAlgo (
+ VOID
+ )
+{
+ EFI_RNG_ALGORITHM *CurAlgo;
+ EFI_RNG_ALGORITHM TmpGuid;
+ UINTN Index;
+
+ CurAlgo = [0];
+
+ if (IsZeroGuid (CurAlgo) ||
+ !CompareGuid (CurAlgo, ))
+ {
+ // mAvailableAlgoArray[0] is a valid Rng algorithm.
+ return;
+ }
+
+ for (Index = 1; Index < mAvailableAlgoArrayCount; Index++) {
+ CurAlgo = [Index];
+ if (!IsZeroGuid (CurAlgo) ||
+ CompareGuid (CurAlgo, ))
+ {
+ break;
+ }
+ }
+
+ if (Index == mAvailableAlgoArrayCount) {
+ // No valid Rng algorithm available.
+ return;
+ }
+
+ CopyMem (, CurAlgo, sizeof (EFI_RNG_ALGORITHM));
+ CopyMem (CurAlgo, [0], sizeof
(EFI_RNG_ALGORITHM));
+ CopyMem ([0], , sizeof
(EFI_RNG_ALGORITHM));
+
+ return;
+}
+
/** Allocate and initialize mAvailableAlgoArray with the available
Rng algorithms. Also update mAvailableAlgoArrayCount.
@@ -45,7 +89,7 @@ GetAvailableAlgorithms (
if (!EFI_ERROR (Status)) {
CopyMem (
[mAvailableAlgoArrayCount],
- RngGuid,
+ ,
sizeof (RngGuid)
);
mAvailableAlgoArrayCount++;
@@ -68,5 +112,7 @@ GetAvailableAlgorithms (
mAvailableAlgoArrayCount++;
}
+ RngFindDefaultAlgo ();
+
return EFI_SUCCESS;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106545): https://edk2.groups.io/g/devel/message/106545
Mute This Topic: https://groups.io/mt/98779044/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 3/4] DynamicTablesPkg: Add support to add Strings to package
Add API to add a String to a package created with NamedPackage API.
Signed-off-by: Jeff Brasen
Reviewed-by: Swatisri Kantamsetti
Reviewed-by: Ashish Singhal
---
.../Include/Library/AmlLib/AmlLib.h | 17
.../Common/AmlLib/CodeGen/AmlCodeGen.c| 82 +++
2 files changed, 99 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index b82c7a3ce8..043e7ee901 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1472,4 +1472,21 @@ AmlCreateCpcNode (
OUT AML_OBJECT_NODE_HANDLE *NewCpcNode OPTIONAL
);
+/** AML code generation to add a string to the package in a named node.
+
+
+ @param [in] String String to add
+ @param [in] NamedNode Node to add the string to the included package.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddStringToNamedPackage (
+ IN CHAR8 *String,
+ IN AML_OBJECT_NODE_HANDLE NamedNode
+ );
+
#endif // AML_LIB_H_
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 2b95839e4c..a0d01032d5 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3665,3 +3665,85 @@ error_handler:
AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);
return Status;
}
+
+/** AML code generation to add a string to the package in a named node.
+
+
+ @param [in] Name String to add
+ @param [in] NamedNode Node to add the string to the included package.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddStringToNamedPackage (
+ IN CHAR8 *String,
+ IN AML_OBJECT_NODE_HANDLE NamedNode
+ )
+{
+ EFI_STATUS Status;
+ AML_DATA_NODE *DataNode;
+ CHAR8 *AmlNameString;
+ UINT32 AmlNameStringSize;
+ AML_OBJECT_NODE_HANDLE PackageNode;
+
+ DataNode = NULL;
+
+ if ((NamedNode == NULL) ||
+ (AmlGetNodeType ((AML_NODE_HANDLE)NamedNode) != EAmlNodeObject) ||
+ (!AmlNodeHasOpCode (NamedNode, AML_NAME_OP, 0)))
+ {
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+ }
+
+ PackageNode = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+ NamedNode,
+ EAmlParseIndexTerm1
+ );
+ if ((PackageNode == NULL) ||
+ (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) ||
+ (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
+ {
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+ }
+
+ Status = ConvertAslNameToAmlName (String, );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ Status = AmlGetNameStringSize (AmlNameString, );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+goto exit_handler;
+ }
+
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeNameString,
+ (UINT8 *)AmlNameString,
+ AmlNameStringSize,
+
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+goto exit_handler;
+ }
+
+ Status = AmlVarListAddTail (
+ (AML_NODE_HANDLE)PackageNode,
+ (AML_NODE_HANDLE)DataNode
+ );
+ ASSERT_EFI_ERROR (Status);
+
+exit_handler:
+ if (AmlNameString != NULL) {
+FreePool (AmlNameString);
+ }
+
+ return Status;
+}
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106544): https://edk2.groups.io/g/devel/message/106544
Mute This Topic: https://groups.io/mt/99861341/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 4/4] DynamicTablesPkg: Add Aml NameUnicodeString API
Add API to generate a Name that contains a Unicode string buffer.
Change-Id: I0116b2921cbbbecc3420ff7a42a7ab6e01852534
Reviewed-by: Swatisri Kantamsetti
Reviewed-by: Ashish Singhal
---
.../Include/Library/AmlLib/AmlLib.h | 31 +++
.../Common/AmlLib/CodeGen/AmlCodeGen.c| 86 +++
2 files changed, 117 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 043e7ee901..5150aa4870 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -958,6 +958,37 @@ AmlCodeGenNameResourceTemplate (
OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
);
+/** AML code generation for a Name object node, containing a String.
+
+ AmlCodeGenNameUnicodeString ("_STR", L"String", ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+Name(_STR, Unicode ("String"))
+
+ @ingroup CodeGenApis
+
+ @param [in] NameString The new variable name.
+ Must be a NULL-terminated ASL NameString
+ e.g.: "DEV0", "DV15.DEV0", etc.
+ The input string is copied.
+ @param [in] String NULL terminated Unicode String to associate to
the
+ NameString.
+ @param [in] ParentNode If provided, set ParentNode as the parent
+ of the node created.
+ @param [out] NewObjectNode If success, contains the created node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenNameUnicodeString (
+ IN CONST CHAR8 *NameString,
+ INCHAR16 *String,
+ INAML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
+ );
+
/** Add a _PRT entry.
AmlCodeGenPrtEntry (0x0, 0, "LNKA", 0, PrtNameNode) is
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index a0d01032d5..20ddad031d 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -869,6 +869,92 @@ AmlCodeGenNameResourceTemplate (
return Status;
}
+/** AML code generation for a Name object node, containing a String.
+
+ AmlCodeGenNameUnicodeString ("_STR", L"String", ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+ Name(_STR, Unicode ("String"))
+
+ @ingroup CodeGenApis
+
+ @param [in] NameString The new variable name.
+ Must be a NULL-terminated ASL NameString
+ e.g.: "DEV0", "DV15.DEV0", etc.
+ The input string is copied.
+ @param [in] String NULL terminated Unicode String to associate to the
+ NameString.
+ @param [in] ParentNode If provided, set ParentNode as the parent
+ of the node created.
+ @param [out] NewObjectNode If success, contains the created node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenNameUnicodeString (
+ IN CONST CHAR8 *NameString,
+ INCHAR16 *String,
+ INAML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE *ObjectNode;
+ AML_DATA_NODE*DataNode;
+
+ if ((NameString == NULL) ||
+ (String == NULL) ||
+ ((ParentNode == NULL) && (NewObjectNode == NULL)))
+ {
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+ }
+
+ Status = AmlCodeGenBuffer (NULL, 0, );
+ if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+ }
+
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeRaw,
+ (CONST UINT8 *)String,
+ StrSize (String),
+
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);
+return Status;
+ }
+
+ Status = AmlVarListAddTail (
+ (AML_NODE_HEADER *)ObjectNode,
+ (AML_NODE_HEADER *)DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);
+AmlDeleteTree ((AML_NODE_HANDLE)DataNode);
+return Status;
+ }
+
+ Status = AmlCodeGenName (
+ NameString,
+
[edk2-devel] [PATCH 2/4] DynamicTablesPkg: Add support for simple method invocation.
Add support to add Return objects via AML that pass a single integer
argument to the named method.
Signed-off-by: Jeff Brasen
Reviewed-by: Swatisri Kantamsetti
Reviewed-by: Ashish Singhal
---
.../Include/Library/AmlLib/AmlLib.h | 54 +
.../Common/AmlLib/CodeGen/AmlCodeGen.c| 224 ++
2 files changed, 278 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index d201ae9499..b82c7a3ce8 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1194,6 +1194,60 @@ AmlCodeGenMethodRetInteger (
OUT AML_OBJECT_NODE_HANDLE *NewObjectNodeOPTIONAL
);
+/** AML code generation for a method returning a NameString that takes an
+integer argument.
+
+ AmlCodeGenMethodRetNameStringIntegerArgument (
+"MET0", "MET1", 1, TRUE, 3, 5, ParentNode, NewObjectNode
+);
+ is equivalent of the following ASL code:
+Method(MET0, 1, Serialized, 3) {
+ Return (MET1 (5))
+}
+
+ The ASL parameters "ReturnType" and "ParameterTypes" are not asked
+ in this function. They are optional parameters in ASL.
+
+ @param [in] MethodNameString The new Method's name.
+Must be a NULL-terminated ASL NameString
+e.g.: "MET0", "_SB.MET0", etc.
+The input string is copied.
+ @param [in] ReturnedNameString The name of the object returned by the
+method. Optional parameter, can be:
+ - NULL (ignored).
+ - A NULL-terminated ASL NameString.
+ e.g.: "MET0", "_SB.MET0", etc.
+ The input string is copied.
+ @param [in] NumArgs Number of arguments.
+Must be 0 <= NumArgs <= 6.
+ @param [in] IsSerialized TRUE is equivalent to Serialized.
+FALSE is equivalent to NotSerialized.
+Default is NotSerialized in ASL spec.
+ @param [in] SyncLevelSynchronization level for the method.
+Must be 0 <= SyncLevel <= 15.
+Default is 0 in ASL.
+ @param [in] IntegerArgument Argument to pass to the NameString.
+ @param [in] ParentNode If provided, set ParentNode as the parent
+of the node created.
+ @param [out] NewObjectNodeIf success, contains the created node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenMethodRetNameStringIntegerArgument (
+ IN CONST CHAR8 *MethodNameString,
+ IN CONST CHAR8 *ReturnedNameString OPTIONAL,
+ INUINT8 NumArgs,
+ INBOOLEAN IsSerialized,
+ INUINT8 SyncLevel,
+ INUINT64 IntegerArgument,
+ INAML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewObjectNodeOPTIONAL
+ );
+
/** Create a _LPI name.
AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, ) is
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 88537b7e2d..2b95839e4c 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1881,6 +1881,118 @@ AmlCodeGenReturnInteger (
return Status;
}
+/** AML code generation for a Return object node,
+returning the object as an input NameString with a integer argument.
+
+ AmlCodeGenReturn ("NAM1", 6, ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+Return(NAM1 (6))
+
+ The ACPI 6.3 specification, s20.2.5.3 "Type 1 Opcodes Encoding" states:
+DefReturn := ReturnOp ArgObject
+ReturnOp := 0xA4
+ArgObject := TermArg => DataRefObject
+
+ Thus, the ReturnNode must be evaluated as a DataRefObject. It can
+ be a NameString referencing an object. As this CodeGen Api doesn't
+ do semantic checking, it is strongly advised to check the AML bytecode
+ generated by this function against an ASL compiler.
+
+ The ReturnNode must be generated inside a Method body scope.
+
+ @param [in] NameString The object referenced by this NameString
+ is returned by the Return ASL statement.
+ Must be a NULL-terminated ASL NameString
+
[edk2-devel] [PATCH 0/4] Add support for generating ACPI ThermalZones
Add APIs needed to create thermal zones dynamically. Does not add a generator for this as creating the TMP method generically may be difficult. Jeff Brasen (4): DynamicTablesPkg: Add ThermalZone CodeGen function DynamicTablesPkg: Add support for simple method invocation. DynamicTablesPkg: Add support to add Strings to package DynamicTablesPkg: Add Aml NameUnicodeString API .../Include/Library/AmlLib/AmlLib.h | 130 + .../Common/AmlLib/CodeGen/AmlCodeGen.c| 508 ++ 2 files changed, 638 insertions(+) -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106540): https://edk2.groups.io/g/devel/message/106540 Mute This Topic: https://groups.io/mt/99861336/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function
Add API to generate a ThermalZone object to AmlLib.
Signed-off-by: Jeff Brasen
Reviewed-by: Swatisri Kantamsetti
Reviewed-by: Ashish Singhal
---
.../Include/Library/AmlLib/AmlLib.h | 28 +
.../Common/AmlLib/CodeGen/AmlCodeGen.c| 116 ++
2 files changed, 144 insertions(+)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 9210c50915..d201ae9499 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1038,6 +1038,34 @@ AmlCodeGenDevice (
OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
);
+/** AML code generation for a ThermalZone object node.
+
+ AmlCodeGenThermalZone ("TZ00", ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+ThermalZone(TZ00) {}
+
+ @ingroup CodeGenApis
+
+ @param [in] NameString The new ThermalZone's name.
+ Must be a NULL-terminated ASL NameString
+ e.g.: "DEV0", "DV15.DEV0", etc.
+ The input string is copied.
+ @param [in] ParentNode If provided, set ParentNode as the parent
+ of the node created.
+ @param [out] NewObjectNode If success, contains the created node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenThermalZone (
+ IN CONST CHAR8 *NameString,
+ INAML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
+ );
+
/** AML code generation for a Scope object node.
AmlCodeGenScope ("_SB", ParentNode, NewObjectNode) is
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 0b223379fa..88537b7e2d 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1218,6 +1218,122 @@ error_handler1:
return Status;
}
+/** AML code generation for a ThermalZone object node.
+
+ AmlCodeGenThermalZone ("TZ00", ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+ThermalZone(TZ00) {}
+
+ @ingroup CodeGenApis
+
+ @param [in] NameString The new ThermalZone's name.
+ Must be a NULL-terminated ASL NameString
+ e.g.: "DEV0", "DV15.DEV0", etc.
+ The input string is copied.
+ @param [in] ParentNode If provided, set ParentNode as the parent
+ of the node created.
+ @param [out] NewObjectNode If success, contains the created node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenThermalZone (
+ IN CONST CHAR8 *NameString,
+ INAML_NODE_HANDLE ParentNode OPTIONAL,
+ OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE *ObjectNode;
+ AML_DATA_NODE*DataNode;
+ CHAR8*AmlNameString;
+ UINT32 AmlNameStringSize;
+
+ if ((NameString == NULL) ||
+ ((ParentNode == NULL) && (NewObjectNode == NULL)))
+ {
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+ }
+
+ ObjectNode= NULL;
+ DataNode = NULL;
+ AmlNameString = NULL;
+
+ Status = ConvertAslNameToAmlName (NameString, );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ Status = AmlGetNameStringSize (AmlNameString, );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+goto error_handler1;
+ }
+
+ Status = AmlCreateObjectNode (
+ AmlGetByteEncodingByOpCode (AML_EXT_OP, AML_EXT_THERMAL_ZONE_OP),
+ AmlNameStringSize + AmlComputePkgLengthWidth (AmlNameStringSize),
+
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+goto error_handler1;
+ }
+
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeNameString,
+ (UINT8 *)AmlNameString,
+ AmlNameStringSize,
+
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+goto error_handler2;
+ }
+
+ Status = AmlSetFixedArgument (
+ ObjectNode,
+ EAmlParseIndexTerm0,
+ (AML_NODE_HEADER *)DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+AmlDeleteTree ((AML_NODE_HEADER *)DataNode);
+goto error_handler2;
+ }
+
+ Status = LinkNode (
+ ObjectNode,
+
Re: [edk2-devel] [PATCH] RedfishPkg: Fix SortLib library class name typo.
Sure! https://github.com/tianocore/edk2-redfish-client/pull/43 Thanks, Mike On Tue, Jun 27, 2023 at 3:05 AM Chang, Abner wrote: > > [AMD Official Use Only - General] > > Hi Mike, could you please create a PR for this change? Then I can merge it > once the change passed CI. > > Thanks > Abner > > > -Original Message- > > From: Chang, Abner > > Sent: Wednesday, June 21, 2023 8:26 AM > > To: Mike Maslenkin ; devel@edk2.groups.io > > Cc: nick...@nvidia.com; ig...@ami.com > > Subject: RE: [PATCH] RedfishPkg: Fix SortLib library class name typo. > > > > Reviewed-by: Abner Chang > > > > > -Original Message- > > > From: Mike Maslenkin > > > Sent: Wednesday, June 21, 2023 5:30 AM > > > To: devel@edk2.groups.io > > > Cc: Chang, Abner ; nick...@nvidia.com; > > > ig...@ami.com; Mike Maslenkin > > > Subject: [PATCH] RedfishPkg: Fix SortLib library class name typo. > > > > > > Caution: This message originated from an External Source. Use proper > > caution > > > when opening attachments, clicking links, or responding. > > > > > > > > > BaseSortLib is the library instance name not the class name. > > > > > > Signed-off-by: Mike Maslenkin > > > Cc: Abner Chang > > > Cc: Nickle Wang > > > Cc: Igor Kulchytskyy > > > --- > > > RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf | 2 +- > > > RedfishPkg/RedfishLibs.dsc.inc| 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > index 5c1c7cffa650..6ff5dba75c30 100644 > > > --- a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > +++ b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > @@ -25,7 +25,7 @@ > > > > > > > > > [LibraryClasses] > > > > > >BaseLib > > > > > > - BaseSortLib > > > > > > + SortLib > > > > > >DebugLib > > > > > >MemoryAllocationLib > > > > > >UefiRuntimeServicesTableLib > > > > > > diff --git a/RedfishPkg/RedfishLibs.dsc.inc > > > b/RedfishPkg/RedfishLibs.dsc.inc > > > index 6e497899b556..03b7ef6f3289 100644 > > > --- a/RedfishPkg/RedfishLibs.dsc.inc > > > +++ b/RedfishPkg/RedfishLibs.dsc.inc > > > @@ -14,7 +14,7 @@ > > > !if $(REDFISH_ENABLE) == TRUE > > > > > >RestExLib|RedfishPkg/Library/DxeRestExLib/DxeRestExLib.inf > > > > > >Ucs2Utf8Lib|RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf > > > > > > - BaseSortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf > > > > > > + SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf > > > > > >RedfishCrtLib|RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.inf > > > > > >JsonLib|RedfishPkg/Library/JsonLib/JsonLib.inf > > > > > >RedfishLib|RedfishPkg/PrivateLibrary/RedfishLib/RedfishLib.inf > > > > > > -- > > > 2.32.0 (Apple Git-132) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106539): https://edk2.groups.io/g/devel/message/106539 Mute This Topic: https://groups.io/mt/99664815/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] NetworkPkg/HttpDxe: fix driver binding start issue.
Hi Nickle,
That makes sense. Thanks for the clarification.
Reviewed-by: Saloni Kasbekar
Thanks,
Saloni
-Original Message-
From: Nickle Wang
Sent: Wednesday, June 28, 2023 3:30 PM
To: Kasbekar, Saloni ; devel@edk2.groups.io
Cc: Maciej Rabeda ; Siyuan Fu
; Abner Chang ; Igor Kulchytskyy
; Nick Ramirez
Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding
start issue.
Hi Saloni,
Thanks for your review.
When uninstall fails, per UEFI specification, the protocol will be installed
again and will be visible to UEFI drivers.
Page 190, UEFI spec. 2.10:
"If any errors are generated while the
protocol interfaces are being uninstalled, then the protocols uninstalled prior
to the error will be reinstalled with the boot service
EFI_BOOT_SERVICES.InstallProtocolInterface() and the status code
EFI_INVALID_PARAMETER is returned."
In this case, if we do FreePool while driver still can locate
gEfiHttpServiceBindingProtocolGuid. Driver will access to the memory that is
released to system. Memory issue may happen.
Regards,
Nickle
> -Original Message-
> From: Kasbekar, Saloni
> Sent: Thursday, June 29, 2023 3:07 AM
> To: devel@edk2.groups.io; Nickle Wang
> Cc: Maciej Rabeda ; Siyuan Fu
> ; Abner Chang ; Igor
> Kulchytskyy ; Nick Ramirez
> Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
>
> External email: Use caution opening links or attachments
>
>
> Hi Nickle,
>
> We would want to do the FreePool even if the Uninstall fails (like in
> the case where we failed to install the multiple protocol interfaces
> and then went to ON_ERROR). Do you think it's better if we change it
> to -
>
> if (HttpService != NULL) {
> HttpCleanService (HttpService, UsingIpv6);
> Status = gBS->UninstallMultipleProtocolInterfaces (
> ,
> ,
> >ServiceBinding,
> NULL
> );
> if ((HttpService->Tcp4ChildHandle == NULL) &&
> (HttpService->Tcp6ChildHandle == NULL)) {
> FreePool (HttpService);
> }
> }
>
> Thanks,
> Saloni
>
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Nickle
> Wang via groups.io
> Sent: Tuesday, June 27, 2023 5:56 PM
> To: devel@edk2.groups.io; Nickle Wang
> Cc: Maciej Rabeda ; Siyuan Fu
> ; Abner Chang ; Igor
> Kulchytskyy ; Nick Ramirez
> Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
>
> May I know if someone can help to review this patch?
>
> Thanks,
> Nickle
>
> > -Original Message-
> > From: devel@edk2.groups.io On Behalf Of
> > Nickle Wang via groups.io
> > Sent: Friday, February 10, 2023 8:34 PM
> > To: devel@edk2.groups.io
> > Cc: Maciej Rabeda ; Siyuan Fu
> > ; Abner Chang ; Igor
> > Kulchytskyy ; Nick Ramirez
> > Subject: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > binding start issue.
> >
> > External email: Use caution opening links or attachments
> >
> >
> > When failure happens in HttpDxeStart, the error handling code
> > release the memory buffer but it does not uninstall HTTP service
> > bindnig protocol. As the result, application can still locate this
> > protocol and invoke service binding fucntions in released memory pool.
> >
> > Signed-off-by: Nickle Wang
> > Cc: Maciej Rabeda
> > Cc: Siyuan Fu
> > Cc: Abner Chang
> > Cc: Igor Kulchytskyy
> > Cc: Nick Ramirez
> > ---
> > NetworkPkg/HttpDxe/HttpDriver.c | 13 +++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/NetworkPkg/HttpDxe/HttpDriver.c
> > b/NetworkPkg/HttpDxe/HttpDriver.c index 5d918d3c4d..f6d1263cad
> > 100644
> > --- a/NetworkPkg/HttpDxe/HttpDriver.c
> > +++ b/NetworkPkg/HttpDxe/HttpDriver.c
> > @@ -3,6 +3,7 @@
> >
> >Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> >(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> > + Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> >
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -464,8 +465,16 @@ ON_ERROR:
> >
> >if (HttpService != NULL) {
> > HttpCleanService (HttpService, UsingIpv6);
> > -if ((HttpService->Tcp4ChildHandle == NULL) && (HttpService-
> > >Tcp6ChildHandle == NULL)) {
> > - FreePool (HttpService);
> > +Status = gBS->UninstallMultipleProtocolInterfaces (
> > +,
> > +,
> > +>ServiceBinding,
> > +NULL
> > +);
> > +if (!EFI_ERROR (Status)) {
> > + if ((HttpService->Tcp4ChildHandle == NULL) && (HttpService-
> > >Tcp6ChildHandle == NULL)) {
> > +FreePool (HttpService);
> > + }
> > }
> >}
> >
> > --
> > 2.39.1.windows.1
> >
> >
> >
> >
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106538):
[edk2-devel] [PATCH v2 1/1] OvmfPkg/README: Document Secure Boot
Add the new section for Secure Boot. Signed-off-by: Alexander Goncharov --- OvmfPkg/README | 40 1 file changed, 40 insertions(+) diff --git a/OvmfPkg/README b/OvmfPkg/README index 0a408abf01..a5b447dae3 100644 --- a/OvmfPkg/README +++ b/OvmfPkg/README @@ -120,6 +120,46 @@ $ OvmfPkg/build.sh -a X64 qemu -cdrom /path/to/disk-image.iso To build a 32-bit OVMF without debug messages using GCC 4.8: $ OvmfPkg/build.sh -a IA32 -b RELEASE -t GCC48 +=== Secure Boot === + +Secure Boot is a security feature that ensures only trusted and digitally +signed software is allowed to run during the boot process. This is achieved +by storing Secure Boot keys in UEFI Variables, as result it can be easily +bypassed by writing directly to the flash varstore. To avoid this situation, +it's necessary to make the varstore with SB keys read-only and/or provide an +isolated execution environment for flash access (such as SMM). + +* In order to support Secure Boot, OVMF must be built with the + "-D SECURE_BOOT_ENABLE" option. + +* By default, OVMF is not shipped with any SecureBoot keys installed. The user + need to install them with "Secure Boot Configuration" utility in the firmware + UI, or enroll the default UEFI keys using the OvmfPkg/EnrollDefaultKeys app. + + For the EnrollDefaultKeys application, the hypervisor is expected to add a + string entry to the "OEM Strings" (Type 11) SMBIOS table. The string should + have the following format: + +4e32566d-8e9e-4f52-81d3-5bb9715f9727: + + Such string can be generated with the following script, for example: + +sed \ + -e 's/^-BEGIN CERTIFICATE-$/4e32566d-8e9e-4f52-81d3-5bb9715f9727:/' \ + -e '/^-END CERTIFICATE-$/d' \ + PkKek1.pem \ +| tr -d '\n' \ +> PkKek1.oemstr + + - Using QEMU 5.2 or later, the SMBIOS type 11 field can be specified from a +file: + +-smbios type=11,path=PkKek1.oemstr \ + + - Using QEMU 5.1 or earlier, the string has to be passed as a value: + +-smbios type=11,value="$(< PkKek1.oemstr)" + === SMM support === Requirements: -- 2.41.0 -- Joursoir -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106537): https://edk2.groups.io/g/devel/message/106537 Mute This Topic: https://groups.io/mt/99860809/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/2] dp command without ACPI
Systems that do not boot with ACPI (system that use device tree for example) can not use the shell dp command. This patch adds this to the configuration table so that dp command can get this without the FPDT table. I am open to other ways for this to be passed if desired (Installed protocol, handler of the status code, etc) but wanted to post this to at least get thoughts on this. -Jeff Jeff Brasen (2): MdeModulePkg/DxeCorePerformanceLib: Install BPDT in config table ShellPkg/Dp: Allow dp command to work without ACPI .../DxeCorePerformanceLib/DxeCorePerformanceLib.c | 2 ++ ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 11 --- ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf| 1 + .../DpDynamicCommand/DpDynamicCommand.inf | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106534): https://edk2.groups.io/g/devel/message/106534 Mute This Topic: https://groups.io/mt/99859555/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/2] MdeModulePkg/DxeCorePerformanceLib: Install BPDT in config table
Install the performance table into the UEFI configuration table. This will allow the shell application to get this if the system is not using ACPI. Signed-off-by: Jeff Brasen --- .../Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c index ef14bc0738..e72b04794a 100644 --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c @@ -1403,6 +1403,8 @@ ReportFpdtRecordBuffer ( , sizeof (UINT64) ); + Status = gBS->InstallConfigurationTable (, (VOID *)BPDTAddr); + ASSERT_EFI_ERROR (Status); } // -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106535): https://edk2.groups.io/g/devel/message/106535 Mute This Topic: https://groups.io/mt/99859556/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/2] ShellPkg/Dp: Allow dp command to work without ACPI
If the system does not have ACPI setup use the configuration table
to get the performance info.
Signed-off-by: Jeff Brasen
---
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 11 ---
ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf| 1 +
.../DpDynamicCommand/DpDynamicCommand.inf | 1 +
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index 512a146da6..98c84d2ef9 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -129,17 +129,22 @@ EFI_STATUS
GetBootPerformanceTable (
)
{
+ EFI_STATUS Status;
FIRMWARE_PERFORMANCE_TABLE *FirmwarePerformanceTable;
FirmwarePerformanceTable = (FIRMWARE_PERFORMANCE_TABLE
*)EfiLocateFirstAcpiTable (
EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
);
if (FirmwarePerformanceTable == NULL) {
-ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_FPDT_FAIL),
mDpHiiHandle);
-return EFI_NOT_FOUND;
+Status = EfiGetSystemConfigurationTable
(, (VOID **));
+if (EFI_ERROR (Status)) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_FPDT_FAIL),
mDpHiiHandle);
+ return EFI_NOT_FOUND;
+}
+ } else {
+mBootPerformanceTable = (UINT8
*)(UINTN)FirmwarePerformanceTable->BootPointerRecord.BootPerformanceTablePointer;
}
- mBootPerformanceTable = (UINT8
*)(UINTN)FirmwarePerformanceTable->BootPointerRecord.BootPerformanceTablePointer;
mBootPerformanceTableSize = ((BOOT_PERFORMANCE_TABLE
*)mBootPerformanceTable)->Header.Length;
return EFI_SUCCESS;
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
index 4a58286b8c..d9e1c23a1e 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
@@ -54,6 +54,7 @@
[Guids]
gPerformanceProtocolGuid## CONSUMES ##
SystemTable
+ gEdkiiFpdtExtendedFirmwarePerformanceGuid ## CONSUMES ##
SystemTable
[Protocols]
gEfiLoadedImageProtocolGuid ## CONSUMES
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
index 013bdbd4a0..2723fee706 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
@@ -55,6 +55,7 @@
[Guids]
gPerformanceProtocolGuid## CONSUMES ##
SystemTable
+ gEdkiiFpdtExtendedFirmwarePerformanceGuid ## CONSUMES ##
SystemTable
[Protocols]
gEfiLoadedImageProtocolGuid ## CONSUMES
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106533): https://edk2.groups.io/g/devel/message/106533
Mute This Topic: https://groups.io/mt/99859554/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding
If the platform does not support any PCIe devices using the library
method allow devices to connect to host bridge via driver binding.
Signed-off-by: Jeff Brasen
---
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 649 ++
.../Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
.../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 13 +
.../Pci/PciHostBridgeDxe/PciRootBridgeIo.c| 24 +
MdeModulePkg/MdeModulePkg.dec | 4 +
5 files changed, 562 insertions(+), 129 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index d573e532ba..506c6660ae 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -422,167 +422,320 @@ IoMmuProtocolCallback (
}
/**
+ PCI Root Bridge Memory setup.
- Entry point of this driver.
+ @param RootBridgeRoot Bridge instance.
- @param ImageHandle Image handle of this driver.
- @param SystemTable Pointer to standard EFI system table.
-
- @retval EFI_SUCCESS Succeed.
- @retval EFI_DEVICE_ERROR Fail to install PCI_ROOT_BRIDGE_IO protocol.
+ @retval EFI_SUCCESS Memory was setup correctly
+ @retval othersError in setup
**/
EFI_STATUS
EFIAPI
-InitializePciHostBridge (
- IN EFI_HANDLEImageHandle,
- IN EFI_SYSTEM_TABLE *SystemTable
+PciRootBridgeMemorySetup (
+ IN PCI_ROOT_BRIDGE *RootBridge
)
{
EFI_STATUSStatus;
- PCI_HOST_BRIDGE_INSTANCE *HostBridge;
- PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
- PCI_ROOT_BRIDGE *RootBridges;
- UINTN RootBridgeCount;
- UINTN Index;
+ UINT64HostAddress;
PCI_ROOT_BRIDGE_APERTURE *MemApertures[4];
UINTN MemApertureIndex;
- BOOLEAN ResourceAssigned;
- LIST_ENTRY*Link;
- UINT64HostAddress;
- RootBridges = PciHostBridgeGetRootBridges ();
- if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
-return EFI_UNSUPPORTED;
- }
-
- Status = gBS->LocateProtocol (, NULL, (VOID
**));
- ASSERT_EFI_ERROR (Status);
-
- //
- // Most systems in the world including complex servers have only one Host
Bridge.
- //
- HostBridge = AllocateZeroPool (sizeof (PCI_HOST_BRIDGE_INSTANCE));
- ASSERT (HostBridge != NULL);
-
- HostBridge->Signature= PCI_HOST_BRIDGE_SIGNATURE;
- HostBridge->CanRestarted = TRUE;
- InitializeListHead (>RootBridges);
- ResourceAssigned = FALSE;
-
- //
- // Create Root Bridge Device Handle in this Host Bridge
- //
- for (Index = 0; Index < RootBridgeCount; Index++) {
+ if (RootBridge->Io.Base <= RootBridge->Io.Limit) {
//
-// Create Root Bridge Handle Instance
+// Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
+// For GCD resource manipulation, we need to use host address.
//
-RootBridge = CreateRootBridge ([Index]);
-ASSERT (RootBridge != NULL);
-if (RootBridge == NULL) {
- continue;
+HostAddress = TO_HOST_ADDRESS (
+RootBridge->Io.Base,
+RootBridge->Io.Translation
+);
+
+Status = AddIoSpace (
+ HostAddress,
+ RootBridge->Io.Limit - RootBridge->Io.Base + 1
+ );
+ASSERT_EFI_ERROR (Status);
+if (EFI_ERROR (Status)) {
+ return Status;
}
-//
-// Make sure all root bridges share the same ResourceAssigned value.
-//
-if (Index == 0) {
- ResourceAssigned = RootBridges[Index].ResourceAssigned;
-} else {
- ASSERT (ResourceAssigned == RootBridges[Index].ResourceAssigned);
+if (RootBridge->ResourceAssigned) {
+ Status = gDS->AllocateIoSpace (
+ EfiGcdAllocateAddress,
+ EfiGcdIoTypeIo,
+ 0,
+ RootBridge->Io.Limit - RootBridge->Io.Base + 1,
+ ,
+ gImageHandle,
+ NULL
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+return Status;
+ }
}
+ }
+
+ //
+ // Add all the Mem/PMem aperture to GCD
+ // Mem/PMem shouldn't overlap with each other
+ // Root bridge which needs to combine MEM and PMEM should only report
+ // the MEM aperture in Mem
+ //
+ MemApertures[0] = >Mem;
+ MemApertures[1] = >MemAbove4G;
+ MemApertures[2] = >PMem;
+ MemApertures[3] = >PMemAbove4G;
-if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) {
+ for (MemApertureIndex = 0; MemApertureIndex < ARRAY_SIZE (MemApertures);
MemApertureIndex++) {
+if (MemApertures[MemApertureIndex]->Base <=
MemApertures[MemApertureIndex]->Limit) {
[edk2-devel] [PATCH] ArmPkg/ArmPsciMpServices Add EFI_NOT_READY return
Add EFI_NOT_READY return if the CPU can not be enabled if the
processor is already on.
This can occur in normal use if the CPU is still being turned off from
a previous call when this is called again.
Signed-off-by: Jeff Brasen
---
ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
index f822a9877c..e7f4223513 100644
--- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
+++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
@@ -103,7 +103,9 @@ DispatchCpu (
ArmCallSmc ();
- if (Args.Arg0 != ARM_SMC_PSCI_RET_SUCCESS) {
+ if (Args.Arg0 == ARM_SMC_PSCI_RET_ALREADY_ON) {
+Status = EFI_NOT_READY;
+ } else if (Args.Arg0 != ARM_SMC_PSCI_RET_SUCCESS) {
DEBUG ((DEBUG_ERROR, "PSCI_CPU_ON call failed: %d\n", Args.Arg0));
Status = EFI_DEVICE_ERROR;
}
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106531): https://edk2.groups.io/g/devel/message/106531
Mute This Topic: https://groups.io/mt/99859167/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
This patch seems to have some discrepancy between the title and content :)
Can you please break this patch into 2, so that MdePkg change and
SecurityPkg can be their
own commit?
Thanks,
Kun
On 5/9/2023 12:40 AM, PierreGondois wrote:
From: Pierre Gondois
In order to use PcdCpuRngSupportedAlgorithm in the MdePkg in a
following patch and to avoid making the MdePkg dependent on another
package, move PcdCpuRngSupportedAlgorithm to the MdePkg.
As the Pcf is only used for AARCH64, place it in an AARCH64
specific sections.
Signed-off-by: Pierre Gondois
---
MdePkg/MdePkg.dec | 5 +
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 4 ++--
SecurityPkg/SecurityPkg.dec | 2 --
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index d6c4179b2a48..0ecfad5795e4 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2357,6 +2357,11 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
# @Prompt IPMI KCS Interface I/O Base Address
gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsIoBaseAddress|0xca2|UINT16|0x0031
+[PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
+ ## GUID identifying the Rng algorithm implemented by CPU instruction.
+ # @Prompt CPU Rng algorithm's GUID.
+
gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0032
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## This value is used to set the base address of PCI express hierarchy.
# @Prompt PCI Express Base Address.
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index c8e0ee4ae5d9..d6c2d30195bf 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -79,8 +79,8 @@ [Guids]
[Protocols]
gEfiRngProtocolGuid## PRODUCES
-[Pcd]
- gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
+[Pcd.AARCH64]
+ gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
[Depex]
TRUE
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 0a8042d63fe1..6bb02d58bdf0 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -325,8 +325,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass|0x0303100A|UINT32|0x00010030
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationFail|0x0303100B|UINT32|0x00010031
- gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00010032
-
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## Image verification policy for OptionRom. Only following values are
valid:
# NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has
been removed.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106530): https://edk2.groups.io/g/devel/message/106530
Mute This Topic: https://groups.io/mt/98779039/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
Hi Pierre,
Do we really need this removal of ASSERT? I tried to use the real
ArmTrngLib with this patch
and it seems to work fine with a TFA that does not support TRNG interfaces.
I think it would be valuable to keep the ASSERT to indicate there might
be an integration error?
Please let me know if I missed anything.
Regards,
Kun
On 5/9/2023 12:40 AM, PierreGondois wrote:
From: Pierre Gondois
Remove ASSERTs to allow RngDxe probing the Null implementation
of the TrngLib.
Signed-off-by: Pierre Gondois
---
MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c | 4
1 file changed, 4 deletions(-)
diff --git a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
index 316d78bf5e83..0ea9aafa59f1 100644
--- a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
+++ b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
@@ -41,7 +41,6 @@ GetArmTrngVersion (
OUT UINT16 *MinorRevision
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
@@ -67,7 +66,6 @@ GetArmTrngUuid (
OUT GUID *Guid
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
@@ -83,7 +81,6 @@ GetArmTrngMaxSupportedEntropyBits (
VOID
)
{
- ASSERT (FALSE);
return 0;
}
@@ -116,6 +113,5 @@ GetArmTrngEntropy (
OUT UINT8 *Buffer
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106529): https://edk2.groups.io/g/devel/message/106529
Mute This Topic: https://groups.io/mt/98779038/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64
Hi Pierre/Sami, Thanks for the information. I tried the patch set you referred, it seems to have resolved the zero GUID handling issue! So I am okay to drop the commit here: https://edk2.groups.io/g/devel/message/106484 I also have a few questions on the patch set beyond functionality, we can discuss it there. However, this specific patch is still needed to fix the access violation issue (thanks for the reviewed-by tag, Sami): https://edk2.groups.io/g/devel/message/106485 I can send a v2 to reflect the feedback above. Thanks, Kun On 6/28/2023 11:43 PM, Pierre Gondois wrote: Hello Kun, Thanks for the patch-set, there is another patch-set that also aims to fix the logic at: https://edk2.groups.io/g/devel/message/104341 but I haven't got feedback so far. Would it be possible to try it out to see if it also solves your issue ? Regards, Pierre On 6/28/23 22:33, Kun Qin wrote: REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491 On an ARM system that does not support firmware TRNG, the current logic from RngDxe will cause the system to assert at the below line: `ASSERT (Index != mAvailableAlgoArrayCount);` The reason seems to be: 1. When initializing the number of `mAvailableAlgoArrayCount`, the logic will only treat the zero guid of "PcdCpuRngSupportedAlgorithm" as a warning and still increment the counter because "RngGetBytes" might still succeed: https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c#L51C3-L51C3. 2. This will cause the main entry to publish the RNG protocol and accept further usage. 3. However, during usage, the zero guid is always filtered out: https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c#L91. Thus, this will cause the system to always not able to find the algorithm and fail the boot with an assert. The suggestion is to at least make the logic of initializing "mAvailableAlgoArrayCount" consistent and filtering algorithm consistent. In addition, the usage of `mAvailableAlgoArray` will always trigger a data abortion error, which is caused by buffer allocated is `RNG_AVAILABLE_ALGO_MAX` number of bytes, which should be `RNG_AVAILABLE_ALGO_MAX` nubmer of EFI_RNG_ALGORITHM. This patch fixed the 2 issues above. The change is verified on QEMU virtual platform and proprietary physical platform. Patch v1 branch: https://github.com/kuqin12/edk2/tree/fix_rng_edk2_v1 Cc: Jiewen Yao Cc: Jian J Wang Cc: Sami Mujawar Cc: Pierre Gondois Kun Qin (2): SecurityPkg: RngDxe: Unify handling of zero guid SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 9 + SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106528): https://edk2.groups.io/g/devel/message/106528 Mute This Topic: https://groups.io/mt/99839045/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] 回复: [PATCH v1 00/24] Update Edk2-pytools to latest versions
Liming, In general, due to feedback on this list we have tried to require explicit opt-in for all the stuart based validation tools. So could that be done yes, but it goes against our current consistent approach of requiring opt-in and then would put the burden on a platform/package owner to opt-out if they didn't want this new functionality. Thanks Sean On 6/26/2023 5:59 PM, gaoliming via groups.io wrote: Joey: So, this change is required for all package YAML. If yes, can PrEval be auto set to the package dsc if only one DSC is in Package directory? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106526): https://edk2.groups.io/g/devel/message/106526 Mute This Topic: https://groups.io/mt/99801431/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 v2 2/3] Platform/QemuSbsa: add dynamic PcdSmmuBase
Store Smmu base address in variable in case it would be needed in more than one place. Signed-off-by: Marcin Juszkiewicz --- Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 1 + Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 1 + 2 files changed, 2 insertions(+) diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec index ff2a4721a131..aab2894e6455 100644 --- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec +++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec @@ -73,3 +73,4 @@ [PcdsDynamic.common] # ARM Generic Interrupt Controller ITS gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0|UINT64|0x0120 + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase|0|UINT64|0x0121 diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc index 4ae2479628b6..be406144c242 100644 --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc @@ -525,6 +525,7 @@ [PcdsDynamicDefault.common] # GIC ITS gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0 + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase|0x6005 # # Set video resolution for boot options -- 2.41.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106524): https://edk2.groups.io/g/devel/message/106524 Mute This Topic: https://groups.io/mt/99854680/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 v2 0/3] Platform/QemuSbsa: add GIC ITS
SBSA Reference Platform can have GIC ITS present. And when it has then we can have complex PCI Express setup (and some other things). First patch adds support for GIC ITS. Address is read from TF-A via SMC call. IORT is generated, MADT has ITS information. Linux boots and sees GIC ITS as expected. SMMU information is also provided in IORT and used. Second patch introduces PcdSmmuBase variable to avoid using magic number in IORT generation. Third patch takes care of system where GIC ITS is not present (like QEMU 8.0). If GIC ITS address is not set then there is no mention of it in MADT and IORT tables, Linux boots. Changes since v2: - IORT is generated in C - no ITS == no ITS node in IORT - introduced PcdSmmuBase Marcin Juszkiewicz (2): Platform/QemuSbsa: add dynamic PcdSmmuBase Platform/SbsaQemu: handle systems without GIC ITS Shashi Mallela (1): Platform/SbsaQemu: add GIC ITS support Silicon/Qemu/SbsaQemu/SbsaQemu.dec| 4 + Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 4 + .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 + .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 2 + .../SbsaQemuPlatformDxe.inf | 1 + .../Include/IndustryStandard/SbsaQemuAcpi.h | 11 + .../Include/IndustryStandard/SbsaQemuSmc.h| 1 + .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 216 +- .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 10 + 9 files changed, 249 insertions(+), 1 deletion(-) -- 2.41.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106522): https://edk2.groups.io/g/devel/message/106522 Mute This Topic: https://groups.io/mt/99854678/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 v2 1/3] Platform/SbsaQemu: add GIC ITS support
From: Shashi Mallela
SBSA Reference Platform has GIC ITS support. Let make use of it.
Base address is read from TF-A via SMC call.
GIC ITS allows us to have complex PCI Express setups.
Co-authored-by: Marcin Juszkiewicz
Signed-off-by: Shashi Mallela
Signed-off-by: Marcin Juszkiewicz
---
Silicon/Qemu/SbsaQemu/SbsaQemu.dec| 3 +
Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 3 +
.../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 2 +
.../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 1 +
.../SbsaQemuPlatformDxe.inf | 1 +
.../Include/IndustryStandard/SbsaQemuAcpi.h | 11 ++
.../Include/IndustryStandard/SbsaQemuSmc.h| 1 +
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 12 +-
.../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 10 ++
Silicon/Qemu/SbsaQemu/AcpiTables/Iort.aslc| 135 ++
10 files changed, 178 insertions(+), 1 deletion(-)
create mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Iort.aslc
diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
index 5182978cf56d..ff2a4721a131 100644
--- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
@@ -70,3 +70,6 @@ [PcdsDynamic.common]
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor|0x0|UINT32|0x011E
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor|0x0|UINT32|0x011F
+
+ # ARM Generic Interrupt Controller ITS
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0|UINT64|0x0120
diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index b88729ad8ad6..4ae2479628b6 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -523,6 +523,9 @@ [PcdsDynamicDefault.common]
gArmTokenSpaceGuid.PcdGicDistributorBase|0x4006
gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x4008
+ # GIC ITS
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase|0
+
#
# Set video resolution for boot options
# PlatformDxe can set the former at runtime.
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 0501c670d565..554c5e4b6f9e 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -22,6 +22,7 @@ [Sources]
Gtdt.aslc
Mcfg.aslc
Spcr.aslc
+ Iort.aslc
[Packages]
ArmPlatformPkg/ArmPlatformPkg.dec
@@ -75,3 +76,4 @@ [FixedPcd]
[Pcd]
gArmTokenSpaceGuid.PcdGicDistributorBase
gArmTokenSpaceGuid.PcdGicRedistributorsBase
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index c1c33788567d..3ec7ffd8dd5c 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -48,6 +48,7 @@ [Pcd]
gArmTokenSpaceGuid.PcdGicDistributorBase
gArmTokenSpaceGuid.PcdGicRedistributorsBase
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
[Depex]
gEfiAcpiTableProtocolGuid ## CONSUMES
diff --git
a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
index 545794a8c7ff..0e3b11d60426 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
@@ -43,6 +43,7 @@ [Pcd]
gArmTokenSpaceGuid.PcdGicDistributorBase
gArmTokenSpaceGuid.PcdGicRedistributorsBase
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
[Depex]
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
index 853b81b34df5..983d17f6fa50 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -27,6 +27,7 @@
#define SBSAQEMU_MADT_GIC_HBASE 0x2c01
#define SBSAQEMU_MADT_GIC_PMU_IRQ23
#define SBSAQEMU_MADT_GICR_SIZE 0x400
+#define SBSAQEMU_MADT_GITS_SIZE 0x2
// Macro for MADT GIC Redistributor Structure
#define SBSAQEMU_MADT_GICR_INIT() {
\
@@ -37,6 +38,16 @@
SBSAQEMU_MADT_GICR_SIZE /* DiscoveryRangeLength */
\
}
+// Macro for MADT GIC ITS Structure
+#define SBSAQEMU_MADT_GIC_ITS_INIT(GicItsId) {
\
+ EFI_ACPI_6_5_GIC_ITS, /* Type */
\
+ sizeof (EFI_ACPI_6_5_GIC_ITS_STRUCTURE), /* Length */
\
+ EFI_ACPI_RESERVED_WORD, /* Reserved */
\
+ GicItsId, /* GicItsId */
[edk2-devel] [PATCH edk2-platforms v2 3/3] Platform/SbsaQemu: handle systems without GIC ITS
If firmware is used with QEMU 8.0 or older then there will be no GIC ITS
support.
In such case we would not add information about it into MADT and IORT
tables.
Signed-off-by: Marcin Juszkiewicz
---
.../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 -
.../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf | 1 +
.../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 226 +-
Silicon/Qemu/SbsaQemu/AcpiTables/Iort.aslc| 135 ---
4 files changed, 216 insertions(+), 147 deletions(-)
delete mode 100644 Silicon/Qemu/SbsaQemu/AcpiTables/Iort.aslc
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 554c5e4b6f9e..97021f7971c7 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -22,7 +22,6 @@ [Sources]
Gtdt.aslc
Mcfg.aslc
Spcr.aslc
- Iort.aslc
[Packages]
ArmPlatformPkg/ArmPlatformPkg.dec
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 3ec7ffd8dd5c..14d760b36400 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -49,6 +49,7 @@ [Pcd]
gArmTokenSpaceGuid.PcdGicDistributorBase
gArmTokenSpaceGuid.PcdGicRedistributorsBase
gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdGicItsBase
+ gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase
[Depex]
gEfiAcpiTableProtocolGuid ## CONSUMES
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 961482269678..17d47f56d611 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -8,6 +8,7 @@
**/
#include
#include
+#include
#include
#include
#include
@@ -21,6 +22,34 @@
#include
#include
+#pragma pack(1)
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_ITS_NODENode;
+ UINT32Identifiers;
+} SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
+
+typedef struct
+{
+ EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
+ EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE SmmuIdMap;
+} SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
+
+typedef struct
+{
+ EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
+ EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE RcIdMap;
+} SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort;
+ SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE ItsNode;
+ SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
+ SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
+} SBSA_IO_REMAPPING_STRUCTURE;
+
+#pragma pack ()
+
/*
* A Function to Compute the ACPI Table Checksum
*/
@@ -40,6 +69,169 @@ AcpiPlatformChecksum (
Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);
}
+/*
+ * A function that add the IORT ACPI table.
+ IN EFI_ACPI_COMMON_HEADER*CurrentTable
+ */
+EFI_STATUS
+AddIortTable (
+ IN EFI_ACPI_TABLE_PROTOCOL *AcpiTable
+ )
+{
+ EFI_STATUSStatus;
+ UINTN TableHandle;
+ UINT32TableSize;
+ EFI_PHYSICAL_ADDRESS PageAddress;
+ UINT8 *New;
+ UINTN GicItsBase;
+
+ // Initialize IORT ACPI Header
+ EFI_ACPI_6_0_IO_REMAPPING_TABLE Header = {
+SBSAQEMU_ACPI_HEADER(EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,
+ SBSA_IO_REMAPPING_STRUCTURE,
+ EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00),
+2,
+sizeof(EFI_ACPI_6_0_IO_REMAPPING_TABLE),// NodeOffset
+0 };
+
+ // Initialize SMMU3 Structure
+ SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE Smmu3 = {
+{
+ {
+EFI_ACPI_IORT_TYPE_SMMUv3,
+sizeof (SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE),
+2, // Revision
+0, // Reserved
+1, // NumIdMapping
+OFFSET_OF (SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE, SmmuIdMap) //
IdReference
+ },
+ PcdGet64 (PcdSmmuBase), // Base address
+ EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE, // Flags
+ 0, // Reserved
+ 0, // VATOS address
+ EFI_ACPI_IORT_SMMUv3_MODEL_GENERIC, // SMMUv3 Model
+ 0, // Event
+ 0, // Pri
+ 0, // Gerror
+ 0, // Sync
+ 0, // Proximity domain
+ 1 // DevIDMappingIndex
+},
+ {
+0x, // InputBase
+0x, // NumIds
+0x, // OutputBase
+OFFSET_OF (SBSA_IO_REMAPPING_STRUCTURE, ItsNode), // OutputReference
+0 // Flags
+ }
+ };
+
+ SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE Rc = {
+{
+ {
+EFI_ACPI_IORT_TYPE_ROOT_COMPLEX, // Type
+sizeof (SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE), // Length
+0, // Revision
+0, // Reserved
+1, //
[edk2-devel] [PATCH 4/4] ArmPkg: Add Function Headers to MMU Logic
From: Taylor Beebe Much of the MMU logic was written without function headers. This patch adds function headers where absent and updates function headers which do not match the EDK2 standard. Cc: Leif Lindholm Cc: Ard Biesheuvel Signed-off-by: Taylor Beebe --- ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 92 - ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 86 --- 2 files changed, 169 insertions(+), 9 deletions(-) diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c index d9d386dbed6b..e14eb47ce4c6 100644 --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c @@ -18,6 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define MIN_T0SZ16 #define BITS_PER_LEVEL 9 +/** + Parses T0SZ to determine the level and number of entries at the root + of the translation table. + + @param T0SZ The T0SZ value to be parsed. + @param RootTableLevel The level of the root table. + @param RootTableEntryCount The number of entries in the root table. +**/ STATIC VOID GetRootTranslationTableInfo ( @@ -30,6 +38,13 @@ GetRootTranslationTableInfo ( *RootTableEntryCount = TT_ENTRY_COUNT >> (T0SZ - MIN_T0SZ) % BITS_PER_LEVEL; } +/** + Converts ARM translation table attributes to GCD attributes. + + @param PageAttributes The translation table attributes to be converted. + + @retval The analogous GCD attributes. +**/ STATIC UINT64 PageAttributeToGcdAttribute ( @@ -100,6 +115,14 @@ RegionAttributeToGcdAttribute ( return PageAttributeToGcdAttribute (PageAttributes); } +/** + Retrieves the attribute of the first page entry in the translation table. + + @param[in] FirstLevelTableAddress The base address of the translation table. + @param[in] TableLevel The current level being traversed. + + @retval The attributes of the first page entry found, or INVALID_ENTRY. +**/ STATIC UINT64 GetFirstPageAttribute ( @@ -126,6 +149,19 @@ GetFirstPageAttribute ( } } +/** + This function recursively traverses the translation table heirarchy to + synchronise the GCD with the translation table. + + @param[in]TableAddressThe address of the table being processed. + @param[in]EntryCount The number of entries in the current level of the table. + @param[in]TableLevel The current level of the memory table being processed. + @param[in]BaseAddress The starting address of the region. + @param[in, out] PrevEntryAttribute The attributes of the previous region. + @param[in, out] StartGcdRegion The start of the GCD region. + + @retval The address at the end of the last region processed. +**/ STATIC UINT64 GetNextEntryAttribute ( @@ -220,6 +256,15 @@ GetNextEntryAttribute ( return BaseAddress + (EntryCount * TT_ADDRESS_AT_LEVEL (TableLevel)); } +/** + Sync the GCD memory space attributes with the translation table. + + @param[in] CpuProtocol The CPU architectural protocol instance. + + @retval EFI_SUCCESS The GCD memory space attributes are synced with + the MMU page table. + @retval Others The return value of GetMemorySpaceMap(). +**/ EFI_STATUS SyncCacheConfig ( IN EFI_CPU_ARCH_PROTOCOL *CpuProtocol @@ -298,6 +343,13 @@ SyncCacheConfig ( return EFI_SUCCESS; } +/** + Convert EFI memory attributes to ARM translation table attributes. + + @param[in] EfiAttributes EFI memory attributes. + + @retval The analogous translation table attributes. +**/ UINT64 EfiAttributeToArmAttribute ( IN UINT64 EfiAttributes @@ -345,8 +397,25 @@ EfiAttributeToArmAttribute ( return ArmAttributes; } -// This function will recursively go down the page table to find the first block address linked to 'BaseAddress'. -// And then the function will identify the size of the region that has the same page table attribute. +/** + This function returns the attributes of the memory region containing the + specified address. + + RegionLength and RegionAttributes are only valid if the result is EFI_SUCCESS. + + @param[in]TranslationTable The translation table base address. + @param[in]TableLevelThe level of the translation table. + @param[in]LastBlockEntryThe last block address of the table level. + @param[in, out] BaseAddress The base address of the memory region. + @param[out] RegionLength The length of the memory region. + @param[out] RegionAttributes The attributes of the memory region. + + @retval EFI_SUCCESS The attributes of the memory region were + returned successfully. + @retval EFI_NOT_FOUND The memory region was not found. + @retval EFI_NO_MAPPING The translation table entry associated with + BaseAddress is invalid. +**/ EFI_STATUS GetMemoryRegionRec ( IN UINT64 *TranslationTable, @@ -423,6
[edk2-devel] [PATCH 3/4] ArmPkg: Fix Unsafe ASSERTs in MMU Logic
From: Taylor Beebe
There are ASSERTs present in the MMU logic to ensure various
functions return successfully, but these ASSERTs may be ignored
on release builds causing unsafe behavior. This patch updates
the logic to handle unexpected return values and branch safely.
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Signed-off-by: Taylor Beebe
---
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 21 -
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 36 -
2 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index 0d3bc2809682..d9d386dbed6b 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -148,10 +148,12 @@ GetNextEntryAttribute (
// Get the memory space map from GCD
MemorySpaceMap = NULL;
Status = gDS->GetMemorySpaceMap (,
);
- ASSERT_EFI_ERROR (Status);
- // We cannot get more than 3-level page table
- ASSERT (TableLevel <= 3);
+ if (EFI_ERROR (Status) || (TableLevel > 3)) {
+ASSERT_EFI_ERROR (Status);
+ASSERT (TableLevel <= 3);
+return 0;
+ }
// While the top level table might not contain TT_ENTRY_COUNT entries;
// the subsequent ones should be filled up
@@ -243,7 +245,11 @@ SyncCacheConfig (
//
MemorySpaceMap = NULL;
Status = gDS->GetMemorySpaceMap (,
);
- ASSERT_EFI_ERROR (Status);
+
+ if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+ }
// The GCD implementation maintains its own copy of the state of memory
space attributes. GCD needs
// to know what the initial memory space attributes are. The CPU Arch.
Protocol does not provide a
@@ -277,7 +283,7 @@ SyncCacheConfig (
);
// Update GCD with the last region if valid
- if (PageAttribute != INVALID_ENTRY) {
+ if ((PageAttribute != INVALID_ENTRY) && (EndAddressGcdRegion >
BaseAddressGcdRegion)) {
SetGcdMemorySpaceAttributes (
MemorySpaceMap,
NumberOfDescriptors,
@@ -430,7 +436,10 @@ GetMemoryRegion (
UINTN EntryCount;
UINTN T0SZ;
- ASSERT ((BaseAddress != NULL) && (RegionLength != NULL) && (RegionAttributes
!= NULL));
+ if ((BaseAddress == NULL) || (RegionLength == NULL) || (RegionAttributes ==
NULL)) {
+ASSERT ((BaseAddress != NULL) && (RegionLength != NULL) &&
(RegionAttributes != NULL));
+return EFI_INVALID_PARAMETER;
+ }
TranslationTable = ArmGetTTBR0BaseAddress ();
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 268c0bf3f9ae..5a2f36d06086 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -217,7 +217,10 @@ SyncCacheConfigPage (
} else if (PageAttributes != NextPageAttributes) {
// Convert Section Attributes into GCD Attributes
Status = PageToGcdAttributes (NextPageAttributes, );
-ASSERT_EFI_ERROR (Status);
+if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ GcdAttributes = 0;
+}
// update GCD with these changes (this will recurse into our own
CpuSetMemoryAttributes below which is OK)
SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors,
*NextRegionBase, *NextRegionLength, GcdAttributes);
@@ -230,7 +233,10 @@ SyncCacheConfigPage (
} else if (NextPageAttributes != 0) {
// Convert Page Attributes into GCD Attributes
Status = PageToGcdAttributes (NextPageAttributes, );
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+GcdAttributes = 0;
+ }
// update GCD with these changes (this will recurse into our own
CpuSetMemoryAttributes below which is OK)
SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors,
*NextRegionBase, *NextRegionLength, GcdAttributes);
@@ -278,7 +284,12 @@ SyncCacheConfig (
//
MemorySpaceMap = NULL;
Status = gDS->GetMemorySpaceMap (,
);
- ASSERT_EFI_ERROR (Status);
+
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "SyncCacheConfig - GetMemorySpaceMap() failed!
Status: %r\n", Status));
+ASSERT_EFI_ERROR (Status);
+return Status;
+ }
// The GCD implementation maintains its own copy of the state of memory
space attributes. GCD needs
// to know what the initial memory space attributes are. The CPU Arch.
Protocol does not provide a
@@ -307,7 +318,12 @@ SyncCacheConfig (
} else if (SectionAttributes != NextSectionAttributes) {
// Convert Section Attributes into GCD Attributes
Status = SectionToGcdAttributes (NextSectionAttributes,
);
-ASSERT_EFI_ERROR (Status);
+
+if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "SyncCacheConfig - SectionToGcdAttributes()
failed! Status: %r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ GcdAttributes = 0;
+}
// update GCD with these changes
[edk2-devel] [PATCH 2/4] ArmPkg: Update GetMemoryRegion() to Handle No mapping
From: Taylor Beebe
This patch updates the GetMemoryRegion() function to handle the case
where there is no mapping for the requested address.
The original logic for the ARM would hit an ASSERT after
GetMemoryRegionPage() returned EFI_SUCCESS but did not update The
RegionLength parameter.
The original logic for the AARCH64 would never initialize the
RegionLength parameter to zero and return EFI_SUCCESS after
traversing an unknown number of pages.
To fix this, the logic for both architecture has updated to
return EFI_NO_MAPPING if the BaseAddress being checked is unmapped.
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Signed-off-by: Taylor Beebe
---
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 30 +++--
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 65 +++--
2 files changed, 60 insertions(+), 35 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index 1d02e41e18d8..0d3bc2809682 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -380,10 +380,10 @@ GetMemoryRegionRec (
RegionAttributes
);
-// In case of 'Success', it means the end of the block region has been
found into the upper
-// level translation table
-if (!EFI_ERROR (Status)) {
- return EFI_SUCCESS;
+// EFI_SUCCESS: The end of the end of the region was found.
+// EFI_NO_MAPPING: The translation entry associated with BaseAddress is
invalid.
+if (Status != EFI_NOT_FOUND) {
+ return Status;
}
// Now we processed the table move to the next entry
@@ -395,12 +395,13 @@ GetMemoryRegionRec (
*RegionLength = 0;
*RegionAttributes = *BlockEntry & TT_ATTRIBUTES_MASK;
} else {
-// We have an 'Invalid' entry
-return EFI_UNSUPPORTED;
+return EFI_NO_MAPPING;
}
while (BlockEntry <= LastBlockEntry) {
-if ((*BlockEntry & TT_ATTRIBUTES_MASK) == *RegionAttributes) {
+if (((*BlockEntry & TT_TYPE_MASK) == BlockEntryType) &&
+((*BlockEntry & TT_ATTRIBUTES_MASK) == *RegionAttributes))
+{
*RegionLength = *RegionLength + TT_BLOCK_ENTRY_SIZE_AT_LEVEL
(TableLevel);
} else {
// In case we have found the end of the region we return success
@@ -412,7 +413,7 @@ GetMemoryRegionRec (
// If we have reached the end of the TranslationTable and we have not found
the end of the region then
// we return EFI_NOT_FOUND.
- // The caller will continue to look for the memory region at its level
+ // The caller will continue to look for the memory region at its level.
return EFI_NOT_FOUND;
}
@@ -433,6 +434,11 @@ GetMemoryRegion (
TranslationTable = ArmGetTTBR0BaseAddress ();
+ // Initialize the output parameters. These paramaters are only valid if the
+ // result is EFI_SUCCESS.
+ *RegionLength = 0;
+ *RegionAttributes = 0;
+
T0SZ = ArmGetTCR () & TCR_T0SZ_MASK;
// Get the Table info from T0SZ
GetRootTranslationTableInfo (T0SZ, , );
@@ -447,10 +453,10 @@ GetMemoryRegion (
);
// If the region continues up to the end of the root table then
GetMemoryRegionRec()
- // will return EFI_NOT_FOUND
- if (Status == EFI_NOT_FOUND) {
+ // will return EFI_NOT_FOUND. Check if the region length was updated.
+ if ((Status == EFI_NOT_FOUND) && (*RegionLength > 0)) {
return EFI_SUCCESS;
- } else {
-return Status;
}
+
+ return Status;
}
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index afd6aab60204..268c0bf3f9ae 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -427,17 +427,20 @@ EfiAttributeToArmAttribute (
EFI_STATUS
GetMemoryRegionPage (
IN UINT32 *PageTable,
- IN OUT UINTN *BaseAddress,
- OUTUINTN *RegionLength,
- OUTUINTN *RegionAttributes
+ IN UINTN *BaseAddress,
+ IN UINTN *RegionAttributes,
+ OUTUINTN *RegionLength
)
{
- UINT32 PageAttributes;
- UINT32 TableIndex;
- UINT32 PageDescriptor;
+ UINT32 PageAttributes;
+ UINT32 TableIndex;
+ UINT32 PageDescriptor;
+ EFI_STATUS Status;
// Convert the section attributes into page attributes
PageAttributes = ConvertSectionAttributesToPageAttributes
(*RegionAttributes);
+ Status = EFI_NOT_FOUND;
+ *RegionLength = 0;
// Calculate index into first level translation table for start of
modification
TableIndex = ((*BaseAddress) & TT_DESCRIPTOR_PAGE_INDEX_MASK) >>
TT_DESCRIPTOR_PAGE_BASE_SHIFT;
@@ -449,23 +452,24 @@ GetMemoryRegionPage (
PageDescriptor = PageTable[TableIndex];
if ((PageDescriptor & TT_DESCRIPTOR_PAGE_TYPE_MASK) ==
TT_DESCRIPTOR_PAGE_TYPE_FAULT) {
- // Case: End of the boundary of the region
- return EFI_SUCCESS;
+ Status = (*RegionLength > 0) ? EFI_SUCCESS : EFI_NO_MAPPING;
+ break;
} else if ((PageDescriptor & TT_DESCRIPTOR_PAGE_TYPE_PAGE) ==
TT_DESCRIPTOR_PAGE_TYPE_PAGE) {
- if
[edk2-devel] [PATCH 1/4] ArmPkg: Apply Uncrustify to Non-Compliant Files
From: Taylor Beebe
This patch applies Uncrustify to the following files:
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
ArmPkg/Include/IndustryStandard/ArmStdSmc.h
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Signed-off-by: Taylor Beebe
---
ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c | 6 +++---
ArmPkg/Include/IndustryStandard/ArmStdSmc.h| 8
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
index 5dbe99fc3134..ccb182668d6a 100644
--- a/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
+++ b/ArmPkg/Drivers/MmCommunicationPei/MmCommunicationPei.c
@@ -159,8 +159,8 @@ MmCommunicationPeim (
}
CopyMem (CommBuffer, CommunicateHeader, BufferSize);
- *CommSize = BufferSize;
- Status = EFI_SUCCESS;
+ *CommSize = BufferSize;
+ Status= EFI_SUCCESS;
break;
case ARM_SMC_MM_RET_INVALID_PARAMS:
@@ -197,7 +197,7 @@ STATIC CONST EFI_PEI_MM_COMMUNICATION_PPI
mPeiMmCommunication = {
STATIC CONST EFI_PEI_PPI_DESCRIPTOR mPeiMmCommunicationPpi = {
(EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
,
- (VOID*)
+ (VOID *)
};
/**
diff --git a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
index 616c650d07c8..f3d78d8e789b 100644
--- a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h
@@ -248,9 +248,9 @@
* SMC64 SiP Service Calls
*/
-#define SMC_FASTCALL 0x8000
-#define SMC64_FUNCTION (SMC_FASTCALL | 0x4000)
-#define SMC_SIP_FUNCTION (SMC64_FUNCTION | 0x0200)
-#define SMC_SIP_FUNCTION_ID(n) (SMC_SIP_FUNCTION | (n))
+#define SMC_FASTCALL 0x8000
+#define SMC64_FUNCTION(SMC_FASTCALL | 0x4000)
+#define SMC_SIP_FUNCTION (SMC64_FUNCTION | 0x0200)
+#define SMC_SIP_FUNCTION_ID(n) (SMC_SIP_FUNCTION | (n))
#endif // ARM_STD_SMC_H_
--
2.41.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106518): https://edk2.groups.io/g/devel/message/106518
Mute This Topic: https://groups.io/mt/99854261/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/4] Update CpuDxe MMU Logic to Improve Consistency
This patch series: 1. Updates applies uncrustify to noncompliant files 2. Updates GetMemoryRegion() to handle the case where BaseAddress is an unmapped page, and update some other return values to be more consistent. 3. Adds some branching paths to what were previously only ASSERT statements to avoid dereferencing NULL and producing non-deterministic behavior when ASSERTs are disabled. 4. Adds function headers to the MMU logic documenting the behavior, parameters, and potetial return values. Taylor Beebe (4): ArmPkg: Apply Uncrustify to Non-Compliant Files ArmPkg: Update GetMemoryRegion() to Handle No mapping ArmPkg: Fix Unsafe ASSERTs in MMU Logic ArmPkg: Add Function Headers to MMU Logic Cc: Leif Lindholm Cc: Ard Biesheuvel ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 143 -- ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 187 ++ .../MmCommunicationPei/MmCommunicationPei.c | 6 +- ArmPkg/Include/IndustryStandard/ArmStdSmc.h | 8 +- 4 files changed, 281 insertions(+), 63 deletions(-) -- 2.41.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106517): https://edk2.groups.io/g/devel/message/106517 Mute This Topic: https://groups.io/mt/99854259/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] RE : when will Edk2 Update CryptoPkg with OpenSSL 3.0
Hi, OpenSSL 1.1.1 series reach End of Life (EOL) on 11th September 2023. For Next Edk2 Release, it is possible to update openssl to 3.0? or When Openssl_3.0 will be updated in edk2. Kindly provide your comments. Thanks, Sountharya -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 (#106516): https://edk2.groups.io/g/devel/message/106516 Mute This Topic: https://groups.io/mt/99849823/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/2] SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator
Hi Kun,
Thank you for this fix.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 28/06/2023 09:33 pm, Kun Qin wrote:
From: Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491
mAvailableAlgoArray is currently allocated for "RNG_AVAILABLE_ALGO_MAX"
number of bytes, whereas it was dereferenced as "EFI_RNG_ALGORITHM".
This change fixed the buffer allocation logic by allocating a proper size
of buffer before referencing.
Cc: Jiewen Yao
Cc: Jian J Wang
Cc: Sami Mujawar
Cc: Pierre Gondois
Signed-off-by: Kun Qin
---
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 2 +-
SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index de279cdadeea..1d412ecff61d 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -33,7 +33,7 @@ GetAvailableAlgorithms (
UINT16 MinorRevision;
// Rng algorithms 2 times, one for the allocation, one to populate.
- mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
+ mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX * sizeof
(EFI_RNG_ALGORITHM));
if (mAvailableAlgoArray == NULL) {
return EFI_OUT_OF_RESOURCES;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
index 4b24f5c4a69b..5e621df601fb 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c
@@ -32,7 +32,7 @@ GetAvailableAlgorithms (
UINT16 MinorRevision;
// Rng algorithms 2 times, one for the allocation, one to populate.
- mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
+ mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX * sizeof
(EFI_RNG_ALGORITHM));
if (mAvailableAlgoArray == NULL) {
return EFI_OUT_OF_RESOURCES;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106515): https://edk2.groups.io/g/devel/message/106515
Mute This Topic: https://groups.io/mt/99839049/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/2] SecurityPkg: RngDxe: Unify handling of zero guid
Hi Kun,
Can you look at Pierre's series and see if this issue is resolved, please?
I have also made further suggestions at
https://edk2.groups.io/g/devel/message/106511.
Regards,
Sami Mujawar
On 28/06/2023 09:33 pm, Kun Qin wrote:
From: Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491
The existing logic of initializing `mAvailableAlgoArrayCount` will treat
the zero GUID in `PcdCpuRngSupportedAlgorithm` as a legit case and
increment `mAvailableAlgoArrayCount`, causing the RNG protocol be
published.
However, when the protocol is invoked, any zero GUID will be filtered
out, leaving a possible edge case where the protocol only has a zero GUID
based algorithm and being filtered out will always result in an ASSERT.
This change marked the zero GUID as an issue and will not increment the
counter and thus avoid publishing the protocol completely.
Cc: Jiewen Yao
Cc: Jian J Wang
Cc: Sami Mujawar
Cc: Pierre Gondois
Signed-off-by: Kun Qin
---
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..de279cdadeea 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -47,15 +47,16 @@ GetAvailableAlgorithms (
);
mAvailableAlgoArrayCount++;
-DEBUG_CODE_BEGIN ();
if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+ DEBUG_CODE_BEGIN ();
DEBUG ((
DEBUG_WARN,
"PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
));
+
+ DEBUG_CODE_END ();
+ mAvailableAlgoArrayCount--;
}
-
-DEBUG_CODE_END ();
}
// Raw algorithm (Trng)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106514): https://edk2.groups.io/g/devel/message/106514
Mute This Topic: https://groups.io/mt/99839047/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 8/8] SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
Hi Pierre,
Thank you for this patch.
These changes look good to me.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
The first element of mAvailableAlgoArray is defined as the default
Rng algorithm to use. Don't go through the array at each RngGetRNG()
call and just return the first element of the array.
Signed-off-by: Pierre Gondois
---
.../RandomNumberGenerator/RngDxe/ArmRngDxe.c| 17 -
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index 78a18c5e1177..7a42e3cbe3d2 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -77,7 +77,6 @@ RngGetRNG (
)
{
EFI_STATUS Status;
- UINTN Index;
GUIDRngGuid;
if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
@@ -88,21 +87,13 @@ RngGetRNG (
//
// Use the default RNG algorithm if RNGAlgorithm is NULL.
//
-for (Index = 0; Index < mAvailableAlgoArrayCount; Index++) {
- if (!IsZeroGuid ([Index])) {
-RNGAlgorithm = [Index];
-goto FoundAlgo;
- }
-}
-
-if (Index == mAvailableAlgoArrayCount) {
- // No algorithm available.
- ASSERT (Index != mAvailableAlgoArrayCount);
- return EFI_DEVICE_ERROR;
+if (mAvailableAlgoArrayCount != 0) {
+ RNGAlgorithm = [0];
+} else {
+ return EFI_UNSUPPORTED;
}
}
-FoundAlgo:
Status = GetRngGuid ();
if (!EFI_ERROR (Status) &&
CompareGuid (RNGAlgorithm, ))
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106513): https://edk2.groups.io/g/devel/message/106513
Mute This Topic: https://groups.io/mt/98779045/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
Hi Pierre,
I think this patch would not be required if my suggestions for patch 6/8
are adopted.
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
The first element of mAvailableAlgoArray should be the default
algorithm to avoid going through a selection process at each
RngGetRNG() call.
Once all the available Rng algorithms have been probed, place
a safe Rng algorithm at the first position of mAvailableAlgoArray.
Signed-off-by: Pierre Gondois
---
.../RngDxe/AArch64/AArch64Algo.c | 48 ++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index a1ff7bd58fda..ed236b2e8141 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -17,6 +17,50 @@
// Maximum number of Rng algorithms.
#define RNG_AVAILABLE_ALGO_MAX 2
+/** mAvailableAlgoArray[0] should contain the default Rng algorithm.
+The Rng algorithm at the first index might be unsafe.
+If a safe algorithm is available, choose it as the default one.
+**/
+VOID
+EFIAPI
+RngFindDefaultAlgo (
+ VOID
+ )
+{
+ EFI_RNG_ALGORITHM *CurAlgo;
+ EFI_RNG_ALGORITHM TmpGuid;
+ UINTN Index;
+
+ CurAlgo = [0];
+
+ if (IsZeroGuid (CurAlgo) ||
+ !CompareGuid (CurAlgo, ))
+ {
+// mAvailableAlgoArray[0] is a valid Rng algorithm.
+return;
+ }
+
+ for (Index = 1; Index < mAvailableAlgoArrayCount; Index++) {
+CurAlgo = [Index];
+if (!IsZeroGuid (CurAlgo) ||
+CompareGuid (CurAlgo, ))
+{
+ break;
+}
+ }
+
+ if (Index == mAvailableAlgoArrayCount) {
+// No valid Rng algorithm available.
+return;
+ }
+
+ CopyMem (, CurAlgo, sizeof (EFI_RNG_ALGORITHM));
+ CopyMem (CurAlgo, [0], sizeof (EFI_RNG_ALGORITHM));
+ CopyMem ([0], , sizeof (EFI_RNG_ALGORITHM));
+
+ return;
+}
+
/** Allocate and initialize mAvailableAlgoArray with the available
Rng algorithms. Also update mAvailableAlgoArrayCount.
@@ -45,7 +89,7 @@ GetAvailableAlgorithms (
if (!EFI_ERROR (Status)) {
CopyMem (
[mAvailableAlgoArrayCount],
- RngGuid,
+ ,
sizeof (RngGuid)
);
mAvailableAlgoArrayCount++;
@@ -68,5 +112,7 @@ GetAvailableAlgorithms (
mAvailableAlgoArrayCount++;
}
+ RngFindDefaultAlgo ();
+
return EFI_SUCCESS;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106512): https://edk2.groups.io/g/devel/message/106512
Mute This Topic: https://groups.io/mt/98779044/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
Hi Pierre,
Thank you for this patch.
I think we should treat algorithms that have a zero GUID as unsafe. I
have made some suggestions on those lines marked inline as [SAMI].
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4151
The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
a GetRngGuid() function was added in a previous patch.
The EFI_RNG_PROTOCOL can advertise multiple algorithms through
Guids. The PcdCpuRngSupportedAlgorithm is currently used to
advertise the RngLib in the Arm implementation.
The issues of doing that are:
- the RngLib implementation might not use CPU instructions,
cf. the BaseRngLibTimerLib
- most platforms don't set PcdCpuRngSupportedAlgorithm
A GetRngGuid() was added to the RngLib in a previous patch,
allowing to identify the algorithm implemented by the RngLib.
Make use of this function.
Signed-off-by: Pierre Gondois
---
.../RngDxe/AArch64/AArch64Algo.c | 24 +--
.../RandomNumberGenerator/RngDxe/ArmRngDxe.c | 6 -
.../RandomNumberGenerator/RngDxe/RngDxe.inf | 5 ++--
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
index e8be217f8a8c..a1ff7bd58fda 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include "RngDxeInternals.h"
@@ -28,9 +29,10 @@ GetAvailableAlgorithms (
VOID
)
{
- UINT64 DummyRand;
- UINT16 MajorRevision;
- UINT16 MinorRevision;
+ EFI_STATUS Status;
+ UINT16 MajorRevision;
+ UINT16 MinorRevision;
+ GUIDRngGuid;
// Rng algorithms 2 times, one for the allocation, one to populate.
mAvailableAlgoArray = AllocateZeroPool (RNG_AVAILABLE_ALGO_MAX);
@@ -38,24 +40,22 @@ GetAvailableAlgorithms (
return EFI_OUT_OF_RESOURCES;
}
- // Check RngGetBytes() before advertising PcdCpuRngSupportedAlgorithm.
- if (!EFI_ERROR (RngGetBytes (sizeof (DummyRand), (UINT8 * {
+ // Identify RngLib algorithm.
+ Status = GetRngGuid ();
+ if (!EFI_ERROR (Status)) {
CopyMem (
[mAvailableAlgoArrayCount],
- PcdGetPtr (PcdCpuRngSupportedAlgorithm),
- sizeof (EFI_RNG_ALGORITHM)
+ RngGuid,
+ sizeof (RngGuid)
);
mAvailableAlgoArrayCount++;
-DEBUG_CODE_BEGIN ();
-if (IsZeroGuid (PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+if (IsZeroGuid ()) {
DEBUG ((
DEBUG_WARN,
-"PcdCpuRngSupportedAlgorithm should be a non-zero GUID\n"
+"RngLib should have a non-zero GUID\n"
));
}
-
-DEBUG_CODE_END ();
}
// Raw algorithm (Trng)
...
[SAMI] I think this code should do the following:
---
BOOLEAN UnSafeAlgo = FALSE;
mAvailableAlgoArrayCount = 0;
// Identify RngLib algorithm.
Status = GetRngGuid ();
if(!EFI_ERROR (Status)) {
if((IsZeroGuid ()) ||
(CompareGuid (, ))) {
// Treat zero GUID as an unsafe algorithm
DEBUG ((
DEBUG_WARN,
"RngLib uses an Unsafe algorithm and "
"must not be used for production builds.\n"
));
// Set the UnSafeAlgo flag to indicate an unsafe algorithm was found
// so that it can be added at the end of the algorithm list.
UnSafeAlgo = TRUE;
} else{
CopyMem (
[mAvailableAlgoArrayCount],
,
sizeof(RngGuid)
);
mAvailableAlgoArrayCount++;
}
}
// Raw algorithm (Trng)
if(!EFI_ERROR (GetArmTrngVersion (, ))) {
CopyMem (
[mAvailableAlgoArrayCount],
,
sizeof(EFI_RNG_ALGORITHM)
);
mAvailableAlgoArrayCount++;
}
// Add unsafe algorithms at the end of the list.
if(UnSafeAlgo) {
CopyMem (
[mAvailableAlgoArrayCount],
,
sizeof(EFI_RNG_ALGORITHM)
);
mAvailableAlgoArrayCount++;
}
---
Doing this will not need the call to RngFindDefaultAlgo () introduced in
the next patch "[PATCH v1 7/8] SecurityPkg/RngDxe: Select safe default
Rng algorithm", which can be dropped.
[SAMI]
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
index ce49ff7ae661..78a18c5e1177 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c
@@ -78,6 +78,7 @@ RngGetRNG (
{
EFI_STATUS Status;
UINTN Index;
+ GUIDRngGuid;
if ((This == NULL) || (RNGValueLength == 0) || (RNGValue == NULL)) {
return EFI_INVALID_PARAMETER;
@@ -102,7 +103,10 @@ RngGetRNG (
}
FoundAlgo:
- if (CompareGuid (RNGAlgorithm,
Re: [edk2-devel] [PATCH v1 5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
Hi Pierre,
Thank you for this patch.
Please find my response inline marked [SAMI].
With that addressed,
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
The EFI_RNG_PROTOCOL can use the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
add a GetRngGuid() function to the RngLib.
Signed-off-by: Pierre Gondois
---
MdePkg/Include/Library/RngLib.h | 17
MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 42 +++
MdePkg/Library/BaseRngLib/BaseRngLib.inf | 9
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 26
.../Library/BaseRngLibNull/BaseRngLibNull.c | 22 ++
.../BaseRngLibTimerLib/BaseRngLibTimerLib.inf | 3 ++
.../Library/BaseRngLibTimerLib/RngLibTimer.c | 28 +
MdePkg/Library/DxeRngLib/DxeRngLib.c | 28 +
8 files changed, 175 insertions(+)
diff --git a/MdePkg/Include/Library/RngLib.h b/MdePkg/Include/Library/RngLib.h
index 429ed19e287e..945482cd5e56 100644
--- a/MdePkg/Include/Library/RngLib.h
+++ b/MdePkg/Include/Library/RngLib.h
@@ -1,6 +1,7 @@
/** @file
Provides random number generator services.
+Copyright (c) 2023, Arm Limited. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -77,4 +78,20 @@ GetRandomNumber128 (
OUT UINT64 *Rand
);
+/**
+ Get a GUID identifying the RNG algorithm implementation.
+
+ @param [out] RngGuid If success, contains the GUID identifying
+the RNG algorithm implementation.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_UNSUPPORTED Not supported.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+ GUID *RngGuid
+ );
+
#endif // __RNG_LIB_H__
diff --git a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
index 20811bf3ebf3..d39db62153ee 100644
--- a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
+++ b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
@@ -2,6 +2,7 @@
Random number generator service that uses the RNDR instruction
to provide pseudorandom numbers.
+ Copyright (c) 2023, Arm Limited. All rights reserved.
Copyright (c) 2021, NUVIA Inc. All rights reserved.
Copyright (c) 2015, Intel Corporation. All rights reserved.
@@ -11,6 +12,7 @@
#include
#include
+#include
#include
#include
@@ -138,3 +140,43 @@ ArchIsRngSupported (
{
return mRndrSupported;
}
+
+/**
+ Get a GUID identifying the RNG algorithm implementation.
+
+ @param [out] RngGuid If success, contains the GUID identifying
+the RNG algorithm implementation.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_UNSUPPORTED Not supported.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+GetRngGuid (
+ GUID *RngGuid
+ )
+{
+ GUID *RngLibGuid;
+
+ if (RngGuid == NULL) {
+return EFI_INVALID_PARAMETER;
+ }
+
+ if (!mRndrSupported) {
+return EFI_UNSUPPORTED;
+ }
+
+ //
+ // If the platform advertises the algorithm behind RNDR instruction,
+ // use it. Otherwise use gEfiRngAlgorithmArmRndr.
+ //
+ RngLibGuid = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+ if (!IsZeroGuid (RngLibGuid)) {
+CopyMem (RngGuid, RngLibGuid, sizeof (*RngGuid));
+ } else {
+CopyMem (RngGuid, , sizeof (*RngGuid));
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
index 1fcceb941495..a79fbf03d74c 100644
--- a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
+++ b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
@@ -4,6 +4,7 @@
# BaseRng Library that uses CPU RNG instructions (e.g. RdRand) to
# provide random numbers.
#
+# Copyright (c) 2023, Arm Limited. All rights reserved.
# Copyright (c) 2021, NUVIA Inc. All rights reserved.
# Copyright (c) 2015, Intel Corporation. All rights reserved.
#
@@ -43,9 +44,17 @@ [Sources.AARCH64]
AArch64/ArmReadIdIsar0.asm | MSFT
AArch64/ArmRng.asm | MSFT
+[Guids]
+ gEfiRngAlgorithmArmRndr
+ gEfiRngAlgorithmSp80090Ctr256Guid
+ gEfiRngAlgorithmUnSafe
[SAMI] Maybe we can split the [Guids] section into [Guids.AARCH64] and
[Guids.Ia32, Guids.X64].
Also gEfiRngAlgorithmUnSafe does not appear to be used and can be removed.
[/SAMI]
+
[Packages]
MdePkg/MdePkg.dec
+[Pcd.AARCH64]
+ gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
+
[LibraryClasses]
BaseLib
DebugLib
diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c
b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
index 070d41e2555f..9bd68352f9f7 100644
--- a/MdePkg/Library/BaseRngLib/Rand/RdRand.c
+++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
@@ -2,6 +2,7 @@
Re: [edk2-devel] [PATCH v1 4/8] MdePkg/Rng: Add GUIDs to describe Rng algorithms
Hi Pierre,
Thank you for this patch.
Please find my response inline marked [SAMI].
Other than the concern mentioned below, this patch looks good to me.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
The EFI_RNG_PROTOCOL can rely on the RngLib. The RngLib has multiple
implementations, some of them are unsafe (e.g. BaseRngLibTimerLib).
To allow the RngDxe to detect when such implementation is used,
a GetRngGuid() function is added in a following patch.
Prepare GetRngGuid() return values and add GUIDs describing
Rng algorithms:
- gEfiRngAlgorithmArmRndr
to describe a Rng algorithm accessed through Arm's RNDR instruction.
[1] states that the implementation of this algorithm should be
compliant to NIST SP900-80. The compliance is not guaranteed.
- gEfiRngAlgorithmUnSafe
to describe an unsafe implementation, cf. the BaseRngLibTimerLib.
[1] Arm Architecture Reference Manual Armv8, for A-profile architecture
sK12.1 'Properties of the generated random number'
Signed-off-by: Pierre Gondois
---
MdePkg/Include/Protocol/Rng.h | 20
MdePkg/MdePkg.dec | 2 ++
2 files changed, 22 insertions(+)
diff --git a/MdePkg/Include/Protocol/Rng.h b/MdePkg/Include/Protocol/Rng.h
index baf425587b3c..dfdaf36e41dc 100644
--- a/MdePkg/Include/Protocol/Rng.h
+++ b/MdePkg/Include/Protocol/Rng.h
@@ -67,6 +67,24 @@ typedef EFI_GUID EFI_RNG_ALGORITHM;
{ \
0xe43176d7, 0xb6e8, 0x4827, {0xb7, 0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85,
0x61 } \
}
+///
+/// The Arm Architecture states the RNDR that the DRBG algorithm should be
compliant
+/// with NIST SP800-90A, while not mandating a particular algorithm, so as to
be
+/// inclusive of different geographies.
+///
+#define EFI_RNG_ALGORITHM_ARM_RNDR \
+ { \
+0x43d2fde3, 0x9d4e, 0x4d79, {0x02, 0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08,
0x41} \
+ }
+///
+/// The implementation of a Random Number Generator might be unsafe, when using
+/// a dummy implementation for instance. Allow identifying such implementation
+/// with this GUID.
+///
+#define EFI_RNG_ALGORITHM_UNSAFE \
+ { \
+0x869f728c, 0x409d, 0x4ab4, {0xac, 0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3,
0xf4 } \
+ }
[SAMI] Unlike the EFI_RNG_ALGORITHM_ARM_RNDR which is backed by the code
first spec update at https://mantis.uefi.org/mantis/view.php?id=2386;
the EFI_RNG_ALGORITHM_UNSAFE is not backed by any specification.
Although I agree that a definition of the unsafe algorithm is required
to support some platforms, I am not sure if this file and the macro
prefix is right for this definition.
I would defer this decision, and any advice on how to proceed to the
MdePkg maintainers.
[/SAMI]
/**
Returns information about the random number generation implementation.
@@ -146,5 +164,7 @@ extern EFI_GUID gEfiRngAlgorithmSp80090Ctr256Guid;
extern EFI_GUID gEfiRngAlgorithmX9313DesGuid;
extern EFI_GUID gEfiRngAlgorithmX931AesGuid;
extern EFI_GUID gEfiRngAlgorithmRaw;
+extern EFI_GUID gEfiRngAlgorithmArmRndr;
+extern EFI_GUID gEfiRngAlgorithmUnSafe;
#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 0ecfad5795e4..754085eaa55b 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -633,6 +633,8 @@ [Guids]
gEfiRngAlgorithmX9313DesGuid = { 0x63c4785a, 0xca34, 0x4012, {0xa3,
0xc8, 0x0b, 0x6a, 0x32, 0x4f, 0x55, 0x46 }}
gEfiRngAlgorithmX931AesGuid= { 0xacd03321, 0x777e, 0x4d3d, {0xb1,
0xc8, 0x20, 0xcf, 0xd8, 0x88, 0x20, 0xc9 }}
gEfiRngAlgorithmRaw= { 0xe43176d7, 0xb6e8, 0x4827, {0xb7,
0x84, 0x7f, 0xfd, 0xc4, 0xb6, 0x85, 0x61 }}
+ gEfiRngAlgorithmArmRndr= { 0x43d2fde3, 0x9d4e, 0x4d79, {0x02,
0x96, 0xa8, 0x9b, 0xca, 0x78, 0x08, 0x41 }}
+ gEfiRngAlgorithmUnSafe = { 0x869f728c, 0x409d, 0x4ab4, {0xac,
0x03, 0x71, 0xd3, 0x09, 0xc1, 0xb3, 0xf4 }}
## Include/Protocol/AdapterInformation.h
gEfiAdapterInfoMediaStateGuid = { 0xD7C74207, 0xA831, 0x4A26, {0xB1,
0xF5, 0xD1, 0x93, 0x06, 0x5C, 0xE8, 0xB6 }}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106509): https://edk2.groups.io/g/devel/message/106509
Mute This Topic: https://groups.io/mt/98779041/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 3/8] MdePkg/DxeRngLib: Request raw algorithm instead of default
Hi Pierre,
Thank you for this patch.
These changes look good to me.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
The DxeRngLib tries to generate a random number using the 3 NIST
SP 800-90 compliant DRBG algorithms, i.e. 256-bits CTR, HASH and HMAC.
If none of the call is successful, the fallback option is the default
RNG algorithm of the EFI_RNG_PROTOCOL. This default algorithm might
be an unsafe implementation.
Try requesting the Raw algorithm before requesting the default one.
Signed-off-by: Pierre Gondois
---
MdePkg/Library/DxeRngLib/DxeRngLib.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c
b/MdePkg/Library/DxeRngLib/DxeRngLib.c
index 46aea515924f..a01b66ad7d20 100644
--- a/MdePkg/Library/DxeRngLib/DxeRngLib.c
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -65,9 +65,15 @@ GenerateRandomNumberViaNist800Algorithm (
return Status;
}
+ Status = RngProtocol->GetRNG (RngProtocol, , BufferSize, Buffer);
+ DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm Raw - Status = %r\n", __func__,
Status));
+ if (!EFI_ERROR (Status)) {
+return Status;
+ }
+
// If all the other methods have failed, use the default method from the
RngProtocol
Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
- DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n",
__func__, Status));
+ DEBUG ((DEBUG_INFO, "%a: GetRNG algorithm default - Status = %r\n",
__func__, Status));
if (!EFI_ERROR (Status)) {
return Status;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106508): https://edk2.groups.io/g/devel/message/106508
Mute This Topic: https://groups.io/mt/98779040/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
Hi Pierre,
Please see my response inline marked [SAMI].
With that fixed,
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
In order to use PcdCpuRngSupportedAlgorithm in the MdePkg in a
following patch and to avoid making the MdePkg dependent on another
package, move PcdCpuRngSupportedAlgorithm to the MdePkg.
As the Pcf is only used for AARCH64, place it in an AARCH64
specific sections.
Signed-off-by: Pierre Gondois
---
MdePkg/MdePkg.dec | 5 +
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 4 ++--
SecurityPkg/SecurityPkg.dec | 2 --
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index d6c4179b2a48..0ecfad5795e4 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2357,6 +2357,11 @@ [PcdsFixedAtBuild,PcdsPatchableInModule]
# @Prompt IPMI KCS Interface I/O Base Address
gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsIoBaseAddress|0xca2|UINT16|0x0031
+[PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
+ ## GUID identifying the Rng algorithm implemented by CPU instruction.
+ # @Prompt CPU Rng algorithm's GUID.
+
gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0032
[SAMI] Apparently the token value 0x0032 is already used when
rebased with latest edk2 code and results in the following build error:
The TokenValue [0x0032] of PCD
[gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm] is conflict with:
[gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifSmbusSlaveAddr]
I believe changing this to 0x0037 should fix the issue.
[/SAMI]
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## This value is used to set the base address of PCI express hierarchy.
# @Prompt PCI Express Base Address.
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index c8e0ee4ae5d9..d6c2d30195bf 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -79,8 +79,8 @@ [Guids]
[Protocols]
gEfiRngProtocolGuid## PRODUCES
-[Pcd]
- gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
+[Pcd.AARCH64]
+ gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
[Depex]
TRUE
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 0a8042d63fe1..6bb02d58bdf0 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -325,8 +325,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass|0x0303100A|UINT32|0x00010030
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationFail|0x0303100B|UINT32|0x00010031
- gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00010032
-
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## Image verification policy for OptionRom. Only following values are
valid:
# NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has
been removed.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106507): https://edk2.groups.io/g/devel/message/106507
Mute This Topic: https://groups.io/mt/98779039/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
Hi Pierre,
Thank you for this patch.
This change looks good to me.
Reviewed-by: Sami Mujawar
Regards,
Sami Mujawar
On 09/05/2023 08:40 am, pierre.gond...@arm.com wrote:
From: Pierre Gondois
Remove ASSERTs to allow RngDxe probing the Null implementation
of the TrngLib.
Signed-off-by: Pierre Gondois
---
MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c | 4
1 file changed, 4 deletions(-)
diff --git a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
index 316d78bf5e83..0ea9aafa59f1 100644
--- a/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
+++ b/MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.c
@@ -41,7 +41,6 @@ GetArmTrngVersion (
OUT UINT16 *MinorRevision
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
@@ -67,7 +66,6 @@ GetArmTrngUuid (
OUT GUID *Guid
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
@@ -83,7 +81,6 @@ GetArmTrngMaxSupportedEntropyBits (
VOID
)
{
- ASSERT (FALSE);
return 0;
}
@@ -116,6 +113,5 @@ GetArmTrngEntropy (
OUT UINT8 *Buffer
)
{
- ASSERT (FALSE);
return RETURN_UNSUPPORTED;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106506): https://edk2.groups.io/g/devel/message/106506
Mute This Topic: https://groups.io/mt/98779038/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
Tom, Thanks for the comments. In V8 patch, I've refined the commit message and added comments in the code around the areas being changed to explain this code change. Thanks, Dun -Original Message- From: devel@edk2.groups.io On Behalf Of Lendacky, Thomas via groups.io Sent: Tuesday, June 27, 2023 9:13 PM To: Tan, Dun ; devel@edk2.groups.io Cc: Ard Biesheuvel ; Yao, Jiewen ; Justen, Jordan L ; Gerd Hoffmann ; Ni, Ray Subject: Re: [edk2-devel] [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry On 6/27/23 00:23, Dun Tan wrote: > Remove code that apply AddressEncMask to non-leaf entry when split s/apply/applies the/ s/entry/entries/ s/split/splitting/ > smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it s/smm page table by/SMM page table entries in/ s/In FvbServicesSmm driver, it/The FvbServicesSmm driver/ > calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask s/clear/clear the/ > bit in page table for a specific range. In AMD SEV feature, this > AddressEncMask bit in page table is used to indicate if the memory is > guest private memory or shared memory. But all memory used by page > table are treated as encrypted regardless of encryption bit. But all memory accessed by the hardware page table walker is treated as encrypted, regardless of whether the encryption bit is present. > So remove the EncMask bit for smm non-leaf page table entry doesn't > impact AMD SEV feature. > If page split happens in the AddressEncMask bit clear process, there > will be some new non-leaf entries with AddressEncMask applied in smm > page table. When ReadyToLock, code in PiSmmCpuDxe module will use > CpuPageTableLib to modify smm page table. So remove code to apply > AddressEncMask for new non-leaf entries since CpuPageTableLib doesn't > consume the EncMask PCD. This last paragraph is a bit confusing to read, please rewrite it so it is easier to understand. > > Signed-off-by: Dun Tan > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Gerd Hoffmann > Cc: Tom Lendacky > Reviewed-by: Ray Ni I think it would be best to include comments in the code around the areas being changed explaining why the the encryption mask is not being set for non-leaf entries because of the way CpuPageTableLib works. With comments added: Reviewed-by: Tom Lendacky > --- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git > a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > index cf2441b551..372fc03fde 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > @@ -233,7 +233,7 @@ Split2MPageTo4K ( > // Fill in 2M page entry. > // > *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > } > > /** > @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly ( > PhysicalAddress += LevelSize[Level - 1]; > } > > - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask | > + PageTable[Index] = (UINT64)(UINTN)NewPageTable | >IA32_PG_P | IA32_PG_RW; > PageTable = NewPageTable; > } > @@ -440,7 +440,7 @@ Split1GPageTo2M ( > // Fill in 1G page entry. > // > *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > > PhysicalAddress2M = PhysicalAddress; > for (IndexOfPageDirectoryEntries = 0; @@ -616,7 +616,7 @@ > InternalMemEncryptSevCreateIdentityMap1G ( > } > > SetMem (NewPageTable, EFI_PAGE_SIZE, 0); > - PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable | > AddressEncMask; > + PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable; > PageMapLevel4Entry->Bits.MustBeZero = 0; > PageMapLevel4Entry->Bits.ReadWrite = 1; > PageMapLevel4Entry->Bits.Present= 1; -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106505): https://edk2.groups.io/g/devel/message/106505 Mute This Topic: https://groups.io/mt/99804303/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V8 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry
Remove code that sets AddressEncMask for non-leaf entries when modifing smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask bit in page table for a specific range. In AMD SEV feature, this AddressEncMask bit in page table is used to indicate if the memory is guest private memory or shared memory. But all memory accessed by the hardware page table walker is treated as encrypted, regardless of whether the encryption bit is present. So remove the code to set the EncMask bit for smm non-leaf entries doesn't impact AMD SEV feature. The reason encryption mask should not be set for non-leaf entries is because CpuPageTableLib doesn't consume encryption mask PCD. In PiSmmCpuDxeSmm module, it will use CpuPageTableLib to modify smm page table in next patch. The encryption mask is overlapped with the PageTableBaseAddress field of non-leaf page table entries. If the encryption mask is set for smm non-leaf page table entries, issue happens when CpuPageTableLib code use the non-leaf entry PageTableBaseAddress field with the encryption mask set to find the next level page table. Signed-off-by: Dun Tan Cc: Ard Biesheuvel Cc: Jiewen Yao Cc: Jordan Justen Cc: Gerd Hoffmann Reviewed-by: Tom Lendacky Reviewed-by: Ray Ni --- OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c index cf2441b551..dee3fb8914 100644 --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c @@ -232,8 +232,14 @@ Split2MPageTo4K ( // // Fill in 2M page entry. // + // AddressEncMask is not set for non-leaf entries since CpuPageTableLib doesn't consume + // encryption mask PCD. The encryption mask is overlapped with the PageTableBaseAddress + // field of non-leaf page table entries. If encryption mask is set for non-leaf entries, + // issue happens when CpuPageTableLib code use the non-leaf entry PageTableBaseAddress + // field with the encryption mask set to find the next level page table. + // *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 | - IA32_PG_P | IA32_PG_RW | AddressEncMask); + IA32_PG_P | IA32_PG_RW); } /** @@ -352,7 +358,10 @@ SetPageTablePoolReadOnly ( PhysicalAddress += LevelSize[Level - 1]; } - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask | + // + // AddressEncMask is not set for non-leaf entries because of the way CpuPageTableLib works + // + PageTable[Index] = (UINT64)(UINTN)NewPageTable | IA32_PG_P | IA32_PG_RW; PageTable = NewPageTable; } @@ -439,8 +448,10 @@ Split1GPageTo2M ( // // Fill in 1G page entry. // + // AddressEncMask is not set for non-leaf entries because of the way CpuPageTableLib works + // *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry | - IA32_PG_P | IA32_PG_RW | AddressEncMask); + IA32_PG_P | IA32_PG_RW); PhysicalAddress2M = PhysicalAddress; for (IndexOfPageDirectoryEntries = 0; @@ -616,7 +627,11 @@ InternalMemEncryptSevCreateIdentityMap1G ( } SetMem (NewPageTable, EFI_PAGE_SIZE, 0); - PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable | AddressEncMask; + + // + // AddressEncMask is not set for non-leaf entries because of the way CpuPageTableLib works + // + PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable; PageMapLevel4Entry->Bits.MustBeZero = 0; PageMapLevel4Entry->Bits.ReadWrite = 1; PageMapLevel4Entry->Bits.Present= 1; -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106504): https://edk2.groups.io/g/devel/message/106504 Mute This Topic: https://groups.io/mt/99847924/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch V8 00/14] Subject: [Patch V8 00/14] Use CpuPageTableLib to create and update smm page table
In the V8 patch set: In 'OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry', I refined the commit message and added comments in the code around the areas being changed to explain this code change. Only resend the changed patch in OvmfPkg. The patch set has been reviewed-by Dun Tan (14): OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry MdeModulePkg: Remove other attribute protection in UnsetGuardPage UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute. UefiCpuPkg: Add DEBUG_CODE for special case when clear RP UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h UefiCpuPkg: Add GenSmmPageTable() to create smm page table UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock UefiCpuPkg: Refinement to smm runtime InitPaging() code UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function MdeModulePkg/Core/PiSmmCore/HeapGuard.c| 16 +++- OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 23 +++ UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 5 +++-- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +-- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c| 2 +- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 132 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 40 ++-- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 121 ++--- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 793 ++--- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 322 +- UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 229 - UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +-- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 20 14 files changed, 696 insertions(+), 1014 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106503): https://edk2.groups.io/g/devel/message/106503 Mute This Topic: https://groups.io/mt/99847923/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] MdePkg: Add Cxl20.h into IndustryStandard
Thanks for the update. +} HDM_DECODER_N_INFO; I think this need add CXL_ prefix. Other looks good to me. Thank you Yao, Jiewen > -Original Message- > From: Chris Li OS > Sent: Wednesday, June 28, 2023 1:28 PM > To: devel@edk2.groups.io; Yao, Jiewen > Subject: [PATCH 1/1] MdePkg: Add Cxl20.h into IndustryStandard > > 1) Add CXL 2.0 header file to comply with CXL 2.0 specification > 2) CXL 2.0 header will embed Cxl11.h > 3) Updated Cxl.h to point to 2.0 header file > > Signed-off-by: Chris Li > --- > MdePkg/Include/IndustryStandard/Cxl.h | 2 +- > MdePkg/Include/IndustryStandard/Cxl20.h | 477 > 2 files changed, 478 insertions(+), 1 deletion(-) > create mode 100644 MdePkg/Include/IndustryStandard/Cxl20.h > > diff --git a/MdePkg/Include/IndustryStandard/Cxl.h > b/MdePkg/Include/IndustryStandard/Cxl.h > index 06c1230e3e..9ad3242e25 100644 > --- a/MdePkg/Include/IndustryStandard/Cxl.h > +++ b/MdePkg/Include/IndustryStandard/Cxl.h > @@ -12,7 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #ifndef _CXL_MAIN_H_ > #define _CXL_MAIN_H_ > > -#include > +#include > // > // CXL assigned new Vendor ID > // > diff --git a/MdePkg/Include/IndustryStandard/Cxl20.h > b/MdePkg/Include/IndustryStandard/Cxl20.h > new file mode 100644 > index 00..a08251f4e9 > --- /dev/null > +++ b/MdePkg/Include/IndustryStandard/Cxl20.h > @@ -0,0 +1,477 @@ > +/** @file > + CXL 2.0 Register definitions > + > + This file contains the register definitions based on the Compute Express > Link > + (CXL) Specification Revision 2.0. > + > + Copyright (c) 2023, Ampere Computing LLC. All rights reserved. > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef CXL20_H_ > +#define CXL20_H_ > + > +#include > + > +// > +// Ensure proper structure formats > +// > +#pragma pack(1) > + > + > +// > +// CXL DVSEC IDs and Revisions > +// Compute Express Link Specification Revision 2.0 - Chapter 8.1.1 > +// > +#define CXL_DVSEC_ID_PCIE_DVSEC_FOR_CXL_DEVICE0x0 > +#define CXL_DVSEC_ID_NON_CXL_FUNCTION_MAP 0x2 > +#define CXL_DVSEC_ID_CXL20_EXTENSIONS_DVSEC_FOR_PORTS 0x3 > +#define CXL_DVSEC_ID_GPF_DVSEC_FOR_CXL_PORTS 0x4 > +#define CXL_DVSEC_ID_GPF_DVSEC_FOR_CXL_DEVICES0x5 > +#define CXL_DVSEC_ID_PCIE_DVSEC_FOR_FLEX_BUS_PORT 0x7 > +#define CXL_DVSEC_ID_REGISTER_LOCATOR 0x8 > +#define CXL_DVSEC_ID_MLD 0x9 > +#define CXL_DVSEC_ID_PCIE_DVSEC_FOR_TEST_CAPABILITY 0xA > + > +#define CXL20_DVSEC_REVISON_REGISTOR_LOCATOR 0x0 > + > +// > +// Register Block ID > +// Compute Express Link Specification Revision 2.0 - Chapter 8.1.9.1 > +// > +#define CXL_REGISTER_BLOCK_ID_EMPTY 0x0 > +#define CXL_REGISTER_BLOCK_ID_COMPONENT 0x01 > +#define CXL_REGISTER_BLOCK_ID_BAR_VIRTUALIZATION_ACL 0x02 > +#define CXL_REGISTER_BLOCK_ID_DEVICE 0x03 > + > +// > +// Component Register Block Register Ranges Offset > +// Compute Express Link Specification Revision 2.0 - Chapter 8.2.4 > +// > +#define CXL_COMPONENT_REGISTERS_RANGE_OFFSET_IO 0x0 > +#define CXL_COMPONENT_REGISTERS_RANGE_OFFSET_CACHE_MEM > 0x1000 > +#define CXL_COMPONENT_REGISTERS_RANGE_OFFSET_ARB_MUX 0xE000 > + > +// > +// CXL Cache Memory Capability IDs > +// Compute Express Link Specification Revision 2.0 - Chapter 8.2.5 > +// > +#define CXL_CACHE_MEM_CAPABILITY_ID_CXL 0x1 > +#define CXL_CACHE_MEM_CAPABILITY_ID_RAS 0x2 > +#define CXL_CACHE_MEM_CAPABILITY_ID_SECURITY 0x3 > +#define CXL_CACHE_MEM_CAPABILITY_ID_LINK 0x4 > +#define CXL_CACHE_MEM_CAPABILITY_ID_HDM_DECODER 0x5 > +#define CXL_CACHE_MEM_CAPABILITY_ID_EXTENDED_SECURITY 0x6 > +#define CXL_CACHE_MEM_CAPABILITY_ID_IDE 0x7 > +#define CXL_CACHE_MEM_CAPABILITY_ID_SNOOP_FILTER 0x8 > +#define CXL_CACHE_MEM_CAPABILITY_ID_MASK 0x > + > +// > +// Generic CXL Device Capability IDs 0x ~ 0x3FFF > +// Compute Express Link Specification Revision 2.0 - Chapter 8.2.8.2.1 > +// > +#define CXL_DEVICE_CAPABILITY_ID_CAPABILITIES_ARRAY_REGISTER > 0x > +#define CXL_DEVICE_CAPABILITY_ID_DEVICE_STATUS 0x0001 > +#define CXL_DEVICE_CAPABILITY_ID_PRIMARY_MAILBOX0x0002 > +#define CXL_DEVICE_CAPABILITY_ID_SECONDARY_MAILBOX 0x0003 > + > +// > +// Specific CXL Device Capability IDs 0x4000 ~ 0x7FFF > +// Compute Express Link Specification Revision 2.0 - Chapter 8.2.8.2.1 > +// > +// (ref: CXL 2.0 spec $8.2.8.5) > +#define CXL_DEVICE_CAPABILITY_ID_MEMORY_DEVICE_STATUS > 0x4000 > +#define CXL_DEVICE_CAPABILITY_ID_MASK 0x > + > +// > +// Memory Device Status > +// Compute Express Link Specification Revision 2.0 - Chapter 8.2.8.5.1.1 > +// > +#define
Re: [edk2-devel] [Patch V2 4/4] BaseTools: FMMT replace new free space fixing in replace
Reviewed-by: Bob Feng -Original Message- From: Chen, Christine Sent: Thursday, June 29, 2023 11:35 AM To: devel@edk2.groups.io Cc: Rebecca Cran ; Gao, Liming ; Feng, Bob C Subject: [Patch V2 4/4] BaseTools: FMMT replace new free space fixing in replace In FMMT replace function, when newffs size <= targetffs size, the new free space is calculated wrong as loss the pad data delta size. That will cause invalid binary generated. This patch fixes this issue. Cc: Rebecca Cran Cc: Liming Gao Cc: Bob Feng Signed-off-by: Yuwei Chen --- BaseTools/Source/Python/FMMT/core/FvHandler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/FMMT/core/FvHandler.py b/BaseTools/Source/Python/FMMT/core/FvHandler.py index 49bbc35baa4d..7a6076033681 100644 --- a/BaseTools/Source/Python/FMMT/core/FvHandler.py +++ b/BaseTools/Source/Python/FMMT/core/FvHandler.py @@ -456,7 +456,7 @@ class FvHandler: # Start free space calculating and moving process. self.ModifyTest(TargetFv.Parent, Needed_Space) else: -New_Free_Space = self.TargetFfs.Data.Size - self.NewFfs.Data.Size +New_Free_Space = self.TargetFfs.Data.Size + + len(self.TargetFfs.Data.PadData) - self.NewFfs.Data.Size - + len(self.NewFfs.Data.PadData) # If TargetFv already have free space, move the new free space into it. if TargetFv.Data.Free_Space: TargetFv.Child[-1].Data.Data += b'\xff' * New_Free_Space -- 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106500): https://edk2.groups.io/g/devel/message/106500 Mute This Topic: https://groups.io/mt/99845068/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 3/4] BaseTools: FMMT support ELF UPLD parser
Reviewed-by: Bob Feng
-Original Message-
From: Chen, Christine
Sent: Thursday, June 29, 2023 11:35 AM
To: devel@edk2.groups.io
Cc: Rebecca Cran ; Feng, Bob C ; Gao,
Liming
Subject: [Patch V2 3/4] BaseTools: FMMT support ELF UPLD parser
FMMT add new function to support the .elf file parsing.
Using '-v' option, the UPLD info will be printed out.
'''
- UNIVERSAL_PAYLOAD_INFO
- 4 bytes align (BOOLEAN)
- Identifier
- SpecRevision
- Attribute
- Revision
- Capability
- ProducerId
- ImageId
UPLD Buffer
'''
Cc: Rebecca Cran
Cc: Bob Feng
Cc: Liming Gao
Signed-off-by: Yuwei Chen
---
BaseTools/Source/Python/FMMT/FMMT.py | 2 ++
BaseTools/Source/Python/FMMT/core/BinaryFactoryProduct.py | 36
+++-
BaseTools/Source/Python/FMMT/core/BiosTree.py | 48
++--
BaseTools/Source/Python/FMMT/core/BiosTreeNode.py | 56
+++-
BaseTools/Source/Python/FMMT/core/FMMTParser.py| 2 +-
BaseTools/Source/Python/FirmwareStorageFormat/UPLHeader.py | 244
6 files changed, 383 insertions(+), 5 deletions(-)
diff --git a/BaseTools/Source/Python/FMMT/FMMT.py
b/BaseTools/Source/Python/FMMT/FMMT.py
index bf580b3843a8..26fc4c5792c4 100644
--- a/BaseTools/Source/Python/FMMT/FMMT.py
+++ b/BaseTools/Source/Python/FMMT/FMMT.py
@@ -84,6 +84,8 @@ class FMMT():
ROOT_TYPE = ROOT_FFS_TREE
elif filetype == '.sec':
ROOT_TYPE = ROOT_SECTION_TREE
+elif filetype == '.elf':
+ROOT_TYPE = ROOT_ELF_TREE
else:
ROOT_TYPE = ROOT_TREE
ViewFile(inputfile, ROOT_TYPE, layoutfilename, outputfile) diff --git
a/BaseTools/Source/Python/FMMT/core/BinaryFactoryProduct.py
b/BaseTools/Source/Python/FMMT/core/BinaryFactoryProduct.py
index 2d4e6d9276d7..de174f26ab23 100644
--- a/BaseTools/Source/Python/FMMT/core/BinaryFactoryProduct.py
+++ b/BaseTools/Source/Python/FMMT/core/BinaryFactoryProduct.py
@@ -15,10 +15,13 @@ from core.GuidTools import GUIDTools from utils.FmmtLogger
import FmmtLogger as logger
ROOT_TREE = 'ROOT'
+ROOT_ELF_TREE = 'ROOT_ELF_TREE'
ROOT_FV_TREE = 'ROOT_FV_TREE'
ROOT_FFS_TREE = 'ROOT_FFS_TREE'
ROOT_SECTION_TREE = 'ROOT_SECTION_TREE'
+ELF_TREE = 'ELF'
+ELF_SECTION_TREE = 'ELF_SECTION_TREE'
FV_TREE = 'FV'
DATA_FV_TREE = 'DATA_FV'
FFS_TREE = 'FFS'
@@ -49,6 +52,12 @@ class BinaryProduct():
def ParserData():
pass
+class ElfFactory(BinaryFactory):
+type = [ROOT_ELF_TREE, ELF_TREE]
+
+def Create_Product():
+return ElfProduct()
+
class SectionFactory(BinaryFactory):
type = [SECTION_TREE]
@@ -354,6 +363,30 @@ class FdProduct(BinaryProduct):
tmp_index += 1
return Fd_Struct
+class ElfSectionProduct(BinaryProduct):
+## Decompress the compressed section.
+def ParserData(self, Section_Tree, whole_Data: bytes, Rel_Whole_Offset:
int=0) -> None:
+pass
+def ParserSectionData(self, Section_Tree, whole_Data: bytes,
Rel_Whole_Offset: int=0) -> None:
+pass
+def ParserProgramData(self, Section_Tree, whole_Data: bytes,
Rel_Whole_Offset: int=0) -> None:
+pass
+
+class ElfProduct(BinaryProduct):
+
+def ParserData(self, ParTree, Whole_Data: bytes, Rel_Whole_Offset: int=0)
-> None:
+Elf_Info = ElfNode(Whole_Data)
+if Elf_Info.Header.ELF_PHOff != 0:
+Elf_Info.GetProgramList(Whole_Data[Elf_Info.Header.ELF_PHOff:])
+if Elf_Info.Header.ELF_SHOff != 0:
+Elf_Info.GetSectionList(Whole_Data[Elf_Info.Header.ELF_SHOff:])
+Elf_Info.FindUPLDSection(Whole_Data)
+Elf_Tree = BIOSTREE(Elf_Info.Name)
+Elf_Tree.type = ELF_TREE
+Elf_Info.Data = Whole_Data[Elf_Info.HeaderLength:]
+Elf_Tree.Data = Elf_Info
+ParTree.insertChild(Elf_Tree)
+
class ParserEntry():
FactoryTable:dict = {
SECTION_TREE: SectionFactory,
@@ -364,6 +397,7 @@ class ParserEntry():
SEC_FV_TREE: FvFactory,
ROOT_FV_TREE: FdFactory,
ROOT_TREE: FdFactory,
+ROOT_ELF_TREE: ElfFactory,
}
def GetTargetFactory(self, Tree_type: str) -> BinaryFactory:
@@ -377,4 +411,4 @@ class ParserEntry():
def DataParser(self, Tree, Data: bytes, Offset: int) -> None:
TargetFactory = self.GetTargetFactory(Tree.type)
if TargetFactory:
-self.Generate_Product(TargetFactory, Tree, Data, Offset)
\ No newline at end of file
+self.Generate_Product(TargetFactory, Tree, Data, Offset)
diff --git a/BaseTools/Source/Python/FMMT/core/BiosTree.py
Re: [edk2-devel] [Patch V2 2/4] BaseTools: FMMT replace output file is not generated successfully
Reviewed-by: Bob Feng
-Original Message-
From: Chen, Christine
Sent: Thursday, June 29, 2023 11:35 AM
To: devel@edk2.groups.io
Cc: Rebecca Cran ; Feng, Bob C ; Gao,
Liming
Subject: [Patch V2 2/4] BaseTools: FMMT replace output file is not generated
successfully
For replace function, when target Ffs and new ffs are with same size, the
output file can not be generated successfully.
This patch fixes this issue.
Cc: Rebecca Cran
Cc: Bob Feng
Cc: Liming Gao
Signed-off-by: Yuwei Chen
---
BaseTools/Source/Python/FMMT/core/BiosTree.py | 4 ++--
BaseTools/Source/Python/FMMT/core/FvHandler.py | 19 ---
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/BaseTools/Source/Python/FMMT/core/BiosTree.py
b/BaseTools/Source/Python/FMMT/core/BiosTree.py
index d8fa4743354a..137f49748b09 100644
--- a/BaseTools/Source/Python/FMMT/core/BiosTree.py
+++ b/BaseTools/Source/Python/FMMT/core/BiosTree.py
@@ -56,7 +56,7 @@ class BIOSTREE:
if len(self.Child) == 0:
self.Child.append(newNode)
else:
-if not pos:
+if not pos or pos == len(self.Child):
LastTree = self.Child[-1]
self.Child.append(newNode)
LastTree.NextRel = newNode @@ -195,4 +195,4 @@ class BIOSTREE:
for item in self.Child:
TreeInfo[key].setdefault('Files',[]).append( item.ExportTree())
-return TreeInfo
\ No newline at end of file
+return TreeInfo
diff --git a/BaseTools/Source/Python/FMMT/core/FvHandler.py
b/BaseTools/Source/Python/FMMT/core/FvHandler.py
index b0cc1951a1c6..49bbc35baa4d 100644
--- a/BaseTools/Source/Python/FMMT/core/FvHandler.py
+++ b/BaseTools/Source/Python/FMMT/core/FvHandler.py
@@ -387,7 +387,21 @@ class FvHandler:
if self.NewFfs.Data.Size >= self.TargetFfs.Data.Size:
Needed_Space = self.NewFfs.Data.Size +
len(self.NewFfs.Data.PadData) - self.TargetFfs.Data.Size -
len(self.TargetFfs.Data.PadData)
# If TargetFv have enough free space, just move part of the free
space to NewFfs.
-if TargetFv.Data.Free_Space >= Needed_Space:
+if Needed_Space == 0:
+Target_index = TargetFv.Child.index(self.TargetFfs)
+TargetFv.Child.remove(self.TargetFfs)
+TargetFv.insertChild(self.NewFfs, Target_index)
+# Modify TargetFv Header and ExtHeader info.
+TargetFv.Data.ModFvExt()
+TargetFv.Data.ModFvSize()
+TargetFv.Data.ModExtHeaderData()
+ModifyFvExtData(TargetFv)
+TargetFv.Data.ModCheckSum()
+# Recompress from the Fv node to update all the related node
data.
+self.CompressData(TargetFv)
+# return the Status
+self.Status = True
+elif TargetFv.Data.Free_Space >= Needed_Space:
# Modify TargetFv Child info and BiosTree.
TargetFv.Child[-1].Data.Data = b'\xff' *
(TargetFv.Data.Free_Space - Needed_Space)
TargetFv.Data.Free_Space -= Needed_Space @@ -450,7 +464,6 @@
class FvHandler:
Target_index = TargetFv.Child.index(self.TargetFfs)
TargetFv.Child.remove(self.TargetFfs)
TargetFv.insertChild(self.NewFfs, Target_index)
-self.Status = True
# If TargetFv do not have free space, create free space for Fv.
else:
New_Free_Space_Tree = BIOSTREE('FREE_SPACE') @@ -461,7 +474,6
@@ class FvHandler:
Target_index = TargetFv.Child.index(self.TargetFfs)
TargetFv.Child.remove(self.TargetFfs)
TargetFv.insertChild(self.NewFfs, Target_index)
-self.Status = True
# Modify TargetFv Header and ExtHeader info.
TargetFv.Data.ModFvExt()
TargetFv.Data.ModFvSize()
@@ -470,6 +482,7 @@ class FvHandler:
TargetFv.Data.ModCheckSum()
# Recompress from the Fv node to update all the related node data.
self.CompressData(TargetFv)
+self.Status = True
logger.debug('Done!')
return self.Status
--
2.27.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106498): https://edk2.groups.io/g/devel/message/106498
Mute This Topic: https://groups.io/mt/99845058/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V2 1/4] BaseTools: fixing FMMT ShrinkFv issue
Reviewed-by: Bob Feng -Original Message- From: Chen, Christine Sent: Thursday, June 29, 2023 11:34 AM To: devel@edk2.groups.io Cc: Rebecca Cran ; Gao, Liming ; Feng, Bob C Subject: [Patch V2 1/4] BaseTools: fixing FMMT ShrinkFv issue 1. FvLength not change issue; 2. FileSystemGuid align with File Size; Cc: Rebecca Cran Cc: Liming Gao Cc: Bob Feng Signed-off-by: Yuwei Chen --- BaseTools/Source/Python/FMMT/core/FMMTOperation.py | 2 +- BaseTools/Source/Python/FMMT/core/FvHandler.py | 10 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/BaseTools/Source/Python/FMMT/core/FMMTOperation.py b/BaseTools/Source/Python/FMMT/core/FMMTOperation.py index a86f8dda9a1a..d4aa3397036d 100644 --- a/BaseTools/Source/Python/FMMT/core/FMMTOperation.py +++ b/BaseTools/Source/Python/FMMT/core/FMMTOperation.py @@ -188,7 +188,7 @@ def ExtractFfs(inputfile: str, Ffs_name: str, outputfile: str, Fv_name: str=None FmmtParser.WholeFvTree.Findlist.remove(FmmtParser.WholeFvTree.Findlist[index]) if FmmtParser.WholeFvTree.Findlist != []: TargetNode = FmmtParser.WholeFvTree.Findlist[0] -if TargetNode.type == FV_TREE or SEC_FV_TREE or DATA_FV_TREE: +if TargetNode.type == FV_TREE or TargetNode.type == SEC_FV_TREE or TargetNode.type == DATA_FV_TREE: FinalData = struct2stream(TargetNode.Data.Header) + TargetNode.Data.Data with open(outputfile, "wb") as f: f.write(FinalData) diff --git a/BaseTools/Source/Python/FMMT/core/FvHandler.py b/BaseTools/Source/Python/FMMT/core/FvHandler.py index ff3d637623f8..b0cc1951a1c6 100644 --- a/BaseTools/Source/Python/FMMT/core/FvHandler.py +++ b/BaseTools/Source/Python/FMMT/core/FvHandler.py @@ -279,7 +279,7 @@ class FvHandler: ParTree.Child.remove(ParTree.Child[-1]) ParTree.Data.Free_Space = 0 ParTree.Data.Size += Needed_Space -ParTree.Data.Header.Fvlength = ParTree.Data.Size +ParTree.Data.Header.FvLength = ParTree.Data.Size ModifyFvSystemGuid(ParTree) for item in ParTree.Child: if item.type == FFS_FREE_SPACE: @@ -650,8 +650,12 @@ class FvHandler: Removed_Space = TargetFv.Data.Free_Space - New_Free_Space TargetFv.Child[-1].Data.Data = b'\xff' * New_Free_Space TargetFv.Data.Size -= Removed_Space -TargetFv.Data.Header.Fvlength = TargetFv.Data.Size -ModifyFvSystemGuid(TargetFv) +TargetFv.Data.Header.FvLength = TargetFv.Data.Size +if struct2stream(TargetFv.Data.Header.FileSystemGuid) == EFI_FIRMWARE_FILE_SYSTEM3_GUID_BYTE: +if TargetFv.Data.Size <= 0xFF: +TargetFv.Data.Header.FileSystemGuid = ModifyGuidFormat( +"8c8ce578-8a3d-4f1c-9935-896185c32dd3") + for item in TargetFv.Child: if item.type == FFS_FREE_SPACE: TargetFv.Data.Data += item.Data.Data + item.Data.PadData -- 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106497): https://edk2.groups.io/g/devel/message/106497 Mute This Topic: https://groups.io/mt/99845056/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 0/2] Fixing RngDxe error for ARM/AARCH64
Hello Kun, Thanks for the patch-set, there is another patch-set that also aims to fix the logic at: https://edk2.groups.io/g/devel/message/104341 but I haven't got feedback so far. Would it be possible to try it out to see if it also solves your issue ? Regards, Pierre On 6/28/23 22:33, Kun Qin wrote: REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4491 On an ARM system that does not support firmware TRNG, the current logic from RngDxe will cause the system to assert at the below line: `ASSERT (Index != mAvailableAlgoArrayCount);` The reason seems to be: 1. When initializing the number of `mAvailableAlgoArrayCount`, the logic will only treat the zero guid of "PcdCpuRngSupportedAlgorithm" as a warning and still increment the counter because "RngGetBytes" might still succeed: https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c#L51C3-L51C3. 2. This will cause the main entry to publish the RNG protocol and accept further usage. 3. However, during usage, the zero guid is always filtered out: https://github.com/tianocore/edk2/blob/1a39bdf2c53858ebb39e6de1362203c65c163c63/SecurityPkg/RandomNumberGenerator/RngDxe/ArmRngDxe.c#L91. Thus, this will cause the system to always not able to find the algorithm and fail the boot with an assert. The suggestion is to at least make the logic of initializing "mAvailableAlgoArrayCount" consistent and filtering algorithm consistent. In addition, the usage of `mAvailableAlgoArray` will always trigger a data abortion error, which is caused by buffer allocated is `RNG_AVAILABLE_ALGO_MAX` number of bytes, which should be `RNG_AVAILABLE_ALGO_MAX` nubmer of EFI_RNG_ALGORITHM. This patch fixed the 2 issues above. The change is verified on QEMU virtual platform and proprietary physical platform. Patch v1 branch: https://github.com/kuqin12/edk2/tree/fix_rng_edk2_v1 Cc: Jiewen Yao Cc: Jian J Wang Cc: Sami Mujawar Cc: Pierre Gondois Kun Qin (2): SecurityPkg: RngDxe: Unify handling of zero guid SecurityPkg: RngDxe: Fixing mAvailableAlgoArray allocator SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/AArch64Algo.c | 9 + SecurityPkg/RandomNumberGenerator/RngDxe/Arm/ArmAlgo.c | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106496): https://edk2.groups.io/g/devel/message/106496 Mute This Topic: https://groups.io/mt/99839045/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
