LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC PATCH 6/8] powerpc/mm/book3s64/hash: drop pre 2.06 tlbiel for clang
From: Nicholas Piggin @ 2021-03-19  2:01 UTC (permalink / raw)
  To: Daniel Axtens, linuxppc-dev, llvmlinux
In-Reply-To: <20210225031006.1204774-7-dja@axtens.net>

Excerpts from Daniel Axtens's message of February 25, 2021 1:10 pm:
> The llvm integrated assembler does not recognise the ISA 2.05 tlbiel
> version. Eventually do this more smartly.

The whole thing with TLBIE and TLBIEL in this file seems a bit too 
clever. We should have PPC_TLBIE* macros for all of them.

Thanks,
Nick

> 
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  arch/powerpc/mm/book3s64/hash_native.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
> index 52e170bd95ae..c5937f69a452 100644
> --- a/arch/powerpc/mm/book3s64/hash_native.c
> +++ b/arch/powerpc/mm/book3s64/hash_native.c
> @@ -267,9 +267,14 @@ static inline void __tlbiel(unsigned long vpn, int psize, int apsize, int ssize)
>  		va |= ssize << 8;
>  		sllp = get_sllp_encoding(apsize);
>  		va |= sllp << 5;
> +#if 0
>  		asm volatile(ASM_FTR_IFSET("tlbiel %0", "tlbiel %0,0", %1)
>  			     : : "r" (va), "i" (CPU_FTR_ARCH_206)
>  			     : "memory");
> +#endif
> +		asm volatile("tlbiel %0"
> +			     : : "r" (va)
> +			     : "memory");
>  		break;
>  	default:
>  		/* We need 14 to 14 + i bits of va */
> @@ -286,9 +291,14 @@ static inline void __tlbiel(unsigned long vpn, int psize, int apsize, int ssize)
>  		 */
>  		va |= (vpn & 0xfe);
>  		va |= 1; /* L */
> +#if 0
>  		asm volatile(ASM_FTR_IFSET("tlbiel %0", "tlbiel %0,1", %1)
>  			     : : "r" (va), "i" (CPU_FTR_ARCH_206)
>  			     : "memory");
> +#endif
> +		asm volatile("tlbiel %0"
> +			     : : "r" (va)
> +			     : "memory");
>  		break;
>  	}
>  	trace_tlbie(0, 1, va, 0, 0, 0, 0);
> -- 
> 2.27.0
> 
> 

^ permalink raw reply

* Re: [RFC PATCH 7/8] powerpc/purgatory: drop .machine specifier
From: Nicholas Piggin @ 2021-03-19  2:05 UTC (permalink / raw)
  To: Daniel Axtens, Segher Boessenkool; +Cc: llvmlinux, linuxppc-dev
In-Reply-To: <20210225155836.GG28121@gate.crashing.org>

Excerpts from Segher Boessenkool's message of February 26, 2021 1:58 am:
> On Thu, Feb 25, 2021 at 02:10:05PM +1100, Daniel Axtens wrote:
>> It's ignored by future versions of llvm's integrated assembler (by not -11).
>> I'm not sure what it does for us in gas.
> 
> It enables all insns that exist on 620 (the first 64-bit PowerPC CPU).

Same question for this, why do we have it at all?

Thanks,
Nick

^ permalink raw reply

* Re: [RFC PATCH 8/8] powerpc/64/asm: don't reassign labels
From: Nicholas Piggin @ 2021-03-19  2:15 UTC (permalink / raw)
  To: Daniel Axtens, Segher Boessenkool; +Cc: llvmlinux, linuxppc-dev
In-Reply-To: <87lfbboeo9.fsf@dja-thinkpad.axtens.net>

Excerpts from Daniel Axtens's message of February 26, 2021 10:28 am:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> 
>> On Thu, Feb 25, 2021 at 02:10:06PM +1100, Daniel Axtens wrote:
>>> The assembler really does not like us reassigning things to the same
>>> label:
>>> 
>>> <instantiation>:7:9: error: invalid reassignment of non-absolute variable 'fs_label'
>>> 
>>> This happens across a bunch of platforms:
>>> https://github.com/ClangBuiltLinux/linux/issues/1043
>>> https://github.com/ClangBuiltLinux/linux/issues/1008
>>> https://github.com/ClangBuiltLinux/linux/issues/920
>>> https://github.com/ClangBuiltLinux/linux/issues/1050
>>> 
>>> There is no hope of getting this fixed in LLVM, so if we want to build
>>> with LLVM_IAS, we need to hack around it ourselves.
>>> 
>>> For us the big problem comes from this:
>>> 
>>> \#define USE_FIXED_SECTION(sname)				\
>>> 	fs_label = start_##sname;				\
>>> 	fs_start = sname##_start;				\
>>> 	use_ftsec sname;
>>> 
>>> \#define USE_TEXT_SECTION()
>>> 	fs_label = start_text;					\
>>> 	fs_start = text_start;					\
>>> 	.text
>>> 
>>> and in particular fs_label.
>>
>> The "Setting Symbols" super short chapter reads:
>>
>> "A symbol can be given an arbitrary value by writing a symbol, followed
>> by an equals sign '=', followed by an expression.  This is equivalent
>> to using the '.set' directive."
>>
>> And ".set" has
>>
>> "Set the value of SYMBOL to EXPRESSION.  This changes SYMBOL's value and
>> type to conform to EXPRESSION.  If SYMBOL was flagged as external, it
>> remains flagged.
>>
>> You may '.set' a symbol many times in the same assembly provided that
>> the values given to the symbol are constants.  Values that are based on
>> expressions involving other symbols are allowed, but some targets may
>> restrict this to only being done once per assembly.  This is because
>> those targets do not set the addresses of symbols at assembly time, but
>> rather delay the assignment until a final link is performed.  This
>> allows the linker a chance to change the code in the files, changing the
>> location of, and the relative distance between, various different
>> symbols.
>>
>> If you '.set' a global symbol, the value stored in the object file is
>> the last value stored into it."
>>
>> So this really should be fixed in clang: it is basic assembler syntax.
> 
> No doubt I have explained this poorly.
> 
> LLVM does allow some things, this builds fine for example:
> 
> .set foo, 8192
> addi %r3, %r3, foo
> .set foo, 1234
> addi %r3, %r3, foo
> 
> However, this does not:
> 
> a:
> .set foo, a
> addi %r3, %r3, foo@l
> b:
> .set foo, b
> addi %r3, %r3, foo-a
> 
> clang -target ppc64le -integrated-as  foo.s -o foo.o -c
> foo.s:5:11: error: invalid reassignment of non-absolute variable 'foo' in '.set' directive
> .set foo, b
>           ^

So that does seem to be allowed by the specification.

I don't have a huge problem with the patch actually, doesn't seem too 
bad.

Thanks,
Nick

^ permalink raw reply

