> On 12 Oct 2021, at 02:31, Stefano Stabellini <sstabell...@kernel.org> wrote:
>
> On Mon, 11 Oct 2021, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 11/10/2021 22:24, Stefano Stabellini wrote:
>>>> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
>>>> index 840728d6c0..076b827bdd 100644
>>>> --- a/xen/arch/arm/efi/efi-boot.h
>>>> +++ b/xen/arch/arm/efi/efi-boot.h
>>>> @@ -713,10 +713,12 @@ static int __init handle_module_node(EFI_FILE_HANDLE
>>>> dir_handle,
>>>> char mod_string[24]; /* Placeholder for module@ + a 64-bit number +
>>>> \0 */
>>>> int uefi_name_len, file_idx, module_compat;
>>>> module_name *file;
>>>> + const char *compat_string = is_domu_module ? "multiboot,module" :
>>>> + "xen,multiboot-module";
>>>> /* Check if the node is a multiboot,module otherwise return */
>>>> module_compat = fdt_node_check_compatible(fdt, module_node_offset,
>>>> - "multiboot,module");
>>>> + compat_string);
>>>> if ( module_compat < 0 )
>>>> /* Error while checking the compatible string */
>>>> return ERROR_CHECK_MODULE_COMPAT;
>>>
>>>
>>> Well... not exactly like this because this would stop a normal
>>> "multiboot,module" dom0 kernel from being recognized.
>>>
>>> So we need for domU: only "multiboot,module"
>>> For Dom0, either "multiboot,module" or "xen,multiboot-module"
>>
>> Looking at the history, xen,multiboot-module has been considered as a legacy
>> binding since before UEFI was introduced. In fact, without this series, I
>> believe, there is limited reasons for the compatible to be present in the DT
>> as you would either use grub (which use the new compatible) or xen.cfg (the
>> stub will create the node).
>>
>> So I would argue that this compatible should not be used in combination with
>> UEFI and therefore we should not handle it. This would make the code simpler
>> :).
>
Hi Stefano,
> What you suggested is a viable option, however ImageBuilder is still
> using the "xen,multiboot-module" format somehow today (no idea why) and
> we have the following written in docs/misc/arm/device-tree/booting.txt:
>
> Xen 4.4 supported a different set of legacy compatible strings
> which remain supported such that systems supporting both 4.4
> and later can use a single DTB.
>
> - "xen,multiboot-module" equivalent to "multiboot,module"
> - "xen,linux-zimage" equivalent to "multiboot,kernel"
> - "xen,linux-initrd" equivalent to "multiboot,ramdisk"
>
> For compatibility with Xen 4.4 the more specific "xen,linux-*"
> names are non-optional and must be included.
>
> My preference is to avoid breaking compatibility (even with UEFI
> booting). The way I suggested above is one way to do it.
>
> But I don't feel strongly about this at all, I am fine with ignoring
> "xen,multiboot-module" in the EFI stub. I can get ImageBuilder fixed
> very quickly (I should do that in any case). If we are going to ignore
> "xen,multiboot-module" then we probably want to update the text in
> docs/misc/arm/device-tree/booting.txt also.
The changes to support legacy compatible strings can be done but it will result
in
complex code, I would go for Julien suggestion to just drop it for UEFI.
I can add a note on docs/misc/arm/device-tree/booting.txt saying that for UEFI
boot
the legacy strings are not supported.
Something like:
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -51,6 +51,8 @@ Each node contains the following properties:
Xen 4.4 supported a different set of legacy compatible strings
which remain supported such that systems supporting both 4.4
and later can use a single DTB.
+ However when booting Xen using UEFI and Device Tree, the legacy
compatible
+ strings are not supported.
- "xen,multiboot-module" equivalent to "multiboot,module"
- "xen,linux-zimage" equivalent to "multiboot,kernel”
What do you think about that?
Cheers,
Luca
