Re: [PATCH-4.16 v2] xen/efi: Fix Grub2 boot on arm64

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan