| Message ID | 20210930142846.13348-4-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm/efi: Add dom0less support to UEFI boot | expand |
Hi Luca, > On 30 Sep 2021, at 15:28, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > Add support to load Dom0 boot modules from > the device tree using the xen,uefi-binary property. > > Update documentation about that. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > Changes in v4: > - Add check to avoid double definition of Dom0 ramdisk > from cfg file and DT > - Fix if conditions indentation in boot.c > - Moved Dom0 kernel verification code after check for > presence for Dom0 or DomU(s) > - Changed uefi,binary property to xen,uefi-binary > 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 | 47 ++++++++++++++++++-- > xen/common/efi/boot.c | 16 ++++--- > 4 files changed, 123 insertions(+), 12 deletions(-) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 7258e7e1ec..8e0c327ba4 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 876cd55005..4abbb5bb82 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"; > + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; > + bootargs = "[domain 0 command line options]"; > + }; > + > + module@2 { > + compatible = "multiboot,ramdisk", "multiboot,module"; > + xen,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 > -"xen,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"; > + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; > + bootargs = "[domain 0 command line options]"; > + }; > + > + module@2 { > + compatible = "multiboot,ramdisk", "multiboot,module"; > + xen,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"; > + xen,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 50b3829ae0..b122e2846a 100644 > --- a/xen/arch/arm/efi/efi-boot.h > +++ b/xen/arch/arm/efi/efi-boot.h > @@ -31,8 +31,11 @@ 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_DOM0_RAMDISK_FOUND (-7) > #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 +48,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 +705,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 +748,34 @@ static int __init handle_module_node(EFI_FILE_HANDLE dir_handle, > return ERROR_SET_REG_PROPERTY; > } > > + if ( !is_domu_module ) > + { > + if ( (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; > + } > + else if ( ramdisk.addr && > + (fdt_node_check_compatible(fdt, module_node_offset, > + "multiboot,ramdisk") == 0) ) > + { > + PrintMessage(L"Dom0 ramdisk already found in cfg file."); > + return ERROR_DOM0_RAMDISK_FOUND; > + } > + } > + > return 0; > } > > @@ -799,7 +832,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 +842,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 +869,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 boot modules file names if any */ > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index daf7c26099..e4295dbe34 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) ) > @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > if ( !dt_modules_found && !kernel.addr ) > blexit(L"No Dom0 kernel image specified."); > > + /* > + * The Dom0 kernel can be loaded from the configuration file or by the > + * device tree through the efi_arch_check_dt_boot function, in this stage > + * verify it. > + */ > + if ( kernel.addr && > + !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); > + > efi_arch_edd(); > > /* XXX Collect EDID info. */ > -- > 2.17.1 >
On Thu, 30 Sep 2021, Luca Fancellu wrote: > Add support to load Dom0 boot modules from > the device tree using the xen,uefi-binary property. > > Update documentation about that. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > Changes in v4: > - Add check to avoid double definition of Dom0 ramdisk > from cfg file and DT > - Fix if conditions indentation in boot.c > - Moved Dom0 kernel verification code after check for > presence for Dom0 or DomU(s) > - Changed uefi,binary property to xen,uefi-binary > 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 | 47 ++++++++++++++++++-- > xen/common/efi/boot.c | 16 ++++--- > 4 files changed, 123 insertions(+), 12 deletions(-) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 7258e7e1ec..8e0c327ba4 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) Needs to be renamed, but it could be done on commit: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > + 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 876cd55005..4abbb5bb82 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"; > + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; > + bootargs = "[domain 0 command line options]"; > + }; > + > + module@2 { > + compatible = "multiboot,ramdisk", "multiboot,module"; > + xen,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 > -"xen,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"; > + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; > + bootargs = "[domain 0 command line options]"; > + }; > + > + module@2 { > + compatible = "multiboot,ramdisk", "multiboot,module"; > + xen,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"; > + xen,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 50b3829ae0..b122e2846a 100644 > --- a/xen/arch/arm/efi/efi-boot.h > +++ b/xen/arch/arm/efi/efi-boot.h > @@ -31,8 +31,11 @@ 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_DOM0_RAMDISK_FOUND (-7) > #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 +48,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 +705,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 +748,34 @@ static int __init handle_module_node(EFI_FILE_HANDLE dir_handle, > return ERROR_SET_REG_PROPERTY; > } > > + if ( !is_domu_module ) > + { > + if ( (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; > + } > + else if ( ramdisk.addr && > + (fdt_node_check_compatible(fdt, module_node_offset, > + "multiboot,ramdisk") == 0) ) > + { > + PrintMessage(L"Dom0 ramdisk already found in cfg file."); > + return ERROR_DOM0_RAMDISK_FOUND; > + } > + } > + > return 0; > } > > @@ -799,7 +832,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 +842,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 +869,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 boot modules file names if any */ > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index daf7c26099..e4295dbe34 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) ) > @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > if ( !dt_modules_found && !kernel.addr ) > blexit(L"No Dom0 kernel image specified."); > > + /* > + * The Dom0 kernel can be loaded from the configuration file or by the > + * device tree through the efi_arch_check_dt_boot function, in this stage > + * verify it. > + */ > + if ( kernel.addr && > + !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); > + > efi_arch_edd(); > > /* XXX Collect EDID info. */ > -- > 2.17.1 >
On 30.09.2021 16:28, Luca Fancellu wrote: > Add support to load Dom0 boot modules from > the device tree using the xen,uefi-binary property. > > Update documentation about that. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com> despite ... > @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > if ( !dt_modules_found && !kernel.addr ) > blexit(L"No Dom0 kernel image specified."); > > + /* > + * The Dom0 kernel can be loaded from the configuration file or by the > + * device tree through the efi_arch_check_dt_boot function, in this stage > + * verify it. > + */ > + if ( kernel.addr && ... me still being a little unhappy with the inconsistent use of the union fields so close together: This one is now consistent with the one visible further up in context, but ... > + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,> + (void **)&shim_lock)) && > + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) ... is now inconsistent with this use. But yeah - read_file() is even worse in that sense, except that there the different uses are for specific reasons, while here the only requirement is to satisfy shim_lock->Verify(). Please feel free to retain my ack in case you decide to use .ptr in all three places. Jan
> On 1 Oct 2021, at 12:16, Jan Beulich <jbeulich@suse.com> wrote: > > On 30.09.2021 16:28, Luca Fancellu wrote: >> Add support to load Dom0 boot modules from >> the device tree using the xen,uefi-binary property. >> >> Update documentation about that. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > despite ... > >> @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) >> if ( !dt_modules_found && !kernel.addr ) >> blexit(L"No Dom0 kernel image specified."); >> >> + /* >> + * The Dom0 kernel can be loaded from the configuration file or by the >> + * device tree through the efi_arch_check_dt_boot function, in this stage >> + * verify it. >> + */ >> + if ( kernel.addr && > > ... me still being a little unhappy with the inconsistent use of the > union fields so close together: This one is now consistent with the > one visible further up in context, but ... > >> + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,> + (void **)&shim_lock)) && >> + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) > > ... is now inconsistent with this use. But yeah - read_file() is > even worse in that sense, except that there the different uses are > for specific reasons, while here the only requirement is to satisfy > shim_lock->Verify(). > > Please feel free to retain my ack in case you decide to use .ptr in > all three places. Hi Jan, Sure I will do the modification you suggested, I will fix also my silly mistake that Stefano pointed out. Just to be sure, I explain what I will do: In the second patch I will change: if ( !dt_modules_found && !kernel.addr ) To if ( !dt_modules_found && !kernel.ptr ) And in this patch I will use: 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); Do you agree on them? Can I retain your ack to this patch doing these changes? Cheers, Luca > > Jan >
On 01.10.2021 16:08, Luca Fancellu wrote: > > >> On 1 Oct 2021, at 12:16, Jan Beulich <jbeulich@suse.com> wrote: >> >> On 30.09.2021 16:28, Luca Fancellu wrote: >>> Add support to load Dom0 boot modules from >>> the device tree using the xen,uefi-binary property. >>> >>> Update documentation about that. >>> >>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> >> Acked-by: Jan Beulich <jbeulich@suse.com> >> despite ... >> >>> @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) >>> if ( !dt_modules_found && !kernel.addr ) >>> blexit(L"No Dom0 kernel image specified."); >>> >>> + /* >>> + * The Dom0 kernel can be loaded from the configuration file or by the >>> + * device tree through the efi_arch_check_dt_boot function, in this stage >>> + * verify it. >>> + */ >>> + if ( kernel.addr && >> >> ... me still being a little unhappy with the inconsistent use of the >> union fields so close together: This one is now consistent with the >> one visible further up in context, but ... >> >>> + !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL,> + (void **)&shim_lock)) && >>> + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) >> >> ... is now inconsistent with this use. But yeah - read_file() is >> even worse in that sense, except that there the different uses are >> for specific reasons, while here the only requirement is to satisfy >> shim_lock->Verify(). >> >> Please feel free to retain my ack in case you decide to use .ptr in >> all three places. > > Hi Jan, > > Sure I will do the modification you suggested, I will fix also my silly mistake that > Stefano pointed out. > > Just to be sure, I explain what I will do: > > In the second patch I will change: > > if ( !dt_modules_found && !kernel.addr ) > > To > > if ( !dt_modules_found && !kernel.ptr ) > > > And in this patch I will use: > > 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); > > Do you agree on them? Yes and ... > Can I retain your ack to this patch doing these changes? ... as previously said, yes. Jan
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 7258e7e1ec..8e0c327ba4 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 876cd55005..4abbb5bb82 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"; + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; + bootargs = "[domain 0 command line options]"; + }; + + module@2 { + compatible = "multiboot,ramdisk", "multiboot,module"; + xen,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 -"xen,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"; + xen,uefi-binary = "vmlinuz-3.0.31-0.4-xen"; + bootargs = "[domain 0 command line options]"; + }; + + module@2 { + compatible = "multiboot,ramdisk", "multiboot,module"; + xen,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"; + xen,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 50b3829ae0..b122e2846a 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -31,8 +31,11 @@ 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_DOM0_RAMDISK_FOUND (-7) #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 +48,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 +705,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 +748,34 @@ static int __init handle_module_node(EFI_FILE_HANDLE dir_handle, return ERROR_SET_REG_PROPERTY; } + if ( !is_domu_module ) + { + if ( (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; + } + else if ( ramdisk.addr && + (fdt_node_check_compatible(fdt, module_node_offset, + "multiboot,ramdisk") == 0) ) + { + PrintMessage(L"Dom0 ramdisk already found in cfg file."); + return ERROR_DOM0_RAMDISK_FOUND; + } + } + return 0; } @@ -799,7 +832,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 +842,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 +869,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 boot modules file names if any */ diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index daf7c26099..e4295dbe34 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) ) @@ -1385,6 +1380,17 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) if ( !dt_modules_found && !kernel.addr ) blexit(L"No Dom0 kernel image specified."); + /* + * The Dom0 kernel can be loaded from the configuration file or by the + * device tree through the efi_arch_check_dt_boot function, in this stage + * verify it. + */ + if ( kernel.addr && + !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); + efi_arch_edd(); /* XXX Collect EDID info. */
Add support to load Dom0 boot modules from the device tree using the xen,uefi-binary property. Update documentation about that. Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- Changes in v4: - Add check to avoid double definition of Dom0 ramdisk from cfg file and DT - Fix if conditions indentation in boot.c - Moved Dom0 kernel verification code after check for presence for Dom0 or DomU(s) - Changed uefi,binary property to xen,uefi-binary 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 | 47 ++++++++++++++++++-- xen/common/efi/boot.c | 16 ++++--- 4 files changed, 123 insertions(+), 12 deletions(-)