Re: [PATCH 1/2] xen/efi: Restrict check for DT boot modules on EFI boot

[Jan Beulich <jbeulich@suse.com>]

On 16.09.2021 02:16, Stefano Stabellini wrote:
> So I am thinking that we have no choice but introducing a new property
> to tell us whether we should or should not load xen.cfg when
> multiboot,modules are present.
> 
> Taking inspiration from HyperLaunch, it could be a new node:
> 
> chosen {
>     cfg {
>         compatible = "xen,uefi-config", "multiboot,module";
>         uefi,binary = "xen.cfg";
>     };
> };
> 
> In efi_arch_use_config_file we would check if there are any nodes
> compatible with "multiboot,module". If there are none, it returns true.
> 
> If there are any, and one of them is also compatible "xen,uefi-config",
> then efi_arch_use_config_file returns true and also the specified
> configuration filename.
> 
> If there are nodes compatible to "multiboot,module" but none of them is
> compatible to "module,uefi-config", then efi_arch_use_config_file
> returns false. We use the device tree only.
> 
> I think that would be clearer and more consistent from a specification
> perspective, but it requires one change in common code:
> efi_arch_use_config_file would not just return bool but it would also
> return a filename if found (it could be a pointer parameter to the
> function).
> 
> 
> Otherwise, we could add a simple property like the following, without a
> specific value and without a filename:
> 
> chosen {
>     xen,uefi-config;
> };
> 
> The presence of xen,uefi-config could mean that Xen should go through
> the usual guessing game to figure out the right cfg file to load. This
> would not require any common code changes because
> efi_arch_use_config_file could simply return bool as it does today.
> 
> My preference is the xen,uefi-config compatible node, but I think the
> property would also work.
> 
> 
> Jan, do you have an opinion on whether efi_arch_use_config_file has to
> stay as it is today, or would you be open to the possibility of making
> efi_arch_use_config_file return a filename too?

I see no issue with making such a change; my preference would be an
altered return type, provided all present cases can be expressed
alongside the new one you need.

Jan