Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

2014-09-12 Thread Michael Ellerman 
On Wed, 2014-09-10 at 17:41 -0500, Thomas Falcon wrote:
 When removing a cpu, this patch makes sure that values
 gotten from or passed to firmware are in the correct
 endian format.
 
 Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
  arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
  2 files changed, 11 insertions(+), 11 deletions(-)
 
 diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
 b/arch/powerpc/platforms/pseries/dlpar.c
 index cd425dc..c5ecfdb 100644
 --- a/arch/powerpc/platforms/pseries/dlpar.c
 +++ b/arch/powerpc/platforms/pseries/dlpar.c
 @@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
   int rc = 0;
   unsigned int cpu;
   int len, nthreads, i;
 - const u32 *intserv;
 + const __be32 *intserv;
  
   intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
   if (!intserv)
 @@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
   cpu_maps_update_begin();
   for (i = 0; i  nthreads; i++) {

Can you please do the conversion once here for each value of i.

You can call the converted value thread ?

   for_each_present_cpu(cpu) {
 - if (get_hard_smp_processor_id(cpu) != intserv[i])
 + if (get_hard_smp_processor_id(cpu) != 
 be32_to_cpu(intserv[i]))
   continue;

Rather than doing it for every cpu in the system for every value of i.

Not that performance is really an issue, but it's just ugly.

And obviously the other places that use it in the loop should use the converted
value.


 @@ -494,7 +494,7 @@ out:
  static ssize_t dlpar_cpu_release(const char *buf, size_t count)
  {
   struct device_node *dn;
 - const u32 *drc_index;
 + const __be32 *drc_index;
   int rc;
  
   dn = of_find_node_by_path(buf);
 @@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
 count)
   return -EINVAL;
   }

Here again you should do the conversion once.

Better still use of_property_read_u32().

 - rc = dlpar_release_drc(*drc_index);
 + rc = dlpar_release_drc(be32_to_cpup(drc_index));
   if (rc) {
   of_node_put(dn);
   return rc;
 @@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
 count)
  
   rc = dlpar_detach_node(dn);
   if (rc) {
 - dlpar_acquire_drc(*drc_index);
 + dlpar_acquire_drc(be32_to_cpup(drc_index));
   return rc;
   }

cheers


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

Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

2014-09-12 Thread Thomas Falcon 

On 09/12/2014 03:53 AM, Michael Ellerman wrote:

On Wed, 2014-09-10 at 17:41 -0500, Thomas Falcon wrote:

When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
  arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
  arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
  2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index cd425dc..c5ecfdb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
  
  	intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);

if (!intserv)
@@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {

Can you please do the conversion once here for each value of i.

You can call the converted value thread ?


for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;

Rather than doing it for every cpu in the system for every value of i.

Not that performance is really an issue, but it's just ugly.

And obviously the other places that use it in the loop should use the converted
value.



@@ -494,7 +494,7 @@ out:
  static ssize_t dlpar_cpu_release(const char *buf, size_t count)
  {
struct device_node *dn;
-   const u32 *drc_index;
+   const __be32 *drc_index;
int rc;
  
  	dn = of_find_node_by_path(buf);

@@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}

Here again you should do the conversion once.

Better still use of_property_read_u32().


-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(be32_to_cpup(drc_index));
if (rc) {
of_node_put(dn);
return rc;
@@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
  
  	rc = dlpar_detach_node(dn);

if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(be32_to_cpup(drc_index));
return rc;
}

cheers


Thanks for the feedback.  Sending patches with your suggested changes.

tom





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

[PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

2014-09-10 Thread Thomas Falcon 
When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index cd425dc..c5ecfdb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -475,14 +475,14 @@ static int dlpar_offline_cpu(struct device_node *dn)
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
-   BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+   BUG_ON(plpar_hcall_norets(H_PROD, 
be32_to_cpu(intserv[i]))
!= H_SUCCESS);
__cpu_die(cpu);
break;
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to offline 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 
@@ -494,7 +494,7 @@ out:
 static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 {
struct device_node *dn;
-   const u32 *drc_index;
+   const __be32 *drc_index;
int rc;
 
dn = of_find_node_by_path(buf);
@@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(be32_to_cpup(drc_index));
if (rc) {
of_node_put(dn);
return rc;
@@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
 
rc = dlpar_detach_node(dn);
if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(be32_to_cpup(drc_index));
return rc;
}
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 447f8c6..031762d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -90,7 +90,7 @@ static void rtas_stop_self(void)
 {
static struct rtas_args args = {
.nargs = 0,
-   .nret = 1,
+   .nret = cpu_to_be32(1),
.rets = args.args[0],
};
 
@@ -312,7 +312,7 @@ static void pseries_remove_processor(struct device_node *np)
 {
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(np, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -323,7 +323,7 @@ static void pseries_remove_processor(struct device_node *np)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
BUG_ON(cpu_online(cpu));
set_cpu_present(cpu, false);
@@ -332,7 +332,7 @@ static void pseries_remove_processor(struct device_node *np)
}
if (cpu = nr_cpu_ids)
printk(KERN_WARNING Could not find cpu to remove 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 }
-- 
1.8.5.2

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