LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 03/22] powerpc/book3s64/kuap/kuep: Add PPC_PKEY config on book3s64
From: Aneesh Kumar K.V @ 2020-11-27  4:44 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
In-Reply-To: <20201127044424.40686-1-aneesh.kumar@linux.ibm.com>

The config CONFIG_PPC_PKEY is used to select the base support that is
required for PPC_MEM_KEYS, KUAP, and KUEP. Adding this dependency
reduces the code complexity(in terms of #ifdefs) and enables us to
move some of the initialization code to pkeys.c

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 .../powerpc/include/asm/book3s/64/kup-radix.h |  4 ++--
 arch/powerpc/include/asm/book3s/64/mmu.h      |  2 +-
 arch/powerpc/include/asm/ptrace.h             |  7 +++++-
 arch/powerpc/kernel/asm-offsets.c             |  3 +++
 arch/powerpc/mm/book3s64/Makefile             |  2 +-
 arch/powerpc/mm/book3s64/pkeys.c              | 24 ++++++++++++-------
 arch/powerpc/platforms/Kconfig.cputype        |  5 ++++
 7 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 28716e2f13e3..68eaa2fac3ab 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -16,7 +16,7 @@
 #ifdef CONFIG_PPC_KUAP
 	BEGIN_MMU_FTR_SECTION_NESTED(67)
 	mfspr	\gpr1, SPRN_AMR
-	ld	\gpr2, STACK_REGS_KUAP(r1)
+	ld	\gpr2, STACK_REGS_AMR(r1)
 	cmpd	\gpr1, \gpr2
 	beq	998f
 	isync
@@ -48,7 +48,7 @@
 	bne	\msr_pr_cr, 99f
 	.endif
 	mfspr	\gpr1, SPRN_AMR
-	std	\gpr1, STACK_REGS_KUAP(r1)
+	std	\gpr1, STACK_REGS_AMR(r1)
 	li	\gpr2, (AMR_KUAP_BLOCKED >> AMR_KUAP_SHIFT)
 	sldi	\gpr2, \gpr2, AMR_KUAP_SHIFT
 	cmpd	\use_cr, \gpr1, \gpr2
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index e0b52940e43c..a2a015066bae 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -199,7 +199,7 @@ extern int mmu_io_psize;
 void mmu_early_init_devtree(void);
 void hash__early_init_devtree(void);
 void radix__early_init_devtree(void);
-#ifdef CONFIG_PPC_MEM_KEYS
+#ifdef CONFIG_PPC_PKEY
 void pkey_early_init_devtree(void);
 #else
 static inline void pkey_early_init_devtree(void) {}
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e2c778c176a3..e7f1caa007a4 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -53,9 +53,14 @@ struct pt_regs
 #ifdef CONFIG_PPC64
 			unsigned long ppr;
 #endif
+			union {
 #ifdef CONFIG_PPC_KUAP
-			unsigned long kuap;
+				unsigned long kuap;
 #endif
+#ifdef CONFIG_PPC_PKEY
+				unsigned long amr;
+#endif
+			};
 		};
 		unsigned long __pad[2];	/* Maintain 16 byte interrupt stack alignment */
 	};
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index c2722ff36e98..418a0b314a33 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -354,6 +354,9 @@ int main(void)
 	STACK_PT_REGS_OFFSET(_PPR, ppr);
 #endif /* CONFIG_PPC64 */
 
+#ifdef CONFIG_PPC_PKEY
+	STACK_PT_REGS_OFFSET(STACK_REGS_AMR, amr);
+#endif
 #ifdef CONFIG_PPC_KUAP
 	STACK_PT_REGS_OFFSET(STACK_REGS_KUAP, kuap);
 #endif
diff --git a/arch/powerpc/mm/book3s64/Makefile b/arch/powerpc/mm/book3s64/Makefile
index fd393b8be14f..1b56d3af47d4 100644
--- a/arch/powerpc/mm/book3s64/Makefile
+++ b/arch/powerpc/mm/book3s64/Makefile
@@ -17,7 +17,7 @@ endif
 obj-$(CONFIG_TRANSPARENT_HUGEPAGE) += hash_hugepage.o
 obj-$(CONFIG_PPC_SUBPAGE_PROT)	+= subpage_prot.o
 obj-$(CONFIG_SPAPR_TCE_IOMMU)	+= iommu_api.o
-obj-$(CONFIG_PPC_MEM_KEYS)	+= pkeys.o
+obj-$(CONFIG_PPC_PKEY)	+= pkeys.o
 
 # Instrumenting the SLB fault path can lead to duplicate SLB entries
 KCOV_INSTRUMENT_slb.o := n
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index b1d091a97611..7dc71f85683d 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -89,12 +89,14 @@ static int scan_pkey_feature(void)
 		}
 	}
 
+#ifdef CONFIG_PPC_MEM_KEYS
 	/*
 	 * Adjust the upper limit, based on the number of bits supported by
 	 * arch-neutral code.
 	 */
 	pkeys_total = min_t(int, pkeys_total,
 			    ((ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT) + 1));
+#endif
 	return pkeys_total;
 }
 
@@ -102,6 +104,7 @@ void __init pkey_early_init_devtree(void)
 {
 	int pkeys_total, i;
 
+#ifdef CONFIG_PPC_MEM_KEYS
 	/*
 	 * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
 	 * generic defines for PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE.
@@ -117,7 +120,7 @@ void __init pkey_early_init_devtree(void)
 	BUILD_BUG_ON(__builtin_clzl(ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT) +
 		     __builtin_popcountl(ARCH_VM_PKEY_FLAGS >> VM_PKEY_SHIFT)
 				!= (sizeof(u64) * BITS_PER_BYTE));
-
+#endif
 	/*
 	 * Only P7 and above supports SPRN_AMR update with MSR[PR] = 1
 	 */
@@ -223,14 +226,6 @@ void __init pkey_early_init_devtree(void)
 	return;
 }
 
-void pkey_mm_init(struct mm_struct *mm)
-{
-	if (!mmu_has_feature(MMU_FTR_PKEY))
-		return;
-	mm_pkey_allocation_map(mm) = initial_allocation_mask;
-	mm->context.execute_only_pkey = execute_only_key;
-}
-
 static inline u64 read_amr(void)
 {
 	return mfspr(SPRN_AMR);
@@ -257,6 +252,15 @@ static inline void write_iamr(u64 value)
 	mtspr(SPRN_IAMR, value);
 }
 
+#ifdef CONFIG_PPC_MEM_KEYS
+void pkey_mm_init(struct mm_struct *mm)
+{
+	if (!mmu_has_feature(MMU_FTR_PKEY))
+		return;
+	mm_pkey_allocation_map(mm) = initial_allocation_mask;
+	mm->context.execute_only_pkey = execute_only_key;
+}
+
 static inline void init_amr(int pkey, u8 init_bits)
 {
 	u64 new_amr_bits = (((u64)init_bits & 0x3UL) << pkeyshift(pkey));
@@ -445,3 +449,5 @@ void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm)
 	mm_pkey_allocation_map(mm) = mm_pkey_allocation_map(oldmm);
 	mm->context.execute_only_pkey = oldmm->context.execute_only_pkey;
 }
+
+#endif /* CONFIG_PPC_MEM_KEYS */
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index c194c4ae8bc7..f255e8f32155 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -395,6 +395,11 @@ config PPC_KUAP_DEBUG
 	  Add extra debugging for Kernel Userspace Access Protection (KUAP)
 	  If you're unsure, say N.
 
+config PPC_PKEY
+	def_bool y
+	depends on PPC_BOOK3S_64
+	depends on PPC_MEM_KEYS || PPC_KUAP || PPC_KUEP
+
 config ARCH_ENABLE_HUGEPAGE_MIGRATION
 	def_bool y
 	depends on PPC_BOOK3S_64 && HUGETLB_PAGE && MIGRATION
-- 
2.28.0


^ permalink raw reply related

* [PATCH v7 04/22] powerpc/book3s64/kuap/kuep: Move uamor setup to pkey init
From: Aneesh Kumar K.V @ 2020-11-27  4:44 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
In-Reply-To: <20201127044424.40686-1-aneesh.kumar@linux.ibm.com>

This patch consolidates UAMOR update across pkey, kuap and kuep features.
The boot cpu initialize UAMOR via pkey init and both radix/hash do the
secondary cpu UAMOR init in early_init_mmu_secondary.

We don't check for mmu_feature in radix secondary init because UAMOR
is a supported SPRN with all CPUs supporting radix translation.
The old code was not updating UAMOR if we had smap disabled and smep enabled.
This change handles that case.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 3adcf730f478..f5f248d44d5c 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -620,9 +620,6 @@ void setup_kuap(bool disabled)
 		cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
 	}
 
-	/* Make sure userspace can't change the AMR */
-	mtspr(SPRN_UAMOR, 0);
-
 	/*
 	 * Set the default kernel AMR values on all cpus.
 	 */
@@ -721,6 +718,9 @@ void radix__early_init_mmu_secondary(void)
 
 	radix__switch_mmu_context(NULL, &init_mm);
 	tlbiel_all();
+
+	/* Make sure userspace can't change the AMR */
+	mtspr(SPRN_UAMOR, 0);
 }
 
 void radix__mmu_cleanup_all(void)
-- 
2.28.0


