All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal
@ 2014-09-10 22:41 Thomas Falcon
  2014-09-12  8:53 ` Michael Ellerman
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Falcon @ 2014-09-10 22:41 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: 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 <tlfalcon@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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal
  2014-09-10 22:41 [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal Thomas Falcon
@ 2014-09-12  8:53 ` Michael Ellerman
  2014-09-12 19:05   ` Thomas Falcon
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Ellerman @ 2014-09-12  8:53 UTC (permalink / raw)
  To: Thomas Falcon; +Cc: linuxppc-dev

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 <tlfalcon@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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal
  2014-09-12  8:53 ` Michael Ellerman
@ 2014-09-12 19:05   ` Thomas Falcon
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Falcon @ 2014-09-12 19:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

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 <tlfalcon@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

>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-09-12 19:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10 22:41 [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal Thomas Falcon
2014-09-12  8:53 ` Michael Ellerman
2014-09-12 19:05   ` Thomas Falcon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.