[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 3/3] arm/efi: load dom0 modules from DT using UEFI
> On 29 Sep 2021, at 09:00, Jan Beulich <jbeulich@xxxxxxxx> wrote: > > 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>. > Sure I will improve the comment to explain that. > Also, two nits: You've broken indentation here (you've lost a space too > much compared to the original code) and ... Yes sorry for that, we didn’t see it, I will fix it. > >> /* >> * 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. I will do all of the above for the v4. Thanks for all the feedbacks. Cheers, Luca > > Jan >
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |