From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH v2] ARM: parse separate DT properties for different commandlines Date: Tue, 20 Aug 2013 14:09:34 +0200 Message-ID: <52135C7E.3080105@linaro.org> References: <1370267034-21467-1-git-send-email-andre.przywara@calxeda.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Stefano Stabellini , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 07/10/2013 01:48 PM, Julien Grall wrote: > On 3 June 2013 14:43, Andre Przywara wrote: >> 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 also have a simple patch for u-boot to transfer the content of the >> "xen_bootargs" environment variable into the xen,xen-bootargs dtb >> property. >> I will post the u-boot patch to their ML later. >> >> Changes from v1: >> - fix whitespace issues > > Any news about this patch ? :) Sorry for the lag ;-) > >> Signed-off-by: Andre Przywara >> --- >> xen/arch/arm/domain_build.c | 13 ++++++++++--- >> xen/common/device_tree.c | 7 ++++++- >> 2 files changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index b92c64b..5809489 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -139,6 +139,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> u32 address_cells, u32 size_cells) >> { >> const char *bootargs = NULL; >> + int had_dom0_bootargs = 0; >> int prop; >> >> if ( early_info.modules.nr_mods >= 1 && >> @@ -169,11 +170,17 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> */ >> if ( device_tree_node_matches(fdt, node, "chosen") ) >> { >> - if ( strcmp(prop_name, "bootargs") == 0 ) >> + if ( strcmp(prop_name, "xen,xen-bootargs") == 0 ) >> + continue; >> + if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 ) >> + { >> + had_dom0_bootargs = 1; >> + bootargs = prop_data; > > Here, you overwrite the previous "bootargs". This variable is set if > the module node contains "bootargs" property > (see process_multiboot_node in common/device_tree.c) I'd say that is intended. I think those command lines directly under /chosen should have the highest priority. If someone has /chosen/xen,dom0-bootargs, that should be used instead of a most likely hard-coded value under modules. I have incorporated your previous comments and will send out a new version ASAP. Regards, Andre. > > Could you either: > 1) Remove the possibility to set the command line via the custom multiboot > 2) Check if "bootargs" was not set > > I think nobody uses the custom multiboot with the command line. So the > first solution seems to be the best. > > -- > Julien Grall > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >