Re: [Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-29 Thread Chen Baozi
On Sat, May 30, 2015 at 10:08:21AM +0800, Chen Baozi wrote:
> Hi Julien,
> 
> On Fri, May 29, 2015 at 05:08:08PM +0100, Julien Grall wrote:
> > On 29/05/15 16:55, Ian Campbell wrote:
> > > On Fri, 2015-05-29 at 16:44 +0100, Julien Grall wrote:
> > > 
> > >>> +name = GCSPRINTF("cpu@%lx", mpidr_aff);
> > >>
> > >> It's not necessary to change the cpu@.
> > > 
> > > AIUI it is conventional in DT for this to match the first reg entry.
> > 
> > Well, it's conventional when the reg field is containing an address.
> > 
> > It's not the case of the cpu node as "reg" doesn't contain an address.
> > There is a mix of both in the different DTS.
> > 
> > Although, increment an ID make easier to read the dumped DTS for debugging.
> 
> I did the change because it would be more friendly for user to read the
> cpu node in /proc/device-tree/. Because the number is no longer continuous
> when we have a guest more than 16 cpus. In that case, the cpu nodes will be
> looked like as the following:
> 
> cpu@0, cpu@1, ..., cpu@15, cpu@256, cpu@257, ..., cpu@271, cpu@512, ...

Ah, I misundertood your comment. I think what you mean here is to keep the
name as "cpu@i" not about the form of the "cpu@%d".

However, I still prefer the new form :)

Cheers,

Baozi.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-29 Thread Chen Baozi
Hi Julien,

On Fri, May 29, 2015 at 05:08:08PM +0100, Julien Grall wrote:
> On 29/05/15 16:55, Ian Campbell wrote:
> > On Fri, 2015-05-29 at 16:44 +0100, Julien Grall wrote:
> > 
> >>> +name = GCSPRINTF("cpu@%lx", mpidr_aff);
> >>
> >> It's not necessary to change the cpu@.
> > 
> > AIUI it is conventional in DT for this to match the first reg entry.
> 
> Well, it's conventional when the reg field is containing an address.
> 
> It's not the case of the cpu node as "reg" doesn't contain an address.
> There is a mix of both in the different DTS.
> 
> Although, increment an ID make easier to read the dumped DTS for debugging.

I did the change because it would be more friendly for user to read the
cpu node in /proc/device-tree/. Because the number is no longer continuous
when we have a guest more than 16 cpus. In that case, the cpu nodes will be
looked like as the following:

cpu@0, cpu@1, ..., cpu@15, cpu@256, cpu@257, ..., cpu@271, cpu@512, ...

Considering changing to the new form doesn't introduce new risk (and follows
the convention to a certain degree), I prefer the new way.

Cheers,

Baozi.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-29 Thread Julien Grall
On 29/05/15 16:55, Ian Campbell wrote:
> On Fri, 2015-05-29 at 16:44 +0100, Julien Grall wrote:
> 
>>> +name = GCSPRINTF("cpu@%lx", mpidr_aff);
>>
>> It's not necessary to change the cpu@.
> 
> AIUI it is conventional in DT for this to match the first reg entry.

Well, it's conventional when the reg field is containing an address.

It's not the case of the cpu node as "reg" doesn't contain an address.
There is a mix of both in the different DTS.

Although, increment an ID make easier to read the dumped DTS for debugging.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-29 Thread Ian Campbell
On Fri, 2015-05-29 at 16:44 +0100, Julien Grall wrote:

> > +name = GCSPRINTF("cpu@%lx", mpidr_aff);
> 
> It's not necessary to change the cpu@.

AIUI it is conventional in DT for this to match the first reg entry.

Ian.

> 
> Regards,
> 



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-29 Thread Julien Grall
Hi Chen,

On 28/05/15 11:15, Chen Baozi wrote:
> From: Chen Baozi 
> 
> According to ARM CPUs bindings, the reg field should match the MPIDR's
> affinity bits. We will use AFF0 and AFF1 when constructing the reg value
> of the guest at the moment, for it is enough for the current max vcpu
> number.
> 
> Signed-off-by: Chen Baozi 
> ---
>  tools/libxl/libxl_arm.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index c5088c4..8aa4815 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
> nr_cpus,
>const struct arch_info *ainfo)
>  {
>  int res, i;
> +uint64_t mpidr_aff;
>  
>  res = fdt_begin_node(fdt, "cpus");
>  if (res) return res;
> @@ -283,7 +284,16 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
> nr_cpus,
>  if (res) return res;
>  
>  for (i = 0; i < nr_cpus; i++) {
> -const char *name = GCSPRINTF("cpu@%d", i);
> +const char *name;
> +
> +/*
> + * According to ARM CPUs bindings, the reg field should match
> + * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
> + * constructing the reg value of the guest at the moment, for it
> + * is enough for the current max vcpu number.
> + */
> +mpidr_aff = (uint64_t)((i & 0x0f) | (((i >> 4) & 0xff) << 8));

The (uint64_t) is not necessary.

> +name = GCSPRINTF("cpu@%lx", mpidr_aff);

It's not necessary to change the cpu@.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH V4 5/8] tools/libxl: Set 'reg' of cpu node equal to MPIDR affinity for domU

2015-05-28 Thread Chen Baozi
From: Chen Baozi 

According to ARM CPUs bindings, the reg field should match the MPIDR's
affinity bits. We will use AFF0 and AFF1 when constructing the reg value
of the guest at the moment, for it is enough for the current max vcpu
number.

Signed-off-by: Chen Baozi 
---
 tools/libxl/libxl_arm.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index c5088c4..8aa4815 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
   const struct arch_info *ainfo)
 {
 int res, i;
+uint64_t mpidr_aff;
 
 res = fdt_begin_node(fdt, "cpus");
 if (res) return res;
@@ -283,7 +284,16 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
 if (res) return res;
 
 for (i = 0; i < nr_cpus; i++) {
-const char *name = GCSPRINTF("cpu@%d", i);
+const char *name;
+
+/*
+ * According to ARM CPUs bindings, the reg field should match
+ * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+ * constructing the reg value of the guest at the moment, for it
+ * is enough for the current max vcpu number.
+ */
+mpidr_aff = (uint64_t)((i & 0x0f) | (((i >> 4) & 0xff) << 8));
+name = GCSPRINTF("cpu@%lx", mpidr_aff);
 
 res = fdt_begin_node(fdt, name);
 if (res) return res;
@@ -297,7 +307,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
nr_cpus,
 res = fdt_property_string(fdt, "enable-method", "psci");
 if (res) return res;
 
-res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
+res = fdt_property_regs(gc, fdt, 1, 0, 1, mpidr_aff);
 if (res) return res;
 
 res = fdt_end_node(fdt);
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel