* [PATCH v3 05/14] powerpc sstep: Add support for prefixed fixed-point arithmetic
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
This adds emulation support for the following prefixed Fixed-Point
Arithmetic instructions:
* Prefixed Add Immediate (paddi)
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 8e4ec953e279..f2010a3e1e06 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1331,6 +1331,26 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
switch (opcode) {
#ifdef __powerpc64__
+ case 1:
+ prefix_r = instr & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+ rd = (suffix >> 21) & 0x1f;
+ op->reg = rd;
+ op->val = regs->gpr[rd];
+ suffixopcode = suffix >> 26;
+ prefixtype = (instr >> 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(instr, suffix, regs);
+ goto compute_done;
+ }
+ }
+ break;
case 2: /* tdi */
if (rd & trap_compare(regs->gpr[ra], (short) instr))
goto trap;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 06/14] powerpc: Support prefixed instructions in alignment handler
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
Alignment interrupts can be caused by prefixed instructions accessing
memory. In the alignment handler the instruction that caused the
exception is loaded and attempted emulate. If the instruction is a
prefixed instruction load the prefix and suffix to emulate. After
emulating increment the NIP by 8.
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().
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.
---
arch/powerpc/include/asm/uaccess.h | 25 +++++++++++++++++++++++++
arch/powerpc/kernel/align.c | 8 +++++---
arch/powerpc/kernel/traps.c | 21 ++++++++++++++++++++-
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2f500debae21..8903a96cbb4b 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -105,6 +105,31 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
#define __put_user_inatomic(x, ptr) \
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
+/*
+ * When reading an instruction iff it is a prefix, the suffix needs to be also
+ * loaded.
+ */
+#define __get_user_instr(x, y, ptr) \
+({ \
+ long __gui_ret = 0; \
+ y = 0; \
+ __gui_ret = __get_user(x, ptr); \
+ if (!__gui_ret && IS_PREFIX(x)) \
+ __gui_ret = __get_user(y, ptr + 1); \
+ __gui_ret; \
+})
+
+#define __get_user_instr_inatomic(x, y, ptr) \
+({ \
+ long __gui_ret = 0; \
+ y = 0; \
+ __gui_ret = __get_user_inatomic(x, ptr); \
+ if (!__gui_ret && IS_PREFIX(x)) \
+ __gui_ret = __get_user_inatomic(y, ptr + 1); \
+ __gui_ret; \
+})
+
+
extern long __put_user_bad(void);
/*
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index ba3bf5c3ab62..4984cf681215 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs, unsigned int reg,
int fix_alignment(struct pt_regs *regs)
{
- unsigned int instr;
+ unsigned int instr, suffix;
struct instruction_op op;
int r, type;
@@ -303,13 +303,15 @@ int fix_alignment(struct pt_regs *regs)
*/
CHECK_FULL_REGS(regs);
- if (unlikely(__get_user(instr, (unsigned int __user *)regs->nip)))
+ if (unlikely(__get_user_instr(instr, suffix,
+ (unsigned int __user *)regs->nip)))
return -EFAULT;
if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
/* We don't handle PPC little-endian any more... */
if (cpu_has_feature(CPU_FTR_PPC_LE))
return -EIO;
instr = swab32(instr);
+ suffix = swab32(suffix);
}
#ifdef CONFIG_SPE
@@ -334,7 +336,7 @@ int fix_alignment(struct pt_regs *regs)
if ((instr & 0xfc0006fe) == (PPC_INST_COPY & 0xfc0006fe))
return -EIO;
- r = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX);
+ r = analyse_instr(&op, regs, instr, suffix);
if (r < 0)
return -EINVAL;
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 82a3438300fd..d80b82fc1ae3 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -583,6 +583,10 @@ 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
+
+#define inst_length(reason) 4
/* single-step stuff */
#define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)
@@ -597,6 +601,10 @@ 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 inst_length(reason) (((reason) & REASON_PREFIXED) ? 8 : 4)
#define single_stepping(regs) ((regs)->msr & MSR_SE)
#define clear_single_step(regs) ((regs)->msr &= ~MSR_SE)
@@ -1593,11 +1601,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 +1623,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 +1637,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 v3 07/14] powerpc/traps: Check for prefixed instructions in facility_unavailable_exception()
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
If prefixed instructions are made unavailable by the [H]FSCR, attempting
to use them will cause a facility unavailable exception. Add "PREFIX" to
the facility_strings[].
Currently there are no prefixed instructions that are actually emulated
by emulate_instruction() within facility_unavailable_exception().
However, when caused by a prefixed instructions the SRR1 PREFIXED bit is
set. Prepare for dealing with emulated prefixed instructions by checking
for this bit.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
arch/powerpc/kernel/traps.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index d80b82fc1ae3..cd8b3043c268 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1739,6 +1739,7 @@ void facility_unavailable_exception(struct pt_regs *regs)
[FSCR_TAR_LG] = "TAR",
[FSCR_MSGP_LG] = "MSGP",
[FSCR_SCV_LG] = "SCV",
+ [FSCR_PREFIX_LG] = "PREFIX",
};
char *facility = "unknown";
u64 value;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 08/14] powerpc/xmon: Remove store_inst() for patch_instruction()
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
For modifying instructions in xmon, patch_instruction() can serve the
same role that store_inst() is performing with the advantage of not
being specific to xmon. In some places patch_instruction() is already
being using followed by store_inst(). In these cases just remove the
store_inst(). Otherwise replace store_inst() with patch_instruction().
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
arch/powerpc/xmon/xmon.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 897e512c6379..a673cf55641c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -325,11 +325,6 @@ static inline void sync(void)
asm volatile("sync; isync");
}
-static inline void store_inst(void *p)
-{
- asm volatile ("dcbst 0,%0; sync; icbi 0,%0; isync" : : "r" (p));
-}
-
static inline void cflush(void *p)
{
asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p));
@@ -882,8 +877,7 @@ static struct bpt *new_breakpoint(unsigned long a)
for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
bp->address = a;
- bp->instr[1] = bpinstr;
- store_inst(&bp->instr[1]);
+ patch_instruction(&bp->instr[1], bpinstr);
return bp;
}
}
@@ -913,7 +907,7 @@ static void insert_bpts(void)
bp->enabled = 0;
continue;
}
- store_inst(&bp->instr[0]);
+ patch_instruction(&bp->instr[0], bp->instr[0]);
if (bp->enabled & BP_CIABR)
continue;
if (patch_instruction((unsigned int *)bp->address,
@@ -923,7 +917,6 @@ static void insert_bpts(void)
bp->enabled &= ~BP_TRAP;
continue;
}
- store_inst((void *)bp->address);
}
}
@@ -958,8 +951,6 @@ static void remove_bpts(void)
(unsigned int *)bp->address, bp->instr[0]) != 0)
printf("Couldn't remove breakpoint at %lx\n",
bp->address);
- else
- store_inst((void *)bp->address);
}
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
A prefixed instruction is composed of a word prefix and a word suffix.
It does not make sense to be able to have a breakpoint on the suffix of
a prefixed instruction, so make this impossible.
When leaving xmon_core() we check to see if we are currently at a
breakpoint. If this is the case, the breakpoint needs to be proceeded
from. Initially emulate_step() is tried, but if this fails then we need
to execute the saved instruction out of line. The NIP is set to the
address of bpt::instr[] for the current breakpoint. bpt::instr[]
contains the instruction replaced by the breakpoint, followed by a trap
instruction. After bpt::instr[0] is executed and we hit the trap we
enter back into xmon_bpt(). We know that if we got here and the offset
indicates we are at bpt::instr[1] then we have just executed out of line
so we can put the NIP back to the instruction after the breakpoint
location and continue on.
Adding prefixed instructions complicates this as the bpt::instr[1] needs
to be used to hold the suffix. To deal with this make bpt::instr[] big
enough for three word instructions. bpt::instr[2] contains the trap,
and in the case of word instructions pad bpt::instr[1] with a noop.
No support for disassembling prefixed instructions.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
v3: - Just directly use PPC_INST_NOP
- Typo: plac -> place
- Rename read_inst() to mread_inst(). Do not have it call mread().
---
arch/powerpc/xmon/xmon.c | 90 ++++++++++++++++++++++++++++++++++------
1 file changed, 78 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a673cf55641c..a73a35aa4a75 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
/* Breakpoint stuff */
struct bpt {
unsigned long address;
- unsigned int instr[2];
+ /* Prefixed instructions can not cross 64-byte boundaries */
+ unsigned int instr[3] __aligned(64);
atomic_t ref_count;
int enabled;
unsigned long pad;
@@ -120,6 +121,7 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
static int cmds(struct pt_regs *);
static int mread(unsigned long, void *, int);
static int mwrite(unsigned long, void *, int);
+static int mread_instr(unsigned long, unsigned int *, unsigned int *);
static int handle_fault(struct pt_regs *);
static void byterev(unsigned char *, int);
static void memex(void);
@@ -701,7 +703,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
bp = at_breakpoint(regs->nip);
if (bp != NULL) {
int stepped = emulate_step(regs, bp->instr[0],
- PPC_NO_SUFFIX);
+ bp->instr[1]);
if (stepped == 0) {
regs->nip = (unsigned long) &bp->instr[0];
atomic_inc(&bp->ref_count);
@@ -756,8 +758,8 @@ static int xmon_bpt(struct pt_regs *regs)
/* Are we at the trap at bp->instr[1] for some bp? */
bp = in_breakpoint_table(regs->nip, &offset);
- if (bp != NULL && offset == 4) {
- regs->nip = bp->address + 4;
+ if (bp != NULL && (offset == 4 || offset == 8)) {
+ regs->nip = bp->address + offset;
atomic_dec(&bp->ref_count);
return 1;
}
@@ -858,8 +860,9 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
if (off >= sizeof(bpts))
return NULL;
off %= sizeof(struct bpt);
- if (off != offsetof(struct bpt, instr[0])
- && off != offsetof(struct bpt, instr[1]))
+ if (off != offsetof(struct bpt, instr[0]) &&
+ off != offsetof(struct bpt, instr[1]) &&
+ off != offsetof(struct bpt, instr[2]))
return NULL;
*offp = off - offsetof(struct bpt, instr[0]);
return (struct bpt *) (nip - off);
@@ -876,8 +879,16 @@ static struct bpt *new_breakpoint(unsigned long a)
for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
+ /*
+ * Prefixed instructions are two words, but regular
+ * instructions are only one. Use a nop to pad out the
+ * regular instructions so that we can place the trap
+ * at the same place. For prefixed instructions the nop
+ * will get overwritten during insert_bpts().
+ */
bp->address = a;
- patch_instruction(&bp->instr[1], bpinstr);
+ patch_instruction(&bp->instr[1], PPC_INST_NOP);
+ patch_instruction(&bp->instr[2], bpinstr);
return bp;
}
}
@@ -889,13 +900,15 @@ static struct bpt *new_breakpoint(unsigned long a)
static void insert_bpts(void)
{
int i;
- struct bpt *bp;
+ unsigned int prefix;
+ struct bpt *bp, *bp2;
bp = bpts;
for (i = 0; i < NBPTS; ++i, ++bp) {
if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
continue;
- if (mread(bp->address, &bp->instr[0], 4) != 4) {
+ if (!mread_instr(bp->address, &bp->instr[0],
+ &bp->instr[1])) {
printf("Couldn't read instruction at %lx, "
"disabling breakpoint there\n", bp->address);
bp->enabled = 0;
@@ -907,7 +920,34 @@ 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(bp->address - 4, &prefix, 4) == 4) &&
+ IS_PREFIX(prefix)) {
+ 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 && IS_PREFIX(bp2->instr[0])) {
+ 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[0], bp->instr[0]);
+ if (IS_PREFIX(bp->instr[0]))
+ patch_instruction(&bp->instr[1], bp->instr[1]);
if (bp->enabled & BP_CIABR)
continue;
if (patch_instruction((unsigned int *)bp->address,
@@ -1155,14 +1195,14 @@ static int do_step(struct pt_regs *regs)
*/
static int do_step(struct pt_regs *regs)
{
- unsigned int instr;
+ unsigned int instr, suffix;
int stepped;
force_enable_xmon();
/* check we are in 64-bit kernel mode, translation enabled */
if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
- if (mread(regs->nip, &instr, 4) == 4) {
- stepped = emulate_step(regs, instr, PPC_NO_SUFFIX);
+ if (mread_instr(regs->nip, &instr, &suffix)) {
+ stepped = emulate_step(regs, instr, suffix);
if (stepped < 0) {
printf("Couldn't single-step %s instruction\n",
(IS_RFID(instr)? "rfid": "mtmsrd"));
@@ -2121,6 +2161,32 @@ mwrite(unsigned long adrs, void *buf, int size)
return n;
}
+static int
+mread_instr(unsigned long adrs, unsigned int *instr,
+ unsigned int *suffix)
+{
+ int n;
+
+ n = 0;
+ if (setjmp(bus_error_jmp) == 0) {
+ catch_memory_errors = 1;
+ sync();
+ *instr = *(unsigned int *)adrs;
+ sync();
+ if (!IS_PREFIX(*instr)) {
+ n = 4;
+ } else {
+ *suffix = *(unsigned int *)(adrs + 4);
+ n = 8;
+ }
+ sync();
+ /* wait a little while to see if we get a machine check */
+ __delay(200);
+ }
+ catch_memory_errors = 0;
+ return n;
+}
+
static int fault_type;
static int fault_except;
static char *fault_chars[] = { "--", "**", "##" };
--
2.17.1
^ permalink raw reply related
* [PATCH v3 10/14] powerpc/xmon: Dump prefixed instructions
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
Currently when xmon is dumping instructions it reads a word at a time
and then prints that instruction (either as a hex number or by
disassembling it). For prefixed instructions it would be nice to show
its prefix and suffix as together. Use read_instr() so that if a prefix
is encountered its suffix is loaded too. Then print these in the form:
prefix:suffix
Xmon uses the disassembly routines from GNU binutils. These currently do
not support prefixed instructions so we will not disassemble the
prefixed instructions yet.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
v3: Simplify generic_inst_dump()
---
arch/powerpc/xmon/xmon.c | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a73a35aa4a75..bf304189e33a 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2900,6 +2900,21 @@ prdump(unsigned long adrs, long ndump)
}
}
+static bool instrs_are_equal(unsigned long insta, unsigned long suffixa,
+ unsigned long instb, unsigned long suffixb)
+{
+ if (insta != instb)
+ return false;
+
+ if (!IS_PREFIX(insta) && !IS_PREFIX(instb))
+ return true;
+
+ if (IS_PREFIX(insta) && IS_PREFIX(instb))
+ return suffixa == suffixb;
+
+ return false;
+}
+
typedef int (*instruction_dump_func)(unsigned long inst, unsigned long addr);
static int
@@ -2908,12 +2923,11 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
{
int nr, dotted;
unsigned long first_adr;
- unsigned int inst, last_inst = 0;
- unsigned char val[4];
+ unsigned int inst, suffix, last_inst = 0, last_suffix = 0;
dotted = 0;
- for (first_adr = adr; count > 0; --count, adr += 4) {
- nr = mread(adr, val, 4);
+ for (first_adr = adr; count > 0; --count, adr += nr) {
+ nr = mread_instr(adr, &inst, &suffix);
if (nr == 0) {
if (praddr) {
const char *x = fault_chars[fault_type];
@@ -2921,8 +2935,9 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
}
break;
}
- inst = GETWORD(val);
- if (adr > first_adr && inst == last_inst) {
+ if (adr > first_adr && instrs_are_equal(inst, suffix,
+ last_inst,
+ last_suffix)) {
if (!dotted) {
printf(" ...\n");
dotted = 1;
@@ -2931,10 +2946,17 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
}
dotted = 0;
last_inst = inst;
- if (praddr)
+ last_suffix = suffix;
+ if (praddr) {
printf(REG" %.8x", adr, inst);
+ if (IS_PREFIX(inst))
+ printf(":%.8x", suffix);
+ }
printf("\t");
- dump_func(inst, adr);
+ if (IS_PREFIX(inst))
+ printf("%.8x:%.8x", inst, suffix);
+ else
+ dump_func(inst, adr);
printf("\n");
}
return adr - first_adr;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
A prefixed instruction is composed of a word prefix followed by a word
suffix. It does not make sense to be able to have a kprobe on the suffix
of a prefixed instruction, so make this impossible.
Kprobes work by replacing an instruction with a trap and saving that
instruction to be single stepped out of place later. Currently there is
not enough space allocated to keep a prefixed instruction for single
stepping. Increase the amount of space allocated for holding the
instruction copy.
kprobe_post_handler() expects all instructions to be 4 bytes long which
means that it does not function correctly for prefixed instructions.
Add checks for prefixed instructions which will use a length of 8 bytes
instead.
For optprobes we normally patch in loading the instruction we put a
probe on into r4 before calling emulate_step(). We now make space and
patch in loading the suffix into r5 as well.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v3: - Base on top of https://patchwork.ozlabs.org/patch/1232619/
- Change printing format to %x:%x
---
arch/powerpc/include/asm/kprobes.h | 5 ++--
arch/powerpc/kernel/kprobes.c | 43 +++++++++++++++++++++-------
arch/powerpc/kernel/optprobes.c | 32 ++++++++++++---------
arch/powerpc/kernel/optprobes_head.S | 6 ++++
4 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
index 66b3f2983b22..0d44ce8a3163 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -38,12 +38,13 @@ extern kprobe_opcode_t optprobe_template_entry[];
extern kprobe_opcode_t optprobe_template_op_address[];
extern kprobe_opcode_t optprobe_template_call_handler[];
extern kprobe_opcode_t optprobe_template_insn[];
+extern kprobe_opcode_t optprobe_template_suffix[];
extern kprobe_opcode_t optprobe_template_call_emulate[];
extern kprobe_opcode_t optprobe_template_ret[];
extern kprobe_opcode_t optprobe_template_end[];
-/* Fixed instruction size for powerpc */
-#define MAX_INSN_SIZE 1
+/* Prefixed instructions are two words */
+#define MAX_INSN_SIZE 2
#define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
#define MAX_OPTINSN_SIZE (optprobe_template_end - optprobe_template_entry)
#define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 6b2e9e37f12b..9ccf1b9a1275 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -117,16 +117,28 @@ void *alloc_insn_page(void)
int arch_prepare_kprobe(struct kprobe *p)
{
int ret = 0;
+ struct kprobe *prev;
kprobe_opcode_t insn = *p->addr;
+ kprobe_opcode_t prefix = *(p->addr - 1);
+ preempt_disable();
if ((unsigned long)p->addr & 0x03) {
printk("Attempt to register kprobe at an unaligned address\n");
ret = -EINVAL;
} 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 (IS_PREFIX(prefix)) {
+ printk("Cannot register a kprobe on the second word of prefixed instruction\n");
+ ret = -EINVAL;
+ }
+ prev = get_kprobe(p->addr - 1);
+ if (prev && IS_PREFIX(*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
* not explicitly required on ppc32 (right now), but it doesn't hurt */
if (!ret) {
@@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
}
if (!ret) {
- patch_instruction(p->ainsn.insn, *p->addr);
+ patch_instruction(&p->ainsn.insn[0], p->addr[0]);
+ if (IS_PREFIX(insn))
+ patch_instruction(&p->ainsn.insn[1], p->addr[1]);
p->opcode = *p->addr;
}
p->ainsn.boostable = 0;
+ preempt_enable_no_resched();
return ret;
}
NOKPROBE_SYMBOL(arch_prepare_kprobe);
@@ -225,10 +240,11 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
{
int ret;
- unsigned int insn = *p->ainsn.insn;
+ unsigned int insn = p->ainsn.insn[0];
+ unsigned int suffix = p->ainsn.insn[1];
/* regs->nip is also adjusted if emulate_step returns 1 */
- ret = emulate_step(regs, insn, PPC_NO_SUFFIX);
+ ret = emulate_step(regs, insn, suffix);
if (ret > 0) {
/*
* Once this instruction has been boosted
@@ -242,7 +258,11 @@ static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
* So, we should never get here... but, its still
* good to catch them, just in case...
*/
- printk("Can't step on instruction %x\n", insn);
+ if (!IS_PREFIX(insn))
+ printk("Can't step on instruction %x\n", insn);
+ else
+ printk("Can't step on instruction %x:%x\n", insn,
+ suffix);
BUG();
} else {
/*
@@ -284,7 +304,7 @@ int kprobe_handler(struct pt_regs *regs)
if (kprobe_running()) {
p = get_kprobe(addr);
if (p) {
- kprobe_opcode_t insn = *p->ainsn.insn;
+ kprobe_opcode_t insn = p->ainsn.insn[0];
if (kcb->kprobe_status == KPROBE_HIT_SS &&
is_trap(insn)) {
/* Turn off 'trace' bits */
@@ -457,9 +477,10 @@ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs)
* the link register properly so that the subsequent 'blr' in
* kretprobe_trampoline jumps back to the right instruction.
*
- * For nip, we should set the address to the previous instruction since
- * we end up emulating it in kprobe_handler(), which increments the nip
- * again.
+ * To keep the nip at the correct address we need to counter the
+ * increment that happens when we emulate the kretprobe_trampoline noop
+ * in kprobe_handler(). We do this by decrementing the address by the
+ * length of the noop which is always 4 bytes.
*/
regs->nip = orig_ret_address - 4;
regs->link = orig_ret_address;
@@ -487,12 +508,14 @@ int kprobe_post_handler(struct pt_regs *regs)
{
struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ kprobe_opcode_t insn;
if (!cur || user_mode(regs))
return 0;
+ insn = *cur->ainsn.insn;
/* make sure we got here for instruction we have a kprobe on */
- if (((unsigned long)cur->ainsn.insn + 4) != regs->nip)
+ if ((unsigned long)cur->ainsn.insn + PPC_INST_LENGTH(insn) != regs->nip)
return 0;
if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
@@ -501,7 +524,7 @@ int kprobe_post_handler(struct pt_regs *regs)
}
/* Adjust nip to after the single-stepped instruction */
- regs->nip = (unsigned long)cur->addr + 4;
+ regs->nip = (unsigned long)cur->addr + PPC_INST_LENGTH(insn);
regs->msr |= kcb->kprobe_saved_msr;
/*Restore back the original saved kprobes variables and continue. */
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index f908d9422557..60cf8e8485ab 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -27,6 +27,8 @@
(optprobe_template_op_address - optprobe_template_entry)
#define TMPL_INSN_IDX \
(optprobe_template_insn - optprobe_template_entry)
+#define TMPL_SUFX_IDX \
+ (optprobe_template_suffix - optprobe_template_entry)
#define TMPL_END_IDX \
(optprobe_template_end - optprobe_template_entry)
@@ -100,8 +102,8 @@ static unsigned long can_optimize(struct kprobe *p)
* and that can be emulated.
*/
if (!is_conditional_branch(*p->ainsn.insn) &&
- analyse_instr(&op, ®s, *p->ainsn.insn,
- PPC_NO_SUFFIX) == 1) {
+ analyse_instr(&op, ®s, p->ainsn.insn[0],
+ p->ainsn.insn[1]) == 1) {
emulate_update_regs(®s, &op);
nip = regs.nip;
}
@@ -141,27 +143,27 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
}
/*
- * emulate_step() requires insn to be emulated as
- * second parameter. Load register 'r4' with the
- * instruction.
+ * emulate_step() requires insn to be emulated as second parameter, and the
+ * suffix as the third parameter. Load these into registers.
*/
-void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
+static void patch_imm32_load_insns(int reg, unsigned int val,
+ kprobe_opcode_t *addr)
{
- /* addis r4,0,(insn)@h */
- patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(4) |
+ /* addis reg,0,(insn)@h */
+ patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(reg) |
((val >> 16) & 0xffff));
addr++;
- /* ori r4,r4,(insn)@l */
- patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(4) |
- ___PPC_RS(4) | (val & 0xffff));
+ /* ori reg,reg,(insn)@l */
+ patch_instruction(addr, PPC_INST_ORI | ___PPC_RA(reg) |
+ ___PPC_RS(reg) | (val & 0xffff));
}
/*
* Generate instructions to load provided immediate 64-bit value
* to register 'r3' and patch these instructions at 'addr'.
*/
-void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
+static void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
{
/* lis r3,(op)@highest */
patch_instruction(addr, PPC_INST_ADDIS | ___PPC_RT(3) |
@@ -267,9 +269,11 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
patch_instruction(buff + TMPL_EMULATE_IDX, branch_emulate_step);
/*
- * 3. load instruction to be emulated into relevant register, and
+ * 3. load instruction and suffix to be emulated into the relevant
+ * registers, and
*/
- patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
+ patch_imm32_load_insns(4, p->ainsn.insn[0], buff + TMPL_INSN_IDX);
+ patch_imm32_load_insns(5, p->ainsn.insn[1], buff + TMPL_SUFX_IDX);
/*
* 4. branch back from trampoline
diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
index cf383520843f..395d1643f59d 100644
--- a/arch/powerpc/kernel/optprobes_head.S
+++ b/arch/powerpc/kernel/optprobes_head.S
@@ -95,6 +95,12 @@ optprobe_template_insn:
nop
nop
+ .global optprobe_template_suffix
+optprobe_template_suffix:
+ /* Pass suffix to be emulated in r5 */
+ nop
+ nop
+
.global optprobe_template_call_emulate
optprobe_template_call_emulate:
/* Branch to emulate_step() */
--
2.17.1
^ permalink raw reply related
* [PATCH v3 12/14] powerpc/uprobes: Add support for prefixed instructions
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
Uprobes can execute instructions out of line. Increase the size of the
buffer used for this so that this works for prefixed instructions. Take
into account the length of prefixed instructions when fixing up the nip.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Fix typo
- Use macro for instruction length
---
arch/powerpc/include/asm/uprobes.h | 16 ++++++++++++----
arch/powerpc/kernel/uprobes.c | 4 ++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h
index 2bbdf27d09b5..5516ab27db47 100644
--- a/arch/powerpc/include/asm/uprobes.h
+++ b/arch/powerpc/include/asm/uprobes.h
@@ -14,18 +14,26 @@
typedef ppc_opcode_t uprobe_opcode_t;
+/*
+ * Ensure we have enough space for prefixed instructions, which
+ * are double the size of a word instruction, i.e. 8 bytes.
+ */
#define MAX_UINSN_BYTES 4
-#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
+#define UPROBE_XOL_SLOT_BYTES (2 * MAX_UINSN_BYTES)
/* The following alias is needed for reference from arch-agnostic code */
#define UPROBE_SWBP_INSN BREAKPOINT_INSTRUCTION
#define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */
struct arch_uprobe {
+ /*
+ * Ensure there is enough space for prefixed instructions. Prefixed
+ * instructions must not cross 64-byte boundaries.
+ */
union {
- u32 insn;
- u32 ixol;
- };
+ uprobe_opcode_t insn[2];
+ uprobe_opcode_t ixol[2];
+ } __aligned(64);
};
struct arch_uprobe_task {
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 4ab40c4b576f..7e0334ad5cfe 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -111,7 +111,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
* support doesn't exist and have to fix-up the next instruction
* to be executed.
*/
- regs->nip = utask->vaddr + MAX_UINSN_BYTES;
+ regs->nip = utask->vaddr + PPC_INST_LENGTH(auprobe->insn[0]);
user_disable_single_step(current);
return 0;
@@ -173,7 +173,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
* emulate_step() returns 1 if the insn was successfully emulated.
* For all other cases, we need to single-step in hardware.
*/
- ret = emulate_step(regs, auprobe->insn, PPC_NO_SUFFIX);
+ ret = emulate_step(regs, auprobe->insn[0], auprobe->insn[1]);
if (ret > 0)
return true;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 13/14] powerpc/hw_breakpoints: Initial support for prefixed instructions
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
Currently when getting an instruction to emulate in
hw_breakpoint_handler() we do not load the suffix of a prefixed
instruction. Ensure we load the suffix if the instruction we need to
emulate is a prefixed instruction.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
v3: Add __user to type cast to remove sparse warning
---
arch/powerpc/kernel/hw_breakpoint.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 3a7ec6760dab..edf46356dfb2 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -243,15 +243,16 @@ dar_range_overlaps(unsigned long dar, int size, struct arch_hw_breakpoint *info)
static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
struct arch_hw_breakpoint *info)
{
- unsigned int instr = 0;
+ unsigned int instr = 0, suffix = 0;
int ret, type, size;
struct instruction_op op;
unsigned long addr = info->address;
- if (__get_user_inatomic(instr, (unsigned int *)regs->nip))
+ if (__get_user_instr_inatomic(instr, suffix,
+ (unsigned int __user *)regs->nip))
goto fail;
- ret = analyse_instr(&op, regs, instr, PPC_NO_SUFFIX);
+ ret = analyse_instr(&op, regs, instr, suffix);
type = GETTYPE(op.type);
size = GETSIZE(op.type);
@@ -275,7 +276,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
return false;
}
- if (!emulate_step(regs, instr, PPC_NO_SUFFIX))
+ if (!emulate_step(regs, instr, suffix))
goto fail;
return true;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 14/14] powerpc: Add prefix support to mce_find_instr_ea_and_pfn()
From: Jordan Niethe @ 2020-02-26 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, bala24, Jordan Niethe, dja
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>
mce_find_instr_ea_and_pfn analyses an instruction to determine the
effective address that caused the machine check. Update this to load and
pass the suffix to analyse_instr for prefixed instructions.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Rename sufx to suffix
---
arch/powerpc/kernel/mce_power.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 824eda536f5d..091bab4a5464 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -365,7 +365,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs *regs, uint64_t *addr,
* in real-mode is tricky and can lead to recursive
* faults
*/
- int instr;
+ int instr, suffix = 0;
unsigned long pfn, instr_addr;
struct instruction_op op;
struct pt_regs tmp = *regs;
@@ -374,7 +374,9 @@ static int mce_find_instr_ea_and_phys(struct pt_regs *regs, uint64_t *addr,
if (pfn != ULONG_MAX) {
instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
instr = *(unsigned int *)(instr_addr);
- if (!analyse_instr(&op, &tmp, instr, PPC_NO_SUFFIX)) {
+ if (IS_PREFIX(instr))
+ suffix = *(unsigned int *)(instr_addr + 4);
+ if (!analyse_instr(&op, &tmp, instr, suffix)) {
pfn = addr_to_pfn(regs, op.ea);
*addr = op.ea;
*phys_addr = (pfn << PAGE_SHIFT);
--
2.17.1
^ permalink raw reply related
* [PATCH] ocxl: Fix misleading comment
From: Andrew Donnellan @ 2020-02-26 4:39 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Frederic Barrat
In ocxl_context_free() we note that the AFU reference we're releasing was
taken in "ocxl_context_init", a function that doesn't actually exist.
Fix it to say ocxl_context_alloc() instead, which I expect was what was
intended.
Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
Cc: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
---
drivers/misc/ocxl/context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index de8a66b9d76b..c21f65a5c762 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -287,7 +287,7 @@ void ocxl_context_free(struct ocxl_context *ctx)
ocxl_afu_irq_free_all(ctx);
idr_destroy(&ctx->irq_idr);
- /* reference to the AFU taken in ocxl_context_init */
+ /* reference to the AFU taken in ocxl_context_alloc() */
ocxl_afu_put(ctx->afu);
kfree(ctx);
}
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] mm/vma: some more minor changes
From: Anshuman Khandual @ 2020-02-26 4:50 UTC (permalink / raw)
To: linux-mm
Cc: Anshuman Khandual, x86, linux-kernel, Ingo Molnar, Paul Mackerras,
Thomas Gleixner, linuxppc-dev, Andrew Morton
The motivation here is to consolidate VMA flags and helpers in generic
memory header and reduce code duplication when ever applicable. If there
are other possible similar instances which might be missing here, please
do let me me know. I will be happy to incorporate them.
This series is based on v5.6-rc3. This series has been build tested on
multiple platforms but boot tested only on arm64 and x86.
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
Anshuman Khandual (3):
mm/vma: Move VM_NO_KHUGEPAGED into generic header
mm/vma: Make vma_is_foreign() available for general use
mm/vma: Make is_vma_temporary_stack() available for general use
arch/powerpc/mm/book3s64/pkeys.c | 12 ------------
arch/x86/include/asm/mmu_context.h | 15 ---------------
include/linux/huge_mm.h | 2 --
include/linux/mm.h | 28 +++++++++++++++++++++++++++-
mm/khugepaged.c | 2 --
mm/rmap.c | 14 --------------
6 files changed, 27 insertions(+), 46 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 2/3] mm/vma: Make vma_is_foreign() available for general use
From: Anshuman Khandual @ 2020-02-26 4:50 UTC (permalink / raw)
To: linux-mm
Cc: Anshuman Khandual, x86, linux-kernel, Ingo Molnar, Paul Mackerras,
Thomas Gleixner, linuxppc-dev, Andrew Morton
In-Reply-To: <1582692658-3294-1-git-send-email-anshuman.khandual@arm.com>
Idea of a foreign VMA with respect to the present context is very generic.
But currently there are two identical definitions for this in powerpc and
x86 platforms. Lets consolidate those redundant definitions while making
vma_is_foreign() available for general use later. This should not cause
any functional change.
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/powerpc/mm/book3s64/pkeys.c | 12 ------------
arch/x86/include/asm/mmu_context.h | 15 ---------------
include/linux/mm.h | 11 +++++++++++
3 files changed, 11 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 59e0ebbd8036..07527f1ed108 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -381,18 +381,6 @@ bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
* So do not enforce things if the VMA is not from the current mm, or if we are
* in a kernel thread.
*/
-static inline bool vma_is_foreign(struct vm_area_struct *vma)
-{
- if (!current->mm)
- return true;
-
- /* if it is not our ->mm, it has to be foreign */
- if (current->mm != vma->vm_mm)
- return true;
-
- return false;
-}
-
bool arch_vma_access_permitted(struct vm_area_struct *vma, bool write,
bool execute, bool foreign)
{
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index b538d9ddee9c..4e55370e48e8 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -213,21 +213,6 @@ static inline void arch_unmap(struct mm_struct *mm, unsigned long start,
* So do not enforce things if the VMA is not from the current
* mm, or if we are in a kernel thread.
*/
-static inline bool vma_is_foreign(struct vm_area_struct *vma)
-{
- if (!current->mm)
- return true;
- /*
- * Should PKRU be enforced on the access to this VMA? If
- * the VMA is from another process, then PKRU has no
- * relevance and should not be enforced.
- */
- if (current->mm != vma->vm_mm)
- return true;
-
- return false;
-}
-
static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
bool write, bool execute, bool foreign)
{
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6f7e400e6ea3..2fd4b9bec4be 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -27,6 +27,7 @@
#include <linux/memremap.h>
#include <linux/overflow.h>
#include <linux/sizes.h>
+#include <linux/sched.h>
struct mempolicy;
struct anon_vma;
@@ -542,6 +543,16 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
return !vma->vm_ops;
}
+static inline bool vma_is_foreign(struct vm_area_struct *vma)
+{
+ if (!current->mm)
+ return true;
+
+ if (current->mm != vma->vm_mm)
+ return true;
+
+ return false;
+}
#ifdef CONFIG_SHMEM
/*
* The vma_is_shmem is not inline because it is used only by slow
--
2.20.1
^ permalink raw reply related
* [RFC PATCH] Use IS_ENABLED() instead of #ifdefs
From: Christophe Leroy @ 2020-02-26 5:04 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <d3647cce-ece3-d302-f541-b02b1f2b5e9e@huawei.com>
---
This works for me. Only had to leave the #ifdef around the map_mem_in_cams()
Also had to set linear_sz and ram for the alternative case, otherwise I get
arch/powerpc/mm/nohash/kaslr_booke.c: In function 'kaslr_early_init':
arch/powerpc/mm/nohash/kaslr_booke.c:355:33: error: 'linear_sz' may be used uninitialized in this function [-Werror=maybe-uninitialized]
regions.pa_end = memstart_addr + linear_sz;
~~~~~~~~~~~~~~^~~~~~~~~~~
arch/powerpc/mm/nohash/kaslr_booke.c:315:21: note: 'linear_sz' was declared here
unsigned long ram, linear_sz;
^~~~~~~~~
arch/powerpc/mm/nohash/kaslr_booke.c:187:8: error: 'ram' may be used uninitialized in this function [-Werror=maybe-uninitialized]
ret = parse_crashkernel(boot_command_line, size, &crash_size,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
&crash_base);
~~~~~~~~~~~~
arch/powerpc/mm/nohash/kaslr_booke.c:315:16: note: 'ram' was declared here
unsigned long ram, linear_sz;
---
arch/powerpc/mm/mmu_decl.h | 2 +-
arch/powerpc/mm/nohash/kaslr_booke.c | 97 +++++++++++++++-------------
2 files changed, 52 insertions(+), 47 deletions(-)
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index b869ea893301..3700e7c04e51 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -139,9 +139,9 @@ extern unsigned long calc_cam_sz(unsigned long ram, unsigned long virt,
extern void adjust_total_lowmem(void);
extern int switch_to_as1(void);
extern void restore_to_as0(int esel, int offset, void *dt_ptr, int bootcpu);
+#endif
void create_kaslr_tlb_entry(int entry, unsigned long virt, phys_addr_t phys);
extern int is_second_reloc;
-#endif
void reloc_kernel_entry(void *fdt, long addr);
extern void loadcam_entry(unsigned int index);
diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c
index c6f5c1db1394..bf69cece9b8c 100644
--- a/arch/powerpc/mm/nohash/kaslr_booke.c
+++ b/arch/powerpc/mm/nohash/kaslr_booke.c
@@ -267,35 +267,37 @@ static unsigned long __init kaslr_legal_offset(void *dt_ptr, unsigned long rando
unsigned long start;
unsigned long offset;
-#ifdef CONFIG_PPC32
- /*
- * Decide which 64M we want to start
- * Only use the low 8 bits of the random seed
- */
- unsigned long index = random & 0xFF;
- index %= regions.linear_sz / SZ_64M;
-
- /* Decide offset inside 64M */
- offset = random % (SZ_64M - regions.kernel_size);
- offset = round_down(offset, SZ_16K);
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ unsigned long index;
+
+ /*
+ * Decide which 64M we want to start
+ * Only use the low 8 bits of the random seed
+ */
+ index = random & 0xFF;
+ index %= regions.linear_sz / SZ_64M;
+
+ /* Decide offset inside 64M */
+ offset = random % (SZ_64M - regions.kernel_size);
+ offset = round_down(offset, SZ_16K);
+
+ while ((long)index >= 0) {
+ offset = memstart_addr + index * SZ_64M + offset;
+ start = memstart_addr + index * SZ_64M;
+ koffset = get_usable_address(dt_ptr, start, offset);
+ if (koffset)
+ break;
+ index--;
+ }
+ } else {
+ /* Decide kernel offset inside 1G */
+ offset = random % (SZ_1G - regions.kernel_size);
+ offset = round_down(offset, SZ_64K);
- while ((long)index >= 0) {
- offset = memstart_addr + index * SZ_64M + offset;
- start = memstart_addr + index * SZ_64M;
+ start = memstart_addr;
+ offset = memstart_addr + offset;
koffset = get_usable_address(dt_ptr, start, offset);
- if (koffset)
- break;
- index--;
}
-#else
- /* Decide kernel offset inside 1G */
- offset = random % (SZ_1G - regions.kernel_size);
- offset = round_down(offset, SZ_64K);
-
- start = memstart_addr;
- offset = memstart_addr + offset;
- koffset = get_usable_address(dt_ptr, start, offset);
-#endif
if (koffset != 0)
koffset -= memstart_addr;
@@ -342,6 +344,8 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
/* If the linear size is smaller than 64M, do not randmize */
if (linear_sz < SZ_64M)
return 0;
+#else
+ linear_sz = ram = size;
#endif
/* check for a reserved-memory node and record its cell sizes */
@@ -373,17 +377,19 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
{
unsigned long offset;
unsigned long kernel_sz;
+ unsigned int *__kaslr_offset;
+ unsigned int *__run_at_load;
-#ifdef CONFIG_PPC64
- unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
- unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
+ if (IS_ENABLED(CONFIG_PPC64)) {
+ __kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
+ __run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
- if (*__run_at_load == 1)
- return;
+ if (*__run_at_load == 1)
+ return;
- /* Setup flat device-tree pointer */
- initial_boot_params = dt_ptr;
-#endif
+ /* Setup flat device-tree pointer */
+ initial_boot_params = dt_ptr;
+ }
kernel_sz = (unsigned long)_end - (unsigned long)_stext;
@@ -394,20 +400,19 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
kernstart_virt_addr += offset;
kernstart_addr += offset;
-#ifdef CONFIG_PPC32
- is_second_reloc = 1;
-
- if (offset >= SZ_64M) {
- unsigned long tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
- phys_addr_t tlb_phys = round_down(kernstart_addr, SZ_64M);
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ is_second_reloc = 1;
+ if (offset >= SZ_64M) {
+ unsigned long tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
+ phys_addr_t tlb_phys = round_down(kernstart_addr, SZ_64M);
- /* Create kernel map to relocate in */
- create_kaslr_tlb_entry(1, tlb_virt, tlb_phys);
+ /* Create kernel map to relocate in */
+ create_kaslr_tlb_entry(1, tlb_virt, tlb_phys);
+ }
+ } else {
+ *__kaslr_offset = kernstart_virt_addr - KERNELBASE;
+ *__run_at_load = 1;
}
-#else
- *__kaslr_offset = kernstart_virt_addr - KERNELBASE;
- *__run_at_load = 1;
-#endif
/* Copy the kernel to it's new location and run */
memcpy((void *)kernstart_virt_addr, (void *)_stext, kernel_sz);
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v3 10/27] powerpc: Add driver for OpenCAPI Persistent Memory
From: Andrew Donnellan @ 2020-02-26 5:07 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-11-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This driver exposes LPC memory on OpenCAPI pmem cards
> as an NVDIMM, allowing the existing nvram infrastructure
> to be used.
>
> Namespace metadata is stored on the media itself, so
> scm_reserve_metadata() maps 1 section's worth of PMEM storage
> at the start to hold this. The rest of the PMEM range is registered
> with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size() provide
> callbacks to libnvdimm to access the metadata.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
I'm not particularly familiar with the nvdimm subsystem, so the scope of
my review is more on the ocxl + misc issues side.
A few minor checkpatch warnings that don't matter all that much:
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/11786//artifact/linux/checkpatch.log
A few other comments below.
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> new file mode 100644
> index 000000000000..3c4eeb5dcc0f
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -0,0 +1,473 @@
> +// SPDX-License-Id
> +// Copyright 2019 IBM Corp.
> +
> +/*
> + * A driver for OpenCAPI devices that implement the Storage Class
> + * Memory specification.
> + */
> +
> +#include <linux/module.h>
> +#include <misc/ocxl.h>
> +#include <linux/ndctl.h>
> +#include <linux/mm_types.h>
> +#include <linux/memory_hotplug.h>
> +#include "ocxl_internal.h"
> +
> +
> +static const struct pci_device_id ocxlpmem_pci_tbl[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, ocxlpmem_pci_tbl);
> +
> +#define NUM_MINORS 256 // Total to reserve
> +
> +static dev_t ocxlpmem_dev;
> +static struct class *ocxlpmem_class;
> +static struct mutex minors_idr_lock;
> +static struct idr minors_idr;
> +
> +/**
> + * ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA command from ndctl
> + * @ocxlpmem: the device metadata
> + * @command: the incoming data to write
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_write(struct ocxlpmem *ocxlpmem,
> + struct nd_cmd_set_config_hdr *command)
> +{
> + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> + return -EINVAL;
> +
> + memcpy_flushcache(ocxlpmem->metadata_addr + command->in_offset, command->in_buf,
> + command->in_length);
Out of scope for this patch - given that we use memcpy_mcsafe in the
config read, does it make sense to change memcpy_flushcache to be mcsafe
as well?
> +
> + return 0;
> +}
> +
> +/**
> + * ndctl_config_read() - Handle a ND_CMD_GET_CONFIG_DATA command from ndctl
> + * @ocxlpmem: the device metadata
> + * @command: the read request
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_read(struct ocxlpmem *ocxlpmem,
> + struct nd_cmd_get_config_data_hdr *command)
> +{
> + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> + return -EINVAL;
> +
> + memcpy_mcsafe(command->out_buf, ocxlpmem->metadata_addr + command->in_offset,
> + command->in_length);
> +
> + return 0;
> +}
> +
> +/**
> + * ndctl_config_size() - Handle a ND_CMD_GET_CONFIG_SIZE command from ndctl
> + * @command: the read request
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_size(struct nd_cmd_get_config_size *command)
> +{
> + command->status = 0;
> + command->config_size = LABEL_AREA_SIZE;
> + command->max_xfer = PAGE_SIZE;
> +
> + return 0;
> +}
> +
> +static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> + struct nvdimm *nvdimm,
> + unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> +{
> + struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct ocxlpmem, bus_desc);
> +
> + switch (cmd) {
> + case ND_CMD_GET_CONFIG_SIZE:
> + *cmd_rc = ndctl_config_size(buf);
> + return 0;
> +
> + case ND_CMD_GET_CONFIG_DATA:
> + *cmd_rc = ndctl_config_read(ocxlpmem, buf);
> + return 0;
> +
> + case ND_CMD_SET_CONFIG_DATA:
> + *cmd_rc = ndctl_config_write(ocxlpmem, buf);
> + return 0;
> +
> + default:
> + return -ENOTTY;
> + }
> +}
> +
> +/**
> + * reserve_metadata() - Reserve space for nvdimm metadata
> + * @ocxlpmem: the device metadata
> + * @lpc_mem: The resource representing the LPC memory of the OpenCAPI device
> + */
> +static int reserve_metadata(struct ocxlpmem *ocxlpmem,
> + struct resource *lpc_mem)
> +{
> + ocxlpmem->metadata_addr = devm_memremap(&ocxlpmem->dev, lpc_mem->start,
> + LABEL_AREA_SIZE, MEMREMAP_WB);
> + if (IS_ERR(ocxlpmem->metadata_addr))
> + return PTR_ERR(ocxlpmem->metadata_addr);
> +
> + return 0;
> +}
> +
> +/**
> + * register_lpc_mem() - Discover persistent memory on a device and register it with the NVDIMM subsystem
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success
> + */
> +static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> +{
> + struct nd_region_desc region_desc;
> + struct nd_mapping_desc nd_mapping_desc;
> + struct resource *lpc_mem;
> + const struct ocxl_afu_config *config;
> + const struct ocxl_fn_config *fn_config;
> + int rc;
> + unsigned long nvdimm_cmd_mask = 0;
> + unsigned long nvdimm_flags = 0;
> + int target_node;
> + char serial[16+1];
inb4 mpe tells you to Reverse Christmas Tree
> +
> + // Set up the reserved metadata area
> + rc = ocxl_afu_map_lpc_mem(ocxlpmem->ocxl_afu);
> + if (rc < 0)
> + return rc;
> +
> + lpc_mem = ocxl_afu_lpc_mem(ocxlpmem->ocxl_afu);
> + if (lpc_mem == NULL || lpc_mem->start == 0)
> + return -EINVAL;
> +
> + config = ocxl_afu_config(ocxlpmem->ocxl_afu);
> + fn_config = ocxl_function_config(ocxlpmem->ocxl_fn);
> +
> + rc = reserve_metadata(ocxlpmem, lpc_mem);
> + if (rc)
> + return rc;
> +
> + ocxlpmem->bus_desc.provider_name = "ocxl-pmem";
> + ocxlpmem->bus_desc.ndctl = ndctl;
> + ocxlpmem->bus_desc.module = THIS_MODULE;
> +
> + ocxlpmem->nvdimm_bus = nvdimm_bus_register(&ocxlpmem->dev,
> + &ocxlpmem->bus_desc);
> + if (!ocxlpmem->nvdimm_bus)
> + return -EINVAL;
> +
> + ocxlpmem->pmem_res.start = (u64)lpc_mem->start + LABEL_AREA_SIZE;
> + ocxlpmem->pmem_res.end = (u64)lpc_mem->start + config->lpc_mem_size - 1;
> + ocxlpmem->pmem_res.name = "OpenCAPI persistent memory";
> +
> + set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> +
> + set_bit(NDD_ALIASING, &nvdimm_flags);
> +
> + snprintf(serial, sizeof(serial), "%llx", fn_config->serial);
> + nd_mapping_desc.nvdimm = nvdimm_create(ocxlpmem->nvdimm_bus, ocxlpmem,
> + NULL, nvdimm_flags, nvdimm_cmd_mask,
> + 0, NULL);
> + if (!nd_mapping_desc.nvdimm)
> + return -ENOMEM;
> +
> + if (nvdimm_bus_check_dimm_count(ocxlpmem->nvdimm_bus, 1))
> + return -EINVAL;
> +
> + nd_mapping_desc.start = ocxlpmem->pmem_res.start;
> + nd_mapping_desc.size = resource_size(&ocxlpmem->pmem_res);
> + nd_mapping_desc.position = 0;
> +
> + ocxlpmem->nd_set.cookie1 = fn_config->serial + 1; // allow for empty serial
> + ocxlpmem->nd_set.cookie2 = fn_config->serial + 1;
> +
> + target_node = of_node_to_nid(ocxlpmem->pdev->dev.of_node);
> +
> + memset(®ion_desc, 0, sizeof(region_desc));
> + region_desc.res = &ocxlpmem->pmem_res;
> + region_desc.numa_node = NUMA_NO_NODE;
> + region_desc.target_node = target_node;
> + region_desc.num_mappings = 1;
> + region_desc.mapping = &nd_mapping_desc;
> + region_desc.nd_set = &ocxlpmem->nd_set;
> +
> + set_bit(ND_REGION_PAGEMAP, ®ion_desc.flags);
> + /*
> + * NB: libnvdimm copies the data from ndr_desc into it's own
> + * structures so passing a stack pointer is fine.
> + */
> + ocxlpmem->nd_region = nvdimm_pmem_region_create(ocxlpmem->nvdimm_bus,
> + ®ion_desc);
> + if (!ocxlpmem->nd_region)
> + return -EINVAL;
> +
> + dev_info(&ocxlpmem->dev,
> + "Onlining %lluMB of persistent memory\n",
> + nd_mapping_desc.size / SZ_1M);
> +
> + return 0;
> +}
> +
> +/**
> + * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem device
> + * @ocxlpmem: the device metadata
> + * Return: the allocated minor number
> + */
> +static int allocate_minor(struct ocxlpmem *ocxlpmem)
> +{
> + int minor;
> +
> + mutex_lock(&minors_idr_lock);
> + minor = idr_alloc(&minors_idr, ocxlpmem, 0, NUM_MINORS, GFP_KERNEL);
> + mutex_unlock(&minors_idr_lock);
> + return minor;
> +}
> +
> +static void free_minor(struct ocxlpmem *ocxlpmem)
The lack of a kerneldoc comment here is inconsistent :)
> +{
> + mutex_lock(&minors_idr_lock);
> + idr_remove(&minors_idr, MINOR(ocxlpmem->dev.devt));
> + mutex_unlock(&minors_idr_lock);
> +}
> +
> +/**
> + * free_ocxlpmem() - Free all members of an ocxlpmem struct
> + * @ocxlpmem: the device struct to clear
> + */
> +static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + if (ocxlpmem->nvdimm_bus)
> + nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> +
> + free_minor(ocxlpmem);
> +
> + if (ocxlpmem->metadata_addr)
> + devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> +
> + if (ocxlpmem->ocxl_context) {
> + rc = ocxl_context_detach(ocxlpmem->ocxl_context);
> + if (rc == -EBUSY)
> + dev_warn(&ocxlpmem->dev, "Timeout detaching ocxl context\n");
> + else
> + ocxl_context_free(ocxlpmem->ocxl_context);
> +
> + }
> +
> + if (ocxlpmem->ocxl_afu)
> + ocxl_afu_put(ocxlpmem->ocxl_afu);
> +
> + if (ocxlpmem->ocxl_fn)
> + ocxl_function_close(ocxlpmem->ocxl_fn);
> +
> + kfree(ocxlpmem);
> +}
> +
> +/**
> + * free_ocxlpmem_dev() - Free an OpenCAPI persistent memory device
> + * @dev: The device struct
> + */
> +static void free_ocxlpmem_dev(struct device *dev)
> +{
> + struct ocxlpmem *ocxlpmem = container_of(dev, struct ocxlpmem, dev);
> +
> + free_ocxlpmem(ocxlpmem);
> +}
> +
> +/**
> + * ocxlpmem_register() - Register an OpenCAPI pmem device with the kernel
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> + int minor = allocate_minor(ocxlpmem);
> +
> + if (minor < 0)
> + return minor;
> +
> + ocxlpmem->dev.release = free_ocxlpmem_dev;
> + rc = dev_set_name(&ocxlpmem->dev, "ocxlpmem%d", minor);
> + if (rc < 0)
> + return rc;
> +
> + ocxlpmem->dev.devt = MKDEV(MAJOR(ocxlpmem_dev), minor);
> + ocxlpmem->dev.class = ocxlpmem_class;
> + ocxlpmem->dev.parent = &ocxlpmem->pdev->dev;
> +
> + return device_register(&ocxlpmem->dev);
> +}
> +
> +/**
> + * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> + * @pdev: the PCI device information struct
> + */
> +static void ocxlpmem_remove(struct pci_dev *pdev)
> +{
> + if (PCI_FUNC(pdev->devfn) == 0) {
> + struct ocxlpmem_function0 *func0 = pci_get_drvdata(pdev);
> +
> + if (func0) {
> + ocxl_function_close(func0->ocxl_fn);
> + func0->ocxl_fn = NULL;
> + }
> + } else {
> + struct ocxlpmem *ocxlpmem = pci_get_drvdata(pdev);
> +
> + if (ocxlpmem)
> + device_unregister(&ocxlpmem->dev);
> + }
> +}
> +
> +/**
> + * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
> + * This is important as it enables templates higher than 0 across all other functions,
> + * which in turn enables higher bandwidth accesses
> + * @pdev: the PCI device information struct
> + * Return: 0 on success, negative on failure
> + */
> +static int probe_function0(struct pci_dev *pdev)
> +{
> + struct ocxlpmem_function0 *func0 = NULL;
> + struct ocxl_fn *fn;
> +
> + func0 = kzalloc(sizeof(*func0), GFP_KERNEL);
> + if (!func0)
> + return -ENOMEM;
> +
> + func0->pdev = pdev;
> + fn = ocxl_function_open(pdev);
> + if (IS_ERR(fn)) {
> + kfree(func0);
> + dev_err(&pdev->dev, "failed to open OCXL function\n");
> + return PTR_ERR(fn);
> + }
> + func0->ocxl_fn = fn;
> +
> + pci_set_drvdata(pdev, func0);
> +
> + return 0;
> +}
> +
> +/**
> + * probe() - Init an OpenCAPI persistent memory device
> + * @pdev: the PCI device information struct
> + * @ent: The entry from ocxlpmem_pci_tbl
> + * Return: 0 on success, negative on failure
> + */
> +static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> + struct ocxlpmem *ocxlpmem;
> + int rc;
> +
> + if (PCI_FUNC(pdev->devfn) == 0)
> + return probe_function0(pdev);
> + else if (PCI_FUNC(pdev->devfn) != 1)
> + return 0;
> +
> + ocxlpmem = kzalloc(sizeof(*ocxlpmem), GFP_KERNEL);
> + if (!ocxlpmem) {
> + dev_err(&pdev->dev, "Could not allocate OpenCAPI persistent memory metadata\n");
> + rc = -ENOMEM;
> + goto err;
> + }
> + ocxlpmem->pdev = pdev;
> +
> + pci_set_drvdata(pdev, ocxlpmem);
> +
> + ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> + if (IS_ERR(ocxlpmem->ocxl_fn)) {
> + kfree(ocxlpmem);
You can't free this yet...
> + pci_set_drvdata(pdev, NULL);
> + dev_err(&pdev->dev, "failed to open OCXL function\n");
> + rc = PTR_ERR(ocxlpmem->ocxl_fn);
> + goto err;
> + }
> +
> + ocxlpmem->ocxl_afu = ocxl_function_fetch_afu(ocxlpmem->ocxl_fn, 0);
> + if (ocxlpmem->ocxl_afu == NULL) {
> + dev_err(&pdev->dev, "Could not get OCXL AFU from function\n");
> + rc = -ENXIO;
> + goto err;
Meanwhile in this case, I think ocxlpmem and ocxlpmem->ocxl_fn get
leaked (it's before ocxlpmem_register(), so the cleanup handler hasn't
been registered).
Add some new error labels for these two paths?
> + }
> +
> + ocxl_afu_get(ocxlpmem->ocxl_afu);
> +
> + // Resources allocated below here are cleaned up in the release handler
> +
> + rc = ocxlpmem_register(ocxlpmem);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not register OpenCAPI persistent memory device with the kernel\n");
> + goto err;
> + }
> +
> + rc = ocxl_context_alloc(&ocxlpmem->ocxl_context, ocxlpmem->ocxl_afu, NULL);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not allocate OCXL context\n");
> + goto err;
> + }
> +
> + rc = ocxl_context_attach(ocxlpmem->ocxl_context, 0, NULL);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not attach ocxl context\n");
> + goto err;
> + }
> +
> + rc = register_lpc_mem(ocxlpmem);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not register OpenCAPI persistent memory with libnvdimm\n");
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + /*
> + * Further cleanup is done in the release handler via free_ocxlpmem()
> + * This allows us to keep the character device live to handle IOCTLs to
> + * investigate issues if the card has an error
> + */
> +
> + dev_err(&pdev->dev,
> + "Error detected, will not register OpenCAPI persistent memory\n");
> + return rc;
> +}
> +
> +static struct pci_driver pci_driver = {
> + .name = "ocxl-pmem",
> + .id_table = ocxlpmem_pci_tbl,
> + .probe = probe,
> + .remove = ocxlpmem_remove,
> + .shutdown = ocxlpmem_remove,
> +};
> +
> +static int __init ocxlpmem_init(void)
> +{
> + int rc = 0;
> +
> + rc = pci_register_driver(&pci_driver);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +static void ocxlpmem_exit(void)
> +{
> + pci_unregister_driver(&pci_driver);
> +}
> +
> +module_init(ocxlpmem_init);
> +module_exit(ocxlpmem_exit);
> +
> +MODULE_DESCRIPTION("OpenCAPI Persistent Memory");
> +MODULE_LICENSE("GPL");
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> new file mode 100644
> index 000000000000..0faf3740e9b8
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 IBM Corp.
> +
> +#include <linux/pci.h>
> +#include <misc/ocxl.h>
> +#include <linux/libnvdimm.h>
> +#include <linux/mm.h>
> +
> +#define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> +
> +struct ocxlpmem_function0 {
> + struct pci_dev *pdev;
> + struct ocxl_fn *ocxl_fn;
> +};
Hmm, given this struct serves no purpose other than associating an
ocxl_fn with a pci_dev, and special handling for function 0 is probably
going to be a common pattern in ocxl dependent drivers, perhaps we
should export a function from ocxl that converts ocxl_fn -> pci_dev, and
then you can just point the drvdata straight at the ocxl_fn?
> +
> +struct ocxlpmem {
> + struct device dev;
> + struct pci_dev *pdev;
> + struct ocxl_fn *ocxl_fn;
> + struct nd_interleave_set nd_set;
> + struct nvdimm_bus_descriptor bus_desc;
> + struct nvdimm_bus *nvdimm_bus;
> + struct ocxl_afu *ocxl_afu;
> + struct ocxl_context *ocxl_context;
> + void *metadata_addr;
> + struct resource pmem_res;
> + struct nd_region *nd_region;
> +};
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
From: Christophe Leroy @ 2020-02-26 5:08 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <dbe0b316-40a2-7da4-c26b-e59efa555400@huawei.com>
Le 26/02/2020 à 03:40, Jason Yan a écrit :
>
>
> 在 2020/2/20 21:48, Christophe Leroy 写道:
>>
>>
>> Le 06/02/2020 à 03:58, Jason Yan a écrit :
>>> /*
>>> * Decide which 64M we want to start
>>> * Only use the low 8 bits of the random seed
>>> */
>>> - index = random & 0xFF;
>>> + unsigned long index = random & 0xFF;
>>
>> That's not good in terms of readability, index declaration should
>> remain at the top of the function, should be possible if using
>> IS_ENABLED() instead
>
> I'm wondering how to declare a variable inside a code block such as if
> (IS_ENABLED(CONFIG_PPC32)) at the top of the function and use the
> variable in another if (IS_ENABLED(CONFIG_PPC32)). Is there any good idea?
You declare it outside the block as usual:
unsigned long some_var;
if (condition) {
some_var = something;
}
do_many_things();
do_other_things();
if (condition)
return some_var;
else
return 0;
Christophe
^ permalink raw reply
* Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
From: Christophe Leroy @ 2020-02-26 5:10 UTC (permalink / raw)
To: Jason Yan, mpe, linuxppc-dev, diana.craciun, benh, paulus,
npiggin, keescook, kernel-hardening, oss
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <d3647cce-ece3-d302-f541-b02b1f2b5e9e@huawei.com>
Le 26/02/2020 à 04:33, Jason Yan a écrit :
>
>
> 在 2020/2/26 10:40, Jason Yan 写道:
>>
>>
>> 在 2020/2/20 21:48, Christophe Leroy 写道:
>>>
>>>
>>> Le 06/02/2020 à 03:58, Jason Yan a écrit :
> Hi Christophe,
>
> When using a standard C if/else, all code compiled for PPC32 and PPC64,
> but this will bring some build error because not all variables both
> defined for PPC32 and PPC64.
>
> [yanaijie@138 linux]$ sh ppc64build.sh
> CALL scripts/atomic/check-atomics.sh
> CALL scripts/checksyscalls.sh
> CHK include/generated/compile.h
> CC arch/powerpc/mm/nohash/kaslr_booke.o
> arch/powerpc/mm/nohash/kaslr_booke.c: In function 'kaslr_choose_location':
> arch/powerpc/mm/nohash/kaslr_booke.c:341:30: error:
> 'CONFIG_LOWMEM_CAM_NUM' undeclared (first use in this function); did you
> mean 'CONFIG_FLATMEM_MANUAL'?
> ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true);
> ^~~~~~~~~~~~~~~~~~~~~
> CONFIG_FLATMEM_MANUAL
This one has to remain inside an #ifdef. That's the only one that has to
remain.
> arch/powerpc/mm/nohash/kaslr_booke.c:341:30: note: each undeclared
> identifier is reported only once for each function it appears in
> arch/powerpc/mm/nohash/kaslr_booke.c: In function 'kaslr_early_init':
> arch/powerpc/mm/nohash/kaslr_booke.c:404:3: error: 'is_second_reloc'
In mmu_decl.h, put the declaration outside the #ifdef CONFIG_PPC32
> undeclared (first use in this function); did you mean '__cond_lock'?
> is_second_reloc = 1;
> ^~~~~~~~~~~~~~~
> __cond_lock
> arch/powerpc/mm/nohash/kaslr_booke.c:411:4: error: implicit declaration
> of function 'create_kaslr_tlb_entry'; did you mean 'reloc_kernel_entry'?
Same, put the declaration outside of the #ifdef
> [-Werror=implicit-function-declaration]
> create_kaslr_tlb_entry(1, tlb_virt, tlb_phys);
> ^~~~~~~~~~~~~~~~~~~~~~
> reloc_kernel_entry
> cc1: all warnings being treated as errors
> make[3]: *** [scripts/Makefile.build:268:
> arch/powerpc/mm/nohash/kaslr_booke.o] Error 1
> make[2]: *** [scripts/Makefile.build:505: arch/powerpc/mm/nohash] Error 2
> make[1]: *** [scripts/Makefile.build:505: arch/powerpc/mm] Error 2
> make: *** [Makefile:1681: arch/powerpc] Error 2
See the patch I sent you. It builds ok for me.
Christophe
^ permalink raw reply
* Re: [PATCH v3 10/27] powerpc: Add driver for OpenCAPI Persistent Memory
From: Alastair D'Silva @ 2020-02-26 5:49 UTC (permalink / raw)
To: Andrew Donnellan
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <a2909b6a-5ceb-c732-c618-dd0a375475ce@linux.ibm.com>
On Wed, 2020-02-26 at 16:07 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This driver exposes LPC memory on OpenCAPI pmem cards
> > as an NVDIMM, allowing the existing nvram infrastructure
> > to be used.
> >
> > Namespace metadata is stored on the media itself, so
> > scm_reserve_metadata() maps 1 section's worth of PMEM storage
> > at the start to hold this. The rest of the PMEM range is registered
> > with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size()
> > provide
> > callbacks to libnvdimm to access the metadata.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>
> I'm not particularly familiar with the nvdimm subsystem, so the scope
> of
> my review is more on the ocxl + misc issues side.
>
> A few minor checkpatch warnings that don't matter all that much:
>
> https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/11786//artifact/linux/checkpatch.log
>
> A few other comments below.
>
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > new file mode 100644
> > index 000000000000..3c4eeb5dcc0f
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -0,0 +1,473 @@
> > +// SPDX-License-Id
> > +// Copyright 2019 IBM Corp.
> > +
> > +/*
> > + * A driver for OpenCAPI devices that implement the Storage Class
> > + * Memory specification.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <misc/ocxl.h>
> > +#include <linux/ndctl.h>
> > +#include <linux/mm_types.h>
> > +#include <linux/memory_hotplug.h>
> > +#include "ocxl_internal.h"
> > +
> > +
> > +static const struct pci_device_id ocxlpmem_pci_tbl[] = {
> > + { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(pci, ocxlpmem_pci_tbl);
> > +
> > +#define NUM_MINORS 256 // Total to reserve
> > +
> > +static dev_t ocxlpmem_dev;
> > +static struct class *ocxlpmem_class;
> > +static struct mutex minors_idr_lock;
> > +static struct idr minors_idr;
> > +
> > +/**
> > + * ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA command
> > from ndctl
> > + * @ocxlpmem: the device metadata
> > + * @command: the incoming data to write
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ndctl_config_write(struct ocxlpmem *ocxlpmem,
> > + struct nd_cmd_set_config_hdr *command)
> > +{
> > + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> > + return -EINVAL;
> > +
> > + memcpy_flushcache(ocxlpmem->metadata_addr + command->in_offset,
> > command->in_buf,
> > + command->in_length);
>
> Out of scope for this patch - given that we use memcpy_mcsafe in the
> config read, does it make sense to change memcpy_flushcache to be
> mcsafe
> as well?
>
Aneesh has confirmed that stores don't generate machine checks.
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * ndctl_config_read() - Handle a ND_CMD_GET_CONFIG_DATA command
> > from ndctl
> > + * @ocxlpmem: the device metadata
> > + * @command: the read request
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ndctl_config_read(struct ocxlpmem *ocxlpmem,
> > + struct nd_cmd_get_config_data_hdr
> > *command)
> > +{
> > + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> > + return -EINVAL;
> > +
> > + memcpy_mcsafe(command->out_buf, ocxlpmem->metadata_addr +
> > command->in_offset,
> > + command->in_length);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * ndctl_config_size() - Handle a ND_CMD_GET_CONFIG_SIZE command
> > from ndctl
> > + * @command: the read request
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ndctl_config_size(struct nd_cmd_get_config_size
> > *command)
> > +{
> > + command->status = 0;
> > + command->config_size = LABEL_AREA_SIZE;
> > + command->max_xfer = PAGE_SIZE;
> > +
> > + return 0;
> > +}
> > +
> > +static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> > + struct nvdimm *nvdimm,
> > + unsigned int cmd, void *buf, unsigned int buf_len, int
> > *cmd_rc)
> > +{
> > + struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct
> > ocxlpmem, bus_desc);
> > +
> > + switch (cmd) {
> > + case ND_CMD_GET_CONFIG_SIZE:
> > + *cmd_rc = ndctl_config_size(buf);
> > + return 0;
> > +
> > + case ND_CMD_GET_CONFIG_DATA:
> > + *cmd_rc = ndctl_config_read(ocxlpmem, buf);
> > + return 0;
> > +
> > + case ND_CMD_SET_CONFIG_DATA:
> > + *cmd_rc = ndctl_config_write(ocxlpmem, buf);
> > + return 0;
> > +
> > + default:
> > + return -ENOTTY;
> > + }
> > +}
> > +
> > +/**
> > + * reserve_metadata() - Reserve space for nvdimm metadata
> > + * @ocxlpmem: the device metadata
> > + * @lpc_mem: The resource representing the LPC memory of the
> > OpenCAPI device
> > + */
> > +static int reserve_metadata(struct ocxlpmem *ocxlpmem,
> > + struct resource *lpc_mem)
> > +{
> > + ocxlpmem->metadata_addr = devm_memremap(&ocxlpmem->dev,
> > lpc_mem->start,
> > + LABEL_AREA_SIZE,
> > MEMREMAP_WB);
> > + if (IS_ERR(ocxlpmem->metadata_addr))
> > + return PTR_ERR(ocxlpmem->metadata_addr);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * register_lpc_mem() - Discover persistent memory on a device and
> > register it with the NVDIMM subsystem
> > + * @ocxlpmem: the device metadata
> > + * Return: 0 on success
> > + */
> > +static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> > +{
> > + struct nd_region_desc region_desc;
> > + struct nd_mapping_desc nd_mapping_desc;
> > + struct resource *lpc_mem;
> > + const struct ocxl_afu_config *config;
> > + const struct ocxl_fn_config *fn_config;
> > + int rc;
> > + unsigned long nvdimm_cmd_mask = 0;
> > + unsigned long nvdimm_flags = 0;
> > + int target_node;
> > + char serial[16+1];
>
> inb4 mpe tells you to Reverse Christmas Tree
>
-1, petty :P
> > +
> > + // Set up the reserved metadata area
> > + rc = ocxl_afu_map_lpc_mem(ocxlpmem->ocxl_afu);
> > + if (rc < 0)
> > + return rc;
> > +
> > + lpc_mem = ocxl_afu_lpc_mem(ocxlpmem->ocxl_afu);
> > + if (lpc_mem == NULL || lpc_mem->start == 0)
> > + return -EINVAL;
> > +
> > + config = ocxl_afu_config(ocxlpmem->ocxl_afu);
> > + fn_config = ocxl_function_config(ocxlpmem->ocxl_fn);
> > +
> > + rc = reserve_metadata(ocxlpmem, lpc_mem);
> > + if (rc)
> > + return rc;
> > +
> > + ocxlpmem->bus_desc.provider_name = "ocxl-pmem";
> > + ocxlpmem->bus_desc.ndctl = ndctl;
> > + ocxlpmem->bus_desc.module = THIS_MODULE;
> > +
> > + ocxlpmem->nvdimm_bus = nvdimm_bus_register(&ocxlpmem->dev,
> > + &ocxlpmem-
> > >bus_desc);
> > + if (!ocxlpmem->nvdimm_bus)
> > + return -EINVAL;
> > +
> > + ocxlpmem->pmem_res.start = (u64)lpc_mem->start +
> > LABEL_AREA_SIZE;
> > + ocxlpmem->pmem_res.end = (u64)lpc_mem->start + config-
> > >lpc_mem_size - 1;
> > + ocxlpmem->pmem_res.name = "OpenCAPI persistent memory";
> > +
> > + set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> > + set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> > + set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> > +
> > + set_bit(NDD_ALIASING, &nvdimm_flags);
> > +
> > + snprintf(serial, sizeof(serial), "%llx", fn_config->serial);
> > + nd_mapping_desc.nvdimm = nvdimm_create(ocxlpmem->nvdimm_bus,
> > ocxlpmem,
> > + NULL, nvdimm_flags, nvdimm_cmd_mask,
> > + 0, NULL);
> > + if (!nd_mapping_desc.nvdimm)
> > + return -ENOMEM;
> > +
> > + if (nvdimm_bus_check_dimm_count(ocxlpmem->nvdimm_bus, 1))
> > + return -EINVAL;
> > +
> > + nd_mapping_desc.start = ocxlpmem->pmem_res.start;
> > + nd_mapping_desc.size = resource_size(&ocxlpmem->pmem_res);
> > + nd_mapping_desc.position = 0;
> > +
> > + ocxlpmem->nd_set.cookie1 = fn_config->serial + 1; // allow for
> > empty serial
> > + ocxlpmem->nd_set.cookie2 = fn_config->serial + 1;
> > +
> > + target_node = of_node_to_nid(ocxlpmem->pdev->dev.of_node);
> > +
> > + memset(®ion_desc, 0, sizeof(region_desc));
> > + region_desc.res = &ocxlpmem->pmem_res;
> > + region_desc.numa_node = NUMA_NO_NODE;
> > + region_desc.target_node = target_node;
> > + region_desc.num_mappings = 1;
> > + region_desc.mapping = &nd_mapping_desc;
> > + region_desc.nd_set = &ocxlpmem->nd_set;
> > +
> > + set_bit(ND_REGION_PAGEMAP, ®ion_desc.flags);
> > + /*
> > + * NB: libnvdimm copies the data from ndr_desc into it's own
> > + * structures so passing a stack pointer is fine.
> > + */
> > + ocxlpmem->nd_region = nvdimm_pmem_region_create(ocxlpmem-
> > >nvdimm_bus,
> > + ®ion_desc);
> > + if (!ocxlpmem->nd_region)
> > + return -EINVAL;
> > +
> > + dev_info(&ocxlpmem->dev,
> > + "Onlining %lluMB of persistent memory\n",
> > + nd_mapping_desc.size / SZ_1M);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * allocate_minor() - Allocate a minor number to use for an
> > OpenCAPI pmem device
> > + * @ocxlpmem: the device metadata
> > + * Return: the allocated minor number
> > + */
> > +static int allocate_minor(struct ocxlpmem *ocxlpmem)
> > +{
> > + int minor;
> > +
> > + mutex_lock(&minors_idr_lock);
> > + minor = idr_alloc(&minors_idr, ocxlpmem, 0, NUM_MINORS,
> > GFP_KERNEL);
> > + mutex_unlock(&minors_idr_lock);
> > + return minor;
> > +}
> > +
> > +static void free_minor(struct ocxlpmem *ocxlpmem)
>
> The lack of a kerneldoc comment here is inconsistent :)
>
> > +{
> > + mutex_lock(&minors_idr_lock);
> > + idr_remove(&minors_idr, MINOR(ocxlpmem->dev.devt));
> > + mutex_unlock(&minors_idr_lock);
> > +}
> > +
> > +/**
> > + * free_ocxlpmem() - Free all members of an ocxlpmem struct
> > + * @ocxlpmem: the device struct to clear
> > + */
> > +static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
> > +{
> > + int rc;
> > +
> > + if (ocxlpmem->nvdimm_bus)
> > + nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> > +
> > + free_minor(ocxlpmem);
> > +
> > + if (ocxlpmem->metadata_addr)
> > + devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> > +
> > + if (ocxlpmem->ocxl_context) {
> > + rc = ocxl_context_detach(ocxlpmem->ocxl_context);
> > + if (rc == -EBUSY)
> > + dev_warn(&ocxlpmem->dev, "Timeout detaching
> > ocxl context\n");
> > + else
> > + ocxl_context_free(ocxlpmem->ocxl_context);
> > +
> > + }
> > +
> > + if (ocxlpmem->ocxl_afu)
> > + ocxl_afu_put(ocxlpmem->ocxl_afu);
> > +
> > + if (ocxlpmem->ocxl_fn)
> > + ocxl_function_close(ocxlpmem->ocxl_fn);
> > +
> > + kfree(ocxlpmem);
> > +}
> > +
> > +/**
> > + * free_ocxlpmem_dev() - Free an OpenCAPI persistent memory device
> > + * @dev: The device struct
> > + */
> > +static void free_ocxlpmem_dev(struct device *dev)
> > +{
> > + struct ocxlpmem *ocxlpmem = container_of(dev, struct ocxlpmem,
> > dev);
> > +
> > + free_ocxlpmem(ocxlpmem);
> > +}
> > +
> > +/**
> > + * ocxlpmem_register() - Register an OpenCAPI pmem device with the
> > kernel
> > + * @ocxlpmem: the device metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
> > +{
> > + int rc;
> > + int minor = allocate_minor(ocxlpmem);
> > +
> > + if (minor < 0)
> > + return minor;
> > +
> > + ocxlpmem->dev.release = free_ocxlpmem_dev;
> > + rc = dev_set_name(&ocxlpmem->dev, "ocxlpmem%d", minor);
> > + if (rc < 0)
> > + return rc;
> > +
> > + ocxlpmem->dev.devt = MKDEV(MAJOR(ocxlpmem_dev), minor);
> > + ocxlpmem->dev.class = ocxlpmem_class;
> > + ocxlpmem->dev.parent = &ocxlpmem->pdev->dev;
> > +
> > + return device_register(&ocxlpmem->dev);
> > +}
> > +
> > +/**
> > + * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> > + * @pdev: the PCI device information struct
> > + */
> > +static void ocxlpmem_remove(struct pci_dev *pdev)
> > +{
> > + if (PCI_FUNC(pdev->devfn) == 0) {
> > + struct ocxlpmem_function0 *func0 =
> > pci_get_drvdata(pdev);
> > +
> > + if (func0) {
> > + ocxl_function_close(func0->ocxl_fn);
> > + func0->ocxl_fn = NULL;
> > + }
> > + } else {
> > + struct ocxlpmem *ocxlpmem = pci_get_drvdata(pdev);
> > +
> > + if (ocxlpmem)
> > + device_unregister(&ocxlpmem->dev);
> > + }
> > +}
> > +
> > +/**
> > + * probe_function0() - Set up function 0 for an OpenCAPI
> > persistent memory device
> > + * This is important as it enables templates higher than 0 across
> > all other functions,
> > + * which in turn enables higher bandwidth accesses
> > + * @pdev: the PCI device information struct
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int probe_function0(struct pci_dev *pdev)
> > +{
> > + struct ocxlpmem_function0 *func0 = NULL;
> > + struct ocxl_fn *fn;
> > +
> > + func0 = kzalloc(sizeof(*func0), GFP_KERNEL);
> > + if (!func0)
> > + return -ENOMEM;
> > +
> > + func0->pdev = pdev;
> > + fn = ocxl_function_open(pdev);
> > + if (IS_ERR(fn)) {
> > + kfree(func0);
> > + dev_err(&pdev->dev, "failed to open OCXL function\n");
> > + return PTR_ERR(fn);
> > + }
> > + func0->ocxl_fn = fn;
> > +
> > + pci_set_drvdata(pdev, func0);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * probe() - Init an OpenCAPI persistent memory device
> > + * @pdev: the PCI device information struct
> > + * @ent: The entry from ocxlpmem_pci_tbl
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int probe(struct pci_dev *pdev, const struct pci_device_id
> > *ent)
> > +{
> > + struct ocxlpmem *ocxlpmem;
> > + int rc;
> > +
> > + if (PCI_FUNC(pdev->devfn) == 0)
> > + return probe_function0(pdev);
> > + else if (PCI_FUNC(pdev->devfn) != 1)
> > + return 0;
> > +
> > + ocxlpmem = kzalloc(sizeof(*ocxlpmem), GFP_KERNEL);
> > + if (!ocxlpmem) {
> > + dev_err(&pdev->dev, "Could not allocate OpenCAPI
> > persistent memory metadata\n");
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > + ocxlpmem->pdev = pdev;
> > +
> > + pci_set_drvdata(pdev, ocxlpmem);
> > +
> > + ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> > + if (IS_ERR(ocxlpmem->ocxl_fn)) {
> > + kfree(ocxlpmem);
>
> You can't free this yet...
Whoops
>
> > + pci_set_drvdata(pdev, NULL);
> > + dev_err(&pdev->dev, "failed to open OCXL function\n");
> > + rc = PTR_ERR(ocxlpmem->ocxl_fn);
> > + goto err;
> > + }
> > +
> > + ocxlpmem->ocxl_afu = ocxl_function_fetch_afu(ocxlpmem->ocxl_fn,
> > 0);
> > + if (ocxlpmem->ocxl_afu == NULL) {
> > + dev_err(&pdev->dev, "Could not get OCXL AFU from
> > function\n");
> > + rc = -ENXIO;
> > + goto err;
>
> Meanwhile in this case, I think ocxlpmem and ocxlpmem->ocxl_fn get
> leaked (it's before ocxlpmem_register(), so the cleanup handler
> hasn't
> been registered).
>
> Add some new error labels for these two paths?
>
Ok
> > + }
> > +
> > + ocxl_afu_get(ocxlpmem->ocxl_afu);
> > +
> > + // Resources allocated below here are cleaned up in the release
> > handler
> > +
> > + rc = ocxlpmem_register(ocxlpmem);
> > + if (rc) {
> > + dev_err(&pdev->dev, "Could not register OpenCAPI
> > persistent memory device with the kernel\n");
> > + goto err;
> > + }
> > +
> > + rc = ocxl_context_alloc(&ocxlpmem->ocxl_context, ocxlpmem-
> > >ocxl_afu, NULL);
> > + if (rc) {
> > + dev_err(&pdev->dev, "Could not allocate OCXL
> > context\n");
> > + goto err;
> > + }
> > +
> > + rc = ocxl_context_attach(ocxlpmem->ocxl_context, 0, NULL);
> > + if (rc) {
> > + dev_err(&pdev->dev, "Could not attach ocxl context\n");
> > + goto err;
> > + }
> > +
> > + rc = register_lpc_mem(ocxlpmem);
> > + if (rc) {
> > + dev_err(&pdev->dev, "Could not register OpenCAPI
> > persistent memory with libnvdimm\n");
> > + goto err;
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + /*
> > + * Further cleanup is done in the release handler via
> > free_ocxlpmem()
> > + * This allows us to keep the character device live to handle
> > IOCTLs to
> > + * investigate issues if the card has an error
> > + */
> > +
> > + dev_err(&pdev->dev,
> > + "Error detected, will not register OpenCAPI persistent
> > memory\n");
> > + return rc;
> > +}
> > +
> > +static struct pci_driver pci_driver = {
> > + .name = "ocxl-pmem",
> > + .id_table = ocxlpmem_pci_tbl,
> > + .probe = probe,
> > + .remove = ocxlpmem_remove,
> > + .shutdown = ocxlpmem_remove,
> > +};
> > +
> > +static int __init ocxlpmem_init(void)
> > +{
> > + int rc = 0;
> > +
> > + rc = pci_register_driver(&pci_driver);
> > + if (rc)
> > + return rc;
> > +
> > + return 0;
> > +}
> > +
> > +static void ocxlpmem_exit(void)
> > +{
> > + pci_unregister_driver(&pci_driver);
> > +}
> > +
> > +module_init(ocxlpmem_init);
> > +module_exit(ocxlpmem_exit);
> > +
> > +MODULE_DESCRIPTION("OpenCAPI Persistent Memory");
> > +MODULE_LICENSE("GPL");
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > new file mode 100644
> > index 000000000000..0faf3740e9b8
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > @@ -0,0 +1,28 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +// Copyright 2019 IBM Corp.
> > +
> > +#include <linux/pci.h>
> > +#include <misc/ocxl.h>
> > +#include <linux/libnvdimm.h>
> > +#include <linux/mm.h>
> > +
> > +#define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> > +
> > +struct ocxlpmem_function0 {
> > + struct pci_dev *pdev;
> > + struct ocxl_fn *ocxl_fn;
> > +};
>
> Hmm, given this struct serves no purpose other than associating an
> ocxl_fn with a pci_dev, and special handling for function 0 is
> probably
> going to be a common pattern in ocxl dependent drivers, perhaps we
> should export a function from ocxl that converts ocxl_fn -> pci_dev,
> and
> then you can just point the drvdata straight at the ocxl_fn?
>
Yup, the struct is vestigial, I'll remove it and deal directly with the
ocxl_fn instead.
>
> > +
> > +struct ocxlpmem {
> > + struct device dev;
> > + struct pci_dev *pdev;
> > + struct ocxl_fn *ocxl_fn;
> > + struct nd_interleave_set nd_set;
> > + struct nvdimm_bus_descriptor bus_desc;
> > + struct nvdimm_bus *nvdimm_bus;
> > + struct ocxl_afu *ocxl_afu;
> > + struct ocxl_context *ocxl_context;
> > + void *metadata_addr;
> > + struct resource pmem_res;
> > + struct nd_region *nd_region;
> > +};
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* [PATCH] powerpc: fix emulate_step std test
From: Nicholas Piggin @ 2020-02-26 5:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/lib/test_emulate_step.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
index 42347067739c..00d70253cb5b 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -160,7 +160,7 @@ static void __init test_std(void)
/* std r5, 0(r3) */
stepped = emulate_step(®s, TEST_STD(5, 3, 0));
- if (stepped == 1 || regs.gpr[5] == a)
+ if (stepped == 1 && regs.gpr[5] == a)
show_result("std", "PASS");
else
show_result("std", "FAIL");
--
2.23.0
^ permalink raw reply related
* [PATCH v4 2/8] powerpc/kprobes: Mark newly allocated probes as RO
From: Russell Currey @ 2020-02-26 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, Russell Currey, dja
In-Reply-To: <20200226062403.63790-1-ruscur@russell.cc>
From: Christophe Leroy <christophe.leroy@c-s.fr>
With CONFIG_STRICT_KERNEL_RWX=y and CONFIG_KPROBES=y, there will be one
W+X page at boot by default. This can be tested with
CONFIG_PPC_PTDUMP=y and CONFIG_PPC_DEBUG_WX=y set, and checking the
kernel log during boot.
powerpc doesn't implement its own alloc() for kprobes like other
architectures do, but we couldn't immediately mark RO anyway since we do
a memcpy to the page we allocate later. After that, nothing should be
allowed to modify the page, and write permissions are removed well
before the kprobe is armed.
The memcpy() would fail if >1 probes were allocated, so use
patch_instruction() instead which is safe for RO.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/kprobes.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 2d27ec4feee4..bfab91ded234 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -24,6 +24,8 @@
#include <asm/sstep.h>
#include <asm/sections.h>
#include <linux/uaccess.h>
+#include <linux/set_memory.h>
+#include <linux/vmalloc.h>
DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -102,6 +104,16 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
return addr;
}
+void *alloc_insn_page(void)
+{
+ void *page = vmalloc_exec(PAGE_SIZE);
+
+ if (page)
+ set_memory_ro((unsigned long)page, 1);
+
+ return page;
+}
+
int arch_prepare_kprobe(struct kprobe *p)
{
int ret = 0;
@@ -124,11 +136,8 @@ int arch_prepare_kprobe(struct kprobe *p)
}
if (!ret) {
- memcpy(p->ainsn.insn, p->addr,
- MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
+ patch_instruction(p->ainsn.insn, *p->addr);
p->opcode = *p->addr;
- flush_icache_range((unsigned long)p->ainsn.insn,
- (unsigned long)p->ainsn.insn + sizeof(kprobe_opcode_t));
}
p->ainsn.boostable = 0;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 0/8] set_memory() routines and STRICT_MODULE_RWX
From: Russell Currey @ 2020-02-26 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, Russell Currey, dja
Picking up from Christophe's last series, including the following changes:
- [6/8] Cast "data" to unsigned long instead of int to fix build
- [8/8] New, to fix an issue reported by Jordan Niethe
Christophe's last series is here:
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=156428
Christophe Leroy (4):
powerpc/mm: Implement set_memory() routines
powerpc/kprobes: Mark newly allocated probes as RO
powerpc/mm: implement set_memory_attr()
powerpc/32: use set_memory_attr()
Russell Currey (4):
powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig
powerpc/mm: Disable set_memory() routines when strict RWX isn't
enabled
arch/powerpc/Kconfig | 2 +
arch/powerpc/Kconfig.debug | 6 +-
arch/powerpc/configs/skiroot_defconfig | 1 +
arch/powerpc/include/asm/set_memory.h | 34 ++++++++
arch/powerpc/kernel/kprobes.c | 17 +++-
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/pageattr.c | 112 +++++++++++++++++++++++++
arch/powerpc/mm/pgtable_32.c | 95 +++------------------
arch/powerpc/mm/ptdump/ptdump.c | 21 ++++-
9 files changed, 197 insertions(+), 93 deletions(-)
create mode 100644 arch/powerpc/include/asm/set_memory.h
create mode 100644 arch/powerpc/mm/pageattr.c
--
2.25.1
^ permalink raw reply
* [PATCH v4 1/8] powerpc/mm: Implement set_memory() routines
From: Russell Currey @ 2020-02-26 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, Russell Currey, dja
In-Reply-To: <20200226062403.63790-1-ruscur@russell.cc>
From: Christophe Leroy <christophe.leroy@c-s.fr>
The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
and are generally useful primitives to have. This implementation is
designed to be completely generic across powerpc's many MMUs.
It's possible that this could be optimised to be faster for specific
MMUs, but the focus is on having a generic and safe implementation for
now.
This implementation does not handle cases where the caller is attempting
to change the mapping of the page it is executing from, or if another
CPU is concurrently using the page being altered. These cases likely
shouldn't happen, but a more complex implementation with MMU-specific code
could safely handle them, so that is left as a TODO for now.
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/set_memory.h | 32 ++++++++++++
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/pageattr.c | 74 +++++++++++++++++++++++++++
4 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/set_memory.h
create mode 100644 arch/powerpc/mm/pageattr.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..bd074246e34e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -129,6 +129,7 @@ config PPC
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_MEMBARRIER_CALLBACKS
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
+ select ARCH_HAS_SET_MEMORY
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE
diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
new file mode 100644
index 000000000000..64011ea444b4
--- /dev/null
+++ b/arch/powerpc/include/asm/set_memory.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SET_MEMORY_H
+#define _ASM_POWERPC_SET_MEMORY_H
+
+#define SET_MEMORY_RO 0
+#define SET_MEMORY_RW 1
+#define SET_MEMORY_NX 2
+#define SET_MEMORY_X 3
+
+int change_memory_attr(unsigned long addr, int numpages, long action);
+
+static inline int set_memory_ro(unsigned long addr, int numpages)
+{
+ return change_memory_attr(addr, numpages, SET_MEMORY_RO);
+}
+
+static inline int set_memory_rw(unsigned long addr, int numpages)
+{
+ return change_memory_attr(addr, numpages, SET_MEMORY_RW);
+}
+
+static inline int set_memory_nx(unsigned long addr, int numpages)
+{
+ return change_memory_attr(addr, numpages, SET_MEMORY_NX);
+}
+
+static inline int set_memory_x(unsigned long addr, int numpages)
+{
+ return change_memory_attr(addr, numpages, SET_MEMORY_X);
+}
+
+#endif
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 5e147986400d..a998fdac52f9 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -5,7 +5,7 @@
ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
-obj-y := fault.o mem.o pgtable.o mmap.o \
+obj-y := fault.o mem.o pgtable.o mmap.o pageattr.o \
init_$(BITS).o pgtable_$(BITS).o \
pgtable-frag.o ioremap.o ioremap_$(BITS).o \
init-common.o mmu_context.o drmem.o
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
new file mode 100644
index 000000000000..2b573768a7f7
--- /dev/null
+++ b/arch/powerpc/mm/pageattr.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * MMU-generic set_memory implementation for powerpc
+ *
+ * Copyright 2019, IBM Corporation.
+ */
+
+#include <linux/mm.h>
+#include <linux/set_memory.h>
+
+#include <asm/mmu.h>
+#include <asm/page.h>
+#include <asm/pgtable.h>
+
+
+/*
+ * Updates the attributes of a page in three steps:
+ *
+ * 1. invalidate the page table entry
+ * 2. flush the TLB
+ * 3. install the new entry with the updated attributes
+ *
+ * This is unsafe if the caller is attempting to change the mapping of the
+ * page it is executing from, or if another CPU is concurrently using the
+ * page being altered.
+ *
+ * TODO make the implementation resistant to this.
+ */
+static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
+{
+ long action = (long)data;
+ pte_t pte;
+
+ spin_lock(&init_mm.page_table_lock);
+
+ /* invalidate the PTE so it's safe to modify */
+ pte = ptep_get_and_clear(&init_mm, addr, ptep);
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
+ /* modify the PTE bits as desired, then apply */
+ switch (action) {
+ case SET_MEMORY_RO:
+ pte = pte_wrprotect(pte);
+ break;
+ case SET_MEMORY_RW:
+ pte = pte_mkwrite(pte);
+ break;
+ case SET_MEMORY_NX:
+ pte = pte_exprotect(pte);
+ break;
+ case SET_MEMORY_X:
+ pte = pte_mkexec(pte);
+ break;
+ default:
+ break;
+ }
+
+ set_pte_at(&init_mm, addr, ptep, pte);
+ spin_unlock(&init_mm.page_table_lock);
+
+ return 0;
+}
+
+int change_memory_attr(unsigned long addr, int numpages, long action)
+{
+ unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
+ unsigned long sz = numpages * PAGE_SIZE;
+
+ if (!numpages)
+ return 0;
+
+ return apply_to_page_range(&init_mm, start, sz, change_page_attr, (void *)action);
+}
--
2.25.1
^ permalink raw reply related
* [PATCH v4 3/8] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
From: Russell Currey @ 2020-02-26 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, Russell Currey, dja
In-Reply-To: <20200226062403.63790-1-ruscur@russell.cc>
Very rudimentary, just
echo 1 > [debugfs]/check_wx_pages
and check the kernel log. Useful for testing strict module RWX.
Updated the Kconfig entry to reflect this.
Also fixed a typo.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/Kconfig.debug | 6 ++++--
arch/powerpc/mm/ptdump/ptdump.c | 21 ++++++++++++++++++++-
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 0b063830eea8..e37960ef68c6 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -370,7 +370,7 @@ config PPC_PTDUMP
If you are unsure, say N.
config PPC_DEBUG_WX
- bool "Warn on W+X mappings at boot"
+ bool "Warn on W+X mappings at boot & enable manual checks at runtime"
depends on PPC_PTDUMP && STRICT_KERNEL_RWX
help
Generate a warning if any W+X mappings are found at boot.
@@ -384,7 +384,9 @@ config PPC_DEBUG_WX
of other unfixed kernel bugs easier.
There is no runtime or memory usage effect of this option
- once the kernel has booted up - it's a one time check.
+ once the kernel has booted up, it only automatically checks once.
+
+ Enables the "check_wx_pages" debugfs entry for checking at runtime.
If in doubt, say "Y".
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 206156255247..a15e19a3b14e 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -4,7 +4,7 @@
*
* This traverses the kernel pagetables and dumps the
* information about the used sections of memory to
- * /sys/kernel/debug/kernel_pagetables.
+ * /sys/kernel/debug/kernel_page_tables.
*
* Derived from the arm64 implementation:
* Copyright (c) 2014, The Linux Foundation, Laura Abbott.
@@ -413,6 +413,25 @@ void ptdump_check_wx(void)
else
pr_info("Checked W+X mappings: passed, no W+X pages found\n");
}
+
+static int check_wx_debugfs_set(void *data, u64 val)
+{
+ if (val != 1ULL)
+ return -EINVAL;
+
+ ptdump_check_wx();
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(check_wx_fops, NULL, check_wx_debugfs_set, "%llu\n");
+
+static int ptdump_check_wx_init(void)
+{
+ return debugfs_create_file("check_wx_pages", 0200, NULL,
+ NULL, &check_wx_fops) ? 0 : -ENOMEM;
+}
+device_initcall(ptdump_check_wx_init);
#endif
static int ptdump_init(void)
--
2.25.1
^ permalink raw reply related
* [PATCH v4 4/8] powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
From: Russell Currey @ 2020-02-26 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, Russell Currey, dja
In-Reply-To: <20200226062403.63790-1-ruscur@russell.cc>
To enable strict module RWX on powerpc, set:
CONFIG_STRICT_MODULE_RWX=y
You should also have CONFIG_STRICT_KERNEL_RWX=y set to have any real
security benefit.
ARCH_HAS_STRICT_MODULE_RWX is set to require ARCH_HAS_STRICT_KERNEL_RWX.
This is due to a quirk in arch/Kconfig and arch/powerpc/Kconfig that
makes STRICT_MODULE_RWX *on by default* in configurations where
STRICT_KERNEL_RWX is *unavailable*.
Since this doesn't make much sense, and module RWX without kernel RWX
doesn't make much sense, having the same dependencies as kernel RWX
works around this problem.
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index bd074246e34e..e1fc7fba10bf 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -131,6 +131,7 @@ config PPC
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
select ARCH_HAS_SET_MEMORY
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
+ select ARCH_HAS_STRICT_MODULE_RWX if ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE
select ARCH_HAS_UACCESS_MCSAFE if PPC64
--
2.25.1
^ permalink raw reply related
* [PATCH v4 5/8] powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig
From: Russell Currey @ 2020-02-26 6:24 UTC (permalink / raw)
To: linuxppc-dev
Cc: ajd, kernel-hardening, Joel Stanley, npiggin, joel,
Russell Currey, dja
In-Reply-To: <20200226062403.63790-1-ruscur@russell.cc>
skiroot_defconfig is the only powerpc defconfig with STRICT_KERNEL_RWX
enabled, and if you want memory protection for kernel text you'd want it
for modules too, so enable STRICT_MODULE_RWX there.
Acked-by: Joel Stanley <joel@joel.id.au>
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/configs/skiroot_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index 1b6bdad36b13..66d20dbe67b7 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -51,6 +51,7 @@ CONFIG_CMDLINE="console=tty0 console=hvc0 ipr.fast_reboot=1 quiet"
# CONFIG_PPC_MEM_KEYS is not set
CONFIG_JUMP_LABEL=y
CONFIG_STRICT_KERNEL_RWX=y
+CONFIG_STRICT_MODULE_RWX=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_SIG_FORCE=y
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox