LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 07/30] ABI: sysfs-class-cxl: place "not in a guest" at description
From: Mauro Carvalho Chehab @ 2021-09-16  8:59 UTC (permalink / raw)
  To: Linux Doc Mailing List, Greg Kroah-Hartman
  Cc: Andrew Donnellan, Jonathan Corbet, Mauro Carvalho Chehab,
	linux-kernel, Frederic Barrat, linuxppc-dev
In-Reply-To: <cover.1631782432.git.mchehab+huawei@kernel.org>

The What: field should have just the location of the ABI.
Anything else should be inside the description.

This fixes its parsing by get_abi.pl script.

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 Documentation/ABI/testing/sysfs-class-cxl | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-cxl b/Documentation/ABI/testing/sysfs-class-cxl
index 818f55970efb..3c77677e0ca7 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -166,10 +166,11 @@ Description:    read only
                 Decimal value of the Per Process MMIO space length.
 Users:		https://github.com/ibm-capi/libcxl
 
-What:           /sys/class/cxl/<afu>m/pp_mmio_off (not in a guest)
+What:           /sys/class/cxl/<afu>m/pp_mmio_off
 Date:           September 2014
 Contact:        linuxppc-dev@lists.ozlabs.org
 Description:    read only
+                (not in a guest)
                 Decimal value of the Per Process MMIO space offset.
 Users:		https://github.com/ibm-capi/libcxl
 
@@ -190,28 +191,31 @@ Description:    read only
                 Identifies the revision level of the PSL.
 Users:		https://github.com/ibm-capi/libcxl
 
-What:           /sys/class/cxl/<card>/base_image (not in a guest)
+What:           /sys/class/cxl/<card>/base_image
 Date:           September 2014
 Contact:        linuxppc-dev@lists.ozlabs.org
 Description:    read only
+                (not in a guest)
                 Identifies the revision level of the base image for devices
                 that support loadable PSLs. For FPGAs this field identifies
                 the image contained in the on-adapter flash which is loaded
                 during the initial program load.
 Users:		https://github.com/ibm-capi/libcxl
 
-What:           /sys/class/cxl/<card>/image_loaded (not in a guest)
+What:           /sys/class/cxl/<card>/image_loaded
 Date:           September 2014
 Contact:        linuxppc-dev@lists.ozlabs.org
 Description:    read only
+                (not in a guest)
                 Will return "user" or "factory" depending on the image loaded
                 onto the card.
 Users:		https://github.com/ibm-capi/libcxl
 
-What:           /sys/class/cxl/<card>/load_image_on_perst (not in a guest)
+What:           /sys/class/cxl/<card>/load_image_on_perst
 Date:           December 2014
 Contact:        linuxppc-dev@lists.ozlabs.org
 Description:    read/write
+                (not in a guest)
                 Valid entries are "none", "user", and "factory".
                 "none" means PERST will not cause image to be loaded to the
                 card.  A power cycle is required to load the image.
@@ -235,10 +239,11 @@ Description:    write only
                 contexts on the card AFUs.
 Users:		https://github.com/ibm-capi/libcxl
 
-What:		/sys/class/cxl/<card>/perst_reloads_same_image (not in a guest)
+What:		/sys/class/cxl/<card>/perst_reloads_same_image
 Date:		July 2015
 Contact:	linuxppc-dev@lists.ozlabs.org
 Description:	read/write
+                (not in a guest)
 		Trust that when an image is reloaded via PERST, it will not
 		have changed.
 
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH] serial: 8250: SERIAL_8250_FSL should not default to y when compile-testing
From: Johan Hovold @ 2021-09-16  9:08 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, linuxppc-dev,
	Li Yang, Scott Wood, open list:SERIAL DRIVERS, Shawn Guo,
	Jiri Slaby, Linux ARM
In-Reply-To: <CAMuHMdX7_AOuGEjvTHpQ-4KHMH+m800KTu7wads6UTfMZiu9BQ@mail.gmail.com>

On Thu, Sep 16, 2021 at 10:55:49AM +0200, Geert Uytterhoeven wrote:
> Hi Johan,
> 
> On Thu, Sep 16, 2021 at 10:46 AM Johan Hovold <johan@kernel.org> wrote:
> > On Wed, Sep 15, 2021 at 02:56:52PM +0200, Geert Uytterhoeven wrote:
> > > Commit b1442c55ce8977aa ("serial: 8250: extend compile-test coverage")
> > > added compile-test support to the Freescale 16550 driver.  However, as
> > > SERIAL_8250_FSL is an invisible symbol, merely enabling COMPILE_TEST now
> > > enables this driver.
> > >
> > > Fix this by making SERIAL_8250_FSL visible.  Tighten the dependencies to
> > > prevent asking the user about this driver when configuring a kernel
> > > without appropriate Freescale SoC or ACPI support.
> >
> > This tightening is arguable a separate change which risk introducing
> > regressions if you get it wrong and should go in a separate patch at
> > least.
> 
> Getting it wrong would indeed be a regression, but not tightening
> that at the same time would mean I have to send a separate patch with
> a Fixes tag referring to this fix, following this template:
> 
>     foo should depend on bar
> 
>     The foo hardware is only present on bar SoCs.  Hence add a
>     dependency on bar, to prevent asking the user about this driver
>     when configuring a kernel without bar support.

I know this is a pet peeve of yours, but asking users about one more
symbol when configuring their kernels is hardly something that requires
a Fixes tag.

Either way it's a pretty weak argument for not separating the change.

Johan

^ permalink raw reply

* Re: [PATCH] swiotlb: set IO TLB segment size via cmdline
From: Roman Skakun @ 2021-09-16  9:40 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: linux-doc, Peter Zijlstra, Viresh Kumar,
	Linux Kernel Mailing List, Paul Mackerras, Jan Beulich,
	Will Deacon, Boris Ostrovsky, Marek Szyprowski, Jonathan Corbet,
	Christoph Hellwig, xen-devel, Paul E. McKenney,
	Konrad Rzeszutek Wilk, Muchun Song, Thomas Gleixner,
	Juergen Gross, Thomas Bogendoerfer, Andrii Anisov, linuxppc-dev,
	Randy Dunlap, linux-mips, iommu, Roman Skakun, Andrew Morton,
	Maciej W. Rozycki, Robin Murphy, Mike Rapoport, Lu Baolu
In-Reply-To: <alpine.DEB.2.21.2109141838290.21985@sstabellini-ThinkPad-T480s>

Hi Stefano,

> Also, Option 1 listed in the webpage seems to be a lot better. Any
> reason you can't do that? Because that option both solves the problem
> and increases performance.

Yes, Option 1 is probably more efficient.
But I use another platform under Xen without DMA adjustment functionality.
I pinned this webpage only for example to describe the similar problem I had.

Cheers,
Roman

ср, 15 сент. 2021 г. в 04:51, Stefano Stabellini <sstabellini@kernel.org>:

>
> On Tue, 14 Sep 2021, Christoph Hellwig wrote:
> > On Tue, Sep 14, 2021 at 05:29:07PM +0200, Jan Beulich wrote:
> > > I'm not convinced the swiotlb use describe there falls under "intended
> > > use" - mapping a 1280x720 framebuffer in a single chunk? (As an aside,
> > > the bottom of this page is also confusing, as following "Then we can
> > > confirm the modified swiotlb size in the boot log:" there is a log
> > > fragment showing the same original size of 64Mb.
> >
> > It doesn't.  We also do not add hacks to the kernel for whacky out
> > of tree modules.
>
> Also, Option 1 listed in the webpage seems to be a lot better. Any
> reason you can't do that? Because that option both solves the problem
> and increases performance.



--
Best Regards, Roman.

^ permalink raw reply

* Re: [PATCH 1/3] perf: Add macros to specify onchip L2/L3 accesses
From: Michael Ellerman @ 2021-09-16 10:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mark.rutland, atrajeev, ak, daniel, rnsastry, alexander.shishkin,
	Kajol Jain, linux-kernel, acme, ast, linux-perf-users, yao.jin,
	mingo, paulus, maddy, jolsa, namhyung, songliubraving,
	linuxppc-dev, kan.liang
In-Reply-To: <YUCMUZbchMjD54eY@hirez.programming.kicks-ass.net>

Peter Zijlstra <peterz@infradead.org> writes:
> On Tue, Sep 14, 2021 at 08:40:38PM +1000, Michael Ellerman wrote:
>> Peter Zijlstra <peterz@infradead.org> writes:
>
>> > I'm thinking we ought to keep hops as steps along the NUMA fabric, with
>> > 0 hops being the local node. That only gets us:
>> >
>> >  L2, remote=0, hops=HOPS_0 -- our L2
>> >  L2, remote=1, hops=HOPS_0 -- L2 on the local node but not ours
>> >  L2, remote=1, hops!=HOPS_0 -- L2 on a remote node
>> 
>> Hmm. I'm not sure about tying it directly to NUMA hops. I worry we're
>> going to see more and more systems where there's a hierarchy within the
>> chip/package, in addition to the traditional NUMA hierarchy.
>> 
>> Although then I guess it becomes a question of what exactly is a NUMA
>> hop, maybe the answer is that on those future systems those
>> intra-chip/package hops should be represented as NUMA hops.
>> 
>> It's not like we have a hard definition of what a NUMA hop is?
>
> Not really, typically whatever the BIOS/DT/whatever tables tell us. I
> think in case of Power you're mostly making things up in software :-)

Firmware is software so yes :)

> But yeah, I think we have plenty wriggle room there.

OK.

cheers

^ permalink raw reply

* Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
From: Michael Ellerman @ 2021-09-16 11:51 UTC (permalink / raw)
  To: Christoph Hellwig, Christophe Leroy
  Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
	dri-devel, platform-driver-x86, Paul Mackerras, linux-s390,
	Andi Kleen, Joerg Roedel, x86, amd-gfx, Christoph Hellwig,
	linux-graphics-maintainer, Tom Lendacky, Tianyu Lan,
	Borislav Petkov, kexec, linux-kernel, iommu, linux-fsdevel,
	linuxppc-dev
In-Reply-To: <YULztMRLJ55YLVU9@infradead.org>

Christoph Hellwig <hch@infradead.org> writes:
> On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote:
>> Could you please provide more explicit explanation why inlining such an
>> helper is considered as bad practice and messy ?
>
> Because now we get architectures to all subly differ.  Look at the mess
> for ioremap and the ioremap* variant.
>
> The only good reason to allow for inlines if if they are used in a hot
> path.  Which cc_platform_has is not, especially not on powerpc.

Yes I agree, it's not a hot path so it doesn't really matter, which is
why I Acked it.

I think it is possible to do both, share the declaration across arches
but also give arches flexibility to use an inline if they prefer, see
patch below.

I'm not suggesting we actually do that for this series now, but I think
it would solve the problem if we ever needed to in future.

cheers


diff --git a/arch/powerpc/platforms/pseries/cc_platform.c b/arch/powerpc/include/asm/cc_platform.h
similarity index 74%
rename from arch/powerpc/platforms/pseries/cc_platform.c
rename to arch/powerpc/include/asm/cc_platform.h
index e8021af83a19..6285c3c385a6 100644
--- a/arch/powerpc/platforms/pseries/cc_platform.c
+++ b/arch/powerpc/include/asm/cc_platform.h
@@ -7,13 +7,10 @@
  * Author: Tom Lendacky <thomas.lendacky@amd.com>
  */
 
-#include <linux/export.h>
 #include <linux/cc_platform.h>
-
-#include <asm/machdep.h>
 #include <asm/svm.h>
 
-bool cc_platform_has(enum cc_attr attr)
+static inline bool arch_cc_platform_has(enum cc_attr attr)
 {
 	switch (attr) {
 	case CC_ATTR_MEM_ENCRYPT:
@@ -23,4 +20,3 @@ bool cc_platform_has(enum cc_attr attr)
 		return false;
 	}
 }
-EXPORT_SYMBOL_GPL(cc_platform_has);
diff --git a/arch/x86/include/asm/cc_platform.h b/arch/x86/include/asm/cc_platform.h
new file mode 100644
index 000000000000..0a4220697043
--- /dev/null
+++ b/arch/x86/include/asm/cc_platform.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_CC_PLATFORM_H_
+#define _ASM_X86_CC_PLATFORM_H_
+
+bool arch_cc_platform_has(enum cc_attr attr);
+
+#endif // _ASM_X86_CC_PLATFORM_H_
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
index 3c9bacd3c3f3..77e8c3465979 100644
--- a/arch/x86/kernel/cc_platform.c
+++ b/arch/x86/kernel/cc_platform.c
@@ -11,11 +11,11 @@
 #include <linux/cc_platform.h>
 #include <linux/mem_encrypt.h>
 
-bool cc_platform_has(enum cc_attr attr)
+bool arch_cc_platform_has(enum cc_attr attr)
 {
 	if (sme_me_mask)
 		return amd_cc_platform_has(attr);
 
 	return false;
 }
-EXPORT_SYMBOL_GPL(cc_platform_has);
+EXPORT_SYMBOL_GPL(arch_cc_platform_has);
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index 253f3ea66cd8..f3306647c5d9 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -65,6 +65,8 @@ enum cc_attr {
 
 #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
 
+#include <asm/cc_platform.h>
+
 /**
  * cc_platform_has() - Checks if the specified cc_attr attribute is active
  * @attr: Confidential computing attribute to check
@@ -77,7 +79,10 @@ enum cc_attr {
  * * TRUE  - Specified Confidential Computing attribute is active
  * * FALSE - Specified Confidential Computing attribute is not active
  */
-bool cc_platform_has(enum cc_attr attr);
+static inline bool cc_platform_has(enum cc_attr attr)
+{
+	return arch_cc_platform_has(attr);
+}
 
 #else	/* !CONFIG_ARCH_HAS_CC_PLATFORM */
 



^ permalink raw reply related

* Re: [PATCH] nvmem: NVMEM_NINTENDO_OTP should depend on WII
From: Emmanuel Gil Peyrot @ 2021-09-16 12:12 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Emmanuel Gil Peyrot, Greg Kroah-Hartman, linux-kernel,
	Srinivas Kandagatla, Paul Mackerras, linuxppc-dev
In-Reply-To: <01318920709dddc4d85fe895e2083ca0eee234d8.1631611652.git.geert+renesas@glider.be>

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

Hi, thanks for this patch, once the Wii U platform will be added it will
need an additional test for WIIU, but for now this is:

Reviewed-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>

On Tue, Sep 14, 2021 at 11:29:49AM +0200, Geert Uytterhoeven wrote:
> The Nintendo Wii and Wii U OTP is only present on Nintendo Wii and Wii U
> consoles.  Hence add a dependency on WII, to prevent asking the user
> about this driver when configuring a kernel without Nintendo Wii and Wii
> U console support.
> 
> Fixes: 3683b761fe3a10ad ("nvmem: nintendo-otp: Add new driver for the Wii and Wii U OTP")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/nvmem/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 39854d43758be3fb..da414617a54d4b99 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -109,6 +109,7 @@ config MTK_EFUSE
>  
>  config NVMEM_NINTENDO_OTP
>  	tristate "Nintendo Wii and Wii U OTP Support"
> +	depends on WII || COMPILE_TEST
>  	help
>  	  This is a driver exposing the OTP of a Nintendo Wii or Wii U console.
>  
> -- 
> 2.25.1

-- 
Emmanuel Gil Peyrot

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: Paul Menzel @ 2021-09-16 14:22 UTC (permalink / raw)
  To: Nathan Chancellor, Nick Desaulniers
  Cc: Paul Menzel, llvm, Zhen Lei, linux-kernel, Paul Mackerras,
	Andrew Morton, linuxppc-dev

Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1
shows the warning below.

    arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function]
    get_unaligned16(const unsigned short *p)
    ^
    1 warning generated.

Fix it, by moving the check from the preprocessor to C, so the compiler
sees the use.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 lib/zlib_inflate/inffast.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index f19c4fbe1be7..444ad3c3ccd3 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start)
 			sfrom = (unsigned short *)(from);
 			loops = len >> 1;
 			do
-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
-			    *sout++ = *sfrom++;
-#else
-			    *sout++ = get_unaligned16(sfrom++);
-#endif
+			    *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
 			while (--loops);
 			out = (unsigned char *)sout;
 			from = (unsigned char *)sfrom;
-- 
2.33.0


^ permalink raw reply related

* RE: [PATCH] powerpc: warn on emulation of dcbz instruction
From: David Laight @ 2021-09-16 14:36 UTC (permalink / raw)
  To: 'Christophe Leroy', Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman
  Cc: Finn Thain, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, Stan Johnson
In-Reply-To: <43f736d4-8625-2848-786f-79b902d5c753@csgroup.eu>

From: Christophe Leroy
> Sent: 16 September 2021 08:24
> 
> Le 16/09/2021 à 09:16, Benjamin Herrenschmidt a écrit :
> > On Thu, 2021-09-16 at 17:15 +1000, Benjamin Herrenschmidt wrote:
> >> On Wed, 2021-09-15 at 16:31 +0200, Christophe Leroy wrote:
> >>> dcbz instruction shouldn't be used on non-cached memory. Using
> >>> it on non-cached memory can result in alignment exception and
> >>> implies a heavy handling.
> >>>
> >>> Instead of silentely emulating the instruction and resulting in
> >>> high
> >>> performance degradation, warn whenever an alignment exception is
> >>> taken due to dcbz, so that the user is made aware that dcbz
> >>> instruction has been used unexpectedly.
> >>>
> >>> Reported-by: Stan Johnson <userm57@yahoo.com>
> >>> Cc: Finn Thain <fthain@linux-m68k.org>
> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >>> ---
> >>>   arch/powerpc/kernel/align.c | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/powerpc/kernel/align.c
> >>> b/arch/powerpc/kernel/align.c
> >>> index bbb4181621dd..adc3a4a9c6e4 100644
> >>> --- a/arch/powerpc/kernel/align.c
> >>> +++ b/arch/powerpc/kernel/align.c
> >>> @@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
> >>>   		if (op.type != CACHEOP + DCBZ)
> >>>   			return -EINVAL;
> >>>   		PPC_WARN_ALIGNMENT(dcbz, regs);
> >>> +		WARN_ON_ONCE(1);
> >>
> >> This is heavy handed ... It will be treated as an oops by various
> >> things uselessly spit out a kernel backtrace. Isn't
> >> PPC_WARN_ALIGNMENT
> >> enough ?
> 
> 
> PPC_WARN_ALIGNMENT() only warns if explicitely activated, I want to
> catch uses on 'dcbz' on non-cached memory all the time as they are most
> often the result of using memset() instead of memset_io().
> 
> >
> > Ah I saw your other one about fbdev...  Ok what about you do that in a
> > if (!user_mode(regs)) ?
> 
> Yes I can do WARN_ON_ONCE(!user_mode(regs)); instead.
> 
> > Indeed the kernel should not do that.
> 
> Does userspace accesses non-cached memory directly ?

It probably can if a driver mmaps PCI space directly into user space.
That certainly works on x86-64.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [RFC PATCH 1/8] arm64: add CPU field to struct thread_info
From: Catalin Marinas @ 2021-09-16 14:41 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
	linux-s390, Arnd Bergmann, Russell King, Christian Borntraeger,
	Ingo Molnar, Albert Ou, Kees Cook, Vasily Gorbik, Heiko Carstens,
	Keith Packard, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
	Thomas Gleixner, linux-arm-kernel, linuxppc-dev, linux-kernel,
	Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-2-ardb@kernel.org>

On Tue, Sep 14, 2021 at 02:10:29PM +0200, Ard Biesheuvel wrote:
> The CPU field will be moved back into thread_info even when
> THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
> struct thread_info.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* [PATCH v2] powerpc: warn on emulation of dcbz instruction in kernel mode
From: Christophe Leroy @ 2021-09-16 14:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Stan Johnson, Finn Thain, linuxppc-dev, linux-kernel

dcbz instruction shouldn't be used on non-cached memory. Using
it on non-cached memory can result in alignment exception and
implies a heavy handling.

Instead of silentely emulating the instruction and resulting in high
performance degradation, warn whenever an alignment exception is
taken in kernel mode due to dcbz, so that the user is made aware that
dcbz instruction has been used unexpectedly by the kernel.

Reported-by: Stan Johnson <userm57@yahoo.com>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Warn only when emulating kernel mode
---
 arch/powerpc/kernel/align.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index bbb4181621dd..bf96b954a4eb 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -349,6 +349,7 @@ int fix_alignment(struct pt_regs *regs)
 		if (op.type != CACHEOP + DCBZ)
 			return -EINVAL;
 		PPC_WARN_ALIGNMENT(dcbz, regs);
+		WARN_ON_ONCE(!user_mode(regs));
 		r = emulate_dcbz(op.ea, regs);
 	} else {
 		if (type == LARX || type == STCX)
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: Nathan Chancellor @ 2021-09-16 14:58 UTC (permalink / raw)
  To: Paul Menzel, Nick Desaulniers
  Cc: llvm, Zhen Lei, linux-kernel, Paul Mackerras, Andrew Morton,
	linuxppc-dev
In-Reply-To: <20210916142210.26722-1-pmenzel@molgen.mpg.de>

Hi Paul,

On 9/16/2021 7:22 AM, Paul Menzel wrote:
> Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1
> shows the warning below.
> 
>      arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function]
>      get_unaligned16(const unsigned short *p)
>      ^
>      1 warning generated.
> 
> Fix it, by moving the check from the preprocessor to C, so the compiler
> sees the use.
> 
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>   lib/zlib_inflate/inffast.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
> index f19c4fbe1be7..444ad3c3ccd3 100644
> --- a/lib/zlib_inflate/inffast.c
> +++ b/lib/zlib_inflate/inffast.c
> @@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start)
>   			sfrom = (unsigned short *)(from);
>   			loops = len >> 1;
>   			do
> -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> -			    *sout++ = *sfrom++;
> -#else
> -			    *sout++ = get_unaligned16(sfrom++);
> -#endif
> +			    *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
>   			while (--loops);
>   			out = (unsigned char *)sout;
>   			from = (unsigned char *)sfrom;
> 

