Re: [PATCH 2/2] mkeficapsule: Support "--flags reset" option

On 1/25/22 12:31, Masami Hiramatsu wrote:

Support "--flags reset" option to set the CAPSULE_FLAGS_INITIATE_RESET flag to capsule header.

Signed-off-by: Masami Hiramatsu masami.hiramatsu@linaro.org

tools/mkeficapsule.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index 4995ba4e0c..ca3a1c77ad 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -44,6 +44,7 @@ static struct option options[] = { {"raw", required_argument, NULL, 'r'}, {"index", required_argument, NULL, 'i'}, {"instance", required_argument, NULL, 'I'},

• {"flags", required_argument, NULL, 'F' }, {"help", no_argument, NULL, 'h'}, {NULL, 0, NULL, 0}, };

@@ -57,12 +58,13 @@ static void print_usage(void) "\t-r, --raw <raw image> new raw image file\n" "\t-i, --index <index> update image index\n" "\t-I, --instance <instance> update hardware instance\n"

•       "\t-F, --flags <flags>         set capsule flags (support only \"reset\")\n"
  

How should a user know if this "reset" means CAPSULE_FLAGS_PERSIST_ACROSS_RESET 0x00010000 or CAPSULE_FLAGS_INITIATE_RESET 0x00040000?

We need a man-page file for mkeficapsule (cf. /doc/mkimage.1 for mkimage).

The tool should support CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE 0x00020000 even if it is not implemented in U-Boot yet.

CAPSULE_FLAGS_PERSIST_ACROSS_RESET should not be set automatically if --flags is provided.

Isn't CAPSULE_FLAGS_PERSIST_ACROSS_RESET | CAPSULE_FLAGS_INITIATE_RESET what we need by default?

Best regards

Heinrich

      "\t-h, --help                  print a help message\n",
      tool_name);

}

static int create_fwbin(char *path, char *bin, efi_guid_t *guid,

• 	unsigned long index, unsigned long instance)
  

• 	unsigned long index, unsigned long instance, u32 flags)
  

  { struct efi_capsule_header header; struct efi_firmware_management_capsule_header capsule;

@@ -101,7 +103,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, header.capsule_guid = efi_guid_fm_capsule; header.header_size = sizeof(header); /* TODO: The current implementation ignores flags */

• header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET;

• header.flags = CAPSULE_FLAGS_PERSIST_ACROSS_RESET | flags; header.capsule_image_size = sizeof(header) + sizeof(capsule) + sizeof(u64) + sizeof(image)

@@ -171,6 +173,14 @@ err_1: return -1; }

+int decode_capsule_flags(const char *flagstr, u32 *flags) +{

• if (strcmp(flagstr, "reset"))

• return -EINVAL;
  

• *flags = CAPSULE_FLAGS_INITIATE_RESET;
• return 0;

+}

• /*
  • Usage:
  • $ mkeficapsule -f <firmware binary> <output file>

@@ -178,6 +188,7 @@ err_1: int main(int argc, char **argv) { char *file;

• u32 flags; efi_guid_t *guid; unsigned long index, instance; int c, idx;

@@ -186,8 +197,9 @@ int main(int argc, char **argv) guid = NULL; index = 0; instance = 0;

• flags = 0; for (;;) {

• c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
  

• c = getopt_long(argc, argv, "f:r:i:I:v:F:h", options, &idx);
  

  if (c == -1) break;

@@ -214,6 +226,12 @@ int main(int argc, char **argv) case 'I': instance = strtoul(optarg, NULL, 0); break;

• case 'F':
  

• 	if (decode_capsule_flags(optarg, &flags) < 0) {
  

• 		printf("Unsupported flags %s\n", optarg);
  

• 		return -1;
  

• 	}
  

• 	break;
  

  case 'h': print_usage(); return 0;

@@ -232,7 +250,7 @@ int main(int argc, char **argv) exit(EXIT_SUCCESS); }

• if (create_fwbin(argv[optind], file, guid, index, instance)

• if (create_fwbin(argv[optind], file, guid, index, instance, flags) < 0) { printf("Creating firmware capsule failed\n"); exit(EXIT_FAILURE);