^ permalink raw reply related

* [PATCH v7 02/22] KVM: PPC: BOOK3S: PR: Ignore UAMOR SPR
From: Aneesh Kumar K.V @ 2020-11-27  4:44 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
In-Reply-To: <20201127044424.40686-1-aneesh.kumar@linux.ibm.com>

With power7 and above we expect the cpu to support keys. The
number of keys are firmware controlled based on device tree.
PR KVM do not expose key details via device tree. Hence when running with PR KVM
we do run with MMU_FTR_KEY support disabled. But we can still
get updates on UAMOR. Hence ignore access to them and for mfstpr return
0 indicating no AMR/IAMR update is no allowed.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_emulate.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 0effd48c8f4d..b08cc15f31c7 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -840,6 +840,9 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
 	case SPRN_MMCR1:
 	case SPRN_MMCR2:
 	case SPRN_UMMCR2:
+	case SPRN_UAMOR:
+	case SPRN_IAMR:
+	case SPRN_AMR:
 #endif
 		break;
 unprivileged:
@@ -1004,6 +1007,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
 	case SPRN_MMCR2:
 	case SPRN_UMMCR2:
 	case SPRN_TIR:
+	case SPRN_UAMOR:
+	case SPRN_IAMR:
+	case SPRN_AMR:
 #endif
 		*spr_val = 0;
 		break;
-- 
2.28.0


^ permalink raw reply related

* [PATCH v7 00/22] Kernel userspace access/execution prevention with hash translation
From: Aneesh Kumar K.V @ 2020-11-27  4:44 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V

This patch series implements KUAP and KUEP with hash translation mode using
memory keys. The kernel now uses memory protection key 3 to control access
to the kernel. Kernel page table entries are now configured with key 3.
Access to locations configured with any other key value is denied when in
kernel mode (MSR_PR=0). This includes userspace which is by default configured
with key 0.

null-syscall benchmark results:

With smap/smep disabled:
Without patch:
	845.29 ns    2451.44 cycles
With patch series:
	858.38 ns    2489.30 cycles

With smap/smep enabled:
Without patch:
	NA
With patch series:
	1021.51 ns    2962.44 cycles

Changes from v6:
* Address review comments
* Rename MMU FTR defines

Changes from v5:
* Rework the patch based on suggestion from Michael to avoid the
  usage of CONFIG_PPC_PKEY on BOOKE platforms. 

Changes from v4:
* Repost with other pkey related changes split out as a separate series.
* Improve null-syscall benchmark by optimizing SPRN save and restore.

Changes from v3:
* Fix build error reported by kernel test robot <lkp@intel.com>

Changes from v2:
* Rebase to the latest kernel.
* Fixed a bug with disabling KUEP/KUAP on kernel command line
* Added a patch to make kup key dynamic.

Changes from V1:
* Rebased on latest kernel

Aneesh Kumar K.V (22):
  powerpc: Add new macro to handle NESTED_IFCLR
  KVM: PPC: BOOK3S: PR: Ignore UAMOR SPR
  powerpc/book3s64/kuap/kuep: Add PPC_PKEY config on book3s64
  powerpc/book3s64/kuap/kuep: Move uamor setup to pkey init
  powerpc/book3s64/kuap: Move KUAP related function outside radix
  powerpc/book3s64/kuep: Move KUEP related function outside radix
  powerpc/book3s64/kuap: Rename MMU_FTR_RADIX_KUAP and MMU_FTR_KUEP
  powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash
    translation
  powerpc/exec: Set thread.regs early during exec
  powerpc/book3s64/pkeys: Store/restore userspace AMR/IAMR correctly on
    entry and exit from kernel
  powerpc/book3s64/pkeys: Inherit correctly on fork.
  powerpc/book3s64/pkeys: Reset userspace AMR correctly on exec
  powerpc/ptrace-view: Use pt_regs values instead of thread_struct based
    one.
  powerpc/book3s64/pkeys: Don't update SPRN_AMR when in kernel mode.
  powerpc/book3s64/kuap: Restrict access to userspace based on userspace
    AMR
  powerpc/book3s64/kuap: Improve error reporting with KUAP
  powerpc/book3s64/kuap: Use Key 3 to implement KUAP with hash
    translation.
  powerpc/book3s64/kuep: Use Key 3 to implement KUEP with hash
    translation.
  powerpc/book3s64/hash/kuap: Enable kuap on hash
  powerpc/book3s64/hash/kuep: Enable KUEP on hash
  powerpc/book3s64/hash/kup: Don't hardcode kup key
  powerpc/book3s64/pkeys: Optimize KUAP and KUEP feature disabled case

 arch/powerpc/include/asm/book3s/32/kup.h      |   4 +-
 .../powerpc/include/asm/book3s/64/hash-pkey.h |  10 +-
 arch/powerpc/include/asm/book3s/64/hash.h     |   2 +-
 .../powerpc/include/asm/book3s/64/kup-radix.h | 203 --------
 arch/powerpc/include/asm/book3s/64/kup.h      | 442 ++++++++++++++++++
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |   1 +
 arch/powerpc/include/asm/book3s/64/mmu.h      |   2 +-
 arch/powerpc/include/asm/book3s/64/pkeys.h    |   3 +
 arch/powerpc/include/asm/feature-fixups.h     |   3 +
 arch/powerpc/include/asm/kup.h                |   8 +-
 arch/powerpc/include/asm/mmu.h                |  24 +-
 arch/powerpc/include/asm/mmu_context.h        |   2 +-
 arch/powerpc/include/asm/nohash/32/kup-8xx.h  |   4 +-
 arch/powerpc/include/asm/processor.h          |   4 -
 arch/powerpc/include/asm/ptrace.h             |  12 +-
 arch/powerpc/include/asm/thread_info.h        |   2 -
 arch/powerpc/kernel/asm-offsets.c             |   5 +
 arch/powerpc/kernel/entry_64.S                |   6 +-
 arch/powerpc/kernel/exceptions-64s.S          |   4 +-
 arch/powerpc/kernel/process.c                 |  47 +-
 arch/powerpc/kernel/ptrace/ptrace-view.c      |   7 +-
 arch/powerpc/kernel/syscall_64.c              |  38 +-
 arch/powerpc/kernel/traps.c                   |   6 -
 arch/powerpc/kvm/book3s_emulate.c             |   6 +
 arch/powerpc/mm/book3s64/Makefile             |   2 +-
 arch/powerpc/mm/book3s64/hash_4k.c            |   2 +-
 arch/powerpc/mm/book3s64/hash_64k.c           |   4 +-
 arch/powerpc/mm/book3s64/hash_hugepage.c      |   2 +-
 arch/powerpc/mm/book3s64/hash_hugetlbpage.c   |   2 +-
 arch/powerpc/mm/book3s64/hash_pgtable.c       |   2 +-
 arch/powerpc/mm/book3s64/hash_utils.c         |  10 +-
 arch/powerpc/mm/book3s64/pkeys.c              | 177 ++++---
 arch/powerpc/mm/book3s64/radix_pgtable.c      |  45 +-
 arch/powerpc/mm/fault.c                       |   2 +-
 arch/powerpc/platforms/Kconfig.cputype        |   5 +
 35 files changed, 709 insertions(+), 389 deletions(-)
 delete mode 100644 arch/powerpc/include/asm/book3s/64/kup-radix.h
 create mode 100644 arch/powerpc/include/asm/book3s/64/kup.h

-- 
2.28.0


^ permalink raw reply

* [PATCH v7 01/22] powerpc: Add new macro to handle NESTED_IFCLR
From: Aneesh Kumar K.V @ 2020-11-27  4:44 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
In-Reply-To: <20201127044424.40686-1-aneesh.kumar@linux.ibm.com>

This will be used by the following patches

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/asm/feature-fixups.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index fbd406cd6916..5cdba929a8ae 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -100,6 +100,9 @@ label##5:							\
 #define END_MMU_FTR_SECTION_NESTED_IFSET(msk, label)	\
 	END_MMU_FTR_SECTION_NESTED((msk), (msk), label)
 
+#define END_MMU_FTR_SECTION_NESTED_IFCLR(msk, label)	\
+	END_MMU_FTR_SECTION_NESTED((msk), 0, label)
+
 #define END_MMU_FTR_SECTION_IFSET(msk)	END_MMU_FTR_SECTION((msk), (msk))
 #define END_MMU_FTR_SECTION_IFCLR(msk)	END_MMU_FTR_SECTION((msk), 0)
 
-- 
2.28.0


^ permalink raw reply related

* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Bill Wendling @ 2020-11-27  1:10 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev
In-Reply-To: <87ft4vy5jp.fsf@mpe.ellerman.id.au>

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

On Thu, Nov 26, 2020, 5:03 PM Michael Ellerman <mpe@ellerman.id.au> wrote:

> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au>
> wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> >> > <segher@kernel.crashing.org> wrote:
> >> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
> >> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
> >> >> > <segher@kernel.crashing.org> wrote:
> >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> >> > > > <segher@kernel.crashing.org> wrote:
> >> >> > > > > "true" (as a result of a comparison) in as is -1, not 1.
> >> >> > >
> >> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote:
> >> >> > > > What Segher said. :-) Also, if you reverse the comparison,
> you'll get
> >> >> > > > a build error.
> >> >> > >
> >> >> > > But that means your patch is the wrong way around?
> >> >> > >
> >> >> > > -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> >> > > -       .error "Feature section else case larger than body";    \
> >> >> > > -       .endif;                                                 \
> >> >> > > +       .org . - ((label##4b-label##3b) >
> (label##2b-label##1b)); \
> >> >> > >
> >> >> > > It should be a + in that last line, not a -.
> >> >> >
> >> >> > I said so in a follow up email.
> >> >>
> >> >> Yeah, and that arrived a second after I pressed "send" :-)
> >> >>
> >> > Michael, I apologize for the churn with these patches. I believe the
> >> > policy is to resend the match as "v4", correct?
> >> >
> >> > I ran tests with the change above. It compiled with no error. If I
> >> > switch the labels around to ".org . + ((label##2b-label##1b) >
> >> > (label##4b-label##3b))", then it fails as expected.
> >>
> >> I wanted to retain the nicer error reporting for gcc builds, so I did it
> >> like this:
> >>
> >> diff --git a/arch/powerpc/include/asm/feature-fixups.h
> b/arch/powerpc/include/asm/feature-fixups.h
> >> index b0af97add751..c4ad33074df5 100644
> >> --- a/arch/powerpc/include/asm/feature-fixups.h
> >> +++ b/arch/powerpc/include/asm/feature-fixups.h
> >> @@ -36,6 +36,24 @@ label##2:
> \
> >>         .align 2;                                       \
> >>  label##3:
> >>
> >> +
> >> +#ifndef CONFIG_CC_IS_CLANG
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .ifgt (else_size) - (body_size);                        \
> >> +       .error "Feature section else case larger than body";    \
> >> +       .endif;
> >> +#else
> >> +/*
> >> + * If we use the ifgt syntax above, clang's assembler complains about
> the
> >> + * expression being non-absolute when the code appears in an inline
> assembly
> >> + * statement.
> >> + * As a workaround use an .org directive that has no effect if the
> else case
> >> + * instructions are smaller than the body, but fails otherwise.
> >> + */
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .org . + ((else_size) > (body_size));
> >> +#endif
> >> +
> >>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)          \
> >>  label##4:                                                      \
> >>         .popsection;                                            \
> >> @@ -48,9 +66,7 @@ label##5:
>          \
> >>         FTR_ENTRY_OFFSET label##2b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##3b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##4b-label##5b;                   \
> >> -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> -       .error "Feature section else case larger than body";    \
> >> -       .endif;                                                 \
> >> +       CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \
> >>         .popsection;
> >>
> >>
> >>
> >> I've pushed a branch with all your patches applied to:
> >>
> >>   https://github.com/linuxppc/linux/commits/next-test
> >>
> > This works for me. Thanks!
>
> Great.
>
> >> Are you able to give that a quick test? It builds clean with clang for
> >> me, but we must be using different versions of clang because my branch
> >> already builds clean for me even without your patches.
> >>
> > You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That
> > turns on clang's integrated assembler, which I think is disabled by
> > default.
>
> Yep that does it.
>
> But then I get:
>   clang: error: unsupported argument '-mpower4' to option 'Wa,'
>   clang: error: unsupported argument '-many' to option 'Wa,'
>
> So I guess I'm still missing something?
>

I've seen that too. I'm not entirely sure what's causing it, but I'll look
into it. I've got a backlog of things to work on still. :-) It's probably a
clang issue. There's another one that came up having to do with the format
of some PPC instructions. I have a clang fix on review for those.

> Note that with clang's integrated assembler, arch/powerpc/boot/util.S
> > fails to compile. Alan Modra mentioned that he sent you a patch to
> > "modernize" the file so that clang can compile it.
>
> Ah you're right he did, it didn't go to patchwork so I missed it. Have
> grabbed it now.
>

Thanks!
-bw

>

[-- Attachment #2: Type: text/html, Size: 8408 bytes --]

^ permalink raw reply

* [PATCH] powerpc: Allow relative pointers in bug table entries
From: Jordan Niethe @ 2020-11-27  3:02 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Jordan Niethe

This enables GENERIC_BUG_RELATIVE_POINTERS on Power so that 32-bit
offsets are stored in the bug entries rather than 64-bit pointers.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/Kconfig           |  4 ++++
 arch/powerpc/include/asm/bug.h | 37 ++++++++++++++++++++++++++++++++--
 arch/powerpc/xmon/xmon.c       | 17 ++++++++++++++--
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e9f13fe08492..294108e0e5c6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -311,6 +311,10 @@ config GENERIC_BUG
 	default y
 	depends on BUG
 
+config GENERIC_BUG_RELATIVE_POINTERS
+	def_bool y
+	depends on GENERIC_BUG
+
 config SYS_SUPPORTS_APM_EMULATION
 	default y if PMAC_APM_EMU
 	bool
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 338f36cd9934..d03d834042a1 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -12,7 +12,11 @@
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 .macro EMIT_BUG_ENTRY addr,file,line,flags
 	 .section __bug_table,"aw"
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
 5001:	 PPC_LONG \addr, 5002f
+#else
+5001:	 .4byte \addr - 5001b, 5002f - 5001b
+#endif /* CONFIG_GENERIC_BUG_RELATIVE_POINTERS */
 	 .short \line, \flags
 	 .org 5001b+BUG_ENTRY_SIZE
 	 .previous
@@ -23,7 +27,11 @@
 #else
 .macro EMIT_BUG_ENTRY addr,file,line,flags
 	 .section __bug_table,"aw"
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
 5001:	 PPC_LONG \addr
+#else
+5001:	 .4byte \addr - 5001b
+#endif /* CONFIG_GENERIC_BUG_RELATIVE_POINTERS */
 	 .short \flags
 	 .org 5001b+BUG_ENTRY_SIZE
 	 .previous
@@ -34,20 +42,45 @@
 /* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
    sizeof(struct bug_entry), respectively */
 #ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
 #define _EMIT_BUG_ENTRY				\
 	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b, %0\n"		\
 	"\t.short %1, %2\n"			\
 	".org 2b+%3\n"				\
 	".previous\n"
-#else
+
+#else /* relative pointers */
+
+#define _EMIT_BUG_ENTRY				\
+	".section __bug_table,\"aw\"\n"		\
+	"2:\t.4byte 1b - 2b, %0 - 2b\n"		\
+	"\t.short %1, %2\n"			\
+	".org 2b+%3\n"				\
+	".previous\n"
+#endif /* relative pointers */
+
+#else /* verbose */
+
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
 #define _EMIT_BUG_ENTRY				\
 	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b\n"			\
 	"\t.short %2\n"				\
 	".org 2b+%3\n"				\
 	".previous\n"
-#endif
+
+#else /* relative pointers */
+
+#define _EMIT_BUG_ENTRY				\
+	".section __bug_table,\"aw\"\n"		\
+	"2:\t.4byte 1b - 2b\n"		\
+	"\t.short %2\n"				\
+	".org 2b+%3\n"				\
+	".previous\n"
+
+#endif /* relative pointers */
+#endif /* verbose */
 
 #define BUG_ENTRY(insn, flags, ...)			\
 	__asm__ __volatile__(				\
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 55c43a6c9111..5f7cf7e95767 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1731,6 +1731,9 @@ static void print_bug_trap(struct pt_regs *regs)
 #ifdef CONFIG_BUG
 	const struct bug_entry *bug;
 	unsigned long addr;
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+	char *file;
+#endif
 
 	if (regs->msr & MSR_PR)
 		return;		/* not in kernel */
@@ -1744,10 +1747,20 @@ static void print_bug_trap(struct pt_regs *regs)
 		return;
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
+	file = bug->file;
+#else /* relative pointers */
+	file = (char *)bug + bug->file_disp;
+#endif /* relative pointers */
 	printf("kernel BUG at %s:%u!\n",
-	       bug->file, bug->line);
+	       file, bug->line);
 #else
-	printf("kernel BUG at %px!\n", (void *)bug->bug_addr);
+#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
+	addr = bug->addr;
+#else /* relative pointers */
+	addr = (unsigned long)bug + bug->bug_addr_disp;
+#endif /* relative pointers */
+	printf("kernel BUG at %px!\n", (void *)addr);
 #endif
 #endif /* CONFIG_BUG */
 }
-- 
2.17.1


^ permalink raw reply related

* [Bug 209733] Starting new KVM virtual machines on PPC64 starts to hang after box is up for a while
From: bugzilla-daemon @ 2020-11-27  2:26 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-209733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=209733

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #6 from Michael Ellerman (michael@ellerman.id.au) ---
Nick pointed out that it was actually:
  2da9f6305f30 ("mm/vmscan: fix NR_ISOLATED_FILE corruption on 64-bit")

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* powerpc 5.10-rcN boot failures with RCU_SCALE_TEST=m
From: Daniel Axtens @ 2020-11-27  2:02 UTC (permalink / raw)
  To: rcu, linuxppc-dev

Hi all,

I'm having some difficulty tracking down a bug.

Some configurations of the powerpc kernel since somewhere in the 5.10
merge window fail to boot on some ppc64 systems. They hang while trying
to bring up SMP. It seems to depend on the RCU_SCALE/PERF_TEST option.
(It was renamed in the 5.10 merge window.)

I can reproduce it as follows with qemu tcg:

make -j64 pseries_le_defconfig
scripts/config -m RCU_SCALE_TEST
scripts/config -m RCU_PERF_TEST
make -j 64 vmlinux CC="ccache gcc"

qemu-system-ppc64 -cpu power9 -M pseries -m 1G -nographic -vga none -smp 4 -kernel vmlinux

...
[    0.036284][    T0] Mount-cache hash table entries: 8192 (order: 0, 65536 bytes, linear)
[    0.036481][    T0] Mountpoint-cache hash table entries: 8192 (order: 0, 65536 bytes, linear)
[    0.148168][    T1] POWER9 performance monitor hardware support registered
[    0.151118][    T1] rcu: Hierarchical SRCU implementation.
[    0.186660][    T1] smp: Bringing up secondary CPUs ...
<hangs>

I have no idea why RCU_SCALE/PERF_TEST would be causing this, but that
seems to be what does it: if I don't set that, the kernel boots fine.

I've tried to git bisect it, but I keep getting different results:
always a random merge of a seemingly-unrelated subsystem tree - things
like armsoc or integrity or input.

It appears to also depend on the way the kernel is booted. Testing with
a Canonical kernel, so a slightly different config but including
RCU_SCALE_TEST=m, I see:

Power8 host + KVM + grub         -> boots
Power9 host bare metal (kexec)   -> fails
Power9 host + KVM + grub         -> fails
Power9 host + KVM + qemu -kernel -> boots
qemu TCG + power9 cpu            -> fails
qemu TCG + power8 cpu            -> fails

Any ideas?

Kind regards,
Daniel

$ qemu-system-ppc64 -version
QEMU emulator version 4.2.1 (Debian 1:4.2-3ubuntu6.9)

$ gcc --version
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

It also happens when compiling with GCC 7 and 10.



^ permalink raw reply

* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Bill Wendling @ 2020-11-27  1:59 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev
In-Reply-To: <87ft4vy5jp.fsf@mpe.ellerman.id.au>

On Thu, Nov 26, 2020 at 5:03 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> >> > <segher@kernel.crashing.org> wrote:
> >> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
> >> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
> >> >> > <segher@kernel.crashing.org> wrote:
> >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> >> > > > <segher@kernel.crashing.org> wrote:
> >> >> > > > > "true" (as a result of a comparison) in as is -1, not 1.
> >> >> > >
> >> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote:
> >> >> > > > What Segher said. :-) Also, if you reverse the comparison, you'll get
> >> >> > > > a build error.
> >> >> > >
> >> >> > > But that means your patch is the wrong way around?
> >> >> > >
> >> >> > > -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> >> > > -       .error "Feature section else case larger than body";    \
> >> >> > > -       .endif;                                                 \
> >> >> > > +       .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
> >> >> > >
> >> >> > > It should be a + in that last line, not a -.
> >> >> >
> >> >> > I said so in a follow up email.
> >> >>
> >> >> Yeah, and that arrived a second after I pressed "send" :-)
> >> >>
> >> > Michael, I apologize for the churn with these patches. I believe the
> >> > policy is to resend the match as "v4", correct?
> >> >
> >> > I ran tests with the change above. It compiled with no error. If I
> >> > switch the labels around to ".org . + ((label##2b-label##1b) >
> >> > (label##4b-label##3b))", then it fails as expected.
> >>
> >> I wanted to retain the nicer error reporting for gcc builds, so I did it
> >> like this:
> >>
> >> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
> >> index b0af97add751..c4ad33074df5 100644
> >> --- a/arch/powerpc/include/asm/feature-fixups.h
> >> +++ b/arch/powerpc/include/asm/feature-fixups.h
> >> @@ -36,6 +36,24 @@ label##2:                                            \
> >>         .align 2;                                       \
> >>  label##3:
> >>
> >> +
> >> +#ifndef CONFIG_CC_IS_CLANG
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .ifgt (else_size) - (body_size);                        \
> >> +       .error "Feature section else case larger than body";    \
> >> +       .endif;
> >> +#else
> >> +/*
> >> + * If we use the ifgt syntax above, clang's assembler complains about the
> >> + * expression being non-absolute when the code appears in an inline assembly
> >> + * statement.
> >> + * As a workaround use an .org directive that has no effect if the else case
> >> + * instructions are smaller than the body, but fails otherwise.
> >> + */
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .org . + ((else_size) > (body_size));
> >> +#endif
> >> +
> >>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)          \
> >>  label##4:                                                      \
> >>         .popsection;                                            \
> >> @@ -48,9 +66,7 @@ label##5:                                                     \
> >>         FTR_ENTRY_OFFSET label##2b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##3b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##4b-label##5b;                   \
> >> -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> -       .error "Feature section else case larger than body";    \
> >> -       .endif;                                                 \
> >> +       CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \
> >>         .popsection;
> >>
> >>
> >>
> >> I've pushed a branch with all your patches applied to:
> >>
> >>   https://github.com/linuxppc/linux/commits/next-test
> >>
> > This works for me. Thanks!
>
> Great.
>
> >> Are you able to give that a quick test? It builds clean with clang for
> >> me, but we must be using different versions of clang because my branch
> >> already builds clean for me even without your patches.
> >>
> > You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That
> > turns on clang's integrated assembler, which I think is disabled by
> > default.
>
> Yep that does it.
>
> But then I get:
>   clang: error: unsupported argument '-mpower4' to option 'Wa,'
>   clang: error: unsupported argument '-many' to option 'Wa,'
>
> So I guess I'm still missing something?
>
[Resent, because my previous email went out as non-plain text.]

I've seen that too. I'm not entirely sure what's causing it, but I'll
look into it. I've got a backlog of things to work on still. :-) It's
probably a clang issue. There's another one that came up having to do
with the format of some PPC instructions. I have a clang fix on review
for those.

> > Note that with clang's integrated assembler, arch/powerpc/boot/util.S
> > fails to compile. Alan Modra mentioned that he sent you a patch to
> > "modernize" the file so that clang can compile it.
>
> Ah you're right he did, it didn't go to patchwork so I missed it. Have
> grabbed it now.
>
Thanks!

-bw

^ permalink raw reply

* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Michael Ellerman @ 2020-11-27  1:03 UTC (permalink / raw)
  To: Bill Wendling; +Cc: Nick Desaulniers, linuxppc-dev
In-Reply-To: <CAGG=3QUSF4UwcZQHhFE-PW6As7GVJknsyGkgVMENDXghABzy5A@mail.gmail.com>

Bill Wendling <morbo@google.com> writes:
> On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Bill Wendling <morbo@google.com> writes:
>> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
>> > <segher@kernel.crashing.org> wrote:
>> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
>> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
>> >> > <segher@kernel.crashing.org> wrote:
>> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
>> >> > > > <segher@kernel.crashing.org> wrote:
>> >> > > > > "true" (as a result of a comparison) in as is -1, not 1.
>> >> > >
>> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote:
>> >> > > > What Segher said. :-) Also, if you reverse the comparison, you'll get
>> >> > > > a build error.
>> >> > >
>> >> > > But that means your patch is the wrong way around?
>> >> > >
>> >> > > -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
>> >> > > -       .error "Feature section else case larger than body";    \
>> >> > > -       .endif;                                                 \
>> >> > > +       .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
>> >> > >
>> >> > > It should be a + in that last line, not a -.
>> >> >
>> >> > I said so in a follow up email.
>> >>
>> >> Yeah, and that arrived a second after I pressed "send" :-)
>> >>
>> > Michael, I apologize for the churn with these patches. I believe the
>> > policy is to resend the match as "v4", correct?
>> >
>> > I ran tests with the change above. It compiled with no error. If I
>> > switch the labels around to ".org . + ((label##2b-label##1b) >
>> > (label##4b-label##3b))", then it fails as expected.
>>
>> I wanted to retain the nicer error reporting for gcc builds, so I did it
>> like this:
>>
>> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
>> index b0af97add751..c4ad33074df5 100644
>> --- a/arch/powerpc/include/asm/feature-fixups.h
>> +++ b/arch/powerpc/include/asm/feature-fixups.h
>> @@ -36,6 +36,24 @@ label##2:                                            \
>>         .align 2;                                       \
>>  label##3:
>>
>> +
>> +#ifndef CONFIG_CC_IS_CLANG
>> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
>> +       .ifgt (else_size) - (body_size);                        \
>> +       .error "Feature section else case larger than body";    \
>> +       .endif;
>> +#else
>> +/*
>> + * If we use the ifgt syntax above, clang's assembler complains about the
>> + * expression being non-absolute when the code appears in an inline assembly
>> + * statement.
>> + * As a workaround use an .org directive that has no effect if the else case
>> + * instructions are smaller than the body, but fails otherwise.
>> + */
>> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
>> +       .org . + ((else_size) > (body_size));
>> +#endif
>> +
>>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)          \
>>  label##4:                                                      \
>>         .popsection;                                            \
>> @@ -48,9 +66,7 @@ label##5:                                                     \
>>         FTR_ENTRY_OFFSET label##2b-label##5b;                   \
>>         FTR_ENTRY_OFFSET label##3b-label##5b;                   \
>>         FTR_ENTRY_OFFSET label##4b-label##5b;                   \
>> -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
>> -       .error "Feature section else case larger than body";    \
>> -       .endif;                                                 \
>> +       CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \
>>         .popsection;
>>
>>
>>
>> I've pushed a branch with all your patches applied to:
>>
>>   https://github.com/linuxppc/linux/commits/next-test
>>
> This works for me. Thanks!

Great.

>> Are you able to give that a quick test? It builds clean with clang for
>> me, but we must be using different versions of clang because my branch
>> already builds clean for me even without your patches.
>>
> You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That
> turns on clang's integrated assembler, which I think is disabled by
> default.

Yep that does it.

But then I get:
  clang: error: unsupported argument '-mpower4' to option 'Wa,'
  clang: error: unsupported argument '-many' to option 'Wa,'

So I guess I'm still missing something?

> Note that with clang's integrated assembler, arch/powerpc/boot/util.S
> fails to compile. Alan Modra mentioned that he sent you a patch to
> "modernize" the file so that clang can compile it.

Ah you're right he did, it didn't go to patchwork so I missed it. Have
grabbed it now.

cheers

^ permalink raw reply

* Re: [PATCH net 1/2] ibmvnic: Ensure that SCRQ entry reads are correctly ordered
From: Michael Ellerman @ 2020-11-26 23:57 UTC (permalink / raw)
  To: Thomas Falcon, netdev
  Cc: cforno12, ljp, ricklind, dnbanerg, drt, brking, sukadev,
	linuxppc-dev
In-Reply-To: <2da3e517-f1dd-95c9-11db-a6c62bf61978@linux.ibm.com>

Thomas Falcon <tlfalcon@linux.ibm.com> writes:
> On 11/24/20 11:43 PM, Michael Ellerman wrote:
>> Thomas Falcon <tlfalcon@linux.ibm.com> writes:
>>> Ensure that received Subordinate Command-Response Queue (SCRQ)
>>> entries are properly read in order by the driver. These queues
>>> are used in the ibmvnic device to process RX buffer and TX completion
>>> descriptors. dma_rmb barriers have been added after checking for a
>>> pending descriptor to ensure the correct descriptor entry is checked
>>> and after reading the SCRQ descriptor to ensure the entire
>>> descriptor is read before processing.
>>>
>>> Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
>>> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
>>> ---
>>>   drivers/net/ethernet/ibm/ibmvnic.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>>> index 2aa40b2..489ed5e 100644
>>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>>> @@ -2403,6 +2403,8 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
>>>   
>>>   		if (!pending_scrq(adapter, adapter->rx_scrq[scrq_num]))
>>>   			break;
>>> +		/* ensure that we do not prematurely exit the polling loop */
>>> +		dma_rmb();
>> I'd be happier if these comments were more specific about which read(s)
>> they are ordering vs which other read(s).
>>
>> I'm sure it's obvious to you, but it may not be to a future author,
>> and/or after the code has been refactored over time.
>
> Thank you for reviewing! I will submit a v2 soon with clearer comments 
> on the reads being ordered here.

Thanks.

cheers

^ permalink raw reply

* [Bug 209733] Starting new KVM virtual machines on PPC64 starts to hang after box is up for a while
From: bugzilla-daemon @ 2020-11-26 23:16 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-209733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=209733

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |michael@ellerman.id.au

--- Comment #5 from Michael Ellerman (michael@ellerman.id.au) ---
Thanks for persisting with the testing.

I wonder if it was fixed by:

c4629e4e7e09 ("mm/compaction: stop isolation if too many pages are isolated and
we have pages to migrate")
or
38935861d85a ("mm/compaction: count pages and stop correctly during page
isolation")

They fix a potential infinte loop in a path that's used by the HTAB allocation.

Those landed in v5.9.9, and fix a commit that was introduced in v5.7 (which
doesn't match your observation that v5.7.x was OK).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH v2 00/19] Add generic vdso_base tracking
From: Dmitry Safonov @ 2020-11-26 20:41 UTC (permalink / raw)
  To: Christophe Leroy, linux-kernel
  Cc: Thomas Bogendoerfer, Arnd Bergmann, Catalin Marinas, x86,
	Dmitry Safonov, Oleg Nesterov, Russell King,
	linuxppc-dev@lists.ozlabs.org, Ingo Molnar, Borislav Petkov,
	Alexander Viro, Andy Lutomirski, H. Peter Anvin, Guo Ren,
	Andrew Morton, Vincenzo Frascino, Will Deacon, Thomas Gleixner
In-Reply-To: <5e315bf6-b03d-e66e-9557-22ece397080e@csgroup.eu>

Hi Christophe,

On 11/24/20 6:53 AM, Christophe Leroy wrote:
> 
> 
> Le 24/11/2020 à 01:29, Dmitry Safonov a écrit :
>> v2 Changes:
>> - Rename user_landing to vdso_base as it tracks vDSO VMA start address,
>>    rather than the explicit address to land (Andy)
>> - Reword and don't use "new-execed" and "new-born" task (Andy)
>> - Fix failures reported by build robot
>>
>> Started from discussion [1], where was noted that currently a couple of
>> architectures support mremap() for vdso/sigpage, but not munmap().
>> If an application maps something on the ex-place of vdso/sigpage,
>> later after processing signal it will land there (good luck!)
>>
>> Patches set is based on linux-next (next-20201123) and it depends on
>> changes in x86/cleanups (those reclaim TIF_IA32/TIF_X32) and also
>> on my changes in akpm (fixing several mremap() issues).
> 
> I have a series that cleans up VDSO init on powerpc and migrates powerpc
> to _install_special_mapping() (patch 10 of the series).
> 
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=204396&state=%2A&archive=both
> 
> 
> I'm wondering how we should coordinate with your series for merging.
> 
> I guess your series will also imply removal of arch_unmap() ? see
> https://elixir.bootlin.com/linux/v5.10-rc4/source/arch/powerpc/include/asm/mmu_context.h#L262

I think our series intersect only in that moment where I re-introduce
arch_setup_additional_pages() parameters. So, in theory we could
minimize the conflicts by merging both series in parallel and cleanup
the result by moving to generic vdso_base on the top, what do you think?

Thanks,
          Dmitry

^ permalink raw reply

* Re: [PATCH] ASoC: fsl: Fix config name of CONFIG_ARCH_MXC
From: Mark Brown @ 2020-11-26 20:05 UTC (permalink / raw)
  To: tiwai, timur, perex, festevam, Xiubo.Lee, Shengjiu Wang,
	alsa-devel, nicoleotsuka
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1606371293-29099-1-git-send-email-shengjiu.wang@nxp.com>

On Thu, 26 Nov 2020 14:14:53 +0800, Shengjiu Wang wrote:
> CONFIG_ARCH_MXC should be ARCH_MXC

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: fsl: Fix config name of CONFIG_ARCH_MXC
      commit: c61d1142cfd45f58b63bf9d2d59523f91096e873

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* [RFC PATCH 14/14] samples/ftrace: Add powerpc support for ftrace direct samples
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add a simple powerpc trampoline to demonstrate use of ftrace direct on
powerpc.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/Kconfig                       |  2 +-
 samples/ftrace/ftrace-direct-modify.c | 58 +++++++++++++++++++++++++++
 samples/ftrace/ftrace-direct-too.c    | 48 ++++++++++++++++++++--
 samples/ftrace/ftrace-direct.c        | 45 +++++++++++++++++++--
 4 files changed, 144 insertions(+), 9 deletions(-)

diff --git a/samples/Kconfig b/samples/Kconfig
index 0ed6e4d71d87b1..fdc9e44dba3b95 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -26,7 +26,7 @@ config SAMPLE_TRACE_PRINTK
 config SAMPLE_FTRACE_DIRECT
 	tristate "Build register_ftrace_direct() example"
 	depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS && m
-	depends on X86_64 # has x86_64 inlined asm
+	depends on X86_64 || PPC64 # has inlined asm
 	help
 	  This builds an ftrace direct function example
 	  that hooks to wake_up_process and prints the parameters.
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index c13a5bc5095bea..89d66a12d300e1 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func1(void)
 {
@@ -18,6 +19,7 @@ extern void my_tramp2(void *);
 
 static unsigned long my_ip = (unsigned long)schedule;
 
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp1, @function\n"
@@ -38,6 +40,58 @@ asm (
 "	.size		my_tramp2, .-my_tramp2\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp1, @function\n"
+"	.global		my_tramp1\n"
+"   my_tramp1:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (3f - 1b)(12)\n"
+"	bl	my_direct_func1\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp1, .-my_tramp1\n"
+"	nop\n"
+"	.type		my_tramp2, @function\n"
+"	.global		my_tramp2\n"
+"   my_tramp2:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 2f\n"
+"2:	mflr	12\n"
+"	ld	2, (3f - 2b)(12)\n"
+"	bl	my_direct_func2\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp2, .-my_tramp2\n"
+"3:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
+
 
 static unsigned long my_tramp = (unsigned long)my_tramp1;
 static unsigned long tramps[2] = {
@@ -72,6 +126,10 @@ static int __init ftrace_direct_init(void)
 {
 	int ret;
 
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+
 	ret = register_ftrace_direct(my_ip, my_tramp);
 	if (!ret)
 		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index d5c5022be66429..9a82abecbe0dcc 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -3,6 +3,7 @@
 
 #include <linux/mm.h> /* for handle_mm_fault() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func(struct vm_area_struct *vma,
 			unsigned long address, unsigned int flags)
@@ -13,6 +14,9 @@ void my_direct_func(struct vm_area_struct *vma,
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)handle_mm_fault;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -31,18 +35,54 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	std	4, 144(1)\n"
+"	std	5, 152(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	5, 152(1)\n"
+"	ld	4, 144(1)\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)handle_mm_fault,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)handle_mm_fault,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index 63ca06d42c803f..da67b6217f91d2 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h> /* for wake_up_process() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h> /* for ppc_function_entry() */
 
 void my_direct_func(struct task_struct *p)
 {
@@ -11,6 +12,9 @@ void my_direct_func(struct task_struct *p)
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)wake_up_process;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -25,18 +29,51 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)wake_up_process,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	/* Ftrace location is (usually) the second instruction at a function's local entry point */
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)wake_up_process,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 11/14] powerpc/ftrace: Use GPR save/restore macros in ftrace_graph_caller()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Use SAVE_8GPRS(), REST_8GPRS() and _NIP(), along with using the standard
SWITCH_FRAME_SIZE for the stack frame in ftrace_graph_caller() to
simplify code. This increases the stack frame size, but it is unlikely
to be an issue since ftrace_[regs_]caller() have just used a similar
stack frame size, and it isn't evident that the graph caller has too
deep a call stack to cause issues.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 .../powerpc/kernel/trace/ftrace_64_mprofile.S | 28 +++++--------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index f9fd5f743eba34..bbe871b47ade58 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -279,24 +279,17 @@ livepatch_handler:
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 _GLOBAL(ftrace_graph_caller)
-	stdu	r1, -112(r1)
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
-	std	r10, 104(r1)
-	std	r9, 96(r1)
-	std	r8, 88(r1)
-	std	r7, 80(r1)
-	std	r6, 72(r1)
-	std	r5, 64(r1)
-	std	r4, 56(r1)
-	std	r3, 48(r1)
+	SAVE_8GPRS(3, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
 	ld	r2, PACATOC(r13)	/* get kernel TOC in r2 */
 
-	addi	r5, r1, 112
+	addi	r5, r1, SWITCH_FRAME_SIZE
 	mfctr	r4		/* ftrace_caller has moved local addr here */
-	std	r4, 40(r1)
+	std	r4, _NIP(r1)
 	mflr	r3		/* ftrace_caller has restored LR from stack */
 	subi	r4, r4, MCOUNT_INSN_SIZE
 
@@ -309,21 +302,14 @@ _GLOBAL(ftrace_graph_caller)
 	 */
 	mtlr	r3
 
-	ld	r0, 40(r1)
+	ld	r0, _NIP(r1)
 	mtctr	r0
-	ld	r10, 104(r1)
-	ld	r9, 96(r1)
-	ld	r8, 88(r1)
-	ld	r7, 80(r1)
-	ld	r6, 72(r1)
-	ld	r5, 64(r1)
-	ld	r4, 56(r1)
-	ld	r3, 48(r1)
+	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
-	addi	r1, r1, 112
+	addi	r1, r1, SWITCH_FRAME_SIZE
 	mflr	r0
 	std	r0, LRSAVE(r1)
 	bctr
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 09/14] powerpc/ftrace: Use a hash table for tracking ftrace stubs
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

In preparation for having to deal with large number of ftrace stubs in
support of ftrace direct calls, convert existing stubs to use a hash
table. The hash table is key'ed off the target address for the stubs
since there could be multiple stubs for the same target to cover the
full kernel text.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 75 +++++++++++++-----------------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 14b39f7797d455..7ddb6e4b527c39 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) "ftrace-powerpc: " fmt
 
+#include <linux/hashtable.h>
 #include <linux/spinlock.h>
 #include <linux/hardirq.h>
 #include <linux/uaccess.h>
@@ -32,14 +33,12 @@
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
-/*
- * We generally only have a single long_branch tramp and at most 2 or 3 plt
- * tramps generated. But, we don't use the plt tramps currently. We also allot
- * 2 tramps after .text and .init.text. So, we only end up with around 3 usable
- * tramps in total. Set aside 8 just to be sure.
- */
-#define	NUM_FTRACE_TRAMPS	8
-static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
+static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
+struct ppc_ftrace_stub_data {
+	unsigned long addr;
+	unsigned long target;
+	struct hlist_node hentry;
+};
 
 static struct ppc_inst
 ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
@@ -288,36 +287,31 @@ __ftrace_make_nop(struct module *mod,
 #endif /* PPC64 */
 #endif /* CONFIG_MODULES */
 
-static unsigned long find_ftrace_tramp(unsigned long ip)
+static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 	struct ppc_inst instr;
 
-	/*
-	 * We have the compiler generated long_branch tramps at the end
-	 * and we prefer those
-	 */
-	for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
-		if (!ftrace_tramps[i])
-			continue;
-		else if (create_branch(&instr, (void *)ip,
-				       ftrace_tramps[i], 0) == 0)
-			return ftrace_tramps[i];
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target)
+		if (stub->target == target && !create_branch(&instr, (void *)ip, stub->addr, 0))
+			return stub->addr;
 
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i]) {
-			ftrace_tramps[i] = tramp;
-			return 0;
-		}
+	stub = kmalloc(sizeof(*stub), GFP_KERNEL);
+	if (!stub)
+		return -1;
 
-	return -1;
+	stub->addr = tramp;
+	stub->target = target;
+	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
+
+	return 0;
 }
 
 /*
@@ -328,16 +322,14 @@ static int add_ftrace_tramp(unsigned long tramp)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
-	int i;
 	struct ppc_inst op;
-	unsigned long ptr;
 	struct ppc_inst instr;
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
 	/* Is this a known long jump tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i])
-			break;
-		else if (ftrace_tramps[i] == tramp)
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
+		if (stub->target == ftrace_target && stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -361,19 +353,18 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
-	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
+	if (create_branch(&instr, (void *)tramp, ftrace_target, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
-				(void *)ptr);
+				(void *)ftrace_target);
 		return -1;
 	}
 
-	if (patch_branch((struct ppc_inst *)tramp, ptr, 0)) {
+	if (patch_branch((struct ppc_inst *)tramp, ftrace_target, 0)) {
 		pr_debug("REL24 out of range!\n");
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp)) {
+	if (add_ftrace_tramp(tramp, ftrace_target)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -405,7 +396,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 
 	if (setup_mcount_compiler_tramp(tramp)) {
 		/* Are other trampolines reachable? */
-		if (!find_ftrace_tramp(ip)) {
+		if (!find_ftrace_tramp(ip, FTRACE_REGS_ADDR)) {
 			pr_err("No ftrace trampolines reachable from %ps\n",
 					(void *)ip);
 			return -EINVAL;
@@ -646,7 +637,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip);
+	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -894,7 +885,7 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i]);
+		add_ftrace_tramp((unsigned long)tramp[i], addr);
 	}
 
 	return 0;
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 13/14] powerpc/ftrace: Add support for register_ftrace_direct() for MPROFILE_KERNEL
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add support for register_ftrace_direct() for MPROFILE_KERNEL, as it
depends on DYNAMIC_FTRACE_WITH_REGS.

Since powerpc only provides a branch range of 32MB, we set aside a 64k
area within kernel text for creating stubs that can be used to branch to
the provided trampoline, which can be located in the module area. This
is limited to kernel text, and as such, ftrace direct calls are not
supported for functions in kernel modules at this time.

We use orig_gpr3 to stash the address of the direct call trampoline in
arch_ftrace_set_direct_caller().  ftrace_regs_caller() is updated to
check for this to determine if we need to redirect to a direct call
trampoline. As the direct call trampoline has to work as an alternative
for the ftrace trampoline, we setup LR and r0 appropriately, and update
ctr to the trampoline address.  Finally, ftrace_graph_caller() is
updated to save/restore r0.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                          |   1 +
 arch/powerpc/include/asm/ftrace.h             |  14 ++
 arch/powerpc/kernel/trace/ftrace.c            | 140 +++++++++++++++++-
 .../powerpc/kernel/trace/ftrace_64_mprofile.S |  40 ++++-
 4 files changed, 182 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index cfc6dd787f532c..a87ac2e403196e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -197,6 +197,7 @@ config PPC
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_DYNAMIC_FTRACE
+	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS	if MPROFILE_KERNEL
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS	if MPROFILE_KERNEL
 	select HAVE_EBPF_JIT			if PPC64
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index bc76970b6ee532..2f1c46e9f5d416 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -10,6 +10,8 @@
 
 #define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
 
+#define FTRACE_STUBS_SIZE	65536
+
 #ifdef __ASSEMBLY__
 
 /* Based off of objdump optput from glibc */
@@ -59,6 +61,18 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 struct dyn_arch_ftrace {
 	struct module *mod;
 };
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+/*
+ * When there is a direct caller registered, we use regs->orig_gpr3 (similar to
+ * how x86 uses orig_ax) to let ftrace_{regs_}_caller know that we should go
+ * there instead of returning to the function
+ */
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+	regs->orig_gpr3 = addr;
+}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index fcb21a9756e456..815b14ae45a71f 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -37,6 +37,7 @@ static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
 struct ppc_ftrace_stub_data {
 	unsigned long addr;
 	unsigned long target;
+	refcount_t refs;
 	struct hlist_node hentry;
 };
 
