Re: [PATCH v6 2/2] arm/efi: load dom0 modules from DT using UEFI

[Julien Grall <julien@xxxxxxx>]

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 :).
Cheers,

--
Julien Grall