[PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness

[Christophe Leroy]

Michael ? Any reason for not picking that one ?

Le 30/10/2023 à 14:16, Aneesh Kumar K.V a écrit :
> Benjamin Gray <bgray at linux.ibm.com> writes:
> 
>> Sparse reports an endian mismatch in hvc_get_chars().
>>
>> At first it seemed like the retbuf should be __be64[], but actually
>> retbuf holds serialized registers returned by the hypervisor call, so
>> it's correctly CPU endian typed.
>>
>> Instead, it is the be64_to_cpu() that's misleading. The intent is to do
>> a byte swap on a little endian CPU. The swap is required because we
>> wanted to store the register values to memory without 'swapping' bytes,
>> so that the high order byte of the first register is the first byte
>> in the result buffer.
>>
>> In diagram form, on a LE CPU with the letters representing the return
>> string bytes:
>>
>>      (register bytes) A B C D E F G H   I J K L M N O P
>>    (retbuf mem bytes) H G F E D C B A   P O N M L K J I
>> (buf/lbuf mem bytes) A B C D E F G H   I J K L M N O P
>>
>> So retbuf stores the registers in CPU endian, and buf always stores in
>> big endian.
>>
>> So replace the byte swap function with cpu_to_be64() and cast lbuf as an
>> array of __be64 to match the semantics closer.
>>
>> Signed-off-by: Benjamin Gray <bgray at linux.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/hvconsole.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c
>> index 1ac52963e08b..647718a15e78 100644
>> --- a/arch/powerpc/platforms/pseries/hvconsole.c
>> +++ b/arch/powerpc/platforms/pseries/hvconsole.c
>> @@ -29,11 +29,11 @@ int hvc_get_chars(uint32_t vtermno, char *buf, int count)
>>   {
>>   	long ret;
>>   	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
>> -	unsigned long *lbuf = (unsigned long *)buf;
>> +	__be64 *lbuf = (__be64 __force *)buf;
>>   
>>   	ret = plpar_hcall(H_GET_TERM_CHAR, retbuf, vtermno);
>> -	lbuf[0] = be64_to_cpu(retbuf[1]);
>> -	lbuf[1] = be64_to_cpu(retbuf[2]);
>> +	lbuf[0] = cpu_to_be64(retbuf[1]);
>> +	lbuf[1] = cpu_to_be64(retbuf[2]);
>>   
>>   	if (ret == H_SUCCESS)
>>   		return retbuf[0];
>>
> 
> There is no functionality change in this patch. It is clarifying the
> details that it expect the buf to have the big-endian format and retbuf
> contains native endian format.
> 
> Not sure why this was not picked.
> 
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>