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

From: Julien Grall <julien@xen.org>
To: Luca Fancellu <luca.fancellu@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Jan Beulich <jbeulich@suse.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	wei.chen@arm.com, Ian Jackson <iwj@xenproject.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH-4.16 v2] xen/efi: Fix Grub2 boot on arm64
Date: Mon, 15 Nov 2021 18:57:24 +0000	[thread overview]
Message-ID: <a07ff375-b910-0d73-e957-15ba9d2535fe@xen.org> (raw)
In-Reply-To: <725F3F68-A1CE-42FB-9C8A-0700CDD02E8E@arm.com>

Hi Luca,

On 10/11/2021 14:02, Luca Fancellu wrote:
> 
> 
>> On 10 Nov 2021, at 13:36, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Luca,
>>
>> On 10/11/2021 13:05, Luca Fancellu wrote:
>>> I thought having the EFI_FILE_HANDLE global in efi-boot.h was a “no go”, but if it’s not then instead of
>>> calling get_parent_handle in efi_check_dt_boot (that is the main issue with EDK2+Grub2), we can do
>>> something like this:
>>
>> fs_dir_handle is only used by callees of efi_check_boot_dt_boot(). So the global variable is not an option for me because the risk is not worth it (it is easy to misuse a global variable).
>>
>> Instead, I think fs_dir_handle should be an argument of allocate_module_file() and propagated up to the first call in efi_check_dt_boot().
>>
> 
> Yes you are right, changing the interface of handle_dom0less_domain_node, handle_module_node, allocate_module_file to host also an argument EFI_FILE_HANDLE *dir_handle
> avoids the use of the global, then the handle is requested in allocate_module_file only once and closed in efi_check_dt_boot only if it’s not null.

That would indeed be better. I'd like this patch to be merged in 4.16. 
Would you be able to send a new version in the next couple of days?

Cheers,

-- 
Julien Grall