[PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Thomas Falcon]

The function rtas_flash_firmware passes the address of a data structure,
flash_block_list, when making the update-flash-64-and-reboot rtas call.
While the endianness of the address is handled correctly, the endianness
of the data is not.  This patch ensures that the data in flash_block_list
is big endian when passed to rtas on little endian hosts.

Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/rtas_flash.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 658e89d..db2b482 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -611,17 +611,19 @@ static void rtas_flash_firmware(int reboot_type)
 	for (f = flist; f; f = next) {
 		/* Translate data addrs to absolute */
 		for (i = 0; i < f->num_blocks; i++) {
-			f->blocks[i].data = (char *)__pa(f->blocks[i].data);
+			f->blocks[i].data = (char *)cpu_to_be64(__pa(f->blocks[i].data));
 			image_size += f->blocks[i].length;
+			f->blocks[i].length = cpu_to_be64(f->blocks[i].length);
 		}
 		next = f->next;
 		/* Don't translate NULL pointer for last entry */
 		if (f->next)
-			f->next = (struct flash_block_list *)__pa(f->next);
+			f->next = (struct flash_block_list *)cpu_to_be64(__pa(f->next));
 		else
 			f->next = NULL;
 		/* make num_blocks into the version/length field */
 		f->num_blocks = (FLASH_BLOCK_LIST_VERSION << 56) | ((f->num_blocks+1)*16);
+		f->num_blocks = cpu_to_be64(f->num_blocks);
 	}
 
 	printk(KERN_ALERT "FLASH: flash image is %ld bytes\n", image_size);
-- 
1.9.3

Re: [PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Michael Ellerman]

Hi Thomas,

On Fri, 2014-07-25 at 12:47 -0500, Thomas Falcon wrote:
> [PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

Minor nit, but the commit title should use the imperative mood, so eg:

  [PATCH] powerpc: Fix endianness of flash_block_list in rtas_flash

> The function rtas_flash_firmware passes the address of a data structure,
> flash_block_list, when making the update-flash-64-and-reboot rtas call.
> While the endianness of the address is handled correctly, the endianness
> of the data is not.  This patch ensures that the data in flash_block_list
> is big endian when passed to rtas on little endian hosts.

This looks good.

But you can do even better by changing the data types to be explicitly BE, so
eg. length should be __be64 I think.

Then if you build with "make C=2 CF=-D__CHECK_ENDIAN__" and have sparse
installed it will tell you when incorrectly assign from/to the BE types.

cheers

Re: [PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Vasant Hegde]

On 07/25/2014 11:17 PM, Thomas Falcon wrote:
> The function rtas_flash_firmware passes the address of a data structure,
> flash_block_list, when making the update-flash-64-and-reboot rtas call.
> While the endianness of the address is handled correctly, the endianness
> of the data is not.  This patch ensures that the data in flash_block_list
> is big endian when passed to rtas on little endian hosts.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
>   arch/powerpc/kernel/rtas_flash.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>

Tom,

In validate_flash rtas call returns  update_results in BE.. I think we need 
below changes as well.

Rest looks good.

diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index db2b482..1eae0d8 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -449,7 +449,7 @@ error:
  static void validate_flash(struct rtas_validate_flash_t *args_buf)
  {
         int token = rtas_token("ibm,validate-flash-image");
-       int update_results;
+       __be32 update_results;
         s32 rc;

         rc = 0;
@@ -463,7 +463,7 @@ static void validate_flash(struct rtas_validate_flash_t 
*args_buf)
         } while (rtas_busy_delay(rc));

         args_buf->status = rc;
-       args_buf->update_results = update_results;
+       args_buf->update_results =  be32_to_cpu(update_results);
  }


-Vasant

Re: [PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Thomas Falcon]

On 08/01/2014 04:32 AM, Vasant Hegde wrote:
> On 07/25/2014 11:17 PM, Thomas Falcon wrote:
>> The function rtas_flash_firmware passes the address of a data structure,
>> flash_block_list, when making the update-flash-64-and-reboot rtas call.
>> While the endianness of the address is handled correctly, the endianness
>> of the data is not.  This patch ensures that the data in 
>> flash_block_list
>> is big endian when passed to rtas on little endian hosts.
>>
>> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/kernel/rtas_flash.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>
> Tom,
>
> In validate_flash rtas call returns  update_results in BE.. I think we 
> need below changes as well.
>
> Rest looks good.
>
> diff --git a/arch/powerpc/kernel/rtas_flash.c 
> b/arch/powerpc/kernel/rtas_flash.c
> index db2b482..1eae0d8 100644
> --- a/arch/powerpc/kernel/rtas_flash.c
> +++ b/arch/powerpc/kernel/rtas_flash.c
> @@ -449,7 +449,7 @@ error:
>  static void validate_flash(struct rtas_validate_flash_t *args_buf)
>  {
>         int token = rtas_token("ibm,validate-flash-image");
> -       int update_results;
> +       __be32 update_results;
>         s32 rc;
>
>         rc = 0;
> @@ -463,7 +463,7 @@ static void validate_flash(struct 
> rtas_validate_flash_t *args_buf)
>         } while (rtas_busy_delay(rc));
>
>         args_buf->status = rc;
> -       args_buf->update_results = update_results;
> +       args_buf->update_results = be32_to_cpu(update_results);
>  }
>
I do not think this conversion is needed.  Any integers returned are 
converted to cpu endian in the rtas_call function.

tom
>
> -Vasant
>

--