@@ -299,7 +300,7 @@ static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target, int lock)
 {
 	struct ppc_ftrace_stub_data *stub;
 
@@ -309,11 +310,123 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 
 	stub->addr = tramp;
 	stub->target = target;
+	refcount_set(&stub->refs, 1);
+	if (lock)
+		refcount_inc(&stub->refs);
 	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
 
 	return 0;
 }
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+static u32 ftrace_direct_stub_insns[] = {
+	PPC_RAW_LIS(12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_SLDI(12, 12, 32),
+	PPC_RAW_ORIS(12, 12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_MTCTR(12),
+	PPC_RAW_BCTR(),
+};
+#define FTRACE_NUM_STUBS	(FTRACE_STUBS_SIZE / sizeof(ftrace_direct_stub_insns))
+static DECLARE_BITMAP(stubs_bitmap, FTRACE_NUM_STUBS);
+extern unsigned int ftrace_stubs[];
+
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	struct ppc_ftrace_stub_data *stub_data;
+	struct ppc_inst instr;
+	unsigned int *stub;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub_data, hentry, target) {
+		if (stub_data->target == target &&
+				!create_branch(&instr, (void *)ip, stub_data->addr, 0)) {
+			refcount_inc(&stub_data->refs);
+			return stub_data->addr;
+		}
+	}
+
+	/* Allocate a stub */
+	do {
+		index = find_first_zero_bit(stubs_bitmap, FTRACE_NUM_STUBS);
+		if (index >= FTRACE_NUM_STUBS) {
+			pr_err("No stubs available\n");
+			return 0;
+		}
+	} while (test_and_set_bit(index, stubs_bitmap));
+	stub = &ftrace_stubs[index * sizeof(ftrace_direct_stub_insns) / 4];
+
+	if (create_branch(&instr, (void *)ip, (unsigned long)stub, 0)) {
+		/* Stub is not reachable from the ftrace location */
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	memcpy(stub, ftrace_direct_stub_insns, sizeof(ftrace_direct_stub_insns));
+	stub[0] |= IMM_L(target >> 48);
+	stub[1] |= IMM_L(target >> 32);
+	stub[3] |= IMM_L(target >> 16);
+	stub[4] |= IMM_L(target);
+	if (add_ftrace_tramp((unsigned long)stub, target, 0)) {
+		pr_err("Error allocating ftrace stub");
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	return (unsigned long)stub;
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr)
+{
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long tramp = 0;
+	struct ppc_inst instr;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target) {
+		if (stub->target == target && stub->addr == stub_addr &&
+				!create_branch(&instr, (void *)ip, stub->addr, 0)) {
+			if (refcount_dec_and_test(&stub->refs)) {
+				tramp = stub->addr;
+				hash_del(&stub->hentry);
+				kfree(stub);
+				break;
+			}
+			return;
+		}
+	}
+
+	if (tramp) {
+		synchronize_rcu_tasks();
+		index = (tramp - (unsigned long)ftrace_stubs) / sizeof(ftrace_direct_stub_insns);
+		clear_bit(index, stubs_bitmap);
+	}
+}
+
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	if (addr & 0x03) {
+		pr_err("Target address is not at instruction boundary: 0x%lx\n", addr);
+		return -EINVAL;
+	}
+
+	if (is_module_text_address(ip)) {
+		pr_err("Kernel modules are not supported for direct calls\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#else
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	return find_ftrace_tramp(ip, target);
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr) { }
+#endif
+
 /*
  * If this is a compiler generated long_branch trampoline (essentially, a
  * trampoline that has a branch to _mcount()), we re-write the branch to
@@ -365,7 +478,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp, ftrace_target)) {
+	if (add_ftrace_tramp(tramp, ftrace_target, 1)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -409,6 +522,8 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EPERM;
 	}
 
+	remove_ftrace_tramp(ip, addr, tramp);
+
 	return 0;
 }
 
@@ -631,7 +746,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
+	tramp = get_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -782,7 +897,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 {
 	struct ppc_inst op;
 	unsigned long ip = rec->ip;
-	unsigned long entry, ptr, tramp;
+	unsigned long entry, ptr, tramp, tramp_old = 0;
 
 	/* read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)ip)) {
@@ -814,6 +929,8 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			pr_err("we don't know about the tramp at %lx!\n", tramp);
 			return -EFAULT;
 		}
+
+		tramp_old = tramp;
 	}
 
 	/* The new target may be within range */
@@ -824,7 +941,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			return -EINVAL;
 		}
 
-		return 0;
+		goto out;
 	}
 
 	ptr = ppc_global_function_entry((void *)addr);
@@ -836,7 +953,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
 
-	tramp = find_ftrace_tramp(ip, ptr);
+	tramp = get_ftrace_tramp(ip, ptr);
 
 	if (!tramp) {
 		pr_err("Couldn't find a trampoline\n");
@@ -850,8 +967,13 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		return -EINVAL;
 	}
 
+out:
+	if (tramp_old)
+		remove_ftrace_tramp(ip, old_addr, tramp_old);
+
 	return 0;
 }
+
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -950,9 +1072,13 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i], addr);
+		add_ftrace_tramp((unsigned long)tramp[i], addr, 1);
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	bitmap_zero(stubs_bitmap, FTRACE_NUM_STUBS);
+#endif
+
 	return 0;
 }
 #else
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index c5602e9b07faa3..ffd2e33ff979bc 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -13,6 +13,13 @@
 #include <asm/bug.h>
 #include <asm/ptrace.h>
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	.balign	4
+.global ftrace_stubs
+ftrace_stubs:
+	.space FTRACE_STUBS_SIZE
+#endif
+
 /*
  *
  * ftrace_caller()/ftrace_regs_caller() is the function that replaces _mcount()
@@ -91,6 +98,10 @@ _GLOBAL(ftrace_regs_caller)
 	std     r10, _XER(r1)
 	std     r11, _CCR(r1)
 
+	/* Clear out orig_gpr3 */
+	li	r6, 0
+	std	r6, ORIG_GPR3(r1)
+
 	/* Load &pt_regs in r6 for call below */
 	addi    r6, r1 ,STACK_FRAME_OVERHEAD
 
@@ -103,20 +114,34 @@ ftrace_regs_call:
 	/* Load ctr with the possibly modified NIP */
 	ld	r3, _NIP(r1)
 	mtctr	r3
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	/* Check if we should go to a direct call next */
+	ld	r4, ORIG_GPR3(r1)
+	cmpdi	r4, 0
+	beq+	1f
+	/* r4 has the direct call target, setup LR and r0 as on our entry, reset cr0 */
+	mtctr	r4
+	mtlr	r3
+	ld	r0, _LINK(r1)
+	cmpd	r3, r3
+	b	2f
+#endif
+
+1:
 #ifdef CONFIG_LIVEPATCH
 	cmpd	r14, r3		/* has NIP been altered? */
 #endif
 
-	/* Restore gprs */
-	REST_GPR(0,r1)
-	REST_10GPRS(2,r1)
-	REST_10GPRS(12,r1)
-	REST_10GPRS(22,r1)
-
 	/* Restore possibly modified LR */
 	ld	r0, _LINK(r1)
 	mtlr	r0
 
+	/* Restore gprs */
+2:	REST_10GPRS(2,r1)
+	REST_10GPRS(12,r1)
+	REST_10GPRS(22,r1)
+
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
@@ -282,6 +307,7 @@ _GLOBAL(ftrace_graph_caller)
 	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
 	SAVE_8GPRS(3, r1)
+	SAVE_GPR(0, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
@@ -304,6 +330,8 @@ _GLOBAL(ftrace_graph_caller)
 
 	ld	r0, _NIP(r1)
 	mtctr	r0
+
+	REST_GPR(0, r1)
 	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 10/14] powerpc/ftrace: Drop assumptions about ftrace trampoline target
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

We currently assume that ftrace locations are patched to go to either
ftrace_caller or ftrace_regs_caller. Drop this assumption in preparation
for supporting ftrace direct calls.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 107 +++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 7ddb6e4b527c39..fcb21a9756e456 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -322,14 +322,15 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
+	int i;
 	struct ppc_inst op;
 	struct ppc_inst instr;
 	struct ppc_ftrace_stub_data *stub;
 	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
