Re: Arm EFI boot issue for Dom0 module listed inside subnode of chosen

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 2 Nov 2021, Luca Fancellu wrote:
> Hi all,
> 
> We recently discovered that there is a way to list Dom0 modules that is not 
> supported by the EFI boot,
> It’s happened browsing some Wiki pages like this one:
> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/Lager
> 
> In that page the Dom0 modules are listed inside a subnode of the /chosen node:
> 
> chosen {
> 
>     modules {
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         module@0x72000000 {
>             compatible = "multiboot,kernel", "multiboot,module";
>             reg = <0x72000000 0x2fd158>;
>         };
> 
>         module@0x74000000 {
>             compatible = "xen,xsm-policy", "multiboot,module";
>             reg = <0x74000000 0x2559>;
>         };
>     };
> };
> 
> Instead for how it is implemented now in the EFI code and described in:
> 1) https://xenbits.xen.org/docs/unstable/misc/arm/device-tree/booting.txt
> 2) https://xenbits.xen.org/docs/unstable/misc/efi.html
> 
> Only the following approach is supported, so Dom0 modules must be a direct 
> child of /chosen:
> 
> chosen {
>     #address-cells = <1>;
>     #size-cells = <1>;
> 
>     module@0x72000000 {
>         compatible = "multiboot,kernel", "multiboot,module";
>         reg = <0x72000000 0x2fd158>;
>     };
> 
>     module@0x74000000 {
>         compatible = "xen,xsm-policy", "multiboot,module";
>         reg = <0x74000000 0x2559>;
>     };
> };
> 
> Is this a problem that needs a fix?


Let me start by saying that I don't feel strongly either way, so I am
happy to go with other people's opinion on this one.

In this kind of situations I usually look at two things:
- what the specification says
- what the existing code actually does

In general, I try to follow the specification unless obviously
production code relies on something that contradicts the spec, in which
case I'd say to update the spec.

In this case, although it is true that "modules" could be nice to have,
it is missing a compatible string, it is not described in
arm/device-tree/booting.txt, and it is only rarely used.

For these reasons, I don't think it is a problem that we need to fix.
Especially considering that the EFI case is the only case not working
and it was never supported until now.

If we want to add support for "modules", that could be fine, but I think
we should describe it in arm/device-tree/booting.txt and also add a
compatible string for it. For 4.16 I'd just update the wikipage.