> On Tue, 28 Sep 2021, Luca Fancellu wrote:
>> Introduce the uefi,cfg-load DT property of /chosen
>> node for ARM whose presence decide whether to force
>> the load of the UEFI Xen configuration file.
>>
>> The logic is that if any multiboot,module is found in
>> the DT, then the uefi,cfg-load property is used to see
>> if the UEFI Xen configuration file is needed.
>>
>> Modify a comment in efi_arch_use_config_file, removing
>> the part that states "dom0 required" because it's not
>> true anymore with this commit.
>>
>> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
>
> The patch looks good. Only one minor change: given that this is a Xen
> parameter that we are introducing and not a parameter defined by UEFI
> Forum, I think uefi,cfg-load should be called xen,uefi-cfg-load instead.
> Because "xen," is our prefix, while "uefi," is not.
>
> With that minor change:
>
> Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
Yes I will rename it.
>
>
> Note that the uefi,binary property is different because that property is
> for xen,domain nodes, so we are already in a Xen specific namespace when
> we are using it. Instead this property is for /chosen which is not a Xen
> specific node.
Given that uefi,binary will be used also for multiboot,module directly under
/chosen
for Dom0, do you think I should rename also that to xen,uefi-binary?
Cheers,
Luca
>
>
>
>> ---
>> v3 changes:
>> - add documentation to misc/arm/device-tree/booting.txt
>> - Modified variable name and logic from skip_cfg_file to
>> load_cfg_file
>> - Add in the commit message that I'm modifying a comment.
>> v2 changes:
>> - Introduced uefi,cfg-load property
>> - Add documentation about the property
>> ---
>> docs/misc/arm/device-tree/booting.txt | 8 ++++++++
>> docs/misc/efi.pandoc | 2 ++
>> xen/arch/arm/efi/efi-boot.h | 28 ++++++++++++++++++++++-----
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/misc/arm/device-tree/booting.txt
>> b/docs/misc/arm/device-tree/booting.txt
>> index 44cd9e1a9a..cf878b478e 100644
>> --- a/docs/misc/arm/device-tree/booting.txt
>> +++ b/docs/misc/arm/device-tree/booting.txt
>> @@ -121,6 +121,14 @@ A Xen-aware bootloader would set xen,xen-bootargs for
>> Xen, xen,dom0-bootargs
>> for Dom0 and bootargs for native Linux.
>>
>>
>> +UEFI boot and DT
>> +================
>> +
>> +When Xen is booted using UEFI, it doesn't read the configuration file if any
>> +multiboot module is specified. To force Xen to load the configuration file,
>> the
>> +boolean property uefi,cfg-load must be declared in the /chosen node.
>> +
>> +
>> Creating Multiple Domains directly from Xen
>> ===========================================
>>
>> diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
>> index ac3cd58cae..e289c5e7ba 100644
>> --- a/docs/misc/efi.pandoc
>> +++ b/docs/misc/efi.pandoc
>> @@ -14,6 +14,8 @@ loaded the modules and describes them in the device tree
>> provided to Xen. If a
>> bootloader provides a device tree containing modules then any configuration
>> files are ignored, and the bootloader is responsible for populating all
>> relevant device tree nodes.
>> +The property "uefi,cfg-load" can be specified in the /chosen node to force
>> Xen
>> +to load the configuration file even if multiboot modules are found.
>>
>> Once built, `make install-xen` will place the resulting binary directly into
>> the EFI boot partition, provided `EFI_VENDOR` is set in the environment (and
>> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
>> index cf9c37153f..4f1b01757d 100644
>> --- a/xen/arch/arm/efi/efi-boot.h
>> +++ b/xen/arch/arm/efi/efi-boot.h
>> @@ -581,22 +581,40 @@ static void __init
>> efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image)
>>
>> static bool __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>> {
>> + bool load_cfg_file = true;
>> /*
>> * For arm, we may get a device tree from GRUB (or other bootloader)
>> * that contains modules that have already been loaded into memory. In
>> - * this case, we do not use a configuration file, and rely on the
>> - * bootloader to have loaded all required modules and appropriate
>> - * options.
>> + * this case, we search for the property uefi,cfg-load in the /chosen
>> node
>> + * to decide whether to skip the UEFI Xen configuration file or not.
>> */
>>
>> fdt = lookup_fdt_config_table(SystemTable);
>> dtbfile.ptr = fdt;
>> dtbfile.need_to_free = false; /* Config table memory can't be freed. */
>> - if ( !fdt || fdt_node_offset_by_compatible(fdt, 0, "multiboot,module")
>> < 0 )
>> +
>> + if ( fdt_node_offset_by_compatible(fdt, 0, "multiboot,module") > 0 )
>> + {
>> + /* Locate chosen node */
>> + int node = fdt_subnode_offset(fdt, 0, "chosen");
>> + const void *cfg_load_prop;
>> + int cfg_load_len;
>> +
>> + if ( node > 0 )
>> + {
>> + /* Check if uefi,cfg-load property exists */
>> + cfg_load_prop = fdt_getprop(fdt, node, "uefi,cfg-load",
>> + &cfg_load_len);
>> + if ( !cfg_load_prop )
>> + load_cfg_file = false;
>> + }
>> + }
>> +
>> + if ( !fdt || load_cfg_file )
>> {
>> /*
>> * We either have no FDT, or one without modules, so we must have a
>> - * Xen EFI configuration file to specify modules. (dom0 required)
>> + * Xen EFI configuration file to specify modules.
>> */
>> return true;
>> }
>> --
>> 2.17.1
>>
