thr3ads.net - Xen devel - [PATCH v3] ARM: parse separate DT properties for different commandlines [Aug 2013]

If this information is useful, please help other people find it:
Share via:

Andre Przywara

2013-Aug-20 15:21 UTC

[PATCH v3] ARM: parse separate DT properties for different commandlines

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.

v1 .. v2:
 - fix whitespace issues
v2 .. v3:
 - add documentation

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 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 01492bb..c82ece1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -140,6 +140,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 &&
@@ -166,15 +167,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 ( 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;
                 continue;
-            else if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
+            }
+            if ( strcmp(prop_name, "bootargs") == 0 )
             {
-                if ( !bootargs )
+                if ( !bootargs  && !had_dom0_bootargs )
                     bootargs = prop_data;
                 continue;
             }
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 84d704d..e22bd22 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -325,7 +325,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;
 
-- 
1.7.12.1

Julien Grall

2013-Aug-21 22:53 UTC

head link

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

On 20 August 2013 16:21, Andre Przywara <andre.przywara@linaro.org>
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 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.
Since we plan to support multiboot, is it relevant to send a u-boot patch
for a temporary workaround?

We could use a u-boot script to create the correct properties/nodes in
the device tree. What do you think?
> v1 .. v2:
>  - fix whitespace issues
> v2 .. v3:
>  - add documentation
>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

BTW, I have modified this code with my latest patch series. I will
rebase it on top
of this patch.

Cheers,
> ---
>  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 01492bb..c82ece1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -140,6 +140,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 &&
> @@ -166,15 +167,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 ( 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;
>                  continue;
> -            else if ( strcmp(prop_name, "xen,dom0-bootargs") ==
0 )
> +            }
> +            if ( strcmp(prop_name, "bootargs") == 0 )
>              {
> -                if ( !bootargs )
> +                if ( !bootargs  && !had_dom0_bootargs )
>                      bootargs = prop_data;
>                  continue;
>              }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 84d704d..e22bd22 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -325,7 +325,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;
-- 
Julien Grall

Ian Campbell

2013-Aug-27 13:34 UTC

head link

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

On Wed, 2013-08-21 at 23:53 +0100, Julien Grall wrote:> On 20 August 2013 16:21, Andre Przywara <andre.przywara@linaro.org>
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 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.
> 
> Since we plan to support multiboot, is it relevant to send a u-boot patch
> for a temporary workaround?
> 
> We could use a u-boot script to create the correct properties/nodes in
> the device tree. What do you think?
I think a combination of propagating xen_bootargs and using a script to
populate the /chosen/modules@N nodes sounds like quite a convenient way
to do things.

It''s not clear that multiboot adds much more than some syntactic sugar
to this.
> 
> > v1 .. v2:
> >  - fix whitespace issues
> > v2 .. v3:
> >  - add documentation
> >
> > Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> 
> BTW, I have modified this code with my latest patch series. I will
> rebase it on top of this patch.
Does this mean I should wait for a series from you incorporating this
patch or should I consider just applying this because you''ve rebased
your series on it?

Ian.

Julien Grall

2013-Aug-27 15:03 UTC

head link

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

On 08/27/2013 02:34 PM, Ian Campbell wrote:> On Wed, 2013-08-21 at 23:53 +0100, Julien Grall wrote:
>> On 20 August 2013 16:21, Andre Przywara
<andre.przywara@linaro.org> 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 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.
>>
>> Since we plan to support multiboot, is it relevant to send a u-boot
patch
>> for a temporary workaround?
>>
>> We could use a u-boot script to create the correct properties/nodes in
>> the device tree. What do you think?
> 
> I think a combination of propagating xen_bootargs and using a script to
> populate the /chosen/modules@N nodes sounds like quite a convenient way
> to do things.
> 
> It''s not clear that multiboot adds much more than some syntactic
sugar
> to this.
> 
>>
>>> v1 .. v2:
>>>  - fix whitespace issues
>>> v2 .. v3:
>>>  - add documentation
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>
>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>
>> BTW, I have modified this code with my latest patch series. I will
>> rebase it on top of this patch.
> 
> Does this mean I should wait for a series from you incorporating this
> patch or should I consider just applying this because you''ve
rebased
> your series on it?
For the moment I have rebased this patch on top of my patch series (so
my patch series applied, then Andre''s patch).

If Andre is fine to delay this patch, I can carry it on my patch series
and modify the necessary things to switch to dt_* functions.

Andre, what do you think?

-- 
Julien Grall

Andre Przywara

2013-Aug-27 15:05 UTC

head link

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

On 08/27/2013 05:03 PM, Julien Grall wrote:> On 08/27/2013 02:34 PM, Ian Campbell wrote:
>> On Wed, 2013-08-21 at 23:53 +0100, Julien Grall wrote:
>>> On 20 August 2013 16:21, Andre Przywara
<andre.przywara@linaro.org> 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 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.
>>>
>>> Since we plan to support multiboot, is it relevant to send a u-boot
patch
>>> for a temporary workaround?
>>>
>>> We could use a u-boot script to create the correct properties/nodes
in
>>> the device tree. What do you think?
>>
>> I think a combination of propagating xen_bootargs and using a script to
>> populate the /chosen/modules@N nodes sounds like quite a convenient way
>> to do things.
>>
>> It''s not clear that multiboot adds much more than some
syntactic sugar
>> to this.
>>
>>>
>>>> v1 .. v2:
>>>>   - fix whitespace issues
>>>> v2 .. v3:
>>>>   - add documentation
>>>>
>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>
>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>>
>>> BTW, I have modified this code with my latest patch series. I will
>>> rebase it on top of this patch.
>>
>> Does this mean I should wait for a series from you incorporating this
>> patch or should I consider just applying this because you''ve
rebased
>> your series on it?
>
> For the moment I have rebased this patch on top of my patch series (so
> my patch series applied, then Andre''s patch).
>
> If Andre is fine to delay this patch, I can carry it on my patch series
> and modify the necessary things to switch to dt_* functions.
>
> Andre, what do you think?
Yes, that''s fine for me.
We need to rethink this whole multiboot approach anyway.

Regards,
Andre.

Xen devel - Aug 2013 - [PATCH v3] ARM: parse separate DT properties for different commandlines

[PATCH v3] ARM: parse separate DT properties for different commandlines

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines

Re: [PATCH v3] ARM: parse separate DT properties for different commandlines