-	/* Is this a known long jump tramp? */
-	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
-		if (stub->target == ftrace_target && stub->addr == tramp)
+	/* Is this a known tramp? */
+	hash_for_each(ppc_ftrace_stubs, i, stub, hentry)
+		if (stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -608,23 +609,16 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 {
 	struct ppc_inst op;
 	void *ip = (void *)rec->ip;
-	unsigned long tramp, entry, ptr;
+	unsigned long tramp, ptr;
 
-	/* Make sure we're being asked to patch branch to a known ftrace addr */
-	entry = ppc_global_function_entry((void *)ftrace_caller);
 	ptr = ppc_global_function_entry((void *)addr);
 
-	if (ptr != entry) {
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		entry = ppc_global_function_entry((void *)ftrace_regs_caller);
-		if (ptr != entry) {
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	tramp = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == tramp)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
-			pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
-			return -EINVAL;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		}
-#endif
-	}
 
 	/* Make sure we have a nop */
 	if (probe_kernel_read_inst(&op, ip)) {
@@ -637,7 +631,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
+	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -783,6 +777,81 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 }
 #endif
 
+static int
+__ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr)
+{
+	struct ppc_inst op;
+	unsigned long ip = rec->ip;
+	unsigned long entry, ptr, tramp;
+
+	/* read where this goes */
+	if (probe_kernel_read_inst(&op, (void *)ip)) {
+		pr_err("Fetching opcode failed.\n");
+		return -EFAULT;
+	}
+
+	/* Make sure that this is still a 24bit jump */
+	if (!is_bl_op(op)) {
+		pr_err("Not expected bl: opcode is %s\n", ppc_inst_as_str(op));
+		return -EINVAL;
+	}
+
+	/* lets find where the pointer goes */
+	tramp = find_bl_target(ip, op);
+	entry = ppc_global_function_entry((void *)old_addr);
+
+	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+
+	if (tramp != entry) {
+		/* old_addr is not within range, so we must have used a trampoline */
+		struct ppc_ftrace_stub_data *stub;
+
+		hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, entry)
+			if (stub->target == entry && stub->addr == tramp)
+				break;
+
+		if (stub->target != entry || stub->addr != tramp) {
+			pr_err("we don't know about the tramp at %lx!\n", tramp);
+			return -EFAULT;
+		}
+	}
+
+	/* The new target may be within range */
+	if (test_24bit_addr(ip, addr)) {
+		/* within range */
+		if (patch_branch((struct ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
+			pr_err("REL24 out of range!\n");
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	ptr = ppc_global_function_entry((void *)addr);
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	entry = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == entry)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
+#endif
+
+	tramp = find_ftrace_tramp(ip, ptr);
+
+	if (!tramp) {
+		pr_err("Couldn't find a trampoline\n");
+		return -EFAULT;
+	}
+
+	pr_devel("trampoline %lx target %lx", tramp, ptr);
+
+	if (patch_branch((struct ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
+		pr_err("REL24 out of range!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -800,11 +869,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 		new = ftrace_call_replace(ip, addr, 1);
 		return ftrace_modify_code(ip, old, new);
 	} else if (core_kernel_text(ip)) {
-		/*
-		 * We always patch out of range locations to go to the regs
-		 * variant, so there is nothing to do here
-		 */
-		return 0;
+		return __ftrace_modify_call_kernel(rec, old_addr, addr);
 	}
 
 #ifdef CONFIG_MODULES
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 12/14] powerpc/ftrace: Drop saving LR to stack save area for -mprofile-kernel
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

In -mprofile-kernel variant of ftrace_graph_caller(), we save the
optionally-updated LR address into the stack save area at the end. This
is likely an offshoot of the initial -mprofile-kernel implementation in
gcc emitting the same as part of the -mprofile-kernel instruction
sequence. However, this is not required. Drop it.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index bbe871b47ade58..c5602e9b07faa3 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -310,7 +310,5 @@ _GLOBAL(ftrace_graph_caller)
 	ld	r2, 24(r1)
 
 	addi	r1, r1, SWITCH_FRAME_SIZE
-	mflr	r0
-	std	r0, LRSAVE(r1)
 	bctr
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 08/14] powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace trampoline
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Use FTRACE_REGS_ADDR instead of keying off
CONFIG_DYNAMIC_FTRACE_WITH_REGS to identify the proper ftrace trampoline
address to use.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4fe5f373172fd2..14b39f7797d455 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -361,11 +361,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	ptr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
 				(void *)ptr);
@@ -885,11 +881,7 @@ int __init ftrace_dyn_arch_init(void)
 		0x7d8903a6,		/* mtctr   r12			*/
 		0x4e800420,		/* bctr				*/
 	};
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	unsigned long addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	long reladdr = addr - kernel_toc_addr();
 
 	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 07/14] powerpc/ftrace: Remove dead code
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

ftrace_plt_tramps[] was intended to speed up skipping plt branches, but
the code wasn't completed. It is also not significantly better than
reading and decoding the instruction. Remove the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 42761ebec9f755..4fe5f373172fd2 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -332,7 +332,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	struct ppc_inst op;
 	unsigned long ptr;
 	struct ppc_inst instr;
-	static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
 
 	/* Is this a known long jump tramp? */
 	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
@@ -341,13 +340,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		else if (ftrace_tramps[i] == tramp)
 			return 0;
 
-	/* Is this a known plt tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_plt_tramps[i])
-			break;
-		else if (ftrace_plt_tramps[i] == tramp)
-			return -1;
-
 	/* New trampoline -- read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)tramp)) {
 		pr_debug("Fetching opcode failed.\n");
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 06/14] powerpc: Add support for CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add register_get_kernel_argument() for a rudimentary way to access
kernel function arguments.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig              |  1 +
 arch/powerpc/include/asm/ptrace.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e9f13fe084929b..cfc6dd787f532c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -202,6 +202,7 @@ config PPC
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
 	select HAVE_FAST_GUP
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e2c778c176a3a6..956828c07abd70 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -62,6 +62,8 @@ struct pt_regs
 };
 #endif
 
+#define NR_REG_ARGUMENTS 8
+
 #ifdef __powerpc64__
 
 /*
@@ -85,8 +87,10 @@ struct pt_regs
 
 #ifdef PPC64_ELF_ABI_v2
 #define STACK_FRAME_MIN_SIZE	32
+#define STACK_FRAME_PARM_SAVE	32
 #else
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	48
 #endif
 
 /* Size of dummy stack frame allocated when calling signal handler. */
@@ -103,6 +107,7 @@ struct pt_regs
 #define STACK_INT_FRAME_SIZE	(sizeof(struct pt_regs) + STACK_FRAME_OVERHEAD)
 #define STACK_FRAME_MARKER	2
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	8
 
 /* Size of stack frame allocated when calling signal handler. */
 #define __SIGNAL_FRAMESIZE	64
@@ -309,6 +314,32 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
 		return 0;
 }
 
+/**
+ * regs_get_kernel_argument() - get Nth function argument in kernel
+ * @regs:	pt_regs of that context
+ * @n:		function argument number (start from 0)
+ *
+ * regs_get_argument() returns @n th argument of the function call.
+ * Note that this chooses most probable assignment, and is incorrect
+ * in scenarios where double or fp/vector parameters are involved.
+ * This also doesn't take into account stack alignment requirements.
+ *
+ * This is expected to be called from kprobes or ftrace with regs
+ * at function entry, so the current function has not setup its stack.
+ */
+static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
+						     unsigned int n)
+{
+	if (n >= NR_REG_ARGUMENTS) {
+#ifndef __powerpc64__
+		n -= NR_REG_ARGUMENTS;
+#endif
+		n += STACK_FRAME_PARM_SAVE / sizeof(unsigned long);
+		return regs_get_kernel_stack_nth(regs, n);
+	} else {
+		return regs_get_register(regs, offsetof(struct pt_regs, gpr[n + 3]));
+	}
+}
 #endif /* __ASSEMBLY__ */
 
 #ifndef __powerpc64__
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 05/14] ftrace: Add architectural helpers for [un]register_ftrace_direct()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Architectures may want to do some validation (such as to ensure that the
trampoline code is reachable from the provided ftrace location) before
accepting ftrace direct registration. Add helpers for the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h |  2 ++
 kernel/trace/ftrace.c  | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 46b4b7ee28c41f..3fdcb4c513bc2d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -290,6 +290,8 @@ int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 				unsigned long old_addr,
 				unsigned long new_addr);
 unsigned long ftrace_find_rec_direct(unsigned long ip);
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr);
+void arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr);
 #else
 # define ftrace_direct_func_count 0
 static inline int register_ftrace_direct(unsigned long ip, unsigned long addr)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7476f2458b6d95..0e259b90527722 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5005,6 +5005,13 @@ ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+int __weak arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	return 0;
+}
+
+void __weak arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr) { }
+
 /**
  * register_ftrace_direct - Call a custom trampoline directly
  * @ip: The address of the nop at the beginning of a function
@@ -5028,6 +5035,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	struct ftrace_hash *free_hash = NULL;
 	struct dyn_ftrace *rec;
 	int ret = -EBUSY;
+	int arch_ret;
 
 	mutex_lock(&direct_mutex);
 
@@ -5082,18 +5090,24 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	entry->direct = addr;
 	__add_hash_entry(direct_functions, entry);
 
-	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+	arch_ret = arch_register_ftrace_direct(ip, addr);
 
-	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
-		ret = register_ftrace_function(&direct_ops);
-		if (ret)
-			ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+	if (!arch_ret) {
+		ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+
+		if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
+			ret = register_ftrace_function(&direct_ops);
+			if (ret)
+				ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+		}
 	}
 
-	if (ret) {
+	if (arch_ret || ret) {
 		remove_hash_entry(direct_functions, entry);
 		ftrace_direct_func_count--;
 		kfree(entry);
+		if (!arch_ret)
+			arch_unregister_ftrace_direct(ip, addr);
 	}
  out_unlock:
 	mutex_unlock(&direct_mutex);
@@ -5155,6 +5169,7 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 	remove_hash_entry(direct_functions, entry);
 	ftrace_direct_func_count--;
 	kfree(entry);
+	arch_unregister_ftrace_direct(ip, addr);
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-- 
2.25.4


^ 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