public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [PATCH] IntelSiliconPkg MicrocodeUpdateDxe: Honor FIT table
Date: Mon, 23 Apr 2018 08:49:16 +0000	[thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AB79B07@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1524466601-20812-1-git-send-email-star.zeng@intel.com>

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/MicrocodeUpdateDxe.i
> nf
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.i
> nf
> index dbc90857a0a5..24f06c23d404 100644
> ---
> a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.i
> nf
> +++
> b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.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



  reply	other threads:[~2018-04-23  8:49 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 [this message]
2018-04-23  8:55   ` Zeng, Star
2018-04-23  9:38     ` Zeng, Star
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=74D8A39837DF1E4DA445A8C0B3885C503AB79B07@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