[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH-4.16 v2] xen/efi: Fix Grub2 boot on arm64
On Fri, 5 Nov 2021, Jan Beulich wrote: > On 04.11.2021 22:50, Stefano Stabellini wrote: > > On Thu, 4 Nov 2021, Luca Fancellu wrote: > >>> On 4 Nov 2021, at 21:35, Stefano Stabellini <sstabellini@xxxxxxxxxx> > >>> wrote: > >>> On Thu, 4 Nov 2021, Luca Fancellu wrote: > >>>>> On 4 Nov 2021, at 20:56, Stefano Stabellini <sstabellini@xxxxxxxxxx> > >>>>> wrote: > >>>>> @@ -851,10 +853,14 @@ static int __init > >>>>> handle_dom0less_domain_node(EFI_FILE_HANDLE dir_handle, > >>>>> * dom0 and domU guests to be loaded. > >>>>> * Returns the number of multiboot modules found or a negative number > >>>>> for error. > >>>>> */ > >>>>> -static int __init efi_check_dt_boot(EFI_FILE_HANDLE dir_handle) > >>>>> +static int __init efi_check_dt_boot(EFI_LOADED_IMAGE *loaded_image) > >>>>> { > >>>>> int chosen, node, addr_len, size_len; > >>>>> unsigned int i = 0, modules_found = 0; > >>>>> + EFI_FILE_HANDLE dir_handle; > >>>>> + CHAR16 *file_name; > >>>>> + > >>>>> + dir_handle = get_parent_handle(loaded_image, &file_name); > >>>> > >>>> We can’t use get_parent_handle here because we will end up with the same > >>>> problem, > >>>> we would need to use the filesystem if and only if we need to use it, > >>> > >>> Understood, but it would work the same way as this version of the patch: > >>> if we end up calling read_file with dir_handle == NULL, then read_file > >>> would do: > >>> > >>> blexit(L"Error: No access to the filesystem"); > >>> > >>> If we don't end up calling read_file, then everything works even if > >>> dir_handle == NULL. Right? > >> > >> Oh yes sorry my bad Stefano! Having this version of the patch, it will > >> work. > >> > >> My understanding was instead that the Jan suggestion is to revert the place > >> of call of get_parent_handle (like in your code diff), but also to remove > >> the > >> changes to get_parent_handle and read_file. > >> I guess Jan will specify his preference, but if he meant the last one, then > >> the only way I see... > > > > I think we should keep the changes to get_parent_handle and read_file, > > otherwise it will make it awkward, and those changes are good in their > > own right anyway. > > As a maintainer of this code I'm afraid I have to say that I disagree. > These changes were actually part of the reason why I went and looked > how things could (and imo ought to be) done differently. You know this code and EFI booting better than me -- aren't you concerned about Xen calling get_parent_handle / dir_handle->Close so many times (by allocate_module_file)? My main concern is performance and resource utilization. With v3 of the patch get_parent_handle will get called for every module to be loaded. With dom0less, it could easily get called 10 times or more. Taking a look at get_parent_handle, the Xen side doesn't seem small and I have no idea how the EDK2 side looks. I am just worried that it would actually have an impact on boot times (also depending on the bootloader implementation).
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |