LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 25/30] powerpc: Test prefixed instructions in feature fixups
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

Expand the feature-fixups self-tests to includes tests for prefixed
instructions.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v6: New to series
v8: Use OP_PREFIX
---
 arch/powerpc/lib/feature-fixups-test.S | 69 ++++++++++++++++++++++++
 arch/powerpc/lib/feature-fixups.c      | 73 ++++++++++++++++++++++++++
 2 files changed, 142 insertions(+)

diff --git a/arch/powerpc/lib/feature-fixups-test.S b/arch/powerpc/lib/feature-fixups-test.S
index b12168c2447a..480172fbd024 100644
--- a/arch/powerpc/lib/feature-fixups-test.S
+++ b/arch/powerpc/lib/feature-fixups-test.S
@@ -7,6 +7,7 @@
 #include <asm/ppc_asm.h>
 #include <asm/synch.h>
 #include <asm/asm-compat.h>
+#include <asm/ppc-opcode.h>
 
 	.text
 
@@ -791,3 +792,71 @@ globl(lwsync_fixup_test_expected_SYNC)
 1:	or	1,1,1
 	sync
 
+globl(ftr_fixup_prefix1)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+globl(end_ftr_fixup_prefix1)
+
+globl(ftr_fixup_prefix1_orig)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+
+globl(ftr_fixup_prefix1_expected)
+	or	1,1,1
+	nop
+	nop
+	or	2,2,2
+
+globl(ftr_fixup_prefix2)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+globl(end_ftr_fixup_prefix2)
+
+globl(ftr_fixup_prefix2_orig)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+
+globl(ftr_fixup_prefix2_alt)
+	.long OP_PREFIX << 26
+	.long 0x0000001
+
+globl(ftr_fixup_prefix2_expected)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000001
+	or	2,2,2
+
+globl(ftr_fixup_prefix3)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+	or	3,3,3
+globl(end_ftr_fixup_prefix3)
+
+globl(ftr_fixup_prefix3_orig)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000000
+	or	2,2,2
+	or	3,3,3
+
+globl(ftr_fixup_prefix3_alt)
+	.long OP_PREFIX << 26
+	.long 0x0000001
+	nop
+
+globl(ftr_fixup_prefix3_expected)
+	or	1,1,1
+	.long OP_PREFIX << 26
+	.long 0x0000001
+	nop
+	or	3,3,3
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index a8238eff3a31..5144854713e6 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -689,6 +689,74 @@ static void test_lwsync_macros(void)
 	}
 }
 
+#ifdef __powerpc64__
+static void __init test_prefix_patching(void)
+{
+	extern unsigned int ftr_fixup_prefix1[];
+	extern unsigned int end_ftr_fixup_prefix1[];
+	extern unsigned int ftr_fixup_prefix1_orig[];
+	extern unsigned int ftr_fixup_prefix1_expected[];
+	int size = sizeof(unsigned int) * (end_ftr_fixup_prefix1 - ftr_fixup_prefix1);
+
+	fixup.value = fixup.mask = 8;
+	fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix1 + 1);
+	fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix1 + 3);
+	fixup.alt_start_off = fixup.alt_end_off = 0;
+
+	/* Sanity check */
+	check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_orig, size) == 0);
+
+	patch_feature_section(0, &fixup);
+	check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_expected, size) == 0);
+	check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_orig, size) != 0);
+}
+
+static void __init test_prefix_alt_patching(void)
+{
+	extern unsigned int ftr_fixup_prefix2[];
+	extern unsigned int end_ftr_fixup_prefix2[];
+	extern unsigned int ftr_fixup_prefix2_orig[];
+	extern unsigned int ftr_fixup_prefix2_expected[];
+	extern unsigned int ftr_fixup_prefix2_alt[];
+	int size = sizeof(unsigned int) * (end_ftr_fixup_prefix2 - ftr_fixup_prefix2);
+
+	fixup.value = fixup.mask = 8;
+	fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix2 + 1);
+	fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix2 + 3);
+	fixup.alt_start_off = calc_offset(&fixup, ftr_fixup_prefix2_alt);
+	fixup.alt_end_off = calc_offset(&fixup, ftr_fixup_prefix2_alt + 2);
+	/* Sanity check */
+	check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_orig, size) == 0);
+
+	patch_feature_section(0, &fixup);
+	check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_expected, size) == 0);
+	check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_orig, size) != 0);
+}
+
+static void __init test_prefix_word_alt_patching(void)
+{
+	extern unsigned int ftr_fixup_prefix3[];
+	extern unsigned int end_ftr_fixup_prefix3[];
+	extern unsigned int ftr_fixup_prefix3_orig[];
+	extern unsigned int ftr_fixup_prefix3_expected[];
+	extern unsigned int ftr_fixup_prefix3_alt[];
+	int size = sizeof(unsigned int) * (end_ftr_fixup_prefix3 - ftr_fixup_prefix3);
+
+	fixup.value = fixup.mask = 8;
+	fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix3 + 1);
+	fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix3 + 4);
+	fixup.alt_start_off = calc_offset(&fixup, ftr_fixup_prefix3_alt);
+	fixup.alt_end_off = calc_offset(&fixup, ftr_fixup_prefix3_alt + 3);
+	/* Sanity check */
+	check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_orig, size) == 0);
+
+	patch_feature_section(0, &fixup);
+	check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_expected, size) == 0);
+	patch_feature_section(0, &fixup);
+	check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_orig, size) != 0);
+}
+#endif /* __powerpc64__ */
+
 static int __init test_feature_fixups(void)
 {
 	printk(KERN_DEBUG "Running feature fixup self-tests ...\n");
@@ -703,6 +771,11 @@ static int __init test_feature_fixups(void)
 	test_cpu_macros();
 	test_fw_macros();
 	test_lwsync_macros();
+#ifdef __powerpc64__
+	test_prefix_patching();
+	test_prefix_alt_patching();
+	test_prefix_word_alt_patching();
+#endif
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH v8 26/30] powerpc/xmon: Don't allow breakpoints on suffixes
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

Do not allow placing xmon breakpoints on the suffix of a prefix
instruction.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v8: Add this back from v3
---
 arch/powerpc/xmon/xmon.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 647b3829c4eb..d082c35c6638 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -889,8 +889,8 @@ static struct bpt *new_breakpoint(unsigned long a)
 static void insert_bpts(void)
 {
 	int i;
-	struct ppc_inst instr;
-	struct bpt *bp;
+	struct ppc_inst instr, instr2;
+	struct bpt *bp, *bp2;
 
 	bp = bpts;
 	for (i = 0; i < NBPTS; ++i, ++bp) {
@@ -908,6 +908,31 @@ static void insert_bpts(void)
 			bp->enabled = 0;
 			continue;
 		}
+		/*
+		 * Check the address is not a suffix by looking for a prefix in
+		 * front of it.
+		 */
+		if (mread_instr(bp->address - 4, &instr2) == 8) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
+		/*
+		 * We might still be a suffix - if the prefix has already been
+		 * replaced by a breakpoint we won't catch it with the above
+		 * test.
+		 */
+		bp2 = at_breakpoint(bp->address - 4);
+		if (bp2 && ppc_inst_prefixed(ppc_inst_read(bp2->instr))) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
+
 		patch_instruction(bp->instr, instr);
 		patch_instruction((void *)bp->instr + ppc_inst_len(instr),
 				  ppc_inst(bpinstr));
-- 
2.17.1


^ permalink raw reply related

* [PATCH v8 27/30] powerpc/kprobes: Don't allow breakpoints on suffixes
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

Do not allow inserting breakpoints on the suffix of a prefix instruction
in kprobes.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v8: Add this back from v3
---
 arch/powerpc/kernel/kprobes.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 33d54b091c70..227510df8c55 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -106,7 +106,9 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
 int arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
+	struct kprobe *prev;
 	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
+	struct ppc_inst prefix = ppc_inst_read((struct ppc_inst *)(p->addr - 1));
 
 	if ((unsigned long)p->addr & 0x03) {
 		printk("Attempt to register kprobe at an unaligned address\n");
@@ -114,6 +116,17 @@ int arch_prepare_kprobe(struct kprobe *p)
 	} else if (IS_MTMSRD(insn) || IS_RFID(insn) || IS_RFI(insn)) {
 		printk("Cannot register a kprobe on rfi/rfid or mtmsr[d]\n");
 		ret = -EINVAL;
+	} else if (ppc_inst_prefixed(prefix)) {
+		printk("Cannot register a kprobe on the second word of prefixed instruction\n");
+		ret = -EINVAL;
+	}
+	preempt_disable();
+	prev = get_kprobe(p->addr - 1);
+	preempt_enable_no_resched();
+	if (prev &&
+	    ppc_inst_prefixed(ppc_inst_read((struct ppc_inst *)prev->ainsn.insn))) {
+		printk("Cannot register a kprobe on the second word of prefixed instruction\n");
+		ret = -EINVAL;
 	}
 
 	/* insn must be on a special executable page on ppc64.  This is
-- 
2.17.1


^ permalink raw reply related

* [PATCH v8 28/30] powerpc: Support prefixed instructions in alignment handler
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

If a prefixed instruction results in an alignment exception, the
SRR1_PREFIXED bit is set. The handler attempts to emulate the
responsible instruction and then increment the NIP past it. Use
SRR1_PREFIXED to determine by how much the NIP should be incremented.

Prefixed instructions are not permitted to cross 64-byte boundaries. If
they do the alignment interrupt is invoked with SRR1 BOUNDARY bit set.
If this occurs send a SIGBUS to the offending process if in user mode.
If in kernel mode call bad_page_fault().

Reviewed-by: Alistair Popple <alistair@popple.id.au>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this
commit (previously in "powerpc sstep: Prepare to support prefixed
instructions").
    - Rename sufx to suffix
    - Use a macro for calculating instruction length
v3: Move __get_user_{instr(), instr_inatomic()} up with the other
get_user definitions and remove nested if.
v4: Rolled into "Add prefixed instructions to instruction data type"
v5: Only one definition of inst_length()
---
 arch/powerpc/kernel/traps.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 493a3fa0ac1a..105242cc2f28 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -583,6 +583,8 @@ static inline int check_io_access(struct pt_regs *regs)
 #define REASON_ILLEGAL		(ESR_PIL | ESR_PUO)
 #define REASON_PRIVILEGED	ESR_PPR
 #define REASON_TRAP		ESR_PTR
+#define REASON_PREFIXED		0
+#define REASON_BOUNDARY		0
 
 /* single-step stuff */
 #define single_stepping(regs)	(current->thread.debug.dbcr0 & DBCR0_IC)
@@ -597,12 +599,16 @@ static inline int check_io_access(struct pt_regs *regs)
 #define REASON_ILLEGAL		SRR1_PROGILL
 #define REASON_PRIVILEGED	SRR1_PROGPRIV
 #define REASON_TRAP		SRR1_PROGTRAP
+#define REASON_PREFIXED		SRR1_PREFIXED
+#define REASON_BOUNDARY		SRR1_BOUNDARY
 
 #define single_stepping(regs)	((regs)->msr & MSR_SE)
 #define clear_single_step(regs)	((regs)->msr &= ~MSR_SE)
 #define clear_br_trace(regs)	((regs)->msr &= ~MSR_BE)
 #endif
 
+#define inst_length(reason)	(((reason) & REASON_PREFIXED) ? 8 : 4)
+
 #if defined(CONFIG_E500)
 int machine_check_e500mc(struct pt_regs *regs)
 {
@@ -1593,11 +1599,20 @@ void alignment_exception(struct pt_regs *regs)
 {
 	enum ctx_state prev_state = exception_enter();
 	int sig, code, fixed = 0;
+	unsigned long  reason;
 
 	/* We restore the interrupt state now */
 	if (!arch_irq_disabled_regs(regs))
 		local_irq_enable();
 
+	reason = get_reason(regs);
+
+	if (reason & REASON_BOUNDARY) {
+		sig = SIGBUS;
+		code = BUS_ADRALN;
+		goto bad;
+	}
+
 	if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
 		goto bail;
 
@@ -1606,7 +1621,8 @@ void alignment_exception(struct pt_regs *regs)
 		fixed = fix_alignment(regs);
 
 	if (fixed == 1) {
-		regs->nip += 4;	/* skip over emulated instruction */
+		/* skip over emulated instruction */
+		regs->nip += inst_length(reason);
 		emulate_single_step(regs);
 		goto bail;
 	}
@@ -1619,6 +1635,7 @@ void alignment_exception(struct pt_regs *regs)
 		sig = SIGBUS;
 		code = BUS_ADRALN;
 	}
+bad:
 	if (user_mode(regs))
 		_exception(sig, regs, code, regs->dar);
 	else
-- 
2.17.1


^ permalink raw reply related

* [PATCH v8 29/30] powerpc sstep: Add support for prefixed load/stores
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

This adds emulation support for the following prefixed integer
load/stores:
  * Prefixed Load Byte and Zero (plbz)
  * Prefixed Load Halfword and Zero (plhz)
  * Prefixed Load Halfword Algebraic (plha)
  * Prefixed Load Word and Zero (plwz)
  * Prefixed Load Word Algebraic (plwa)
  * Prefixed Load Doubleword (pld)
  * Prefixed Store Byte (pstb)
  * Prefixed Store Halfword (psth)
  * Prefixed Store Word (pstw)
  * Prefixed Store Doubleword (pstd)
  * Prefixed Load Quadword (plq)
  * Prefixed Store Quadword (pstq)

the follow prefixed floating-point load/stores:
  * Prefixed Load Floating-Point Single (plfs)
  * Prefixed Load Floating-Point Double (plfd)
  * Prefixed Store Floating-Point Single (pstfs)
  * Prefixed Store Floating-Point Double (pstfd)

and for the following prefixed VSX load/stores:
  * Prefixed Load VSX Scalar Doubleword (plxsd)
  * Prefixed Load VSX Scalar Single-Precision (plxssp)
  * Prefixed Load VSX Vector [0|1]  (plxv, plxv0, plxv1)
  * Prefixed Store VSX Scalar Doubleword (pstxsd)
  * Prefixed Store VSX Scalar Single-Precision (pstxssp)
  * Prefixed Store VSX Vector [0|1] (pstxv, pstxv0, pstxv1)

Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Combine all load/store patches
    - Fix the name of Type 01 instructions
    - Remove sign extension flag from pstd/pld
    - Rename sufx -> suffix
v3: - Move prefixed loads and stores into the switch statement
v6: - Compile on ppc32
    - Add back in + GETLENGTH(op->type)
v8: Use fallthrough; keyword
---
 arch/powerpc/include/asm/sstep.h |   4 +
 arch/powerpc/lib/sstep.c         | 163 ++++++++++++++++++++++++++++++-
 2 files changed, 165 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index c3ce903ac488..9b200a5f8794 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -90,11 +90,15 @@ enum instruction_type {
 #define VSX_LDLEFT	4	/* load VSX register from left */
 #define VSX_CHECK_VEC	8	/* check MSR_VEC not MSR_VSX for reg >= 32 */
 
+/* Prefixed flag, ORed in with type */
+#define PREFIXED       0x800
+
 /* Size field in type word */
 #define SIZE(n)		((n) << 12)
 #define GETSIZE(w)	((w) >> 12)
 
 #define GETTYPE(t)	((t) & INSTR_TYPE_MASK)
+#define GETLENGTH(t)   (((t) & PREFIXED) ? 8 : 4)
 
 #define MKOP(t, f, s)	((t) | (f) | SIZE(s))
 
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index ecd756c346fd..6794a7672ad5 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -187,6 +187,44 @@ static nokprobe_inline unsigned long xform_ea(unsigned int instr,
 	return ea;
 }
 
+/*
+ * Calculate effective address for a MLS:D-form / 8LS:D-form
+ * prefixed instruction
+ */
+static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
+						  unsigned int suffix,
+						  const struct pt_regs *regs)
+{
+	int ra, prefix_r;
+	unsigned int  dd;
+	unsigned long ea, d0, d1, d;
+
+	prefix_r = instr & (1ul << 20);
+	ra = (suffix >> 16) & 0x1f;
+
+	d0 = instr & 0x3ffff;
+	d1 = suffix & 0xffff;
+	d = (d0 << 16) | d1;
+
+	/*
+	 * sign extend a 34 bit number
+	 */
+	dd = (unsigned int)(d >> 2);
+	ea = (signed int)dd;
+	ea = (ea << 2) | (d & 0x3);
+
+	if (!prefix_r && ra)
+		ea += regs->gpr[ra];
+	else if (!prefix_r && !ra)
+		; /* Leave ea as is */
+	else if (prefix_r && !ra)
+		ea += regs->nip;
+	else if (prefix_r && ra)
+		; /* Invalid form. Should already be checked for by caller! */
+
+	return ea;
+}
+
 /*
  * Return the largest power of 2, not greater than sizeof(unsigned long),
  * such that x is a multiple of it.
@@ -1166,6 +1204,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 		  struct ppc_inst instr)
 {
 	unsigned int opcode, ra, rb, rc, rd, spr, u;
+#ifdef __powerpc64__
+	unsigned int suffixopcode, prefixtype, prefix_r;
+#endif
 	unsigned long int imm;
 	unsigned long int val, val2;
 	unsigned int mb, me, sh;
@@ -2652,6 +2693,124 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 			break;
 		}
 		break;
+	case 1: /* Prefixed instructions */
+		prefix_r = word & (1ul << 20);
+		ra = (suffix >> 16) & 0x1f;
+		op->update_reg = ra;
+		rd = (suffix >> 21) & 0x1f;
+		op->reg = rd;
+		op->val = regs->gpr[rd];
+
+		suffixopcode = suffix >> 26;
+		prefixtype = (word >> 24) & 0x3;
+		switch (prefixtype) {
+		case 0: /* Type 00  Eight-Byte Load/Store */
+			if (prefix_r && ra)
+				break;
+			op->ea = mlsd_8lsd_ea(word, suffix, regs);
+			switch (suffixopcode) {
+			case 41:	/* plwa */
+				op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 4);
+				break;
+			case 42:        /* plxsd */
+				op->reg = rd + 32;
+				op->type = MKOP(LOAD_VSX, PREFIXED, 8);
+				op->element_size = 8;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 43:	/* plxssp */
+				op->reg = rd + 32;
+				op->type = MKOP(LOAD_VSX, PREFIXED, 4);
+				op->element_size = 8;
+				op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+				break;
+			case 46:	/* pstxsd */
+				op->reg = rd + 32;
+				op->type = MKOP(STORE_VSX, PREFIXED, 8);
+				op->element_size = 8;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 47:	/* pstxssp */
+				op->reg = rd + 32;
+				op->type = MKOP(STORE_VSX, PREFIXED, 4);
+				op->element_size = 8;
+				op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+				break;
+			case 51:	/* plxv1 */
+				op->reg += 32;
+				fallthrough;
+			case 50:	/* plxv0 */
+				op->type = MKOP(LOAD_VSX, PREFIXED, 16);
+				op->element_size = 16;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 55:	/* pstxv1 */
+				op->reg = rd + 32;
+				fallthrough;
+			case 54:	/* pstxv0 */
+				op->type = MKOP(STORE_VSX, PREFIXED, 16);
+				op->element_size = 16;
+				op->vsx_flags = VSX_CHECK_VEC;
+				break;
+			case 56:        /* plq */
+				op->type = MKOP(LOAD, PREFIXED, 16);
+				break;
+			case 57:	/* pld */
+				op->type = MKOP(LOAD, PREFIXED, 8);
+				break;
+			case 60:        /* stq */
+				op->type = MKOP(STORE, PREFIXED, 16);
+				break;
+			case 61:	/* pstd */
+				op->type = MKOP(STORE, PREFIXED, 8);
+				break;
+			}
+			break;
+		case 1: /* Type 01 Eight-Byte Register-to-Register */
+			break;
+		case 2: /* Type 10 Modified Load/Store */
+			if (prefix_r && ra)
+				break;
+			op->ea = mlsd_8lsd_ea(word, suffix, regs);
+			switch (suffixopcode) {
+			case 32:	/* plwz */
+				op->type = MKOP(LOAD, PREFIXED, 4);
+				break;
+			case 34:	/* plbz */
+				op->type = MKOP(LOAD, PREFIXED, 1);
+				break;
+			case 36:	/* pstw */
+				op->type = MKOP(STORE, PREFIXED, 4);
+				break;
+			case 38:	/* pstb */
+				op->type = MKOP(STORE, PREFIXED, 1);
+				break;
+			case 40:	/* plhz */
+				op->type = MKOP(LOAD, PREFIXED, 2);
+				break;
+			case 42:	/* plha */
+				op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 2);
+				break;
+			case 44:	/* psth */
+				op->type = MKOP(STORE, PREFIXED, 2);
+				break;
+			case 48:        /* plfs */
+				op->type = MKOP(LOAD_FP, PREFIXED | FPCONV, 4);
+				break;
+			case 50:        /* plfd */
+				op->type = MKOP(LOAD_FP, PREFIXED, 8);
+				break;
+			case 52:        /* pstfs */
+				op->type = MKOP(STORE_FP, PREFIXED | FPCONV, 4);
+				break;
+			case 54:        /* pstfd */
+				op->type = MKOP(STORE_FP, PREFIXED, 8);
+				break;
+			}
+			break;
+		case 3: /* Type 11 Modified Register-to-Register */
+			break;
+		}
 #endif /* __powerpc64__ */
 
 	}
@@ -2760,7 +2919,7 @@ void emulate_update_regs(struct pt_regs *regs, struct instruction_op *op)
 {
 	unsigned long next_pc;
 
-	next_pc = truncate_if_32bit(regs->msr, regs->nip + 4);
+	next_pc = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op->type));
 	switch (GETTYPE(op->type)) {
 	case COMPUTE:
 		if (op->type & SETREG)
@@ -3205,7 +3364,7 @@ int emulate_step(struct pt_regs *regs, struct ppc_inst instr)
 	return 0;
 
  instr_done:
-	regs->nip = truncate_if_32bit(regs->msr, regs->nip + 4);
+	regs->nip = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op.type));
 	return 1;
 }
 NOKPROBE_SYMBOL(emulate_step);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v8 30/30] powerpc sstep: Add support for prefixed fixed-point arithmetic
From: Jordan Niethe @ 2020-05-06  3:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: alistair, npiggin, bala24, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20200506034050.24806-1-jniethe5@gmail.com>

This adds emulation support for the following prefixed Fixed-Point
Arithmetic instructions:
  * Prefixed Add Immediate (paddi)

Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v3: Since we moved the prefixed loads/stores into the load/store switch
statement it no longer makes sense to have paddi in there, so move it
out.
---
 arch/powerpc/lib/sstep.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 6794a7672ad5..964fe7bbfce3 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1337,6 +1337,26 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
 
 	switch (opcode) {
 #ifdef __powerpc64__
+	case 1:
+		prefix_r = word & (1ul << 20);
+		ra = (suffix >> 16) & 0x1f;
+		rd = (suffix >> 21) & 0x1f;
+		op->reg = rd;
+		op->val = regs->gpr[rd];
+		suffixopcode = suffix >> 26;
+		prefixtype = (word >> 24) & 0x3;
+		switch (prefixtype) {
+		case 2:
+			if (prefix_r && ra)
+				return 0;
+			switch (suffixopcode) {
+			case 14:	/* paddi */
+				op->type = COMPUTE | PREFIXED;
+				op->val = mlsd_8lsd_ea(word, suffix, regs);
+				goto compute_done;
+			}
+		}
+		break;
 	case 2:		/* tdi */
 		if (rd & trap_compare(regs->gpr[ra], (short) word))
 			goto trap;
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v3 11/15] powerpc/64s: machine check interrupt update NMI accounting
From: Nicholas Piggin @ 2020-05-06  3:50 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar
In-Reply-To: <684cdf24-a87a-1099-70c1-d3ff9a3921ff@c-s.fr>

Excerpts from Christophe Leroy's message of April 7, 2020 3:37 pm:
> 
> 
> Le 07/04/2020 à 07:16, Nicholas Piggin a écrit :
>> machine_check_early is taken as an NMI, so nmi_enter is used there.
>> machine_check_exception is no longer taken as an NMI (it's invoked
>> via irq_work in the case a machine check hits in kernel mode), so
>> remove the nmi_enter from that case.
> 
> Euh ... Is that also the case for PPC32 ?
> 
> AFAIK machine_check_exception() is called as an NMI on PPC32.

Sorry I missed your comment.  You're right, I'll make this change
depend on 64S. Thanks for reviewing them.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/perf: Add support for outputting extended regs in perf intr_regs
From: Madhavan Srinivasan @ 2020-05-06  4:26 UTC (permalink / raw)
  To: Anju T Sudhakar, linuxppc-dev
  Cc: jolsa, ravi.bangoria, maddy, linux-kernel, acme
In-Reply-To: <20200429060415.25930-3-anju@linux.vnet.ibm.com>



On 4/29/20 11:34 AM, Anju T Sudhakar wrote:
> The capability flag PERF_PMU_CAP_EXTENDED_REGS, is used to indicate the
> PMU which support extended registers. The generic code define the mask
> of extended registers as 0 for non supported architectures.
>
> Add support for extended registers in POWER9 architecture. For POWER9,
> the extended registers are mmcr0, mmc1 and mmcr2.
>
> REG_RESERVED mask is redefined to accommodate the extended registers.
>
> With patch:
> ----------------
>
> # perf record -I?
> available registers: r0 r1 r2 r3 r4 r5 r6 r7 r8 r9 r10 r11 r12 r13 r14
> r15 r16 r17 r18 r19 r20 r21 r22 r23 r24 r25 r26 r27 r28 r29 r30 r31 nip
> msr orig_r3 ctr link xer ccr softe trap dar dsisr sier mmcra mmcr0
> mmcr1 mmcr2

Would prefer to have some flexibility in deciding what to expose
in as extended regs. Meaning say if we want to add extended regs
in power8 and if we dont want to show for ex say mmcr2 (just for example).

Maddy

>
> # perf record -I ls
> # perf script -D
>
> PERF_RECORD_SAMPLE(IP, 0x1): 9019/9019: 0 period: 1 addr: 0
> ... intr regs: mask 0xffffffffffff ABI 64-bit
> .... r0    0xc00000000011b12c
> .... r1    0xc000003f9a98b930
> .... r2    0xc000000001a32100
> .... r3    0xc000003f8fe9a800
> .... r4    0xc000003fd1810000
> .... r5    0x3e32557150
> .... r6    0xc000003f9a98b908
> .... r7    0xffffffc1cdae06ac
> .... r8    0x818
> [.....]
> .... r31   0xc000003ffd047230
> .... nip   0xc00000000011b2c0
> .... msr   0x9000000000009033
> .... orig_r3 0xc00000000011b21c
> .... ctr   0xc000000000119380
> .... link  0xc00000000011b12c
> .... xer   0x0
> .... ccr   0x28002222
> .... softe 0x1
> .... trap  0xf00
> .... dar   0x0
> .... dsisr 0x80000000000
> .... sier  0x0
> .... mmcra 0x80000000000
> .... mmcr0 0x82008090
> .... mmcr1 0x1e000000
> .... mmcr2 0x0
>   ... thread: perf:9019
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/include/asm/perf_event_server.h  |  5 +++
>   arch/powerpc/include/uapi/asm/perf_regs.h     | 13 +++++++-
>   arch/powerpc/perf/core-book3s.c               |  1 +
>   arch/powerpc/perf/perf_regs.c                 | 29 ++++++++++++++--
>   arch/powerpc/perf/power9-pmu.c                |  1 +
>   .../arch/powerpc/include/uapi/asm/perf_regs.h | 13 +++++++-
>   tools/perf/arch/powerpc/include/perf_regs.h   |  6 +++-
>   tools/perf/arch/powerpc/util/perf_regs.c      | 33 +++++++++++++++++++
>   8 files changed, 95 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 3e9703f44c7c..1d15953bd99e 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -55,6 +55,11 @@ struct power_pmu {
>   	int 		*blacklist_ev;
>   	/* BHRB entries in the PMU */
>   	int		bhrb_nr;
> +	/*
> +	 * set this flag with `PERF_PMU_CAP_EXTENDED_REGS` if
> +	 * the pmu supports extended perf regs capability
> +	 */
> +	int		capabilities;
>   };
>
>   /*
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index f599064dd8dc..604b831378fe 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
>   	PERF_REG_POWERPC_DSISR,
>   	PERF_REG_POWERPC_SIER,
>   	PERF_REG_POWERPC_MMCRA,
> -	PERF_REG_POWERPC_MAX,
> +	/* Extended registers */
> +	PERF_REG_POWERPC_MMCR0,
> +	PERF_REG_POWERPC_MMCR1,
> +	PERF_REG_POWERPC_MMCR2,
> +	PERF_REG_EXTENDED_MAX,
> +	/* Max regs without the extended regs */
> +	PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
>   };
> +
> +#define PERF_REG_PMU_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
> +#define PERF_REG_EXTENDED_MASK  (((1ULL << (PERF_REG_EXTENDED_MAX))	\
> +			- 1) - PERF_REG_PMU_MASK)
> +
>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3dcfecf858f3..f56b77800a7b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2276,6 +2276,7 @@ int register_power_pmu(struct power_pmu *pmu)
>
>   	power_pmu.attr_groups = ppmu->attr_groups;
>
> +	power_pmu.capabilities |= (ppmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS);
>   #ifdef MSR_HV
>   	/*
>   	 * Use FCHV to ignore kernel events if MSR.HV is set.
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index a213a0aa5d25..57aa02568caf 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -15,7 +15,8 @@
>
>   #define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r)
>
> -#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1))
> +#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK) &	\
> +			(~((1ULL << PERF_REG_POWERPC_MAX) - 1)))
>
>   static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>   	PT_REGS_OFFSET(PERF_REG_POWERPC_R0,  gpr[0]),
> @@ -69,10 +70,22 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>   	PT_REGS_OFFSET(PERF_REG_POWERPC_MMCRA, dsisr),
>   };
>
> +/* Function to return the extended register values */
> +static u64 get_ext_regs_value(int idx)
> +{
> +	switch (idx) {
> +	case PERF_REG_POWERPC_MMCR0:
> +				    return mfspr(SPRN_MMCR0);
> +	case PERF_REG_POWERPC_MMCR1:
> +				    return mfspr(SPRN_MMCR1);
> +	case PERF_REG_POWERPC_MMCR2:
> +				    return mfspr(SPRN_MMCR2);
> +	default: return 0;
> +	}
> +}
> +
>   u64 perf_reg_value(struct pt_regs *regs, int idx)
>   {
> -	if (WARN_ON_ONCE(idx >= PERF_REG_POWERPC_MAX))
> -		return 0;
>
>   	if (idx == PERF_REG_POWERPC_SIER &&
>   	   (IS_ENABLED(CONFIG_FSL_EMB_PERF_EVENT) ||
> @@ -85,6 +98,16 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>   	    IS_ENABLED(CONFIG_PPC32)))
>   		return 0;
>
> +	if (idx >= PERF_REG_POWERPC_MAX && idx < PERF_REG_EXTENDED_MAX)
> +		return get_ext_regs_value(idx);
> +
> +	/*
> +	 * If the idx is referring to value beyond the
> +	 * supported registers, return 0 with a warning
> +	 */
> +	if (WARN_ON_ONCE(idx >= PERF_REG_EXTENDED_MAX))
> +		return 0;
> +
>   	return regs_get_register(regs, pt_regs_offset[idx]);
>   }
>
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 08c3ef796198..c37193b3e73f 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -434,6 +434,7 @@ static struct power_pmu power9_pmu = {
>   	.cache_events		= &power9_cache_events,
>   	.attr_groups		= power9_pmu_attr_groups,
>   	.bhrb_nr		= 32,
> +	.capabilities		= PERF_PMU_CAP_EXTENDED_REGS,
>   };
>
>   int init_power9_pmu(void)
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index f599064dd8dc..d66953294c73 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
>   	PERF_REG_POWERPC_DSISR,
>   	PERF_REG_POWERPC_SIER,
>   	PERF_REG_POWERPC_MMCRA,
> -	PERF_REG_POWERPC_MAX,
> +	/* Extended arch registers */
> +	PERF_REG_POWERPC_MMCR0,
> +	PERF_REG_POWERPC_MMCR1,
> +	PERF_REG_POWERPC_MMCR2,
> +	PERF_REG_EXTENDED_MAX,
> +	/* Max regs without extended arch regs */
> +	PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> +
>   };
> +#define PERF_REG_PMU_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
> +#define PERF_REG_EXTENDED_MASK  (((1ULL << (PERF_REG_EXTENDED_MAX))\
> +			- 1) - PERF_REG_PMU_MASK)
> +
>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index e18a3556f5e3..f7bbdb816f88 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -64,7 +64,11 @@ static const char *reg_names[] = {
>   	[PERF_REG_POWERPC_DAR] = "dar",
>   	[PERF_REG_POWERPC_DSISR] = "dsisr",
>   	[PERF_REG_POWERPC_SIER] = "sier",
> -	[PERF_REG_POWERPC_MMCRA] = "mmcra"
> +	[PERF_REG_POWERPC_MMCRA] = "mmcra",
> +	[PERF_REG_POWERPC_MMCR0] = "mmcr0",
> +	[PERF_REG_POWERPC_MMCR1] = "mmcr1",
> +	[PERF_REG_POWERPC_MMCR2] = "mmcr2",
> +
>   };
>
>   static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index 0a5242900248..37b150f9d1a1 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -6,6 +6,8 @@
>
>   #include "../../../util/perf_regs.h"
>   #include "../../../util/debug.h"
> +#include "../../../util/event.h"
> +#include "../../../perf-sys.h"
>
>   #include <linux/kernel.h>
>
> @@ -55,6 +57,9 @@ const struct sample_reg sample_reg_masks[] = {
>   	SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
>   	SMPL_REG(sier, PERF_REG_POWERPC_SIER),
>   	SMPL_REG(mmcra, PERF_REG_POWERPC_MMCRA),
> +	SMPL_REG(mmcr0, PERF_REG_POWERPC_MMCR0),
> +	SMPL_REG(mmcr1, PERF_REG_POWERPC_MMCR1),
> +	SMPL_REG(mmcr2, PERF_REG_POWERPC_MMCR2),
>   	SMPL_REG_END
>   };
>
> @@ -163,3 +168,31 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op)
>
>   	return SDT_ARG_VALID;
>   }
> +
> +uint64_t arch__intr_reg_mask(void)
> +{
> +	struct perf_event_attr attr = {
> +		.type                   = PERF_TYPE_HARDWARE,
> +		.config                 = PERF_COUNT_HW_CPU_CYCLES,
> +		.sample_type            = PERF_SAMPLE_REGS_INTR,
> +		.sample_regs_intr       = PERF_REG_EXTENDED_MASK,
> +		.precise_ip             = 1,
> +		.disabled               = 1,
> +		.exclude_kernel         = 1,
> +	};
> +	int fd;
> +
> +	attr.sample_period = 1;
> +	event_attr_init(&attr);
> +
> +	/*
> +	 * check if the pmu supports perf extended regs, before
> +	 * returning the register mask to sample.
> +	 */
> +	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> +	if (fd != -1) {
> +		close(fd);
> +		return (PERF_REG_EXTENDED_MASK | PERF_REGS_MASK);
> +	}
> +	return PERF_REGS_MASK;
> +}


^ permalink raw reply

* [PATCH -next] powerpc/kernel/sysfs: Use ARRAY_SIZE instead of calculating the array size
From: Samuel Zou @ 2020-05-06  2:33 UTC (permalink / raw)
  To: mpe, benh, paulus; +Cc: Samuel Zou, linuxppc-dev, linux-kernel

fix coccinelle warning, use ARRAY_SIZE

arch/powerpc/kernel/sysfs.c:853:34-35: WARNING: Use ARRAY_SIZE
arch/powerpc/kernel/sysfs.c:860:33-34: WARNING: Use ARRAY_SIZE
arch/powerpc/kernel/sysfs.c:868:28-29: WARNING: Use ARRAY_SIZE
arch/powerpc/kernel/sysfs.c:947:34-35: WARNING: Use ARRAY_SIZE
arch/powerpc/kernel/sysfs.c:954:33-34: WARNING: Use ARRAY_SIZE
arch/powerpc/kernel/sysfs.c:962:28-29: WARNING: Use ARRAY_SIZE

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Samuel Zou <zou_wei@huawei.com>
---
 arch/powerpc/kernel/sysfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 571b325..13d2423 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -850,14 +850,14 @@ static int register_cpu_online(unsigned int cpu)
 #ifdef HAS_PPC_PMC_IBM
 	case PPC_PMC_IBM:
 		attrs = ibm_common_attrs;
-		nattrs = sizeof(ibm_common_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(ibm_common_attrs);
 		pmc_attrs = classic_pmc_attrs;
 		break;
 #endif /* HAS_PPC_PMC_IBM */
 #ifdef HAS_PPC_PMC_G4
 	case PPC_PMC_G4:
 		attrs = g4_common_attrs;
-		nattrs = sizeof(g4_common_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(g4_common_attrs);
 		pmc_attrs = classic_pmc_attrs;
 		break;
 #endif /* HAS_PPC_PMC_G4 */
@@ -865,7 +865,7 @@ static int register_cpu_online(unsigned int cpu)
 	case PPC_PMC_PA6T:
 		/* PA Semi starts counting at PMC0 */
 		attrs = pa6t_attrs;
-		nattrs = sizeof(pa6t_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(pa6t_attrs);
 		pmc_attrs = NULL;
 		break;
 #endif
@@ -944,14 +944,14 @@ static int unregister_cpu_online(unsigned int cpu)
 #ifdef HAS_PPC_PMC_IBM
 	case PPC_PMC_IBM:
 		attrs = ibm_common_attrs;
-		nattrs = sizeof(ibm_common_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(ibm_common_attrs);
 		pmc_attrs = classic_pmc_attrs;
 		break;
 #endif /* HAS_PPC_PMC_IBM */
 #ifdef HAS_PPC_PMC_G4
 	case PPC_PMC_G4:
 		attrs = g4_common_attrs;
-		nattrs = sizeof(g4_common_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(g4_common_attrs);
 		pmc_attrs = classic_pmc_attrs;
 		break;
 #endif /* HAS_PPC_PMC_G4 */
@@ -959,7 +959,7 @@ static int unregister_cpu_online(unsigned int cpu)
 	case PPC_PMC_PA6T:
 		/* PA Semi starts counting at PMC0 */
 		attrs = pa6t_attrs;
-		nattrs = sizeof(pa6t_attrs) / sizeof(struct device_attribute);
+		nattrs = ARRAY_SIZE(pa6t_attrs);
 		pmc_attrs = NULL;
 		break;
 #endif
-- 
2.6.2


^ permalink raw reply related

* Re: [PATCH V2 05/11] {x86,powerpc,microblaze}/kmap: Move preempt disable
From: Christoph Hellwig @ 2020-05-06  6:11 UTC (permalink / raw)
  To: ira.weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Huang Rui, Paul Mackerras,
	H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
	linux-csky, Ingo Molnar, linux-snps-arc, linux-xtensa,
	Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer, linux-parisc,
	linux-kernel, Christian Koenig, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200504010912.982044-6-ira.weiny@intel.com>

On Sun, May 03, 2020 at 06:09:06PM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> During this kmap() conversion series we must maintain bisect-ability.
> To do this, kmap_atomic_prot() in x86, powerpc, and microblaze need to
> remain functional.
> 
> Create a temporary inline version of kmap_atomic_prot within these
> architectures so we can rework their kmap_atomic() calls and then lift
> kmap_atomic_prot() to the core.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes from V1:
> 	New patch
> ---
>  arch/microblaze/include/asm/highmem.h | 11 ++++++++++-
>  arch/microblaze/mm/highmem.c          | 10 ++--------
>  arch/powerpc/include/asm/highmem.h    | 11 ++++++++++-
>  arch/powerpc/mm/highmem.c             |  9 ++-------
>  arch/x86/include/asm/highmem.h        | 11 ++++++++++-
>  arch/x86/mm/highmem_32.c              | 10 ++--------
>  6 files changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/highmem.h b/arch/microblaze/include/asm/highmem.h
> index 0c94046f2d58..ec9954b091e1 100644
> --- a/arch/microblaze/include/asm/highmem.h
> +++ b/arch/microblaze/include/asm/highmem.h
> @@ -51,7 +51,16 @@ extern pte_t *pkmap_page_table;
>  #define PKMAP_NR(virt)  ((virt - PKMAP_BASE) >> PAGE_SHIFT)
>  #define PKMAP_ADDR(nr)  (PKMAP_BASE + ((nr) << PAGE_SHIFT))
>  
> -extern void *kmap_atomic_prot(struct page *page, pgprot_t prot);
> +extern void *kmap_atomic_high_prot(struct page *page, pgprot_t prot);
> +void *kmap_atomic_prot(struct page *page, pgprot_t prot)

Shouldn't this be marked inline?

The rest looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH V2 06/11] arch/kmap_atomic: Consolidate duplicate code
From: Christoph Hellwig @ 2020-05-06  6:12 UTC (permalink / raw)
  To: ira.weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Huang Rui, Paul Mackerras,
	H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
	linux-csky, Ingo Molnar, linux-snps-arc, linux-xtensa,
	Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer, linux-parisc,
	linux-kernel, Christian Koenig, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200504010912.982044-7-ira.weiny@intel.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH V2 07/11] arch/kunmap_atomic: Consolidate duplicate code
From: Christoph Hellwig @ 2020-05-06  6:12 UTC (permalink / raw)
  To: ira.weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Huang Rui, Paul Mackerras,
	H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
	linux-csky, Ingo Molnar, linux-snps-arc, linux-xtensa,
	Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer, linux-parisc,
	linux-kernel, Christian Koenig, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200504010912.982044-8-ira.weiny@intel.com>

On Sun, May 03, 2020 at 06:09:08PM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Every single architecture (including !CONFIG_HIGHMEM) calls...
> 
> 	pagefault_enable();
> 	preempt_enable();
> 
> ... before returning from __kunmap_atomic().  Lift this code into the
> kunmap_atomic() macro.
> 
> While we are at it rename __kunmap_atomic() to kunmap_atomic_high() to
> be consistent.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* [PATCH V2 0/3] mm/hugetlb: Add some new generic fallbacks
From: Anshuman Khandual @ 2020-05-06  6:12 UTC (permalink / raw)
  To: linux-mm, akpm
  Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
	Heiko Carstens, linux-kernel, James E.J. Bottomley,
	Paul Mackerras, H. Peter Anvin, sparclinux, linux-riscv,
	Will Deacon, linux-arch, linux-s390, Yoshinori Sato, Helge Deller,
	x86, Russell King, Christian Borntraeger, Ingo Molnar, Fenghua Yu,
	Vasily Gorbik, Anshuman Khandual, Thomas Bogendoerfer,
	Borislav Petkov, Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
	Tony Luck, linux-parisc, linux-mips, Palmer Dabbelt, linuxppc-dev,
	David S. Miller, Mike Kravetz

This series adds the following new generic fallbacks. Before that it drops
__HAVE_ARCH_HUGE_PTEP_GET from arm64 platform.

1. is_hugepage_only_range()
2. arch_clear_hugepage_flags()

This has been boot tested on arm64 and x86 platforms but built tested on
some more platforms including the changed ones here. This series applies
on v5.7-rc4. After this arm (32 bit) remains the sole platform defining
it's own huge_ptep_get() via __HAVE_ARCH_HUGE_PTEP_GET.

Changes in V2:

- Adopted "#ifndef func" method (adding a single symbol to namespace) per Andrew
- Updated the commit messages in [PATCH 2/3] and [PATCH 3/3] as required

Changes in V1: (https://patchwork.kernel.org/project/linux-mm/list/?series=270677)

Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Anshuman Khandual (3):
  arm64/mm: Drop __HAVE_ARCH_HUGE_PTEP_GET
  mm/hugetlb: Define a generic fallback for is_hugepage_only_range()
  mm/hugetlb: Define a generic fallback for arch_clear_hugepage_flags()

 arch/arm/include/asm/hugetlb.h     |  7 +------
 arch/arm64/include/asm/hugetlb.h   | 13 +------------
 arch/ia64/include/asm/hugetlb.h    |  5 +----
 arch/mips/include/asm/hugetlb.h    | 11 -----------
 arch/parisc/include/asm/hugetlb.h  | 10 ----------
 arch/powerpc/include/asm/hugetlb.h |  5 +----
 arch/riscv/include/asm/hugetlb.h   | 10 ----------
 arch/s390/include/asm/hugetlb.h    |  8 +-------
 arch/sh/include/asm/hugetlb.h      |  7 +------
 arch/sparc/include/asm/hugetlb.h   | 10 ----------
 arch/x86/include/asm/hugetlb.h     | 10 ----------
 include/linux/hugetlb.h            | 14 ++++++++++++++
 12 files changed, 20 insertions(+), 90 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH V2 2/3] mm/hugetlb: Define a generic fallback for is_hugepage_only_range()
From: Anshuman Khandual @ 2020-05-06  6:12 UTC (permalink / raw)
  To: linux-mm, akpm
  Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
	Heiko Carstens, linux-kernel, James E.J. Bottomley,
	Paul Mackerras, H. Peter Anvin, sparclinux, linux-riscv,
	Will Deacon, linux-arch, linux-s390, Yoshinori Sato, Helge Deller,
	x86, Russell King, Christian Borntraeger, Ingo Molnar, Fenghua Yu,
	Vasily Gorbik, Anshuman Khandual, Thomas Bogendoerfer,
	Borislav Petkov, Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
	Tony Luck, linux-parisc, linux-mips, Palmer Dabbelt, linuxppc-dev,
	David S. Miller, Mike Kravetz
In-Reply-To: <1588745534-24418-1-git-send-email-anshuman.khandual@arm.com>

There are multiple similar definitions for is_hugepage_only_range() on
various platforms. Lets just add it's generic fallback definition for
platforms that do not override. This help reduce code duplication.

Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm/include/asm/hugetlb.h     | 6 ------
 arch/arm64/include/asm/hugetlb.h   | 6 ------
 arch/ia64/include/asm/hugetlb.h    | 1 +
 arch/mips/include/asm/hugetlb.h    | 7 -------
 arch/parisc/include/asm/hugetlb.h  | 6 ------
 arch/powerpc/include/asm/hugetlb.h | 1 +
 arch/riscv/include/asm/hugetlb.h   | 6 ------
 arch/s390/include/asm/hugetlb.h    | 7 -------
 arch/sh/include/asm/hugetlb.h      | 6 ------
 arch/sparc/include/asm/hugetlb.h   | 6 ------
 arch/x86/include/asm/hugetlb.h     | 6 ------
 include/linux/hugetlb.h            | 9 +++++++++
 12 files changed, 11 insertions(+), 56 deletions(-)

diff --git a/arch/arm/include/asm/hugetlb.h b/arch/arm/include/asm/hugetlb.h
index 318dcf5921ab..9ecd516d1ff7 100644
--- a/arch/arm/include/asm/hugetlb.h
+++ b/arch/arm/include/asm/hugetlb.h
@@ -14,12 +14,6 @@
 #include <asm/hugetlb-3level.h>
 #include <asm-generic/hugetlb.h>
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr, unsigned long len)
-{
-	return 0;
-}
-
 static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_dcache_clean, &page->flags);
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index b88878ddc88b..8f58e052697a 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -17,12 +17,6 @@
 extern bool arch_hugetlb_migration_supported(struct hstate *h);
 #endif
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr, unsigned long len)
-{
-	return 0;
-}
-
 static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_dcache_clean, &page->flags);
diff --git a/arch/ia64/include/asm/hugetlb.h b/arch/ia64/include/asm/hugetlb.h
index 36cc0396b214..6ef50b9a4bdf 100644
--- a/arch/ia64/include/asm/hugetlb.h
+++ b/arch/ia64/include/asm/hugetlb.h
@@ -20,6 +20,7 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,
 	return (REGION_NUMBER(addr) == RGN_HPAGE ||
 		REGION_NUMBER((addr)+(len)-1) == RGN_HPAGE);
 }
+#define is_hugepage_only_range is_hugepage_only_range
 
 #define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH
 static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
diff --git a/arch/mips/include/asm/hugetlb.h b/arch/mips/include/asm/hugetlb.h
index 425bb6fc3bda..8b201e281f67 100644
--- a/arch/mips/include/asm/hugetlb.h
+++ b/arch/mips/include/asm/hugetlb.h
@@ -11,13 +11,6 @@
 
 #include <asm/page.h>
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len)
-{
-	return 0;
-}
-
 #define __HAVE_ARCH_PREPARE_HUGEPAGE_RANGE
 static inline int prepare_hugepage_range(struct file *file,
 					 unsigned long addr,
diff --git a/arch/parisc/include/asm/hugetlb.h b/arch/parisc/include/asm/hugetlb.h
index 7cb595dcb7d7..411d9d867baa 100644
--- a/arch/parisc/include/asm/hugetlb.h
+++ b/arch/parisc/include/asm/hugetlb.h
@@ -12,12 +12,6 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
 pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
 			      pte_t *ptep);
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len) {
-	return 0;
-}
-
 /*
  * If the arch doesn't supply something else, assume that hugepage
  * size aligned regions are ok without further preparation.
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index bd6504c28c2f..b167c869d72d 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -30,6 +30,7 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,
 		return slice_is_hugepage_only_range(mm, addr, len);
 	return 0;
 }
+#define is_hugepage_only_range is_hugepage_only_range
 
 #define __HAVE_ARCH_HUGETLB_FREE_PGD_RANGE
 void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
diff --git a/arch/riscv/include/asm/hugetlb.h b/arch/riscv/include/asm/hugetlb.h
index 728a5db66597..866f6ae6467c 100644
--- a/arch/riscv/include/asm/hugetlb.h
+++ b/arch/riscv/include/asm/hugetlb.h
@@ -5,12 +5,6 @@
 #include <asm-generic/hugetlb.h>
 #include <asm/page.h>
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len) {
-	return 0;
-}
-
 static inline void arch_clear_hugepage_flags(struct page *page)
 {
 }
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
index de8f0bf5f238..7d27ea96ec2f 100644
--- a/arch/s390/include/asm/hugetlb.h
+++ b/arch/s390/include/asm/hugetlb.h
@@ -21,13 +21,6 @@ pte_t huge_ptep_get(pte_t *ptep);
 pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
 			      unsigned long addr, pte_t *ptep);
 
-static inline bool is_hugepage_only_range(struct mm_struct *mm,
-					  unsigned long addr,
-					  unsigned long len)
-{
-	return false;
-}
-
 /*
  * If the arch doesn't supply something else, assume that hugepage
  * size aligned regions are ok without further preparation.
diff --git a/arch/sh/include/asm/hugetlb.h b/arch/sh/include/asm/hugetlb.h
index 6f025fe18146..536ad2cb8aa4 100644
--- a/arch/sh/include/asm/hugetlb.h
+++ b/arch/sh/include/asm/hugetlb.h
@@ -5,12 +5,6 @@
 #include <asm/cacheflush.h>
 #include <asm/page.h>
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len) {
-	return 0;
-}
-
 /*
  * If the arch doesn't supply something else, assume that hugepage
  * size aligned regions are ok without further preparation.
diff --git a/arch/sparc/include/asm/hugetlb.h b/arch/sparc/include/asm/hugetlb.h
index 3963f80d1cb3..a056fe1119f5 100644
--- a/arch/sparc/include/asm/hugetlb.h
+++ b/arch/sparc/include/asm/hugetlb.h
@@ -20,12 +20,6 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
 pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
 			      pte_t *ptep);
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len) {
-	return 0;
-}
-
 #define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH
 static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
 					 unsigned long addr, pte_t *ptep)
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index f65cfb48cfdd..cc98f79074d0 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -7,12 +7,6 @@
 
 #define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
 
-static inline int is_hugepage_only_range(struct mm_struct *mm,
-					 unsigned long addr,
-					 unsigned long len) {
-	return 0;
-}
-
 static inline void arch_clear_hugepage_flags(struct page *page)
 {
 }
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 43a1cef8f0f1..c01c0c6f7fd4 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -591,6 +591,15 @@ static inline unsigned int blocks_per_huge_page(struct hstate *h)
 
 #include <asm/hugetlb.h>
 
+#ifndef is_hugepage_only_range
+static inline int is_hugepage_only_range(struct mm_struct *mm,
+					unsigned long addr, unsigned long len)
+{
+	return 0;
+}
+#define is_hugepage_only_range is_hugepage_only_range
+#endif
+
 #ifndef arch_make_huge_pte
 static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
 				       struct page *page, int writable)
-- 
2.20.1


^ permalink raw reply related

* [PATCH V2 3/3] mm/hugetlb: Define a generic fallback for arch_clear_hugepage_flags()
From: Anshuman Khandual @ 2020-05-06  6:12 UTC (permalink / raw)
  To: linux-mm, akpm
  Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
	Heiko Carstens, linux-kernel, James E.J. Bottomley,
	Paul Mackerras, H. Peter Anvin, sparclinux, linux-riscv,
	Will Deacon, linux-arch, linux-s390, Yoshinori Sato, Helge Deller,
	x86, Russell King, Christian Borntraeger, Ingo Molnar, Fenghua Yu,
	Vasily Gorbik, Anshuman Khandual, Thomas Bogendoerfer,
	Borislav Petkov, Paul Walmsley, Thomas Gleixner, linux-arm-kernel,
	Tony Luck, linux-parisc, linux-mips, Palmer Dabbelt, linuxppc-dev,
	David S. Miller, Mike Kravetz
In-Reply-To: <1588745534-24418-1-git-send-email-anshuman.khandual@arm.com>

There are multiple similar definitions for arch_clear_hugepage_flags() on
various platforms. Lets just add it's generic fallback definition for
platforms that do not override. This help reduce code duplication.

Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm/include/asm/hugetlb.h     | 1 +
 arch/arm64/include/asm/hugetlb.h   | 1 +
 arch/ia64/include/asm/hugetlb.h    | 4 ----
 arch/mips/include/asm/hugetlb.h    | 4 ----
 arch/parisc/include/asm/hugetlb.h  | 4 ----
 arch/powerpc/include/asm/hugetlb.h | 4 ----
 arch/riscv/include/asm/hugetlb.h   | 4 ----
 arch/s390/include/asm/hugetlb.h    | 1 +
 arch/sh/include/asm/hugetlb.h      | 1 +
 arch/sparc/include/asm/hugetlb.h   | 4 ----
 arch/x86/include/asm/hugetlb.h     | 4 ----
 include/linux/hugetlb.h            | 5 +++++
 12 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/arch/arm/include/asm/hugetlb.h b/arch/arm/include/asm/hugetlb.h
index 9ecd516d1ff7..d02d6ca88e92 100644
--- a/arch/arm/include/asm/hugetlb.h
+++ b/arch/arm/include/asm/hugetlb.h
@@ -18,5 +18,6 @@ static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_dcache_clean, &page->flags);
 }
+#define arch_clear_hugepage_flags arch_clear_hugepage_flags
 
 #endif /* _ASM_ARM_HUGETLB_H */
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index 8f58e052697a..94ba0c5bced2 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -21,6 +21,7 @@ static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_dcache_clean, &page->flags);
 }
+#define arch_clear_hugepage_flags arch_clear_hugepage_flags
 
 extern pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
 				struct page *page, int writable);
diff --git a/arch/ia64/include/asm/hugetlb.h b/arch/ia64/include/asm/hugetlb.h
index 6ef50b9a4bdf..7e46ebde8c0c 100644
--- a/arch/ia64/include/asm/hugetlb.h
+++ b/arch/ia64/include/asm/hugetlb.h
@@ -28,10 +28,6 @@ static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
 {
 }
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #include <asm-generic/hugetlb.h>
 
 #endif /* _ASM_IA64_HUGETLB_H */
diff --git a/arch/mips/include/asm/hugetlb.h b/arch/mips/include/asm/hugetlb.h
index 8b201e281f67..10e3be870df7 100644
--- a/arch/mips/include/asm/hugetlb.h
+++ b/arch/mips/include/asm/hugetlb.h
@@ -75,10 +75,6 @@ static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 	return changed;
 }
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #include <asm-generic/hugetlb.h>
 
 #endif /* __ASM_HUGETLB_H */
diff --git a/arch/parisc/include/asm/hugetlb.h b/arch/parisc/include/asm/hugetlb.h
index 411d9d867baa..a69cf9efb0c1 100644
--- a/arch/parisc/include/asm/hugetlb.h
+++ b/arch/parisc/include/asm/hugetlb.h
@@ -42,10 +42,6 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 					     unsigned long addr, pte_t *ptep,
 					     pte_t pte, int dirty);
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #include <asm-generic/hugetlb.h>
 
 #endif /* _ASM_PARISC64_HUGETLB_H */
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index b167c869d72d..e6dfa63da552 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -61,10 +61,6 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 			       unsigned long addr, pte_t *ptep,
 			       pte_t pte, int dirty);
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #include <asm-generic/hugetlb.h>
 
 #else /* ! CONFIG_HUGETLB_PAGE */
diff --git a/arch/riscv/include/asm/hugetlb.h b/arch/riscv/include/asm/hugetlb.h
index 866f6ae6467c..a5c2ca1d1cd8 100644
--- a/arch/riscv/include/asm/hugetlb.h
+++ b/arch/riscv/include/asm/hugetlb.h
@@ -5,8 +5,4 @@
 #include <asm-generic/hugetlb.h>
 #include <asm/page.h>
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #endif /* _ASM_RISCV_HUGETLB_H */
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
index 7d27ea96ec2f..9ddf4a43a590 100644
--- a/arch/s390/include/asm/hugetlb.h
+++ b/arch/s390/include/asm/hugetlb.h
@@ -39,6 +39,7 @@ static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_arch_1, &page->flags);
 }
+#define arch_clear_hugepage_flags arch_clear_hugepage_flags
 
 static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
 				  pte_t *ptep, unsigned long sz)
diff --git a/arch/sh/include/asm/hugetlb.h b/arch/sh/include/asm/hugetlb.h
index 536ad2cb8aa4..ae4de7b89210 100644
--- a/arch/sh/include/asm/hugetlb.h
+++ b/arch/sh/include/asm/hugetlb.h
@@ -30,6 +30,7 @@ static inline void arch_clear_hugepage_flags(struct page *page)
 {
 	clear_bit(PG_dcache_clean, &page->flags);
 }
+#define arch_clear_hugepage_flags arch_clear_hugepage_flags
 
 #include <asm-generic/hugetlb.h>
 
diff --git a/arch/sparc/include/asm/hugetlb.h b/arch/sparc/include/asm/hugetlb.h
index a056fe1119f5..53838a173f62 100644
--- a/arch/sparc/include/asm/hugetlb.h
+++ b/arch/sparc/include/asm/hugetlb.h
@@ -47,10 +47,6 @@ static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
 	return changed;
 }
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #define __HAVE_ARCH_HUGETLB_FREE_PGD_RANGE
 void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
 			    unsigned long end, unsigned long floor,
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index cc98f79074d0..1721b1aadeb1 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -7,8 +7,4 @@
 
 #define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
 
-static inline void arch_clear_hugepage_flags(struct page *page)
-{
-}
-
 #endif /* _ASM_X86_HUGETLB_H */
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index c01c0c6f7fd4..04bc794becfc 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -600,6 +600,11 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,
 #define is_hugepage_only_range is_hugepage_only_range
 #endif
 
+#ifndef arch_clear_hugepage_flags
+static inline void arch_clear_hugepage_flags(struct page *page) { }
+#define arch_clear_hugepage_flags arch_clear_hugepage_flags
+#endif
+
 #ifndef arch_make_huge_pte
 static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
 				       struct page *page, int writable)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH V2 08/11] arch/kmap: Ensure kmap_prot visibility
From: Christoph Hellwig @ 2020-05-06  6:13 UTC (permalink / raw)
  To: ira.weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Huang Rui, Paul Mackerras,
	H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
	linux-csky, Ingo Molnar, linux-snps-arc, linux-xtensa,
	Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer, linux-parisc,
	linux-kernel, Christian Koenig, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200504010912.982044-9-ira.weiny@intel.com>