Thanks for the patch. This should probably be

IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ? ...

which matches the rest of the kernel tree, as certain CONFIG_... values 
are not guaranteed to always be defined.

Cheers,
Nathan

^ permalink raw reply

* Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
From: Borislav Petkov @ 2021-09-16 15:02 UTC (permalink / raw)
  To: Kuppuswamy, Sathyanarayanan
  Cc: linux-efi, Brijesh Singh, kvm, David Airlie, Dave Hansen,
	dri-devel, platform-driver-x86, Paul Mackerras, Will Deacon,
	Ard Biesheuvel, linux-s390, Andi Kleen, Baoquan He, Joerg Roedel,
	x86, amd-gfx, Christoph Hellwig, Christian Borntraeger,
	Ingo Molnar, linux-graphics-maintainer, Dave Young, Tom Lendacky,
	Tianyu Lan, Thomas Zimmermann, Vasily Gorbik, Heiko Carstens,
	Maarten Lankhorst, Maxime Ripard, Andy Lutomirski,
	Thomas Gleixner, Peter Zijlstra, kexec, linux-kernel, iommu,
	Daniel Vetter, linux-fsdevel, linuxppc-dev
In-Reply-To: <d48e6a17-d2b4-67da-56d1-fc9a61dfe2b8@linux.intel.com>

On Wed, Sep 15, 2021 at 10:26:06AM -0700, Kuppuswamy, Sathyanarayanan wrote:
> I have a Intel variant patch (please check following patch). But it includes
> TDX changes as well. Shall I move TDX changes to different patch and just
> create a separate patch for adding intel_cc_platform_has()?

Yes, please, so that I can expedite that stuff separately and so that it
can go in early in order for future work to be based ontop.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* [PATCH 5.10 163/306] hvsi: dont panic on tty_register_driver failure
From: Greg Kroah-Hartman @ 2021-09-16 15:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby, linuxppc-dev, stable
In-Reply-To: <20210916155753.903069397@linuxfoundation.org>

From: Jiri Slaby <jslaby@suse.cz>

[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]

The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.

So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.

This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().

Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210723074317.32690-3-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/tty/hvc/hvsi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index e8c58f9bd263..d6afaae1729a 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
 
 static int __init hvsi_init(void)
 {
-	int i;
+	int i, ret;
 
 	hvsi_driver = alloc_tty_driver(hvsi_count);
 	if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
 	}
 	hvsi_wait = wait_for_state; /* irqs active now */
 
-	if (tty_register_driver(hvsi_driver))
-		panic("Couldn't register hvsi console driver\n");
+	ret = tty_register_driver(hvsi_driver);
+	if (ret) {
+		pr_err("Couldn't register hvsi console driver\n");
+		goto err_free_irq;
+	}
 
 	printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
 
 	return 0;
+err_free_irq:
+	hvsi_wait = poll_for_state;
+	for (i = 0; i < hvsi_count; i++) {
+		struct hvsi_struct *hp = &hvsi_ports[i];
+
+		free_irq(hp->virq, hp);
+	}
+	tty_driver_kref_put(hvsi_driver);
+
+	return ret;
 }
 device_initcall(hvsi_init);
 
-- 
2.30.2




^ permalink raw reply related

* [PATCH 5.13 205/380] hvsi: dont panic on tty_register_driver failure
From: Greg Kroah-Hartman @ 2021-09-16 15:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby, linuxppc-dev, stable
In-Reply-To: <20210916155803.966362085@linuxfoundation.org>

From: Jiri Slaby <jslaby@suse.cz>

[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]

The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.

So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.

This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().

Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210723074317.32690-3-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/tty/hvc/hvsi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index e8c58f9bd263..d6afaae1729a 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
 
 static int __init hvsi_init(void)
 {
-	int i;
+	int i, ret;
 
 	hvsi_driver = alloc_tty_driver(hvsi_count);
 	if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
 	}
 	hvsi_wait = wait_for_state; /* irqs active now */
 
-	if (tty_register_driver(hvsi_driver))
-		panic("Couldn't register hvsi console driver\n");
+	ret = tty_register_driver(hvsi_driver);
+	if (ret) {
+		pr_err("Couldn't register hvsi console driver\n");
+		goto err_free_irq;
+	}
 
 	printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
 
 	return 0;
+err_free_irq:
+	hvsi_wait = poll_for_state;
+	for (i = 0; i < hvsi_count; i++) {
+		struct hvsi_struct *hp = &hvsi_ports[i];
+
+		free_irq(hp->virq, hp);
+	}
+	tty_driver_kref_put(hvsi_driver);
+
+	return ret;
 }
 device_initcall(hvsi_init);
 
-- 
2.30.2




^ permalink raw reply related

* [PATCH 5.14 232/432] hvsi: dont panic on tty_register_driver failure
From: Greg Kroah-Hartman @ 2021-09-16 15:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Sasha Levin, Greg Kroah-Hartman, Jiri Slaby, linuxppc-dev, stable
In-Reply-To: <20210916155810.813340753@linuxfoundation.org>

From: Jiri Slaby <jslaby@suse.cz>

[ Upstream commit 7ccbdcc4d08a6d7041e4849219bbb12ffa45db4c ]

The alloc_tty_driver failure is handled gracefully in hvsi_init. But
tty_register_driver is not. panic is called if that one fails.

So handle the failure of tty_register_driver gracefully too. This will
keep at least the console functional as it was enabled earlier by
console_initcall in hvsi_console_init. Instead of shooting down the
whole system.

This means, we disable interrupts and restore hvsi_wait back to
poll_for_state().

Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Link: https://lore.kernel.org/r/20210723074317.32690-3-jslaby@suse.cz
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/tty/hvc/hvsi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index bfc15279d5bc..f0bc8e780051 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1038,7 +1038,7 @@ static const struct tty_operations hvsi_ops = {
 
 static int __init hvsi_init(void)
 {
-	int i;
+	int i, ret;
 
 	hvsi_driver = alloc_tty_driver(hvsi_count);
 	if (!hvsi_driver)
@@ -1069,12 +1069,25 @@ static int __init hvsi_init(void)
 	}
 	hvsi_wait = wait_for_state; /* irqs active now */
 
-	if (tty_register_driver(hvsi_driver))
-		panic("Couldn't register hvsi console driver\n");
+	ret = tty_register_driver(hvsi_driver);
+	if (ret) {
+		pr_err("Couldn't register hvsi console driver\n");
+		goto err_free_irq;
+	}
 
 	printk(KERN_DEBUG "HVSI: registered %i devices\n", hvsi_count);
 
 	return 0;
+err_free_irq:
+	hvsi_wait = poll_for_state;
+	for (i = 0; i < hvsi_count; i++) {
+		struct hvsi_struct *hp = &hvsi_ports[i];
+
+		free_irq(hp->virq, hp);
+	}
+	tty_driver_kref_put(hvsi_driver);
+
+	return ret;
 }
 device_initcall(hvsi_init);
 
-- 
2.30.2




^ permalink raw reply related

* Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
From: Kuppuswamy, Sathyanarayanan @ 2021-09-16 18:38 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-efi, Brijesh Singh, kvm, David Airlie, Dave Hansen,
	dri-devel, platform-driver-x86, Paul Mackerras, Will Deacon,
	Ard Biesheuvel, linux-s390, Andi Kleen, Baoquan He, Joerg Roedel,
	x86, amd-gfx, Christoph Hellwig, Christian Borntraeger,
	Ingo Molnar, linux-graphics-maintainer, Dave Young, Tom Lendacky,
	Tianyu Lan, Thomas Zimmermann, Vasily Gorbik, Heiko Carstens,
	Maarten Lankhorst, Maxime Ripard, Andy Lutomirski,
	Thomas Gleixner, Peter Zijlstra, kexec, linux-kernel, iommu,
	Daniel Vetter, linux-fsdevel, linuxppc-dev
In-Reply-To: <YUNckGH0+KXdEmqu@zn.tnic>



On 9/16/21 8:02 AM, Borislav Petkov wrote:
> On Wed, Sep 15, 2021 at 10:26:06AM -0700, Kuppuswamy, Sathyanarayanan wrote:
>> I have a Intel variant patch (please check following patch). But it includes
>> TDX changes as well. Shall I move TDX changes to different patch and just
>> create a separate patch for adding intel_cc_platform_has()?
> 
> Yes, please, so that I can expedite that stuff separately and so that it
> can go in early in order for future work to be based ontop.

Sent it part of TDX patch series. Please check and cherry pick it.

https://lore.kernel.org/lkml/20210916183550.15349-2-sathyanarayanan.kuppuswamy@linux.intel.com/

> 
> Thx.
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

^ permalink raw reply

* [PATCH] powerpc/lib/sstep: Don't use __{get/put}_user() on kernel addresses
From: Christophe Leroy @ 2021-09-16 18:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: Stan Johnson, Finn Thain, linuxppc-dev, linux-kernel

In the old days, when we didn't have kernel userspace access
protection and had set_fs(), it was wise to use __get_user()
and friends to read kernel memory.

Nowadays, get_user() and put_user() are granting userspace access and
are exclusively for userspace access.

Convert single step emulation functions to user_access_begin() and
friends and use unsafe_get_user() and unsafe_put_user().

When addressing kernel addresses, there is no need to open userspace
access. And for book3s/32 it is particularly important to no try and
open userspace access on kernel address, because that would break the
content of kernel space segment registers. No guard has been put
against that risk in order to avoid degrading performance.

copy_from_kernel_nofault() and copy_to_kernel_nofault() should
be used but they are out-of-line functions which would degrade
performance. Those two functions are making use of
__get_kernel_nofault() and __put_kernel_nofault() macros.
Those two macros are just wrappers behind __get_user_size_goto() and
__put_user_size_goto().

unsafe_get_user() and unsafe_put_user() are also wrappers of
__get_user_size_goto() and __put_user_size_goto(). Use them to
access kernel space. That allows refactoring userspace and
kernelspace access.

Reported-by: Stan Johnson <userm57@yahoo.com>
Cc: Finn Thain <fthain@linux-m68k.org>
Depends-on: 4fe5cda9f89d ("powerpc/uaccess: Implement user_read_access_begin and user_write_access_begin")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/sstep.c | 197 ++++++++++++++++++++++++++++-----------
 1 file changed, 140 insertions(+), 57 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d8d5f901cee1..86f49e3e7cf5 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -302,33 +302,51 @@ static nokprobe_inline void do_byte_reverse(void *ptr, int nb)
 	}
 }
 