* [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation
From: Nicolas Boichat @ 2021-03-18 23:54 UTC (permalink / raw)
  To: stable
  Cc: Sasha Levin, linux-arch, Nicolas Boichat, Alexandre Chartre,
	Arnd Bergmann, linux-kbuild, Peter Zijlstra, Christopher Li,
	linux-kernel, Nicholas Piggin, Masahiro Yamada, linux-sparse,
	Michal Marek, Paul Mackerras, Greg Kroah-Hartman, Thomas Gleixner,
	linuxppc-dev, Naveen N. Rao, Daniel Axtens
In-Reply-To: <20210318235416.794798-1-drinkcat@chromium.org>

From: Thomas Gleixner <tglx@linutronix.de>

commit 6553896666433e7efec589838b400a2a652b3ffa upstream.

Some code pathes, especially the low level entry code, must be protected
against instrumentation for various reasons:

 - Low level entry code can be a fragile beast, especially on x86.

 - With NO_HZ_FULL RCU state needs to be established before using it.

Having a dedicated section for such code allows to validate with tooling
that no unsafe functions are invoked.

Add the .noinstr.text section and the noinstr attribute to mark
functions. noinstr implies notrace. Kprobes will gain a section check
later.

Provide also a set of markers: instrumentation_begin()/end()

These are used to mark code inside a noinstr function which calls
into regular instrumentable text section as safe.

The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
enabled as the end marker emits a NOP to prevent the compiler from merging
the annotation points. This means the objtool verification requires a
kernel compiled with this option.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de

[Nicolas: context conflicts in:
	arch/powerpc/kernel/vmlinux.lds.S
	include/asm-generic/vmlinux.lds.h
	include/linux/compiler.h
	include/linux/compiler_types.h]
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

---

 arch/powerpc/kernel/vmlinux.lds.S |  1 +
 include/asm-generic/sections.h    |  3 ++
 include/asm-generic/vmlinux.lds.h | 10 ++++++
 include/linux/compiler.h          | 54 +++++++++++++++++++++++++++++++
 include/linux/compiler_types.h    |  4 +++
 scripts/mod/modpost.c             |  2 +-
 6 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 695432965f20..9b346f3d2814 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -99,6 +99,7 @@ SECTIONS
 #endif
 		/* careful! __ftr_alt_* sections need to be close to .text */
 		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text);
+		NOINSTR_TEXT
 		SCHED_TEXT
 		CPUIDLE_TEXT
 		LOCK_TEXT
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 849cd8eb5ca0..ea5987bb0b84 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -53,6 +53,9 @@ extern char __ctors_start[], __ctors_end[];
 /* Start and end of .opd section - used for function descriptors. */
 extern char __start_opd[], __end_opd[];
 
+/* Start and end of instrumentation protected text section */
+extern char __noinstr_text_start[], __noinstr_text_end[];
+
 extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 2d632a74cc5e..88484ee023ca 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -482,6 +482,15 @@
 		__security_initcall_end = .;				\
 	}
 
+/*
+ * Non-instrumentable text section
+ */
+#define NOINSTR_TEXT							\
+		ALIGN_FUNCTION();					\
+		__noinstr_text_start = .;				\
+		*(.noinstr.text)					\
+		__noinstr_text_end = .;
+
 /*
  * .text section. Map to function alignment to avoid address changes
  * during second ld run in second ld pass when generating System.map
@@ -496,6 +505,7 @@
 		*(TEXT_MAIN .text.fixup)				\
 		*(.text.unlikely .text.unlikely.*)			\
 		*(.text.unknown .text.unknown.*)			\
+		NOINSTR_TEXT						\
 		*(.text..refcount)					\
 		*(.ref.text)						\
 	MEM_KEEP(init.text*)						\
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 6b6505e3b2c7..6a53300cbd1e 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -129,11 +129,65 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 	".pushsection .discard.unreachable\n\t"				\
 	".long 999b - .\n\t"						\
 	".popsection\n\t"
+
+#ifdef CONFIG_DEBUG_ENTRY
+/* Begin/end of an instrumentation safe region */
+#define instrumentation_begin() ({					\
+	asm volatile("%c0:\n\t"						\
+		     ".pushsection .discard.instr_begin\n\t"		\
+		     ".long %c0b - .\n\t"				\
+		     ".popsection\n\t" : : "i" (__COUNTER__));		\
+})
+
+/*
+ * Because instrumentation_{begin,end}() can nest, objtool validation considers
+ * _begin() a +1 and _end() a -1 and computes a sum over the instructions.
+ * When the value is greater than 0, we consider instrumentation allowed.
+ *
+ * There is a problem with code like:
+ *
+ * noinstr void foo()
+ * {
+ *	instrumentation_begin();
+ *	...
+ *	if (cond) {
+ *		instrumentation_begin();
+ *		...
+ *		instrumentation_end();
+ *	}
+ *	bar();
+ *	instrumentation_end();
+ * }
+ *
+ * If instrumentation_end() would be an empty label, like all the other
+ * annotations, the inner _end(), which is at the end of a conditional block,
+ * would land on the instruction after the block.
+ *
+ * If we then consider the sum of the !cond path, we'll see that the call to
+ * bar() is with a 0-value, even though, we meant it to happen with a positive
+ * value.
+ *
+ * To avoid this, have _end() be a NOP instruction, this ensures it will be
+ * part of the condition block and does not escape.
+ */
+#define instrumentation_end() ({					\
+	asm volatile("%c0: nop\n\t"					\
+		     ".pushsection .discard.instr_end\n\t"		\
+		     ".long %c0b - .\n\t"				\
+		     ".popsection\n\t" : : "i" (__COUNTER__));		\
+})
+#endif /* CONFIG_DEBUG_ENTRY */
+
 #else
 #define annotate_reachable()
 #define annotate_unreachable()
 #endif
 
+#ifndef instrumentation_begin
+#define instrumentation_begin()		do { } while(0)
+#define instrumentation_end()		do { } while(0)
+#endif
+
 #ifndef ASM_UNREACHABLE
 # define ASM_UNREACHABLE
 #endif
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 2b8ed70c4c77..a9b0495051a3 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -234,6 +234,10 @@ struct ftrace_likely_data {
 #define notrace			__attribute__((no_instrument_function))
 #endif
 
+/* Section for code which can't be instrumented at all */
+#define noinstr								\
+	noinline notrace __attribute((__section__(".noinstr.text")))
+
 /*
  * it doesn't make sense on ARM (currently the only user of __naked)
  * to trace naked functions because then mcount is called without
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 91a80036c05d..7c693bd775c1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -895,7 +895,7 @@ static void check_section(const char *modname, struct elf_info *elf,
 
 #define DATA_SECTIONS ".data", ".data.rel"
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
-		".kprobes.text", ".cpuidle.text"
+		".kprobes.text", ".cpuidle.text", ".noinstr.text"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
 		".fixup", ".entry.text", ".exception.text", ".text.*", \
 		".coldtext"
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply related

* [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64
From: Nicolas Boichat @ 2021-03-18 23:54 UTC (permalink / raw)
  To: stable
  Cc: Alexandre Chartre, Peter Zijlstra, Christopher Li,
	Masahiro Yamada, Paul Mackerras, Sasha Levin, Nicolas Boichat,
	clang-built-linux, linux-sparse, Naveen N. Rao, linux-arch,
	Kees Cook, Arnd Bergmann, linux-kbuild, Nicholas Piggin,
	Thomas Gleixner, Daniel Axtens, Michal Marek, Greg Kroah-Hartman,
	linux-kernel, linuxppc-dev


Backport 2 patches that are required to make KASAN+LKDTM work
with recent clang (patch 2/2 has a complete description).
Tested on our chromeos-4.19 branch.

Patch 1/2 is context conflict only, and 2/2 is a clean backport.

These patches have been merged to 5.4 stable already. We might
need to backport to older stable branches, but this is what I
could test for now.


Mark Rutland (1):
  lkdtm: don't move ctors to .rodata

Thomas Gleixner (1):
  vmlinux.lds.h: Create section for protection against instrumentation

 arch/powerpc/kernel/vmlinux.lds.S |  1 +
 drivers/misc/lkdtm/Makefile       |  2 +-
 drivers/misc/lkdtm/rodata.c       |  2 +-
 include/asm-generic/sections.h    |  3 ++
 include/asm-generic/vmlinux.lds.h | 10 ++++++
 include/linux/compiler.h          | 54 +++++++++++++++++++++++++++++++
 include/linux/compiler_types.h    |  4 +++
 scripts/mod/modpost.c             |  2 +-
 8 files changed, 75 insertions(+), 3 deletions(-)

-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply

* Re: [PATCH 04/10] MIPS: disable CONFIG_IDE in sb1250_swarm_defconfig
From: Maciej W. Rozycki @ 2021-03-19  0:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, Russell King,
	linux-kernel, linux-ide, linux-m68k, Ivan Kokshaysky, linux-alpha,
	Geert Uytterhoeven, Matt Turner, linux-mips, linuxppc-dev,
	David S. Miller, linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-5-hch@lst.de>

On Thu, 18 Mar 2021, Christoph Hellwig wrote:

> sb1250_swarm_defconfig enables CONFIG_IDE but no actual host controller
> driver, so just drop CONFIG_IDE, CONFIG_BLK_DEV_IDECD and
> CONFIG_BLK_DEV_IDETAPE as they are useless.

 Actually BLK_DEV_PLATFORM would handle the SWARM's platform driver as an 
IDE device, however the driver has supported libata ever since commit 
2fef357cf391 ("IDE: Fix platform device registration in Swarm IDE driver 
(v2)") back in 2008, so this is good to go.  We should probably enable 
PATA_PLATFORM in the defconfig instead.

 The printed name of the driver could be improved I suppose though:

scsi host0: pata_platform
ata1: PATA max PIO0 mmio cmd 0x100b3e00 ctl 0x100b7ec0 irq 36

(PIO3 is actually hardwired; it's an odd interface and people reported 
issues with it, but I have never had any myself be it with IDE or libata).

Acked-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

^ permalink raw reply

* Re: [PATCH 00/36] [Set 4] Rid W=1 warnings in SCSI
From: Martin K. Petersen @ 2021-03-19  3:41 UTC (permalink / raw)
  To: Lee Jones
  Cc: Uma Krishnan, Brian Macy, Hannes Reinecke, Anil Ravindranath,
	Tyrel Datwyler, willy, Le Moal, Dave Boutcher, Marvell,
	Jirka Hanika, Linda Xie, C.L. Huang, target-devel, Drew Eckhardt,
	Brian King, Christoph Hellwig, Alan Cox, linux-drivers,
	Nicholas A. Bellinger, Linux GmbH, linux-scsi, Shaun Tancheff,
	Subbu Seetharaman, Sathya Prakash, Doug Ledford,
	Leonard N. Zubkoff, Ketan Mukadam, Dave Boutcher, Colin DeVilbiss,
	Karan Tilak Kumar, Badari Pulavarty, Bryant G. Ly,
	Douglas Gilbert, Jamie Lenehan, MPT-FusionLinux.pdl,
	Richard Gooch, MPT-FusionLinux.pdl, Bas Vermeulen,
	Artur Paszkiewicz, Michael Cyr, dc395x, Satish Kharat,
	Suganath Prabu Subramani, James E.J. Bottomley, Luben Tuikov,
	Ali Akcaagac, Kurt Garloff, Jitendra Bhivare, Brian King,
	Hannes Reinecke, Erich Chen, David Chaw, Santiago Leon,
	Matthew R. Ochs, Manoj N. Kumar, Sreekanth Reddy,
	Martin K. Petersen, Eric Youngdale, Oliver Neukum, linux-kernel,
	Sesidhar Baddela, Alex Davis, Torben Mathiasen, Paul Mackerras,
	FUJITA Tomonori, linuxppc-dev
In-Reply-To: <20210317091230.2912389-1-lee.jones@linaro.org>


Lee,

> This set is part of a larger effort attempting to clean-up W=1 kernel
> builds, which are currently overwhelmingly riddled with niggly little
> warnings.

Applied to 5.13/scsi-staging, thanks! I fixed a few little things.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH] ASoC: fsl_sai: remove reset code from dai_probe
From: Shengjiu Wang @ 2021-03-19  4:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel@alsa-project.org, timur@kernel.org,
	Xiubo.Lee@gmail.com, linuxppc-dev@lists.ozlabs.org, S.j. Wang,
	tiwai@suse.com, lgirdwood@gmail.com, nicoleotsuka@gmail.com,
	Viorel Suman, festevam@gmail.com, linux-kernel@vger.kernel.org
In-Reply-To: <20210316134915.GB4309@sirena.org.uk>

Hi Mark

On Tue, Mar 16, 2021 at 9:51 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Mar 16, 2021 at 01:42:40PM +0000, Viorel Suman wrote:
>
> > To me it makes sense to manage the clocks and reset from the same place.
> > Currently we have the clocks management moved completely into runtime PM
> > fsl_sai_runtime_resume and fsl_sai_runtime_suspend callbacks.
>
> Usually the pattern is to have probe() leave everything powered up then
> let runtime PM power things down if it's enabled, you can often do the
> power up by having an open coded call to the resume callback in probe().

It seems some drivers very depend on runtime PM, if the CONFIG_PM=n,
the drivers should not work.  What's the strategy for this?
Do we need to support both cases, or only one case is also acceptable?

Best regards
Wang Shengjiu

^ permalink raw reply

* Re: remove the legacy ide driver
From: Christoph Hellwig @ 2021-03-19  5:43 UTC (permalink / raw)
  To: Finn Thain
  Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, Russell King,
	David S. Miller, linux-ide, linux-m68k, Ivan Kokshaysky,
	linux-arm-kernel, linux-alpha, Geert Uytterhoeven, Matt Turner,
	linux-mips, linuxppc-dev, Christoph Hellwig, linux-kernel,
	Richard Henderson
In-Reply-To: <c1fa8e6-a05d-9ea1-f47e-9e85ea6ea65e@telegraphics.com.au>

On Fri, Mar 19, 2021 at 12:43:48PM +1100, Finn Thain wrote:
> A few months ago I wrote another patch to move some more platforms away 
> from macide but it has not been tested yet. That is not to say you should 
> wait. However, my patch does have some changes that are missing from your 
> patch series, relating to ide platform devices in arch/m68k/mac/config.c. 
> I hope to be able to test this patch before the 5.13 merge window closes.

Normally we do not remove drivers for hardware that is still used.  So
at leat for macide my plan was not to take it away unless the users 
are sufficiently happy.  Or in other words:  I think waiting it the
right choice, but hopefully we can make that wait as short as possible.

^ permalink raw reply

* Re: [PATCH] powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S
From: Michael Ellerman @ 2021-03-19  6:12 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, dja
In-Reply-To: <20210316024434.GE16691@gate.crashing.org>

Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi!
>
> On Mon, Mar 15, 2021 at 02:41:59PM +1100, Michael Ellerman wrote:
>> The ".machine" directive allows changing the machine for which code is
>> being generated. It's equivalent to passing an -mcpu option on the
>> command line.
>> 
>> Although it can be useful, it's generally a bad idea because it adds
>> another way to influence code generation separate from the flags
>> passed via the build system. ie. if we need to build different pieces
>> of code with different flags we should do that via our Makefiles, not
>> using ".machine".
>
> It does not influence code generation.  It says which instructions are
> valid, instead.  There are a few cases where the same mnemonic will
> generate a different binary encoding depending on machine selected,
> maybe you mean that?

Yeah that's what I was referring to. Which is code generation in my
mind, but I guess that's probably not the right terminology to use
around compiler people :)

And I guess you're right, the more common case is that the mnemonics are
just not valid for other machines and wouldn't assemble at all.

I'll reword it.

> It is *normal* to use .machine push/pop and a specific .machine around
> instructions that require a machine other than what you are building
> for.  The compiler does this itself, and it is the recommended way to
> use "foreign" instructions in inline assembler.

Right, but it also makes it easy to build code that won't run :) So we'd
like to avoid it.

We had that in the past where we were building the power7-only memcpy
routines for Book3E 64. They weren't being used due to runtime patching
stuff, but it's better to not build them in the first place for those
CPUs because they could never work.

> That said...
>
>> However as best as I can tell the ".machine" directive in
>> trampoline_64.S is not necessary at all.
>> 
>> It was added in commit 0d97631392c2 ("powerpc: Add purgatory for
>> kexec_file_load() implementation."), which created the file based on
>> the kexec-tools purgatory. It may be/have-been necessary in the
>> kexec-tools version, but we have a completely different build system,
>> and we already pass the desired CPU flags, eg:
>> 
>>   gcc ... -m64 -Wl,-a64 -mabi=elfv2 -Wa,-maltivec -Wa,-mpower4 -Wa,-many
>>   ... arch/powerpc/purgatory/trampoline_64.S
>> 
>> So drop the ".machine" directive and rely on the assembler flags.
>
>> -	.machine ppc64
>
> Please make sure to test this on a big endian config.

Done.

> A ppc64le-linux assembler defaults to power8.  A ppc64-linux assembler
> defaults to power3 (that is the same as .machine ppc64).  Or maybe it
> makes it power4?  I get lost :-)

For book3s64 we always specify -mpower4 since 15a3204d24a3
("powerpc/64s: Set assembler machine type to POWER4") (Apr 2018).

That does leave 64-bit book3e, but I just tested that and it also builds
fine.

> It certainly *should* work, but, test please :-)
>
> (And with a *default* powerpc64-linux config, not one that defaults to
> power7 or power8 or similar!  Arnd's toolchains at
> <https://mirrors.edge.kernel.org/pub/tools/crosstool/>
> are fine for this.)

Yep, I used Arnd's 10.1.0.

> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>

Thanks.

cheers

^ permalink raw reply

* [PATCH v6] powerpc/irq: inline call_do_irq() and call_do_softirq() on PPC32
From: Christophe Leroy @ 2021-03-19  6:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

call_do_irq() and call_do_softirq() are simple enough to be
worth inlining.

Inlining them avoids an mflr/mtlr pair plus a save/reload on stack.
It also allows GCC to keep the saved ksp_limit in an nonvolatile reg.

This is inspired from S390 arch. Several other arches do more or
less the same. The way sparc arch does seems odd thought.

For the time being this is limited to PPC32 because there are
incertainties on the handling of r2 which is the TOC on PPC64,
see discussion at https://patchwork.ozlabs.org/patch/1174288/

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
---
v2: no change.
v3: no change.
v4:
- comment reminding the purpose of the inline asm block.
- added r2 as clobbered reg
v5:
- Limiting the change to PPC32 for now.
- removed r2 from the clobbered regs list (on PPC32 r2 points to current all the time)
- Removed patch 1 and merged ksp_limit handling in here.
v6:
- Rebase on top of merge-test (ca6e327fefb2).
- Remove the ksp_limit stuff as it's doesn't exist anymore.
---
 arch/powerpc/include/asm/irq.h |  2 ++
 arch/powerpc/kernel/irq.c      | 34 ++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/misc_32.S  | 25 -------------------------
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index f3f264e441a7..23c28974ca29 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -53,8 +53,10 @@ extern void *mcheckirq_ctx[NR_CPUS];
 extern void *hardirq_ctx[NR_CPUS];
 extern void *softirq_ctx[NR_CPUS];
 
+#ifdef CONFIG_PPC64
 void call_do_softirq(void *sp);
 void call_do_irq(struct pt_regs *regs, void *sp);
+#endif
 extern void do_IRQ(struct pt_regs *regs);
 extern void __init init_IRQ(void);
 extern void __do_irq(struct pt_regs *regs);
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 5b72abbff96c..327422c57ae8 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -667,6 +667,40 @@ static inline void check_stack_overflow(void)
 	}
 }
 
+#ifdef CONFIG_PPC32
+static inline void call_do_softirq(const void *sp)
+{
+	register unsigned long ret asm("r3");
+
+	/* Temporarily switch r1 to sp, call __do_softirq() then restore r1. */
+	asm volatile(
+		"	"PPC_STLU"	1, %2(%1);\n"
+		"	mr		1, %1;\n"
+		"	bl		%3;\n"
+		"	"PPC_LL"	1, 0(1);\n" :
+		"=r"(ret) :
+		"b"(sp), "i"(THREAD_SIZE - STACK_FRAME_OVERHEAD), "i"(__do_softirq) :
+		"lr", "xer", "ctr", "memory", "cr0", "cr1", "cr5", "cr6", "cr7",
+		"r0", "r4", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12");
+}
+
+static inline void call_do_irq(struct pt_regs *regs, void *sp)
+{
+	register unsigned long r3 asm("r3") = (unsigned long)regs;
+
+	/* Temporarily switch r1 to sp, call __do_irq() then restore r1. */
+	asm volatile(
+		"	"PPC_STLU"	1, %2(%1);\n"
+		"	mr		1, %1;\n"
+		"	bl		%3;\n"
+		"	"PPC_LL"	1, 0(1);\n" :
+		"+r"(r3) :
+		"b"(sp), "i"(THREAD_SIZE - STACK_FRAME_OVERHEAD), "i"(__do_irq) :
+		"lr", "xer", "ctr", "memory", "cr0", "cr1", "cr5", "cr6", "cr7",
+		"r0", "r4", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12");
+}
+#endif
+
 void __do_irq(struct pt_regs *regs)
 {
 	unsigned int irq;
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index acc410043b96..6a076bef2932 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -27,31 +27,6 @@
 
 	.text
 
-_GLOBAL(call_do_softirq)
-	mflr	r0
-	stw	r0,4(r1)
-	stwu	r1,THREAD_SIZE-STACK_FRAME_OVERHEAD(r3)
-	mr	r1,r3
-	bl	__do_softirq
-	lwz	r1,0(r1)
-	lwz	r0,4(r1)
-	mtlr	r0
-	blr
-
-/*
- * void call_do_irq(struct pt_regs *regs, void *sp);
- */
-_GLOBAL(call_do_irq)
-	mflr	r0
-	stw	r0,4(r1)
-	stwu	r1,THREAD_SIZE-STACK_FRAME_OVERHEAD(r4)
-	mr	r1,r4
-	bl	__do_irq
-	lwz	r1,0(r1)
-	lwz	r0,4(r1)
-	mtlr	r0
-	blr
-
 /*
  * This returns the high 64 bits of the product of two 64-bit numbers.
  */
-- 
2.25.0


^ permalink raw reply related

* Clang: powerpc: kvm/book3s_hv_nested.c:264:6: error: stack frame size of 2480 bytes in function 'kvmhv_enter_nested_guest'
From: Naresh Kamboju @ 2021-03-19  7:20 UTC (permalink / raw)
  To: clang-built-linux, open list, lkft-triage, linuxppc-dev, kvm-ppc
  Cc: Nathan Chancellor, Arnd Bergmann, Nick Desaulniers,
	Nathan Chancellor

Linux mainline master build breaks for powerpc defconfig.
There are multiple errors / warnings with clang-12 and clang-11 and 10.
 - powerpc (defconfig) with clang-12
 - powerpc (defconfig) with clang-11
 - powerpc (defconfig) with clang-10

The following build errors / warnings triggered with clang-12.
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp LLVM=1 ARCH=powerpc
CROSS_COMPILE=powerpc64le-linux-gnu- 'HOSTCC=sccache clang'
'CC=sccache clang'
/builds/linux/arch/powerpc/kvm/book3s_hv_nested.c:264:6: error: stack
frame size of 2480 bytes in function 'kvmhv_enter_nested_guest'
[-Werror,-Wframe-larger-than=]
long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
     ^
1 error generated.
make[3]: *** [/builds/linux/scripts/Makefile.build:271:
arch/powerpc/kvm/book3s_hv_nested.o] Error 1

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>

The following build errors / warnings triggered with clang-10 and clang-11.
 - powerpc (defconfig) with clang-11
 - powerpc (defconfig) with clang-10
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp LLVM=1 ARCH=powerpc
CROSS_COMPILE=powerpc64le-linux-gnu- 'HOSTCC=sccache clang'
'CC=sccache clang'

/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/sigtramp.o: compiled for a little endian
system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/sigtramp.o
/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/gettimeofday.o: compiled for a little
endian system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/gettimeofday.o
/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/datapage.o: compiled for a little endian
system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/datapage.o
/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/cacheflush.o: compiled for a little endian
system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/cacheflush.o
/usr/bin/powerpc64le-linux-gnu-ld: arch/powerpc/kernel/vdso32/note.o:
compiled for a little endian system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/note.o
/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/getcpu.o: compiled for a little endian
system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/getcpu.o
/usr/bin/powerpc64le-linux-gnu-ld:
arch/powerpc/kernel/vdso32/vgettimeofday.o: compiled for a little
endian system and target is big endian
/usr/bin/powerpc64le-linux-gnu-ld: failed to merge target specific
data of file arch/powerpc/kernel/vdso32/vgettimeofday.o
clang: error: unable to execute command: Segmentation fault (core dumped)
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [/builds/linux/arch/powerpc/kernel/vdso32/Makefile:51:
arch/powerpc/kernel/vdso32/vdso32.so.dbg] Error 254
make[2]: Target 'include/generated/vdso32-offsets.h' not remade
because of errors.

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>

build link,
https://gitlab.com/Linaro/lkft/mirrors/torvalds/linux-mainline/-/jobs/1110841371#L59

--
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply

* [PATCH] ASoC: fsl_sai: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-19  8:06 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
	perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel

When there is power domain bind with bus clock,

The call flow:
devm_regmap_init_mmio_clk
   - clk_prepare()
      - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk, then explicitly enable clock when
using by pm_runtime_get(), if CONFIG_PM=n, then
fsl_sai_runtime_resume will be explicitly called.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 48 +++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 9e7893f91882..f2c70a31c7bb 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -987,6 +987,9 @@ static int fsl_sai_check_version(struct device *dev)
 	return 0;
 }
 
+static int fsl_sai_runtime_suspend(struct device *dev);
+static int fsl_sai_runtime_resume(struct device *dev);
+
 static int fsl_sai_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -1019,24 +1022,21 @@ static int fsl_sai_probe(struct platform_device *pdev)
 			ARRAY_SIZE(fsl_sai_reg_defaults_ofs8);
 	}
 
-	sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
-			"bus", base, &fsl_sai_regmap_config);
-
-	/* Compatible with old DTB cases */
-	if (IS_ERR(sai->regmap) && PTR_ERR(sai->regmap) != -EPROBE_DEFER)
-		sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
-				"sai", base, &fsl_sai_regmap_config);
+	sai->regmap = devm_regmap_init_mmio(&pdev->dev, base, &fsl_sai_regmap_config);
 	if (IS_ERR(sai->regmap)) {
 		dev_err(&pdev->dev, "regmap init failed\n");
 		return PTR_ERR(sai->regmap);
 	}
 
