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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 11 Oct 2021, Bertrand Marquis wrote:
> 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.

I also confirm my reviewed-by. Also, I am aware of the change suggested
by Julien about using multiboot,module for the is_boot_module check and
I am fine with it too.