-static nokprobe_inline int read_mem_aligned(unsigned long *dest,
-					    unsigned long ea, int nb,
-					    struct pt_regs *regs)
+static __always_inline int
+__read_mem_aligned(unsigned long *dest, unsigned long ea, int nb, struct pt_regs *regs)
 {
-	int err = 0;
 	unsigned long x = 0;
 
 	switch (nb) {
 	case 1:
-		err = __get_user(x, (unsigned char __user *) ea);
+		unsafe_get_user(x, (unsigned char __user *)ea, Efault);
 		break;
 	case 2:
-		err = __get_user(x, (unsigned short __user *) ea);
+		unsafe_get_user(x, (unsigned short __user *)ea, Efault);
 		break;
 	case 4:
-		err = __get_user(x, (unsigned int __user *) ea);
+		unsafe_get_user(x, (unsigned int __user *)ea, Efault);
 		break;
 #ifdef __powerpc64__
 	case 8:
-		err = __get_user(x, (unsigned long __user *) ea);
+		unsafe_get_user(x, (unsigned long __user *)ea, Efault);
 		break;
 #endif
 	}
-	if (!err)
-		*dest = x;
-	else
+	*dest = x;
+	return 0;
+
+Efault:
+	regs->dar = ea;
+	return -EFAULT;
+}
+
+static nokprobe_inline int
+read_mem_aligned(unsigned long *dest, unsigned long ea, int nb, struct pt_regs *regs)
+{
+	int err;
+
+	if (is_kernel_addr(ea))
+		return __read_mem_aligned(dest, ea, nb, regs);
+
+	if (user_read_access_begin((void __user *)ea, nb)) {
+		err = __read_mem_aligned(dest, ea, nb, regs);
+		user_read_access_end();
+	} else {
+		err = -EFAULT;
 		regs->dar = ea;
+	}
+
 	return err;
 }
 