-	/* No error out for old DTB cases but only mark the clock NULL */
 	sai->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	/* Compatible with old DTB cases */
+	if (IS_ERR(sai->bus_clk) && PTR_ERR(sai->bus_clk) != -EPROBE_DEFER)
+		sai->bus_clk = devm_clk_get(&pdev->dev, "sai");
 	if (IS_ERR(sai->bus_clk)) {
 		dev_err(&pdev->dev, "failed to get bus clock: %ld\n",
 				PTR_ERR(sai->bus_clk));
-		sai->bus_clk = NULL;
+		/* -EPROBE_DEFER */
+		return PTR_ERR(sai->bus_clk);
 	}
 
 	for (i = 1; i < FSL_SAI_MCLK_MAX; i++) {
@@ -1117,6 +1117,18 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	sai->dma_params_tx.maxburst = FSL_SAI_MAXBURST_TX;
 
 	platform_set_drvdata(pdev, sai);
+	pm_runtime_enable(&pdev->dev);
+	if (!pm_runtime_enabled(&pdev->dev)) {
+		ret = fsl_sai_runtime_resume(&pdev->dev);
+		if (ret)
+			goto err_pm_disable;
+	}
+
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		goto err_pm_get_sync;
+	}
 
 	/* Get sai version */
 	ret = fsl_sai_check_version(&pdev->dev);
