Re: [edk2] [PATCH v2 2/2] MdeModulePkg/CapsuleApp: Enhance CapsuleApp to support Capsule-on-Disk.
Hi Chen,
Here're some overall comments:
1. Update all copyright year to 2019
2. Split this patch to smaller granularity (in public standalone file unit or
module unit),
and keep in mind that the patch order also matters. This patch can be split
into 3
patches.
3. Provide test information you've done.
See my other comments below.
> -Original Message-
> From: Chen, Chen A
> Sent: Monday, January 28, 2019 9:21 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A ; Wang, Jian J
> ; Wu, Hao A ; Zhang, Chao B
>
> Subject: [PATCH v2 2/2] MdeModulePkg/CapsuleApp: Enhance CapsuleApp to
> support Capsule-on-Disk.
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1482
>
> CapsuleApp is used for trigger capsule update.
> Add -OD option in CapsuleApp to support doing capsule update via storage.
> Add -F and -L options to support dumping information feature.
>
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Zhang Chao B
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen
> ---
> MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 153 +++-
> MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 8 +
> MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 535
> +-
> .../Application/CapsuleApp/CapsuleOnDisk.c | 802
> +
> MdeModulePkg/Include/Library/UefiBootManagerLib.h | 19 +
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 22 +
> 6 files changed, 1524 insertions(+), 15 deletions(-)
> create mode 100644 MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
>
> diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> index 4d907242f3..ca9baa0a6a 100644
> --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -105,6 +106,44 @@ DumpEsrtData (
>VOID
>);
>
> +/**
> + Dump Provisioned Capsule.
> +
> + @param[in] DumpCapsuleInfo The flag to indicate whether to dump the
> capsule inforomation.
> +**/
> +VOID
> +DumpProvisionedCapsule (
> + IN BOOLEAN DumpCapsuleInfo
> + );
> +
> +/**
> + Dump all EFI System Parition.
> +**/
> +VOID
> +DumpAllEfiSysPartition (
> + VOID
> + );
> +
> +/**
> + Process Capsule On Disk.
> +
> + @param[in] CapsuleBufferAn array of pointer to capsule images
> + @param[in] FileSize An array of UINTN to capsule images size
> + @param[in] OrgFileName An array of orginal capsule images name
> + @param[in] NewFileName An array of new capsule images name
> + @param[in] CapsuleNum The count of capsule images
> +
> + @retval EFI_SUCCESS Capsule on disk secceed.
> +**/
> +EFI_STATUS
> +ProcessCapsuleOnDisk (
> + IN VOID **CapsuleBuffer,
> + IN UINTN *CapsuleBufferSize,
> + IN CHAR16**FilePath,
> + IN CHAR16*Map,
> + IN UINTN CapsuleNum
> + );
> +
> /**
>Read a file.
>
> @@ -799,19 +838,22 @@ PrintUsage (
>)
> {
>Print(L"CapsuleApp: usage\n");
> - Print(L" CapsuleApp [-NR]\n");
> + Print(L" CapsuleApp [-NR] [-OD [FSx]]\n");
>Print(L" CapsuleApp -S\n");
>Print(L" CapsuleApp -C\n");
>Print(L" CapsuleApp -P\n");
>Print(L" CapsuleApp -E\n");
> + Print(L" CapsuleApp -L\n");
> + Print(L" CapsuleApp -L INFO\n");
> + Print(L" CapsuleApp -F\n");
>Print(L" CapsuleApp -G -O \n");
>Print(L" CapsuleApp -N -O \n");
>Print(L" CapsuleApp -D \n");
>Print(L" CapsuleApp -P GET -O \n");
>Print(L"Parameter:\n");
> - Print(L" -NR: No reset will be triggered for the capsule with\n");
> - Print(L" CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without\n");
> - Print(L" CAPSULE_FLAGS_INITIATE_RESET.\n");
> + Print(L" -NR: No reset will be triggered for the capsule\n");
> + Print(L" with CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without
> CAPSULE_FLAGS_INITIATE_RESET.\n");
> + Print(L" -OD: Delivery of Capsules via file on Mass Storage device.");
>Print(L" -S: Dump capsule report variable (EFI_CAPSULE_REPORT_GUID),\n");
>Print(L" which is defined in UEFI specification.\n");
>Print(L" -C: Clear capsule report variable
> (EFI_CAPSULE_REPORT_GUID),\n");
> @@ -820,6 +862,8 @@ PrintUsage (
>Print(L" ImageTypeId and Index (decimal format) to a file if
> 'GET'\n");
>Print(L" option is used.\n");
>Print(L" -E: Dump UEFI ESRT table info.\n");
> + Print(L" -L: Dump provisioned capsule image information.\n");
> + Print(L" -F: Dump all EFI System Partition.\n");
>Print(L" -G: Convert a BMP file to be an UX capsule,\n");
>Print(L" according to Windows Firmware Update document\n");
>Print(L" -N: Append a Capsule Header to an existing FMP
Re: [edk2] [PATCH v2 2/2] MdeModulePkg/CapsuleApp: Enhance CapsuleApp to support Capsule-on-Disk.
Chen,
Did you just sent this patch for v2? If yes, I think you should send all
patches in series even the one with title [PATCH 0/2]. It'd be better to add
change description since last version.
Regards,
Jian
> -Original Message-
> From: Chen, Chen A
> Sent: Monday, January 28, 2019 9:21 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A ; Wang, Jian J
> ; Wu, Hao A ; Zhang, Chao B
>
> Subject: [PATCH v2 2/2] MdeModulePkg/CapsuleApp: Enhance CapsuleApp to
> support Capsule-on-Disk.
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1482
>
> CapsuleApp is used for trigger capsule update.
> Add -OD option in CapsuleApp to support doing capsule update via storage.
> Add -F and -L options to support dumping information feature.
>
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Zhang Chao B
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen
> ---
> MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 153 +++-
> MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 8 +
> MdeModulePkg/Application/CapsuleApp/CapsuleDump.c | 535
> +-
> .../Application/CapsuleApp/CapsuleOnDisk.c | 802
> +
> MdeModulePkg/Include/Library/UefiBootManagerLib.h | 19 +
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 22 +
> 6 files changed, 1524 insertions(+), 15 deletions(-)
> create mode 100644 MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
>
> diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> index 4d907242f3..ca9baa0a6a 100644
> --- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> +++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -105,6 +106,44 @@ DumpEsrtData (
>VOID
>);
>
> +/**
> + Dump Provisioned Capsule.
> +
> + @param[in] DumpCapsuleInfo The flag to indicate whether to dump the
> capsule inforomation.
> +**/
> +VOID
> +DumpProvisionedCapsule (
> + IN BOOLEAN DumpCapsuleInfo
> + );
> +
> +/**
> + Dump all EFI System Parition.
> +**/
> +VOID
> +DumpAllEfiSysPartition (
> + VOID
> + );
> +
> +/**
> + Process Capsule On Disk.
> +
> + @param[in] CapsuleBufferAn array of pointer to capsule images
> + @param[in] FileSize An array of UINTN to capsule images size
> + @param[in] OrgFileName An array of orginal capsule images name
> + @param[in] NewFileName An array of new capsule images name
> + @param[in] CapsuleNum The count of capsule images
> +
> + @retval EFI_SUCCESS Capsule on disk secceed.
> +**/
> +EFI_STATUS
> +ProcessCapsuleOnDisk (
> + IN VOID **CapsuleBuffer,
> + IN UINTN *CapsuleBufferSize,
> + IN CHAR16**FilePath,
> + IN CHAR16*Map,
> + IN UINTN CapsuleNum
> + );
> +
> /**
>Read a file.
>
> @@ -799,19 +838,22 @@ PrintUsage (
>)
> {
>Print(L"CapsuleApp: usage\n");
> - Print(L" CapsuleApp [-NR]\n");
> + Print(L" CapsuleApp [-NR] [-OD [FSx]]\n");
>Print(L" CapsuleApp -S\n");
>Print(L" CapsuleApp -C\n");
>Print(L" CapsuleApp -P\n");
>Print(L" CapsuleApp -E\n");
> + Print(L" CapsuleApp -L\n");
> + Print(L" CapsuleApp -L INFO\n");
> + Print(L" CapsuleApp -F\n");
>Print(L" CapsuleApp -G -O \n");
>Print(L" CapsuleApp -N -O \n");
>Print(L" CapsuleApp -D \n");
>Print(L" CapsuleApp -P GET -O \n");
>Print(L"Parameter:\n");
> - Print(L" -NR: No reset will be triggered for the capsule with\n");
> - Print(L" CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without\n");
> - Print(L" CAPSULE_FLAGS_INITIATE_RESET.\n");
> + Print(L" -NR: No reset will be triggered for the capsule\n");
> + Print(L" with CAPSULE_FLAGS_PERSIST_ACROSS_RESET and without
> CAPSULE_FLAGS_INITIATE_RESET.\n");
> + Print(L" -OD: Delivery of Capsules via file on Mass Storage device.");
>Print(L" -S: Dump capsule report variable (EFI_CAPSULE_REPORT_GUID),\n");
>Print(L" which is defined in UEFI specification.\n");
>Print(L" -C: Clear capsule report variable
> (EFI_CAPSULE_REPORT_GUID),\n");
> @@ -820,6 +862,8 @@ PrintUsage (
>Print(L" ImageTypeId and Index (decimal format) to a file if
> 'GET'\n");
>Print(L" option is used.\n");
>Print(L" -E: Dump UEFI ESRT table info.\n");
> + Print(L" -L: Dump provisioned capsule image information.\n");
> + Print(L" -F: Dump all EFI System Partition.\n");
>Print(L" -G: Convert a BMP file to be an UX capsule,\n");
>Print(L" according to Windows Firmware Update document\n");
>Print(L" -N: Append a Capsule Header to an existing FMP capsule
> image\n");
> @@ -851,7 +895,7 @@ UefiMain (
> {
>EFI_STATUSStatus;
>RETURN_STATUS
