[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Luca Fancellu <luca.fancellu@xxxxxxx>
  • Date: Tue, 12 Oct 2021 09:07:24 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7LqK2djBglZ6W876FhQQNQJ/B9g3vWex0Q3gE7gNLhs=; b=BrAR9eILzIvTnTdTo7lK7M36FkEfWCqE7nZk0GXDCtPZTqZu40Ko4Yv+lT8PjtCD5ozRZcU4Rr2wV1C8cBsBYygKdy6rAMXC7yCuUlgyzee7f3BrK+Ylqx6rhQ4ojGZkbnrZkzBWq89p609w+2dCDGktsHZrEt4fHU+nnxXAXMkw+LNqwdHZsLazAA8kYxtelcBKdZ++Y+K8fBmJ9VoJPwgIp9Grsk/8CZN6k81sbgAzYXdtqYYYiq++uvPLT5Wn+qjflaap4trh/MI7nCFmheBSbKtEa9Ewv2PllYNZqNtxXbSNy2kJlmryQicmcWT7dOlKAlcjCjT/k+M6+Nv3hg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SCIImYdmKt5vEppc89yj143IAEg9McjGhXlvze647EDhTdHSUBJCXuUVTiztDWskAX1fyzwFHP8jbhDkzCe8uTVtCBL++cm9ogziHTrF4gRXLwBX2GZcSDc3RpDHzlBDjLvNzLoLH7fuq2rodRCGFsk6w+/ekmytPYPw/uc/fFoNgk8/PkoR48MD+G+2/tzHgXCQMkaZE3Qk2aNM7VYycb6M2V/S1JIC+Lx6foKAg3I77Slj2uT+NdNl4/DjvY536LcztfCj2icavaGHyroYQ3brno2sAC5a/aZfkaoEqGFJnuVc/VDFLjte+CASx4UE0LAsKG/4DbBKc0PQVvco+Q==
  • Authentication-results-original: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <julien@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Bertrand Marquis <bertrand.marquis@xxxxxxx>, wei.chen@xxxxxxx, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Tue, 12 Oct 2021 08:07:50 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;

> On 12 Oct 2021, at 02:31, Stefano Stabellini <sstabellini@xxxxxxxxxx> 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 
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 
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 
+       strings are not supported.
        - "xen,multiboot-module" equivalent to "multiboot,module"
        - "xen,linux-zimage"     equivalent to "multiboot,kernel”

What do you think about that?




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.