From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
To: Guenter Roeck <linux@roeck-us.net>,
Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>,
linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "sh: Handle calling csum_partial with misaligned data"
Date: Thu, 02 May 2024 12:28:59 +0200 [thread overview]
Message-ID: <1b7525bcda4c58eccaf7737bd33bdb1b7f764e28.camel@physik.fu-berlin.de> (raw)
In-Reply-To: <20240324231804.841099-1-linux@roeck-us.net>
On Sun, 2024-03-24 at 16:18 -0700, Guenter Roeck wrote:
> This reverts commit cadc4e1a2b4d20d0cc0e81f2c6ba0588775e54e5.
>
> Commit cadc4e1a2b4d ("sh: Handle calling csum_partial with misaligned
> data") causes bad checksum calculations on unaligned data. Reverting
> it fixes the problem.
>
> # Subtest: checksum
> # module: checksum_kunit
> 1..5
> # test_csum_fixed_random_inputs: ASSERTION FAILED at lib/checksum_kunit.c:500
> Expected ( u64)result == ( u64)expec, but
> ( u64)result == 53378 (0xd082)
> ( u64)expec == 33488 (0x82d0)
> # test_csum_fixed_random_inputs: pass:0 fail:1 skip:0 total:1
> not ok 1 test_csum_fixed_random_inputs
> # test_csum_all_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:525
> Expected ( u64)result == ( u64)expec, but
> ( u64)result == 65281 (0xff01)
> ( u64)expec == 65280 (0xff00)
> # test_csum_all_carry_inputs: pass:0 fail:1 skip:0 total:1
> not ok 2 test_csum_all_carry_inputs
> # test_csum_no_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:573
> Expected ( u64)result == ( u64)expec, but
> ( u64)result == 65535 (0xffff)
> ( u64)expec == 65534 (0xfffe)
> # test_csum_no_carry_inputs: pass:0 fail:1 skip:0 total:1
> not ok 3 test_csum_no_carry_inputs
> # test_ip_fast_csum: pass:1 fail:0 skip:0 total:1
> ok 4 test_ip_fast_csum
> # test_csum_ipv6_magic: pass:1 fail:0 skip:0 total:1
> ok 5 test_csum_ipv6_magic
> # checksum: pass:2 fail:3 skip:0 total:5
> # Totals: pass:2 fail:3 skip:0 total:5
> not ok 22 checksum
>
> Fixes: cadc4e1a2b4d ("sh: Handle calling csum_partial with misaligned data")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> arch/sh/lib/checksum.S | 67 ++++++++++++------------------------------
> 1 file changed, 18 insertions(+), 49 deletions(-)
>
> diff --git a/arch/sh/lib/checksum.S b/arch/sh/lib/checksum.S
> index 3e07074e0098..06fed5a21e8b 100644
> --- a/arch/sh/lib/checksum.S
> +++ b/arch/sh/lib/checksum.S
> @@ -33,7 +33,8 @@
> */
>
> /*
> - * asmlinkage __wsum csum_partial(const void *buf, int len, __wsum sum);
> + * unsigned int csum_partial(const unsigned char *buf, int len,
> + * unsigned int sum);
> */
>
> .text
> @@ -45,31 +46,11 @@ ENTRY(csum_partial)
> * Fortunately, it is easy to convert 2-byte alignment to 4-byte
> * alignment for the unrolled loop.
> */
> + mov r5, r1
> mov r4, r0
> - tst #3, r0 ! Check alignment.
> - bt/s 2f ! Jump if alignment is ok.
> - mov r4, r7 ! Keep a copy to check for alignment
> + tst #2, r0 ! Check alignment.
> + bt 2f ! Jump if alignment is ok.
> !
> - tst #1, r0 ! Check alignment.
> - bt 21f ! Jump if alignment is boundary of 2bytes.
> -
> - ! buf is odd
> - tst r5, r5
> - add #-1, r5
> - bt 9f
> - mov.b @r4+, r0
> - extu.b r0, r0
> - addc r0, r6 ! t=0 from previous tst
> - mov r6, r0
> - shll8 r6
> - shlr16 r0
> - shlr8 r0
> - or r0, r6
> - mov r4, r0
> - tst #2, r0
> - bt 2f
> -21:
> - ! buf is 2 byte aligned (len could be 0)
> add #-2, r5 ! Alignment uses up two bytes.
> cmp/pz r5 !
> bt/s 1f ! Jump if we had at least two bytes.
> @@ -77,17 +58,16 @@ ENTRY(csum_partial)
> bra 6f
> add #2, r5 ! r5 was < 2. Deal with it.
> 1:
> + mov r5, r1 ! Save new len for later use.
> mov.w @r4+, r0
> extu.w r0, r0
> addc r0, r6
> bf 2f
> add #1, r6
> 2:
> - ! buf is 4 byte aligned (len could be 0)
> - mov r5, r1
> mov #-5, r0
> - shld r0, r1
> - tst r1, r1
> + shld r0, r5
> + tst r5, r5
> bt/s 4f ! if it's =0, go to 4f
> clrt
> .align 2
> @@ -109,31 +89,30 @@ ENTRY(csum_partial)
> addc r0, r6
> addc r2, r6
> movt r0
> - dt r1
> + dt r5
> bf/s 3b
> cmp/eq #1, r0
> - ! here, we know r1==0
> - addc r1, r6 ! add carry to r6
> + ! here, we know r5==0
> + addc r5, r6 ! add carry to r6
> 4:
> - mov r5, r0
> + mov r1, r0
> and #0x1c, r0
> tst r0, r0
> - bt 6f
> - ! 4 bytes or more remaining
> - mov r0, r1
> - shlr2 r1
> + bt/s 6f
> + mov r0, r5
> + shlr2 r5
> mov #0, r2
> 5:
> addc r2, r6
> mov.l @r4+, r2
> movt r0
> - dt r1
> + dt r5
> bf/s 5b
> cmp/eq #1, r0
> addc r2, r6
> - addc r1, r6 ! r1==0 here, so it means add carry-bit
> + addc r5, r6 ! r5==0 here, so it means add carry-bit
> 6:
> - ! 3 bytes or less remaining
> + mov r1, r5
> mov #3, r0
> and r0, r5
> tst r5, r5
> @@ -159,16 +138,6 @@ ENTRY(csum_partial)
> mov #0, r0
> addc r0, r6
> 9:
> - ! Check if the buffer was misaligned, if so realign sum
> - mov r7, r0
> - tst #1, r0
> - bt 10f
> - mov r6, r0
> - shll8 r6
> - shlr16 r0
> - shlr8 r0
> - or r0, r6
> -10:
> rts
> mov r6, r0
Applied to my sh-linux tree in the for-next branch.
Thanks,
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
prev parent reply other threads:[~2024-05-02 10:29 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-24 23:18 [PATCH] Revert "sh: Handle calling csum_partial with misaligned data" Guenter Roeck
2024-03-25 7:39 ` John Paul Adrian Glaubitz
2024-03-25 14:34 ` Guenter Roeck
2024-04-02 14:06 ` Guenter Roeck
2024-04-02 14:09 ` John Paul Adrian Glaubitz
2024-05-01 8:28 ` John Paul Adrian Glaubitz
2024-05-01 14:44 ` Guenter Roeck
2024-05-01 14:49 ` John Paul Adrian Glaubitz
2024-05-02 7:21 ` Geert Uytterhoeven
2024-05-02 7:23 ` John Paul Adrian Glaubitz
2024-03-25 15:35 ` Guenter Roeck
2024-05-02 10:28 ` John Paul Adrian Glaubitz [this message]
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=1b7525bcda4c58eccaf7737bd33bdb1b7f764e28.camel@physik.fu-berlin.de \
--to=glaubitz@physik.fu-berlin.de \
--cc=dalias@libc.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=ysato@users.sourceforge.jp \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).