@@ -1130,26 +1142,30 @@ static int fsl_sai_probe(struct platform_device *pdev)
 				   FSL_SAI_MCTL_MCLK_EN, FSL_SAI_MCTL_MCLK_EN);
 	}
 
-	pm_runtime_enable(&pdev->dev);
-	regcache_cache_only(sai->regmap, true);
+	ret = pm_runtime_put_sync(&pdev->dev);
+	if (ret < 0)
+		goto err_pm_get_sync;
 
 	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
 					      &sai->cpu_dai_drv, 1);
 	if (ret)
-		goto err_pm_disable;
+		goto err_pm_get_sync;
 
 	if (sai->soc_data->use_imx_pcm) {
 		ret = imx_pcm_dma_init(pdev, IMX_SAI_DMABUF_SIZE);
 		if (ret)
-			goto err_pm_disable;
+			goto err_pm_get_sync;
 	} else {
 		ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 		if (ret)
-			goto err_pm_disable;
+			goto err_pm_get_sync;
 	}
 
 	return ret;
 
+err_pm_get_sync:
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		fsl_sai_runtime_suspend(&pdev->dev);
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
 
@@ -1159,6 +1175,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
 static int fsl_sai_remove(struct platform_device *pdev)
 {
 	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		fsl_sai_runtime_suspend(&pdev->dev);
 
 	return 0;
 }
@@ -1219,7 +1237,6 @@ static const struct of_device_id fsl_sai_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, fsl_sai_ids);
 
-#ifdef CONFIG_PM
 static int fsl_sai_runtime_suspend(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
@@ -1292,7 +1309,6 @@ static int fsl_sai_runtime_resume(struct device *dev)
 
 	return ret;
 }
-#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops fsl_sai_pm_ops = {
 	SET_RUNTIME_PM_OPS(fsl_sai_runtime_suspend,
-- 
2.27.0


^ permalink raw reply related

* Re: [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64
From: Greg Kroah-Hartman @ 2021-03-19 10:30 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Alexandre Chartre, Peter Zijlstra, Christopher Li,
	Masahiro Yamada, Paul Mackerras, Sasha Levin, clang-built-linux,
	linux-sparse, Naveen N. Rao, linux-arch, Kees Cook, Arnd Bergmann,
	linux-kbuild, Nicholas Piggin, Thomas Gleixner, Daniel Axtens,
	Michal Marek, linux-kernel, stable, linuxppc-dev
In-Reply-To: <20210318235416.794798-1-drinkcat@chromium.org>

On Fri, Mar 19, 2021 at 07:54:14AM +0800, Nicolas Boichat wrote:
> 
> Backport 2 patches that are required to make KASAN+LKDTM work
> with recent clang (patch 2/2 has a complete description).
> Tested on our chromeos-4.19 branch.
> 
> Patch 1/2 is context conflict only, and 2/2 is a clean backport.
> 
> These patches have been merged to 5.4 stable already. We might
> need to backport to older stable branches, but this is what I
> could test for now.

Both now queued up, thanks.

greg k-h

^ permalink raw reply

* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation
From: Greg Kroah-Hartman @ 2021-03-19 10:39 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Sasha Levin, linux-arch, Michal Marek, Alexandre Chartre,
	Arnd Bergmann, linux-kbuild, Peter Zijlstra, Christopher Li,
	linux-kernel, stable, Masahiro Yamada, linux-sparse,
	Paul Mackerras, Nicholas Piggin, Thomas Gleixner, linuxppc-dev,
	Naveen N. Rao, Daniel Axtens
In-Reply-To: <20210319075410.for-stable-4.19.1.I222f801866f71be9f7d85e5b10665cd4506d78ec@changeid>

On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> commit 6553896666433e7efec589838b400a2a652b3ffa upstream.
> 
> Some code pathes, especially the low level entry code, must be protected
> against instrumentation for various reasons:
> 
>  - Low level entry code can be a fragile beast, especially on x86.
> 
>  - With NO_HZ_FULL RCU state needs to be established before using it.
> 
> Having a dedicated section for such code allows to validate with tooling
> that no unsafe functions are invoked.
> 
> Add the .noinstr.text section and the noinstr attribute to mark
> functions. noinstr implies notrace. Kprobes will gain a section check
> later.
> 
> Provide also a set of markers: instrumentation_begin()/end()
> 
> These are used to mark code inside a noinstr function which calls
> into regular instrumentable text section as safe.
> 
> The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
> enabled as the end marker emits a NOP to prevent the compiler from merging
> the annotation points. This means the objtool verification requires a
> kernel compiled with this option.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
> Acked-by: Peter Zijlstra <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de
> 
> [Nicolas: context conflicts in:
> 	arch/powerpc/kernel/vmlinux.lds.S
> 	include/asm-generic/vmlinux.lds.h
> 	include/linux/compiler.h
> 	include/linux/compiler_types.h]
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

Did you build this on x86?

I get the following build error:

ld:./arch/x86/kernel/vmlinux.lds:20: syntax error

And that line looks like:

 . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*)

So I'm going to drop both of these patches from the queue.

thanks,

greg k-h

^ permalink raw reply

* Re: [RFC PATCH 2/8] powerpc: check for support for -Wa, -m{power4, any}
From: Michael Ellerman @ 2021-03-19 10:53 UTC (permalink / raw)
  To: Nicholas Piggin, Daniel Axtens, linuxppc-dev, llvmlinux
In-Reply-To: <1616117194.vwe39qw3i4.astroid@bobo.none>

Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Daniel Axtens's message of February 25, 2021 1:10 pm:
>> LLVM's integrated assembler does not like either -Wa,-mpower4
>> or -Wa,-many. So just don't pass them if they're not supported.
>> 
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> ---
>>  arch/powerpc/Makefile | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 08cf0eade56a..3e2c72d20bb8 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -252,7 +252,9 @@ cpu-as-$(CONFIG_E500)		+= -Wa,-me500
>>  # When using '-many -mpower4' gas will first try and find a matching power4
>>  # mnemonic and failing that it will allow any valid mnemonic that GAS knows
>>  # about. GCC will pass -many to GAS when assembling, clang does not.
>> -cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= -Wa,-mpower4 -Wa,-many
>> +# LLVM IAS doesn't understand either flag: https://github.com/ClangBuiltLinux/linux/issues/675
>> +# but LLVM IAS only supports ISA >= 2.06 for Book3S 64 anyway...
>> +cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= $(call as-option,-Wa$(comma)-mpower4) $(call as-option,-Wa$(comma)-many)
>>  cpu-as-$(CONFIG_PPC_E500MC)	+= $(call as-option,-Wa$(comma)-me500mc)
>>  
>>  KBUILD_AFLAGS += $(cpu-as-y)
>
> I'm wondering why we even have this now. Kbuild's "AS" command goes 
> through the C compiler now with relevant options like -mcpu.

It uses $(CC) but it doesn't pass it $CFLAGS AFAIK. So it would use
whatever the compiler default is for -mcpu etc. I think.

> I assume it used to be useful for cross compiling when as was called
> directly but I'm not sure.

We still use it directly in vdso32/Makefile.

cheers

^ permalink raw reply

* Re: [RFC PATCH 7/8] powerpc/purgatory: drop .machine specifier
From: Michael Ellerman @ 2021-03-19 10:59 UTC (permalink / raw)
  To: Nicholas Piggin, Daniel Axtens, Segher Boessenkool
  Cc: linuxppc-dev, llvmlinux
In-Reply-To: <1616119361.tyoejtbh8j.astroid@bobo.none>

Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Segher Boessenkool's message of February 26, 2021 1:58 am:
>> On Thu, Feb 25, 2021 at 02:10:05PM +1100, Daniel Axtens wrote:
>>> It's ignored by future versions of llvm's integrated assembler (by not -11).
>>> I'm not sure what it does for us in gas.
>> 
>> It enables all insns that exist on 620 (the first 64-bit PowerPC CPU).
>
> Same question for this, why do we have it at all?

I sent a patch to drop it.

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210315034159.315675-1-mpe@ellerman.id.au/

cheers

^ permalink raw reply

* [PATCH 00/10] Convert signal32 to user read access by block
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linux-arch, linuxppc-dev, linux-kernel

Similarly to the work done earlier with writes, this series
converts signal32 to using user_read_access_begin/end and
unsafe_get_user() and friends.

Applies on to of the signal64 series, ie on merge-test (ca6e327fefb2)

Christophe Leroy (10):
  signal: Add unsafe_get_compat_sigset()
  powerpc/uaccess: Also perform 64 bits copies in
    unsafe_copy_from_user() on ppc32
  powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
  powerpc/signal32: Rename save_user_regs_unsafe() and
    save_general_regs_unsafe()
  powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
  powerpc/signal32: Perform access_ok() inside restore_user_regs()
  powerpc/signal32: Reorder user reads in restore_tm_user_regs()
  powerpc/signal32: Convert restore_[tm]_user_regs() to user access
    block
  powerpc/signal32: Convert do_setcontext[_tm]() to user access block
  powerpc/signal32: Simplify logging in sigreturn()

 arch/powerpc/include/asm/ptrace.h  |   2 +-
 arch/powerpc/include/asm/uaccess.h |   6 +-
 arch/powerpc/kernel/signal.h       |  22 +++
 arch/powerpc/kernel/signal_32.c    | 251 ++++++++++++++++-------------
 include/linux/compat.h             |  35 ++++
 include/linux/uaccess.h            |   1 +
 6 files changed, 205 insertions(+), 112 deletions(-)

-- 
2.25.0


^ permalink raw reply

* [PATCH 02/10] powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_from_user() on ppc32
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

Similarly to commit 5cf773fc8f37 ("powerpc/uaccess: Also perform
64 bits copies in unsafe_copy_to_user() on ppc32")

ppc32 has an efficiant 64 bits unsafe_get_user(), so also use it in
order to unroll loops more.

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

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 77d837b16e4d..a4e791bcd3fe 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -432,9 +432,9 @@ do {											\
 	size_t _len = (l);								\
 	int _i;										\
 											\
-	for (_i = 0; _i < (_len & ~(sizeof(long) - 1)); _i += sizeof(long))		\
-		unsafe_get_user(*(long *)(_dst + _i), (long __user *)(_src + _i), e);	\
-	if (IS_ENABLED(CONFIG_PPC64) && (_len & 4)) {					\
+	for (_i = 0; _i < (_len & ~(sizeof(u64) - 1)); _i += sizeof(u64))		\
+		unsafe_get_user(*(u64 *)(_dst + _i), (u64 __user *)(_src + _i), e);	\
+	if (_len & 4) {									\
 		unsafe_get_user(*(u32 *)(_dst + _i), (u32 __user *)(_src + _i), e);	\
 		_i += 4;								\
 	}										\
-- 
2.25.0


^ permalink raw reply related

* [PATCH 03/10] powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

Add unsafe_copy_ckfpr_from_user() and unsafe_copy_ckvsx_from_user()

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

diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 1393876f3814..a5152ff3c52f 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -100,6 +100,26 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
 		unsafe_put_user(__t->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET], \
 				&buf[i], label);\
 } while (0)
+
+#define unsafe_copy_ckfpr_from_user(task, from, label)	do {		\
+	struct task_struct *__t = task;					\
+	u64 __user *buf = (u64 __user *)from;				\
+	int i;								\
+									\
+	for (i = 0; i < ELF_NFPREG - 1 ; i++)				\
+		unsafe_get_user(__t->thread.TS_CKFPR(i), &buf[i], label);\
+	unsafe_get_user(__t->thread.ckfp_state.fpscr, &buf[i], failed);	\
+} while (0)
+
+#define unsafe_copy_ckvsx_from_user(task, from, label)	do {		\
+	struct task_struct *__t = task;					\
+	u64 __user *buf = (u64 __user *)from;				\
+	int i;								\
+									\
+	for (i = 0; i < ELF_NVSRHALFREG ; i++)				\
+		unsafe_get_user(__t->thread.ckfp_state.fpr[i][TS_VSRLOWOFFSET], \
+				&buf[i], label);			\
+} while (0)
 #endif
 #elif defined(CONFIG_PPC_FPU_REGS)
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH 01/10] signal: Add unsafe_get_compat_sigset()
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linux-arch, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

In the same way as commit 14026b94ccfe ("signal: Add
unsafe_put_compat_sigset()"), this time add
unsafe_get_compat_sigset() macro which is the 'unsafe'
version of get_compat_sigset()

For the bigendian, use unsafe_get_user() directly
to avoid intermediate copy through the stack.

For the littleendian, use a straight unsafe_copy_from_user().

This commit adds the generic fallback for unsafe_copy_from_user().
Architectures wanting to use unsafe_get_compat_sigset() have to
make sure they have their own unsafe_copy_from_user().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 include/linux/compat.h  | 35 +++++++++++++++++++++++++++++++++++
 include/linux/uaccess.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be753603..5112c3e35782 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -465,6 +465,34 @@ put_compat_sigset(compat_sigset_t __user *compat, const sigset_t *set,
 		unsafe_put_user(__s->sig[0], &__c->sig[0], label);	\
 	}								\
 } while (0)
+
+#define unsafe_get_compat_sigset(set, compat, label) do {		\
+	const compat_sigset_t __user *__c = compat;			\
+	compat_sigset_word hi, lo;					\
+	sigset_t *__s = set;						\
+									\
+	switch (_NSIG_WORDS) {						\
+	case 4:								\
+		unsafe_get_user(lo, &__c->sig[7], label);		\
+		unsafe_get_user(hi, &__c->sig[6], label);		\
+		__s->sig[3] = hi | (((long)lo) << 32);			\
+		fallthrough;						\
+	case 3:								\
+		unsafe_get_user(lo, &__c->sig[5], label);		\
+		unsafe_get_user(hi, &__c->sig[4], label);		\
+		__s->sig[2] = hi | (((long)lo) << 32);			\
+		fallthrough;						\
+	case 2:								\
+		unsafe_get_user(lo, &__c->sig[3], label);		\
+		unsafe_get_user(hi, &__c->sig[2], label);		\
+		__s->sig[1] = hi | (((long)lo) << 32);			\
+		fallthrough;						\
+	case 1:								\
+		unsafe_get_user(lo, &__c->sig[1], label);		\
+		unsafe_get_user(hi, &__c->sig[0], label);		\
+		__s->sig[0] = hi | (((long)lo) << 32);			\
+	}								\
+} while (0)
 #else
 #define unsafe_put_compat_sigset(compat, set, label) do {		\
 	compat_sigset_t __user *__c = compat;				\
@@ -472,6 +500,13 @@ put_compat_sigset(compat_sigset_t __user *compat, const sigset_t *set,
 									\
 	unsafe_copy_to_user(__c, __s, sizeof(*__c), label);		\
 } while (0)
+
+#define unsafe_get_compat_sigset(set, compat, label) do {		\
+	const compat_sigset_t __user *__c = compat;			\
+	sigset_t *__s = set;						\
+									\
+	unsafe_copy_from_user(__s, __c, sizeof(*__c), label);		\
+} while (0)
 #endif
 
 extern int compat_ptrace_request(struct task_struct *child,
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index c7c6e8b8344d..c05e903cef02 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -397,6 +397,7 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
 #define unsafe_get_user(x,p,e) unsafe_op_wrap(__get_user(x,p),e)
 #define unsafe_put_user(x,p,e) unsafe_op_wrap(__put_user(x,p),e)
 #define unsafe_copy_to_user(d,s,l,e) unsafe_op_wrap(__copy_to_user(d,s,l),e)
+#define unsafe_copy_from_user(d,s,l,e) unsafe_op_wrap(__copy_from_user(d,s,l),e)
 static inline unsigned long user_access_save(void) { return 0UL; }
 static inline void user_access_restore(unsigned long flags) { }
 #endif
-- 
2.25.0


^ permalink raw reply related

* [PATCH 04/10] powerpc/signal32: Rename save_user_regs_unsafe() and save_general_regs_unsafe()
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

Convention is to prefix functions with __unsafe_ instead of
suffixing it with _unsafe.

Rename save_user_regs_unsafe() and save_general_regs_unsafe()
accordingly, that is respectively __unsafe_save_general_regs() and
__unsafe_save_user_regs().

Suggested-by: Christopher M. Riedl <cmr@codefail.de>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal_32.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index c505b444a613..3b78748d6d85 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -94,7 +94,7 @@ static inline int get_sigset_t(sigset_t *set,
 #define from_user_ptr(p)	compat_ptr(p)
 
 static __always_inline int
-save_general_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame)
+__unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
 {
 	elf_greg_t64 *gregs = (elf_greg_t64 *)regs;
 	int val, i;
@@ -151,7 +151,7 @@ static inline int get_sigset_t(sigset_t *set, const sigset_t __user *uset)
 #define from_user_ptr(p)	((void __user *)(p))
 
 static __always_inline int
-save_general_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame)
+__unsafe_save_general_regs(struct pt_regs *regs, struct mcontext __user *frame)
 {
 	WARN_ON(!FULL_REGS(regs));
 	unsafe_copy_to_user(&frame->mc_gregs, regs, GP_REGS_SIZE, failed);
@@ -177,7 +177,7 @@ static inline int restore_general_regs(struct pt_regs *regs,
 #endif
 
 #define unsafe_save_general_regs(regs, frame, label) do {	\
-	if (save_general_regs_unsafe(regs, frame))	\
+	if (__unsafe_save_general_regs(regs, frame))		\
 		goto label;					\
 } while (0)
 
@@ -260,8 +260,8 @@ static void prepare_save_user_regs(int ctx_has_vsx_region)
 #endif
 }
 
-static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *frame,
-				 struct mcontext __user *tm_frame, int ctx_has_vsx_region)
+static int __unsafe_save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
+				   struct mcontext __user *tm_frame, int ctx_has_vsx_region)
 {
 	unsigned long msr = regs->msr;
 
@@ -338,7 +338,7 @@ static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *f
 }
 
 #define unsafe_save_user_regs(regs, frame, tm_frame, has_vsx, label) do { \
-	if (save_user_regs_unsafe(regs, frame, tm_frame, has_vsx))	\
+	if (__unsafe_save_user_regs(regs, frame, tm_frame, has_vsx))	\
 		goto label;						\
 } while (0)
 
@@ -350,7 +350,7 @@ static int save_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user *f
  * We also save the transactional registers to a second ucontext in the
  * frame.
  *
- * See save_user_regs_unsafe() and signal_64.c:setup_tm_sigcontexts().
+ * See __unsafe_save_user_regs() and signal_64.c:setup_tm_sigcontexts().
  */
 static void prepare_save_tm_user_regs(void)
 {
@@ -441,7 +441,7 @@ static int save_tm_user_regs_unsafe(struct pt_regs *regs, struct mcontext __user
 #endif /* CONFIG_VSX */
 #ifdef CONFIG_SPE
 	/* SPE regs are not checkpointed with TM, so this section is
-	 * simply the same as in save_user_regs_unsafe().
+	 * simply the same as in __unsafe_save_user_regs().
 	 */
 	if (current->thread.used_spe) {
 		unsafe_copy_to_user(&frame->mc_vregs, current->thread.evr,
-- 
2.25.0


^ permalink raw reply related

* [PATCH 06/10] powerpc/signal32: Perform access_ok() inside restore_user_regs()
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

In preparation of using user_access_begin/end in restore_user_regs(),
move the access_ok() inside the function.

It makes no difference as the behaviour on a failed access_ok() is
the same as on failed restore_user_regs().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal_32.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 8dfe4fe77706..e2b1d2a0abad 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -492,6 +492,8 @@ static long restore_user_regs(struct pt_regs *regs,
 	int i;
 #endif
 
+	if (!access_ok(sr, sizeof(*sr)))
+		return 1;
 	/*
 	 * restore general registers but not including MSR or SOFTE. Also
 	 * take care of keeping r2 (TLS) intact if not a signal
@@ -963,13 +965,10 @@ static int do_setcontext(struct ucontext __user *ucp, struct pt_regs *regs, int
 		if (__get_user(cmcp, &ucp->uc_regs))
 			return -EFAULT;
 		mcp = (struct mcontext __user *)(u64)cmcp;
-		/* no need to check access_ok(mcp), since mcp < 4GB */
 	}
 #else
 	if (__get_user(mcp, &ucp->uc_regs))
 		return -EFAULT;
-	if (!access_ok(mcp, sizeof(*mcp)))
-		return -EFAULT;
 #endif
 	set_current_blocked(&set);
 	if (restore_user_regs(regs, mcp, sig))
@@ -1362,8 +1361,7 @@ SYSCALL_DEFINE0(sigreturn)
 	} else {
 		sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
 		addr = sr;
-		if (!access_ok(sr, sizeof(*sr))
-		    || restore_user_regs(regs, sr, 1))
+		if (restore_user_regs(regs, sr, 1))
 			goto badframe;
 	}
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH 05/10] powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

In the same spirit as commit f1cf4f93de2f ("powerpc/signal32: Remove
ifdefery in middle of if/else")

MSR_TM_ACTIVE() is always defined and returns always 0 when
CONFIG_PPC_TRANSACTIONAL_MEM is not selected, so the awful
ifdefery in the middle of an if/else can be removed.

Make 'msr_hi' a 'long long' to avoid build failure on PPC32
due to the 32 bits left shift.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal_32.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 3b78748d6d85..8dfe4fe77706 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -740,6 +740,12 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 
 	return 0;
 }
+#else
+static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *sr,
+				 struct mcontext __user *tm_sr)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_PPC64
@@ -1317,10 +1323,9 @@ SYSCALL_DEFINE0(sigreturn)
 	struct mcontext __user *sr;
 	void __user *addr;
 	sigset_t set;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-	struct mcontext __user *mcp, *tm_mcp;
-	unsigned long msr_hi;
-#endif
+	struct mcontext __user *mcp;
+	struct mcontext __user *tm_mcp = NULL;
+	unsigned long long msr_hi = 0;
 
 	/* Always make any pending restarted system calls return -EINTR */
 	current->restart_block.fn = do_no_restart_syscall;
@@ -1343,19 +1348,18 @@ SYSCALL_DEFINE0(sigreturn)
 #endif
 	set_current_blocked(&set);
 
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	mcp = (struct mcontext __user *)&sf->mctx;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_mcp = (struct mcontext __user *)&sf->mctx_transact;
 	if (__get_user(msr_hi, &tm_mcp->mc_gregs[PT_MSR]))
 		goto badframe;
+#endif
 	if (MSR_TM_ACTIVE(msr_hi<<32)) {
 		if (!cpu_has_feature(CPU_FTR_TM))
 			goto badframe;
 		if (restore_tm_user_regs(regs, mcp, tm_mcp))
 			goto badframe;
-	} else
-#endif
-	{
+	} else {
 		sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
 		addr = sr;
 		if (!access_ok(sr, sizeof(*sr))
-- 
2.25.0


^ permalink raw reply related

* [PATCH 07/10] powerpc/signal32: Reorder user reads in restore_tm_user_regs()
From: Christophe Leroy @ 2021-03-19 11:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, cmr
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

In restore_tm_user_regs(), regroup the reads from 'sr' and the ones
from 'tm_sr' together in order to allow two block user accesses
in following patch.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/signal_32.c | 49 +++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index e2b1d2a0abad..088c83853026 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -607,8 +607,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 	 * TFHAR is restored from the checkpointed NIP; TEXASR and TFIAR
 	 * were set by the signal delivery.
 	 */
-	err = restore_general_regs(regs, tm_sr);
-	err |= restore_general_regs(&current->thread.ckpt_regs, sr);
+	err = restore_general_regs(&current->thread.ckpt_regs, sr);
 
 	err |= __get_user(current->thread.tm_tfhar, &sr->mc_gregs[PT_NIP]);
 
@@ -624,9 +623,6 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 	if (msr & MSR_VEC) {
 		/* restore altivec registers from the stack */
 		if (__copy_from_user(&current->thread.ckvr_state, &sr->mc_vregs,
-				     sizeof(sr->mc_vregs)) ||
-		    __copy_from_user(&current->thread.vr_state,
-				     &tm_sr->mc_vregs,
 				     sizeof(sr->mc_vregs)))
 			return 1;
 		current->thread.used_vr = true;
@@ -639,9 +635,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 
 	/* Always get VRSAVE back */
 	if (__get_user(current->thread.ckvrsave,
-		       (u32 __user *)&sr->mc_vregs[32]) ||
-	    __get_user(current->thread.vrsave,
-		       (u32 __user *)&tm_sr->mc_vregs[32]))
+		       (u32 __user *)&sr->mc_vregs[32]))
 		return 1;
 	if (cpu_has_feature(CPU_FTR_ALTIVEC))
 		mtspr(SPRN_VRSAVE, current->thread.ckvrsave);
@@ -649,8 +643,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 
 	regs->msr &= ~(MSR_FP | MSR_FE0 | MSR_FE1);
 
-	if (copy_fpr_from_user(current, &sr->mc_fregs) ||
-	    copy_ckfpr_from_user(current, &tm_sr->mc_fregs))
+	if (copy_fpr_from_user(current, &sr->mc_fregs))
 		return 1;
 
 #ifdef CONFIG_VSX
@@ -660,8 +653,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 		 * Restore altivec registers from the stack to a local
 		 * buffer, then write this out to the thread_struct
 		 */
-		if (copy_vsx_from_user(current, &tm_sr->mc_vsregs) ||
-		    copy_ckvsx_from_user(current, &sr->mc_vsregs))
+		if (copy_ckvsx_from_user(current, &sr->mc_vsregs))
 			return 1;
 		current->thread.used_vsr = true;
 	} else if (current->thread.used_vsr)
@@ -690,6 +682,39 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 		return 1;
 #endif /* CONFIG_SPE */
 
+	err = restore_general_regs(regs, tm_sr);
+	if (err)
+		return 1;
+
+#ifdef CONFIG_ALTIVEC
+	/* restore altivec registers from the stack */
+	if (msr & MSR_VEC)
+		if (__copy_from_user(&current->thread.vr_state,
+				     &tm_sr->mc_vregs,
+				     sizeof(sr->mc_vregs)))
+			return 1;
+
+	/* Always get VRSAVE back */
+	if (__get_user(current->thread.vrsave,
+		       (u32 __user *)&tm_sr->mc_vregs[32]))
+		return 1;
+#endif /* CONFIG_ALTIVEC */
+
+	if (copy_ckfpr_from_user(current, &tm_sr->mc_fregs))
+		return 1;
+
+#ifdef CONFIG_VSX
+	if (msr & MSR_VSX) {
+		/*
+		 * Restore altivec registers from the stack to a local
+		 * buffer, then write this out to the thread_struct
+		 */
+		if (copy_vsx_from_user(current, &tm_sr->mc_vsregs))
+			return 1;
+		current->thread.used_vsr = true;
+	}
+#endif /* CONFIG_VSX */
+
 	/* Get the top half of the MSR from the user context */
 	if (__get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR]))
 		return 1;
-- 
2.25.0


^ 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