Re: [PATCH] arm/efi: Handle Xen bootargs from both xen.cfg and DT

[Luca Fancellu <luca.fancellu@xxxxxxx>]

> On 8 Dec 2021, at 18:10, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 06/12/2021 15:36, Luca Fancellu wrote:
>> Currently the Xen UEFI stub can accept Xen boot arguments from
>> the Xen configuration file using the "options=" keyword, but also
>> directly from the device tree specifying xen,xen-bootargs
>> property.
>> When the configuration file is used, device tree boot arguments
>> are ignored and overwritten even if the keyword "options=" is
>> not used.
>> This patch handle this case, if xen,xen-bootargs is found in the
>> device tree, it is used for xen boot arguments regardless they
>> are specified in the Xen configuration file or not.
>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>> ---
>>  docs/misc/efi.pandoc        | 4 ++++
>>  xen/arch/arm/efi/efi-boot.h | 7 +++++++
>>  2 files changed, 11 insertions(+)
>> diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
>> index abafb3452758..b7d99de87f15 100644
>> --- a/docs/misc/efi.pandoc
>> +++ b/docs/misc/efi.pandoc
>> @@ -249,6 +249,10 @@ UEFI stub for module loading.
>>  When adding DomU modules to device tree, also add the property
>>  xen,uefi-cfg-load under chosen for Xen to load the Xen config file.
>>  Otherwise, Xen will skip the config file and rely on device tree alone.
>> +When using the Xen configuration file in conjunction with the device tree, 
>> you
>> +can specify the Xen boot arguments in the configuration file with the 
>> "options="
>> +keyword or in the device tree with the "xen,xen-bootargs" property, but be
>> +aware that a device tree value has a precedence over the configuration file.
> 
> I am not sure I agree with the precedence chosen here. With UEFI environment 
> it is a lot easier to update the configuration file over the Device-Tree. So 
> this could be used to quickly test a command line before updating the 
> Device-Tree.
> 
> Also, somewhat unrelated to this patch. Looking at the code, the command line 
> is a concatenation of "options=" from the configuration file and the extra 
> arguments provided on the command line used to execute the UEFI binary.
> 
> When using the command line from the Device-Tree, I think it would still make 
> sense to append the later because it could allow a user to provide a single 
> Device-Tree with extra options on the UEFI command line.
> 
> But I think this is a separate subject. So if we are going to go with the 
> precedence you suggested, then we should at least clarify in the 
> documentation that it will replace both.

Hi Julien,

Yes I see your point, currently the boot arguments are done in this way <image 
name> <CFG bootargs> <CMD line bootargs> as you pointed out,

would it be ok in your opinion to check if <CFG bootargs> is specified and if 
it’s not, use the <DT bootargs> instead (if any)?

Cheers,
Luca


> 
> Cheers,
> 
> -- 
> Julien Grall