| Message ID | 20181114231137.44690-3-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi_loader: Code refactoring and improvement | expand |
On 11/15/18 12:11 AM, Simon Glass wrote: > The functions in bootefi are very long because they mix high-level code > and control with the low-level implementation. To help with this, create > functions which handle preparing for running the test and cleaning up > afterwards. > > Also shorten the awfully long variable names here. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v14: > - Go back to the horrible long variable names > > Changes in v13: > - Rebase to efi/efi-next > > Changes in v12: > - Rename image to image_prot > > Changes in v11: None > Changes in v9: > - Add comments to bootefi_test_prepare() about the memset()s > > Changes in v7: None > Changes in v5: > - Drop call to efi_init_obj_list() which is now done in do_bootefi() > > Changes in v4: None > Changes in v3: > - Add new patch to split out test init/uninit into functions > > cmd/bootefi.c | 81 +++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 65 insertions(+), 16 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 3e37805ea13..b633e956c23 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -453,6 +453,67 @@ exit: > return ret; > } > > +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST > +/** > + * bootefi_test_prepare() - prepare to run an EFI test > + * > + * This sets things up so we can call EFI functions. This involves preparing > + * the 'gd' pointer and setting up the load ed image data structures. > + * > + * @image_objp: loaded_image_infop: Pointer to a struct which will hold the > + * loaded image object. This struct will be inited by this function before > + * use. > + * @loaded_image_infop: Pointer to a struct which will hold the loaded image > + * info. This struct will be inited by this function before use. > + * @path: File path to the test being run (often just the test name with a > + * backslash before it > + * @test_func: Address of the test function that is being run > + * @return 0 if OK, -ve on error > + */ > +static efi_status_t bootefi_test_prepare( CHECK: Lines should not end with a '(' #43: FILE: cmd/bootefi.c:474: +static efi_status_t bootefi_test_prepare( So the parenthesis has to go onto the next line. > + struct efi_loaded_image_obj **image_objp, (struct efi_loaded_image_obj **image_objp, > + struct efi_loaded_image **loaded_image_infop, > + const char *path, > + ulong test_func) > +{ > + efi_status_t r; > + > + /* Construct a dummy device path */ > + bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, > + (uintptr_t)test_func, > + (uintptr_t)test_func); We have to check the return value. It may be NULL. > + bootefi_image_path = efi_dp_from_file(NULL, 0, path); Same here. > + r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path, > + image_objp, loaded_image_infop); > + if (r) > + return r; > + /* > + * gd lives in a fixed register which may get clobbered while we execute > + * the payload. So save it here and restore it on every callback entry > + */ > + efi_save_gd(); We do this already in efi_init_obj_list(). Best regards Heinrich > + > + /* Transfer environment variable efi_selftest as load options */ > + set_load_options(*loaded_image_infop, "efi_selftest"); > + > + return 0; > +} > + > +/** > + * bootefi_test_finish() - finish up after running an EFI test > + * > + * @image_obj: Pointer to a struct which holds the loaded image object > + * @loaded_image_info: Pointer to a struct which holds the loaded image info > + */ > +static void bootefi_test_finish(struct efi_loaded_image_obj *image_obj, > + struct efi_loaded_image *loaded_image_info) > +{ > + efi_restore_gd(); > + free(loaded_image_info->load_options); > + efi_delete_handle(&image_obj->header); > +} > +#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ > + > static int do_bootefi_bootmgr_exec(void) > { > struct efi_device_path *device_path, *file_path; > @@ -528,26 +589,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > struct efi_loaded_image_obj *image_obj; > struct efi_loaded_image *loaded_image_info; > > - /* Construct a dummy device path. */ > - bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, > - (uintptr_t)&efi_selftest, > - (uintptr_t)&efi_selftest); > - bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest"); > - > - r = efi_setup_loaded_image(bootefi_device_path, > - bootefi_image_path, &image_obj, > - &loaded_image_info); > - if (r != EFI_SUCCESS) > + if (bootefi_test_prepare(&image_obj, &loaded_image_info, > + "\\selftest", > + (uintptr_t)&efi_selftest)) > return CMD_RET_FAILURE; > > - efi_save_gd(); > - /* Transfer environment variable efi_selftest as load options */ > - set_load_options(loaded_image_info, "efi_selftest"); > /* Execute the test */ > r = efi_selftest(&image_obj->header, &systab); > - efi_restore_gd(); > - free(loaded_image_info->load_options); > - efi_delete_handle(&image_obj->header); > + bootefi_test_finish(image_obj, loaded_image_info); > return r != EFI_SUCCESS; > } else > #endif >
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 3e37805ea13..b633e956c23 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -453,6 +453,67 @@ exit: return ret; } +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST +/** + * bootefi_test_prepare() - prepare to run an EFI test + * + * This sets things up so we can call EFI functions. This involves preparing + * the 'gd' pointer and setting up the load ed image data structures. + * + * @image_objp: loaded_image_infop: Pointer to a struct which will hold the + * loaded image object. This struct will be inited by this function before + * use. + * @loaded_image_infop: Pointer to a struct which will hold the loaded image + * info. This struct will be inited by this function before use. + * @path: File path to the test being run (often just the test name with a + * backslash before it + * @test_func: Address of the test function that is being run + * @return 0 if OK, -ve on error + */ +static efi_status_t bootefi_test_prepare( + struct efi_loaded_image_obj **image_objp, + struct efi_loaded_image **loaded_image_infop, + const char *path, + ulong test_func) +{ + efi_status_t r; + + /* Construct a dummy device path */ + bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, + (uintptr_t)test_func, + (uintptr_t)test_func); + bootefi_image_path = efi_dp_from_file(NULL, 0, path); + r = efi_setup_loaded_image(bootefi_device_path, bootefi_image_path, + image_objp, loaded_image_infop); + if (r) + return r; + /* + * gd lives in a fixed register which may get clobbered while we execute + * the payload. So save it here and restore it on every callback entry + */ + efi_save_gd(); + + /* Transfer environment variable efi_selftest as load options */ + set_load_options(*loaded_image_infop, "efi_selftest"); + + return 0; +} + +/** + * bootefi_test_finish() - finish up after running an EFI test + * + * @image_obj: Pointer to a struct which holds the loaded image object + * @loaded_image_info: Pointer to a struct which holds the loaded image info + */ +static void bootefi_test_finish(struct efi_loaded_image_obj *image_obj, + struct efi_loaded_image *loaded_image_info) +{ + efi_restore_gd(); + free(loaded_image_info->load_options); + efi_delete_handle(&image_obj->header); +} +#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ + static int do_bootefi_bootmgr_exec(void) { struct efi_device_path *device_path, *file_path; @@ -528,26 +589,14 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) struct efi_loaded_image_obj *image_obj; struct efi_loaded_image *loaded_image_info; - /* Construct a dummy device path. */ - bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, - (uintptr_t)&efi_selftest, - (uintptr_t)&efi_selftest); - bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest"); - - r = efi_setup_loaded_image(bootefi_device_path, - bootefi_image_path, &image_obj, - &loaded_image_info); - if (r != EFI_SUCCESS) + if (bootefi_test_prepare(&image_obj, &loaded_image_info, + "\\selftest", + (uintptr_t)&efi_selftest)) return CMD_RET_FAILURE; - efi_save_gd(); - /* Transfer environment variable efi_selftest as load options */ - set_load_options(loaded_image_info, "efi_selftest"); /* Execute the test */ r = efi_selftest(&image_obj->header, &systab); - efi_restore_gd(); - free(loaded_image_info->load_options); - efi_delete_handle(&image_obj->header); + bootefi_test_finish(image_obj, loaded_image_info); return r != EFI_SUCCESS; } else #endif
The functions in bootefi are very long because they mix high-level code and control with the low-level implementation. To help with this, create functions which handle preparing for running the test and cleaning up afterwards. Also shorten the awfully long variable names here. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v14: - Go back to the horrible long variable names Changes in v13: - Rebase to efi/efi-next Changes in v12: - Rename image to image_prot Changes in v11: None Changes in v9: - Add comments to bootefi_test_prepare() about the memset()s Changes in v7: None Changes in v5: - Drop call to efi_init_obj_list() which is now done in do_bootefi() Changes in v4: None Changes in v3: - Add new patch to split out test init/uninit into functions cmd/bootefi.c | 81 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 16 deletions(-)