From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C04BDC4167B for ; Tue, 31 Oct 2023 22:59:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Ifdv2yHvR5c1Bg66JVeTFDCpkaW4wxAW49c7FYweXn8=; b=EGXgbdgTjuh1qg 5S9kAt7MgSuH3LsaHFEZXjVlm8RNwUWiDnpXMQdtg0+B4Jqlq1g+SWiDCxGJGndr69NhSzPtPbI78 6h/6Sz7a2T0So4VB19BUXMhSmYi/UTZI+GUg7LlJGSd6+DVgOM2wtSzuH5y5GrQdBLs8Fw4m90MYr 4yGtO7eNHc40oLylBKQNWJE6r+u1613X+DgrFTDIuSQoknrtccHC2Eq11At2ObI5PJutOSUSGAG+c Zy87UMGmwlAzgMKgEK16LEcECw3Iq8TbeTte8l1mxxSjirkkF5odX1/GWHZhh9C4gYCAJJNZjefv/ aBP0XA96FQcqT9WDn+jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qxxi3-006HOn-1N; Tue, 31 Oct 2023 22:59:43 +0000 Received: from mail-pl1-x62e.google.com ([2607:f8b0:4864:20::62e]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qxxhy-006HOP-23 for linux-riscv@lists.infradead.org; Tue, 31 Oct 2023 22:59:41 +0000 Received: by mail-pl1-x62e.google.com with SMTP id d9443c01a7336-1cc1ee2d8dfso43763345ad.3 for ; Tue, 31 Oct 2023 15:59:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1698793175; x=1699397975; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=LDJ8Iq5L9/V1OZAz3rRoxOdZZ5rvNVFmc7VfBTtpIhM=; b=bGnNYFZFMyK0H5rKaFgM9bWxu574MInOnJbmAjWmKxX4oTuKbGwahtM9KOMFHtK2QZ gqJkRafqfQ2QJ/BuYTpJQLS+WgYoVECmMNF9lAQqiKDG3LVcoJ+lVsAXKbWuKOK7qyyE oao7SLI4i/WuSyD/pr9uJiJWSP92f96cQ3UEvtPcimpEI9RgGhm90eykKcVhTmudg/Qq lJ21tyGPtdn5Re3f6l+evelhDk7vGYWBauvXh8hmHYlMKdBpXBuvVfDf1+ld61zktaHv ge17JPBbppO9j5AitK1z+3ETNJgYkJMPpEtbIbJ70hI3Jl77Tl9BhgWnUJKTJXwPspLJ JDxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698793175; x=1699397975; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=LDJ8Iq5L9/V1OZAz3rRoxOdZZ5rvNVFmc7VfBTtpIhM=; b=LKNQ9v3xDx9eggIkBdv9bZEiE9mfVmX0V6UUyzbSccbGsWuyrVsznrTc4cZLGLr8/A IlCSV0o/WRr4p7fZ8N3d80wi2ApOtOgTNtPiM4q6uJx1vmaeTb5QphnyU2JoIlCsVfpD J5mfNmjNk0tD7sA4eGoRlXtSZtuKpYFthhSmbDTgmi8c9cMpxaNmORg+ZT0LIkbN0SLx uchLgLhkNektHdDfSjdvffj3cBSHwzVQJKu0jyt4AJi+KZjEFkYmKnvEc6ZkK5thmTVy xshX1x/4eR7OYQdLVYpNEO5nSS63sSFrCFmHGPGQ7I5rwQd/tOvhq4FnpQETB9Iismjl P0Pg== X-Gm-Message-State: AOJu0YziQvxmQcCeZjZ/ojstbAWOLjhwHIZQU6xiDN/JyAHy85EukQpf sIqNIWueHgAfM+5sD4vRHTze4w== X-Google-Smtp-Source: AGHT+IF7cypq62FZglsiltBntullxZkWLgXL2yhaj4pSt3Ony6luiH3fIVR9OnmDImDDPQKANBZEMg== X-Received: by 2002:a17:902:d4c9:b0:1cc:4e79:4b38 with SMTP id o9-20020a170902d4c900b001cc4e794b38mr8017652plg.3.1698793175507; Tue, 31 Oct 2023 15:59:35 -0700 (PDT) Received: from ghost ([12.44.203.122]) by smtp.gmail.com with ESMTPSA id e18-20020a17090301d200b001cc32f46757sm83952plh.107.2023.10.31.15.59.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Oct 2023 15:59:35 -0700 (PDT) Date: Tue, 31 Oct 2023 15:59:32 -0700 From: Charlie Jenkins To: "Wang, Xiao W" Cc: Palmer Dabbelt , Conor Dooley , Samuel Holland , David Laight , Evan Green , "linux-riscv@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-arch@vger.kernel.org" , Paul Walmsley , Albert Ou , Arnd Bergmann , Conor Dooley Subject: Re: [PATCH v8 4/5] riscv: Add checksum library Message-ID: References: <20231027-optimize_checksum-v8-0-feb7101d128d@rivosinc.com> <20231027-optimize_checksum-v8-4-feb7101d128d@rivosinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231031_155938_682609_C98085BD X-CRM114-Status: GOOD ( 43.12 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Tue, Oct 31, 2023 at 09:51:17AM +0000, Wang, Xiao W wrote: > > > > -----Original Message----- > > From: Charlie Jenkins > > Sent: Saturday, October 28, 2023 6:44 AM > > To: Charlie Jenkins ; Palmer Dabbelt > > ; Conor Dooley ; Samuel Holland > > ; David Laight ; > > Wang, Xiao W ; Evan Green ; > > linux-riscv@lists.infradead.org; linux-kernel@vger.kernel.org; linux- > > arch@vger.kernel.org > > Cc: Paul Walmsley ; Albert Ou > > ; Arnd Bergmann ; Conor Dooley > > > > Subject: [PATCH v8 4/5] riscv: Add checksum library > > > > Provide a 32 and 64 bit version of do_csum. When compiled for 32-bit > > will load from the buffer in groups of 32 bits, and when compiled for > > 64-bit will load in groups of 64 bits. > > > > Signed-off-by: Charlie Jenkins > > Acked-by: Conor Dooley > > --- > > arch/riscv/lib/Makefile | 1 + > > arch/riscv/lib/csum.c | 339 > > ++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 340 insertions(+) > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > index 26cb2502ecf8..2aa1a4ad361f 100644 > > --- a/arch/riscv/lib/Makefile > > +++ b/arch/riscv/lib/Makefile > > @@ -6,6 +6,7 @@ lib-y += memmove.o > > lib-y += strcmp.o > > lib-y += strlen.o > > lib-y += strncmp.o > > +lib-y += csum.o > > lib-$(CONFIG_MMU) += uaccess.o > > lib-$(CONFIG_64BIT) += tishift.o > > lib-$(CONFIG_RISCV_ISA_ZICBOZ) += clear_page.o > > diff --git a/arch/riscv/lib/csum.c b/arch/riscv/lib/csum.c > > new file mode 100644 > > index 000000000000..f90e73606597 > > --- /dev/null > > +++ b/arch/riscv/lib/csum.c > > @@ -0,0 +1,339 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * IP checksum library > > Same comment as patch 3/5. > > > + * > > + * Influenced by arch/arm64/lib/csum.c > > + * Copyright (C) 2023 Rivos Inc. > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +/* Default version is sufficient for 32 bit */ > > +#ifndef CONFIG_32BIT > > Why not use the same #if macro "#ifdef CONFIG_64BIT" as in checksum.h > > > +__sum16 csum_ipv6_magic(const struct in6_addr *saddr, > > + const struct in6_addr *daddr, > > + __u32 len, __u8 proto, __wsum csum) > > +{ > > + unsigned int ulen, uproto; > > + unsigned long sum = csum; > > + > > + sum += saddr->s6_addr32[0]; > > + sum += saddr->s6_addr32[1]; > > + sum += saddr->s6_addr32[2]; > > + sum += saddr->s6_addr32[3]; > > + > > + sum += daddr->s6_addr32[0]; > > + sum += daddr->s6_addr32[1]; > > + sum += daddr->s6_addr32[2]; > > + sum += daddr->s6_addr32[3]; > > + > > + ulen = htonl((unsigned int)len); > > + sum += ulen; > > + > > + uproto = htonl(proto); > > + sum += uproto; > > + > > + /* > > + * Zbb support saves 4 instructions, so not worth checking without > > + * alternatives if supported > > + */ > > + if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && > > + IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { > > + unsigned long fold_temp; > > + > > + /* > > + * Zbb is likely available when the kernel is compiled with Zbb > > + * support, so nop when Zbb is available and jump when Zbb > > is > > + * not available. > > + */ > > + asm_volatile_goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0, > > + RISCV_ISA_EXT_ZBB, 1) > > + : > > + : > > + : > > + : no_zbb); > > + asm(".option push \n\ > > + .option arch,+zbb \n\ > > + rori %[fold_temp], %[sum], 32 \n\ > > + add %[sum], %[fold_temp], %[sum] > > \n\ > > + srli %[sum], %[sum], 32 \n\ > > + not %[fold_temp], %[sum] \n\ > > + roriw %[sum], %[sum], 16 \n\ > > + subw %[sum], %[fold_temp], %[sum] > > \n\ > > + .option pop" > > + : [sum] "+r" (sum), [fold_temp] "=&r" (fold_temp)); > > + return (__force __sum16)(sum >> 16); > > + } > > +no_zbb: > > + sum += ror64(sum, 32); > > + sum >>= 32; > > + return csum_fold((__force __wsum)sum); > > +} > > +EXPORT_SYMBOL(csum_ipv6_magic); > > +#endif /* !CONFIG_32BIT */ > > + > > +#ifdef CONFIG_32BIT > > +#define OFFSET_MASK 3 > > +#elif CONFIG_64BIT > > +#define OFFSET_MASK 7 > > +#endif > > + > > +/* > > + * Algorithm accounts for buff being misaligned. > > + * If buff is not aligned, will over-read bytes but not use the bytes that it > > + * shouldn't. The same thing will occur on the tail-end of the read. > > + */ > > +static inline __no_sanitize_address unsigned int > > do_csum_with_alignment(const unsigned char *buff, int len) > > +{ > > + unsigned int offset, shift; > > + unsigned long csum = 0, carry = 0, data; > > + const unsigned long *ptr, *end; > > + > > + end = (const unsigned long *)(buff + len); > > + > > + /* > > + * Align address to closest word (double word on rv64) that comes > > before > > + * buff. This should always be in the same page and cache line. > > + * Directly call KASAN with the alignment we will be using. > > + */ > > + offset = (unsigned long)buff & OFFSET_MASK; > > + kasan_check_read(buff, len); > > + ptr = (const unsigned long *)(buff - offset); > > + > > + /* > > + * Clear the most significant bytes that were over-read if buff was not > > + * aligned. > > + */ > > + shift = offset * 8; > > + data = *(ptr++); > > +#ifdef __LITTLE_ENDIAN > > + data = (data >> shift) << shift; > > +#else > > + data = (data << shift) >> shift; > > +#endif > > + /* > > + * Do 32-bit reads on RV32 and 64-bit reads otherwise. This should be > > + * faster than doing 32-bit reads on architectures that support larger > > + * reads. > > + */ > > + while (ptr < end) { > > + csum += data; > > + carry += csum < data; > > + len -= sizeof(long); > > + data = *(ptr++); > > + } > > + > > + /* > > + * Perform alignment (and over-read) bytes on the tail if any bytes > > + * leftover. > > + */ > > + shift = ((long)ptr - (long)end) * 8; > > +#ifdef __LITTLE_ENDIAN > > + data = (data << shift) >> shift; > > +#else > > + data = (data >> shift) << shift; > > +#endif > > + csum += data; > > + carry += csum < data; > > + csum += carry; > > + csum += csum < carry; > > + > > + /* > > + * Zbb support saves 6 instructions, so not worth checking without > > + * alternatives if supported > > + */ > > + if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && > > + IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { > > + unsigned long fold_temp; > > + > > + /* > > + * Zbb is likely available when the kernel is compiled with Zbb > > + * support, so nop when Zbb is available and jump when Zbb > > is > > + * not available. > > + */ > > + asm_volatile_goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0, > > + RISCV_ISA_EXT_ZBB, 1) > > + : > > + : > > + : > > + : no_zbb); > > + > > +#ifdef CONFIG_32BIT > > + asm_volatile_goto(".option push \n\ > > + .option arch,+zbb \n\ > > + rori %[fold_temp], %[csum], 16 \n\ > > + andi %[offset], %[offset], 1 \n\ > > + add %[csum], %[fold_temp], %[csum] \n\ > > + beq %[offset], zero, %l[end] \n\ > > + rev8 %[csum], %[csum] \n\ > > + .option pop" > > + : [csum] "+r" (csum), > > + [fold_temp] "=&r" (fold_temp) > > + : [offset] "r" (offset) > > + : > > + : end); > > + > > + return (unsigned short)csum; > > +#else /* !CONFIG_32BIT */ > > + asm_volatile_goto(".option push \n\ > > + .option arch,+zbb \n\ > > + rori %[fold_temp], %[csum], 32 \n\ > > + add %[csum], %[fold_temp], %[csum] \n\ > > + srli %[csum], %[csum], 32 \n\ > > + roriw %[fold_temp], %[csum], 16 \n\ > > + addw %[csum], %[fold_temp], %[csum] \n\ > > + andi %[offset], %[offset], 1 \n\ > > + beq %[offset], zero, %l[end] \n\ > > + rev8 %[csum], %[csum] \n\ > > + .option pop" > > + : [csum] "+r" (csum), > > + [fold_temp] "=&r" (fold_temp) > > + : [offset] "r" (offset) > > + : > > + : end); > > + > > + return (csum << 16) >> 48; > > +#endif /* !CONFIG_32BIT */ > > +end: > > + return csum >> 16; > > + } > > +no_zbb: > > +#ifndef CONFIG_32BIT > > + csum += ror64(csum, 32); > > + csum >>= 32; > > +#endif > > + csum = (u32)csum + ror32((u32)csum, 16); > > + if (offset & 1) > > + return (u16)swab32(csum); > > + return csum >> 16; > > +} > > + > > +/* > > + * Does not perform alignment, should only be used if machine has fast > > + * misaligned accesses, because buff may be misaligned. > > + */ > > +static inline unsigned int do_csum_no_alignment(const unsigned char *buff, > > int len) > > +{ > > + unsigned int offset, shift; > > + unsigned long csum = 0, carry = 0, data; > > + const unsigned long *ptr, *end; > > + > > + end = (const unsigned long *)(buff + len); > > + > > kasan_check_read() missing in this function. > I had thought it wasn't needed since this is the aligned access case so I removed __no_sanitize_address from the function signature. However, I just realized that on the tail end this function over-reads buff so it is still necessary. > > + ptr = (const unsigned long *)(buff); > > + > > + data = *(ptr++); > > + > > + /* > > + * Do 32-bit reads on RV32 and 64-bit reads otherwise. This should be > > + * faster than doing 32-bit reads on architectures that support larger > > + * reads. > > + */ > > + while (ptr < end) { > > + csum += data; > > + carry += csum < data; > > + len -= sizeof(long); > > + data = *(ptr++); > > + } > > + > > + /* > > + * Perform alignment (and over-read) bytes on the tail if any bytes > > + * leftover. > > + */ > > + shift = ((long)ptr - (long)end) * 8; > > +#ifdef __LITTLE_ENDIAN > > + data = (data << shift) >> shift; > > +#else > > + data = (data >> shift) << shift; > > +#endif > > + csum += data; > > + carry += csum < data; > > + csum += carry; > > + csum += csum < carry; > > + > > + /* > > + * Zbb support saves 6 instructions, so not worth checking without > > + * alternatives if supported > > + */ > > + if (IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && > > + IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) { > > + unsigned long fold_temp; > > + > > + /* > > + * Zbb is likely available when the kernel is compiled with Zbb > > + * support, so nop when Zbb is available and jump when Zbb > > is > > + * not available. > > + */ > > + asm_volatile_goto(ALTERNATIVE("j %l[no_zbb]", "nop", 0, > > + RISCV_ISA_EXT_ZBB, 1) > > + : > > + : > > + : > > + : no_zbb); > > + > > +#ifdef CONFIG_32BIT > > + asm (".option push \n\ > > + .option arch,+zbb \n\ > > + rori %[fold_temp], %[csum], 16 \n\ > > + andi %[offset], %[offset], 1 \n\ > > + add %[csum], %[fold_temp], %[csum] \n\ > > + .option pop" > > + : [csum] "+r" (csum), > > + [fold_temp] "=&r" (fold_temp) > > It's better to align the indention here, or we can follow the below CONFIG_64BIT case. > > > + : [offset] "r" (offset) > > + : ); > > + > > +#else /* !CONFIG_32BIT */ > > + asm (".option push \n\ > > + .option arch,+zbb \n\ > > + rori %[fold_temp], %[csum], 32 \n\ > > + add %[csum], %[fold_temp], %[csum] \n\ > > + srli %[csum], %[csum], 32 \n\ > > + roriw %[fold_temp], %[csum], 16 \n\ > > + addw %[csum], %[fold_temp], %[csum] \n\ > > + .option pop" > > + : [csum] "+r" (csum), [fold_temp] "=&r" (fold_temp) > > + : [offset] "r" (offset) > > + : ); > > +#endif /* !CONFIG_32BIT */ > > + return csum >> 16; > > + } > > +no_zbb: > > +#ifndef CONFIG_32BIT > > + csum += ror64(csum, 32); > > + csum >>= 32; > > +#endif > > + csum = (u32)csum + ror32((u32)csum, 16); > > + return csum >> 16; > > +} > > + > > +/* > > + * Perform a checksum on an arbitrary memory address. > > + * Will do a light-weight address alignment if buff is misaligned, unless > > + * cpu supports fast misaligned accesses. > > + */ > > +unsigned int do_csum(const unsigned char *buff, int len) > > +{ > > + if (unlikely(len <= 0)) > > + return 0; > > + > > + /* > > + * Very significant performance gains can be seen by not doing > > alignment > > + * on machines with fast misaligned accesses. > > + * > > + * There is some duplicate code between the "with_alignment" and > > + * "no_alignment" implmentations, but the overlap is too awkward to > > be > > + * able to fit in one function without introducing multiple static > > + * branches. > > + */ > > + if (static_branch_likely(&fast_misaligned_access_speed_key)) > > + return do_csum_no_alignment(buff, len); > > When CPU doesn't support fast misaligned access but the buff addr is aligned (checking > by buff & OFFSET_MASK == 0), did it worth adding this check and then possibly calling > do_csum_no_alignment()? I am not sure how common it is for buff to be unaligned. Adding another branch here will be bad for the unaligned access pathway. However, if the unaligned accesses are slow on the hardware, ideally software will prevent calling this function with unaligned buff. With this assumption, I will add a branch here since that will be beneficial for the aligned pathway. - Charlie > > BRs, > Xiao > > > + > > + return do_csum_with_alignment(buff, len); > > +} > > > > -- > > 2.42.0 > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv