Re: [PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-03-03 Thread Tyrel Datwyler
On 03/03/2015 05:20 PM, Cyril Bur wrote:
 On Tue, 2015-03-03 at 15:15 -0800, Tyrel Datwyler wrote:
 On 03/02/2015 01:49 PM, Tyrel Datwyler wrote:
 On 03/01/2015 09:20 PM, Cyril Bur wrote:
 On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
 We currently use the device tree update code in the kernel after resuming
 from a suspend operation to re-sync the kernels view of the device tree 
 with
 that of the hypervisor. The code as it stands is not endian safe as it 
 relies
 on parsing buffers returned by RTAS calls that thusly contains data in big
 endian format.

 This patch annotates variables and structure members with __be types as 
 well
 as performing necessary byte swaps to cpu endian for data that needs to be
 parsed.

 Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/mobility.c | 36 
 ---
  1 file changed, 19 insertions(+), 17 deletions(-)

 diff --git a/arch/powerpc/platforms/pseries/mobility.c 
 b/arch/powerpc/platforms/pseries/mobility.c
 index 29e4f04..0b1f70e 100644
 --- a/arch/powerpc/platforms/pseries/mobility.c
 +++ b/arch/powerpc/platforms/pseries/mobility.c
 @@ -25,10 +25,10 @@
  static struct kobject *mobility_kobj;
  
  struct update_props_workarea {
 - u32 phandle;
 - u32 state;
 - u64 reserved;
 - u32 nprops;
 + __be32 phandle;
 + __be32 state;
 + __be64 reserved;
 + __be32 nprops;
  } __packed;
  
  #define NODE_ACTION_MASK 0xff00
 @@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
 struct property **prop,
   return 0;
  }
  
 -static int update_dt_node(u32 phandle, s32 scope)
 +static int update_dt_node(__be32 phandle, s32 scope)
  {

 On line 153 of this function:
dn = of_find_node_by_phandle(phandle);

 You're passing a __be32 to device tree code, if we can treat the phandle
 as a opaque value returned to us from the rtas call and pass it around
 like that then all good.

 After digging deeper the device_node-phandle is stored in cpu endian
 under the covers. So, for the of_find_node_by_phandle() we do need to
 convert the phandle to cpu endian first. It appears I got lucky with the
 update fixing the observed RMC issue because the phandle for the root
 node seems to always be 0x.

 I think we've both switched opinions here, initially I thought an endian
 conversion was necessary but turns out that all of_find_node_by_phandle
 really does is:
for_each_of_allnodes(np)
   if (np-phandle == handle)
  break;
of_node_get(np);
 
 The == is safe either way and I think the of code might be trying to
 imply that it doesn't matter by having a typedefed type 'phandle'.
 
 I'm still digging around, we want to get this right!

When the device tree is unflattened the phandle is byte swapped to cpu
endian. The following code is from unflatten_dt_node().

if (strcmp(pname, ibm,phandle) == 0)
np-phandle = be32_to_cpup(p);

I added some debug to the of_find_node_by_phandle() and verified if the
phandle isn't swapped to cpu endian we fail to find a matching node
except in the case where the phandle is equivalent in both big and
little endian.

-Tyrel

 
 
 Cyril
 -Tyrel


 Yes, of_find_node_by_phandle directly compares phandle passed in against
 the handle stored in each device_node when searching for a matching
 node. Since, the device tree is big endian it follows that the big
 endian phandle received in the rtas buffer needs no conversion.

 Further, we need to pass the phandle to ibm,update-properties in the
 work area which is also required to be big endian. So, again it seemed
 that converting to cpu endian was a waste of effort just to convert it
 back to big endian.

 Its also hard to be sure if these need to be BE and have always been
 that way because we've always run BE so they've never actually wanted
 CPU endian its just that CPU endian has always been BE (I think I
 started rambling...)

 Just want to check that *not* converting them is done on purpose.

 Yes, I explicitly did not convert them on purpose. As mentioned above we
 need phandle in BE for the ibm,update-properties rtas work area.
 Similarly, drc_index needs to be in BE for the ibm,configure-connector
 rtas work area. Outside, of that we do no other manipulation of those
 values.


 And having read on, I'm assuming the answer is yes since this
 observation is true for your changes which affect:
delete_dt_node()
update_dt_node()
 add_dt_node()
 Worth noting that you didn't change the definition of delete_dt_node()

 You are correct. Oversight. I will fix that as it should generate a
 sparse complaint.

 -Tyrel


 I'll have a look once you address the non compiling in patch 1/3 (I'm
 getting blocked the unused var because somehow Werror is on, odd it
 didn't trip you up) but I also suspect this will have sparse go a bit
 nuts. 
 I wonder if there is a nice way of shutting sparse up.

   struct update_props_workarea *upwa;
   struct device_node *dn;
 @@ -136,6 

Re: [PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-03-03 Thread Tyrel Datwyler
On 03/02/2015 01:49 PM, Tyrel Datwyler wrote:
 On 03/01/2015 09:20 PM, Cyril Bur wrote:
 On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
 We currently use the device tree update code in the kernel after resuming
 from a suspend operation to re-sync the kernels view of the device tree with
 that of the hypervisor. The code as it stands is not endian safe as it 
 relies
 on parsing buffers returned by RTAS calls that thusly contains data in big
 endian format.

 This patch annotates variables and structure members with __be types as well
 as performing necessary byte swaps to cpu endian for data that needs to be
 parsed.

 Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/mobility.c | 36 
 ---
  1 file changed, 19 insertions(+), 17 deletions(-)

 diff --git a/arch/powerpc/platforms/pseries/mobility.c 
 b/arch/powerpc/platforms/pseries/mobility.c
 index 29e4f04..0b1f70e 100644
 --- a/arch/powerpc/platforms/pseries/mobility.c
 +++ b/arch/powerpc/platforms/pseries/mobility.c
 @@ -25,10 +25,10 @@
  static struct kobject *mobility_kobj;
  
  struct update_props_workarea {
 -   u32 phandle;
 -   u32 state;
 -   u64 reserved;
 -   u32 nprops;
 +   __be32 phandle;
 +   __be32 state;
 +   __be64 reserved;
 +   __be32 nprops;
  } __packed;
  
  #define NODE_ACTION_MASK   0xff00
 @@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
 struct property **prop,
 return 0;
  }
  
 -static int update_dt_node(u32 phandle, s32 scope)
 +static int update_dt_node(__be32 phandle, s32 scope)
  {

 On line 153 of this function:
dn = of_find_node_by_phandle(phandle);

 You're passing a __be32 to device tree code, if we can treat the phandle
 as a opaque value returned to us from the rtas call and pass it around
 like that then all good.

After digging deeper the device_node-phandle is stored in cpu endian
under the covers. So, for the of_find_node_by_phandle() we do need to
convert the phandle to cpu endian first. It appears I got lucky with the
update fixing the observed RMC issue because the phandle for the root
node seems to always be 0x.

-Tyrel

 
 Yes, of_find_node_by_phandle directly compares phandle passed in against
 the handle stored in each device_node when searching for a matching
 node. Since, the device tree is big endian it follows that the big
 endian phandle received in the rtas buffer needs no conversion.
 
 Further, we need to pass the phandle to ibm,update-properties in the
 work area which is also required to be big endian. So, again it seemed
 that converting to cpu endian was a waste of effort just to convert it
 back to big endian.
 
 Its also hard to be sure if these need to be BE and have always been
 that way because we've always run BE so they've never actually wanted
 CPU endian its just that CPU endian has always been BE (I think I
 started rambling...)

 Just want to check that *not* converting them is done on purpose.
 
 Yes, I explicitly did not convert them on purpose. As mentioned above we
 need phandle in BE for the ibm,update-properties rtas work area.
 Similarly, drc_index needs to be in BE for the ibm,configure-connector
 rtas work area. Outside, of that we do no other manipulation of those
 values.
 

 And having read on, I'm assuming the answer is yes since this
 observation is true for your changes which affect:
  delete_dt_node()
  update_dt_node()
 add_dt_node()
 Worth noting that you didn't change the definition of delete_dt_node()
 
 You are correct. Oversight. I will fix that as it should generate a
 sparse complaint.
 
 -Tyrel
 

 I'll have a look once you address the non compiling in patch 1/3 (I'm
 getting blocked the unused var because somehow Werror is on, odd it
 didn't trip you up) but I also suspect this will have sparse go a bit
 nuts. 
 I wonder if there is a nice way of shutting sparse up.

 struct update_props_workarea *upwa;
 struct device_node *dn;
 @@ -136,6 +136,7 @@ static int update_dt_node(u32 phandle, s32 scope)
 char *prop_data;
 char *rtas_buf;
 int update_properties_token;
 +   u32 nprops;
 u32 vd;
  
 update_properties_token = rtas_token(ibm,update-properties);
 @@ -162,6 +163,7 @@ static int update_dt_node(u32 phandle, s32 scope)
 break;
  
 prop_data = rtas_buf + sizeof(*upwa);
 +   nprops = be32_to_cpu(upwa-nprops);
  
 /* On the first call to ibm,update-properties for a node the
  * the first property value descriptor contains an empty
 @@ -170,17 +172,17 @@ static int update_dt_node(u32 phandle, s32 scope)
  */
 if (*prop_data == 0) {
 prop_data++;
 -   vd = *(u32 *)prop_data;
 +   vd = be32_to_cpu(*(__be32 *)prop_data);
 prop_data += vd + sizeof(vd);
 -   upwa-nprops--;
 +   nprops--;
 }
  
 - 

Re: [PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-03-03 Thread Cyril Bur
On Tue, 2015-03-03 at 15:15 -0800, Tyrel Datwyler wrote:
 On 03/02/2015 01:49 PM, Tyrel Datwyler wrote:
  On 03/01/2015 09:20 PM, Cyril Bur wrote:
  On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
  We currently use the device tree update code in the kernel after resuming
  from a suspend operation to re-sync the kernels view of the device tree 
  with
  that of the hypervisor. The code as it stands is not endian safe as it 
  relies
  on parsing buffers returned by RTAS calls that thusly contains data in big
  endian format.
 
  This patch annotates variables and structure members with __be types as 
  well
  as performing necessary byte swaps to cpu endian for data that needs to be
  parsed.
 
  Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
  ---
   arch/powerpc/platforms/pseries/mobility.c | 36 
  ---
   1 file changed, 19 insertions(+), 17 deletions(-)
 
  diff --git a/arch/powerpc/platforms/pseries/mobility.c 
  b/arch/powerpc/platforms/pseries/mobility.c
  index 29e4f04..0b1f70e 100644
  --- a/arch/powerpc/platforms/pseries/mobility.c
  +++ b/arch/powerpc/platforms/pseries/mobility.c
  @@ -25,10 +25,10 @@
   static struct kobject *mobility_kobj;
   
   struct update_props_workarea {
  - u32 phandle;
  - u32 state;
  - u64 reserved;
  - u32 nprops;
  + __be32 phandle;
  + __be32 state;
  + __be64 reserved;
  + __be32 nprops;
   } __packed;
   
   #define NODE_ACTION_MASK 0xff00
  @@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
  struct property **prop,
return 0;
   }
   
  -static int update_dt_node(u32 phandle, s32 scope)
  +static int update_dt_node(__be32 phandle, s32 scope)
   {
 
  On line 153 of this function:
 dn = of_find_node_by_phandle(phandle);
 
  You're passing a __be32 to device tree code, if we can treat the phandle
  as a opaque value returned to us from the rtas call and pass it around
  like that then all good.
 
 After digging deeper the device_node-phandle is stored in cpu endian
 under the covers. So, for the of_find_node_by_phandle() we do need to
 convert the phandle to cpu endian first. It appears I got lucky with the
 update fixing the observed RMC issue because the phandle for the root
 node seems to always be 0x.
 
I think we've both switched opinions here, initially I thought an endian
conversion was necessary but turns out that all of_find_node_by_phandle
really does is:
   for_each_of_allnodes(np)
  if (np-phandle == handle)
 break;
   of_node_get(np);

The == is safe either way and I think the of code might be trying to
imply that it doesn't matter by having a typedefed type 'phandle'.

I'm still digging around, we want to get this right!


Cyril
 -Tyrel
 
  
  Yes, of_find_node_by_phandle directly compares phandle passed in against
  the handle stored in each device_node when searching for a matching
  node. Since, the device tree is big endian it follows that the big
  endian phandle received in the rtas buffer needs no conversion.
  
  Further, we need to pass the phandle to ibm,update-properties in the
  work area which is also required to be big endian. So, again it seemed
  that converting to cpu endian was a waste of effort just to convert it
  back to big endian.
  
  Its also hard to be sure if these need to be BE and have always been
  that way because we've always run BE so they've never actually wanted
  CPU endian its just that CPU endian has always been BE (I think I
  started rambling...)
 
  Just want to check that *not* converting them is done on purpose.
  
  Yes, I explicitly did not convert them on purpose. As mentioned above we
  need phandle in BE for the ibm,update-properties rtas work area.
  Similarly, drc_index needs to be in BE for the ibm,configure-connector
  rtas work area. Outside, of that we do no other manipulation of those
  values.
  
 
  And having read on, I'm assuming the answer is yes since this
  observation is true for your changes which affect:
 delete_dt_node()
 update_dt_node()
  add_dt_node()
  Worth noting that you didn't change the definition of delete_dt_node()
  
  You are correct. Oversight. I will fix that as it should generate a
  sparse complaint.
  
  -Tyrel
  
 
  I'll have a look once you address the non compiling in patch 1/3 (I'm
  getting blocked the unused var because somehow Werror is on, odd it
  didn't trip you up) but I also suspect this will have sparse go a bit
  nuts. 
  I wonder if there is a nice way of shutting sparse up.
 
struct update_props_workarea *upwa;
struct device_node *dn;
  @@ -136,6 +136,7 @@ static int update_dt_node(u32 phandle, s32 scope)
char *prop_data;
char *rtas_buf;
int update_properties_token;
  + u32 nprops;
u32 vd;
   
update_properties_token = rtas_token(ibm,update-properties);
  @@ -162,6 +163,7 @@ static int update_dt_node(u32 phandle, s32 scope)
break;
   
prop_data = rtas_buf + sizeof(*upwa);
  

Re: [PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-03-02 Thread Tyrel Datwyler
On 03/01/2015 09:20 PM, Cyril Bur wrote:
 On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
 We currently use the device tree update code in the kernel after resuming
 from a suspend operation to re-sync the kernels view of the device tree with
 that of the hypervisor. The code as it stands is not endian safe as it relies
 on parsing buffers returned by RTAS calls that thusly contains data in big
 endian format.

 This patch annotates variables and structure members with __be types as well
 as performing necessary byte swaps to cpu endian for data that needs to be
 parsed.

 Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/mobility.c | 36 
 ---
  1 file changed, 19 insertions(+), 17 deletions(-)

 diff --git a/arch/powerpc/platforms/pseries/mobility.c 
 b/arch/powerpc/platforms/pseries/mobility.c
 index 29e4f04..0b1f70e 100644
 --- a/arch/powerpc/platforms/pseries/mobility.c
 +++ b/arch/powerpc/platforms/pseries/mobility.c
 @@ -25,10 +25,10 @@
  static struct kobject *mobility_kobj;
  
  struct update_props_workarea {
 -u32 phandle;
 -u32 state;
 -u64 reserved;
 -u32 nprops;
 +__be32 phandle;
 +__be32 state;
 +__be64 reserved;
 +__be32 nprops;
  } __packed;
  
  #define NODE_ACTION_MASK0xff00
 @@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
 struct property **prop,
  return 0;
  }
  
 -static int update_dt_node(u32 phandle, s32 scope)
 +static int update_dt_node(__be32 phandle, s32 scope)
  {
 
 On line 153 of this function:
dn = of_find_node_by_phandle(phandle);
 
 You're passing a __be32 to device tree code, if we can treat the phandle
 as a opaque value returned to us from the rtas call and pass it around
 like that then all good.

Yes, of_find_node_by_phandle directly compares phandle passed in against
the handle stored in each device_node when searching for a matching
node. Since, the device tree is big endian it follows that the big
endian phandle received in the rtas buffer needs no conversion.

Further, we need to pass the phandle to ibm,update-properties in the
work area which is also required to be big endian. So, again it seemed
that converting to cpu endian was a waste of effort just to convert it
back to big endian.

 Its also hard to be sure if these need to be BE and have always been
 that way because we've always run BE so they've never actually wanted
 CPU endian its just that CPU endian has always been BE (I think I
 started rambling...)
 
 Just want to check that *not* converting them is done on purpose.

Yes, I explicitly did not convert them on purpose. As mentioned above we
need phandle in BE for the ibm,update-properties rtas work area.
Similarly, drc_index needs to be in BE for the ibm,configure-connector
rtas work area. Outside, of that we do no other manipulation of those
values.

 
 And having read on, I'm assuming the answer is yes since this
 observation is true for your changes which affect:
   delete_dt_node()
   update_dt_node()
 add_dt_node()
 Worth noting that you didn't change the definition of delete_dt_node()

You are correct. Oversight. I will fix that as it should generate a
sparse complaint.

-Tyrel

 
 I'll have a look once you address the non compiling in patch 1/3 (I'm
 getting blocked the unused var because somehow Werror is on, odd it
 didn't trip you up) but I also suspect this will have sparse go a bit
 nuts. 
 I wonder if there is a nice way of shutting sparse up.
 
  struct update_props_workarea *upwa;
  struct device_node *dn;
 @@ -136,6 +136,7 @@ static int update_dt_node(u32 phandle, s32 scope)
  char *prop_data;
  char *rtas_buf;
  int update_properties_token;
 +u32 nprops;
  u32 vd;
  
  update_properties_token = rtas_token(ibm,update-properties);
 @@ -162,6 +163,7 @@ static int update_dt_node(u32 phandle, s32 scope)
  break;
  
  prop_data = rtas_buf + sizeof(*upwa);
 +nprops = be32_to_cpu(upwa-nprops);
  
  /* On the first call to ibm,update-properties for a node the
   * the first property value descriptor contains an empty
 @@ -170,17 +172,17 @@ static int update_dt_node(u32 phandle, s32 scope)
   */
  if (*prop_data == 0) {
  prop_data++;
 -vd = *(u32 *)prop_data;
 +vd = be32_to_cpu(*(__be32 *)prop_data);
  prop_data += vd + sizeof(vd);
 -upwa-nprops--;
 +nprops--;
  }
  
 -for (i = 0; i  upwa-nprops; i++) {
 +for (i = 0; i  nprops; i++) {
  char *prop_name;
  
  prop_name = prop_data;
  prop_data += strlen(prop_name) + 1;
 -vd = *(u32 *)prop_data;
 +vd = be32_to_cpu(*(__be32 *)prop_data);
  

Re: [PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-03-01 Thread Cyril Bur
On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
 We currently use the device tree update code in the kernel after resuming
 from a suspend operation to re-sync the kernels view of the device tree with
 that of the hypervisor. The code as it stands is not endian safe as it relies
 on parsing buffers returned by RTAS calls that thusly contains data in big
 endian format.
 
 This patch annotates variables and structure members with __be types as well
 as performing necessary byte swaps to cpu endian for data that needs to be
 parsed.
 
 Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/mobility.c | 36 
 ---
  1 file changed, 19 insertions(+), 17 deletions(-)
 
 diff --git a/arch/powerpc/platforms/pseries/mobility.c 
 b/arch/powerpc/platforms/pseries/mobility.c
 index 29e4f04..0b1f70e 100644
 --- a/arch/powerpc/platforms/pseries/mobility.c
 +++ b/arch/powerpc/platforms/pseries/mobility.c
 @@ -25,10 +25,10 @@
  static struct kobject *mobility_kobj;
  
  struct update_props_workarea {
 - u32 phandle;
 - u32 state;
 - u64 reserved;
 - u32 nprops;
 + __be32 phandle;
 + __be32 state;
 + __be64 reserved;
 + __be32 nprops;
  } __packed;
  
  #define NODE_ACTION_MASK 0xff00
 @@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
 struct property **prop,
   return 0;
  }
  
 -static int update_dt_node(u32 phandle, s32 scope)
 +static int update_dt_node(__be32 phandle, s32 scope)
  {

On line 153 of this function:
   dn = of_find_node_by_phandle(phandle);

You're passing a __be32 to device tree code, if we can treat the phandle
as a opaque value returned to us from the rtas call and pass it around
like that then all good.
Its also hard to be sure if these need to be BE and have always been
that way because we've always run BE so they've never actually wanted
CPU endian its just that CPU endian has always been BE (I think I
started rambling...)

Just want to check that *not* converting them is done on purpose.

And having read on, I'm assuming the answer is yes since this
observation is true for your changes which affect:
delete_dt_node()
update_dt_node()
add_dt_node()
Worth noting that you didn't change the definition of delete_dt_node()

I'll have a look once you address the non compiling in patch 1/3 (I'm
getting blocked the unused var because somehow Werror is on, odd it
didn't trip you up) but I also suspect this will have sparse go a bit
nuts. 
I wonder if there is a nice way of shutting sparse up.

   struct update_props_workarea *upwa;
   struct device_node *dn;
 @@ -136,6 +136,7 @@ static int update_dt_node(u32 phandle, s32 scope)
   char *prop_data;
   char *rtas_buf;
   int update_properties_token;
 + u32 nprops;
   u32 vd;
  
   update_properties_token = rtas_token(ibm,update-properties);
 @@ -162,6 +163,7 @@ static int update_dt_node(u32 phandle, s32 scope)
   break;
  
   prop_data = rtas_buf + sizeof(*upwa);
 + nprops = be32_to_cpu(upwa-nprops);
  
   /* On the first call to ibm,update-properties for a node the
* the first property value descriptor contains an empty
 @@ -170,17 +172,17 @@ static int update_dt_node(u32 phandle, s32 scope)
*/
   if (*prop_data == 0) {
   prop_data++;
 - vd = *(u32 *)prop_data;
 + vd = be32_to_cpu(*(__be32 *)prop_data);
   prop_data += vd + sizeof(vd);
 - upwa-nprops--;
 + nprops--;
   }
  
 - for (i = 0; i  upwa-nprops; i++) {
 + for (i = 0; i  nprops; i++) {
   char *prop_name;
  
   prop_name = prop_data;
   prop_data += strlen(prop_name) + 1;
 - vd = *(u32 *)prop_data;
 + vd = be32_to_cpu(*(__be32 *)prop_data);
   prop_data += sizeof(vd);
  
   switch (vd) {
 @@ -212,7 +214,7 @@ static int update_dt_node(u32 phandle, s32 scope)
   return 0;
  }
  
 -static int add_dt_node(u32 parent_phandle, u32 drc_index)
 +static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
  {
   struct device_node *dn;
   struct device_node *parent_dn;
 @@ -237,7 +239,7 @@ static int add_dt_node(u32 parent_phandle, u32 drc_index)
  int pseries_devicetree_update(s32 scope)
  {
   char *rtas_buf;
 - u32 *data;
 + __be32 *data;
   int update_nodes_token;
   int rc;
  
 @@ -254,17 +256,17 @@ int pseries_devicetree_update(s32 scope)
   if (rc  rc != 1)
   break;
  
 - data = (u32 *)rtas_buf + 4;
 - while (*data  NODE_ACTION_MASK) {
 + data = (__be32 *)rtas_buf + 4;
 + while (be32_to_cpu(*data)  

[PATCH 2/3] powerpc/pseries: Little endian fixes for post mobility device tree update

2015-02-27 Thread Tyrel Datwyler
We currently use the device tree update code in the kernel after resuming
from a suspend operation to re-sync the kernels view of the device tree with
that of the hypervisor. The code as it stands is not endian safe as it relies
on parsing buffers returned by RTAS calls that thusly contains data in big
endian format.

This patch annotates variables and structure members with __be types as well
as performing necessary byte swaps to cpu endian for data that needs to be
parsed.

Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/mobility.c | 36 ---
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c 
b/arch/powerpc/platforms/pseries/mobility.c
index 29e4f04..0b1f70e 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -25,10 +25,10 @@
 static struct kobject *mobility_kobj;
 
 struct update_props_workarea {
-   u32 phandle;
-   u32 state;
-   u64 reserved;
-   u32 nprops;
+   __be32 phandle;
+   __be32 state;
+   __be64 reserved;
+   __be32 nprops;
 } __packed;
 
 #define NODE_ACTION_MASK   0xff00
@@ -127,7 +127,7 @@ static int update_dt_property(struct device_node *dn, 
struct property **prop,
return 0;
 }
 
-static int update_dt_node(u32 phandle, s32 scope)
+static int update_dt_node(__be32 phandle, s32 scope)
 {
struct update_props_workarea *upwa;
struct device_node *dn;
@@ -136,6 +136,7 @@ static int update_dt_node(u32 phandle, s32 scope)
char *prop_data;
char *rtas_buf;
int update_properties_token;
+   u32 nprops;
u32 vd;
 
update_properties_token = rtas_token(ibm,update-properties);
@@ -162,6 +163,7 @@ static int update_dt_node(u32 phandle, s32 scope)
break;
 
prop_data = rtas_buf + sizeof(*upwa);
+   nprops = be32_to_cpu(upwa-nprops);
 
/* On the first call to ibm,update-properties for a node the
 * the first property value descriptor contains an empty
@@ -170,17 +172,17 @@ static int update_dt_node(u32 phandle, s32 scope)
 */
if (*prop_data == 0) {
prop_data++;
-   vd = *(u32 *)prop_data;
+   vd = be32_to_cpu(*(__be32 *)prop_data);
prop_data += vd + sizeof(vd);
-   upwa-nprops--;
+   nprops--;
}
 
-   for (i = 0; i  upwa-nprops; i++) {
+   for (i = 0; i  nprops; i++) {
char *prop_name;
 
prop_name = prop_data;
prop_data += strlen(prop_name) + 1;
-   vd = *(u32 *)prop_data;
+   vd = be32_to_cpu(*(__be32 *)prop_data);
prop_data += sizeof(vd);
 
switch (vd) {
@@ -212,7 +214,7 @@ static int update_dt_node(u32 phandle, s32 scope)
return 0;
 }
 
-static int add_dt_node(u32 parent_phandle, u32 drc_index)
+static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
 {
struct device_node *dn;
struct device_node *parent_dn;
@@ -237,7 +239,7 @@ static int add_dt_node(u32 parent_phandle, u32 drc_index)
 int pseries_devicetree_update(s32 scope)
 {
char *rtas_buf;
-   u32 *data;
+   __be32 *data;
int update_nodes_token;
int rc;
 
@@ -254,17 +256,17 @@ int pseries_devicetree_update(s32 scope)
if (rc  rc != 1)
break;
 
-   data = (u32 *)rtas_buf + 4;
-   while (*data  NODE_ACTION_MASK) {
+   data = (__be32 *)rtas_buf + 4;
+   while (be32_to_cpu(*data)  NODE_ACTION_MASK) {
int i;
-   u32 action = *data  NODE_ACTION_MASK;
-   int node_count = *data  NODE_COUNT_MASK;
+   u32 action = be32_to_cpu(*data)  NODE_ACTION_MASK;
+   u32 node_count = be32_to_cpu(*data)  NODE_COUNT_MASK;
 
data++;
 
for (i = 0; i  node_count; i++) {
-   u32 phandle = *data++;
-   u32 drc_index;
+   __be32 phandle = *data++;
+   __be32 drc_index;
 
switch (action) {
case DELETE_DT_NODE:
-- 
1.7.12.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev