From: "Zeng, Star" <star.zeng@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Zhu, Yonghong" <yonghong.zhu@intel.com>,
"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table
Date: Mon, 23 Apr 2018 09:38:46 +0000 [thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BAD7DB9@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BAD7D3B@shsmsx102.ccr.corp.intel.com>
I also built VS2015/GCC49 (tool chain in my local), and checked ecc result.
Thanks,
Star
-----Original Message-----
From: Zeng, Star
Sent: Monday, April 23, 2018 4:55 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table
Yonghong helped run regression test for the cases without FIT table.
I created below cases with FIT table, checked the boot log and confirmed the signature is correct after microcode update.
1. Update in situ.
2. Empty FIT microcode entry with enough space.
3. Unused FIT microcode entry with enough space.
Thanks,
Star
-----Original Message-----
From: Yao, Jiewen
Sent: Monday, April 23, 2018 4:49 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table
Thanks for the update.
Would you please share what unit test has been done for this patch?
Thank you
Yao Jiewen
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, April 23, 2018 2:57 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table
>
> It is the second step for
> https://bugzilla.tianocore.org/show_bug.cgi?id=540.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> .../Capsule/MicrocodeUpdateDxe/MicrocodeFmp.c | 184 ++++++++++-
> .../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c | 363
> +++++++++++++++++++--
> .../Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.h | 12 +-
> .../MicrocodeUpdateDxe/MicrocodeUpdateDxe.inf | 3 +-
> 4 files changed, 535 insertions(+), 27 deletions(-)
>
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeFmp.c
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeFmp.c
> index ebde93a91eb3..8276aaa0279b 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeFmp.c
> +++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeFmp.
> +++ c
> @@ -272,7 +272,7 @@ FmpSetImage (
> }
>
> Status = MicrocodeWrite(MicrocodeFmpPrivate, (VOID *)Image,
> ImageSize, &MicrocodeFmpPrivate->LastAttempt.LastAttemptVersion,
> &MicrocodeFmpPrivate->LastAttempt.LastAttemptStatus, AbortReason);
> - DEBUG((DEBUG_INFO, "SetImage - LastAttemp Version - 0x%x, State -
> 0x%x\n", MicrocodeFmpPrivate->LastAttempt.LastAttemptVersion,
> MicrocodeFmpPrivate->LastAttempt.LastAttemptStatus));
> + DEBUG((DEBUG_INFO, "SetImage - LastAttempt Version - 0x%x, Status -
> 0x%x\n", MicrocodeFmpPrivate->LastAttempt.LastAttemptVersion,
> MicrocodeFmpPrivate->LastAttempt.LastAttemptStatus));
> VarStatus = gRT->SetVariable(
> MICROCODE_FMP_LAST_ATTEMPT_VARIABLE_NAME,
> &gEfiCallerIdGuid, @@ -280,7 +280,7 @@
> FmpSetImage (
> sizeof(MicrocodeFmpPrivate->LastAttempt),
> &MicrocodeFmpPrivate->LastAttempt
> );
> - DEBUG((DEBUG_INFO, "SetLastAttemp - %r\n", VarStatus));
> + DEBUG((DEBUG_INFO, "SetLastAttempt - %r\n", VarStatus));
>
> if (!EFI_ERROR(Status)) {
> InitializeMicrocodeDescriptor(MicrocodeFmpPrivate);
> @@ -415,6 +415,159 @@ FmpSetPackageInfo ( }
>
> /**
> + Sort FIT microcode entries based upon MicrocodeEntryPoint, from low
> + to
> high.
> +
> + @param[in] MicrocodeFmpPrivate private data structure to be initialized.
> +
> +**/
> +VOID
> +SortFitMicrocodeInfo (
> + IN MICROCODE_FMP_PRIVATE_DATA *MicrocodeFmpPrivate
> + )
> +{
> + FIT_MICROCODE_INFO *FitMicrocodeEntry;
> + FIT_MICROCODE_INFO *NextFitMicrocodeEntry;
> + FIT_MICROCODE_INFO TempFitMicrocodeEntry;
> + FIT_MICROCODE_INFO *FitMicrocodeEntryEnd;
> +
> + FitMicrocodeEntry = MicrocodeFmpPrivate->FitMicrocodeInfo;
> + NextFitMicrocodeEntry = FitMicrocodeEntry + 1; FitMicrocodeEntryEnd
> + = MicrocodeFmpPrivate->FitMicrocodeInfo +
> MicrocodeFmpPrivate->FitMicrocodeEntryCount;
> + while (FitMicrocodeEntry < FitMicrocodeEntryEnd) {
> + while (NextFitMicrocodeEntry < FitMicrocodeEntryEnd) {
> + if (FitMicrocodeEntry->MicrocodeEntryPoint >
> NextFitMicrocodeEntry->MicrocodeEntryPoint) {
> + CopyMem (&TempFitMicrocodeEntry, FitMicrocodeEntry, sizeof
> (FIT_MICROCODE_INFO));
> + CopyMem (FitMicrocodeEntry, NextFitMicrocodeEntry, sizeof
> (FIT_MICROCODE_INFO));
> + CopyMem (NextFitMicrocodeEntry, &TempFitMicrocodeEntry,
> + sizeof
> (FIT_MICROCODE_INFO));
> + }
> +
> + NextFitMicrocodeEntry = NextFitMicrocodeEntry + 1;
> + }
> +
> + FitMicrocodeEntry = FitMicrocodeEntry + 1;
> + NextFitMicrocodeEntry = FitMicrocodeEntry + 1;
> + }
> +}
> +
> +/**
> + Initialize FIT microcode information.
> +
> + @param[in] MicrocodeFmpPrivate private data structure to be initialized.
> +
> +**/
> +VOID
> +InitializeFitMicrocodeInfo (
> + IN MICROCODE_FMP_PRIVATE_DATA *MicrocodeFmpPrivate
> + )
> +{
> + UINT64 FitPointer;
> + FIRMWARE_INTERFACE_TABLE_ENTRY *FitEntry;
> + UINT32 EntryNum;
> + UINT32 MicrocodeEntryNum;
> + UINT32 Index;
> + UINTN Address;
> + VOID *MicrocodePatchAddress;
> + UINTN MicrocodePatchRegionSize;
> + FIT_MICROCODE_INFO *FitMicrocodeInfo;
> + UINTN FitMicrocodeIndex;
> + MICROCODE_INFO *MicrocodeInfo;
> + UINTN MicrocodeIndex;
> +
> + if (MicrocodeFmpPrivate->FitMicrocodeInfo != NULL) {
> + FreePool (MicrocodeFmpPrivate->FitMicrocodeInfo);
> + MicrocodeFmpPrivate->FitMicrocodeInfo = NULL;
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount = 0; }
> +
> + FitPointer = *(UINT64 *) (UINTN) FIT_POINTER_ADDRESS; if
> + ((FitPointer == 0) ||
> + (FitPointer == 0xFFFFFFFFFFFFFFFF) ||
> + (FitPointer == 0xEEEEEEEEEEEEEEEE)) {
> + //
> + // No FIT table;
> + //
> + return;
> + }
> + FitEntry = (FIRMWARE_INTERFACE_TABLE_ENTRY *) (UINTN) FitPointer;
> + if ((FitEntry[0].Type != FIT_TYPE_00_HEADER) ||
> + (FitEntry[0].Address != FIT_TYPE_00_SIGNATURE)) {
> + //
> + // Invalid FIT table;
> + //
> + return;
> + }
> +
> + EntryNum = *(UINT32 *)(&FitEntry[0].Size[0]) & 0xFFFFFF;
> +
> + //
> + // Calculate microcode entry number.
> + //
> + MicrocodeEntryNum = 0;
> + for (Index = 0; Index < EntryNum; Index++) {
> + if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> + MicrocodeEntryNum++;
> + }
> + }
> + if (MicrocodeEntryNum == 0) {
> + //
> + // No FIT microcode entry.
> + //
> + return;
> + }
> +
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount = MicrocodeEntryNum;
> +
> + //
> + // Allocate buffer.
> + //
> + MicrocodeFmpPrivate->FitMicrocodeInfo = AllocateZeroPool
> (MicrocodeEntryNum * sizeof (FIT_MICROCODE_INFO));
> + ASSERT (MicrocodeFmpPrivate->FitMicrocodeInfo != NULL);
> +
> + MicrocodePatchAddress = MicrocodeFmpPrivate->MicrocodePatchAddress;
> + MicrocodePatchRegionSize =
> MicrocodeFmpPrivate->MicrocodePatchRegionSize;
> +
> + //
> + // Collect microcode entry info.
> + //
> + MicrocodeEntryNum = 0;
> + for (Index = 0; Index < EntryNum; Index++) {
> + if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> + Address = (UINTN) FitEntry[Index].Address;
> + ASSERT ((Address >= (UINTN) MicrocodePatchAddress) &&
> + (Address < ((UINTN) MicrocodePatchAddress +
> MicrocodePatchRegionSize)));
> + FitMicrocodeInfo =
> &MicrocodeFmpPrivate->FitMicrocodeInfo[MicrocodeEntryNum];
> + FitMicrocodeInfo->MicrocodeEntryPoint = (CPU_MICROCODE_HEADER
> *) Address;
> + if ((*(UINT32 *) Address) == 0xFFFFFFFF) {
> + //
> + // It is the empty slot as long as the first dword is 0xFFFF_FFFF.
> + //
> + FitMicrocodeInfo->Empty = TRUE;
> + } else {
> + FitMicrocodeInfo->Empty = FALSE;
> + }
> + MicrocodeEntryNum++;
> + }
> + }
> +
> + //
> + // Every microcode should have a FIT microcode entry.
> + //
> + for (MicrocodeIndex = 0; MicrocodeIndex <
> MicrocodeFmpPrivate->DescriptorCount; MicrocodeIndex++) {
> + MicrocodeInfo =
> &MicrocodeFmpPrivate->MicrocodeInfo[MicrocodeIndex];
> + for (FitMicrocodeIndex = 0; FitMicrocodeIndex <
> MicrocodeFmpPrivate->FitMicrocodeEntryCount; FitMicrocodeIndex++) {
> + FitMicrocodeInfo =
> &MicrocodeFmpPrivate->FitMicrocodeInfo[FitMicrocodeIndex];
> + if (MicrocodeInfo->MicrocodeEntryPoint ==
> FitMicrocodeInfo->MicrocodeEntryPoint) {
> + FitMicrocodeInfo->InUse = MicrocodeInfo->InUse;
> + break;
> + }
> + }
> + ASSERT (FitMicrocodeIndex <
> MicrocodeFmpPrivate->FitMicrocodeEntryCount);
> + }
> +
> + SortFitMicrocodeInfo (MicrocodeFmpPrivate); }
> +
> +/**
> Initialize Processor Microcode Index.
>
> @param[in] MicrocodeFmpPrivate private data structure to be initialized.
> @@ -496,6 +649,7 @@ InitializeMicrocodeDescriptor (
> if (MicrocodeFmpPrivate->MicrocodeInfo == NULL) {
> MicrocodeFmpPrivate->MicrocodeInfo =
> AllocateZeroPool(MicrocodeFmpPrivate->DescriptorCount *
> sizeof(MICROCODE_INFO));
> if (MicrocodeFmpPrivate->MicrocodeInfo == NULL) {
> + FreePool (MicrocodeFmpPrivate->ImageDescriptor);
> return EFI_OUT_OF_RESOURCES;
> }
> }
> @@ -505,6 +659,8 @@ InitializeMicrocodeDescriptor (
>
> InitializedProcessorMicrocodeIndex (MicrocodeFmpPrivate);
>
> + InitializeFitMicrocodeInfo (MicrocodeFmpPrivate);
> +
> return EFI_SUCCESS;
> }
>
> @@ -513,7 +669,7 @@ InitializeMicrocodeDescriptor (
>
> @param[in] MicrocodeFmpPrivate private data structure to be initialized.
>
> - @return EFI_SUCCESS private data is initialized.
> + @return EFI_SUCCESS Processor information is initialized.
> **/
> EFI_STATUS
> InitializeProcessorInfo (
> @@ -583,6 +739,7 @@ DumpPrivateInfo (
> PROCESSOR_INFO *ProcessorInfo;
> MICROCODE_INFO *MicrocodeInfo;
> EFI_FIRMWARE_IMAGE_DESCRIPTOR *ImageDescriptor;
> + FIT_MICROCODE_INFO *FitMicrocodeInfo;
>
> DEBUG ((DEBUG_INFO, "ProcessorInfo:\n"));
> DEBUG ((DEBUG_INFO, " ProcessorCount - 0x%x\n",
> MicrocodeFmpPrivate->ProcessorCount));
> @@ -635,6 +792,22 @@ DumpPrivateInfo (
> DEBUG((DEBUG_VERBOSE, " LastAttemptStatus - 0x%x\n",
> ImageDescriptor[Index].LastAttemptStatus));
> DEBUG((DEBUG_VERBOSE, " HardwareInstance -
> 0x%lx\n", ImageDescriptor[Index].HardwareInstance));
> }
> +
> + if (MicrocodeFmpPrivate->FitMicrocodeInfo != NULL) {
> + DEBUG ((DEBUG_INFO, "FitMicrocodeInfo:\n"));
> + FitMicrocodeInfo = MicrocodeFmpPrivate->FitMicrocodeInfo;
> + DEBUG ((DEBUG_INFO, " FitMicrocodeEntryCount - 0x%x\n",
> MicrocodeFmpPrivate->FitMicrocodeEntryCount));
> + for (Index = 0; Index <
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount;
> Index++) {
> + DEBUG ((
> + DEBUG_INFO,
> + " FitMicrocodeInfo[0x%x] - 0x%08x, (0x%x, 0x%x)\n",
> + Index,
> + FitMicrocodeInfo[Index].MicrocodeEntryPoint,
> + FitMicrocodeInfo[Index].InUse,
> + FitMicrocodeInfo[Index].Empty
> + ));
> + }
> + }
> }
>
> /**
> @@ -671,8 +844,8 @@ InitializePrivateData (
> &VarSize,
> &MicrocodeFmpPrivate->LastAttempt
> );
> - DEBUG((DEBUG_INFO, "GetLastAttemp - %r\n", VarStatus));
> - DEBUG((DEBUG_INFO, "GetLastAttemp Version - 0x%x, State - 0x%x\n",
> MicrocodeFmpPrivate->LastAttempt.LastAttemptVersion,
> MicrocodeFmpPrivate->LastAttempt.LastAttemptStatus));
> + DEBUG((DEBUG_INFO, "GetLastAttempt - %r\n", VarStatus));
> + DEBUG((DEBUG_INFO, "GetLastAttempt Version - 0x%x, State - 0x%x\n",
> MicrocodeFmpPrivate->LastAttempt.LastAttemptVersion,
> MicrocodeFmpPrivate->LastAttempt.LastAttemptStatus));
>
> Result =
> GetMicrocodeRegion(&MicrocodeFmpPrivate->MicrocodePatchAddress,
> &MicrocodeFmpPrivate->MicrocodePatchRegionSize);
> if (!Result) {
> @@ -688,6 +861,7 @@ InitializePrivateData (
>
> Status = InitializeMicrocodeDescriptor(MicrocodeFmpPrivate);
> if (EFI_ERROR(Status)) {
> + FreePool (MicrocodeFmpPrivate->ProcessorInfo);
> DEBUG((DEBUG_ERROR, "InitializeMicrocodeDescriptor - %r\n", Status));
> return Status;
> }
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> index 2cb0adbc44d5..9098712c2fc8 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
> @@ -368,7 +368,7 @@ GetMatchedProcessor (
> On output, the index of
> target CPU which matches the Microcode.
>
> @retval EFI_SUCCESS The Microcode image passes
> verification.
> - @retval EFI_VOLUME_CORRUPTED The Microcode image is corrupt.
> + @retval EFI_VOLUME_CORRUPTED The Microcode image is
> corrupted.
> @retval EFI_INCOMPATIBLE_VERSION The Microcode image version is
> incorrect.
> @retval EFI_UNSUPPORTED The Microcode ProcessorSignature
> or ProcessorFlags is incorrect.
> @retval EFI_SECURITY_VIOLATION The Microcode image fails to load.
> @@ -550,7 +550,7 @@ VerifyMicrocode (
> }
> *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
> if (AbortReason != NULL) {
> - *AbortReason =
> AllocateCopyPool(sizeof(L"UnsupportedProcessSignature/ProcessorFlags")
> , L"UnsupportedProcessSignature/ProcessorFlags");
> + *AbortReason =
> AllocateCopyPool(sizeof(L"UnsupportedProcessorSignature/ProcessorFlags
> "), L"UnsupportedProcessorSignature/ProcessorFlags");
> }
> return EFI_UNSUPPORTED;
> }
> @@ -623,6 +623,124 @@ GetNextMicrocode ( }
>
> /**
> + Get next FIT Microcode entrypoint.
> +
> + @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> + @param[in] MicrocodeEntryPoint Current Microcode entrypoint
> +
> + @return next FIT Microcode entrypoint.
> +**/
> +CPU_MICROCODE_HEADER *
> +GetNextFitMicrocode (
> + IN MICROCODE_FMP_PRIVATE_DATA
> *MicrocodeFmpPrivate,
> + IN CPU_MICROCODE_HEADER *MicrocodeEntryPoint
> + )
> +{
> + UINTN Index;
> +
> + for (Index = 0; Index <
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount;
> Index++) {
> + if (MicrocodeEntryPoint ==
> MicrocodeFmpPrivate->FitMicrocodeInfo[Index].MicrocodeEntryPoint) {
> + if (Index == (UINTN)
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount -
> 1) {
> + // it is last one
> + return NULL;
> + } else {
> + // return next one
> + return MicrocodeFmpPrivate->FitMicrocodeInfo[Index +
> 1].MicrocodeEntryPoint;
> + }
> + }
> + }
> +
> + ASSERT(FALSE);
> + return NULL;
> +}
> +
> +/**
> + Find empty FIT Microcode entrypoint.
> +
> + @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> + @param[in] ImageSize The size of Microcode image
> buffer in bytes.
> + @param[out] AvailableSize Available size of the empty FIT
> Microcode entrypoint.
> +
> + @return Empty FIT Microcode entrypoint.
> +**/
> +CPU_MICROCODE_HEADER *
> +FindEmptyFitMicrocode (
> + IN MICROCODE_FMP_PRIVATE_DATA
> *MicrocodeFmpPrivate,
> + IN UINTN ImageSize,
> + OUT UINTN *AvailableSize
> + )
> +{
> + UINTN Index;
> + CPU_MICROCODE_HEADER *MicrocodeEntryPoint;
> + CPU_MICROCODE_HEADER
> *NextMicrocodeEntryPoint;
> + VOID *MicrocodePatchAddress;
> + UINTN MicrocodePatchRegionSize;
> +
> + MicrocodePatchAddress = MicrocodeFmpPrivate->MicrocodePatchAddress;
> + MicrocodePatchRegionSize =
> MicrocodeFmpPrivate->MicrocodePatchRegionSize;
> +
> + for (Index = 0; Index <
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount;
> Index++) {
> + if (MicrocodeFmpPrivate->FitMicrocodeInfo[Index].Empty) {
> + MicrocodeEntryPoint =
> MicrocodeFmpPrivate->FitMicrocodeInfo[Index].MicrocodeEntryPoint;
> + NextMicrocodeEntryPoint = GetNextFitMicrocode
> (MicrocodeFmpPrivate, MicrocodeEntryPoint);
> + if (NextMicrocodeEntryPoint != NULL) {
> + *AvailableSize = (UINTN) NextMicrocodeEntryPoint - (UINTN)
> MicrocodeEntryPoint;
> + } else {
> + *AvailableSize = (UINTN) MicrocodePatchAddress +
> MicrocodePatchRegionSize - (UINTN) MicrocodeEntryPoint;
> + }
> + if (*AvailableSize >= ImageSize) {
> + return MicrocodeEntryPoint;
> + }
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + Find unused FIT Microcode entrypoint.
> +
> + @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> + @param[in] ImageSize The size of Microcode image
> buffer in bytes.
> + @param[out] AvailableSize Available size of the unused FIT
> Microcode entrypoint.
> +
> + @return Unused FIT Microcode entrypoint.
> +**/
> +CPU_MICROCODE_HEADER *
> +FindUnusedFitMicrocode (
> + IN MICROCODE_FMP_PRIVATE_DATA
> *MicrocodeFmpPrivate,
> + IN UINTN ImageSize,
> + OUT UINTN *AvailableSize
> + )
> +{
> + UINTN Index;
> + CPU_MICROCODE_HEADER *MicrocodeEntryPoint;
> + CPU_MICROCODE_HEADER
> *NextMicrocodeEntryPoint;
> + VOID *MicrocodePatchAddress;
> + UINTN MicrocodePatchRegionSize;
> +
> + MicrocodePatchAddress = MicrocodeFmpPrivate->MicrocodePatchAddress;
> + MicrocodePatchRegionSize =
> MicrocodeFmpPrivate->MicrocodePatchRegionSize;
> +
> + for (Index = 0; Index <
> + MicrocodeFmpPrivate->FitMicrocodeEntryCount;
> Index++) {
> + if (!MicrocodeFmpPrivate->FitMicrocodeInfo[Index].InUse) {
> + MicrocodeEntryPoint =
> MicrocodeFmpPrivate->FitMicrocodeInfo[Index].MicrocodeEntryPoint;
> + NextMicrocodeEntryPoint = GetNextFitMicrocode
> (MicrocodeFmpPrivate, MicrocodeEntryPoint);
> + if (NextMicrocodeEntryPoint != NULL) {
> + *AvailableSize = (UINTN) NextMicrocodeEntryPoint - (UINTN)
> MicrocodeEntryPoint;
> + } else {
> + *AvailableSize = (UINTN) MicrocodePatchAddress +
> MicrocodePatchRegionSize - (UINTN) MicrocodeEntryPoint;
> + }
> + if (*AvailableSize >= ImageSize) {
> + return MicrocodeEntryPoint;
> + }
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> Get current Microcode used region size.
>
> @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> @@ -666,7 +784,7 @@ UpdateMicrocode (
>
> DEBUG((DEBUG_INFO, "PlatformUpdate:"));
> DEBUG((DEBUG_INFO, " Address - 0x%lx,", Address));
> - DEBUG((DEBUG_INFO, " Legnth - 0x%x\n", ImageSize));
> + DEBUG((DEBUG_INFO, " Length - 0x%x\n", ImageSize));
>
> Status = MicrocodeFlashWrite (
> Address,
> @@ -682,6 +800,201 @@ UpdateMicrocode ( }
>
> /**
> + Update Microcode flash region with FIT.
> +
> + @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> + @param[in] TargetMicrocodeEntryPoint Target Microcode entrypoint
> + to
> be updated
> + @param[in] Image The Microcode image buffer.
> + @param[in] ImageSize The size of Microcode image
> buffer in bytes.
> + @param[out] LastAttemptStatus The last attempt status, which
> will be recorded in ESRT and FMP EFI_FIRMWARE_IMAGE_DESCRIPTOR.
> +
> + @retval EFI_SUCCESS The Microcode image is written.
> + @retval EFI_WRITE_PROTECTED The flash device is read only.
> +**/
> +EFI_STATUS
> +UpdateMicrocodeFlashRegionWithFit (
> + IN MICROCODE_FMP_PRIVATE_DATA
> *MicrocodeFmpPrivate,
> + IN CPU_MICROCODE_HEADER
> *TargetMicrocodeEntryPoint,
> + IN VOID *Image,
> + IN UINTN ImageSize,
> + OUT UINT32 *LastAttemptStatus
> + )
> +{
> + VOID *MicrocodePatchAddress;
> + UINTN MicrocodePatchRegionSize;
> + UINTN TargetTotalSize;
> + EFI_STATUS Status;
> + VOID
> *MicrocodePatchScratchBuffer;
> + UINT8 *ScratchBufferPtr;
> + UINTN ScratchBufferSize;
> + UINTN RestSize;
> + UINTN AvailableSize;
> + VOID *NextMicrocodeEntryPoint;
> + VOID *EmptyFitMicrocodeEntry;
> + VOID *UnusedFitMicrocodeEntry;
> +
> + DEBUG((DEBUG_INFO, "UpdateMicrocodeFlashRegionWithFit: Image -
> + 0x%x,
> size - 0x%x\n", Image, ImageSize));
> +
> + MicrocodePatchAddress = MicrocodeFmpPrivate->MicrocodePatchAddress;
> + MicrocodePatchRegionSize =
> MicrocodeFmpPrivate->MicrocodePatchRegionSize;
> +
> + MicrocodePatchScratchBuffer = AllocateZeroPool
> (MicrocodePatchRegionSize);
> + if (MicrocodePatchScratchBuffer == NULL) {
> + DEBUG((DEBUG_ERROR, "Fail to allocate Microcode Scratch buffer\n"));
> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_INSUFFICIENT_RESOURCES;
> + return EFI_OUT_OF_RESOURCES;
> + }
> + ScratchBufferPtr = MicrocodePatchScratchBuffer; ScratchBufferSize
> + = 0;
> +
> + //
> + // Target data collection
> + //
> + TargetTotalSize = 0;
> + AvailableSize = 0;
> + if (TargetMicrocodeEntryPoint != NULL) {
> + if (TargetMicrocodeEntryPoint->DataSize == 0) {
> + TargetTotalSize = 2048;
> + } else {
> + TargetTotalSize = TargetMicrocodeEntryPoint->TotalSize;
> + }
> + DEBUG((DEBUG_INFO, " TargetTotalSize - 0x%x\n", TargetTotalSize));
> + NextMicrocodeEntryPoint = GetNextFitMicrocode
> + (MicrocodeFmpPrivate,
> TargetMicrocodeEntryPoint);
> + DEBUG((DEBUG_INFO, " NextMicrocodeEntryPoint - 0x%x\n",
> NextMicrocodeEntryPoint));
> + if (NextMicrocodeEntryPoint != NULL) {
> + ASSERT ((UINTN) NextMicrocodeEntryPoint >= ((UINTN)
> TargetMicrocodeEntryPoint + TargetTotalSize));
> + AvailableSize = (UINTN) NextMicrocodeEntryPoint - (UINTN)
> TargetMicrocodeEntryPoint;
> + } else {
> + AvailableSize = (UINTN) MicrocodePatchAddress +
> MicrocodePatchRegionSize - (UINTN) TargetMicrocodeEntryPoint;
> + }
> + DEBUG((DEBUG_INFO, " AvailableSize - 0x%x\n", AvailableSize));
> + ASSERT (AvailableSize >= TargetTotalSize); } // // Total Size
> + means the Microcode size.
> + // Available Size means the Microcode size plus the pad till (1)
> + next
> Microcode or (2) the end.
> + //
> + // (1)
> + // +------+-----------+-----+------+===================+
> + // | MCU1 | Microcode | PAD | MCU2 | Empty |
> + // +------+-----------+-----+------+===================+
> + // | TotalSize |
> + // |<-AvailableSize->|
> + //
> + // (2)
> + // +------+-----------+===================+
> + // | MCU | Microcode | Empty |
> + // +------+-----------+===================+
> + // | TotalSize |
> + // |<- AvailableSize ->|
> + //
> +
> + //
> + // Update based on policy
> + //
> +
> + //
> + // 1. If there is enough space to update old one in situ, replace
> + old microcode
> in situ.
> + //
> + if (AvailableSize >= ImageSize) {
> + DEBUG((DEBUG_INFO, "Replace old microcode in situ\n"));
> + //
> + // +------+------------+------+===================+
> + // |Other | Old Image | ... | Empty |
> + // +------+------------+------+===================+
> + //
> + // +------+---------+--+------+===================+
> + // |Other |New Image|FF| ... | Empty |
> + // +------+---------+--+------+===================+
> + //
> + // 1.1. Copy new image
> + CopyMem (ScratchBufferPtr, Image, ImageSize);
> + ScratchBufferSize += ImageSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + // 1.2. Pad 0xFF
> + RestSize = AvailableSize - ImageSize;
> + if (RestSize > 0) {
> + SetMem (ScratchBufferPtr, RestSize, 0xFF);
> + ScratchBufferSize += RestSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + }
> + Status = UpdateMicrocode((UINTN)TargetMicrocodeEntryPoint,
> MicrocodePatchScratchBuffer, ScratchBufferSize, LastAttemptStatus);
> + return Status;
> + }
> +
> + //
> + // 2. If there is empty FIT microcode entry with enough space, use it.
> + //
> + EmptyFitMicrocodeEntry = FindEmptyFitMicrocode
> + (MicrocodeFmpPrivate,
> ImageSize, &AvailableSize);
> + if (EmptyFitMicrocodeEntry != NULL) {
> + DEBUG((DEBUG_INFO, "Use empty FIT microcode entry\n"));
> + // 2.1. Copy new image
> + CopyMem (ScratchBufferPtr, Image, ImageSize);
> + ScratchBufferSize += ImageSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + // 2.2. Pad 0xFF
> + RestSize = AvailableSize - ImageSize;
> + if (RestSize > 0) {
> + SetMem (ScratchBufferPtr, RestSize, 0xFF);
> + ScratchBufferSize += RestSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + }
> + Status = UpdateMicrocode ((UINTN) EmptyFitMicrocodeEntry,
> MicrocodePatchScratchBuffer, ScratchBufferSize, LastAttemptStatus);
> + if (!EFI_ERROR (Status) && (TargetMicrocodeEntryPoint != NULL)) {
> + //
> + // Empty old microcode.
> + //
> + ScratchBufferPtr = MicrocodePatchScratchBuffer;
> + SetMem (ScratchBufferPtr, TargetTotalSize, 0xFF);
> + ScratchBufferSize = TargetTotalSize;
> + ScratchBufferPtr = (UINT8 *) MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + UpdateMicrocode ((UINTN) TargetMicrocodeEntryPoint,
> MicrocodePatchScratchBuffer, ScratchBufferSize, LastAttemptStatus);
> + }
> + return Status;
> + }
> +
> + //
> + // 3. If there is unused microcode entry with enough space, use it.
> + //
> + UnusedFitMicrocodeEntry = FindUnusedFitMicrocode
> (MicrocodeFmpPrivate, ImageSize, &AvailableSize);
> + if (UnusedFitMicrocodeEntry != NULL) {
> + DEBUG((DEBUG_INFO, "Use unused FIT microcode entry\n"));
> + // 3.1. Copy new image
> + CopyMem (ScratchBufferPtr, Image, ImageSize);
> + ScratchBufferSize += ImageSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + // 3.2. Pad 0xFF
> + RestSize = AvailableSize - ImageSize;
> + if (RestSize > 0) {
> + SetMem (ScratchBufferPtr, RestSize, 0xFF);
> + ScratchBufferSize += RestSize;
> + ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + }
> + Status = UpdateMicrocode ((UINTN) UnusedFitMicrocodeEntry,
> MicrocodePatchScratchBuffer, ScratchBufferSize, LastAttemptStatus);
> + if (!EFI_ERROR (Status) && (TargetMicrocodeEntryPoint != NULL)) {
> + //
> + // Empty old microcode.
> + //
> + ScratchBufferPtr = MicrocodePatchScratchBuffer;
> + SetMem (ScratchBufferPtr, TargetTotalSize, 0xFF);
> + ScratchBufferSize = TargetTotalSize;
> + ScratchBufferPtr = (UINT8 *) MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> + UpdateMicrocode ((UINTN) TargetMicrocodeEntryPoint,
> MicrocodePatchScratchBuffer, ScratchBufferSize, LastAttemptStatus);
> + }
> + return Status;
> + }
> +
> + //
> + // 4. No usable FIT microcode entry.
> + //
> + DEBUG((DEBUG_ERROR, "No usable FIT microcode entry\n"));
> + *LastAttemptStatus =
> LAST_ATTEMPT_STATUS_ERROR_INSUFFICIENT_RESOURCES;
> + Status = EFI_OUT_OF_RESOURCES;
> +
> + return Status;
> +}
> +
> +/**
> Update Microcode flash region.
>
> @param[in] MicrocodeFmpPrivate The Microcode driver private
> data
> @@ -753,8 +1066,8 @@ UpdateMicrocodeFlashRegion (
> AvailableSize = (UINTN)MicrocodePatchAddress +
> MicrocodePatchRegionSize - (UINTN)TargetMicrocodeEntryPoint;
> }
> DEBUG((DEBUG_INFO, " AvailableSize - 0x%x\n", AvailableSize));
> + ASSERT (AvailableSize >= TargetTotalSize);
> }
> - ASSERT (AvailableSize >= TargetTotalSize);
> UsedRegionSize =
> GetCurrentMicrocodeUsedRegionSize(MicrocodeFmpPrivate);
> DEBUG((DEBUG_INFO, " UsedRegionSize - 0x%x\n", UsedRegionSize));
> ASSERT (UsedRegionSize >= TargetTotalSize); @@ -762,8 +1075,8 @@
> UpdateMicrocodeFlashRegion (
> ASSERT ((UINTN)MicrocodePatchAddress + UsedRegionSize >=
> ((UINTN)TargetMicrocodeEntryPoint + TargetTotalSize));
> }
> //
> - // Total Size means the Microcode data size.
> - // Available Size means the Microcode data size plus the pad till
> (1) next Microcode or (2) the end.
> + // Total Size means the Microcode size.
> + // Available Size means the Microcode size plus the pad till (1)
> + next
> Microcode or (2) the end.
> //
> // (1)
> // +------+-----------+-----+------+===================+
> @@ -793,11 +1106,11 @@ UpdateMicrocodeFlashRegion (
> DEBUG((DEBUG_INFO, "Replace old microcode in situ\n"));
> //
> // +------+------------+------+===================+
> - // |Other1| Old Image |Other2| Empty |
> + // |Other | Old Image | ... | Empty |
> // +------+------------+------+===================+
> //
> // +------+---------+--+------+===================+
> - // |Other1|New Image|FF|Other2| Empty |
> + // |Other |New Image|FF| ... | Empty |
> // +------+---------+--+------+===================+
> //
> // 1.1. Copy new image
> @@ -835,11 +1148,11 @@ UpdateMicrocodeFlashRegion (
> DEBUG((DEBUG_INFO, "Reorg and replace old microcode\n"));
> //
> // +------+------------+------+===================+
> - // |Other1| Old Image |Other2| Empty |
> + // |Other | Old Image | ... | Empty |
> // +------+------------+------+===================+
> //
> // +------+---------------+------+================+
> - // |Other1| New Image |Other2| Empty |
> + // |Other | New Image | ... | Empty |
> // +------+---------------+------+================+
> //
> // 2.1. Copy new image
> @@ -849,7 +1162,7 @@ UpdateMicrocodeFlashRegion (
> // 2.2. Copy rest images after the old image.
> if (NextMicrocodeEntryPoint != 0) {
> RestSize = (UINTN)MicrocodePatchAddress + UsedRegionSize -
> ((UINTN)NextMicrocodeEntryPoint);
> - CopyMem (ScratchBufferPtr, (UINT8 *)TargetMicrocodeEntryPoint +
> TargetTotalSize, RestSize);
> + CopyMem (ScratchBufferPtr, NextMicrocodeEntryPoint,
> + RestSize);
> ScratchBufferSize += RestSize;
> ScratchBufferPtr = (UINT8 *)MicrocodePatchScratchBuffer +
> ScratchBufferSize;
> }
> @@ -932,7 +1245,7 @@ UpdateMicrocodeFlashRegion (
> call to FreePool().
>
> @retval EFI_SUCCESS The Microcode image is written.
> - @retval EFI_VOLUME_CORRUPTED The Microcode image is corrupt.
> + @retval EFI_VOLUME_CORRUPTED The Microcode image is
> corrupted.
> @retval EFI_INCOMPATIBLE_VERSION The Microcode image version is
> incorrect.
> @retval EFI_SECURITY_VIOLATION The Microcode image fails to load.
> @retval EFI_WRITE_PROTECTED The flash device is read only.
> @@ -963,7 +1276,6 @@ MicrocodeWrite (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - *LastAttemptVersion = ((CPU_MICROCODE_HEADER
> *)Image)->UpdateRevision;
> TargetCpuIndex = (UINTN)-1;
> Status = VerifyMicrocode(MicrocodeFmpPrivate, AlignedImage,
> ImageSize, TRUE, LastAttemptStatus, AbortReason, &TargetCpuIndex);
> if (EFI_ERROR(Status)) {
> @@ -972,6 +1284,7 @@ MicrocodeWrite (
> return Status;
> }
> DEBUG((DEBUG_INFO, "Pass VerifyMicrocode\n"));
> + *LastAttemptVersion = ((CPU_MICROCODE_HEADER
> *)Image)->UpdateRevision;
>
> DEBUG((DEBUG_INFO, " TargetCpuIndex - 0x%x\n", TargetCpuIndex));
> ASSERT (TargetCpuIndex < MicrocodeFmpPrivate->ProcessorCount);
> @@ -985,13 +1298,23 @@ MicrocodeWrite (
> }
> DEBUG((DEBUG_INFO, " TargetMicrocodeEntryPoint - 0x%x\n",
> TargetMicrocodeEntryPoint));
>
> - Status = UpdateMicrocodeFlashRegion(
> - MicrocodeFmpPrivate,
> - TargetMicrocodeEntryPoint,
> - AlignedImage,
> - ImageSize,
> - LastAttemptStatus
> - );
> + if (MicrocodeFmpPrivate->FitMicrocodeInfo != NULL) {
> + Status = UpdateMicrocodeFlashRegionWithFit (
> + MicrocodeFmpPrivate,
> + TargetMicrocodeEntryPoint,
> + AlignedImage,
> + ImageSize,
> + LastAttemptStatus
> + );
> + } else {
> + Status = UpdateMicrocodeFlashRegion (
> + MicrocodeFmpPrivate,
> + TargetMicrocodeEntryPoint,
> + AlignedImage,
> + ImageSize,
> + LastAttemptStatus
> + );
> + }
>
> FreePool(AlignedImage);
>
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.h
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.h
> index 9dc306324e62..d16b353eefa2 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.h
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.h
> @@ -20,6 +20,8 @@
> #include <Guid/SystemResourceTable.h> #include <Guid/MicrocodeFmp.h>
>
> +#include <IndustryStandard/FirmwareInterfaceTable.h>
> +
> #include <Protocol/FirmwareManagement.h> #include
> <Protocol/MpService.h>
>
> @@ -58,6 +60,12 @@ typedef struct {
> } MICROCODE_INFO;
>
> typedef struct {
> + CPU_MICROCODE_HEADER *MicrocodeEntryPoint;
> + BOOLEAN Empty;
> + BOOLEAN InUse;
> +} FIT_MICROCODE_INFO;
> +
> +typedef struct {
> UINTN CpuIndex;
> UINT32 ProcessorSignature;
> UINT8 PlatformId;
> @@ -86,11 +94,13 @@ struct _MICROCODE_FMP_PRIVATE_DATA {
> UINTN BspIndex;
> UINTN ProcessorCount;
> PROCESSOR_INFO *ProcessorInfo;
> + UINT32 FitMicrocodeEntryCount;
> + FIT_MICROCODE_INFO *FitMicrocodeInfo;
> };
>
> typedef struct _MICROCODE_FMP_PRIVATE_DATA
> MICROCODE_FMP_PRIVATE_DATA;
>
> -#define MICROCODE_FMP_LAST_ATTEMPT_VARIABLE_NAME
> L"MicrocodeLastAttempVar"
> +#define MICROCODE_FMP_LAST_ATTEMPT_VARIABLE_NAME
> L"MicrocodeLastAttemptVar"
>
> /**
> Returns a pointer to the MICROCODE_FMP_PRIVATE_DATA structure from
> the input a as Fmp.
> diff --git
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDx
> e.i
> nf
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDx
> e.i
> nf
> index dbc90857a0a5..24f06c23d404 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDx
> e.i
> nf
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDx
> e.i
> nf
> @@ -3,7 +3,7 @@
> #
> # Produce FMP instance to update Microcode.
> #
> -# Copyright (c) 2016 - 2017, Intel Corporation. All rights
> reserved.<BR>
> +# Copyright (c) 2016 - 2018, Intel Corporation. All rights
> +reserved.<BR>
> # This program and the accompanying materials # are licensed and
> made available under the terms and conditions of the BSD License #
> which accompanies this distribution. The full text of the license may
> be found at @@ -65,6 +65,7 @@ [Pcd]
>
> [Depex]
> gEfiVariableArchProtocolGuid AND
> + gEfiVariableWriteArchProtocolGuid AND
> gEfiMpServiceProtocolGuid
>
> [UserExtensions.TianoCore."ExtraFiles"]
> --
> 2.7.0.windows.1
next prev parent reply other threads:[~2018-04-23 9:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-23 6:56 [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table Star Zeng
2018-04-23 8:49 ` Yao, Jiewen
2018-04-23 8:55 ` Zeng, Star
2018-04-23 9:38 ` Zeng, Star [this message]
2018-04-26 15:15 ` Yao, Jiewen
2018-04-27 8:22 ` Zeng, Star
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103BAD7DB9@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox