On 28.09.2021 18:32, Luca Fancellu wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1296,11 +1296,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> {
> read_file(dir_handle, s2w(&name), &kernel, option_str);
> efi_bs->FreePool(name.w);
> -
> - if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> - (void **)&shim_lock)) &&
> - (status = shim_lock->Verify(kernel.ptr, kernel.size)) !=
> EFI_SUCCESS )
> - PrintErrMesg(L"Dom0 kernel image could not be verified",
> status);
> }
>
> if ( !read_section(loaded_image, L"ramdisk", &ramdisk, NULL) )
> @@ -1372,6 +1367,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> if (dt_module_found < 0)
> /* efi_arch_check_dt_boot throws some error */
> blexit(L"Error processing boot modules on DT.");
> +
> + /* If Dom0 is specified, verify it */
> + if ( kernel.ptr &&
> + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> + (void **)&shim_lock)) &&
> + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS
> )
> + PrintErrMesg(L"Dom0 kernel image could not be verified", status);
Why does this code need moving in the first place? That's (to me at least)
not obvious from looking just at the common (i.e. non-Arm-specific) part.
Hence I would wish for the comment to be slightly extended to indicate
that the kernel image may also have been loaded by <whichever function>.
Also, two nits: You've broken indentation here (you've lost a space too
much compared to the original code) and ...
> /*
> * Check if a proper configuration is provided to start Xen:
> * - Dom0 specified (minimum required)
>
... you will want to insert a blank line not only before, but also after
your insertion (or, imo less desirable, neither of the two blank lines).
Further I wonder whether your addition wouldn't better be after the code
following this comment.
And finally I wonder (looking back at the earlier patch) why you use
kernel.addr there when kernel.ptr is being used here. Unless there's a
reason for the difference, being consistent would be better.
Jan