On Sun, May 03, 2020 at 06:09:09PM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> We want to support kmap_atomic_prot() on all architectures and it makes
> sense to define kmap_atomic() to use the default kmap_prot.
> 
> So we ensure all arch's have a globally available kmap_prot either as a
> define or exported symbol.

FYI, I still think a

#ifndef kmap_prot
#define kmap_prot PAGE_KERNEL
#endif

in linux/highmem.h would be nicer.  Then only xtensa and sparc need
to override it and clearly stand out.

^ permalink raw reply

* Re: [PATCH V2 10/11] arch/kmap: Define kmap_atomic_prot() for all arch's
From: Christoph Hellwig @ 2020-05-06  6:14 UTC (permalink / raw)
  To: ira.weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Huang Rui, Paul Mackerras,
	H. Peter Anvin, sparclinux, Dan Williams, Helge Deller, x86,
	linux-csky, Ingo Molnar, linux-snps-arc, linux-xtensa,
	Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer, linux-parisc,
	linux-kernel, Christian Koenig, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200504010912.982044-11-ira.weiny@intel.com>

On Sun, May 03, 2020 at 06:09:11PM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> To support kmap_atomic_prot(), all architectures need to support
> protections passed to their kmap_atomic_high() function.  Pass
> protections into kmap_atomic_high() and change the name to
> kmap_atomic_high_prot() to match.
> 
> Then define kmap_atomic_prot() as a core function which calls
> kmap_atomic_high_prot() when needed.
> 
> Finally, redefine kmap_atomic() as a wrapper of kmap_atomic_prot() with
> the default kmap_prot exported by the architectures.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH] powerpc/5200: update contact email
From: Michael Ellerman @ 2020-05-06  6:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree, linux-kernel, Rob Herring, Paul Mackerras, kernel,
	linuxppc-dev
In-Reply-To: <20200505160410.GH2468@ninjato>

Wolfram Sang <wsa@kernel.org> writes:
>> > My 'pengutronix' address is defunct for years. Merge the entries and use
>> > the proper contact address.
>> 
>> Is there any point adding the new address? It's just likely to bit-rot
>> one day too.
>
> At least, this one is a group address, not an individual one, so less
> likey.
>
>> I figure the git history is a better source for more up-to-date emails.
>
> But yes, can still be argued. I won't persist if you don't like it.

That's fine, I'll merge this. You've already gone to the trouble to send
it and it's better than what we have now.

cheers

^ permalink raw reply

* Re: remove set_fs calls from the coredump code v6
From: Christoph Hellwig @ 2020-05-06  6:31 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arnd Bergmann, linuxppc-dev, the arch/x86 maintainers,
	Oleg Nesterov, Linux Kernel Mailing List, Jeremy Kerr,
	linux-fsdevel, Andrew Morton, Linus Torvalds, Christoph Hellwig,
	Alexander Viro
In-Reply-To: <877dxqgm7x.fsf@x220.int.ebiederm.org>

On Tue, May 05, 2020 at 03:28:50PM -0500, Eric W. Biederman wrote:
> We probably can.   After introducing a kernel_compat_siginfo that is
> the size that userspace actually would need.
> 
> It isn't something I want to mess with until this code gets merged, as I
> think the set_fs cleanups are more important.
> 
> 
> Christoph made some good points about how ugly the #ifdefs are in
> the generic copy_siginfo_to_user32 implementation.

Take a look at the series you are replying to, the magic x86 ifdefs are
entirely gone from the common code :)

^ permalink raw reply

* [PATCH] powerpc/8xx: Update email address in MAINTAINERS
From: Christophe Leroy @ 2020-05-06  6:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Since 01 May 2020, our email adresses have changed to @csgroup.eu

Update MAINTAINERS accordingly.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2926327e4976..e8714328cc90 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9794,7 +9794,7 @@ F:	arch/powerpc/platforms/83xx/
 F:	arch/powerpc/platforms/85xx/
 
 LINUX FOR POWERPC EMBEDDED PPC8XX
-M:	Christophe Leroy <christophe.leroy@c-s.fr>
+M:	Christophe Leroy <christophe.leroy@csgroup.eu>
 L:	linuxppc-dev@lists.ozlabs.org
 S:	Maintained
 F:	arch/powerpc/platforms/8xx/
-- 
2.25.0


^ permalink raw reply related

* Re: [RFC PATCH 06/10] powerpc/powernv: opal use new opal call entry point if it exists
From: Gautham R Shenoy @ 2020-05-06  7:02 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20200502111914.166578-7-npiggin@gmail.com>

Hello Nicholas,

On Sat, May 02, 2020 at 09:19:10PM +1000, Nicholas Piggin wrote:
> OPAL may advertise new endian-specific entry point which has different
> calling conventions including using the caller's stack, but otherwise
> provides the standard OPAL call API without any changes required to
> the OS.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

[..snip..]

> index 506b1798081a..32857254d268 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -92,6 +92,18 @@ static s64 __opal_call_trace(s64 a0, s64 a1, s64 a2, s64 a3,
>  #define DO_TRACE false
>  #endif /* CONFIG_TRACEPOINTS */
> 
> +struct opal {
> +	u64 base;
> +	u64 entry;
> +	u64 size;
> +	u64 v4_le_entry;
> +};
> +extern struct opal opal;
> +
> +typedef int64_t (*opal_v4_le_entry_fn)(uint64_t r3, uint64_t r4, uint64_t r5,
> +                               uint64_t r6, uint64_t r7, uint64_t r8,
> +                               uint64_t r9, uint64_t r10);
> +
>  static int64_t opal_call(int64_t a0, int64_t a1, int64_t a2, int64_t a3,
>  	     int64_t a4, int64_t a5, int64_t a6, int64_t a7, int64_t opcode)
>  {
> @@ -99,6 +111,30 @@ static int64_t opal_call(int64_t a0, int64_t a1, int64_t a2, int64_t a3,
>  	unsigned long msr = mfmsr();
>  	bool mmu = (msr & (MSR_IR|MSR_DR));
>  	int64_t ret;
> +	opal_v4_le_entry_fn fn;

fn should be initialized to NULL here to ensure correctness when
kernel is built with CONFIG_CPU_BIG_ENDIAN.

> +
> +	if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN))
> +		fn = (opal_v4_le_entry_fn)(opal.v4_le_entry);
> +
> +	if (fn) {
> +		if (!mmu) {
> +			BUG_ON(msr & MSR_EE);
> +			ret = fn(opcode, a0, a1, a2, a3, a4, a5, a6);
> +			return ret;
> +		}
> +
> +		local_irq_save(flags);
> +		hard_irq_disable(); /* XXX r13 */
> +		msr &= ~MSR_EE;
> +		mtmsr(msr & ~(MSR_IR|MSR_DR));
> +
> +		ret = fn(opcode, a0, a1, a2, a3, a4, a5, a6);
> +
> +		mtmsr(msr);
> +		local_irq_restore(flags);
> +
> +		return ret;
> +	}
> 
>  	msr &= ~MSR_EE;
> 

--
Thanks and Regards
gautham.

^ permalink raw reply

* Re: [PATCH] powerpc/xive: Enforce load-after-store ordering when StoreEOI is active
From: Cédric Le Goater @ 2020-05-06  6:58 UTC (permalink / raw)
  To: Alistair Popple; +Cc: linuxppc-dev, Paul Mackerras, Greg Kurz
In-Reply-To: <2990884.uye9WAk7yu@townsend>

On 5/6/20 1:34 AM, Alistair Popple wrote:
> I am still slowly wrapping my head around XIVE and it's interaction with KVM 
> but from what I can see this looks good and is needed so we can enable 
> StoreEOI support in future so:
> 
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
> 
> On Thursday, 20 February 2020 7:15:06 PM AEST Cédric Le Goater wrote:
>> When an interrupt has been handled, the OS notifies the interrupt
>> controller with a EOI sequence. On a POWER9 system using the XIVE
>> interrupt controller, this can be done with a load or a store
>> operation on the ESB interrupt management page of the interrupt. The
>> StoreEOI operation has less latency and improves interrupt handling
>> performance but it was deactivated during the POWER9 DD2.0 timeframe
>> because of ordering issues. We use the LoadEOI today but we plan to
>> reactivate StoreEOI in future architectures.

StoreEOI was considered broken on P9, and never used, but we have to 
prepare ground for the IBM hypervisor which will activate StoreEOI 
without negotiation on the new architecture.

Thanks,

C. 


>>
>> There is usually no need to enforce ordering between ESB load and
>> store operations as they should lead to the same result. E.g. a store
>> trigger and a load EOI can be executed in any order. Assuming the
>> interrupt state is PQ=10, a store trigger followed by a load EOI will
>> return a Q bit. In the reverse order, it will create a new interrupt
>> trigger from HW. In both cases, the handler processing interrupts is
>> notified.
>>
>> In some cases, the XIVE_ESB_SET_PQ_10 load operation is used to
>> disable temporarily the interrupt source (mask/unmask). When the
>> source is reenabled, the OS can detect if interrupts were received
>> while the source was disabled and reinject them. This process needs
>> special care when StoreEOI is activated. The ESB load and store
>> operations should be correctly ordered because a XIVE_ESB_STORE_EOI
>> operation could leave the source enabled if it has not completed
>> before the loads.
>>
>> For those cases, we enforce Load-after-Store ordering with a special
>> load operation offset. To avoid performance impact, this ordering is
>> only enforced when really needed, that is when interrupt sources are
>> temporarily disabled with the XIVE_ESB_SET_PQ_10 load. It should not
>> be needed for other loads.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  arch/powerpc/include/asm/xive-regs.h    | 8 ++++++++
>>  arch/powerpc/kvm/book3s_xive_native.c   | 6 ++++++
>>  arch/powerpc/kvm/book3s_xive_template.c | 3 +++
>>  arch/powerpc/sysdev/xive/common.c       | 3 +++
>>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 5 +++++
>>  5 files changed, 25 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/xive-regs.h
>> b/arch/powerpc/include/asm/xive-regs.h index f2dfcd50a2d3..b1996fbae59a
>> 100644
>> --- a/arch/powerpc/include/asm/xive-regs.h
>> +++ b/arch/powerpc/include/asm/xive-regs.h
>> @@ -37,6 +37,14 @@
>>  #define XIVE_ESB_SET_PQ_10	0xe00 /* Load */
>>  #define XIVE_ESB_SET_PQ_11	0xf00 /* Load */
>>
>> +/*
>> + * Load-after-store ordering
>> + *
>> + * Adding this offset to the load address will enforce
>> + * load-after-store ordering. This is required to use StoreEOI.
>> + */
>> +#define XIVE_ESB_LD_ST_MO	0x40 /* Load-after-store ordering */
>> +
>>  #define XIVE_ESB_VAL_P		0x2
>>  #define XIVE_ESB_VAL_Q		0x1
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c
>> b/arch/powerpc/kvm/book3s_xive_native.c index d83adb1e1490..c80b6a447efd
>> 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -31,6 +31,12 @@ static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32
>> offset) {
>>  	u64 val;
>>
>> +	/*
>> +	 * The KVM XIVE native device does not use the XIVE_ESB_SET_PQ_10
>> +	 * load operation, so there is no need to enforce load-after-store
>> +	 * ordering.
>> +	 */
>> +
>>  	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
>>  		offset |= offset << 4;
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive_template.c
>> b/arch/powerpc/kvm/book3s_xive_template.c index a8a900ace1e6..4ad3c0279458
>> 100644
>> --- a/arch/powerpc/kvm/book3s_xive_template.c
>> +++ b/arch/powerpc/kvm/book3s_xive_template.c
>> @@ -58,6 +58,9 @@ static u8 GLUE(X_PFX,esb_load)(struct xive_irq_data *xd,
>> u32 offset) {
>>  	u64 val;
>>
>> +	if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>> +		offset |= XIVE_ESB_LD_ST_MO;
>> +
>>  	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
>>  		offset |= offset << 4;
>>
>> diff --git a/arch/powerpc/sysdev/xive/common.c
>> b/arch/powerpc/sysdev/xive/common.c index f5fadbd2533a..0dc421bb494f 100644
>> --- a/arch/powerpc/sysdev/xive/common.c
>> +++ b/arch/powerpc/sysdev/xive/common.c
>> @@ -202,6 +202,9 @@ static notrace u8 xive_esb_read(struct xive_irq_data
>> *xd, u32 offset) {
>>  	u64 val;
>>
>> +	if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>> +		offset |= XIVE_ESB_LD_ST_MO;
>> +
>>  	/* Handle HW errata */
>>  	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
>>  		offset |= offset << 4;
>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index e11017897eb0..abe132ff2346
>> 100644
>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> @@ -2911,6 +2911,11 @@ kvm_cede_exit:
>>  	beq	4f
>>  	li	r0, 0
>>  	stb	r0, VCPU_CEDED(r9)
>> +	/*
>> +	 * The escalation interrupts are special as we don't EOI them.
>> +	 * There is no need to use the load-after-store ordering offset
>> +	 * to set PQ to 10 as we won't use StoreEOI.
>> +	 */
>>  	li	r6, XIVE_ESB_SET_PQ_10
>>  	b	5f
>>  4:	li	r0, 1
> 
> 
> 
> 


^ permalink raw reply

* Re: [PATCH 2/2] powerpc/perf: Add support for outputting extended regs in perf intr_regs
From: Athira Rajeev @ 2020-05-06  7:54 UTC (permalink / raw)
  To: Madhavan Srinivasan, linuxppc-dev
  Cc: ravi.bangoria, maddy, linux-kernel, acme, anju@linux.vnet.ibm.com,
	jolsa
In-Reply-To: <b3c3c1e6-077b-89d0-d550-b2784577ccd9@linux.ibm.com>

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



> On 06-May-2020, at 9:56 AM, Madhavan Srinivasan <maddy@linux.ibm.com> wrote:
> 
> 
> 
> On 4/29/20 11:34 AM, Anju T Sudhakar wrote:
>> The capability flag PERF_PMU_CAP_EXTENDED_REGS, is used to indicate the
>> PMU which support extended registers. The generic code define the mask
>> of extended registers as 0 for non supported architectures.
>> 
>> Add support for extended registers in POWER9 architecture. For POWER9,
>> the extended registers are mmcr0, mmc1 and mmcr2.
>> 
>> REG_RESERVED mask is redefined to accommodate the extended registers.
>> 
>> With patch:
>> ----------------
>> 
>> # perf record -I?
>> available registers: r0 r1 r2 r3 r4 r5 r6 r7 r8 r9 r10 r11 r12 r13 r14
>> r15 r16 r17 r18 r19 r20 r21 r22 r23 r24 r25 r26 r27 r28 r29 r30 r31 nip
>> msr orig_r3 ctr link xer ccr softe trap dar dsisr sier mmcra mmcr0
>> mmcr1 mmcr2
> 

> Would prefer to have some flexibility in deciding what to expose
> in as extended regs. Meaning say if we want to add extended regs
> in power8 and if we dont want to show for ex say mmcr2 (just for example).
> 

One way to approach this is to have the "extended mask" exposed in 
sysfs: "/sys/bus/event_source/devices/cpu/caps/ext_regs_mask" by the platform pmu
driver. This way the perf tool side can look at this and platform driver will also have control 
on what to expose as part of the extended regs.

perf tools side uses extended mask to display the platform supported register names (with -I? option)
to the user and also send this mask to the kernel to capture the extended registers in each sample. 
Hence we need to expose the appropriated mask to the perf tool side.

Thanks
Athira

> Maddy
> 
>> 
>> # perf record -I ls
>> # perf script -D
>> 
>> PERF_RECORD_SAMPLE(IP, 0x1): 9019/9019: 0 period: 1 addr: 0
>> ... intr regs: mask 0xffffffffffff ABI 64-bit
>> .... r0    0xc00000000011b12c
>> .... r1    0xc000003f9a98b930
>> .... r2    0xc000000001a32100
>> .... r3    0xc000003f8fe9a800
>> .... r4    0xc000003fd1810000
>> .... r5    0x3e32557150
>> .... r6    0xc000003f9a98b908
>> .... r7    0xffffffc1cdae06ac
>> .... r8    0x818
>> [.....]
>> .... r31   0xc000003ffd047230
>> .... nip   0xc00000000011b2c0
>> .... msr   0x9000000000009033
>> .... orig_r3 0xc00000000011b21c
>> .... ctr   0xc000000000119380
>> .... link  0xc00000000011b12c
>> .... xer   0x0
>> .... ccr   0x28002222
>> .... softe 0x1
>> .... trap  0xf00
>> .... dar   0x0
>> .... dsisr 0x80000000000
>> .... sier  0x0
>> .... mmcra 0x80000000000
>> .... mmcr0 0x82008090
>> .... mmcr1 0x1e000000
>> .... mmcr2 0x0
>>  ... thread: perf:9019
>> 
>> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/perf_event_server.h  |  5 +++
>>  arch/powerpc/include/uapi/asm/perf_regs.h     | 13 +++++++-
>>  arch/powerpc/perf/core-book3s.c               |  1 +
>>  arch/powerpc/perf/perf_regs.c                 | 29 ++++++++++++++--
>>  arch/powerpc/perf/power9-pmu.c                |  1 +
>>  .../arch/powerpc/include/uapi/asm/perf_regs.h | 13 +++++++-
>>  tools/perf/arch/powerpc/include/perf_regs.h   |  6 +++-
>>  tools/perf/arch/powerpc/util/perf_regs.c      | 33 +++++++++++++++++++
>>  8 files changed, 95 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
>> index 3e9703f44c7c..1d15953bd99e 100644
>> --- a/arch/powerpc/include/asm/perf_event_server.h
>> +++ b/arch/powerpc/include/asm/perf_event_server.h
>> @@ -55,6 +55,11 @@ struct power_pmu {
>>  	int 		*blacklist_ev;
>>  	/* BHRB entries in the PMU */
>>  	int		bhrb_nr;
>> +	/*
>> +	 * set this flag with `PERF_PMU_CAP_EXTENDED_REGS` if
>> +	 * the pmu supports extended perf regs capability
>> +	 */
>> +	int		capabilities;
>>  };
>> 
>>  /*
>> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
>> index f599064dd8dc..604b831378fe 100644
>> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
>> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
>> @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
>>  	PERF_REG_POWERPC_DSISR,
>>  	PERF_REG_POWERPC_SIER,
>>  	PERF_REG_POWERPC_MMCRA,
>> -	PERF_REG_POWERPC_MAX,
>> +	/* Extended registers */
>> +	PERF_REG_POWERPC_MMCR0,
>> +	PERF_REG_POWERPC_MMCR1,
>> +	PERF_REG_POWERPC_MMCR2,
>> +	PERF_REG_EXTENDED_MAX,
>> +	/* Max regs without the extended regs */
>> +	PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
>>  };
>> +
>> +#define PERF_REG_PMU_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
>> +#define PERF_REG_EXTENDED_MASK  (((1ULL << (PERF_REG_EXTENDED_MAX))	\
>> +			- 1) - PERF_REG_PMU_MASK)
>> +
>>  #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index 3dcfecf858f3..f56b77800a7b 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -2276,6 +2276,7 @@ int register_power_pmu(struct power_pmu *pmu)
>> 
>>  	power_pmu.attr_groups = ppmu->attr_groups;
>> 
>> +	power_pmu.capabilities |= (ppmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS);
>>  #ifdef MSR_HV
>>  	/*
>>  	 * Use FCHV to ignore kernel events if MSR.HV is set.
>> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
>> index a213a0aa5d25..57aa02568caf 100644
>> --- a/arch/powerpc/perf/perf_regs.c
>> +++ b/arch/powerpc/perf/perf_regs.c
>> @@ -15,7 +15,8 @@
>> 
>>  #define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r)
>> 
>> -#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1))
>> +#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK) &	\
>> +			(~((1ULL << PERF_REG_POWERPC_MAX) - 1)))
>> 
>>  static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>>  	PT_REGS_OFFSET(PERF_REG_POWERPC_R0,  gpr[0]),
>> @@ -69,10 +70,22 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>>  	PT_REGS_OFFSET(PERF_REG_POWERPC_MMCRA, dsisr),
>>  };
>> 
>> +/* Function to return the extended register values */
>> +static u64 get_ext_regs_value(int idx)
>> +{
>> +	switch (idx) {
>> +	case PERF_REG_POWERPC_MMCR0:
>> +				    return mfspr(SPRN_MMCR0);
>> +	case PERF_REG_POWERPC_MMCR1:
>> +				    return mfspr(SPRN_MMCR1);
>> +	case PERF_REG_POWERPC_MMCR2:
>> +				    return mfspr(SPRN_MMCR2);
>> +	default: return 0;
>> +	}
>> +}
>> +
>>  u64 perf_reg_value(struct pt_regs *regs, int idx)
>>  {
>> -	if (WARN_ON_ONCE(idx >= PERF_REG_POWERPC_MAX))
>> -		return 0;
>> 
>>  	if (idx == PERF_REG_POWERPC_SIER &&
>>  	   (IS_ENABLED(CONFIG_FSL_EMB_PERF_EVENT) ||
>> @@ -85,6 +98,16 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>>  	    IS_ENABLED(CONFIG_PPC32)))
>>  		return 0;
>> 
>> +	if (idx >= PERF_REG_POWERPC_MAX && idx < PERF_REG_EXTENDED_MAX)
>> +		return get_ext_regs_value(idx);
>> +
>> +	/*
>> +	 * If the idx is referring to value beyond the
>> +	 * supported registers, return 0 with a warning
>> +	 */
>> +	if (WARN_ON_ONCE(idx >= PERF_REG_EXTENDED_MAX))
>> +		return 0;
>> +
>>  	return regs_get_register(regs, pt_regs_offset[idx]);
>>  }
>> 
>> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
>> index 08c3ef796198..c37193b3e73f 100644
>> --- a/arch/powerpc/perf/power9-pmu.c
>> +++ b/arch/powerpc/perf/power9-pmu.c
>> @@ -434,6 +434,7 @@ static struct power_pmu power9_pmu = {
>>  	.cache_events		= &power9_cache_events,
>>  	.attr_groups		= power9_pmu_attr_groups,
>>  	.bhrb_nr		= 32,
>> +	.capabilities		= PERF_PMU_CAP_EXTENDED_REGS,
>>  };
>> 
>>  int init_power9_pmu(void)
>> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> index f599064dd8dc..d66953294c73 100644
>> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
>>  	PERF_REG_POWERPC_DSISR,
>>  	PERF_REG_POWERPC_SIER,
>>  	PERF_REG_POWERPC_MMCRA,
>> -	PERF_REG_POWERPC_MAX,
>> +	/* Extended arch registers */
>> +	PERF_REG_POWERPC_MMCR0,
>> +	PERF_REG_POWERPC_MMCR1,
>> +	PERF_REG_POWERPC_MMCR2,
>> +	PERF_REG_EXTENDED_MAX,
>> +	/* Max regs without extended arch regs */
>> +	PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
>> +
>>  };
>> +#define PERF_REG_PMU_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
>> +#define PERF_REG_EXTENDED_MASK  (((1ULL << (PERF_REG_EXTENDED_MAX))\
>> +			- 1) - PERF_REG_PMU_MASK)
>> +
>>  #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
>> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
>> index e18a3556f5e3..f7bbdb816f88 100644
>> --- a/tools/perf/arch/powerpc/include/perf_regs.h
>> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
>> @@ -64,7 +64,11 @@ static const char *reg_names[] = {
>>  	[PERF_REG_POWERPC_DAR] = "dar",
>>  	[PERF_REG_POWERPC_DSISR] = "dsisr",
>>  	[PERF_REG_POWERPC_SIER] = "sier",
>> -	[PERF_REG_POWERPC_MMCRA] = "mmcra"
>> +	[PERF_REG_POWERPC_MMCRA] = "mmcra",
>> +	[PERF_REG_POWERPC_MMCR0] = "mmcr0",
>> +	[PERF_REG_POWERPC_MMCR1] = "mmcr1",
>> +	[PERF_REG_POWERPC_MMCR2] = "mmcr2",
>> +
>>  };
>> 
>>  static inline const char *perf_reg_name(int id)
>> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
>> index 0a5242900248..37b150f9d1a1 100644
>> --- a/tools/perf/arch/powerpc/util/perf_regs.c
>> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
>> @@ -6,6 +6,8 @@
>> 
>>  #include "../../../util/perf_regs.h"
>>  #include "../../../util/debug.h"
>> +#include "../../../util/event.h"
>> +#include "../../../perf-sys.h"
>> 
>>  #include <linux/kernel.h>
>> 
>> @@ -55,6 +57,9 @@ const struct sample_reg sample_reg_masks[] = {
>>  	SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
>>  	SMPL_REG(sier, PERF_REG_POWERPC_SIER),
>>  	SMPL_REG(mmcra, PERF_REG_POWERPC_MMCRA),
>> +	SMPL_REG(mmcr0, PERF_REG_POWERPC_MMCR0),
>> +	SMPL_REG(mmcr1, PERF_REG_POWERPC_MMCR1),
>> +	SMPL_REG(mmcr2, PERF_REG_POWERPC_MMCR2),
>>  	SMPL_REG_END
>>  };
>> 
>> @@ -163,3 +168,31 @@ int arch_sdt_arg_parse_op(char *old_op, char **new_op)
>> 
>>  	return SDT_ARG_VALID;
>>  }
>> +
>> +uint64_t arch__intr_reg_mask(void)
>> +{
>> +	struct perf_event_attr attr = {
>> +		.type                   = PERF_TYPE_HARDWARE,
>> +		.config                 = PERF_COUNT_HW_CPU_CYCLES,
>> +		.sample_type            = PERF_SAMPLE_REGS_INTR,
>> +		.sample_regs_intr       = PERF_REG_EXTENDED_MASK,
>> +		.precise_ip             = 1,
>> +		.disabled               = 1,
>> +		.exclude_kernel         = 1,
>> +	};
>> +	int fd;
>> +
>> +	attr.sample_period = 1;
>> +	event_attr_init(&attr);
>> +
>> +	/*
>> +	 * check if the pmu supports perf extended regs, before
>> +	 * returning the register mask to sample.
>> +	 */
>> +	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
>> +	if (fd != -1) {
>> +		close(fd);
>> +		return (PERF_REG_EXTENDED_MASK | PERF_REGS_MASK);
>> +	}
>> +	return PERF_REGS_MASK;
>> +}


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

