From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH V1 29/29] ARM: parse separate DT properties for different commandlines Date: Mon, 09 Sep 2013 16:06:57 +0200 Message-ID: <522DD601.5060201@linaro.org> References: <1377701263-3319-1-git-send-email-julien.grall@linaro.org> <1377701263-3319-30-git-send-email-julien.grall@linaro.org> <1378727958.19967.88.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1378727958.19967.88.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Julien Grall , patches@linaro.org, stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 09/09/2013 01:59 PM, Ian Campbell wrote: > On Wed, 2013-08-28 at 15:47 +0100, Julien Grall wrote: >> From: Andre Przywara >> >> Currently we use the chosen/bootargs property as the Xen commandline >> and rely on xen,dom0-bootargs for Dom0. However this brings issues >> with bootloaders, which usually build bootargs by bootscripts for a >> Linux kernel - and not for the entirely different Xen hypervisor. >> >> Introduce a new possible device tree property "xen,xen-bootargs" >> explicitly for the Xen hypervisor and make the selection of which to >> use more fine grained: >> - If xen,xen-bootargs is present, it will be used for Xen. >> - If xen,dom0-bootargs is present, it will be used for Dom0. >> - If xen,xen-bootargs is _not_ present, but xen,dom0-bootargs is, >> bootargs will be used for Xen. Like the current situation. >> - If no Xen specific properties are present, bootargs is for Dom0. >> - If xen,xen-bootargs is present, but xen,dom0-bootargs is missing, >> bootargs will be used for Dom0. >> >> The aim is to allow common bootscripts to boot both Xen and native >> Linux with the same device tree blob. If needed, one could hard-code >> the Xen commandline into the DTB, leaving bootargs for Dom0 to be set >> by the (non Xen-aware) bootloader. >> >> I will send out a appropriate u-boot patch, which writes the content >> of the "xen_bootargs" environment variable into the xen,xen-bootargs >> dtb property. >> >> Signed-off-by: Andre Przywara >> Signed-off-by: Julien Grall > > Acked-by: Ian Campbell > > Do we want/need to wait for the result of the bindings proposal Andre > made? > > This is unrelated to Andre's "use more flexible node naming", isn't it? Yes and no. Unrelated: the code does not conflict and it adds flexibility in booting, allowing less capable boot loaders to start Xen. Related: The flexible node naming solves this particular issue in a more generic manner. But as I think one goal was to make Xen boot on every toaster, we may keep this patch (or a rebased version of it) for flexibility. Regards, Andre. >> --- >> Changes in v2: >> - Add and rebase this patch to this patch series >> >> Changes before the patch was added to this patch series: >> v1 .. v2: >> - fix whitespace issues >> v2 .. v3: >> - add documentation >> --- >> docs/misc/arm/device-tree/booting.txt | 28 +++++++++++++++++++++++++++- >> xen/arch/arm/domain_build.c | 15 +++++++++++---- >> xen/common/device_tree.c | 7 ++++++- >> 3 files changed, 44 insertions(+), 6 deletions(-) >> >> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt >> index 94cd3f1..08ed775 100644 >> --- a/docs/misc/arm/device-tree/booting.txt >> +++ b/docs/misc/arm/device-tree/booting.txt >> @@ -1,3 +1,6 @@ >> +Dom0 kernel and ramdisk modules >> +================================ >> + >> Xen is passed the dom0 kernel and initrd via a reference in the /chosen >> node of the device tree. >> >> @@ -22,4 +25,27 @@ properties: >> >> - bootargs (optional) >> >> - Command line associated with this module >> + Command line associated with this module. This is deprecated and should >> + be replaced by the bootargs variations described below. >> + >> + >> +Command lines >> +============= >> + >> +Xen also checks for properties directly under /chosen to find suitable command >> +lines for Xen and Dom0. The logic is the following: >> + >> + - If xen,xen-bootargs is present, it will be used for Xen. >> + - If xen,dom0-bootargs is present, it will be used for Dom0. >> + - If xen,xen-bootargs is _not_ present, but xen,dom0-bootargs is, >> + bootargs will be used for Xen. >> + - If no Xen specific properties are present, bootargs is for Dom0. >> + - If xen,xen-bootargs is present, but xen,dom0-bootargs is missing, >> + bootargs will be used for Dom0. >> + >> +Most of these cases is to make booting with Xen-unaware bootloaders easier. >> +For those you would hardcode the Xen commandline in the DTB under >> +/chosen/xen,xen-bootargs and would let the bootloader set the Dom0 command >> +line by writing bootargs (as for native Linux). >> +A Xen-aware bootloader would set xen,xen-bootargs for Xen, xen,dom0-bootargs >> +for Dom0 and bootargs for native Linux. >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 1ac261e..e45e0e7 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -146,6 +146,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> const char *bootargs = NULL; >> const struct dt_property *pp; >> int res = 0; >> + int had_dom0_bootargs = 0; >> >> if ( early_info.modules.nr_mods >= MOD_KERNEL && >> early_info.modules.module[MOD_KERNEL].cmdline[0] ) >> @@ -162,15 +163,21 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> * >> * * remember xen,dom0-bootargs if we don't already have >> * bootargs (from module #1, above). >> - * * remove bootargs and xen,dom0-bootargs. >> + * * remove bootargs, xen,dom0-bootargs and xen,xen-bootargs. >> */ >> if ( dt_node_path_is_equal(np, "/chosen") ) >> { >> - if ( dt_property_name_is_equal(pp, "bootargs") ) >> + if ( dt_property_name_is_equal(pp, "xen,xen-bootargs") ) >> + continue; >> + if ( dt_property_name_is_equal(pp, "xen,dom0-bootargs") ) >> + { >> + had_dom0_bootargs = 1; >> + bootargs = pp->value; >> continue; >> - else if ( dt_property_name_is_equal(pp, "xen,dom0-bootargs") ) >> + } >> + if ( dt_property_name_is_equal(pp, "bootargs") ) >> { >> - if ( !bootargs ) >> + if ( !bootargs && !had_dom0_bootargs ) >> bootargs = pp->value; >> continue; >> } >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index 4bc1ce4..5620b23 100644 >> --- a/xen/common/device_tree.c >> +++ b/xen/common/device_tree.c >> @@ -261,7 +261,12 @@ const char *device_tree_bootargs(const void *fdt) >> if ( node < 0 ) >> return NULL; >> >> - prop = fdt_get_property(fdt, node, "bootargs", NULL); >> + prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL); >> + if ( prop == NULL ) >> + { >> + if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL)) >> + prop = fdt_get_property(fdt, node, "bootargs", NULL); >> + } >> if ( prop == NULL ) >> return NULL; >> > >