Date   

Re: [PATCH 1/1] OvmfPkg/VmgExitLib: Fix uninitialized variable warning

Rebecca Cran
 

Reviewed-by: Rebecca Cran <rebecca@...>


--
Rebecca Cran

On 12/13/21 12:38, Brijesh Singh via groups.io wrote:
The XCODE5 reported the below warning

OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1895:12: note: uninitialized use occurs here
Compacted
^^^^^^^^^

Initialize the 'Compacted' variable to fix the warning.

Fixes: d2b998fbdca4 (OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values)
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Rebecca Cran <rebecca@...>
Cc: Michael Roth <Michael.Roth@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index a40a31f7c275..ff367411cc59 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -1872,6 +1872,7 @@ GetCpuidFw (
UINT32 XSaveSize;
XssMsr.Uint64 = 0;
+ Compacted = FALSE;
if (EcxIn == 1) {
/*
* The PPR and APM aren't clear on what size should be encoded in


Re: [PATCH v1 2/2] ArmPkg: MmCommunicationDxe: Update MM communicate input arguments checks

Sami Mujawar
 

Hi Kun,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 30/11/2021 12:39 AM, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few steps before
proceeding with SMC calls. However, some inspection steps are different
from PI specification.

This patch updated MM communicate input argument inspection routine to
match the following PI descriptions:
1. Return code `EFI_INVALID_PARAMETER` represents "the `CommBuffer**`
parameters do not refer to the same location in memory".
2. `CommSize` represents "the size of the data buffer being passed in"
instead of "the size of the data being used from data buffer".
3. Regarding `MessageLength`, "if the `MessageLength` is zero, or too
large for the MM implementation to manage, the MM implementation must
update the `MessageLength` to reflect the size of the `Data` buffer that
it can tolerate".

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 44 ++++++++++++--------
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index b1e309580988..8a2bd222957f 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -41,15 +41,19 @@ STATIC EFI_HANDLE mMmCommunicateHandle;
This function provides a service to send and receive messages from a registered UEFI service.
- @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
- @param[in] CommBufferPhysical Physical address of the MM communication buffer
- @param[in] CommBufferVirtual Virtual address of the MM communication buffer
- @param[in] CommSize The size of the data buffer being passed in. On exit, the size of data
- being returned. Zero if the handler does not wish to reply with any data.
- This parameter is optional and may be NULL.
+ @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer
+ @param[in, out] CommSize The size of the data buffer being passed in. On input, when not
+ omitted, the buffer should cover EFI_MM_COMMUNICATE_HEADER and the
+ value of MessageLength field. On exit, the size of data being
+ returned. Zero if the handler does not wish to reply with any data.
+ This parameter is optional and may be NULL.
@retval EFI_SUCCESS The message was successfully posted.
- @retval EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+ @retval EFI_INVALID_PARAMETER CommBufferPhysical or CommBufferVirtual was NULL, or integer value
+ pointed by CommSize does not cover EFI_MM_COMMUNICATE_HEADER and the
+ value of MessageLength field.
@retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM implementation.
If this error is returned, the MessageLength field
in the CommBuffer header or the integer pointed by
@@ -82,10 +86,11 @@ MmCommunication2Communicate (
//
// Check parameters
//
- if (CommBufferVirtual == NULL) {
+ if (CommBufferVirtual == NULL || CommBufferPhysical == NULL) {
return EFI_INVALID_PARAMETER;
}
+ Status = EFI_SUCCESS;
CommunicateHeader = CommBufferVirtual;
// CommBuffer is a mandatory parameter. Hence, Rely on
// MessageLength + Header to ascertain the
@@ -95,33 +100,38 @@ MmCommunication2Communicate (
sizeof (CommunicateHeader->HeaderGuid) +
sizeof (CommunicateHeader->MessageLength);
- // If the length of the CommBuffer is 0 then return the expected length.
- if (CommSize != 0) {
+ // If CommSize is not omitted, perform size inspection before proceeding.
+ if (CommSize != NULL) {
// This case can be used by the consumer of this driver to find out the
// max size that can be used for allocating CommBuffer.
if ((*CommSize == 0) ||
(*CommSize > mNsCommBuffMemRegion.Length)) {
*CommSize = mNsCommBuffMemRegion.Length;
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
}
//
- // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
+ // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
//
- if (*CommSize != BufferSize) {
- return EFI_INVALID_PARAMETER;
+ if (*CommSize < BufferSize) {
+ Status = EFI_INVALID_PARAMETER;
}
}
//
- // If the buffer size is 0 or greater than what can be tolerated by the MM
+ // If the message length is 0 or greater than what can be tolerated by the MM
// environment then return the expected size.
//
- if ((BufferSize == 0) ||
+ if ((CommunicateHeader->MessageLength == 0) ||
(BufferSize > mNsCommBuffMemRegion.Length)) {
CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length -
sizeof (CommunicateHeader->HeaderGuid) -
sizeof (CommunicateHeader->MessageLength);
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
+ }
+
+ // MessageLength or CommSize check has failed, return here.
+ if (EFI_ERROR (Status)) {
+ return Status;
}
// SMC Function ID


[PATCH V3 1/1] UefiCpuPkg: Extend measurement of microcode patches to TPM

Longlong Yang
 

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

TCG specification says BIOS should extend measurement of microcode to TPM.
However, reference BIOS is not doing this. BIOS shall extend measurement of
microcode to TPM.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Min M Xu <min.m.xu@...>
Cc: Qi Zhang <qi1.zhang@...>
Signed-off-by: Longlong Yang <longlong.yang@...>
---
.../MicrocodeMeasurementDxe.c | 280 ++++++++++++++++++
.../MicrocodeMeasurementDxe.inf | 56 ++++
.../MicrocodeMeasurementDxe.uni | 15 +
.../MicrocodeMeasurementDxeExtra.uni | 12 +
UefiCpuPkg/UefiCpuPkg.dsc | 1 +
5 files changed, 364 insertions(+)
create mode 100644 UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.c
create mode 100644 UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.inf
create mode 100644 UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.uni
create mode 100644 UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxeExtra.uni

diff --git a/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.c b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.c
new file mode 100644
index 000000000000..6f7dcda789f9
--- /dev/null
+++ b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.c
@@ -0,0 +1,280 @@
+/** @file
+ This driver measures microcode patches to TPM.
+
+ This driver consumes gEdkiiMicrocodePatchHobGuid, packs all unique microcode patch found in gEdkiiMicrocodePatchHobGuid to a binary blob, and measures the binary blob to TPM.
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <IndustryStandard/UefiTcgPlatform.h>
+#include <Guid/EventGroup.h>
+#include <Guid/MicrocodePatchHob.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/HobLib.h>
+#include <Library/MicrocodeLib.h>
+#include <Library/TpmMeasurementLib.h>
+
+#define CPU_MICROCODE_MEASUREMENT_DESCRIPTION "Microcode Measurement"
+#define CPU_MICROCODE_MEASUREMENT_EVENT_LOG_DESCRIPTION_LEN sizeof (CPU_MICROCODE_MEASUREMENT_DESCRIPTION)
+
+#pragma pack(1)
+typedef struct {
+ UINT8 Description[CPU_MICROCODE_MEASUREMENT_EVENT_LOG_DESCRIPTION_LEN];
+ UINTN NumberOfMicrocodePatchesMeasured;
+ UINTN SizeOfMicrocodePatchesMeasured;
+} CPU_MICROCODE_MEASUREMENT_EVENT_LOG;
+#pragma pack()
+
+/**
+ Helping function.
+
+ The function is called by QuickSort to compare the order of offsets of
+ two microcode patches in RAM relative to their base address. Elements
+ will be in ascending order.
+
+ @param[in] Offset1 The pointer to the offset of first microcode patch.
+ @param[in] Offset2 The pointer to the offset of second microcode patch.
+
+ @retval 1 The offset of first microcode patch is bigger than that of the second.
+ @retval -1 The offset of first microcode patch is smaller than that of the second.
+ @retval 0 The offset of first microcode patch equals to that of the second.
+**/
+INTN
+EFIAPI
+MicrocodePatchOffsetCompareFunction (
+ IN CONST VOID *Offset1,
+ IN CONST VOID *Offset2
+ )
+{
+ if (*(UINT64 *)(Offset1) > *(UINT64 *)(Offset2)) {
+ return 1;
+ } else if (*(UINT64 *)(Offset1) < *(UINT64 *)(Offset2)) {
+ return -1;
+ } else {
+ return 0;
+ }
+}
+
+/**
+ This function remove duplicate and invalid offsets in Offsets.
+
+ This function remove duplicate and invalid offsets in Offsets. Invalid offset means MAX_UINT64 in Offsets.
+
+ @param[in] Offsets Microcode offset list.
+ @param[in, out] Count On call as the count of raw microcode offset list; On return as count of the clean microcode offset list.
+ **/
+VOID
+RemoveDuplicateAndInvalidOffset (
+ IN UINT64 *Offsets,
+ IN OUT UINTN *Count
+ )
+{
+ UINTN Index;
+ UINTN NewCount;
+ UINT64 LastOffset;
+ UINT64 QuickSortBuffer;
+
+ //
+ // The order matters when packing all applied microcode patches to a single binary blob.
+ // Therefore it is a must to do sorting before packing.
+ // NOTE: We assumed that the order of address of every microcode patch in RAM is the same
+ // with the order of those in the Microcode Firmware Volume in FLASH. If any future updates
+ // made this assumption untenable, then needs a new solution to measure microcode patches.
+ //
+ QuickSort (
+ Offsets,
+ *Count,
+ sizeof (UINT64),
+ MicrocodePatchOffsetCompareFunction,
+ (VOID *)&QuickSortBuffer
+ );
+
+ NewCount = 0;
+ LastOffset = MAX_UINT64;
+ for (Index = 0; Index < *Count; Index++) {
+ //
+ // When MAX_UINT64 element is met, all following elements are MAX_UINT64.
+ //
+ if (Offsets[Index] == MAX_UINT64) {
+ break;
+ }
+
+ //
+ // Remove duplicated offsets
+ //
+ if (Offsets[Index] != LastOffset) {
+ LastOffset = Offsets[Index];
+ Offsets[NewCount] = Offsets[Index];
+ NewCount++;
+ }
+ }
+
+ *Count = NewCount;
+}
+
+/**
+ Callback function.
+
+ Called after signaling of the Ready to Boot Event. Measure microcode patches binary blob with event type EV_CPU_MICROCODE to PCR[1] in TPM.
+
+ @param[in] Event Event whose notification function is being invoked.
+ @param[in] Context Pointer to the notification function's context.
+
+**/
+VOID
+EFIAPI
+MeasureMicrocodePatches (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+ UINT32 PCRIndex;
+ UINT32 EventType;
+ CPU_MICROCODE_MEASUREMENT_EVENT_LOG EventLog;
+ UINT32 EventLogSize;
+ EFI_HOB_GUID_TYPE *GuidHob;
+ EDKII_MICROCODE_PATCH_HOB *MicrocodePatchHob;
+ UINT64 *Offsets;
+ UINTN Count;
+ UINTN Index;
+ UINTN TotalMicrocodeSize;
+ UINT8 *MicrocodePatchesBlob;
+
+ PCRIndex = 1;
+ EventType = EV_CPU_MICROCODE;
+ AsciiStrCpyS (
+ (CHAR8 *)(EventLog.Description),
+ CPU_MICROCODE_MEASUREMENT_EVENT_LOG_DESCRIPTION_LEN,
+ CPU_MICROCODE_MEASUREMENT_DESCRIPTION
+ );
+ EventLog.NumberOfMicrocodePatchesMeasured = 0;
+ EventLog.SizeOfMicrocodePatchesMeasured = 0;
+ EventLogSize = sizeof (CPU_MICROCODE_MEASUREMENT_EVENT_LOG);
+ Offsets = NULL;
+ TotalMicrocodeSize = 0;
+ Count = 0;
+
+ GuidHob = GetFirstGuidHob (&gEdkiiMicrocodePatchHobGuid);
+ if (NULL == GuidHob) {
+ DEBUG ((DEBUG_ERROR, "ERROR: GetFirstGuidHob (&gEdkiiMicrocodePatchHobGuid) failed.\n"));
+ return;
+ }
+
+ MicrocodePatchHob = GET_GUID_HOB_DATA (GuidHob);
+ DEBUG (
+ (DEBUG_INFO,
+ "INFO: Got MicrocodePatchHob with microcode patches starting address:0x%x, microcode patches region size:0x%x, processor count:0x%x\n",
+ MicrocodePatchHob->MicrocodePatchAddress, MicrocodePatchHob->MicrocodePatchRegionSize,
+ MicrocodePatchHob->ProcessorCount)
+ );
+
+ Offsets = AllocateCopyPool (
+ MicrocodePatchHob->ProcessorCount * sizeof (UINT64),
+ MicrocodePatchHob->ProcessorSpecificPatchOffset
+ );
+ Count = MicrocodePatchHob->ProcessorCount;
+
+ RemoveDuplicateAndInvalidOffset (Offsets, &Count);
+
+ if (0 == Count) {
+ DEBUG ((DEBUG_INFO, "INFO: No microcode patch is ever applied, skip the measurement of microcode!\n"));
+ FreePool (Offsets);
+ return;
+ }
+
+ for (Index = 0; Index < Count; Index++) {
+ TotalMicrocodeSize +=
+ GetMicrocodeLength ((CPU_MICROCODE_HEADER *)((UINTN)(MicrocodePatchHob->MicrocodePatchAddress + Offsets[Index])));
+ }
+
+ EventLog.NumberOfMicrocodePatchesMeasured = Count;
+ EventLog.SizeOfMicrocodePatchesMeasured = TotalMicrocodeSize;
+
+ MicrocodePatchesBlob = AllocateZeroPool (TotalMicrocodeSize);
+ if (NULL == MicrocodePatchesBlob) {
+ DEBUG ((DEBUG_ERROR, "ERROR: AllocateZeroPool to MicrocodePatchesBlob failed!\n"));
+ FreePool (Offsets);
+ return;
+ }
+
+ TotalMicrocodeSize = 0;
+ for (Index = 0; Index < Count; Index++) {
+ CopyMem (
+ (VOID *)(MicrocodePatchesBlob + TotalMicrocodeSize),
+ (VOID *)((UINTN)(MicrocodePatchHob->MicrocodePatchAddress + Offsets[Index])),
+ (UINTN)(GetMicrocodeLength (
+ (CPU_MICROCODE_HEADER *)((UINTN)(MicrocodePatchHob->MicrocodePatchAddress +
+ Offsets[Index]))
+ ))
+ );
+ TotalMicrocodeSize +=
+ GetMicrocodeLength ((CPU_MICROCODE_HEADER *)((UINTN)(MicrocodePatchHob->MicrocodePatchAddress + Offsets[Index])));
+ }
+
+ Status = TpmMeasureAndLogData (
+ PCRIndex, // PCRIndex
+ EventType, // EventType
+ &EventLog, // EventLog
+ EventLogSize, // LogLen
+ MicrocodePatchesBlob, // HashData
+ TotalMicrocodeSize // HashDataLen
+ );
+ if (!EFI_ERROR (Status)) {
+ gBS->CloseEvent (Event);
+ DEBUG (
+ (DEBUG_INFO,
+ "INFO: %d Microcode patches are successfully extended to TPM! The total size measured to TPM is 0x%x\n",
+ Count,
+ TotalMicrocodeSize)
+ );
+ } else {
+ DEBUG ((DEBUG_ERROR, "ERROR: TpmMeasureAndLogData failed with status %a!\n", Status));
+ }
+
+ FreePool (Offsets);
+ FreePool (MicrocodePatchesBlob);
+ return;
+}
+
+/**
+
+ Driver to produce microcode measurement.
+
+ Driver to produce microcode measurement. Which install a callback function on ready to boot event.
+
+ @param ImageHandle Module's image handle
+ @param SystemTable Pointer of EFI_SYSTEM_TABLE
+
+ @return EFI_SUCCESS This function always complete successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+MicrocodeMeasurementDriverEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_EVENT Event;
+
+ //
+ // Measure Microcode patches
+ //
+ EfiCreateEventReadyToBootEx (
+ TPL_CALLBACK,
+ MeasureMicrocodePatches,
+ NULL,
+ &Event
+ );
+
+ return EFI_SUCCESS;
+}
diff --git a/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.inf b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.inf
new file mode 100644
index 000000000000..649fb9403fd2
--- /dev/null
+++ b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.inf
@@ -0,0 +1,56 @@
+## @file
+# This driver measures microcode patches to TPM.
+#
+# This driver consumes gEdkiiMicrocodePatchHobGuid, packs all unique
+# microcode patch found in gEdkiiMicrocodePatchHobGuid to a binary blob,
+# and measures the binary blob to TPM.
+#
+# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = MicrocodeMeasurementDxe
+ MODULE_UNI_FILE = MicrocodeMeasurementDxe.uni
+ FILE_GUID = 0A32A803-ACDF-4C89-8293-91011548CD91
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = MicrocodeMeasurementDriverEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64
+#
+
+[Sources]
+ MicrocodeMeasurementDxe.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ UefiBootServicesTableLib
+ MemoryAllocationLib
+ BaseMemoryLib
+ BaseLib
+ UefiLib
+ UefiDriverEntryPoint
+ DebugLib
+ HobLib
+ MicrocodeLib
+ TpmMeasurementLib
+
+[Guids]
+ gEdkiiMicrocodePatchHobGuid ## CONSUMES ## HOB
+
+[UserExtensions.TianoCore."ExtraFiles"]
+ MicrocodeMeasurementDxeExtra.uni
+
+[Depex]
+ TRUE
diff --git a/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.uni b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.uni
new file mode 100644
index 000000000000..5a21e955fbbf
--- /dev/null
+++ b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.uni
@@ -0,0 +1,15 @@
+// /** @file
+// This driver measures microcode patches to TPM.
+//
+// This driver consumes gEdkiiMicrocodePatchHobGuid, packs all uniquemicrocode patch found in gEdkiiMicrocodePatchHobGuid to a binary blob, and measures the binary blob to TPM.
+//
+// Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "This driver measures Microcode Patches to TPM."
+
+#string STR_MODULE_DESCRIPTION #language en-US "This driver consumes gEdkiiMicrocodePatchHobGuid, packs all microcode patch found in gEdkiiMicrocodePatchHobGuid to a binary blob, and measure the binary blob to TPM."
diff --git a/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxeExtra.uni b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxeExtra.uni
new file mode 100644
index 000000000000..6990cee8c6fd
--- /dev/null
+++ b/UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxeExtra.uni
@@ -0,0 +1,12 @@
+// /** @file
+// MicrocodeMeasurementDxe Localized Strings and Content
+//
+// Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_PROPERTIES_MODULE_NAME
+#language en-US
+"Microcode Patches Measurement DXE Driver"
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 870b45284087..d1d61dd6a03b 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -119,6 +119,7 @@
UefiCpuPkg/Library/CpuTimerLib/BaseCpuTimerLib.inf
UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
+ UefiCpuPkg/MicrocodeMeasurementDxe/MicrocodeMeasurementDxe.inf

[Components.IA32, Components.X64]
UefiCpuPkg/CpuDxe/CpuDxe.inf
--
2.31.1.windows.1


Re: [PATCH v4 6/7] OvmfPkg/PlatformCI: dummy grub.efi for AmdSev

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:14, Gerd Hoffmann wrote:
Building grub.efi for AmdSev is difficult because it depends on patches
not yet merged to upstream grub. So shortcut the grub build by simply
creating an empty grub.efi file. That allows to at least build-test the
AmdSev variant.

Signed-off-by: Gerd Hoffmann <kraxel@...>
Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
Reviewed-by: Dov Murik <dovmurik@...>
---
OvmfPkg/PlatformCI/AmdSevBuild.py | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/PlatformCI/AmdSevBuild.py b/OvmfPkg/PlatformCI/AmdSevBuild.py
index 2dd72cfe80d9..816caafb0084 100644
--- a/OvmfPkg/PlatformCI/AmdSevBuild.py
+++ b/OvmfPkg/PlatformCI/AmdSevBuild.py
@@ -6,6 +6,7 @@
##
import os
import sys
+import subprocess

sys.path.append(os.path.dirname(os.path.abspath(__file__)))
from PlatformBuildLib import SettingsManager
@@ -35,3 +36,7 @@ class CommonPlatform():

import PlatformBuildLib
PlatformBuildLib.CommonPlatform = CommonPlatform
+
+# hack alert -- create dummy grub.efi
+subprocess.run(['touch', 'OvmfPkg/AmdSev/Grub/grub.efi'])
+subprocess.run(['ls', '-l', '--sort=time', 'OvmfPkg/AmdSev/Grub'])
Why run 'ls'?

What about replacing subprocess by open?

open('OvmfPkg/AmdSev/Grub/grub.efi', 'a').close()


Re: [PATCH v3 1/5] OvmfPkg/Microvm/fdt: add device tree support

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:16, Gerd Hoffmann wrote:
Add fdt parser from EmbeddedPkg (FdtLib and FdtClientDxe) to MicrovmX64.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 6 ++++++
OvmfPkg/Microvm/MicrovmX64.fdf | 2 ++
2 files changed, 8 insertions(+)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 4/5] OvmfPkg/Microvm/virtio: add virtio-mmio support

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:16, Gerd Hoffmann wrote:
Add virtio-mmio support (VirtioMmioDeviceLib and VirtioFdtDxe).

With this patch added and a new enough qemu version (6.2+) edk2
will detect virtio-mmio devices, so it is possible to boot from
storage (virtio-blk, virtio-scsi) or network (virtio-net).

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/Microvm/MicrovmX64.dsc | 2 ++
OvmfPkg/Microvm/MicrovmX64.fdf | 1 +
2 files changed, 3 insertions(+)
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 5/5] OvmfPkg/Microvm: add README

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:16, Gerd Hoffmann wrote:
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3599
Signed-off-by: Gerd Hoffmann <kraxel@...>
Acked-by: Jiewen Yao <Jiewen.yao@...>
---
OvmfPkg/Microvm/README | 50 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 OvmfPkg/Microvm/README
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v3 3/5] OvmfPkg/Microvm/fdt: add empty fdt

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:16, Gerd Hoffmann wrote:
FdtClient is unhappy without a device tree, so add an empty fdt
which we can use in case etc/fdt is not present in fw_cfg.

On ARM machines a device tree is mandatory for hardware detection,
thats why FdtClient fails hard.
"that's"


On microvm the device tree is only used to detect virtio-mmio devices
(this patch series) and the pcie host (future series). So edk2 can
continue with limited functionality in case no device tree is present:
no storage, no network, but serial console and direct kernel boot
works.

qemu release 6.2 & newer will provide a device tree for microvm.

https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
Reviewed-by: Philippe Mathieu-Daude <philmd@...>
---
OvmfPkg/PlatformPei/Platform.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)


Re: [PATCH v3 2/5] OvmfPkg/Microvm/fdt: load fdt from fw_cfg

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:16, Gerd Hoffmann wrote:
Needed for hardware detection: virtio-mmio devices for now,
later also pcie root bridge.

Depends on patched qemu which actually provides an fdt:
https://gitlab.com/kraxel/qemu/-/commits/sirius/microvm-device-tree
Link returns 404.


https://bugzilla.tianocore.org/show_bug.cgi?id=3689
Signed-off-by: Gerd Hoffmann <kraxel@...>
---
OvmfPkg/PlatformPei/PlatformPei.inf | 1 +
OvmfPkg/PlatformPei/Platform.c | 45 +++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)

diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 1c56ba275835..8ef404168c45 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -44,6 +44,7 @@ [Packages]

[Guids]
gEfiMemoryTypeInformationGuid
+ gFdtHobGuid

[LibraryClasses]
BaseLib
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 906f64615de7..c9ec1d7e99fb 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -321,6 +321,50 @@ PciExBarInitialization (
);
}

+VOID
+MicrovmInitialization (
+ VOID
+ )
+{
+ FIRMWARE_CONFIG_ITEM FdtItem;
+ UINTN FdtSize;
+ UINTN FdtPages;
+ EFI_STATUS Status;
+ UINT64 *FdtHobData;
+ VOID *NewBase;
+
+ Status = QemuFwCfgFindFile ("etc/fdt", &FdtItem, &FdtSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: no etc/fdt found in fw_cfg\n", __FUNCTION__));
+ return;
+ }
+
+ FdtPages = EFI_SIZE_TO_PAGES (FdtSize);
+ NewBase = AllocatePages (FdtPages);
+ if (NewBase == NULL) {
+ DEBUG ((DEBUG_INFO, "%a: AllocatePages failed\n", __FUNCTION__));
+ return;
+ }
+
+ QemuFwCfgSelectItem (FdtItem);
+ QemuFwCfgReadBytes (FdtSize, NewBase);
+
+ FdtHobData = BuildGuidHob (&gFdtHobGuid, sizeof (*FdtHobData));
+ if (FdtHobData == NULL) {
+ DEBUG ((DEBUG_INFO, "%a: BuildGuidHob failed\n", __FUNCTION__));
FreePages (NewBase)?

Otherwise:
Reviewed-by: Philippe Mathieu-Daude <philmd@...>

+ return;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "%a: fdt at 0x%x (size %d)\n",
+ __FUNCTION__,
+ NewBase,
+ FdtSize
+ ));
+ *FdtHobData = (UINTN)NewBase;
+}
+
VOID
MiscInitialization (
VOID
@@ -368,6 +412,7 @@ MiscInitialization (
break;
case 0xffff: /* microvm */
DEBUG ((DEBUG_INFO, "%a: microvm\n", __FUNCTION__));
+ MicrovmInitialization ();
PcdStatus = PcdSet16S (
PcdOvmfHostBridgePciDevId,
MICROVM_PSEUDO_DEVICE_ID


Re: [PATCH v4 7/7] OvmfPkg/PlatformCI: add XenBuild.py

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:14, Gerd Hoffmann wrote:
Add build test for OvmfXen.

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
.../.azurepipelines/Ubuntu-GCC5.yml | 9 +++++
OvmfPkg/PlatformCI/XenBuild.py | 37 +++++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 OvmfPkg/PlatformCI/XenBuild.py
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [PATCH v4 3/7] OvmfPkg/PlatformCI: add BhyveBuild.py

Philippe Mathieu-Daudé <philmd@...>
 

On 12/13/21 09:14, Gerd Hoffmann wrote:
Add build test for OvmfPkg/Bhyve.

Signed-off-by: Gerd Hoffmann <kraxel@...>
Acked-by: Jiewen Yao <Jiewen.yao@...>
Acked-by: Ard Biesheuvel <ardb@...>
---
.../.azurepipelines/Ubuntu-GCC5.yml | 9 +++++
OvmfPkg/PlatformCI/BhyveBuild.py | 37 +++++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 OvmfPkg/PlatformCI/BhyveBuild.py
Reviewed-by: Philippe Mathieu-Daude <philmd@...>


Re: [edk2-libc Patch V4 6/6] AppPkg/Applications/Python: to fix readme files in edk2-libc

Rebecca Cran
 

It looks like it did get pushed to master - see https://github.com/tianocore/edk2-libc/commits/master .


--

Rebecca Cran

On 11/3/21 12:28 AM, Jayaprakash, N wrote:
Hi Mike,

Has this change been merged to master?

Regards,
JP

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: 02 November 2021 00:30
To: Jayaprakash, N <n.jayaprakash@...>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>
Cc: Rebecca Cran <rebecca@...>
Subject: RE: [edk2-libc Patch V4 6/6] AppPkg/Applications/Python: to fix readme files in edk2-libc

Series Reviewed-by: Michael D Kinney <michael.d.kinney@...>


-----Original Message-----
From: Jayaprakash, N <n.jayaprakash@...>
Sent: Monday, November 1, 2021 11:35 AM
To: devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@...>; Kinney, Michael D
<michael.d.kinney@...>; Jayaprakash, N <n.jayaprakash@...>
Subject: [edk2-libc Patch V4 6/6] AppPkg/Applications/Python: to fix
readme files in edk2-libc

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

This commit is to update remaining references to py 2.7.10 in
StdLib/Readme.txt and StdLibPrivateInternalFiles/ReadMe.txt
documents to py3.6.8.

Cc: Rebecca Cran <rebecca@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Signed-off-by: Jayaprakash N <n.jayaprakash@...>
---
StdLib/ReadMe.txt | 15 +++++++--------
StdLibPrivateInternalFiles/ReadMe.txt | 14 ++++++--------
2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/StdLib/ReadMe.txt b/StdLib/ReadMe.txt index
5199692..8e0305e 100644
--- a/StdLib/ReadMe.txt
+++ b/StdLib/ReadMe.txt
@@ -158,14 +158,13 @@ There are some boiler-plate declarations and
definitions that need to be included in your application's INF and
DSC build files. These are described in the CONFIGURATION section, below.

-A subset of the Python 2.7.2 distribution is included as part of
AppPkg. If desired, -the full Python 2.7.2 distribution may be downloaded from python.org and used instead.
-Delete or rename the existing Python-2.7.2 directory then extract the
downloaded -Python-2.7.2.tgz file into the AppPkg\Applications\Python
directory. This will produce a
-Python-2.7.2 directory containing the full Python distribution.
Python files that had to be -modified for EDK II are in the
AppPkg\Applications\Python\PyMod-2.7.2 directory. These -files need
to be copied into the corresponding directories within the extracted Python-2.7.2 -directory before Python can be built.
+A full distribution of the Python 3.6.8 has been included as part of
+AppPkg. But only a subset of the features have been enabled for UEFI
+use case. Python files that had to be modified for EDK II are in the AppPkg\Applications\Python\Python-3.6.8\PyMod-3.6.8 directory.
+These files need to be copied into the corresponding directories
+within the Python-3.6.8 directory before Python can be built. This
+can be achieved by running the srcprep.py available under AppPkg\Applications\Python\Python-3.6.8.
+


BUILDING
diff --git a/StdLibPrivateInternalFiles/ReadMe.txt
b/StdLibPrivateInternalFiles/ReadMe.txt
index 424ee96..e21d2c8 100644
--- a/StdLibPrivateInternalFiles/ReadMe.txt
+++ b/StdLibPrivateInternalFiles/ReadMe.txt
@@ -146,14 +146,12 @@ There are some boiler-plate declarations and
definitions that need to be included in your application's INF and
DSC build files. These are described in the CONFIGURATION section, below.

-A subset of the Python 2.7.2 distribution is included as part of
AppPkg. If desired, -the full Python 2.7.2 distribution may be downloaded from python.org and used instead.
-Delete or rename the existing Python-2.7.2 directory then extract the
downloaded -Python-2.7.2.tgz file into the AppPkg\Applications\Python
directory. This will produce a
-Python-2.7.2 directory containing the full Python distribution.
Python files that had to be -modified for EDK II are in the
AppPkg\Applications\Python\PyMod-2.7.2 directory. These -files need
to be copied into the corresponding directories within the extracted Python-2.7.2 -directory before Python can be built.
+A full distribution of the Python 3.6.8 has been included as part of
+AppPkg. But only a subset of the features have been enabled for UEFI
+use case. Python files that had to be modified for EDK II are in the AppPkg\Applications\Python\Python-3.6.8\PyMod-3.6.8 directory.
+These files need to be copied into the corresponding directories
+within the Python-3.6.8 directory before Python can be built. This
+can be achieved by running the srcprep.py available under AppPkg\Applications\Python\Python-3.6.8.


BUILDING
--
2.32.0.windows.2



[PATCH 1/1] OvmfPkg/VmgExitLib: Fix uninitialized variable warning

Brijesh Singh
 

The XCODE5 reported the below warning

OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1895:12: note: uninitialized use occurs here
Compacted
^^^^^^^^^

Initialize the 'Compacted' variable to fix the warning.

Fixes: d2b998fbdca4 (OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values)
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Rebecca Cran <rebecca@...>
Cc: Michael Roth <Michael.Roth@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index a40a31f7c275..ff367411cc59 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -1872,6 +1872,7 @@ GetCpuidFw (
UINT32 XSaveSize;

XssMsr.Uint64 = 0;
+ Compacted = FALSE;
if (EcxIn == 1) {
/*
* The PPR and APM aren't clear on what size should be encoded in
--
2.25.1


Re: Building OvmfPkgX64.dsc with XCODE5 (Apple clang 12.0.5) fails in VmgExitLib

Brijesh Singh
 

On 12/13/21 11:48 AM, Rebecca Cran wrote:
I tried building OvmfPkg/OvmfPkgX64.dsc with XCODE5 (with Apple Clang 12.0.5 from XCode 13.1) and it failed with the following error:
/Users/bcran/src/uefi/edk2/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1875:9: error: variable 'Compacted' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
    if (EcxIn == 1) {
        ^~~~~~~~~~
/Users/bcran/src/uefi/edk2/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1895:12: note: uninitialized use occurs here
           Compacted
           ^~~~~~~~~
/Users/bcran/src/uefi/edk2/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1875:5: note: remove the 'if' if its condition is always true
    if (EcxIn == 1) {
    ^~~~~~~~~~~~~~~~
/Users/bcran/src/uefi/edk2/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c:1871:37: note: initialize the variable 'Compacted' to silence this warning
    BOOLEAN                Compacted;
                                    ^
                                     = '\0'
Thanks for reporting, I will submit a patch sooner to resolve this warning.

-Brijesh


Re: [edk2-platforms PATCH] Platform/RaspberryPi: Fix miniuart base address and length

Andrei Warkentin
 

If I understand correctly, you want to describe the UART at 0x00215000 to be at 0x00215040.

This will break SPCR and DBG2 - so that's a regression for Windows, ESXi and possibly the NetBSDs.

I guess that's a NAK unless I misunderstood something.

From: Ard Biesheuvel <ardb@...>
Sent: Monday, December 13, 2021 9:14 AM
To: Adrien Thierry <athierry@...>; Andrei Warkentin <awarkentin@...>; Pete Batard <pete@...>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>
Subject: Re: [edk2-platforms PATCH] Platform/RaspberryPi: Fix miniuart base address and length
 
On Mon, 13 Dec 2021 at 15:54, Adrien Thierry <athierry@...> wrote:
>
> Hi Ard, Leif, Pete
>
> Do you have any feedback on this patch ?
>

No objections from me but I'd like an ack from someone else as well.


[PATCH v3 2/2] ArmPkg: Update SMC calls to use the new ArmCallSmc0/1/2/3 functions

Rebecca Cran <rebecca@...>
 

New SMC helper functions have been added to reduce the amount of
template code. Update ArmSmcPsciResetSystemLib and
Smbios/ProcessorSubClassDxe to use them.

Signed-off-by: Rebecca Cran <rebecca@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c | 10 +----
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c | 40 ++++++++------------
2 files changed, 18 insertions(+), 32 deletions(-)

diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
index 6688fca37a8b..af6738459e43 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c
@@ -31,11 +31,8 @@ ResetCold (
VOID
)
{
- ARM_SMC_ARGS ArmSmcArgs;
-
// Send a PSCI 0.2 SYSTEM_RESET command
- ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_RESET;
- ArmCallSmc (&ArmSmcArgs);
+ ArmCallSmc0 (ARM_SMC_ID_PSCI_SYSTEM_RESET, NULL, NULL, NULL);
}

/**
@@ -66,11 +63,8 @@ ResetShutdown (
VOID
)
{
- ARM_SMC_ARGS ArmSmcArgs;
-
// Send a PSCI 0.2 SYSTEM_OFF command
- ArmSmcArgs.Arg0 = ARM_SMC_ID_PSCI_SYSTEM_OFF;
- ArmCallSmc (&ArmSmcArgs);
+ ArmCallSmc0 (ARM_SMC_ID_PSCI_SYSTEM_OFF, NULL, NULL, NULL);
}

/**
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
index 7a8c3ca56784..e0010a40e489 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
@@ -88,22 +88,18 @@ HasSmcArm64SocId (
VOID
)
{
- ARM_SMC_ARGS Args;
- INT32 SmcCallStatus;
- BOOLEAN Arm64SocIdSupported;
+ INT32 SmcCallStatus;
+ BOOLEAN Arm64SocIdSupported;
+ UINTN SmcParam;

Arm64SocIdSupported = FALSE;

- Args.Arg0 = SMCCC_VERSION;
- ArmCallSmc (&Args);
- SmcCallStatus = (INT32)Args.Arg0;
+ SmcCallStatus = ArmCallSmc0 (SMCCC_VERSION, NULL, NULL, NULL);

if ((SmcCallStatus < 0) || ((SmcCallStatus >> 16) >= 1)) {
- Args.Arg0 = SMCCC_ARCH_FEATURES;
- Args.Arg1 = SMCCC_ARCH_SOC_ID;
- ArmCallSmc (&Args);
-
- if (Args.Arg0 >= 0) {
+ SmcParam = SMCCC_ARCH_SOC_ID;
+ SmcCallStatus = ArmCallSmc1 (SMCCC_ARCH_FEATURES, &SmcParam, NULL, NULL);
+ if (SmcCallStatus >= 0) {
Arm64SocIdSupported = TRUE;
}
}
@@ -125,30 +121,26 @@ SmbiosGetSmcArm64SocId (
OUT INT32 *SocRevision
)
{
- ARM_SMC_ARGS Args;
- INT32 SmcCallStatus;
- EFI_STATUS Status;
+ INT32 SmcCallStatus;
+ EFI_STATUS Status;
+ UINTN SmcParam;

Status = EFI_SUCCESS;

- Args.Arg0 = SMCCC_ARCH_SOC_ID;
- Args.Arg1 = 0;
- ArmCallSmc (&Args);
- SmcCallStatus = (INT32)Args.Arg0;
+ SmcParam = 0;
+ SmcCallStatus = ArmCallSmc1 (SMCCC_ARCH_SOC_ID, &SmcParam, NULL, NULL);

if (SmcCallStatus >= 0) {
- *Jep106Code = (INT32)Args.Arg0;
+ *Jep106Code = (INT32)SmcParam;
} else {
Status = EFI_UNSUPPORTED;
}

- Args.Arg0 = SMCCC_ARCH_SOC_ID;
- Args.Arg1 = 1;
- ArmCallSmc (&Args);
- SmcCallStatus = (INT32)Args.Arg0;
+ SmcParam = 1;
+ SmcCallStatus = ArmCallSmc1 (SMCCC_ARCH_SOC_ID, &SmcParam, NULL, NULL);

if (SmcCallStatus >= 0) {
- *SocRevision = (INT32)Args.Arg0;
+ *SocRevision = (INT32)SmcParam;
} else {
Status = EFI_UNSUPPORTED;
}
--
2.31.1


[PATCH v3 1/2] ArmPkg: Add SMC helper functions

Rebecca Cran <rebecca@...>
 

Add functions ArmCallSmc0/1/2/3 to do SMC calls with 0, 1, 2 or 3
arguments.
The functions return up to 3 values.

Signed-off-by: Rebecca Cran <rebecca@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
ArmPkg/Include/Library/ArmSmcLib.h | 73 +++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmc.c | 129 ++++++++++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 3 +
ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c | 85 +++++++++++++
4 files changed, 290 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmSmcLib.h b/ArmPkg/Include/Library/ArmSmcLib.h
index f5b45f0a8cdf..beef0175c35c 100644
--- a/ArmPkg/Include/Library/ArmSmcLib.h
+++ b/ArmPkg/Include/Library/ArmSmcLib.h
@@ -1,5 +1,6 @@
/** @file
*
+* Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
* Copyright (c) 2012-2014, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -37,4 +38,76 @@ ArmCallSmc (
IN OUT ARM_SMC_ARGS *Args
);

+/** Trigger an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ IN OUT UINTN *Arg3 OPTIONAL
+ );
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ );
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ );
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ );
+
#endif // ARM_SMC_LIB_H_
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmc.c b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
new file mode 100644
index 000000000000..254507e21916
--- /dev/null
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmc.c
@@ -0,0 +1,129 @@
+/** @file
+ SMC helper functions.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseMemoryLib.h>
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ IN OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ ARM_SMC_ARGS Args;
+ UINTN ErrorCode;
+
+ ZeroMem (&Args, sizeof (ARM_SMC_ARGS));
+
+ Args.Arg0 = Function;
+
+ if (Arg1 != NULL) {
+ Args.Arg1 = *Arg1;
+ }
+
+ if (Arg2 != NULL) {
+ Args.Arg2 = *Arg2;
+ }
+
+ if (Arg3 != NULL) {
+ Args.Arg3 = *Arg3;
+ }
+
+ ArmCallSmc (&Args);
+
+ ErrorCode = Args.Arg0;
+
+ if (Arg1 != NULL) {
+ *Arg1 = Args.Arg1;
+ }
+
+ if (Arg2 != NULL) {
+ *Arg2 = Args.Arg2;
+ }
+
+ if (Arg3 != NULL) {
+ *Arg3 = Args.Arg3;
+ }
+
+ return ErrorCode;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return ArmCallSmc3 (Function, Arg1, Arg2, Arg3);
+}
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
index 4f4b09f4528a..a89f9203fb7e 100644
--- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
@@ -20,6 +20,9 @@
[Sources.AARCH64]
AArch64/ArmSmc.S

+[Sources]
+ ArmSmc.c
+
[Packages]
MdePkg/MdePkg.dec
ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
index 3c1adef8ebe6..28514e43c2de 100644
--- a/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
+++ b/ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c
@@ -1,4 +1,5 @@
//
+// Copyright (c) 2021, NUVIA Inc. All rights reserved.
// Copyright (c) 2016, Linaro Limited. All rights reserved.
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -7,6 +8,7 @@

#include <Base.h>
#include <Library/ArmSmcLib.h>
+#include <IndustryStandard/ArmStdSmc.h>

VOID
ArmCallSmc (
@@ -14,3 +16,86 @@ ArmCallSmc (
)
{
}
+
+/** Triggers an SMC call with 3 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Argument/result.
+
+ @return The SMC error code.
+**/
+UINTN
+ArmCallSmc3 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ IN OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 2 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Argument/result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc2 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ IN OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 1 argument.
+
+ @param Function The SMC function.
+ @param Arg1 Argument/result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc1 (
+ IN UINTN Function,
+ IN OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
+
+/** Trigger an SMC call with 0 arguments.
+
+ @param Function The SMC function.
+ @param Arg1 Result.
+ @param Arg2 Result.
+ @param Arg3 Result.
+
+ @return The SMC error code.
+
+**/
+UINTN
+ArmCallSmc0 (
+ IN UINTN Function,
+ OUT UINTN *Arg1 OPTIONAL,
+ OUT UINTN *Arg2 OPTIONAL,
+ OUT UINTN *Arg3 OPTIONAL
+ )
+{
+ return SMC_ARCH_CALL_NOT_SUPPORTED;
+}
--
2.31.1


[PATCH v3 0/2] ArmPkg: Add SMC helper functions

Rebecca Cran <rebecca@...>
 

To reduce the amount of template code, introduce SMC helper
functions. Update ArmSmcPsciResetSystemLib and Universal/Smbios to use
them.

Changes from v2 to v3:

o Fixed code style issues with Uncrustify.
o Fixed patch 2/2 subject.
o Added OPTIONAL tag to functions.
o Added ZeroMem call to initialize Args.

GitHub Test PR: https://github.com/tianocore/edk2/pull/2300

Rebecca Cran (2):
ArmPkg: Add SMC helper functions
ArmPkg: Update SMC calls to use the new ArmCallSmc0/1/2/3 functions

ArmPkg/Include/Library/ArmSmcLib.h | 73 +++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmc.c | 129 ++++++++++++++++++++
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 3 +
ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c | 85 +++++++++++++
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c | 10 +-
ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c | 40 +++---
6 files changed, 308 insertions(+), 32 deletions(-)
create mode 100644 ArmPkg/Library/ArmSmcLib/ArmSmc.c

--
2.31.1


Re: [PATCH v1 0/2] MM communicate functionality in variable policy

Kun Qin
 

Hi ArmPkg and MdeModulePkg maintainers,

Now that the hard freeze is lifted, could you please provide some feedback on these patches when you have a chance?

Thanks in advance.

Regards,
Kun

On 12/06/2021 10:41, Ard Biesheuvel wrote:
On Mon, 6 Dec 2021 at 19:35, Kun Qin <kuqin12@...> wrote:

Hi ArmPkg and MdeModulePkg maintainers,

It has been a week since the patches were sent. Could you please review
the changes and let me know if there is any feedback? Any input is
appreciated.
As far as I know, we are still in hard freeze for the upcoming stable tag.


On 11/29/2021 16:39, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Currently, setups with variable policy operations used together with MM
communicate from ArmPkg could fail with `EFI_INVALID_PARAMETER`. This was
due to the errors from 2 following aspects:

1. For variable policy implementations in MdeModulePkg, the DXE runtime
agent would communicate to MM to disable, register or query policies.
However, during these operations, the MessageLength calculation is
including MM communicate header. This could lead to MM agent read data
across the given buffer boundary and/or trigger other errors.

2. On the other hand, current MM communicate routine from ArmPkg would
fail the function if the input message length does not equal to input
buffer size.

As defined in PI specification, the `CommSize`, when as input, should
stand for "The size of the data buffer being passed in", which would mean
the maximal number of bytes `CommBuffer` can hold. In turn, the value of
this input parameter can be used for MM handlers to determine whether the
output data is too large to fit in this buffer. Enforcing the incoming
buffer to hold exactly the number of used bytes mismatches with the PI
spec description.

This change fix MessageLength field calculation from variable policy and
updated input argument inspections from MM communicate routine in ArmPkg
to match PI spec descriptions.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/mm_communicate_check

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Kun Qin (2):
MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy Message
Length
ArmPkg: MmCommunicationDxe: Update MM communicate input arguments
checks

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 44 ++++++++++++--------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | 10 ++---
2 files changed, 32 insertions(+), 22 deletions(-)


Re: Adding *.uncrustify_plugin to .gitignore?

Michael Kubacki
 

Thank you for confirming. I do not expect for them to be left around. I suppose we can leave it as-is for now, so users are aware in case the files remain (for any reason) in their workspace.

On 12/13/2021 10:25 AM, Rebecca Cran wrote:
Sorry, it was operator error: those were obviously left-overs from when I was figuring out how to run Uncrustify manually. Now when I run e.g. "git ls-files ArmPkg*.c | ./.pytool/Plugin/Uncrustify/mu-uncrustify-release_extdep/Linux-x86/uncrustify -c ./.pytool/Plugin/UncrustifyCheck/uncrustify.cfg -F - --replace --no-backup --if-changed" I don't get those files generated.

11861 - 11880 of 96543