* [PATCH v6 21/28] powerpc: Enable Prefixed Instructions
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
From: Alistair Popple <alistair@popple.id.au>
Prefix instructions have their own FSCR bit which needs to enabled via
a CPU feature. The kernel will save the FSCR for problem state but it
needs to be enabled initially.
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.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Alistair Popple <alistair@popple.id.au>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v4:
- Squash "Check for prefixed instructions in
facility_unavailable_exception()" here
- Remove dt parts for now
---
arch/powerpc/include/asm/reg.h | 3 +++
arch/powerpc/kernel/traps.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 1aa46dff0957..c7758c2ccc5f 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -397,6 +397,7 @@
#define SPRN_RWMR 0x375 /* Region-Weighting Mode Register */
/* HFSCR and FSCR bit numbers are the same */
+#define FSCR_PREFIX_LG 13 /* Enable Prefix Instructions */
#define FSCR_SCV_LG 12 /* Enable System Call Vectored */
#define FSCR_MSGP_LG 10 /* Enable MSGP */
#define FSCR_TAR_LG 8 /* Enable Target Address Register */
@@ -408,11 +409,13 @@
#define FSCR_VECVSX_LG 1 /* Enable VMX/VSX */
#define FSCR_FP_LG 0 /* Enable Floating Point */
#define SPRN_FSCR 0x099 /* Facility Status & Control Register */
+#define FSCR_PREFIX __MASK(FSCR_PREFIX_LG)
#define FSCR_SCV __MASK(FSCR_SCV_LG)
#define FSCR_TAR __MASK(FSCR_TAR_LG)
#define FSCR_EBB __MASK(FSCR_EBB_LG)
#define FSCR_DSCR __MASK(FSCR_DSCR_LG)
#define SPRN_HFSCR 0xbe /* HV=1 Facility Status & Control Register */
+#define HFSCR_PREFIX __MASK(FSCR_PREFIX_LG)
#define HFSCR_MSGP __MASK(FSCR_MSGP_LG)
#define HFSCR_TAR __MASK(FSCR_TAR_LG)
#define HFSCR_EBB __MASK(FSCR_EBB_LG)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 82a3438300fd..a4764b039749 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1720,6 +1720,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 v6 22/28] powerpc: Define new SRR1 bits for a future ISA version
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
Add the BOUNDARY SRR1 bit definition for when the cause of an alignment
exception is a prefixed instruction that crosses a 64-byte boundary.
Add the PREFIXED SRR1 bit definition for exceptions caused by prefixed
instructions.
Bit 35 of SRR1 is called SRR1_ISI_N_OR_G. This name comes from it being
used to indicate that an ISI was due to the access being no-exec or
guarded. A future ISA version adds another purpose. It is also set if
there is an access in a cache-inhibited location for prefixed
instruction. Rename from SRR1_ISI_N_OR_G to SRR1_ISI_N_G_OR_CIP.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Combined all the commits concerning SRR1 bits.
---
arch/powerpc/include/asm/reg.h | 4 +++-
arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index c7758c2ccc5f..173f33df4fab 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -762,7 +762,7 @@
#endif
#define SRR1_ISI_NOPT 0x40000000 /* ISI: Not found in hash */
-#define SRR1_ISI_N_OR_G 0x10000000 /* ISI: Access is no-exec or G */
+#define SRR1_ISI_N_G_OR_CIP 0x10000000 /* ISI: Access is no-exec or G or CI for a prefixed instruction */
#define SRR1_ISI_PROT 0x08000000 /* ISI: Other protection fault */
#define SRR1_WAKEMASK 0x00380000 /* reason for wakeup */
#define SRR1_WAKEMASK_P8 0x003c0000 /* reason for wakeup on POWER8 and 9 */
@@ -789,6 +789,8 @@
#define SRR1_PROGADDR 0x00010000 /* SRR0 contains subsequent addr */
#define SRR1_MCE_MCP 0x00080000 /* Machine check signal caused interrupt */
+#define SRR1_BOUNDARY 0x10000000 /* Prefixed instruction crosses 64-byte boundary */
+#define SRR1_PREFIXED 0x20000000 /* Exception caused by prefixed instruction */
#define SPRN_HSRR0 0x13A /* Save/Restore Register 0 */
#define SPRN_HSRR1 0x13B /* Save/Restore Register 1 */
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index dc97e5be76f6..6ab685227574 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -1169,7 +1169,7 @@ static int kvmhv_translate_addr_nested(struct kvm_vcpu *vcpu,
} else if (vcpu->arch.trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
/* Can we execute? */
if (!gpte_p->may_execute) {
- flags |= SRR1_ISI_N_OR_G;
+ flags |= SRR1_ISI_N_G_OR_CIP;
goto forward_to_l1;
}
} else {
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 220305454c23..b53a9f1c1a46 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -1260,7 +1260,7 @@ long kvmppc_hpte_hv_fault(struct kvm_vcpu *vcpu, unsigned long addr,
status &= ~DSISR_NOHPTE; /* DSISR_NOHPTE == SRR1_ISI_NOPT */
if (!data) {
if (gr & (HPTE_R_N | HPTE_R_G))
- return status | SRR1_ISI_N_OR_G;
+ return status | SRR1_ISI_N_G_OR_CIP;
if (!hpte_read_permission(pp, slb_v & key))
return status | SRR1_ISI_PROT;
} else if (status & DSISR_ISSTORE) {
--
2.17.1
^ permalink raw reply related
* [PATCH v6 23/28] powerpc: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
For powerpc64, redefine the ppc_inst type so both word and prefixed
instructions can be represented. On powerpc32 the type will remain the
same. Update places which had assumed instructions to be 4 bytes long.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v4: New to series
v5: - Distinguish normal instructions from prefixed instructions with a
0xff marker for the suffix.
- __patch_instruction() using std for prefixed instructions
v6: - Return false instead of 0 in ppc_inst_prefixed()
- Fix up types for ppc32 so it compiles
- remove ppc_inst_write()
- __patching_instruction(): move flush out of condition
---
arch/powerpc/include/asm/inst.h | 68 +++++++++++++++++++++++++---
arch/powerpc/include/asm/kprobes.h | 2 +-
arch/powerpc/include/asm/uaccess.h | 32 ++++++++++++-
arch/powerpc/include/asm/uprobes.h | 2 +-
arch/powerpc/kernel/optprobes.c | 42 +++++++++--------
arch/powerpc/kernel/optprobes_head.S | 3 ++
arch/powerpc/lib/code-patching.c | 13 ++++--
arch/powerpc/lib/feature-fixups.c | 5 +-
arch/powerpc/lib/inst.c | 40 ++++++++++++++++
arch/powerpc/lib/sstep.c | 4 +-
arch/powerpc/xmon/xmon.c | 4 +-
arch/powerpc/xmon/xmon_bpts.S | 2 +
12 files changed, 180 insertions(+), 37 deletions(-)
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 2f3c9d5bcf7c..1e743635c214 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -8,23 +8,72 @@
struct ppc_inst {
u32 val;
+#ifdef __powerpc64__
+ u32 suffix;
+#endif /* __powerpc64__ */
} __packed;
-#define ppc_inst(x) ((struct ppc_inst){ .val = x })
-
static inline u32 ppc_inst_val(struct ppc_inst x)
{
return x.val;
}
-static inline int ppc_inst_len(struct ppc_inst x)
+static inline int ppc_inst_primary_opcode(struct ppc_inst x)
{
- return sizeof(struct ppc_inst);
+ return ppc_inst_val(x) >> 26;
}
-static inline int ppc_inst_primary_opcode(struct ppc_inst x)
+#ifdef __powerpc64__
+#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
+
+#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
{
- return ppc_inst_val(x) >> 26;
+ return x.suffix;
+}
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) != 0xff;
+}
+
+static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
+{
+ return ppc_inst_prefix(swab32(ppc_inst_val(x)),
+ swab32(ppc_inst_suffix(x)));
+}
+
+static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+{
+ u32 val, suffix;
+
+ val = *(u32 *)ptr;
+ if ((val >> 26) == 1) {
+ suffix = *((u32 *)ptr + 1);
+ return ppc_inst_prefix(val, suffix);
+ } else {
+ return ppc_inst(val);
+ }
+}
+
+static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
+{
+ return *(u64 *)&x == *(u64 *)&y;
+}
+
+#else
+
+#define ppc_inst(x) ((struct ppc_inst){ .val = x })
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return false;
+}
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
+{
+ return 0;
}
static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
@@ -42,6 +91,13 @@ static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
return ppc_inst_val(x) == ppc_inst_val(y);
}
+#endif /* __powerpc64__ */
+
+static inline int ppc_inst_len(struct ppc_inst x)
+{
+ return (ppc_inst_prefixed(x)) ? 8 : 4;
+}
+
int probe_user_read_inst(struct ppc_inst *inst,
struct ppc_inst *nip);
int probe_kernel_read_inst(struct ppc_inst *inst,
diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
index 66b3f2983b22..4fc0e15e23a5 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
extern kprobe_opcode_t optprobe_template_end[];
/* Fixed instruction size for powerpc */
-#define MAX_INSN_SIZE 1
+#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/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index c0a35e4586a5..12e52aa179b6 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -105,11 +105,41 @@ 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)))
+#ifdef __powerpc64__
+#define __get_user_instr(x, ptr) \
+({ \
+ long __gui_ret = 0; \
+ unsigned int prefix, suffix; \
+ __gui_ret = __get_user(prefix, (unsigned int __user *)ptr); \
+ if (!__gui_ret && (prefix >> 26) == 1) { \
+ __gui_ret = __get_user(suffix, (unsigned int __user *)ptr + 1); \
+ (x) = ppc_inst_prefix(prefix, suffix); \
+ } else { \
+ (x) = ppc_inst(prefix); \
+ } \
+ __gui_ret; \
+})
+
+#define __get_user_instr_inatomic(x, ptr) \
+({ \
+ long __gui_ret = 0; \
+ unsigned int prefix, suffix; \
+ __gui_ret = __get_user_inatomic(prefix, (unsigned int __user *)ptr); \
+ if (!__gui_ret && (prefix >> 26) == 1) { \
+ __gui_ret = __get_user_inatomic(suffix, (unsigned int __user *)ptr + 1); \
+ (x) = ppc_inst_prefix(prefix, suffix); \
+ } else { \
+ (x) = ppc_inst(prefix); \
+ } \
+ __gui_ret; \
+})
+#else
#define __get_user_instr(x, ptr) \
__get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
-
#define __get_user_instr_inatomic(x, ptr) \
__get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
+#endif
+
extern long __put_user_bad(void);
/*
diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h
index 7e3b329ba2d3..5bf65f5d44a9 100644
--- a/arch/powerpc/include/asm/uprobes.h
+++ b/arch/powerpc/include/asm/uprobes.h
@@ -15,7 +15,7 @@
typedef ppc_opcode_t uprobe_opcode_t;
-#define MAX_UINSN_BYTES 4
+#define MAX_UINSN_BYTES 8
#define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
/* The following alias is needed for reference from arch-agnostic code */
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index d704f9598f48..a67c5288cf50 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -159,38 +159,38 @@ void patch_imm32_load_insns(unsigned int val, kprobe_opcode_t *addr)
/*
* Generate instructions to load provided immediate 64-bit value
- * to register 'r3' and patch these instructions at 'addr'.
+ * to register 'reg' and patch these instructions at 'addr'.
*/
-void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
+void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
{
- /* lis r3,(op)@highest */
- patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(3) |
+ /* lis reg,(op)@highest */
+ patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS | ___PPC_RT(reg) |
((val >> 48) & 0xffff)));
addr++;
- /* ori r3,r3,(op)@higher */
- patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
- ___PPC_RS(3) | ((val >> 32) & 0xffff)));
+ /* ori reg,reg,(op)@higher */
+ patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
+ ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
addr++;
- /* rldicr r3,r3,32,31 */
- patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR | ___PPC_RA(3) |
- ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
+ /* rldicr reg,reg,32,31 */
+ patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR | ___PPC_RA(reg) |
+ ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
addr++;
- /* oris r3,r3,(op)@h */
- patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS | ___PPC_RA(3) |
- ___PPC_RS(3) | ((val >> 16) & 0xffff)));
+ /* oris reg,reg,(op)@h */
+ patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS | ___PPC_RA(reg) |
+ ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
addr++;
- /* ori r3,r3,(op)@l */
- patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(3) |
- ___PPC_RS(3) | (val & 0xffff)));
+ /* ori reg,reg,(op)@l */
+ patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
+ ___PPC_RS(reg) | (val & 0xffff)));
}
int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
{
- struct ppc_inst branch_op_callback, branch_emulate_step;
+ struct ppc_inst branch_op_callback, branch_emulate_step, temp;
kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
long b_offset;
unsigned long nip, size;
@@ -240,7 +240,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
* Fixup the template with instructions to:
* 1. load the address of the actual probepoint
*/
- patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
+ patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
/*
* 2. branch to optimized_callback() and emulate_step()
@@ -271,7 +271,11 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
/*
* 3. load instruction to be emulated into relevant register, and
*/
- patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
+ temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
+ patch_imm64_load_insns(ppc_inst_val(temp) |
+ ((u64)ppc_inst_suffix(temp) << 32),
+ 4,
+ buff + TMPL_INSN_IDX);
/*
* 4. branch back from trampoline
diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
index cf383520843f..ff8ba4d3824d 100644
--- a/arch/powerpc/kernel/optprobes_head.S
+++ b/arch/powerpc/kernel/optprobes_head.S
@@ -94,6 +94,9 @@ optprobe_template_insn:
/* 2, Pass instruction to be emulated in r4 */
nop
nop
+ nop
+ nop
+ nop
.global optprobe_template_call_emulate
optprobe_template_call_emulate:
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 53b0d5cbc86a..dd2f982bb29e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -24,13 +24,18 @@ static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr
{
int err = 0;
- __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
- if (err)
- return err;
+ if (!ppc_inst_prefixed(instr)) {
+ __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
+ if (err)
+ return err;
+ } else {
+ __put_user_asm((u64)ppc_inst_suffix(instr) << 32 | ppc_inst_val(instr), patch_addr, err, "std");
+ if (err)
+ return err;
+ }
asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
"r" (exec_addr));
-
return 0;
}
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index f4845e740338..243011f85287 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
src = alt_start;
dest = start;
- for (; src < alt_end; src++, dest++) {
+ for (; src < alt_end; src = (void *)src + ppc_inst_len(ppc_inst_read(src)),
+ (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
if (patch_alt_instruction(src, dest, alt_start, alt_end))
return 1;
}
- for (; dest < end; dest++)
+ for (; dest < end; dest = (void *)dest + ppc_inst_len(ppc_inst(PPC_INST_NOP)))
raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
return 0;
diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
index 08dedd927268..71101791edcc 100644
--- a/arch/powerpc/lib/inst.c
+++ b/arch/powerpc/lib/inst.c
@@ -6,6 +6,45 @@
#include <linux/uaccess.h>
#include <asm/inst.h>
+#ifdef __powerpc64__
+int probe_user_read_inst(struct ppc_inst *inst,
+ struct ppc_inst *nip)
+{
+ unsigned int val, suffix;
+ int err;
+
+ err = probe_user_read(&val, nip, sizeof(val));
+ if (err)
+ return err;
+ if ((val >> 26) == 1) {
+ err = probe_user_read(&suffix, (void *)nip+4,
+ sizeof(unsigned int));
+ *inst = ppc_inst_prefix(val, suffix);
+ } else {
+ *inst = ppc_inst(val);
+ }
+ return err;
+}
+
+int probe_kernel_read_inst(struct ppc_inst *inst,
+ struct ppc_inst *src)
+{
+ unsigned int val, suffix;
+ int err;
+
+ err = probe_kernel_read(&val, src, sizeof(val));
+ if (err)
+ return err;
+ if ((val >> 26) == 1) {
+ err = probe_kernel_read(&suffix, (void *)src+4,
+ sizeof(unsigned int));
+ *inst = ppc_inst_prefix(val, suffix);
+ } else {
+ *inst = ppc_inst(val);
+ }
+ return err;
+}
+#else
int probe_user_read_inst(struct ppc_inst *inst,
struct ppc_inst *nip)
{
@@ -27,3 +66,4 @@ int probe_kernel_read_inst(struct ppc_inst *inst,
*inst = ppc_inst(val);
return err;
}
+#endif /* __powerpc64__ */
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 13a81a55f58d..81c4c44262a6 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
- unsigned int word;
+ unsigned int word, suffix;
long ival;
word = ppc_inst_val(instr);
+ suffix = ppc_inst_suffix(instr);
+
op->type = COMPUTE;
opcode = ppc_inst_primary_opcode(instr);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ce2a0150d43c..464f93b7037d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -757,8 +757,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;
}
diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
index f3ad0ab50854..69726814cd27 100644
--- a/arch/powerpc/xmon/xmon_bpts.S
+++ b/arch/powerpc/xmon/xmon_bpts.S
@@ -4,6 +4,8 @@
#include <asm/asm-offsets.h>
#include "xmon_bpts.h"
+/* Prefixed instructions can not cross 64 byte boundaries */
+.align 6
.global bpt_table
bpt_table:
.space NBPTS * BPT_SIZE
--
2.17.1
^ permalink raw reply related
* [PATCH v6 24/28] powerpc: Test prefixed code patching
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
Expand the code-patching self-tests to includes tests for patching
prefixed instructions.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v6: New to series
---
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/code-patching.c | 21 +++++++++++++++++++++
arch/powerpc/lib/test_code-patching.S | 19 +++++++++++++++++++
3 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/lib/test_code-patching.S
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 546591848219..5e994cda8e40 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -16,7 +16,7 @@ CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
endif
-obj-y += alloc.o code-patching.o feature-fixups.o pmem.o inst.o
+obj-y += alloc.o code-patching.o feature-fixups.o pmem.o inst.o test_code-patching.o
ifndef CONFIG_KASAN
obj-y += string.o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index dd2f982bb29e..ad5754c5f007 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -700,6 +700,24 @@ static void __init test_translate_branch(void)
vfree(buf);
}
+#ifdef __powerpc64__
+static void __init test_prefixed_patching(void)
+{
+ extern unsigned int code_patching_test1[];
+ extern unsigned int code_patching_test1_expected[];
+ extern unsigned int end_code_patching_test1[];
+
+ __patch_instruction((struct ppc_inst *)code_patching_test1,
+ ppc_inst_prefix(1 << 26, 0x00000000),
+ (struct ppc_inst *)code_patching_test1);
+
+ check(!memcmp(code_patching_test1,
+ code_patching_test1_expected,
+ sizeof(unsigned int) *
+ (end_code_patching_test1 - code_patching_test1)));
+}
+#endif
+
static int __init test_code_patching(void)
{
printk(KERN_DEBUG "Running code patching self-tests ...\n");
@@ -708,6 +726,9 @@ static int __init test_code_patching(void)
test_branch_bform();
test_create_function_call();
test_translate_branch();
+#ifdef __powerpc64__
+ test_prefixed_patching();
+#endif
return 0;
}
diff --git a/arch/powerpc/lib/test_code-patching.S b/arch/powerpc/lib/test_code-patching.S
new file mode 100644
index 000000000000..91aab208a804
--- /dev/null
+++ b/arch/powerpc/lib/test_code-patching.S
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 IBM Corporation
+ */
+
+ .text
+
+#define globl(x) \
+ .globl x; \
+x:
+
+globl(code_patching_test1)
+ nop
+ nop
+globl(end_code_patching_test1)
+
+globl(code_patching_test1_expected)
+ .long 1 << 26
+ .long 0x0000000
--
2.17.1
^ permalink raw reply related
* [PATCH v6 25/28] powerpc: Test prefixed instructions in feature fixups
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
Expand the feature-fixups self-tests to includes tests for prefixed
instructions.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v6: New to series
---
arch/powerpc/lib/feature-fixups-test.S | 68 +++++++++++++++++++++++
arch/powerpc/lib/feature-fixups.c | 74 ++++++++++++++++++++++++++
2 files changed, 142 insertions(+)
diff --git a/arch/powerpc/lib/feature-fixups-test.S b/arch/powerpc/lib/feature-fixups-test.S
index b12168c2447a..6e2da9123a9b 100644
--- a/arch/powerpc/lib/feature-fixups-test.S
+++ b/arch/powerpc/lib/feature-fixups-test.S
@@ -791,3 +791,71 @@ globl(lwsync_fixup_test_expected_SYNC)
1: or 1,1,1
sync
+globl(ftr_fixup_prefix1)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+globl(end_ftr_fixup_prefix1)
+
+globl(ftr_fixup_prefix1_orig)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+
+globl(ftr_fixup_prefix1_expected)
+ or 1,1,1
+ nop
+ nop
+ or 2,2,2
+
+globl(ftr_fixup_prefix2)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+globl(end_ftr_fixup_prefix2)
+
+globl(ftr_fixup_prefix2_orig)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+
+globl(ftr_fixup_prefix2_alt)
+ .long 0x7000000
+ .long 0x0000001
+
+globl(ftr_fixup_prefix2_expected)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000001
+ or 2,2,2
+
+globl(ftr_fixup_prefix3)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+ or 3,3,3
+globl(end_ftr_fixup_prefix3)
+
+globl(ftr_fixup_prefix3_orig)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000000
+ or 2,2,2
+ or 3,3,3
+
+globl(ftr_fixup_prefix3_alt)
+ .long 1 << 26
+ .long 0x0000001
+ nop
+
+globl(ftr_fixup_prefix3_expected)
+ or 1,1,1
+ .long 1 << 26
+ .long 0x0000001
+ nop
+ or 3,3,3
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 243011f85287..6fc499b1d63e 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -687,6 +687,75 @@ static void test_lwsync_macros(void)
}
}
+#ifdef __powerpc64__
+static void __init test_prefix_patching(void)
+{
+ extern unsigned int ftr_fixup_prefix1[];
+ extern unsigned int end_ftr_fixup_prefix1[];
+ extern unsigned int ftr_fixup_prefix1_orig[];
+ extern unsigned int ftr_fixup_prefix1_expected[];
+ int size = sizeof(unsigned int) * (end_ftr_fixup_prefix1 - ftr_fixup_prefix1);
+
+ fixup.value = fixup.mask = 8;
+ fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix1 + 1);
+ fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix1 + 3);
+ fixup.alt_start_off = fixup.alt_end_off = 0;
+
+ /* Sanity check */
+ check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_orig, size) == 0);
+
+ patch_feature_section(0, &fixup);
+ check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_expected, size) == 0);
+ check(memcmp(ftr_fixup_prefix1, ftr_fixup_prefix1_orig, size) != 0);
+}
+
+static void __init test_prefix_alt_patching(void)
+{
+ extern unsigned int ftr_fixup_prefix2[];
+ extern unsigned int end_ftr_fixup_prefix2[];
+ extern unsigned int ftr_fixup_prefix2_orig[];
+ extern unsigned int ftr_fixup_prefix2_expected[];
+ extern unsigned int ftr_fixup_prefix2_alt[];
+ int size = sizeof(unsigned int) * (end_ftr_fixup_prefix2 - ftr_fixup_prefix2);
+
+ fixup.value = fixup.mask = 8;
+ fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix2 + 1);
+ fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix2 + 3);
+ fixup.alt_start_off = calc_offset(&fixup, ftr_fixup_prefix2_alt);
+ fixup.alt_end_off = calc_offset(&fixup, ftr_fixup_prefix2_alt + 2);
+ /* Sanity check */
+ check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_orig, size) == 0);
+
+ patch_feature_section(0, &fixup);
+ check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_expected, size) == 0);
+ patch_feature_section(0, &fixup);
+ check(memcmp(ftr_fixup_prefix2, ftr_fixup_prefix2_orig, size) != 0);
+}
+
+static void __init test_prefix_word_alt_patching(void)
+{
+ extern unsigned int ftr_fixup_prefix3[];
+ extern unsigned int end_ftr_fixup_prefix3[];
+ extern unsigned int ftr_fixup_prefix3_orig[];
+ extern unsigned int ftr_fixup_prefix3_expected[];
+ extern unsigned int ftr_fixup_prefix3_alt[];
+ int size = sizeof(unsigned int) * (end_ftr_fixup_prefix3 - ftr_fixup_prefix3);
+
+ fixup.value = fixup.mask = 8;
+ fixup.start_off = calc_offset(&fixup, ftr_fixup_prefix3 + 1);
+ fixup.end_off = calc_offset(&fixup, ftr_fixup_prefix3 + 4);
+ fixup.alt_start_off = calc_offset(&fixup, ftr_fixup_prefix3_alt);
+ fixup.alt_end_off = calc_offset(&fixup, ftr_fixup_prefix3_alt + 3);
+ /* Sanity check */
+ check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_orig, size) == 0);
+
+ patch_feature_section(0, &fixup);
+ check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_expected, size) == 0);
+ patch_feature_section(0, &fixup);
+ check(memcmp(ftr_fixup_prefix3, ftr_fixup_prefix3_orig, size) != 0);
+}
+#endif /* __powerpc64__ */
+
static int __init test_feature_fixups(void)
{
printk(KERN_DEBUG "Running feature fixup self-tests ...\n");
@@ -701,6 +770,11 @@ static int __init test_feature_fixups(void)
test_cpu_macros();
test_fw_macros();
test_lwsync_macros();
+#ifdef __powerpc64__
+ test_prefix_patching();
+ test_prefix_alt_patching();
+ test_prefix_word_alt_patching();
+#endif
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v6 26/28] powerpc: Support prefixed instructions in alignment handler
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
If a prefixed instruction results in an alignment exception, the
SRR1_PREFIXED bit is set. The handler attempts to emulate the
responsible instruction and then increment the NIP past it. Use
SRR1_PREFIXED to determine by how much the NIP should be incremented.
Prefixed instructions are not permitted to cross 64-byte boundaries. If
they do the alignment interrupt is invoked with SRR1 BOUNDARY bit set.
If this occurs send a SIGBUS to the offending process if in user mode.
If in kernel mode call bad_page_fault().
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Move __get_user_instr() and __get_user_instr_inatomic() to this
commit (previously in "powerpc sstep: Prepare to support prefixed
instructions").
- Rename sufx to suffix
- Use a macro for calculating instruction length
v3: Move __get_user_{instr(), instr_inatomic()} up with the other
get_user definitions and remove nested if.
v4: Rolled into "Add prefixed instructions to instruction data type"
v5: Only one definition of inst_length()
---
arch/powerpc/kernel/traps.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a4764b039749..9b97d2e6055a 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -583,6 +583,8 @@ static inline int check_io_access(struct pt_regs *regs)
#define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
#define REASON_PRIVILEGED ESR_PPR
#define REASON_TRAP ESR_PTR
+#define REASON_PREFIXED 0
+#define REASON_BOUNDARY 0
/* single-step stuff */
#define single_stepping(regs) (current->thread.debug.dbcr0 & DBCR0_IC)
@@ -597,12 +599,16 @@ static inline int check_io_access(struct pt_regs *regs)
#define REASON_ILLEGAL SRR1_PROGILL
#define REASON_PRIVILEGED SRR1_PROGPRIV
#define REASON_TRAP SRR1_PROGTRAP
+#define REASON_PREFIXED SRR1_PREFIXED
+#define REASON_BOUNDARY SRR1_BOUNDARY
#define single_stepping(regs) ((regs)->msr & MSR_SE)
#define clear_single_step(regs) ((regs)->msr &= ~MSR_SE)
#define clear_br_trace(regs) ((regs)->msr &= ~MSR_BE)
#endif
+#define inst_length(reason) (((reason) & REASON_PREFIXED) ? 8 : 4)
+
#if defined(CONFIG_E500)
int machine_check_e500mc(struct pt_regs *regs)
{
@@ -1593,11 +1599,20 @@ void alignment_exception(struct pt_regs *regs)
{
enum ctx_state prev_state = exception_enter();
int sig, code, fixed = 0;
+ unsigned long reason;
/* We restore the interrupt state now */
if (!arch_irq_disabled_regs(regs))
local_irq_enable();
+ reason = get_reason(regs);
+
+ if (reason & REASON_BOUNDARY) {
+ sig = SIGBUS;
+ code = BUS_ADRALN;
+ goto bad;
+ }
+
if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
goto bail;
@@ -1606,7 +1621,8 @@ void alignment_exception(struct pt_regs *regs)
fixed = fix_alignment(regs);
if (fixed == 1) {
- regs->nip += 4; /* skip over emulated instruction */
+ /* skip over emulated instruction */
+ regs->nip += inst_length(reason);
emulate_single_step(regs);
goto bail;
}
@@ -1619,6 +1635,7 @@ void alignment_exception(struct pt_regs *regs)
sig = SIGBUS;
code = BUS_ADRALN;
}
+bad:
if (user_mode(regs))
_exception(sig, regs, code, regs->dar);
else
--
2.17.1
^ permalink raw reply related
* [PATCH v6 27/28] powerpc sstep: Add support for prefixed load/stores
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
This adds emulation support for the following prefixed integer
load/stores:
* Prefixed Load Byte and Zero (plbz)
* Prefixed Load Halfword and Zero (plhz)
* Prefixed Load Halfword Algebraic (plha)
* Prefixed Load Word and Zero (plwz)
* Prefixed Load Word Algebraic (plwa)
* Prefixed Load Doubleword (pld)
* Prefixed Store Byte (pstb)
* Prefixed Store Halfword (psth)
* Prefixed Store Word (pstw)
* Prefixed Store Doubleword (pstd)
* Prefixed Load Quadword (plq)
* Prefixed Store Quadword (pstq)
the follow prefixed floating-point load/stores:
* Prefixed Load Floating-Point Single (plfs)
* Prefixed Load Floating-Point Double (plfd)
* Prefixed Store Floating-Point Single (pstfs)
* Prefixed Store Floating-Point Double (pstfd)
and for the following prefixed VSX load/stores:
* Prefixed Load VSX Scalar Doubleword (plxsd)
* Prefixed Load VSX Scalar Single-Precision (plxssp)
* Prefixed Load VSX Vector [0|1] (plxv, plxv0, plxv1)
* Prefixed Store VSX Scalar Doubleword (pstxsd)
* Prefixed Store VSX Scalar Single-Precision (pstxssp)
* Prefixed Store VSX Vector [0|1] (pstxv, pstxv0, pstxv1)
Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: - Combine all load/store patches
- Fix the name of Type 01 instructions
- Remove sign extension flag from pstd/pld
- Rename sufx -> suffix
v3: - Move prefixed loads and stores into the switch statement
v6: - Compile on ppc32
- Add back in + GETLENGTH(op->type)
---
arch/powerpc/include/asm/sstep.h | 4 +
arch/powerpc/lib/sstep.c | 165 ++++++++++++++++++++++++++++++-
2 files changed, 167 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index c3ce903ac488..9b200a5f8794 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -90,11 +90,15 @@ enum instruction_type {
#define VSX_LDLEFT 4 /* load VSX register from left */
#define VSX_CHECK_VEC 8 /* check MSR_VEC not MSR_VSX for reg >= 32 */
+/* Prefixed flag, ORed in with type */
+#define PREFIXED 0x800
+
/* Size field in type word */
#define SIZE(n) ((n) << 12)
#define GETSIZE(w) ((w) >> 12)
#define GETTYPE(t) ((t) & INSTR_TYPE_MASK)
+#define GETLENGTH(t) (((t) & PREFIXED) ? 8 : 4)
#define MKOP(t, f, s) ((t) | (f) | SIZE(s))
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 81c4c44262a6..d3ae0a36975f 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -187,6 +187,44 @@ static nokprobe_inline unsigned long xform_ea(unsigned int instr,
return ea;
}
+/*
+ * Calculate effective address for a MLS:D-form / 8LS:D-form
+ * prefixed instruction
+ */
+static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
+ unsigned int suffix,
+ const struct pt_regs *regs)
+{
+ int ra, prefix_r;
+ unsigned int dd;
+ unsigned long ea, d0, d1, d;
+
+ prefix_r = instr & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+
+ d0 = instr & 0x3ffff;
+ d1 = suffix & 0xffff;
+ d = (d0 << 16) | d1;
+
+ /*
+ * sign extend a 34 bit number
+ */
+ dd = (unsigned int)(d >> 2);
+ ea = (signed int)dd;
+ ea = (ea << 2) | (d & 0x3);
+
+ if (!prefix_r && ra)
+ ea += regs->gpr[ra];
+ else if (!prefix_r && !ra)
+ ; /* Leave ea as is */
+ else if (prefix_r && !ra)
+ ea += regs->nip;
+ else if (prefix_r && ra)
+ ; /* Invalid form. Should already be checked for by caller! */
+
+ return ea;
+}
+
/*
* Return the largest power of 2, not greater than sizeof(unsigned long),
* such that x is a multiple of it.
@@ -1166,6 +1204,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
struct ppc_inst instr)
{
unsigned int opcode, ra, rb, rc, rd, spr, u;
+#ifdef __powerpc64__
+ unsigned int suffixopcode, prefixtype, prefix_r;
+#endif
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
@@ -2652,6 +2693,126 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
break;
}
break;
+ case 1: /* Prefixed instructions */
+ prefix_r = word & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+ op->update_reg = ra;
+ rd = (suffix >> 21) & 0x1f;
+ op->reg = rd;
+ op->val = regs->gpr[rd];
+
+ suffixopcode = suffix >> 26;
+ prefixtype = (word >> 24) & 0x3;
+ switch (prefixtype) {
+ case 0: /* Type 00 Eight-Byte Load/Store */
+ if (prefix_r && ra)
+ break;
+ op->ea = mlsd_8lsd_ea(word, suffix, regs);
+ switch (suffixopcode) {
+ case 41: /* plwa */
+ op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 4);
+ break;
+ case 42: /* plxsd */
+ op->reg = rd + 32;
+ op->type = MKOP(LOAD_VSX, PREFIXED, 8);
+ op->element_size = 8;
+ op->vsx_flags = VSX_CHECK_VEC;
+ break;
+ case 43: /* plxssp */
+ op->reg = rd + 32;
+ op->type = MKOP(LOAD_VSX, PREFIXED, 4);
+ op->element_size = 8;
+ op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+ break;
+ case 46: /* pstxsd */
+ op->reg = rd + 32;
+ op->type = MKOP(STORE_VSX, PREFIXED, 8);
+ op->element_size = 8;
+ op->vsx_flags = VSX_CHECK_VEC;
+ break;
+ case 47: /* pstxssp */
+ op->reg = rd + 32;
+ op->type = MKOP(STORE_VSX, PREFIXED, 4);
+ op->element_size = 8;
+ op->vsx_flags = VSX_FPCONV | VSX_CHECK_VEC;
+ break;
+ case 51: /* plxv1 */
+ op->reg += 32;
+
+ /* fallthru */
+ case 50: /* plxv0 */
+ op->type = MKOP(LOAD_VSX, PREFIXED, 16);
+ op->element_size = 16;
+ op->vsx_flags = VSX_CHECK_VEC;
+ break;
+ case 55: /* pstxv1 */
+ op->reg = rd + 32;
+
+ /* fallthru */
+ case 54: /* pstxv0 */
+ op->type = MKOP(STORE_VSX, PREFIXED, 16);
+ op->element_size = 16;
+ op->vsx_flags = VSX_CHECK_VEC;
+ break;
+ case 56: /* plq */
+ op->type = MKOP(LOAD, PREFIXED, 16);
+ break;
+ case 57: /* pld */
+ op->type = MKOP(LOAD, PREFIXED, 8);
+ break;
+ case 60: /* stq */
+ op->type = MKOP(STORE, PREFIXED, 16);
+ break;
+ case 61: /* pstd */
+ op->type = MKOP(STORE, PREFIXED, 8);
+ break;
+ }
+ break;
+ case 1: /* Type 01 Eight-Byte Register-to-Register */
+ break;
+ case 2: /* Type 10 Modified Load/Store */
+ if (prefix_r && ra)
+ break;
+ op->ea = mlsd_8lsd_ea(word, suffix, regs);
+ switch (suffixopcode) {
+ case 32: /* plwz */
+ op->type = MKOP(LOAD, PREFIXED, 4);
+ break;
+ case 34: /* plbz */
+ op->type = MKOP(LOAD, PREFIXED, 1);
+ break;
+ case 36: /* pstw */
+ op->type = MKOP(STORE, PREFIXED, 4);
+ break;
+ case 38: /* pstb */
+ op->type = MKOP(STORE, PREFIXED, 1);
+ break;
+ case 40: /* plhz */
+ op->type = MKOP(LOAD, PREFIXED, 2);
+ break;
+ case 42: /* plha */
+ op->type = MKOP(LOAD, PREFIXED | SIGNEXT, 2);
+ break;
+ case 44: /* psth */
+ op->type = MKOP(STORE, PREFIXED, 2);
+ break;
+ case 48: /* plfs */
+ op->type = MKOP(LOAD_FP, PREFIXED | FPCONV, 4);
+ break;
+ case 50: /* plfd */
+ op->type = MKOP(LOAD_FP, PREFIXED, 8);
+ break;
+ case 52: /* pstfs */
+ op->type = MKOP(STORE_FP, PREFIXED | FPCONV, 4);
+ break;
+ case 54: /* pstfd */
+ op->type = MKOP(STORE_FP, PREFIXED, 8);
+ break;
+ }
+ break;
+ case 3: /* Type 11 Modified Register-to-Register */
+ break;
+ }
#endif /* __powerpc64__ */
}
@@ -2760,7 +2921,7 @@ void emulate_update_regs(struct pt_regs *regs, struct instruction_op *op)
{
unsigned long next_pc;
- next_pc = truncate_if_32bit(regs->msr, regs->nip + 4);
+ next_pc = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op->type));
switch (GETTYPE(op->type)) {
case COMPUTE:
if (op->type & SETREG)
@@ -3204,7 +3365,7 @@ int emulate_step(struct pt_regs *regs, struct ppc_inst instr)
return 0;
instr_done:
- regs->nip = truncate_if_32bit(regs->msr, regs->nip + 4);
+ regs->nip = truncate_if_32bit(regs->msr, regs->nip + GETLENGTH(op.type));
return 1;
}
NOKPROBE_SYMBOL(emulate_step);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 28/28] powerpc sstep: Add support for prefixed fixed-point arithmetic
From: Jordan Niethe @ 2020-04-28 1:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: alistair, npiggin, bala24, Jordan Niethe, dja
In-Reply-To: <20200428015814.15380-1-jniethe5@gmail.com>
This adds emulation support for the following prefixed Fixed-Point
Arithmetic instructions:
* Prefixed Add Immediate (paddi)
Reviewed-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v3: Since we moved the prefixed loads/stores into the load/store switch
statement it no longer makes sense to have paddi in there, so move it
out.
---
arch/powerpc/lib/sstep.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d3ae0a36975f..ea419f65285b 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1337,6 +1337,26 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
switch (opcode) {
#ifdef __powerpc64__
+ case 1:
+ prefix_r = word & (1ul << 20);
+ ra = (suffix >> 16) & 0x1f;
+ rd = (suffix >> 21) & 0x1f;
+ op->reg = rd;
+ op->val = regs->gpr[rd];
+ suffixopcode = suffix >> 26;
+ prefixtype = (word >> 24) & 0x3;
+ switch (prefixtype) {
+ case 2:
+ if (prefix_r && ra)
+ return 0;
+ switch (suffixopcode) {
+ case 14: /* paddi */
+ op->type = COMPUTE | PREFIXED;
+ op->val = mlsd_8lsd_ea(word, suffix, regs);
+ goto compute_done;
+ }
+ }
+ break;
case 2: /* tdi */
if (rd & trap_compare(regs->gpr[ra], (short) word))
goto trap;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 1/3] powerpc/rtas: Export rtas_error_rc
From: Sam Bobroff @ 2020-04-28 2:27 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <874kt8lve8.fsf@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]
On Fri, Apr 24, 2020 at 11:07:43AM -0500, Nathan Lynch wrote:
> Sam Bobroff <sbobroff@linux.ibm.com> writes:
> > Export rtas_error_rc() so that it can be used by other users of
> > rtas_call() (which is already exported).
>
> This will do the right thing for your ibm,configure-pe use case in patch
> 2, but the -900x => errno translations in rtas_error_rc() appear
> tailored for the indicator- and sensor-related calls that currently use
> it. From my reading of PAPR+, the meaning of a -900x RTAS status word
> depends on the call. For example, -9002 commonly means "not authorized",
> which we would typically translate to -EPERM, but rtas_error_rc() would
> translate it to -ENODEV.
>
> Also the semantics of -9001 as a return value seem to vary a bit.
>
> So I don't think rtas_error_rc() should be advertised as a generically
> useful facility in its current form.
>
> (I have had some thoughts about how firmware/hypervisor call status can
> be translated to meaningful Linux error values without tedious switch
> statements, which I'm happy to expand on if anyone is interested, but I
> don't want to hijack your submission for that discussion.)
Ah, interesting.
I'll do another version as you suggest.
Cheers,
Sam.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] powerpc/spufs: simplify spufs core dumping
From: Jeremy Kerr @ 2020-04-28 2:51 UTC (permalink / raw)
To: Al Viro, Christoph Hellwig
Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Eric W . Biederman,
linux-fsdevel, Andrew Morton, linuxppc-dev
In-Reply-To: <20200427204953.GY23230@ZenIV.linux.org.uk>
Hi Al & Christoph,
> Again, this really needs fixing. Preferably - as a separate commit
> preceding this series, so that it could be
> backported. simple_read_from_buffer() is a blocking operation.
I'll put together a patch that fixes this.
Christoph: I'll do it in a way that matches your changes to the _read
functions, so hopefully those hunks would just drop from your change,
leaving only the _dump additions. Would that work?
Cheers,
Jeremy
^ permalink raw reply
* [Bug 199471] [Bisected][Regression] windfarm_pm* no longer gets automatically loaded when CONFIG_I2C_POWERMAC=y is set
From: bugzilla-daemon @ 2020-04-28 3:14 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-199471-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=199471
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |VERIFIED
--- Comment #26 from Michael Ellerman (michael@ellerman.id.au) ---
OK thanks all.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 199471] [Bisected][Regression] windfarm_pm* no longer gets automatically loaded when CONFIG_I2C_POWERMAC=y is set
From: bugzilla-daemon @ 2020-04-28 3:14 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-199471-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=199471
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|VERIFIED |CLOSED
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 104871] bcl+8 in arch/powerpc/kernel/vdso64/datapage.S causes branch prediction issues
From: bugzilla-daemon @ 2020-04-28 3:17 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-104871-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=104871
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
CC| |michael@ellerman.id.au
Resolution|--- |CODE_FIX
--- Comment #2 from Michael Ellerman (michael@ellerman.id.au) ---
Fixed in:
c974809a26a1 ("powerpc/vdso: Avoid link stack corruption in __get_datapage()")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c974809a26a13e40254dbe3cf46f49aa32acca11
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 104871] bcl+8 in arch/powerpc/kernel/vdso64/datapage.S causes branch prediction issues
From: bugzilla-daemon @ 2020-04-28 3:17 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-104871-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=104871
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |CLOSED
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v3 1/3] powerpc/rtas: Export rtas_error_rc
From: Michael Ellerman @ 2020-04-28 3:31 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <e9c245df4a0b1cd1f68171c81e0d9e64a13ab0e9.1587704308.git.sbobroff@linux.ibm.com>
Sam Bobroff <sbobroff@linux.ibm.com> writes:
> Export rtas_error_rc() so that it can be used by other users of
> rtas_call() (which is already exported).
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> v3 * New in this version.
>
> arch/powerpc/include/asm/rtas.h | 1 +
> arch/powerpc/kernel/rtas.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 3c1887351c71..7c9e4d3635cf 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -379,6 +379,7 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
>
> extern unsigned int rtas_busy_delay_time(int status);
> extern unsigned int rtas_busy_delay(int status);
> +extern int rtas_error_rc(int rtas_rc);
>
> extern int early_init_dt_scan_rtas(unsigned long node,
> const char *uname, int depth, void *data);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index c5fa251b8950..238bf112d29a 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -518,7 +518,7 @@ unsigned int rtas_busy_delay(int status)
> }
> EXPORT_SYMBOL(rtas_busy_delay);
>
> -static int rtas_error_rc(int rtas_rc)
> +int rtas_error_rc(int rtas_rc)
> {
> int rc;
>
> @@ -546,6 +546,7 @@ static int rtas_error_rc(int rtas_rc)
> }
> return rc;
> }
> +EXPORT_SYMBOL(rtas_error_rc);
Will it be used in a module somewhere?
AFAICS the only caller you add is built-in.
cheers
^ permalink raw reply
* [PATCH v4 0/2] powerpc/eeh: Release EEH device state synchronously
From: Sam Bobroff @ 2020-04-28 3:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
Hi everyone,
Here are some fixes and cleanups that have come from other work but that I
think stand on their own.
Only one patch ("Release EEH device state synchronously", suggested by Oliver
O'Halloran) is a significant change: it moves the cleanup of some EEH device
data out of the (possibly asynchronous) device release handler and into the
(synchronously called) bus notifier. This is useful for future work as it makes
it easier to reason about the lifetimes of EEH structures.
Note that I've left a few WARN_ON_ONCEs in the code because I'm paranoid, but I
have not been able to hit them during testing.
Cheers,
Sam.
Notes for v4:
Stopped using rtas_error_rc() as it is too specific, intead just translate the
one code that is valid for this RTAS call. Therefore, the new patch to export
rtas_error_rc() is dropped.
Notes for v3:
I've tweaked the fix for pseries_eeh_configure_bridge() to return the correct
error code (even though it's not used) by calling an already present RTAS
function, rtas_error_rc(). However, I had to make another change to export that
function and while it does seem like the right thing to do, but I'm concerned
it's a bit out of scope for such a small fix.
Notes for v2:
I've dropped both cleanup patches (3/4, 4/4) because that type of cleanup
(replacing a call to eeh_rmv_from_parent_pe() with one to eeh_remove_device())
is incorrect: if called during recovery, it will cause edev->pe to remain set
when it would have been cleared previously. This would lead to stale
information in the edev. I think there should be a way to simplify the code
around EEH_PE_KEEP but I'll look at that separately.
Patch set changelog follows:
Patch set v4:
Patch 1/2 (was 2/3): powerpc/eeh: fix pseries_eeh_configure_bridge()
- Just handle the error translation locally, as it's specific to the RTAS call,
but log the unaltered code in case it's useful for debugging.
Patch 2/2 (was 3/3): powerpc/eeh: Release EEH device state synchronously
Dropped (was 1/3) powerpc/rtas: Export rtas_error_rc
Patch set v3:
Patch 1/3 (new in this version): powerpc/rtas: Export rtas_error_rc
Patch 2/3 (was 1/2): powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 3/3 (was 2/2): powerpc/eeh: Release EEH device state synchronously
Patch set v2:
Patch 1/2: powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 2/2: powerpc/eeh: Release EEH device state synchronously
- Added comment explaining why the add case can't be handled similarly to the remove case.
Dropped (was 4/4) powerpc/eeh: Clean up edev cleanup for VFs
Dropped (was 3/4) powerpc/eeh: Remove workaround from eeh_add_device_late()
Patch set v1:
Patch 1/4: powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 2/4: powerpc/eeh: Release EEH device state synchronously
Patch 3/4: powerpc/eeh: Remove workaround from eeh_add_device_late()
Patch 4/4: powerpc/eeh: Clean up edev cleanup for VFs
Sam Bobroff (2):
powerpc/eeh: fix pseries_eeh_configure_bridge()
powerpc/eeh: Release EEH device state synchronously
arch/powerpc/kernel/eeh.c | 31 ++++++++++++++++++++
arch/powerpc/kernel/pci-hotplug.c | 2 --
arch/powerpc/platforms/pseries/eeh_pseries.c | 8 ++++-
3 files changed, 38 insertions(+), 3 deletions(-)
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply
* [PATCH v4 2/2] powerpc/eeh: Release EEH device state synchronously
From: Sam Bobroff @ 2020-04-28 3:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <cover.1588045502.git.sbobroff@linux.ibm.com>
EEH device state is currently removed (by eeh_remove_device()) during
the device release handler, which is invoked as the device's reference
count drops to zero. This may take some time, or forever, as other
threads may hold references.
However, the PCI device state is released synchronously by
pci_stop_and_remove_bus_device(). This mismatch causes problems, for
example the device may be re-discovered as a new device before the
release handler has been called, leaving the PCI and EEH state
mismatched.
So instead, call eeh_remove_device() from the bus device removal
handlers, which are called synchronously in the removal path.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
arch/powerpc/kernel/eeh.c | 31 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/pci-hotplug.c | 2 --
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 17cb3e9b5697..64361311bc8e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1106,6 +1106,37 @@ static int eeh_init(void)
core_initcall_sync(eeh_init);
+static int eeh_device_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = data;
+
+ switch (action) {
+ /*
+ * Note: It's not possible to perform EEH device addition (i.e.
+ * {pseries,pnv}_pcibios_bus_add_device()) here because it depends on
+ * the device's resources, which have not yet been set up.
+ */
+ case BUS_NOTIFY_DEL_DEVICE:
+ eeh_remove_device(to_pci_dev(dev));
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block eeh_device_nb = {
+ .notifier_call = eeh_device_notifier,
+};
+
+static __init int eeh_set_bus_notifier(void)
+{
+ bus_register_notifier(&pci_bus_type, &eeh_device_nb);
+ return 0;
+}
+arch_initcall(eeh_set_bus_notifier);
+
/**
* eeh_add_device_early - Enable EEH for the indicated device node
* @pdn: PCI device node for which to set up EEH
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index d6a67f814983..28e9aa274f64 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -57,8 +57,6 @@ void pcibios_release_device(struct pci_dev *dev)
struct pci_controller *phb = pci_bus_to_host(dev->bus);
struct pci_dn *pdn = pci_get_pdn(dev);
- eeh_remove_device(dev);
-
if (phb->controller_ops.release_device)
phb->controller_ops.release_device(dev);
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply related
* [PATCH v4 1/2] powerpc/eeh: fix pseries_eeh_configure_bridge()
From: Sam Bobroff @ 2020-04-28 3:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <cover.1588045502.git.sbobroff@linux.ibm.com>
If a device is hot unplgged during EEH recovery, it's possible for the
RTAS call to ibm,configure-pe in pseries_eeh_configure() to return
parameter error (-3), however negative return values are not checked
for and this leads to an infinite loop.
Fix this by correctly bailing out on negative values.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v4 - Just handle the error translation locally, as it's specific to the RTAS call,
but log the unaltered code in case it's useful for debugging.
arch/powerpc/platforms/pseries/eeh_pseries.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 893ba3f562c4..04c1ed79bc6e 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -607,6 +607,8 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
if (!ret)
return ret;
+ if (ret < 0)
+ break;
/*
* If RTAS returns a delay value that's above 100ms, cut it
@@ -627,7 +629,11 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
pr_warn("%s: Unable to configure bridge PHB#%x-PE#%x (%d)\n",
__func__, pe->phb->global_number, pe->addr, ret);
- return ret;
+ /* PAPR defines -3 as "Parameter Error" for this function: */
+ if (ret == -3)
+ return -EINVAL;
+ else
+ return -EIO;
}
/**
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply related
* Re: [PATCH v3 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Sandipan Das @ 2020-04-28 4:17 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, Peter Xu,
linux-mm, Paul Mackerras, sparclinux, linux-riscv, Will Deacon,
Mina Almasry, linux-s390, Jonathan Corbet, Christian Borntraeger,
Ingo Molnar, Longpeng, Albert Ou, Vasily Gorbik, Paul Walmsley,
Thomas Gleixner, linux-arm-kernel, Nitesh Narayan Lal,
Randy Dunlap, linux-kernel, Palmer Dabbelt, Andrew Morton,
linuxppc-dev, David S.Miller
In-Reply-To: <b1f04f9f-fa46-c2a0-7693-4a0679d2a1ee@oracle.com>
Hi Mike,
On 28/04/20 12:39 am, Mike Kravetz wrote:
> On 4/27/20 10:25 AM, Mike Kravetz wrote:
>> On 4/26/20 10:04 PM, Sandipan Das wrote:
>>> On 18/04/20 12:20 am, Mike Kravetz wrote:
>>>> Now that architectures provide arch_hugetlb_valid_size(), parsing
>>>> of "hugepagesz=" can be done in architecture independent code.
>>>
>>> This isn't working as expected on powerpc64.
>>>
>>> [ 0.000000] Kernel command line: root=UUID=dc7b49cf-95a2-4996-8e7d-7c64ddc7a6ff hugepagesz=16G hugepages=2
>>> [ 0.000000] HugeTLB: huge pages not supported, ignoring hugepagesz = 16G
>>> [ 0.000000] HugeTLB: huge pages not supported, ignoring hugepages = 2
>>> [ 0.284177] HugeTLB registered 16.0 MiB page size, pre-allocated 0 pages
>>> [ 0.284182] HugeTLB registered 16.0 GiB page size, pre-allocated 0 pages
>>> [ 2.585062] hugepagesz=16G
>>> [ 2.585063] hugepages=2
>>>
>>
>> In the new arch independent version of hugepages_setup, I added the following
>> code in patch 4 off this series:
>>
>>> + if (!hugepages_supported()) {
>>> + pr_warn("HugeTLB: huge pages not supported, ignoring hugepages = %s\n", s);
>>> + return 0;
>>> + }
>>> +
>>
>> The easy solution is to remove all the hugepages_supported() checks from
>> command line parsing routines and rely on the later check in hugetlb_init().
>
> Here is a patch to address the issue. Sorry, as my series breaks all hugetlb
> command line processing on powerpc.
>
> Sandipan, can you test the following patch?
>
The following patch does fix the issue. Thanks.
Tested-by: Sandipan Das <sandipan@linux.ibm.com>
> From 480fe2847361e2a85aeec1fb39fe643bb7100a07 Mon Sep 17 00:00:00 2001
> From: Mike Kravetz <mike.kravetz@oracle.com>
> Date: Mon, 27 Apr 2020 11:37:30 -0700
> Subject: [PATCH] hugetlbfs: fix changes to command line processing
>
> Previously, a check for hugepages_supported was added before processing
> hugetlb command line parameters. On some architectures such as powerpc,
> hugepages_supported() is not set to true until after command line
> processing. Therefore, no hugetlb command line parameters would be
> accepted.
>
> Remove the additional checks for hugepages_supported. In hugetlb_init,
> print a warning if !hugepages_supported and command line parameters were
> specified.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> mm/hugetlb.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 1075abdb5717..5548e8851b93 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3212,8 +3212,11 @@ static int __init hugetlb_init(void)
> {
> int i;
>
> - if (!hugepages_supported())
> + if (!hugepages_supported()) {
> + if (hugetlb_max_hstate || default_hstate_max_huge_pages)
> + pr_warn("HugeTLB: huge pages not supported, ignoring associated command-line parameters\n");
> return 0;
> + }
>
> /*
> * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists. Some
> @@ -3315,11 +3318,6 @@ static int __init hugepages_setup(char *s)
> unsigned long *mhp;
> static unsigned long *last_mhp;
>
> - if (!hugepages_supported()) {
> - pr_warn("HugeTLB: huge pages not supported, ignoring hugepages = %s\n", s);
> - return 0;
> - }
> -
> if (!parsed_valid_hugepagesz) {
> pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s);
> parsed_valid_hugepagesz = true;
> @@ -3372,11 +3370,6 @@ static int __init hugepagesz_setup(char *s)
> struct hstate *h;
>
> parsed_valid_hugepagesz = false;
> - if (!hugepages_supported()) {
> - pr_warn("HugeTLB: huge pages not supported, ignoring hugepagesz = %s\n", s);
> - return 0;
> - }
> -
> size = (unsigned long)memparse(s, NULL);
>
> if (!arch_hugetlb_valid_size(size)) {
> @@ -3424,11 +3417,6 @@ static int __init default_hugepagesz_setup(char *s)
> unsigned long size;
>
> parsed_valid_hugepagesz = false;
> - if (!hugepages_supported()) {
> - pr_warn("HugeTLB: huge pages not supported, ignoring default_hugepagesz = %s\n", s);
> - return 0;
> - }
> -
> if (parsed_default_hugepagesz) {
> pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s);
> return 0;
>
^ permalink raw reply
* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Christophe Leroy @ 2020-04-28 5:19 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, bala24, dja
In-Reply-To: <20200428015814.15380-4-jniethe5@gmail.com>
Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
> The instructions for xmon's breakpoint are stored bpt_table[] which is in
> the data section. This is problematic as the data section may be marked
> as no execute. Move bpt_table[] to the text section.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v6: - New to series. Was part of the previous patch.
> - Make BPT_SIZE available in assembly
> ---
> arch/powerpc/kernel/asm-offsets.c | 8 ++++++++
> arch/powerpc/xmon/Makefile | 2 +-
> arch/powerpc/xmon/xmon.c | 6 +-----
> arch/powerpc/xmon/xmon_bpts.S | 9 +++++++++
> arch/powerpc/xmon/xmon_bpts.h | 14 ++++++++++++++
> 5 files changed, 33 insertions(+), 6 deletions(-)
> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index c25e562f1cd9..2401f415f423 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -70,6 +70,10 @@
> #include <asm/fixmap.h>
> #endif
>
> +#ifdef CONFIG_XMON
> +#include "../xmon/xmon_bpts.h"
> +#endif
> +
> #define STACK_PT_REGS_OFFSET(sym, val) \
> DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
>
> @@ -783,5 +787,9 @@ int main(void)
> DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
> #endif
>
> +#ifdef CONFIG_XMON
> + DEFINE(BPT_SIZE, BPT_SIZE);
> +#endif
> +
> return 0;
> }
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index c3842dbeb1b7..515a13ea6f28 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -21,7 +21,7 @@ endif
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y += xmon.o nonstdio.o spr_access.o
> +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>
> ifdef CONFIG_XMON_DISASSEMBLY
> obj-y += ppc-dis.o ppc-opc.o
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index a064392df1b8..f7ce3ea8694c 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -62,6 +62,7 @@
>
> #include "nonstdio.h"
> #include "dis-asm.h"
> +#include "xmon_bpts.h"
>
> #ifdef CONFIG_SMP
> static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> @@ -108,7 +109,6 @@ struct bpt {
> #define BP_TRAP 2
> #define BP_DABR 4
>
> -#define NBPTS 256
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> -#define BPT_SIZE (sizeof(unsigned int) * 2)
> -#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
> -
> /* Prototypes */
> static int cmds(struct pt_regs *);
> static int mread(unsigned long, void *, int);
> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> new file mode 100644
> index 000000000000..f3ad0ab50854
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.S
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +#include <asm/asm-compat.h>
> +#include <asm/asm-offsets.h>
> +#include "xmon_bpts.h"
> +
> +.global bpt_table
> +bpt_table:
> + .space NBPTS * BPT_SIZE
No alignment required ? Standard alignment (probably 4 bytes ?) is
acceptable ?
> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> new file mode 100644
> index 000000000000..b7e94375db86
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XMON_BPTS_H
> +#define XMON_BPTS_H
> +
> +#define NBPTS 256
> +#ifndef __ASSEMBLY__
> +#define BPT_SIZE (sizeof(unsigned int) * 2)
> +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> +
> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* XMON_BPTS_H */
>
Christophe
^ permalink raw reply
* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Jordan Niethe @ 2020-04-28 5:30 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <6761670b-545c-0120-48e3-1cadf047a95f@c-s.fr>
On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
> > The instructions for xmon's breakpoint are stored bpt_table[] which is in
> > the data section. This is problematic as the data section may be marked
> > as no execute. Move bpt_table[] to the text section.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v6: - New to series. Was part of the previous patch.
> > - Make BPT_SIZE available in assembly
> > ---
> > arch/powerpc/kernel/asm-offsets.c | 8 ++++++++
> > arch/powerpc/xmon/Makefile | 2 +-
> > arch/powerpc/xmon/xmon.c | 6 +-----
> > arch/powerpc/xmon/xmon_bpts.S | 9 +++++++++
> > arch/powerpc/xmon/xmon_bpts.h | 14 ++++++++++++++
> > 5 files changed, 33 insertions(+), 6 deletions(-)
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> > create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
> > diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> > index c25e562f1cd9..2401f415f423 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -70,6 +70,10 @@
> > #include <asm/fixmap.h>
> > #endif
> >
> > +#ifdef CONFIG_XMON
> > +#include "../xmon/xmon_bpts.h"
> > +#endif
> > +
> > #define STACK_PT_REGS_OFFSET(sym, val) \
> > DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
> >
> > @@ -783,5 +787,9 @@ int main(void)
> > DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
> > #endif
> >
> > +#ifdef CONFIG_XMON
> > + DEFINE(BPT_SIZE, BPT_SIZE);
> > +#endif
> > +
> > return 0;
> > }
> > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > index c3842dbeb1b7..515a13ea6f28 100644
> > --- a/arch/powerpc/xmon/Makefile
> > +++ b/arch/powerpc/xmon/Makefile
> > @@ -21,7 +21,7 @@ endif
> >
> > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y += xmon.o nonstdio.o spr_access.o
> > +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
> >
> > ifdef CONFIG_XMON_DISASSEMBLY
> > obj-y += ppc-dis.o ppc-opc.o
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index a064392df1b8..f7ce3ea8694c 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -62,6 +62,7 @@
> >
> > #include "nonstdio.h"
> > #include "dis-asm.h"
> > +#include "xmon_bpts.h"
> >
> > #ifdef CONFIG_SMP
> > static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> > @@ -108,7 +109,6 @@ struct bpt {
> > #define BP_TRAP 2
> > #define BP_DABR 4
> >
> > -#define NBPTS 256
> > static struct bpt bpts[NBPTS];
> > static struct bpt dabr;
> > static struct bpt *iabr;
> > @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
> >
> > #define BP_NUM(bp) ((bp) - bpts + 1)
> >
> > -#define BPT_SIZE (sizeof(unsigned int) * 2)
> > -#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> > -static unsigned int bpt_table[NBPTS * BPT_WORDS];
> > -
> > /* Prototypes */
> > static int cmds(struct pt_regs *);
> > static int mread(unsigned long, void *, int);
> > diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> > new file mode 100644
> > index 000000000000..f3ad0ab50854
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <asm/ppc_asm.h>
> > +#include <asm/asm-compat.h>
> > +#include <asm/asm-offsets.h>
> > +#include "xmon_bpts.h"
> > +
> > +.global bpt_table
> > +bpt_table:
> > + .space NBPTS * BPT_SIZE
>
> No alignment required ? Standard alignment (probably 4 bytes ?) is
> acceptable ?
No, I'll add a .balign 4 to be sure.
>
>
> > diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> > new file mode 100644
> > index 000000000000..b7e94375db86
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef XMON_BPTS_H
> > +#define XMON_BPTS_H
> > +
> > +#define NBPTS 256
> > +#ifndef __ASSEMBLY__
> > +#define BPT_SIZE (sizeof(unsigned int) * 2)
> > +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
> > +
> > +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#endif /* XMON_BPTS_H */
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH v4] pci: Make return value of pcie_capability_read*() consistent
From: Yicong Yang @ 2020-04-28 2:19 UTC (permalink / raw)
To: Bjorn Helgaas, Saheed Bolarinwa
Cc: Thomas Bogendoerfer, linux-pci, skhan, linux-mips, linux-kernel,
bjorn, linuxppc-dev
In-Reply-To: <20200427181304.GA214573@google.com>
On 2020/4/28 2:13, Bjorn Helgaas wrote:
>
> I'm starting to think we're approaching this backwards. I searched
> for PCIBIOS_FUNC_NOT_SUPPORTED, PCIBIOS_BAD_VENDOR_ID, and the other
> error values. Almost every use is a *return* in a config accessor.
> There are very, very few *tests* for these values.
If we have certain reasons to reserve PCI_BIOS* error to identify PCI errors
in PCI drivers, maybe redefine the PCI_BIOS* to generic error codes can solve
the issues, and no need to call pcibios_err_to_errno() to do the conversion.
Few changes may be made to current codes. One possible patch may
look like below. Otherwise, maybe convert all PCI_BIOS* errors to generic error
codes is a better idea.
Not sure it's the best way or not. Just FYI.
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 83ce1cd..843987c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -675,14 +675,18 @@ static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { return false;
/* Error values that may be returned by PCI functions */
#define PCIBIOS_SUCCESSFUL 0x00
-#define PCIBIOS_FUNC_NOT_SUPPORTED 0x81
-#define PCIBIOS_BAD_VENDOR_ID 0x83
-#define PCIBIOS_DEVICE_NOT_FOUND 0x86
-#define PCIBIOS_BAD_REGISTER_NUMBER 0x87
-#define PCIBIOS_SET_FAILED 0x88
-#define PCIBIOS_BUFFER_TOO_SMALL 0x89
-
-/* Translate above to generic errno for passing back through non-PCI code */
+#define PCIBIOS_FUNC_NOT_SUPPORTED -ENOENT
+#define PCIBIOS_BAD_VENDOR_ID -ENOTTY
+#define PCIBIOS_DEVICE_NOT_FOUND -ENODEV
+#define PCIBIOS_BAD_REGISTER_NUMBER -EFAULT
+#define PCIBIOS_SET_FAILED -EIO
+#define PCIBIOS_BUFFER_TOO_SMALL -ENOSPC
+
+/**
+ * Translate above to generic errno for passing back through non-PCI code
+ *
+ * Deprecated. Use the PCIBIOS_* directly without a translation.
+ */
static inline int pcibios_err_to_errno(int err)
{
if (err <= PCIBIOS_SUCCESSFUL)
@@ -690,17 +694,12 @@ static inline int pcibios_err_to_errno(int err)
switch (err) {
case PCIBIOS_FUNC_NOT_SUPPORTED:
- return -ENOENT;
case PCIBIOS_BAD_VENDOR_ID:
- return -ENOTTY;
case PCIBIOS_DEVICE_NOT_FOUND:
- return -ENODEV;
case PCIBIOS_BAD_REGISTER_NUMBER:
- return -EFAULT;
case PCIBIOS_SET_FAILED:
- return -EIO;
case PCIBIOS_BUFFER_TOO_SMALL:
- return -ENOSPC;
+ return err;
}
return -ERANGE;
>
> For example, the only tests for PCIBIOS_FUNC_NOT_SUPPORTED are in
> xen_pcibios_err_to_errno() and pcibios_err_to_errno(), i.e., we're
> just converting that value to -ENOENT or the Xen-specific thing.
>
> So I think the best approach might be to remove the PCIBIOS_* error
> values completely and replace them with the corresponding values from
> pcibios_err_to_errno(). For example, a part of the patch would look
> like this:
>
> diff --git a/arch/mips/pci/ops-emma2rh.c b/arch/mips/pci/ops-emma2rh.c
> index 65f47344536c..d4d9c902c147 100644
> --- a/arch/mips/pci/ops-emma2rh.c
> +++ b/arch/mips/pci/ops-emma2rh.c
> @@ -100,7 +100,7 @@ static int pci_config_read(struct pci_bus *bus, unsigned int devfn, int where,
> break;
> default:
> emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> - return PCIBIOS_FUNC_NOT_SUPPORTED;
> + return -ENOENT;
> }
>
> emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> @@ -149,7 +149,7 @@ static int pci_config_write(struct pci_bus *bus, unsigned int devfn, int where,
> break;
> default:
> emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> - return PCIBIOS_FUNC_NOT_SUPPORTED;
> + return -ENOENT;
> }
> *(volatile u32 *)(base + (PCI_FUNC(devfn) << 8) +
> (where & 0xfffffffc)) = data;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 83ce1cdf5676..f95637a8d391 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -675,7 +675,6 @@ static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { return false;
>
> /* Error values that may be returned by PCI functions */
> #define PCIBIOS_SUCCESSFUL 0x00
> -#define PCIBIOS_FUNC_NOT_SUPPORTED 0x81
> #define PCIBIOS_BAD_VENDOR_ID 0x83
> #define PCIBIOS_DEVICE_NOT_FOUND 0x86
> #define PCIBIOS_BAD_REGISTER_NUMBER 0x87
> @@ -689,8 +688,6 @@ static inline int pcibios_err_to_errno(int err)
> return err; /* Assume already errno */
>
> switch (err) {
> - case PCIBIOS_FUNC_NOT_SUPPORTED:
> - return -ENOENT;
> case PCIBIOS_BAD_VENDOR_ID:
> return -ENOTTY;
> case PCIBIOS_DEVICE_NOT_FOUND:
> .
>
^ permalink raw reply related
* [PATCH] tty: hvc: Fix data abort due to race in hvc_open
From: Raghavendra Rao Ananta @ 2020-04-28 3:26 UTC (permalink / raw)
To: gregkh, jslaby, andrew; +Cc: rananta, linuxppc-dev, linux-kernel
Potentially, hvc_open() can be called in parallel when two tasks calls
open() on /dev/hvcX. In such a scenario, if the hp->ops->notifier_add()
callback in the function fails, where it sets the tty->driver_data to
NULL, the parallel hvc_open() can see this NULL and cause a memory abort.
Hence, serialize hvc_open and check if tty->private_data is NULL before
proceeding ahead.
The issue can be easily reproduced by launching two tasks simultaneously
that does nothing but open() and close() on /dev/hvcX.
For example:
$ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &
Signed-off-by: Raghavendra Rao Ananta <rananta@codeaurora.org>
---
drivers/tty/hvc/hvc_console.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 436cc51c92c3..ebe26fe5ac09 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
*/
static DEFINE_MUTEX(hvc_structs_mutex);
+/* Mutex to serialize hvc_open */
+static DEFINE_MUTEX(hvc_open_mutex);
/*
* This value is used to assign a tty->index value to a hvc_struct based
* upon order of exposure via hvc_probe(), when we can not match it to
@@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver *driver, struct tty_struct *tty)
*/
static int hvc_open(struct tty_struct *tty, struct file * filp)
{
- struct hvc_struct *hp = tty->driver_data;
+ struct hvc_struct *hp;
unsigned long flags;
int rc = 0;
+ mutex_lock(&hvc_open_mutex);
+
+ hp = tty->driver_data;
+ if (!hp) {
+ rc = -EIO;
+ goto out;
+ }
+
spin_lock_irqsave(&hp->port.lock, flags);
/* Check and then increment for fast path open. */
if (hp->port.count++ > 0) {
spin_unlock_irqrestore(&hp->port.lock, flags);
hvc_kick();
- return 0;
+ goto out;
} /* else count == 0 */
spin_unlock_irqrestore(&hp->port.lock, flags);
@@ -384,6 +394,8 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
/* Force wakeup of the polling thread */
hvc_kick();
+out:
+ mutex_unlock(&hvc_open_mutex);
return rc;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc: Discard .rela* sections if CONFIG_RELOCATABLE is undefined
From: Christophe Leroy @ 2020-04-28 5:31 UTC (permalink / raw)
To: H.J. Lu, linux-kernel
Cc: linux-arch, Yu-cheng Yu, Kees Cook, Arnd Bergmann, Paul Mackerras,
Naveen N . Rao, Borislav Petkov, linuxppc-dev
In-Reply-To: <20200428014900.407098-1-hjl.tools@gmail.com>
Hi,
Le 28/04/2020 à 03:48, H.J. Lu a écrit :
> arch/powerpc/kernel/vmlinux.lds.S has
>
> DISCARDS
> /DISCARD/ : {
> *(*.EMB.apuinfo)
> *(.glink .iplt .plt .rela* .comment)
> *(.gnu.version*)
> *(.gnu.attributes)
> *(.eh_frame)
> }
>
> Since .rela* sections are needed when CONFIG_RELOCATABLE is defined,
> change to discard .rela* sections if CONFIG_RELOCATABLE is undefined.
Your explanation and especially the subject are unclear.
From the subject you understand that you are adding a discard of the
.rela* sections if CONFIG_RELOCATABLE is undefined.
But when reading the patch, you see that it is the contrary: you are
removing a discard of the .rela* sections if CONFIG_RELOCATABLE is defined.
So I think the subject could instead be:
Only discard .rela* sections when CONFIG_RELOCATABLE is undefined
Or maybe better:
Keep .rela* sections when CONFIG_RELOCATABLE is defined
And the explanation could be:
Since .rela* sections are needed when CONFIG_RELOCATABLE
is defined, don't discard .rela* sections if
CONFIG_RELOCATABLE is defined.
>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 31a0f201fb6f..4ba07734a210 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -366,9 +366,12 @@ SECTIONS
> DISCARDS
> /DISCARD/ : {
> *(*.EMB.apuinfo)
> - *(.glink .iplt .plt .rela* .comment)
> + *(.glink .iplt .plt .comment)
> *(.gnu.version*)
> *(.gnu.attributes)
> *(.eh_frame)
> +#ifndef CONFIG_RELOCATABLE
> + *(.rela*)
> +#endif
> }
> }
>
Christophe
^ permalink raw reply
* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Christophe Leroy @ 2020-04-28 5:36 UTC (permalink / raw)
To: Jordan Niethe
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <CACzsE9oF4eLH_5JUrSC1XfyWG_QzJ45HK=Rj+XwfxhuVxe5k0g@mail.gmail.com>
Le 28/04/2020 à 07:30, Jordan Niethe a écrit :
> On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>>
>>
>>
>> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
>>> The instructions for xmon's breakpoint are stored bpt_table[] which is in
>>> the data section. This is problematic as the data section may be marked
>>> as no execute. Move bpt_table[] to the text section.
>>>
>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>> ---
>>> v6: - New to series. Was part of the previous patch.
>>> - Make BPT_SIZE available in assembly
>>> ---
>>> arch/powerpc/kernel/asm-offsets.c | 8 ++++++++
>>> arch/powerpc/xmon/Makefile | 2 +-
>>> arch/powerpc/xmon/xmon.c | 6 +-----
>>> arch/powerpc/xmon/xmon_bpts.S | 9 +++++++++
>>> arch/powerpc/xmon/xmon_bpts.h | 14 ++++++++++++++
>>> 5 files changed, 33 insertions(+), 6 deletions(-)
>>> create mode 100644 arch/powerpc/xmon/xmon_bpts.S
>>> create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>>>
>>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>>> index c25e562f1cd9..2401f415f423 100644
>>> --- a/arch/powerpc/kernel/asm-offsets.c
>>> +++ b/arch/powerpc/kernel/asm-offsets.c
>>> @@ -70,6 +70,10 @@
>>> #include <asm/fixmap.h>
>>> #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> +#include "../xmon/xmon_bpts.h"
>>> +#endif
>>> +
>>> #define STACK_PT_REGS_OFFSET(sym, val) \
>>> DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
>>>
>>> @@ -783,5 +787,9 @@ int main(void)
>>> DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>>> #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> + DEFINE(BPT_SIZE, BPT_SIZE);
>>> +#endif
>>> +
>>> return 0;
>>> }
>>> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
>>> index c3842dbeb1b7..515a13ea6f28 100644
>>> --- a/arch/powerpc/xmon/Makefile
>>> +++ b/arch/powerpc/xmon/Makefile
>>> @@ -21,7 +21,7 @@ endif
>>>
>>> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>>>
>>> -obj-y += xmon.o nonstdio.o spr_access.o
>>> +obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>>>
>>> ifdef CONFIG_XMON_DISASSEMBLY
>>> obj-y += ppc-dis.o ppc-opc.o
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index a064392df1b8..f7ce3ea8694c 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -62,6 +62,7 @@
>>>
>>> #include "nonstdio.h"
>>> #include "dis-asm.h"
>>> +#include "xmon_bpts.h"
>>>
>>> #ifdef CONFIG_SMP
>>> static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
>>> @@ -108,7 +109,6 @@ struct bpt {
>>> #define BP_TRAP 2
>>> #define BP_DABR 4
>>>
>>> -#define NBPTS 256
>>> static struct bpt bpts[NBPTS];
>>> static struct bpt dabr;
>>> static struct bpt *iabr;
>>> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>>>
>>> #define BP_NUM(bp) ((bp) - bpts + 1)
>>>
>>> -#define BPT_SIZE (sizeof(unsigned int) * 2)
>>> -#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
>>> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> -
>>> /* Prototypes */
>>> static int cmds(struct pt_regs *);
>>> static int mread(unsigned long, void *, int);
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
>>> new file mode 100644
>>> index 000000000000..f3ad0ab50854
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.S
>>> @@ -0,0 +1,9 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#include <asm/ppc_asm.h>
>>> +#include <asm/asm-compat.h>
>>> +#include <asm/asm-offsets.h>
>>> +#include "xmon_bpts.h"
>>> +
>>> +.global bpt_table
>>> +bpt_table:
>>> + .space NBPTS * BPT_SIZE
>>
>> No alignment required ? Standard alignment (probably 4 bytes ?) is
>> acceptable ?
> No, I'll add a .balign 4 to be sure.
I think it is aligned to 4 by default. My question was to know whether 4
is enough.
I see BPT_SIZE is 8, should the alignment be at least 8 ?
>>
>>
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
>>> new file mode 100644
>>> index 000000000000..b7e94375db86
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef XMON_BPTS_H
>>> +#define XMON_BPTS_H
>>> +
>>> +#define NBPTS 256
>>> +#ifndef __ASSEMBLY__
>>> +#define BPT_SIZE (sizeof(unsigned int) * 2)
>>> +#define BPT_WORDS (BPT_SIZE / sizeof(unsigned int))
>>> +
>>> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> +
>>> +#endif /* __ASSEMBLY__ */
>>> +
>>> +#endif /* XMON_BPTS_H */
>>>
>>
>> Christophe
^ permalink raw reply
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