[PATCH 1/3] efi: Add debugging to efi_set_bootdev()

[Heinrich Schuchardt]

On 1/22/22 02:16, Simon Glass wrote:
> The operation of this function can be confusing. Add some debugging so
> we can see what it is doing and when it is called.
>
> Also drop the preprocessor usage.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>   cmd/bootefi.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 3a8b2b60618..d8685f0e878 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -65,6 +65,9 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
>   	struct efi_device_path *device, *image;
>   	efi_status_t ret;
>
> +	log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
> +		  devnr, path, buffer, buffer_size);
> +
>   	/* Forget overwritten image */
>   	if (buffer + buffer_size >= image_addr &&
>   	    image_addr + image_size >= buffer)
> @@ -72,18 +75,19 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
>
>   	/* Remember only PE-COFF and FIT images */
>   	if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
> -#ifdef CONFIG_FIT
> -		if (fit_check_format(buffer, IMAGE_SIZE_INVAL))
> +		if (IS_ENABLED(CONFIG_FIT) &&
> +		    !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {

This looks ok.

> +			/*
> +			 * FIT images of type EFI_OS are started via command
> +			 * bootm. We should not use their boot device with the
> +			 * bootefi command.
> +			 */
> +			buffer = 0;
> +			buffer_size = 0;
> +		} else {
> +			log_debug("- invalid image\n");

This function is called by the 'load' command. When loading a perfectly
valid device-tree or initial RAM disk the message would confuse me.

What is that '- ' good for?

Best regards

Heinrich

>   			return;
> -		/*
> -		 * FIT images of type EFI_OS are started via command bootm.
> -		 * We should not use their boot device with the bootefi command.
> -		 */
> -		buffer = 0;
> -		buffer_size = 0;
> -#else
> -		return;
> -#endif
> +		}
>   	}
>
>   	/* efi_set_bootdev() is typically called repeatedly, recover memory */
> @@ -103,7 +107,11 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
>   			efi_free_pool(image_tmp);
>   		}
>   		bootefi_image_path = image;
> +		log_debug("- recorded device %ls\n", efi_dp_str(device));
> +		if (image)
> +			log_debug("- and image %ls\n", efi_dp_str(image));
>   	} else {
> +		log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
>   		efi_clear_bootdev();
>   	}
>   }
> @@ -451,6 +459,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
>   	u16 *load_options;
>
>   	if (!bootefi_device_path || !bootefi_image_path) {
> +		log_debug("Not loaded from disk\n");
>   		/*
>   		 * Special case for efi payload not loaded from disk,
>   		 * such as 'bootefi hello' or for example payload
> @@ -476,6 +485,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
>   		file_path = efi_dp_append(bootefi_device_path,
>   					  bootefi_image_path);
>   		msg_path = bootefi_image_path;
> +		log_debug("Loaded from disk\n");
>   	}
>
>   	log_info("Booting %pD\n", msg_path);