@@ -336,10 +354,8 @@ static nokprobe_inline int read_mem_aligned(unsigned long *dest,
  * Copy from userspace to a buffer, using the largest possible
  * aligned accesses, up to sizeof(long).
  */
-static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb,
-				       struct pt_regs *regs)
+static __always_inline int __copy_mem_in(u8 *dest, unsigned long ea, int nb, struct pt_regs *regs)
 {
-	int err = 0;
 	int c;
 
 	for (; nb > 0; nb -= c) {
@@ -348,31 +364,46 @@ static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb,
 			c = max_align(nb);
 		switch (c) {
 		case 1:
-			err = __get_user(*dest, (unsigned char __user *) ea);
+			unsafe_get_user(*dest, (u8 __user *)ea, Efault);
 			break;
 		case 2:
-			err = __get_user(*(u16 *)dest,
-					 (unsigned short __user *) ea);
+			unsafe_get_user(*(u16 *)dest, (u16 __user *)ea, Efault);
 			break;
 		case 4:
-			err = __get_user(*(u32 *)dest,
-					 (unsigned int __user *) ea);
+			unsafe_get_user(*(u32 *)dest, (u32 __user *)ea, Efault);
 			break;
 #ifdef __powerpc64__
 		case 8:
-			err = __get_user(*(unsigned long *)dest,
-					 (unsigned long __user *) ea);
+			unsafe_get_user(*(u64 *)dest, (u64 __user *)ea, Efault);
 			break;
 #endif
 		}
-		if (err) {
-			regs->dar = ea;
-			return err;
-		}
 		dest += c;
 		ea += c;
 	}
 	return 0;
+
+Efault:
+	regs->dar = ea;
+	return -EFAULT;
+}
+
+static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb, struct pt_regs *regs)
+{
+	int err;
+
+	if (is_kernel_addr(ea))
+		return __copy_mem_in(dest, ea, nb, regs);
+
+	if (user_read_access_begin((void __user *)ea, nb)) {
+		err = __copy_mem_in(dest, ea, nb, regs);
+		user_read_access_end();
+	} else {
+		err = -EFAULT;
+		regs->dar = ea;
+	}
+
+	return err;
 }
 
 static nokprobe_inline int read_mem_unaligned(unsigned long *dest,
@@ -410,30 +441,48 @@ static int read_mem(unsigned long *dest, unsigned long ea, int nb,
 }
 NOKPROBE_SYMBOL(read_mem);
 
-static nokprobe_inline int write_mem_aligned(unsigned long val,
-					     unsigned long ea, int nb,
-					     struct pt_regs *regs)
+static __always_inline int
+__write_mem_aligned(unsigned long val, unsigned long ea, int nb, struct pt_regs *regs)
 {
-	int err = 0;
-
 	switch (nb) {
 	case 1:
-		err = __put_user(val, (unsigned char __user *) ea);
+		unsafe_put_user(val, (unsigned char __user *)ea, Efault);
 		break;
 	case 2:
-		err = __put_user(val, (unsigned short __user *) ea);
+		unsafe_put_user(val, (unsigned short __user *)ea, Efault);
 		break;
 	case 4:
-		err = __put_user(val, (unsigned int __user *) ea);
+		unsafe_put_user(val, (unsigned int __user *)ea, Efault);
 		break;
 #ifdef __powerpc64__
 	case 8:
-		err = __put_user(val, (unsigned long __user *) ea);
+		unsafe_put_user(val, (unsigned long __user *)ea, Efault);
 		break;
 #endif
 	}
-	if (err)
+	return 0;
+
+Efault:
+	regs->dar = ea;
+	return -EFAULT;
+}
+
+static nokprobe_inline int
+write_mem_aligned(unsigned long val, unsigned long ea, int nb, struct pt_regs *regs)
+{
+	int err;
+
+	if (is_kernel_addr(ea))
+		return __write_mem_aligned(val, ea, nb, regs);
+
+	if (user_write_access_begin((void __user *)ea, nb)) {
+		err = __write_mem_aligned(val, ea, nb, regs);
+		user_write_access_end();
+	} else {
+		err = -EFAULT;
 		regs->dar = ea;
+	}
+
 	return err;
 }
 
@@ -441,10 +490,8 @@ static nokprobe_inline int write_mem_aligned(unsigned long val,
  * Copy from a buffer to userspace, using the largest possible
  * aligned accesses, up to sizeof(long).
  */
-static nokprobe_inline int copy_mem_out(u8 *dest, unsigned long ea, int nb,
-					struct pt_regs *regs)
+static nokprobe_inline int __copy_mem_out(u8 *dest, unsigned long ea, int nb, struct pt_regs *regs)
 {
-	int err = 0;
 	int c;
 
 	for (; nb > 0; nb -= c) {
@@ -453,31 +500,46 @@ static nokprobe_inline int copy_mem_out(u8 *dest, unsigned long ea, int nb,
 			c = max_align(nb);
 		switch (c) {
 		case 1:
-			err = __put_user(*dest, (unsigned char __user *) ea);
+			unsafe_put_user(*dest, (u8 __user *)ea, Efault);
 			break;
 		case 2:
-			err = __put_user(*(u16 *)dest,
-					 (unsigned short __user *) ea);
+			unsafe_put_user(*(u16 *)dest, (u16 __user *)ea, Efault);
 			break;
 		case 4:
-			err = __put_user(*(u32 *)dest,
-					 (unsigned int __user *) ea);
+			unsafe_put_user(*(u32 *)dest, (u32 __user *)ea, Efault);
 			break;
 #ifdef __powerpc64__
 		case 8:
-			err = __put_user(*(unsigned long *)dest,
-					 (unsigned long __user *) ea);
+			unsafe_put_user(*(u64 *)dest, (u64 __user *)ea, Efault);
 			break;
 #endif
 		}
-		if (err) {
-			regs->dar = ea;
-			return err;
-		}
 		dest += c;
 		ea += c;
 	}
 	return 0;
+
+Efault:
+	regs->dar = ea;
+	return -EFAULT;
+}
+
+static nokprobe_inline int copy_mem_out(u8 *dest, unsigned long ea, int nb, struct pt_regs *regs)
+{
+	int err;
+
+	if (is_kernel_addr(ea))
+		return __copy_mem_out(dest, ea, nb, regs);
+
+	if (user_write_access_begin((void __user *)ea, nb)) {
+		err = __copy_mem_out(dest, ea, nb, regs);
+		user_write_access_end();
+	} else {
+		err = -EFAULT;
+		regs->dar = ea;
+	}
+
+	return err;
 }
 
 static nokprobe_inline int write_mem_unaligned(unsigned long val,
@@ -986,10 +1048,24 @@ static nokprobe_inline int do_vsx_store(struct instruction_op *op,
 }
 #endif /* CONFIG_VSX */
 
+static int __emulate_dcbz(unsigned long ea)
+{
+	unsigned long i;
+	unsigned long size = l1_dcache_bytes();
+
+	for (i = 0; i < size; i += sizeof(long))
+		unsafe_put_user(0, (unsigned long __user *)(ea + i), Efault);
+
+	return 0;
+
+Efault:
+	return -EFAULT;
+}
+
 int emulate_dcbz(unsigned long ea, struct pt_regs *regs)
 {
 	int err;
-	unsigned long i, size;
+	unsigned long size;
 
 #ifdef __powerpc64__
 	size = ppc64_caches.l1d.block_size;
@@ -1001,14 +1077,21 @@ int emulate_dcbz(unsigned long ea, struct pt_regs *regs)
 	ea &= ~(size - 1);
 	if (!address_ok(regs, ea, size))
 		return -EFAULT;
-	for (i = 0; i < size; i += sizeof(long)) {
-		err = __put_user(0, (unsigned long __user *) (ea + i));
-		if (err) {
-			regs->dar = ea;
-			return err;
-		}
+
+	if (is_kernel_addr(ea)) {
+		err = __emulate_dcbz(ea);
+	} else if (user_write_access_begin((void __user *)ea, size)) {
+		err = __emulate_dcbz(ea);
+		user_write_access_end();
+	} else {
+		err = -EFAULT;
 	}
-	return 0;
+
+	if (err)
+		regs->dar = ea;
+
+
+	return err;
 }
 NOKPROBE_SYMBOL(emulate_dcbz);
 
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: kernel test robot @ 2021-09-16 23:56 UTC (permalink / raw)
  To: Paul Menzel, Nathan Chancellor, Nick Desaulniers
  Cc: Paul Menzel, kbuild-all, llvm, Linux Memory Management List,
	Zhen Lei, Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <20210916142210.26722-1-pmenzel@molgen.mpg.de>

[-- Attachment #1: Type: text/plain, Size: 15370 bytes --]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git ff1ffd71d5f0612cf194f5705c671d6b64bf5f91
config: hexagon-randconfig-r045-20210916 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/127eebd64d8e291cf75563499f6c886bd54a99d8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
        git checkout 127eebd64d8e291cf75563499f6c886bd54a99d8
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash lib/zlib_inflate/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> lib/zlib_inflate/inffast.c:257:18: error: use of undeclared identifier 'CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS'
                               *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
                                         ^
   1 error generated.


vim +/CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +257 lib/zlib_inflate/inffast.c

    29	
    30	/*
    31	   Decode literal, length, and distance codes and write out the resulting
    32	   literal and match bytes until either not enough input or output is
    33	   available, an end-of-block is encountered, or a data error is encountered.
    34	   When large enough input and output buffers are supplied to inflate(), for
    35	   example, a 16K input buffer and a 64K output buffer, more than 95% of the
    36	   inflate execution time is spent in this routine.
    37	
    38	   Entry assumptions:
    39	
    40	        state->mode == LEN
    41	        strm->avail_in >= 6
    42	        strm->avail_out >= 258
    43	        start >= strm->avail_out
    44	        state->bits < 8
    45	
    46	   On return, state->mode is one of:
    47	
    48	        LEN -- ran out of enough output space or enough available input
    49	        TYPE -- reached end of block code, inflate() to interpret next block
    50	        BAD -- error in block data
    51	
    52	   Notes:
    53	
    54	    - The maximum input bits used by a length/distance pair is 15 bits for the
    55	      length code, 5 bits for the length extra, 15 bits for the distance code,
    56	      and 13 bits for the distance extra.  This totals 48 bits, or six bytes.
    57	      Therefore if strm->avail_in >= 6, then there is enough input to avoid
    58	      checking for available input while decoding.
    59	
    60	    - The maximum bytes that a single length/distance pair can output is 258
    61	      bytes, which is the maximum length that can be coded.  inflate_fast()
    62	      requires strm->avail_out >= 258 for each loop to avoid checking for
    63	      output space.
    64	
    65	    - @start:	inflate()'s starting value for strm->avail_out
    66	 */
    67	void inflate_fast(z_streamp strm, unsigned start)
    68	{
    69	    struct inflate_state *state;
    70	    const unsigned char *in;    /* local strm->next_in */
    71	    const unsigned char *last;  /* while in < last, enough input available */
    72	    unsigned char *out;         /* local strm->next_out */
    73	    unsigned char *beg;         /* inflate()'s initial strm->next_out */
    74	    unsigned char *end;         /* while out < end, enough space available */
    75	#ifdef INFLATE_STRICT
    76	    unsigned dmax;              /* maximum distance from zlib header */
    77	#endif
    78	    unsigned wsize;             /* window size or zero if not using window */
    79	    unsigned whave;             /* valid bytes in the window */
    80	    unsigned write;             /* window write index */
    81	    unsigned char *window;      /* allocated sliding window, if wsize != 0 */
    82	    unsigned long hold;         /* local strm->hold */
    83	    unsigned bits;              /* local strm->bits */
    84	    code const *lcode;          /* local strm->lencode */
    85	    code const *dcode;          /* local strm->distcode */
    86	    unsigned lmask;             /* mask for first level of length codes */
    87	    unsigned dmask;             /* mask for first level of distance codes */
    88	    code this;                  /* retrieved table entry */
    89	    unsigned op;                /* code bits, operation, extra bits, or */
    90	                                /*  window position, window bytes to copy */
    91	    unsigned len;               /* match length, unused bytes */
    92	    unsigned dist;              /* match distance */
    93	    unsigned char *from;        /* where to copy match from */
    94	
    95	    /* copy state to local variables */
    96	    state = (struct inflate_state *)strm->state;
    97	    in = strm->next_in;
    98	    last = in + (strm->avail_in - 5);
    99	    out = strm->next_out;
   100	    beg = out - (start - strm->avail_out);
   101	    end = out + (strm->avail_out - 257);
   102	#ifdef INFLATE_STRICT
   103	    dmax = state->dmax;
   104	#endif
   105	    wsize = state->wsize;
   106	    whave = state->whave;
   107	    write = state->write;
   108	    window = state->window;
   109	    hold = state->hold;
   110	    bits = state->bits;
   111	    lcode = state->lencode;
   112	    dcode = state->distcode;
   113	    lmask = (1U << state->lenbits) - 1;
   114	    dmask = (1U << state->distbits) - 1;
   115	
   116	    /* decode literals and length/distances until end-of-block or not enough
   117	       input data or output space */
   118	    do {
   119	        if (bits < 15) {
   120	            hold += (unsigned long)(*in++) << bits;
   121	            bits += 8;
   122	            hold += (unsigned long)(*in++) << bits;
   123	            bits += 8;
   124	        }
   125	        this = lcode[hold & lmask];
   126	      dolen:
   127	        op = (unsigned)(this.bits);
   128	        hold >>= op;
   129	        bits -= op;
   130	        op = (unsigned)(this.op);
   131	        if (op == 0) {                          /* literal */
   132	            *out++ = (unsigned char)(this.val);
   133	        }
   134	        else if (op & 16) {                     /* length base */
   135	            len = (unsigned)(this.val);
   136	            op &= 15;                           /* number of extra bits */
   137	            if (op) {
   138	                if (bits < op) {
   139	                    hold += (unsigned long)(*in++) << bits;
   140	                    bits += 8;
   141	                }
   142	                len += (unsigned)hold & ((1U << op) - 1);
   143	                hold >>= op;
   144	                bits -= op;
   145	            }
   146	            if (bits < 15) {
   147	                hold += (unsigned long)(*in++) << bits;
   148	                bits += 8;
   149	                hold += (unsigned long)(*in++) << bits;
   150	                bits += 8;
   151	            }
   152	            this = dcode[hold & dmask];
   153	          dodist:
   154	            op = (unsigned)(this.bits);
   155	            hold >>= op;
   156	            bits -= op;
   157	            op = (unsigned)(this.op);
   158	            if (op & 16) {                      /* distance base */
   159	                dist = (unsigned)(this.val);
   160	                op &= 15;                       /* number of extra bits */
   161	                if (bits < op) {
   162	                    hold += (unsigned long)(*in++) << bits;
   163	                    bits += 8;
   164	                    if (bits < op) {
   165	                        hold += (unsigned long)(*in++) << bits;
   166	                        bits += 8;
   167	                    }
   168	                }
   169	                dist += (unsigned)hold & ((1U << op) - 1);
   170	#ifdef INFLATE_STRICT
   171	                if (dist > dmax) {
   172	                    strm->msg = (char *)"invalid distance too far back";
   173	                    state->mode = BAD;
   174	                    break;
   175	                }
   176	#endif
   177	                hold >>= op;
   178	                bits -= op;
   179	                op = (unsigned)(out - beg);     /* max distance in output */
   180	                if (dist > op) {                /* see if copy from window */
   181	                    op = dist - op;             /* distance back in window */
   182	                    if (op > whave) {
   183	                        strm->msg = (char *)"invalid distance too far back";
   184	                        state->mode = BAD;
   185	                        break;
   186	                    }
   187	                    from = window;
   188	                    if (write == 0) {           /* very common case */
   189	                        from += wsize - op;
   190	                        if (op < len) {         /* some from window */
   191	                            len -= op;
   192	                            do {
   193	                                *out++ = *from++;
   194	                            } while (--op);
   195	                            from = out - dist;  /* rest from output */
   196	                        }
   197	                    }
   198	                    else if (write < op) {      /* wrap around window */
   199	                        from += wsize + write - op;
   200	                        op -= write;
   201	                        if (op < len) {         /* some from end of window */
   202	                            len -= op;
   203	                            do {
   204	                                *out++ = *from++;
   205	                            } while (--op);
   206	                            from = window;
   207	                            if (write < len) {  /* some from start of window */
   208	                                op = write;
   209	                                len -= op;
   210	                                do {
   211	                                    *out++ = *from++;
   212	                                } while (--op);
   213	                                from = out - dist;      /* rest from output */
   214	                            }
   215	                        }
   216	                    }
   217	                    else {                      /* contiguous in window */
   218	                        from += write - op;
   219	                        if (op < len) {         /* some from window */
   220	                            len -= op;
   221	                            do {
   222	                                *out++ = *from++;
   223	                            } while (--op);
   224	                            from = out - dist;  /* rest from output */
   225	                        }
   226	                    }
   227	                    while (len > 2) {
   228	                        *out++ = *from++;
   229	                        *out++ = *from++;
   230	                        *out++ = *from++;
   231	                        len -= 3;
   232	                    }
   233	                    if (len) {
   234	                        *out++ = *from++;
   235	                        if (len > 1)
   236	                            *out++ = *from++;
   237	                    }
   238	                }
   239	                else {
   240			    unsigned short *sout;
   241			    unsigned long loops;
   242	
   243	                    from = out - dist;          /* copy direct from output */
   244			    /* minimum length is three */
   245			    /* Align out addr */
   246			    if (!((long)(out - 1) & 1)) {
   247				*out++ = *from++;
   248				len--;
   249			    }
   250			    sout = (unsigned short *)(out);
   251			    if (dist > 2) {
   252				unsigned short *sfrom;
   253	
   254				sfrom = (unsigned short *)(from);
   255				loops = len >> 1;
   256				do
 > 257				    *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
   258				while (--loops);
   259				out = (unsigned char *)sout;
   260				from = (unsigned char *)sfrom;
   261			    } else { /* dist == 1 or dist == 2 */
   262				unsigned short pat16;
   263	
   264				pat16 = *(sout-1);
   265				if (dist == 1) {
   266					union uu mm;
   267					/* copy one char pattern to both bytes */
   268					mm.us = pat16;
   269					mm.b[0] = mm.b[1];
   270					pat16 = mm.us;
   271				}
   272				loops = len >> 1;
   273				do
   274				    *sout++ = pat16;
   275				while (--loops);
   276				out = (unsigned char *)sout;
   277			    }
   278			    if (len & 1)
   279				*out++ = *from++;
   280	                }
   281	            }
   282	            else if ((op & 64) == 0) {          /* 2nd level distance code */
   283	                this = dcode[this.val + (hold & ((1U << op) - 1))];
   284	                goto dodist;
   285	            }
   286	            else {
   287	                strm->msg = (char *)"invalid distance code";
   288	                state->mode = BAD;
   289	                break;
   290	            }
   291	        }
   292	        else if ((op & 64) == 0) {              /* 2nd level length code */
   293	            this = lcode[this.val + (hold & ((1U << op) - 1))];
   294	            goto dolen;
   295	        }
   296	        else if (op & 32) {                     /* end-of-block */
   297	            state->mode = TYPE;
   298	            break;
   299	        }
   300	        else {
   301	            strm->msg = (char *)"invalid literal/length code";
   302	            state->mode = BAD;
   303	            break;
   304	        }
   305	    } while (in < last && out < end);
   306	
   307	    /* return unused bytes (on entry, bits < 8, so in won't go too far back) */
   308	    len = bits >> 3;
   309	    in -= len;
   310	    bits -= len << 3;
   311	    hold &= (1U << bits) - 1;
   312	
   313	    /* update state and return */
   314	    strm->next_in = in;
   315	    strm->next_out = out;
   316	    strm->avail_in = (unsigned)(in < last ? 5 + (last - in) : 5 - (in - last));
   317	    strm->avail_out = (unsigned)(out < end ?
   318	                                 257 + (end - out) : 257 - (out - end));
   319	    state->hold = hold;
   320	    state->bits = bits;
   321	    return;
   322	}
   323	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28632 bytes --]

^ permalink raw reply

* Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: kernel test robot @ 2021-09-17  0:42 UTC (permalink / raw)
  To: Paul Menzel, Nathan Chancellor, Nick Desaulniers
  Cc: Paul Menzel, kbuild-all, Zhen Lei, Linux Memory Management List,
	Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <20210916142210.26722-1-pmenzel@molgen.mpg.de>

[-- Attachment #1: Type: text/plain, Size: 15490 bytes --]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.15-rc1 next-20210916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git ff1ffd71d5f0612cf194f5705c671d6b64bf5f91
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/127eebd64d8e291cf75563499f6c886bd54a99d8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Paul-Menzel/lib-zlib_inflate-inffast-Check-config-in-C-to-avoid-unused-function-warning/20210916-222359
        git checkout 127eebd64d8e291cf75563499f6c886bd54a99d8
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   lib/zlib_inflate/inffast.c: In function 'inflate_fast':
>> lib/zlib_inflate/inffast.c:257:39: error: 'CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS' undeclared (first use in this function)
     257 |                             *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
         |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   lib/zlib_inflate/inffast.c:257:39: note: each undeclared identifier is reported only once for each function it appears in


vim +/CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +257 lib/zlib_inflate/inffast.c

    29	
    30	/*
    31	   Decode literal, length, and distance codes and write out the resulting
    32	   literal and match bytes until either not enough input or output is
    33	   available, an end-of-block is encountered, or a data error is encountered.
    34	   When large enough input and output buffers are supplied to inflate(), for
    35	   example, a 16K input buffer and a 64K output buffer, more than 95% of the
    36	   inflate execution time is spent in this routine.
    37	
    38	   Entry assumptions:
    39	
    40	        state->mode == LEN
    41	        strm->avail_in >= 6
    42	        strm->avail_out >= 258
    43	        start >= strm->avail_out
    44	        state->bits < 8
    45	
    46	   On return, state->mode is one of:
    47	
    48	        LEN -- ran out of enough output space or enough available input
    49	        TYPE -- reached end of block code, inflate() to interpret next block
    50	        BAD -- error in block data
    51	
    52	   Notes:
    53	
    54	    - The maximum input bits used by a length/distance pair is 15 bits for the
    55	      length code, 5 bits for the length extra, 15 bits for the distance code,
    56	      and 13 bits for the distance extra.  This totals 48 bits, or six bytes.
    57	      Therefore if strm->avail_in >= 6, then there is enough input to avoid
    58	      checking for available input while decoding.
    59	
    60	    - The maximum bytes that a single length/distance pair can output is 258
    61	      bytes, which is the maximum length that can be coded.  inflate_fast()
    62	      requires strm->avail_out >= 258 for each loop to avoid checking for
    63	      output space.
    64	
    65	    - @start:	inflate()'s starting value for strm->avail_out
    66	 */
    67	void inflate_fast(z_streamp strm, unsigned start)
    68	{
    69	    struct inflate_state *state;
    70	    const unsigned char *in;    /* local strm->next_in */
    71	    const unsigned char *last;  /* while in < last, enough input available */
    72	    unsigned char *out;         /* local strm->next_out */
    73	    unsigned char *beg;         /* inflate()'s initial strm->next_out */
    74	    unsigned char *end;         /* while out < end, enough space available */
    75	#ifdef INFLATE_STRICT
    76	    unsigned dmax;              /* maximum distance from zlib header */
    77	#endif
    78	    unsigned wsize;             /* window size or zero if not using window */
    79	    unsigned whave;             /* valid bytes in the window */
    80	    unsigned write;             /* window write index */
    81	    unsigned char *window;      /* allocated sliding window, if wsize != 0 */
    82	    unsigned long hold;         /* local strm->hold */
    83	    unsigned bits;              /* local strm->bits */
    84	    code const *lcode;          /* local strm->lencode */
    85	    code const *dcode;          /* local strm->distcode */
    86	    unsigned lmask;             /* mask for first level of length codes */
    87	    unsigned dmask;             /* mask for first level of distance codes */
    88	    code this;                  /* retrieved table entry */
    89	    unsigned op;                /* code bits, operation, extra bits, or */
    90	                                /*  window position, window bytes to copy */
    91	    unsigned len;               /* match length, unused bytes */
    92	    unsigned dist;              /* match distance */
    93	    unsigned char *from;        /* where to copy match from */
    94	
    95	    /* copy state to local variables */
    96	    state = (struct inflate_state *)strm->state;
    97	    in = strm->next_in;
    98	    last = in + (strm->avail_in - 5);
    99	    out = strm->next_out;
   100	    beg = out - (start - strm->avail_out);
   101	    end = out + (strm->avail_out - 257);
   102	#ifdef INFLATE_STRICT
   103	    dmax = state->dmax;
   104	#endif
   105	    wsize = state->wsize;
   106	    whave = state->whave;
   107	    write = state->write;
   108	    window = state->window;
   109	    hold = state->hold;
   110	    bits = state->bits;
   111	    lcode = state->lencode;
   112	    dcode = state->distcode;
   113	    lmask = (1U << state->lenbits) - 1;
   114	    dmask = (1U << state->distbits) - 1;
   115	
   116	    /* decode literals and length/distances until end-of-block or not enough
   117	       input data or output space */
   118	    do {
   119	        if (bits < 15) {
   120	            hold += (unsigned long)(*in++) << bits;
   121	            bits += 8;
   122	            hold += (unsigned long)(*in++) << bits;
   123	            bits += 8;
   124	        }
   125	        this = lcode[hold & lmask];
   126	      dolen:
   127	        op = (unsigned)(this.bits);
   128	        hold >>= op;
   129	        bits -= op;
   130	        op = (unsigned)(this.op);
   131	        if (op == 0) {                          /* literal */
   132	            *out++ = (unsigned char)(this.val);
   133	        }
   134	        else if (op & 16) {                     /* length base */
   135	            len = (unsigned)(this.val);
   136	            op &= 15;                           /* number of extra bits */
   137	            if (op) {
   138	                if (bits < op) {
   139	                    hold += (unsigned long)(*in++) << bits;
   140	                    bits += 8;
   141	                }
   142	                len += (unsigned)hold & ((1U << op) - 1);
   143	                hold >>= op;
   144	                bits -= op;
   145	            }
   146	            if (bits < 15) {
   147	                hold += (unsigned long)(*in++) << bits;
   148	                bits += 8;
   149	                hold += (unsigned long)(*in++) << bits;
   150	                bits += 8;
   151	            }
   152	            this = dcode[hold & dmask];
   153	          dodist:
   154	            op = (unsigned)(this.bits);
   155	            hold >>= op;
   156	            bits -= op;
   157	            op = (unsigned)(this.op);
   158	            if (op & 16) {                      /* distance base */
   159	                dist = (unsigned)(this.val);
   160	                op &= 15;                       /* number of extra bits */
   161	                if (bits < op) {
   162	                    hold += (unsigned long)(*in++) << bits;
   163	                    bits += 8;
   164	                    if (bits < op) {
   165	                        hold += (unsigned long)(*in++) << bits;
   166	                        bits += 8;
   167	                    }
   168	                }
   169	                dist += (unsigned)hold & ((1U << op) - 1);
   170	#ifdef INFLATE_STRICT
   171	                if (dist > dmax) {
   172	                    strm->msg = (char *)"invalid distance too far back";
   173	                    state->mode = BAD;
   174	                    break;
   175	                }
   176	#endif
   177	                hold >>= op;
   178	                bits -= op;
   179	                op = (unsigned)(out - beg);     /* max distance in output */
   180	                if (dist > op) {                /* see if copy from window */
   181	                    op = dist - op;             /* distance back in window */
   182	                    if (op > whave) {
   183	                        strm->msg = (char *)"invalid distance too far back";
   184	                        state->mode = BAD;
   185	                        break;
   186	                    }
   187	                    from = window;
   188	                    if (write == 0) {           /* very common case */
   189	                        from += wsize - op;
   190	                        if (op < len) {         /* some from window */
   191	                            len -= op;
   192	                            do {
   193	                                *out++ = *from++;
   194	                            } while (--op);
   195	                            from = out - dist;  /* rest from output */
   196	                        }
   197	                    }
   198	                    else if (write < op) {      /* wrap around window */
   199	                        from += wsize + write - op;
   200	                        op -= write;
   201	                        if (op < len) {         /* some from end of window */
   202	                            len -= op;
   203	                            do {
   204	                                *out++ = *from++;
   205	                            } while (--op);
   206	                            from = window;
   207	                            if (write < len) {  /* some from start of window */
   208	                                op = write;
   209	                                len -= op;
   210	                                do {
   211	                                    *out++ = *from++;
   212	                                } while (--op);
   213	                                from = out - dist;      /* rest from output */
   214	                            }
   215	                        }
   216	                    }
   217	                    else {                      /* contiguous in window */
   218	                        from += write - op;
   219	                        if (op < len) {         /* some from window */
   220	                            len -= op;
   221	                            do {
   222	                                *out++ = *from++;
   223	                            } while (--op);
   224	                            from = out - dist;  /* rest from output */
   225	                        }
   226	                    }
   227	                    while (len > 2) {
   228	                        *out++ = *from++;
   229	                        *out++ = *from++;
   230	                        *out++ = *from++;
   231	                        len -= 3;
   232	                    }
   233	                    if (len) {
   234	                        *out++ = *from++;
   235	                        if (len > 1)
   236	                            *out++ = *from++;
   237	                    }
   238	                }
   239	                else {
   240			    unsigned short *sout;
   241			    unsigned long loops;
   242	
   243	                    from = out - dist;          /* copy direct from output */
   244			    /* minimum length is three */
   245			    /* Align out addr */
   246			    if (!((long)(out - 1) & 1)) {
   247				*out++ = *from++;
   248				len--;
   249			    }
   250			    sout = (unsigned short *)(out);
   251			    if (dist > 2) {
   252				unsigned short *sfrom;
   253	
   254				sfrom = (unsigned short *)(from);
   255				loops = len >> 1;
   256				do
 > 257				    *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);
   258				while (--loops);
   259				out = (unsigned char *)sout;
   260				from = (unsigned char *)sfrom;
   261			    } else { /* dist == 1 or dist == 2 */
   262				unsigned short pat16;
   263	
   264				pat16 = *(sout-1);
   265				if (dist == 1) {
   266					union uu mm;
   267					/* copy one char pattern to both bytes */
   268					mm.us = pat16;
   269					mm.b[0] = mm.b[1];
   270					pat16 = mm.us;
   271				}
   272				loops = len >> 1;
   273				do
   274				    *sout++ = pat16;
   275				while (--loops);
   276				out = (unsigned char *)sout;
   277			    }
   278			    if (len & 1)
   279				*out++ = *from++;
   280	                }
   281	            }
   282	            else if ((op & 64) == 0) {          /* 2nd level distance code */
   283	                this = dcode[this.val + (hold & ((1U << op) - 1))];
   284	                goto dodist;
   285	            }
   286	            else {
   287	                strm->msg = (char *)"invalid distance code";
   288	                state->mode = BAD;
   289	                break;
   290	            }
   291	        }
   292	        else if ((op & 64) == 0) {              /* 2nd level length code */
   293	            this = lcode[this.val + (hold & ((1U << op) - 1))];
   294	            goto dolen;
   295	        }
   296	        else if (op & 32) {                     /* end-of-block */
   297	            state->mode = TYPE;
   298	            break;
   299	        }
   300	        else {
   301	            strm->msg = (char *)"invalid literal/length code";
   302	            state->mode = BAD;
   303	            break;
   304	        }
   305	    } while (in < last && out < end);
   306	
   307	    /* return unused bytes (on entry, bits < 8, so in won't go too far back) */
   308	    len = bits >> 3;
   309	    in -= len;
   310	    bits -= len << 3;
   311	    hold &= (1U << bits) - 1;
   312	
   313	    /* update state and return */
   314	    strm->next_in = in;
   315	    strm->next_out = out;
   316	    strm->avail_in = (unsigned)(in < last ? 5 + (last - in) : 5 - (in - last));
   317	    strm->avail_out = (unsigned)(out < end ?
   318	                                 257 + (end - out) : 257 - (out - end));
   319	    state->hold = hold;
   320	    state->bits = bits;
   321	    return;
   322	}
   323	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 11023 bytes --]

^ permalink raw reply

* Re: [PATCH v5 6/6] sched/fair: Consider SMT in ASYM_PACKING load balance
From: Ricardo Neri @ 2021-09-17  1:00 UTC (permalink / raw)
  To: Vincent Guittot
  Cc: Juri Lelli, Aubrey Li, Srikar Dronamraju, Ravi V. Shankar,
	Peter Zijlstra (Intel), Ricardo Neri, Ben Segall,
	Srinivas Pandruvada, Joel Fernandes (Google), Ingo Molnar,
	Rafael J . Wysocki, Steven Rostedt, Mel Gorman, Len Brown,
	Nicholas Piggin, Aubrey Li, Dietmar Eggemann, Tim Chen,
	Quentin Perret, Daniel Bristot de Oliveira, linux-kernel,
	linuxppc-dev
In-Reply-To: <CAKfTPtBcDP3Yp54sd4+1kP=o=4e_1HEmOf=eMXydag_J38CEng@mail.gmail.com>

On Wed, Sep 15, 2021 at 05:43:44PM +0200, Vincent Guittot wrote:
> On Sat, 11 Sept 2021 at 03:19, Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> >
> > When deciding to pull tasks in ASYM_PACKING, it is necessary not only to
> > check for the idle state of the destination CPU, dst_cpu, but also of
> > its SMT siblings.
> >
> > If dst_cpu is idle but its SMT siblings are busy, performance suffers
> > if it pulls tasks from a medium priority CPU that does not have SMT
> > siblings.
> >
> > Implement asym_smt_can_pull_tasks() to inspect the state of the SMT
> > siblings of both dst_cpu and the CPUs in the candidate busiest group.
> >
> > Cc: Aubrey Li <aubrey.li@intel.com>
> > Cc: Ben Segall <bsegall@google.com>
> > Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Quentin Perret <qperret@google.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > Reviewed-by: Len Brown <len.brown@intel.com>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Changes since v4:
> >   * Use sg_lb_stats::sum_nr_running the idle state of a scheduling group.
> >     (Vincent, Peter)
> >   * Do not even idle CPUs in asym_smt_can_pull_tasks(). (Vincent)
> >   * Updated function documentation and corrected a typo.
> >
> > Changes since v3:
> >   * Removed the arch_asym_check_smt_siblings() hook. Discussions with the
> >     powerpc folks showed that this patch should not impact them. Also, more
> >     recent powerpc processor no longer use asym_packing. (PeterZ)
> >   * Removed unnecessary local variable in asym_can_pull_tasks(). (Dietmar)
> >   * Removed unnecessary check for local CPUs when the local group has zero
> >     utilization. (Joel)
> >   * Renamed asym_can_pull_tasks() as asym_smt_can_pull_tasks() to reflect
> >     the fact that it deals with SMT cases.
> >   * Made asym_smt_can_pull_tasks() return false for !CONFIG_SCHED_SMT so
> >     that callers can deal with non-SMT cases.
> >
> > Changes since v2:
> >   * Reworded the commit message to reflect updates in code.
> >   * Corrected misrepresentation of dst_cpu as the CPU doing the load
> >     balancing. (PeterZ)
> >   * Removed call to arch_asym_check_smt_siblings() as it is now called in
> >     sched_asym().
> >
> > Changes since v1:
> >   * Don't bailout in update_sd_pick_busiest() if dst_cpu cannot pull
> >     tasks. Instead, reclassify the candidate busiest group, as it
> >     may still be selected. (PeterZ)
> >   * Avoid an expensive and unnecessary call to cpumask_weight() when
> >     determining if a sched_group is comprised of SMT siblings.
> >     (PeterZ).
> > ---
> >  kernel/sched/fair.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 94 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 26db017c14a3..8d763dd0174b 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8597,10 +8597,98 @@ group_type group_classify(unsigned int imbalance_pct,
> >         return group_has_spare;
> >  }
> >
> > +/**
> > + * asym_smt_can_pull_tasks - Check whether the load balancing CPU can pull tasks
> > + * @dst_cpu:   Destination CPU of the load balancing
> > + * @sds:       Load-balancing data with statistics of the local group
> > + * @sgs:       Load-balancing statistics of the candidate busiest group
> > + * @sg:                The candidate busiest group
> > + *
> > + * Check the state of the SMT siblings of both @sds::local and @sg and decide
> > + * if @dst_cpu can pull tasks.
> > + *
> > + * If @dst_cpu does not have SMT siblings, it can pull tasks if two or more of
> > + * the SMT siblings of @sg are busy. If only one CPU in @sg is busy, pull tasks
> > + * only if @dst_cpu has higher priority.
> > + *
> > + * If both @dst_cpu and @sg have SMT siblings, and @sg has exactly one more
> > + * busy CPU than @sds::local, let @dst_cpu pull tasks if it has higher priority.
> > + * Bigger imbalances in the number of busy CPUs will be dealt with in
> > + * update_sd_pick_busiest().
> > + *
> > + * If @sg does not have SMT siblings, only pull tasks if all of the SMT siblings
> > + * of @dst_cpu are idle and @sg has lower priority.
> > + */
> > +static bool asym_smt_can_pull_tasks(int dst_cpu, struct sd_lb_stats *sds,
> > +                                   struct sg_lb_stats *sgs,
> > +                                   struct sched_group *sg)
> > +{
> > +#ifdef CONFIG_SCHED_SMT
> > +       bool local_is_smt, sg_is_smt;
> > +       int sg_busy_cpus;
> > +
> > +       local_is_smt = sds->local->flags & SD_SHARE_CPUCAPACITY;
> > +       sg_is_smt = sg->flags & SD_SHARE_CPUCAPACITY;
> > +
> > +       sg_busy_cpus = sgs->group_weight - sgs->idle_cpus;
> > +
> > +       if (!local_is_smt) {
> > +               /*
> > +                * If we are here, @dst_cpu is idle and does not have SMT
> > +                * siblings. Pull tasks if candidate group has two or more
> > +                * busy CPUs.
> > +                */
> > +               if (sg_is_smt && sg_busy_cpus >= 2)
> 
> Do you really need to test sg_is_smt ? if sg_busy_cpus >= 2 then
> sd_is_smt must be true ?

Thank you very much for your feedback Vincent!

Yes, it is true that sg_busy_cpus >=2 is only true if @sg is SMT. I will
remove this check.

> 
> Also, This is the default behavior where we want to even the number of
> busy cpu. Shouldn't you return false and fall back to the default
> behavior ?

This is also true.

> 
> That being said, the default behavior tries to even the number of idle
> cpus which is easier to compute and is equal to even the number of
> busy cpus in "normal" system with the same number of cpus in groups
> but this is not the case here. It could be good to change the default
> behavior to even the number of busy cpus and that you use the default
> behavior here. Additional condition will be used to select the busiest
> group like more busy cpu or more number of running tasks

That is a very good observation. Checking the number of idle CPUs
assumes that both groups have the same number of CPUs. I'll look into
modifying the default behavior.

> 
> > +                       return true;
> > +
> > +               /*
> > +                * @dst_cpu does not have SMT siblings. @sg may have SMT
> > +                * siblings and only one is busy. In such case, @dst_cpu
> > +                * can help if it has higher priority and is idle (i.e.,
> > +                * it has no running tasks).
> 
> The previous comment above assume that "@dst_cpu is idle" but now you
> need to check that sds->local_stat.sum_nr_running == 0

But we already know that, right? We are here because in
update_sg_lb_stats() we determine that dst CPU is idle (env->idle !=
CPU_NOT_IDLE).

Thanks and BR,
Ricardo

^ permalink raw reply

* Re: [PATCH] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: Christophe Leroy @ 2021-09-17  5:17 UTC (permalink / raw)
  To: Paul Menzel, Nathan Chancellor, Nick Desaulniers
  Cc: llvm, linux-kernel, Paul Mackerras, Zhen Lei, Andrew Morton,
	linuxppc-dev
In-Reply-To: <20210916142210.26722-1-pmenzel@molgen.mpg.de>



Le 16/09/2021 à 16:22, Paul Menzel a écrit :
> Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1
> shows the warning below.
> 
>      arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function]
>      get_unaligned16(const unsigned short *p)
>      ^
>      1 warning generated.
> 
> Fix it, by moving the check from the preprocessor to C, so the compiler
> sees the use.
> 
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>   lib/zlib_inflate/inffast.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
> index f19c4fbe1be7..444ad3c3ccd3 100644
> --- a/lib/zlib_inflate/inffast.c
> +++ b/lib/zlib_inflate/inffast.c
> @@ -254,11 +254,7 @@ void inflate_fast(z_streamp strm, unsigned start)
>   			sfrom = (unsigned short *)(from);
>   			loops = len >> 1;
>   			do
> -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> -			    *sout++ = *sfrom++;
> -#else
> -			    *sout++ = get_unaligned16(sfrom++);
> -#endif
> +			    *sout++ = CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ? *sfrom++ : get_unaligned16(sfrom++);

You can't do that, CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is not 
something that have value 0 or 1, it is something which is either 
defined or not. You have to use IS_ENABLED() macro, so it should become 
something like:

	if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
		*sout++ = *sfrom++;
	else
		*sout++ = get_unaligned16(sfrom++);


>   			while (--loops);
>   			out = (unsigned char *)sout;
>   			from = (unsigned char *)sfrom;
> 

^ permalink raw reply

* Re: [PATCH v2] powerpc/mce: Fix access error in mce handler
From: Daniel Axtens @ 2021-09-17  6:39 UTC (permalink / raw)
  To: Ganesh Goudar, linuxppc-dev, mpe; +Cc: Ganesh Goudar, mahesh, npiggin
In-Reply-To: <20210909064330.312432-1-ganeshgr@linux.ibm.com>

Hi Ganesh,

> We queue an irq work for deferred processing of mce event
> in realmode mce handler, where translation is disabled.
> Queuing of the work may result in accessing memory outside
> RMO region, such access needs the translation to be enabled
> for an LPAR running with hash mmu else the kernel crashes.
>
> After enabling translation in mce_handle_error() we used to
> leave it enabled to avoid crashing here, but now with the
> commit 74c3354bc1d89 ("powerpc/pseries/mce: restore msr before
> returning from handler") we are restoring the MSR to disable
> translation.
>
> Hence to fix this enable the translation before queuing the work.

[snip]

> Fixes: 74c3354bc1d89 ("powerpc/pseries/mce: restore msr before returning from handler")

That patch changes arch/powerpc/powerpc/platforms/pseries/ras.c just
below this comment:

    /*
     * Enable translation as we will be accessing per-cpu variables
     * in save_mce_event() which may fall outside RMO region, also
     * leave it enabled because subsequently we will be queuing work
     * to workqueues where again per-cpu variables accessed, besides
     * fwnmi_release_errinfo() crashes when called in realmode on
     * pseries.
     * Note: All the realmode handling like flushing SLB entries for
     *       SLB multihit is done by now.
     */

That suggests per-cpu variables need protection. In your patch, you
enable translations just around irq_work_queue:

> +	/* Queue irq work to process this event later. Before
> +	 * queuing the work enable translation for non radix LPAR,
> +	 * as irq_work_queue may try to access memory outside RMO
> +	 * region.
> +	 */
> +	if (!radix_enabled() && firmware_has_feature(FW_FEATURE_LPAR)) {
> +		msr = mfmsr();
> +		mtmsr(msr | MSR_IR | MSR_DR);
> +		irq_work_queue(&mce_event_process_work);
> +		mtmsr(msr);
> +	} else {
> +		irq_work_queue(&mce_event_process_work);
> +	}

However, just before that in the function, there are a few things that
access per-cpu variables via the local_paca, e.g.:

memcpy(&local_paca->mce_info->mce_event_queue[index],
       &evt, sizeof(evt));

Do we need to widen the window where translations are enabled in order
to protect accesses to local_paca?

Kind regards,
Daniel

^ permalink raw reply

* [PATCH v5 1/5] powerpc/signal64: Access function descriptor with user access block
From: Christophe Leroy @ 2021-09-17  6:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	ebiederm, hch
  Cc: linuxppc-dev, linux-kernel

Access the function descriptor of the handler within a
user access block.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v3: Flatten the change to avoid nested gotos.
---
 arch/powerpc/kernel/signal_64.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 1831bba0582e..7b1cd50bc4fb 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -936,8 +936,13 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 		func_descr_t __user *funct_desc_ptr =
 			(func_descr_t __user *) ksig->ka.sa.sa_handler;
 
-		err |= get_user(regs->ctr, &funct_desc_ptr->entry);
-		err |= get_user(regs->gpr[2], &funct_desc_ptr->toc);
+		if (!user_read_access_begin(funct_desc_ptr, sizeof(func_descr_t)))
+			goto badfunc;
+
+		unsafe_get_user(regs->ctr, &funct_desc_ptr->entry, badfunc_block);
+		unsafe_get_user(regs->gpr[2], &funct_desc_ptr->toc, badfunc_block);
+
+		user_read_access_end();
 	}
 
 	/* enter the signal handler in native-endian mode */
@@ -962,5 +967,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 badframe:
 	signal_fault(current, regs, "handle_rt_signal64", frame);
 
+	return 1;
+
+badfunc_block:
+	user_read_access_end();
+badfunc:
+	signal_fault(current, regs, __func__, (void __user *)ksig->ka.sa.sa_handler);
+
 	return 1;
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 4/5] powerpc/uaccess: Add unsafe_clear_user()
From: Christophe Leroy @ 2021-09-17  6:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	ebiederm, hch
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631861883.git.christophe.leroy@csgroup.eu>

Implement unsafe_clear_user() for powerpc.
It's a copy/paste of unsafe_copy_to_user() with value 0 as source.

It may be improved in a later patch by using 'dcbz' instruction
to zeroize full cache lines at once.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/uaccess.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 22c79ab40006..962b675485ff 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -467,6 +467,26 @@ do {									\
 		unsafe_put_user(*(u8*)(_src + _i), (u8 __user *)(_dst + _i), e); \
 } while (0)
 
+#define unsafe_clear_user(d, l, e)					\
+do {									\
+	u8 __user *_dst = (u8 __user *)(d);				\
+	size_t _len = (l);						\
+	int _i;								\
+									\
+	for (_i = 0; _i < (_len & ~(sizeof(u64) - 1)); _i += sizeof(u64)) \
+		unsafe_put_user(0, (u64 __user *)(_dst + _i), e);	\
+	if (_len & 4) {							\
+		unsafe_put_user(0, (u32 __user *)(_dst + _i), e);	\
+		_i += 4;						\
+	}								\
+	if (_len & 2) {							\
+		unsafe_put_user(0, (u16 __user *)(_dst + _i), e);	\
+		_i += 2;						\
+	}								\
+	if (_len & 1)							\
+		unsafe_put_user(0, (u8 __user *)(_dst + _i), e);	\
+} while (0)
+
 #define HAVE_GET_KERNEL_NOFAULT
 
 #define __get_kernel_nofault(dst, src, type, err_label)			\
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 2/5] powerpc/signal: Include the new stack frame inside the user access block
From: Christophe Leroy @ 2021-09-17  6:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	ebiederm, hch
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1718f38859d5366f82d5bef531f255cedf537b5d.1631861883.git.christophe.leroy@csgroup.eu>

Include the new stack frame inside the user access block and set it up
using unsafe_put_user().

On an mpc 8321 (book3s/32) the improvment is about 4% on a process
sending a signal to itself.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal_32.c | 29 +++++++++++++----------------
 arch/powerpc/kernel/signal_64.c | 14 +++++++-------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 0608581967f0..ff101e2b3bab 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -726,7 +726,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	struct rt_sigframe __user *frame;
 	struct mcontext __user *mctx;
 	struct mcontext __user *tm_mctx = NULL;
-	unsigned long newsp = 0;
+	unsigned long __user *newsp;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
 	/* Save the thread's msr before get_tm_stackpointer() changes it */
@@ -734,6 +734,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 	/* Set up Signal Frame */
 	frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+	newsp = (unsigned long __user *)((unsigned long)frame - (__SIGNAL_FRAMESIZE + 16));
 	mctx = &frame->uc.uc_mcontext;
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_mctx = &frame->uc_transact.uc_mcontext;
@@ -743,7 +744,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	else
 		prepare_save_user_regs(1);
 
-	if (!user_access_begin(frame, sizeof(*frame)))
+	if (!user_access_begin(newsp, __SIGNAL_FRAMESIZE + 16 + sizeof(*frame)))
 		goto badframe;
 
 	/* Put the siginfo & fill in most of the ucontext */
@@ -779,6 +780,9 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	}
 	unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
 
+	/* create a stack frame for the caller of the handler */
+	unsafe_put_user(regs->gpr[1], newsp, failed);
+
 	user_access_end();
 
 	if (copy_siginfo_to_user(&frame->info, &ksig->info))
@@ -790,13 +794,8 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	tsk->thread.fp_state.fpscr = 0;	/* turn off all fp exceptions */
 #endif
 
-	/* create a stack frame for the caller of the handler */
-	newsp = ((unsigned long)frame) - (__SIGNAL_FRAMESIZE + 16);
-	if (put_user(regs->gpr[1], (u32 __user *)newsp))
-		goto badframe;
-
 	/* Fill registers for signal handler */
-	regs->gpr[1] = newsp;
+	regs->gpr[1] = (unsigned long)newsp;
 	regs->gpr[3] = ksig->sig;
 	regs->gpr[4] = (unsigned long)&frame->info;
 	regs->gpr[5] = (unsigned long)&frame->uc;
@@ -826,7 +825,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 	struct sigframe __user *frame;
 	struct mcontext __user *mctx;
 	struct mcontext __user *tm_mctx = NULL;
-	unsigned long newsp = 0;
+	unsigned long __user *newsp;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
 	/* Save the thread's msr before get_tm_stackpointer() changes it */
@@ -834,6 +833,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 	/* Set up Signal Frame */
 	frame = get_sigframe(ksig, tsk, sizeof(*frame), 1);
+	newsp = (unsigned long __user *)((unsigned long)frame - __SIGNAL_FRAMESIZE);
 	mctx = &frame->mctx;
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_mctx = &frame->mctx_transact;
@@ -843,7 +843,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 	else
 		prepare_save_user_regs(1);
 
-	if (!user_access_begin(frame, sizeof(*frame)))
+	if (!user_access_begin(newsp, __SIGNAL_FRAMESIZE + sizeof(*frame)))
 		goto badframe;
 	sc = (struct sigcontext __user *) &frame->sctx;
 
@@ -873,6 +873,8 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 		unsafe_put_user(PPC_RAW_SC(), &mctx->mc_pad[1], failed);
 		asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
 	}
+	/* create a stack frame for the caller of the handler */
+	unsafe_put_user(regs->gpr[1], newsp, failed);
 	user_access_end();
 
 	regs->link = tramp;
@@ -881,12 +883,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 	tsk->thread.fp_state.fpscr = 0;	/* turn off all fp exceptions */
 #endif
 
-	/* create a stack frame for the caller of the handler */
-	newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
-	if (put_user(regs->gpr[1], (u32 __user *)newsp))
-		goto badframe;
-
-	regs->gpr[1] = newsp;
+	regs->gpr[1] = (unsigned long)newsp;
 	regs->gpr[3] = ksig->sig;
 	regs->gpr[4] = (unsigned long) sc;
 	regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler);
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 7b1cd50bc4fb..d80ff83cacb9 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -847,13 +847,14 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 		struct task_struct *tsk)
 {
 	struct rt_sigframe __user *frame;
-	unsigned long newsp = 0;
+	unsigned long __user *newsp;
 	long err = 0;
 	struct pt_regs *regs = tsk->thread.regs;
 	/* Save the thread's msr before get_tm_stackpointer() changes it */
 	unsigned long msr = regs->msr;
 
 	frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
+	newsp = (unsigned long __user *)((unsigned long)frame - __SIGNAL_FRAMESIZE);
 
 	/*
 	 * This only applies when calling unsafe_setup_sigcontext() and must be
@@ -862,7 +863,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	if (!MSR_TM_ACTIVE(msr))
 		prepare_setup_sigcontext(tsk);
 
-	if (!user_write_access_begin(frame, sizeof(*frame)))
+	if (!user_write_access_begin(newsp, __SIGNAL_FRAMESIZE + sizeof(*frame)))
 		goto badframe;
 
 	unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
@@ -900,6 +901,9 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	}
 
 	unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
+	/* Allocate a dummy caller frame for the signal handler. */
+	unsafe_put_user(regs->gpr[1], newsp, badframe_block);
+
 	user_write_access_end();
 
 	/* Save the siginfo outside of the unsafe block. */
@@ -919,10 +923,6 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 		regs_set_return_ip(regs, (unsigned long) &frame->tramp[0]);
 	}
 
-	/* Allocate a dummy caller frame for the signal handler. */
-	newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
-	err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
-
 	/* Set up "regs" so we "return" to the signal handler. */
 	if (is_elf2_task()) {
 		regs->ctr = (unsigned long) ksig->ka.sa.sa_handler;
@@ -947,7 +947,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 
 	/* enter the signal handler in native-endian mode */
 	regs_set_return_msr(regs, (regs->msr & ~MSR_LE) | (MSR_KERNEL & MSR_LE));
-	regs->gpr[1] = newsp;
+	regs->gpr[1] = (unsigned long)newsp;
 	regs->gpr[3] = ksig->sig;
 	regs->result = 0;
 	if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
-- 
2.31.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox