|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 3/3] arm/efi: load dom0 modules from DT using UEFI
On Tue, 28 Sep 2021, Luca Fancellu wrote:
> Add support to load Dom0 boot modules from
> the device tree using the uefi,binary property.
>
> Update documentation about that.
>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
It is great how simple this patch is!
The patch looks all correct. Only one question: do we need a check to
make sure the dom0 ramdisk is not loaded twice? Once via uefi,binary and
another time via the config file? In other words...
> ---
> Changes in v3:
> - new patch
> ---
> docs/misc/arm/device-tree/booting.txt | 8 ++++
> docs/misc/efi.pandoc | 64 +++++++++++++++++++++++++--
> xen/arch/arm/efi/efi-boot.h | 36 +++++++++++++--
> xen/common/efi/boot.c | 12 ++---
> 4 files changed, 108 insertions(+), 12 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt
> b/docs/misc/arm/device-tree/booting.txt
> index 354bb43fe1..e73f6476d4 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -70,6 +70,14 @@ Each node contains the following properties:
> priority of this field vs. other mechanisms of specifying the
> bootargs for the kernel.
>
> +- uefi,binary (UEFI boot only)
> +
> + String property that specifies the file name to be loaded by the UEFI
> + boot for this module. If this is specified, there is no need to specify
> + the reg property because it will be created by the UEFI stub on boot.
> + This option is needed only when UEFI boot is used, the node needs to be
> + compatible with multiboot,kernel or multiboot,ramdisk.
> +
> Examples
> ========
>
> diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
> index 800e67a233..4cebc47a18 100644
> --- a/docs/misc/efi.pandoc
> +++ b/docs/misc/efi.pandoc
> @@ -167,6 +167,28 @@ sbsign \
> --output xen.signed.efi \
> xen.unified.efi
> ```
> +## UEFI boot and Dom0 modules on ARM
> +
> +When booting using UEFI on ARM, it is possible to specify the Dom0 modules
> +directly from the device tree without using the Xen configuration file, here
> an
> +example:
> +
> +chosen {
> + #size-cells = <0x1>;
> + #address-cells = <0x1>;
> + xen,xen-bootargs = "[Xen boot arguments]"
> +
> + module@1 {
> + compatible = "multiboot,kernel", "multiboot,module";
> + uefi,binary = "vmlinuz-3.0.31-0.4-xen";
> + bootargs = "[domain 0 command line options]";
> + };
> +
> + module@2 {
> + compatible = "multiboot,ramdisk", "multiboot,module";
> + uefi,binary = "initrd-3.0.31-0.4-xen";
> + };
> +}
>
> ## UEFI boot and dom0less on ARM
>
> @@ -326,10 +348,10 @@ chosen {
> ### Boot Xen, Dom0 and DomU(s)
>
> This configuration is a mix of the two configuration above, to boot this one
> -the configuration file must be processed so the /chosen node must have the
> -"uefi,cfg-load" property.
> +the configuration file can be processed or the Dom0 modules can be read from
> +the device tree.
>
> -Here an example:
> +Here the first example:
>
> Xen configuration file:
>
> @@ -369,4 +391,40 @@ chosen {
> };
> ```
>
> +Here the second example:
> +
> +Device tree:
> +
> +```
> +chosen {
> + #size-cells = <0x1>;
> + #address-cells = <0x1>;
> + xen,xen-bootargs = "[Xen boot arguments]"
> +
> + module@1 {
> + compatible = "multiboot,kernel", "multiboot,module";
> + uefi,binary = "vmlinuz-3.0.31-0.4-xen";
> + bootargs = "[domain 0 command line options]";
> + };
> +
> + module@2 {
> + compatible = "multiboot,ramdisk", "multiboot,module";
> + uefi,binary = "initrd-3.0.31-0.4-xen";
> + };
> +
> + domU1 {
> + #size-cells = <0x1>;
> + #address-cells = <0x1>;
> + compatible = "xen,domain";
> + cpus = <0x1>;
> + memory = <0x0 0xc0000>;
> + vpl011;
>
> + module@1 {
> + compatible = "multiboot,kernel", "multiboot,module";
> + uefi,binary = "Image-domu1.bin";
> + bootargs = "console=ttyAMA0 root=/dev/ram0 rw";
> + };
> + };
> +};
> +```
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 4f7c913f86..df63387136 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -31,8 +31,10 @@ static unsigned int __initdata modules_idx;
> #define ERROR_MISSING_DT_PROPERTY (-3)
> #define ERROR_RENAME_MODULE_NAME (-4)
> #define ERROR_SET_REG_PROPERTY (-5)
> +#define ERROR_DOM0_ALREADY_FOUND (-6)
> #define ERROR_DT_MODULE_DOMU (-1)
> #define ERROR_DT_CHOSEN_NODE (-2)
> +#define ERROR_DT_MODULE_DOM0 (-3)
>
> void noreturn efi_xen_start(void *fdt_ptr, uint32_t fdt_size);
> void __flush_dcache_area(const void *vaddr, unsigned long size);
> @@ -45,7 +47,8 @@ static int allocate_module_file(EFI_FILE_HANDLE dir_handle,
> static int handle_module_node(EFI_FILE_HANDLE dir_handle,
> int module_node_offset,
> int reg_addr_cells,
> - int reg_size_cells);
> + int reg_size_cells,
> + bool is_domu_module);
> static bool is_boot_module(int dt_module_offset);
> static int handle_dom0less_domain_node(EFI_FILE_HANDLE dir_handle,
> int domain_node);
> @@ -701,7 +704,8 @@ static int __init allocate_module_file(EFI_FILE_HANDLE
> dir_handle,
> static int __init handle_module_node(EFI_FILE_HANDLE dir_handle,
> int module_node_offset,
> int reg_addr_cells,
> - int reg_size_cells)
> + int reg_size_cells,
> + bool is_domu_module)
> {
> const void *uefi_name_prop;
> char mod_string[24]; /* Placeholder for module@ + a 64-bit number + \0 */
> @@ -743,6 +747,24 @@ static int __init handle_module_node(EFI_FILE_HANDLE
> dir_handle,
> return ERROR_SET_REG_PROPERTY;
> }
>
> + if ( !is_domu_module &&
> + (fdt_node_check_compatible(fdt, module_node_offset,
> + "multiboot,kernel") == 0) )
> + {
> + /*
> + * This is the Dom0 kernel, wire it to the kernel variable because it
> + * will be verified by the shim lock protocol later in the common
> code.
> + */
> + if ( kernel.addr )
> + {
> + PrintMessage(L"Dom0 kernel already found in cfg file.");
> + return ERROR_DOM0_ALREADY_FOUND;
> + }
> + kernel.need_to_free = false; /* Freed using the module array */
> + kernel.addr = file->addr;
> + kernel.size = file->size;
> + }
... is it necessary to update ramdisk as well or check if it is already
set? As far as I can tell it is the only other potential conflict that
we could have.
> return 0;
> }
>
> @@ -799,7 +821,7 @@ static int __init
> handle_dom0less_domain_node(EFI_FILE_HANDLE dir_handle,
> if ( is_boot_module(module_node) )
> {
> int ret = handle_module_node(dir_handle, module_node, addr_cells,
> - size_cells);
> + size_cells, true);
> if ( ret < 0 )
> return ret;
> }
> @@ -809,7 +831,7 @@ static int __init
> handle_dom0less_domain_node(EFI_FILE_HANDLE dir_handle,
>
> /*
> * This function checks for xen domain nodes under the /chosen node for
> possible
> - * domU guests to be loaded.
> + * dom0 and domU guests to be loaded.
> * Returns the number of modules loaded or a negative number for error.
> */
> static int __init efi_arch_check_dt_boot(EFI_FILE_HANDLE dir_handle)
> @@ -836,6 +858,12 @@ static int __init efi_arch_check_dt_boot(EFI_FILE_HANDLE
> dir_handle)
> if ( handle_dom0less_domain_node(dir_handle, node) < 0 )
> return ERROR_DT_MODULE_DOMU;
> }
> + else if ( is_boot_module(node) )
> + {
> + if ( handle_module_node(dir_handle, node, addr_len, size_len,
> + false) < 0 )
> + return ERROR_DT_MODULE_DOM0;
> + }
> }
>
> /* Free dom0less file names if any */
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index c8c57fbb54..b221494a06 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1296,11 +1296,6 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> {
> read_file(dir_handle, s2w(&name), &kernel, option_str);
> efi_bs->FreePool(name.w);
> -
> - if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> - (void **)&shim_lock)) &&
> - (status = shim_lock->Verify(kernel.ptr, kernel.size)) !=
> EFI_SUCCESS )
> - PrintErrMesg(L"Dom0 kernel image could not be verified",
> status);
> }
>
> if ( !read_section(loaded_image, L"ramdisk", &ramdisk, NULL) )
> @@ -1372,6 +1367,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> if (dt_module_found < 0)
> /* efi_arch_check_dt_boot throws some error */
> blexit(L"Error processing boot modules on DT.");
> +
> + /* If Dom0 is specified, verify it */
> + if ( kernel.ptr &&
> + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,
> + (void **)&shim_lock)) &&
> + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS
> )
> + PrintErrMesg(L"Dom0 kernel image could not be verified", status);
> /*
> * Check if a proper configuration is provided to start Xen:
> * - Dom0 specified (minimum required)
> --
> 2.17.1
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |