Re: [PATCH v5 1/2] arm/efi: Use dom0less configuration when using EFI boot

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jan,

> On 11 Oct 2021, at 09:52, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 11.10.2021 10:50, Luca Fancellu wrote:
>>> On 11 Oct 2021, at 09:11, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 11.10.2021 10:03, Luca Fancellu wrote:
>>>> This patch introduces the support for dom0less configuration
>>>> when using UEFI boot on ARM, it permits the EFI boot to
>>>> continue if no dom0 kernel is specified but at least one domU
>>>> is found.
>>>> 
>>>> Introduce the new property "xen,uefi-binary" for device tree boot
>>>> module nodes that are subnode of "xen,domain" compatible nodes.
>>>> The property holds a string containing the file name of the
>>>> binary that shall be loaded by the uefi loader from the filesystem.
>>>> 
>>>> Introduce a new call efi_check_dt_boot(...) called during EFI boot
>>>> that checks for module to be loaded using device tree.
>>>> Architectures that don't support device tree don't have to
>>>> provide this function.
>>>> 
>>>> Update efi documentation about how to start a dom0less
>>>> setup using UEFI
>>>> 
>>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>>>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> 
>>> Did you get indication that these are fine to retain with ...
>>> 
>>>> ---
>>>> Changes in v5:
>>>> - Removed unneeded variable initialization
>>>> - Fixed comment
>>>> - Fixed error message for the absence of an initial domain kernel
>>>> - changed efi_arch_check_dt_boot to efi_check_dt_boot and add
>>>> a stub if CONFIG_HAS_DEVICE_TREE is not declared, updated commit
>>>> message about the call introduction in the EFI boot flow.
>>> 
>>> ... all of these changes? Every individual change may be minor enough,
>>> but their sum makes me wonder. If so (or if at least one of the two
>>> gets re-offered)
>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> albeit preferably with ...
>>> 
>>>> --- a/xen/common/efi/boot.c
>>>> +++ b/xen/common/efi/boot.c
>>>> @@ -166,6 +166,13 @@ static void __init PrintErr(const CHAR16 *s)
>>>>    StdErr->OutputString(StdErr, (CHAR16 *)s );
>>>> }
>>>> 
>>>> +#ifndef CONFIG_HAS_DEVICE_TREE
>>>> +static inline int __init efi_check_dt_boot(EFI_FILE_HANDLE dir_handle)
>>> 
>>> ... the "inline" here dropped. We don't normally add this outside of
>>> headers, leaving it to the compiler to decide. In headers it's wanted
>>> to avoid "defined by never used" style warnings.
>> 
>> Ok I can drop it in a next serie and retain your Ack, or is it something that
>> can be done on commit? 
> 
> I guess that's easy enough to do while committing. Provided of course
> the two R-b get confirmed.

I confirm my R-b.

Cheers
Bertrand

> 
> Jan