public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: Jiewen Yao <jiewen.yao@intel.com>, edk2-devel@lists.01.org
Cc: Feng Tian <feng.tian@intel.com>,
	Chao Zhang <chao.b.zhang@intel.com>,
	star.zeng@intel.com
Subject: Re: [PATCH V2] MdeModulePkg/CapsuleLib: Add CapsuleTarget support.
Date: Mon, 26 Dec 2016 10:56:03 +0800	[thread overview]
Message-ID: <63065c0b-9508-327c-693e-11a4f595aa09@intel.com> (raw)
In-Reply-To: <1482508959-16144-1-git-send-email-jiewen.yao@intel.com>

Minor comment inline.

On 2016/12/24 0:02, Jiewen Yao wrote:
> UEFI spec requires CapsuleTarget to be a device path associated
> with FMP producer.
>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c           | 15 +++++++++--
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c     | 28 ++++++++++++++++----
>  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLibNull.c |  4 ++-
>  3 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> index af08886..2bb6ac8 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> @@ -100,6 +100,7 @@ RecordCapsuleStatusVariable (
>    @param[in] CapsuleStatus  The capsule process stauts
>    @param[in] PayloadIndex   FMP payload index
>    @param[in] ImageHeader    FMP image header
> +  @param[in] FmpDevicePath  DevicePath associated with the FMP producer
>
>    @retval EFI_SUCCESS          The capsule status variable is recorded.
>    @retval EFI_OUT_OF_RESOURCES No resource to record the capsule status variable.
> @@ -109,7 +110,8 @@ RecordFmpCapsuleStatusVariable (
>    IN EFI_CAPSULE_HEADER                            *CapsuleHeader,
>    IN EFI_STATUS                                    CapsuleStatus,
>    IN UINTN                                         PayloadIndex,
> -  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader
> +  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader,
> +  IN EFI_DEVICE_PATH_PROTOCOL                      *FmpDevicePath OPTIONAL
>    );
>
>  /**
> @@ -818,6 +820,7 @@ ProcessFmpCapsuleImage (
>    UINTN                                         Index2;
>    MEMMAP_DEVICE_PATH                            MemMapNode;
>    EFI_DEVICE_PATH_PROTOCOL                      *DriverDevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL                      *FmpDevicePath;
>    ESRT_MANAGEMENT_PROTOCOL                      *EsrtProtocol;
>    EFI_SYSTEM_RESOURCE_ENTRY                     EsrtEntry;
>    VOID                                          *VendorCode;
> @@ -941,6 +944,13 @@ ProcessFmpCapsuleImage (
>          continue;
>        }
>
> +      FmpDevicePath = NULL;
> +      gBS->HandleProtocol(
> +             HandleBuffer[Index1],
> +             &gEfiDevicePathProtocolGuid,
> +             (VOID **)&FmpDevicePath
> +             );
> +
>        ImageInfoSize = 0;
>        Status = Fmp->GetImageInfo (
>                        Fmp,
> @@ -1060,7 +1070,8 @@ ProcessFmpCapsuleImage (
>                CapsuleHeader,                                 // CapsuleGuid
>                Status,                                        // CapsuleStatus
>                Index - FmpCapsuleHeader->EmbeddedDriverCount, // PayloadIndex
> -              ImageHeader                                    // ImageHeader
> +              ImageHeader,                                   // ImageHeader
> +              FmpDevicePath                                  // FmpDevicePath
>                );
>              if (StatusRet != EFI_SUCCESS) {
>                StatusRet = Status;
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index d34eb25..bba30b5 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -30,6 +30,7 @@
>  #include <Library/HobLib.h>
>  #include <Library/PrintLib.h>
>  #include <Library/ReportStatusCodeLib.h>
> +#include <Library/DevicePathLib.h>
>  #include <Library/CapsuleLib.h>
>
>  #include <IndustryStandard/WindowsUxCapsule.h>
> @@ -280,6 +281,7 @@ RecordCapsuleStatusVariable (
>    @param[in] CapsuleStatus  The capsule process stauts
>    @param[in] PayloadIndex   FMP payload index
>    @param[in] ImageHeader    FMP image header
> +  @param[in] FmpDevicePath  DevicePath associated with the FMP producer
>
>    @retval EFI_SUCCESS          The capsule status variable is recorded.
>    @retval EFI_OUT_OF_RESOURCES No resource to record the capsule status variable.
> @@ -289,26 +291,37 @@ RecordFmpCapsuleStatusVariable (
>    IN EFI_CAPSULE_HEADER                            *CapsuleHeader,
>    IN EFI_STATUS                                    CapsuleStatus,
>    IN UINTN                                         PayloadIndex,
> -  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader
> +  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader,
> +  IN EFI_DEVICE_PATH_PROTOCOL                      *FmpDevicePath OPTIONAL
>    )
>  {
>    EFI_CAPSULE_RESULT_VARIABLE_HEADER  *CapsuleResultVariableHeader;
>    EFI_CAPSULE_RESULT_VARIABLE_FMP     *CapsuleResultVariableFmp;
>    EFI_STATUS                          Status;
>    UINT8                               *CapsuleResultVariable;
> -  UINT32                              CapsuleResultVariableSize;
> +  UINTN                               CapsuleResultVariableSize;
> +  CHAR16                              *DevicePathStr;
> +  UINTN                               DevicePathStrSize;
>
> -  CapsuleResultVariable     = NULL;
> +  DevicePathStr = NULL;
> +  if (FmpDevicePath != NULL) {
> +    DevicePathStr = ConvertDevicePathToText (FmpDevicePath, FALSE, FALSE);
> +  }
> +  if (DevicePathStr != NULL) {
> +    DevicePathStrSize = StrSize(DevicePathStr);
> +  } else {
> +    DevicePathStrSize = sizeof(CHAR16);
> +  }
>    //
>    // Allocate zero CHAR16 for CapsuleFileName and CapsuleTarget.

Does the comment need to be updated?

>    //
> -  CapsuleResultVariableSize = sizeof(EFI_CAPSULE_RESULT_VARIABLE_HEADER) + sizeof(EFI_CAPSULE_RESULT_VARIABLE_FMP) + sizeof(CHAR16) * 2;
> +  CapsuleResultVariableSize = sizeof(EFI_CAPSULE_RESULT_VARIABLE_HEADER) + sizeof(EFI_CAPSULE_RESULT_VARIABLE_FMP) + sizeof(CHAR16) + DevicePathStrSize;
>    CapsuleResultVariable     = AllocateZeroPool (CapsuleResultVariableSize);
>    if (CapsuleResultVariable == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
>    CapsuleResultVariableHeader = (VOID *)CapsuleResultVariable;
> -  CapsuleResultVariableHeader->VariableTotalSize = CapsuleResultVariableSize;
> +  CapsuleResultVariableHeader->VariableTotalSize = (UINT32)CapsuleResultVariableSize;
>    CapsuleResultVariableHeader->Reserved = 0;
>    CopyGuid(&CapsuleResultVariableHeader->CapsuleGuid, &CapsuleHeader->CapsuleGuid);
>    ZeroMem(&CapsuleResultVariableHeader->CapsuleProcessed, sizeof(CapsuleResultVariableHeader->CapsuleProcessed));
> @@ -320,6 +333,11 @@ RecordFmpCapsuleStatusVariable (
>    CapsuleResultVariableFmp->PayloadIndex = (UINT8)PayloadIndex;
>    CapsuleResultVariableFmp->UpdateImageIndex = ImageHeader->UpdateImageIndex;
>    CopyGuid (&CapsuleResultVariableFmp->UpdateImageTypeId, &ImageHeader->UpdateImageTypeId);
> +  if (DevicePathStr != NULL) {
> +    CopyMem ((UINT8 *)CapsuleResultVariableFmp + sizeof(EFI_CAPSULE_RESULT_VARIABLE_FMP) + sizeof(CHAR16), DevicePathStr, DevicePathStrSize);
> +    FreePool (DevicePathStr);
> +    DevicePathStr = NULL;
> +  }
>
>    //
>    // Save Local Cache
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLibNull.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLibNull.c
> index bf550e5..6ab198d 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLibNull.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLibNull.c
> @@ -64,6 +64,7 @@ RecordCapsuleStatusVariable (
>    @param[in] CapsuleStatus  The capsule process stauts
>    @param[in] PayloadIndex   FMP payload index
>    @param[in] ImageHeader    FMP image header
> +  @param[in] FmpDevicePath  DevicePath associated with the FMP producer
>
>    @retval EFI_SUCCESS          The capsule status variable is recorded.
>    @retval EFI_OUT_OF_RESOURCES No resource to record the capsule status variable.
> @@ -73,7 +74,8 @@ RecordFmpCapsuleStatusVariable (
>    IN EFI_CAPSULE_HEADER                            *CapsuleHeader,
>    IN EFI_STATUS                                    CapsuleStatus,
>    IN UINTN                                         PayloadIndex,
> -  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader
> +  IN EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER  *ImageHeader,
> +  IN EFI_DEVICE_PATH_PROTOCOL                      *FmpDevicePath OPTIONAL
>    )
>  {
>    return EFI_UNSUPPORTED;
>



  reply	other threads:[~2016-12-26  2:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-23 16:02 [PATCH V2] MdeModulePkg/CapsuleLib: Add CapsuleTarget support Jiewen Yao
2016-12-26  2:56 ` Zeng, Star [this message]
2016-12-26  3:01   ` Yao, Jiewen
2016-12-26  3:03     ` Zeng, Star
2016-12-26  3:12 ` Zhang, Chao B

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=63065c0b-9508-327c-693e-11a4f595aa09@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