From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
Benjamin Gray <bgray@linux.ibm.com>
Subject: Re: [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness
Date: Tue, 20 Feb 2024 07:10:57 +0000 [thread overview]
Message-ID: <37a78bec-e598-478e-8a0a-3c4d9d9345db@csgroup.eu> (raw)
In-Reply-To: <87bkcg9rno.fsf@linux.ibm.com>
Michael ? Any reason for not picking that one ?
Le 30/10/2023 à 14:16, Aneesh Kumar K.V a écrit :
> Benjamin Gray <bgray@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@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@linux.ibm.com>
next prev parent reply other threads:[~2024-02-20 7:12 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-11 5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
2023-10-11 5:37 ` [PATCH 01/12] powerpc/xive: Fix endian conversion size Benjamin Gray
2023-10-11 5:37 ` [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness Benjamin Gray
2023-10-30 13:16 ` Aneesh Kumar K.V
2024-02-20 7:10 ` Christophe Leroy [this message]
2023-10-11 5:37 ` [PATCH 03/12] powerpc: Explicitly reverse bytes when checking for byte reversal Benjamin Gray
2023-10-11 5:37 ` [PATCH 04/12] powerpc: Use NULL instead of 0 for null pointers Benjamin Gray
2023-10-11 5:37 ` [PATCH 05/12] powerpc: Remove extern from function implementations Benjamin Gray
2023-10-11 5:37 ` [PATCH 06/12] powerpc: Annotate endianness of various variables and functions Benjamin Gray
2023-10-11 5:37 ` [PATCH 07/12] powerpc/kvm: Force cast endianness of KVM shared regs Benjamin Gray
2023-10-11 5:37 ` [PATCH 08/12] powerpc/opal: Annotate out param endianness Benjamin Gray
2023-10-11 5:37 ` [PATCH 09/12] powerpc/uaccess: Cast away __user annotation after verification Benjamin Gray
2023-10-11 5:37 ` [PATCH 10/12] powerpc: Cast away __iomem in low level IO routines Benjamin Gray
2023-10-11 5:37 ` [PATCH 11/12] powerpc/eeh: Remove unnecessary cast Benjamin Gray
2023-10-11 5:37 ` [PATCH 12/12] powerpc/fadump: Annotate endianness cast with __force Benjamin Gray
2023-10-27 9:59 ` (subset) [PATCH 00/12] Miscellaneous Sparse fixes Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=37a78bec-e598-478e-8a0a-3c4d9d9345db@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=aneesh.kumar@linux.ibm.com \
--cc=bgray@linux.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).