^ permalink raw reply

* Re: [PATCH] tty: hvc: Fix data abort due to race in hvc_open
From: Greg KH @ 2020-05-06  9:48 UTC (permalink / raw)
  To: Raghavendra Rao Ananta; +Cc: andrew, linuxppc-dev, linux-kernel, jslaby
In-Reply-To: <20200428032601.22127-1-rananta@codeaurora.org>

On Mon, Apr 27, 2020 at 08:26:01PM -0700, Raghavendra Rao Ananta wrote:
> Potentially, hvc_open() can be called in parallel when two tasks calls
> open() on /dev/hvcX. In such a scenario, if the hp->ops->notifier_add()
> callback in the function fails, where it sets the tty->driver_data to
> NULL, the parallel hvc_open() can see this NULL and cause a memory abort.
> Hence, serialize hvc_open and check if tty->private_data is NULL before
> proceeding ahead.
> 
> The issue can be easily reproduced by launching two tasks simultaneously
> that does nothing but open() and close() on /dev/hvcX.
> For example:
> $ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@codeaurora.org>
> ---
>  drivers/tty/hvc/hvc_console.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index 436cc51c92c3..ebe26fe5ac09 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
>   */
>  static DEFINE_MUTEX(hvc_structs_mutex);
>  
> +/* Mutex to serialize hvc_open */
> +static DEFINE_MUTEX(hvc_open_mutex);
>  /*
>   * This value is used to assign a tty->index value to a hvc_struct based
>   * upon order of exposure via hvc_probe(), when we can not match it to
> @@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver *driver, struct tty_struct *tty)
>   */
>  static int hvc_open(struct tty_struct *tty, struct file * filp)
>  {
> -	struct hvc_struct *hp = tty->driver_data;
> +	struct hvc_struct *hp;
>  	unsigned long flags;
>  	int rc = 0;
>  
> +	mutex_lock(&hvc_open_mutex);
> +
> +	hp = tty->driver_data;
> +	if (!hp) {
> +		rc = -EIO;
> +		goto out;
> +	}
> +
>  	spin_lock_irqsave(&hp->port.lock, flags);
>  	/* Check and then increment for fast path open. */
>  	if (hp->port.count++ > 0) {
>  		spin_unlock_irqrestore(&hp->port.lock, flags);
>  		hvc_kick();
> -		return 0;
> +		goto out;
>  	} /* else count == 0 */
>  	spin_unlock_irqrestore(&hp->port.lock, flags);

Wait, why isn't this driver just calling tty_port_open() instead of
trying to open-code all of this?

Keeping a single mutext for open will not protect it from close, it will
just slow things down a bit.  There should already be a tty lock held by
the tty core for open() to keep it from racing things, right?

Try just removing all of this logic and replacing it with a call to
tty_port_open() and see if that fixes this issue.

As "proof" of this, I don't see other serial drivers needing a single
mutex for their open calls, do you?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v7 2/5] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details
From: kajoljain @ 2020-05-06  9:53 UTC (permalink / raw)
  To: Michael Ellerman, acme, linuxppc-dev, sukadev
  Cc: mark.rutland, ravi.bangoria, maddy, tglx, jmario, mpetlan, peterz,
	gregkh, linux-kernel, alexander.shishkin, linux-perf-users, ak,
	yao.jin, anju, mamatha4, jolsa, namhyung, mingo, kan.liang
In-Reply-To: <87ftcmfryt.fsf@mpe.ellerman.id.au>



On 4/29/20 5:01 PM, Michael Ellerman wrote:
> Hi Kajol,
> 
> Some comments inline ...
> 
> Kajol Jain <kjain@linux.ibm.com> writes:
>> For hv_24x7 socket/chip level events, specific chip-id to which
>> the data requested should be added as part of pmu events.
>> But number of chips/socket in the system details are not exposed.
>>
>> Patch implements read_sys_info_pseries() to get system
>> parameter values like number of sockets and chips per socket.
>> Rtas_call with token "PROCESSOR_MODULE_INFO"
>> is used to get these values.
>>
>> Sub-sequent patch exports these values via sysfs.
>>
>> Patch also make these parameters default to 1.
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>>  arch/powerpc/perf/hv-24x7.c              | 72 ++++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/pseries.h |  3 +
>>  2 files changed, 75 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
>> index 48e8f4b17b91..9ae00f29bd21 100644
>> --- a/arch/powerpc/perf/hv-24x7.c
>> +++ b/arch/powerpc/perf/hv-24x7.c
>> @@ -20,6 +20,11 @@
>>  #include <asm/io.h>
>>  #include <linux/byteorder/generic.h>
>>  
>> +#ifdef CONFIG_PPC_RTAS
> 
> This driver can only be build on pseries, and pseries always selects
> RTAS. So the ifdef is unncessary.

Hi Michael,
	Thanks for review, I will remove this check.

> 
>> +#include <asm/rtas.h>
>> +#include <../../platforms/pseries/pseries.h>
>> +#endif
> 
> That's not really what the platform header is intended for.
> 
> You should put the extern in arch/powerpc/include/asm somewhere.
> 
> Maybe rtas.h
> 
>> @@ -57,6 +62,69 @@ static bool is_physical_domain(unsigned domain)
>>  	}
>>  }
>>  
>> +#ifdef CONFIG_PPC_RTAS
> 
> Not needed.
> 
>> +#define PROCESSOR_MODULE_INFO   43
> 
> Please document where these come from, presumably LoPAPR somewhere?
> 

Sure, will add the details.

>> +#define PROCESSOR_MAX_LENGTH	(8 * 1024)
>> +
>> +static int strbe16toh(const char *buf, int offset)
>> +{
>> +	return (buf[offset] << 8) + buf[offset + 1];
>> +}
> 
> I'm confused by this. "str" implies string, a string is an array of
> bytes and has no endian. But then be16 implies it's an array of __be16,
> in which case buf should be a __be16 *.
> 

Yes right, actually I was following implementation in util-linux. But what you
suggested make more sense, will update accordingly.

>> +
>> +static u32		physsockets;	/* Physical sockets */
>> +static u32		physchips;	/* Physical chips */
> 
> No tabs there please.

Sure will update.

> 
>> +
>> +/*
>> + * Function read_sys_info_pseries() make a rtas_call which require
>> + * data buffer of size 8K. As standard 'rtas_data_buf' is of size
>> + * 4K, we are adding new local buffer 'rtas_local_data_buf'.
>> + */
>> +char rtas_local_data_buf[PROCESSOR_MAX_LENGTH] __cacheline_aligned;
> 
> static?
> 
>> +/*
>> + * read_sys_info_pseries()
>> + * Retrieve the number of sockets and chips per socket details
>> + * through the get-system-parameter rtas call.
>> + */
>> +void read_sys_info_pseries(void)
>> +{
>> +	int call_status, len, ntypes;
>> +
>> +	/*
>> +	 * Making system parameter: chips and sockets default to 1.
>> +	 */
>> +	physsockets = 1;
>> +	physchips = 1;
>> +	memset(rtas_local_data_buf, 0, PROCESSOR_MAX_LENGTH);
>> +	spin_lock(&rtas_data_buf_lock);
> 
> You're not using the rtas_data_buf, so why are you taking the
> rtas_data_buf_lock?

Sure, I will add new lock specific for rtas_local_data_buf

> 
>> +	call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
>> +				NULL,
>> +				PROCESSOR_MODULE_INFO,
>> +				__pa(rtas_local_data_buf),
>> +				PROCESSOR_MAX_LENGTH);
>> +
>> +	spin_unlock(&rtas_data_buf_lock);
>> +
>> +	if (call_status != 0) {
>> +		pr_info("%s %s Error calling get-system-parameter (0x%x)\n",
>> +			__FILE__, __func__, call_status);
> 
> pr_err(), don't use __FILE__, this file already uses pr_fmt(). Not sure
> __func__ is really necessary either
> 
> 		return;
> 
> Then you can deindent the next block.
> 
>> +	} else {
>> +		rtas_local_data_buf[PROCESSOR_MAX_LENGTH - 1] = '\0';
>> +		len = strbe16toh(rtas_local_data_buf, 0);
> 
> Why isn't the buffer a __be16 array, and then you just use be16_to_cpu() ?
> 
>> +		if (len < 6)
>> +			return;
>> +
>> +		ntypes = strbe16toh(rtas_local_data_buf, 2);
>> +
>> +		if (!ntypes)
>> +			return;
> 
> What is ntype
ntype specify processor module type

> 
>> +		physsockets = strbe16toh(rtas_local_data_buf, 4);
>> +		physchips = strbe16toh(rtas_local_data_buf, 6);
>> +	}
>> +}
>> +#endif /* CONFIG_PPC_RTAS */
>> +
>>  /* Domains for which more than one result element are returned for each event. */
>>  static bool domain_needs_aggregation(unsigned int domain)
>>  {
>> @@ -1605,6 +1673,10 @@ static int hv_24x7_init(void)
>>  	if (r)
>>  		return r;
>>  
>> +#ifdef CONFIG_PPC_RTAS
>> +	read_sys_info_pseries();
>> +#endif
>

Will remove this config checks.

Thanks,
Kajol Jain

 
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 13fa370a87e4..1727559ce304 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -19,6 +19,9 @@ extern void request_event_sources_irqs(struct device_node *np,
>>  struct pt_regs;
>>  
>>  extern int pSeries_system_reset_exception(struct pt_regs *regs);
>> +#ifdef CONFIG_PPC_RTAS
>> +extern void read_sys_info_pseries(void);
>> +#endif
>>  extern int pSeries_machine_check_exception(struct pt_regs *regs);
>>  extern long pseries_machine_check_realmode(struct pt_regs *regs);
> > 
> cheers
> 

^ permalink raw reply


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