Message ID | 20211206153658.49727-1-luca.fancellu@arm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | arm/efi: Handle Xen bootargs from both xen.cfg and DT | expand |
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@arm.com> > --- > 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. Cheers,
> On 8 Dec 2021, at 18:10, Julien Grall <julien@xen.org> 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@arm.com> >> --- >> 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
Hi Luca, On 10/12/2021 10:26, Luca Fancellu wrote: > > >> On 8 Dec 2021, at 18:10, Julien Grall <julien@xen.org> 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@arm.com> >>> --- >>> 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. > > 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)? I am happy with this approach. Cheers,
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. Example 1 of how to boot a true dom0less configuration: diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h index c4ed41284597..fc1f2b9ad60e 100644 --- a/xen/arch/arm/efi/efi-boot.h +++ b/xen/arch/arm/efi/efi-boot.h @@ -497,6 +497,13 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name, if ( chosen < 0 ) blexit(L"Unable to find chosen node"); + /* If xen,bootargs is found in /chosen, use it for Xen */ + if ( fdt_get_property(fdt, chosen, "xen,xen-bootargs", NULL) ) + { + PrintStr(L"Using Xen boot arguments from device tree.\r\n"); + return; + } + status = efi_bs->AllocatePool(EfiBootServicesData, EFI_PAGE_SIZE, (void **)&buf); if ( EFI_ERROR(status) ) PrintErrMesg(L"Unable to allocate string buffer", status);
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@arm.com> --- docs/misc/efi.pandoc | 4 ++++ xen/arch/arm/efi/efi-boot.h | 7 +++++++ 2 files changed, 11 insertions(+)