Re: [PATCH] kexec_file: skip checksum verification when relocations aren't needed

[Pasha Tatashin <pasha.tatashin@soleen.com>]

Nit: The crash kernel also does not perform relocations, yet a checksum 
is still required. The subject should be something like:
kexec_file: skip purgatory checksum if all segments are CMA allocated


On 06-01 21:11, Michal Clapinski wrote:
> Checksum verification is needed
> 1. for crash kernels. In a crash, we can't be sure the kernel is
>    intact.
> 2. if we're worried about relocating the kernel into a region used by
>    some DMA that wasn't properly cancelled.

Nit: Please add a little background information about CMA segments being 
recently added, as well as the necessity for a fast reboot due to the 
live update use case.

> 
> If we used CMA to allocate segments then
> 1. we're not working with a crash kernel.
> 2. relocations are not going to happen.
> 
> Therefore, we can safely disable checksum verification.
> 
> Instead of adding a new variable to purgatory, just skip adding regions
> and save the default value of SHA256 hash.
> 
> Saves ~250ms on my 4.0 GHz CPU.
> 
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
>  kernel/kexec_file.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 2bfbb2d144e6..2dc8b0435fe6 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -808,6 +808,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  	void *zero_buf;
>  	struct kexec_sha_region *sha_regions;
>  	struct purgatory_info *pi = &image->purgatory_info;
> +	bool can_skip_checksum = true;
>  
>  	if (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY))
>  		return 0;
> @@ -822,6 +823,23 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  
>  	sha256_init(&sctx);
>  
> +	/*
> +	 * If all segments were loaded into contiguous memory, there will be no
> +	 * relocations. In that case there is no risk of memory corruption by
> +	 * uncancelled DMA and we can skip checksum calculation.
> +	 */
> +	for (i = 0; i < image->nr_segments; i++) {
> +		if (!image->segment_cma[i]) {
> +			can_skip_checksum = false;
> +			break;
> +		}
> +	}
> +
> +	if (can_skip_checksum) {
> +		pr_info("disabling checksum verification in purgatory\n");
> +		goto skip_checksum;
> +	}
> +
>  	for (j = i = 0; i < image->nr_segments; i++) {
>  		struct kexec_segment *ksegment;
>  
> @@ -867,6 +885,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
>  		j++;
>  	}
>  
> +skip_checksum:
>  	sha256_final(&sctx, digest);

With the few nits:

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com> 

>  
>  	ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
> -- 
> 2.54.0.929.g9b7fa37559-goog
>