* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Mike Rapoport @ 2020-03-30 9:21 UTC (permalink / raw)
To: Michal Hocko
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200330074246.GA14243@dhcp22.suse.cz>
On Mon, Mar 30, 2020 at 09:42:46AM +0200, Michal Hocko wrote:
> On Sat 28-03-20 11:31:17, Hoan Tran wrote:
> > In NUMA layout which nodes have memory ranges that span across other nodes,
> > the mm driver can detect the memory node id incorrectly.
> >
> > For example, with layout below
> > Node 0 address: 0000 xxxx 0000 xxxx
> > Node 1 address: xxxx 1111 xxxx 1111
> >
> > Note:
> > - Memory from low to high
> > - 0/1: Node id
> > - x: Invalid memory of a node
> >
> > When mm probes the memory map, without CONFIG_NODES_SPAN_OTHER_NODES
> > config, mm only checks the memory validity but not the node id.
> > Because of that, Node 1 also detects the memory from node 0 as below
> > when it scans from the start address to the end address of node 1.
> >
> > Node 0 address: 0000 xxxx xxxx xxxx
> > Node 1 address: xxxx 1111 1111 1111
> >
> > This layout could occur on any architecture. Most of them enables
> > this config by default with CONFIG_NUMA. This patch, by default, enables
> > CONFIG_NODES_SPAN_OTHER_NODES or uses early_pfn_in_nid() for NUMA.
>
> I am not opposed to this at all. It reduces the config space and that is
> a good thing on its own. The history has shown that meory layout might
> be really wild wrt NUMA. The config is only used for early_pfn_in_nid
> which is clearly an overkill.
>
> Your description doesn't really explain why this is safe though. The
> history of this config is somehow messy, though. Mike has tried
> to remove it a94b3ab7eab4 ("[PATCH] mm: remove arch independent
> NODES_SPAN_OTHER_NODES") just to be reintroduced by 7516795739bd
> ("[PATCH] Reintroduce NODES_SPAN_OTHER_NODES for powerpc") without any
> reasoning what so ever. This doesn't make it really easy see whether
> reasons for reintroduction are still there. Maybe there are some subtle
> dependencies. I do not see any TBH but that might be burried deep in an
> arch specific code.
Well, back then early_pfn_in_nid() was arch-dependant, today everyone
except ia64 rely on HAVE_MEMBLOCK_NODE_MAP. So, if the memblock node map
is correct, that using CONFIG_NUMA instead of CONFIG_NODES_SPAN_OTHER_NODES
would only mean that early_pfn_in_nid() will cost several cycles more on
architectures that didn't select CONFIG_NODES_SPAN_OTHER_NODES (i.e. arm64
and sh).
Agian, ia64 is an exception here.
> > v3:
> > * Revise the patch description
> >
> > V2:
> > * Revise the patch description
> >
> > Hoan Tran (5):
> > mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
> > powerpc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > x86: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > sparc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > s390: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> >
> > arch/powerpc/Kconfig | 9 ---------
> > arch/s390/Kconfig | 8 --------
> > arch/sparc/Kconfig | 9 ---------
> > arch/x86/Kconfig | 9 ---------
> > mm/page_alloc.c | 2 +-
> > 5 files changed, 1 insertion(+), 36 deletions(-)
> >
> > --
> > 1.8.3.1
> >
>
> --
> Michal Hocko
> SUSE Labs
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-03-30 9:13 UTC (permalink / raw)
To: Alistair Popple
Cc: Nicholas Piggin, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <4002965.g7YitYsn6A@townsend>
On Mon, Mar 30, 2020 at 8:05 PM Alistair Popple <alistair@popple.id.au> wrote:
>
> <snip>
>
> > @@ -487,12 +487,13 @@ int kprobe_post_handler(struct pt_regs *regs)
> > {
> > struct kprobe *cur = kprobe_running();
> > struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> > + int len = ppc_inst_len(ppc_inst_read(cur->ainsn.insn));
>
> I am no kprobes expert but it seems based on the code here that in some
> circumstances it's ok/expected that cur == NULL, therefore the assignment
> needs to happen after the check below to avoid dereferencing NULL. At least
> this was the problem I was hitting when running some of the kernel selftests.
>
> - Alistair
>
> > if (!cur || user_mode(regs))
> > return 0;
Thank you for pointing that out. I'll move the assignment down here.
> >
> > /* 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 + len != regs->nip)
> > return 0;
> >
> > if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
> > @@ -501,7 +502,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 + len;
> > 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 5b53c373373b..af6761859fba 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -158,38 +158,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(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(3) |
> > + /* lis reg,(op)@highest */
> > + patch_instruction(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(reg) |
> > ((val >> 48) & 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@higher */
> > - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> > + /* ori reg,reg,(op)@higher */
> > + patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> > addr++;
> >
> > - /* rldicr r3,r3,32,31 */
> > - patch_instruction(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(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(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> > + /* oris reg,reg,(op)@h */
> > + patch_instruction(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@l */
> > - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> > - ___PPC_RS(3) | (val & 0xffff)));
> > + /* ori reg,reg,(op)@l */
> > + patch_instruction(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) {
> > - ppc_inst branch_op_callback, branch_emulate_step;
> > + 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;
> > @@ -239,7 +239,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()
> > @@ -268,7 +268,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(*(unsigned int *)p->ainsn.insn, buff +
> > TMPL_INSN_IDX); + temp = ppc_inst_read(p->ainsn.insn);
> > + patch_imm64_load_insns(ppc_inst_word(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/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c index ad451205f268..3b8655f57b4a
> > 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -42,9 +42,24 @@
> > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> >
> > static long
> > -read_inst(ppc_inst *inst, const void *src)
> > +read_inst(ppc_inst *p, const void *src)
> > {
> > - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> > + ppc_inst inst;
> > + long err;
> > +
> > + err = probe_kernel_read((void *)&inst.prefix,
> > + src, MCOUNT_INSN_SIZE);
> > + if (err)
> > + return err;
> > +
> > + if (ppc_inst_prefixed(inst))
> > + err = probe_kernel_read((void *)&inst.suffix,
> > + src + 4, MCOUNT_INSN_SIZE);
> > + if (err)
> > + return err;
> > +
> > + ppc_inst_write(p, inst);
> > + return 0;
> > }
> >
> > static ppc_inst
> > diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> > index d1dff1dc3a11..3e4fbb5c1b1e 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_len(ppc_inst_read(auprobe->insn));
> >
> > user_disable_single_step(current);
> > return 0;
> > diff --git a/arch/powerpc/lib/code-patching.c
> > b/arch/powerpc/lib/code-patching.c index fa7f32adf029..3b8277a64b8f 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -24,17 +24,27 @@ static int __patch_instruction(ppc_inst *exec_addr,
> > ppc_inst instr, {
> > int err = 0;
> >
> > - __put_user_asm(instr, patch_addr, err, "stw");
> > + __put_user_asm(ppc_inst_word(instr), patch_addr, err, "stw");
> > if (err)
> > return err;
> >
> > asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> > "r" (exec_addr));
> >
> > + if (!ppc_inst_prefixed(instr))
> > + return 0;
> > +
> > + __put_user_asm(ppc_inst_suffix(instr), patch_addr + 4, err, "stw");
> > + if (err)
> > + return err;
> > +
> > + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr + 4),
> > + "r" (exec_addr + 4));
> > +
> > return 0;
> > }
> >
> > -int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > +int raw_patch_instruction(void *addr, ppc_inst instr)
> > {
> > return __patch_instruction(addr, instr, addr);
> > }
> > @@ -184,7 +194,7 @@ static int do_patch_instruction(ppc_inst *addr, ppc_inst
> > instr)
> >
> > #endif /* CONFIG_STRICT_KERNEL_RWX */
> >
> > -int patch_instruction(unsigned int *addr, unsigned int instr)
> > +int patch_instruction(void *addr, ppc_inst instr)
> > {
> > /* Make sure we aren't patching a freed init section */
> > if (init_mem_is_free && init_section_contains(addr, 4)) {
> > @@ -195,7 +205,7 @@ int patch_instruction(unsigned int *addr, unsigned int
> > instr) }
> > NOKPROBE_SYMBOL(patch_instruction);
> >
> > -int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> > +int patch_branch(void *addr, unsigned long target, int flags)
> > {
> > return patch_instruction(addr, create_branch(addr, target, flags));
> > }
> > @@ -264,7 +274,7 @@ ppc_inst create_branch(const ppc_inst *addr,
> > return instruction;
> > }
> >
> > -unsigned int create_cond_branch(const unsigned int *addr,
> > +ppc_inst create_cond_branch(const void *addr,
> > unsigned long target, int flags)
> > {
> > ppc_inst instruction;
> > @@ -344,7 +354,7 @@ static unsigned long branch_bform_target(const ppc_inst
> > *instr) return (unsigned long)imm;
> > }
> >
> > -unsigned long branch_target(const ppc_inst *instr)
> > +unsigned long branch_target(const void *instr)
> > {
> > if (instr_is_branch_iform(ppc_inst_read(instr)))
> > return branch_iform_target(instr);
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index bae878a83fa5..ab4c71c43c8c 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_word(instr);
> > + suffix = ppc_inst_suffix(instr);
> > +
> > op->type = COMPUTE;
> >
> > opcode = word >> 26;
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index ee084411f2f5..c5536e1a3356 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -110,7 +110,7 @@ struct bpt {
> > #define BP_DABR 4
> >
> > #define NBPTS 256
> > -#define BPT_WORDS 2
> > +#define BPT_WORDS 4
> > static struct bpt bpts[NBPTS];
> > static struct bpt dabr;
> > static struct bpt *iabr;
> > @@ -118,12 +118,13 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
> >
> > #define BP_NUM(bp) ((bp) - bpts + 1)
> >
> > -static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> > BPT_WORDS]; +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS
> > * BPT_WORDS] __aligned(64);
> >
> > /* Prototypes */
> > 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, ppc_inst *);
> > static int handle_fault(struct pt_regs *);
> > static void byterev(unsigned char *, int);
> > static void memex(void);
> > @@ -759,8 +760,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;
> > }
> > @@ -862,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long
> > nip, unsigned long *offp) if (off >= sizeof(bpt_table))
> > return NULL;
> > bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> > - if (bp_off != 0 && bp_off != 4)
> > + if (bp_off != 0 && bp_off != 4 && bp_off != 8)
> > return NULL;
> > *offp = bp_off;
> > return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
> > @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
> > if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> > bp->address = a;
> > bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> > - patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
> > return bp;
> > }
> > }
> > @@ -900,7 +900,7 @@ static void insert_bpts(void)
> > for (i = 0; i < NBPTS; ++i, ++bp) {
> > if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> > continue;
> > - if (mread(bp->address, &instr, 4) != 4) {
> > + if (!mread_instr(bp->address, &instr)) {
> > printf("Couldn't read instruction at %lx, "
> > "disabling breakpoint there\n", bp->address);
> > bp->enabled = 0;
> > @@ -913,9 +913,10 @@ static void insert_bpts(void)
> > continue;
> > }
> > patch_instruction(bp->instr, instr);
> > + patch_instruction(bp->instr + ppc_inst_len(instr), PPC_INST(bpinstr));
> > if (bp->enabled & BP_CIABR)
> > continue;
> > - if (patch_instruction((ppc_inst *)bp->address,
> > + if (patch_instruction((void *)bp->address,
> > PPC_INST(bpinstr)) != 0) {
> > printf("Couldn't write instruction at %lx, "
> > "disabling breakpoint there\n", bp->address);
> > @@ -950,7 +951,7 @@ static void remove_bpts(void)
> > for (i = 0; i < NBPTS; ++i, ++bp) {
> > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> > continue;
> > - if (mread(bp->address, &instr, 4) == 4
> > + if (mread_instr(bp->address, &instr)
> > && ppc_inst_equal(instr, PPC_INST(bpinstr))
> > && patch_instruction(
> > (ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
> > @@ -1166,7 +1167,7 @@ static int do_step(struct pt_regs *regs)
> > 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) {
> > + if (mread_instr(regs->nip, &instr)) {
> > stepped = emulate_step(regs, instr);
> > if (stepped < 0) {
> > printf("Couldn't single-step %s instruction\n",
> > @@ -1333,7 +1334,7 @@ static long check_bp_loc(unsigned long addr)
> > printf("Breakpoints may only be placed at kernel addresses\n");
> > return 0;
> > }
> > - if (!mread(addr, &instr, sizeof(instr))) {
> > + if (!mread_instr(addr, &instr)) {
> > printf("Can't read instruction at address %lx\n", addr);
> > return 0;
> > }
> > @@ -2126,6 +2127,21 @@ mwrite(unsigned long adrs, void *buf, int size)
> > return n;
> > }
> >
> > +static int
> > +mread_instr(unsigned long adrs, ppc_inst *instr)
> > +{
> > + if (setjmp(bus_error_jmp) == 0) {
> > + catch_memory_errors = 1;
> > + sync();
> > + *instr = ppc_inst_read((void *)adrs);
> > + sync();
> > + /* wait a little while to see if we get a machine check */
> > + __delay(200);
> > + }
> > + catch_memory_errors = 0;
> > + return ppc_inst_len(*instr);
> > +}
> > +
> > static int fault_type;
> > static int fault_except;
> > static char *fault_chars[] = { "--", "**", "##" };
>
>
>
^ permalink raw reply
* Re: [PATCH v4 14/16] powerpc64: Add prefixed instructions to instruction data type
From: Alistair Popple @ 2020-03-30 9:05 UTC (permalink / raw)
To: Jordan Niethe; +Cc: npiggin, bala24, linuxppc-dev, dja
In-Reply-To: <20200320051809.24332-15-jniethe5@gmail.com>
<snip>
> @@ -487,12 +487,13 @@ int kprobe_post_handler(struct pt_regs *regs)
> {
> struct kprobe *cur = kprobe_running();
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> + int len = ppc_inst_len(ppc_inst_read(cur->ainsn.insn));
I am no kprobes expert but it seems based on the code here that in some
circumstances it's ok/expected that cur == NULL, therefore the assignment
needs to happen after the check below to avoid dereferencing NULL. At least
this was the problem I was hitting when running some of the kernel selftests.
- Alistair
> if (!cur || user_mode(regs))
> return 0;
>
> /* 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 + len != regs->nip)
> return 0;
>
> if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
> @@ -501,7 +502,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 + len;
> 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 5b53c373373b..af6761859fba 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -158,38 +158,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(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(3) |
> + /* lis reg,(op)@highest */
> + patch_instruction(addr, PPC_INST(PPC_INST_ADDIS | ___PPC_RT(reg) |
> ((val >> 48) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@higher */
> - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> + /* ori reg,reg,(op)@higher */
> + patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> addr++;
>
> - /* rldicr r3,r3,32,31 */
> - patch_instruction(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(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(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(3) |
> - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> + /* oris reg,reg,(op)@h */
> + patch_instruction(addr, PPC_INST(PPC_INST_ORIS | ___PPC_RA(reg) |
> + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> addr++;
>
> - /* ori r3,r3,(op)@l */
> - patch_instruction(addr, PPC_INST(PPC_INST_ORI | ___PPC_RA(3) |
> - ___PPC_RS(3) | (val & 0xffff)));
> + /* ori reg,reg,(op)@l */
> + patch_instruction(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) {
> - ppc_inst branch_op_callback, branch_emulate_step;
> + 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;
> @@ -239,7 +239,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()
> @@ -268,7 +268,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(*(unsigned int *)p->ainsn.insn, buff +
> TMPL_INSN_IDX); + temp = ppc_inst_read(p->ainsn.insn);
> + patch_imm64_load_insns(ppc_inst_word(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/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c index ad451205f268..3b8655f57b4a
> 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -42,9 +42,24 @@
> static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
>
> static long
> -read_inst(ppc_inst *inst, const void *src)
> +read_inst(ppc_inst *p, const void *src)
> {
> - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> + ppc_inst inst;
> + long err;
> +
> + err = probe_kernel_read((void *)&inst.prefix,
> + src, MCOUNT_INSN_SIZE);
> + if (err)
> + return err;
> +
> + if (ppc_inst_prefixed(inst))
> + err = probe_kernel_read((void *)&inst.suffix,
> + src + 4, MCOUNT_INSN_SIZE);
> + if (err)
> + return err;
> +
> + ppc_inst_write(p, inst);
> + return 0;
> }
>
> static ppc_inst
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index d1dff1dc3a11..3e4fbb5c1b1e 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_len(ppc_inst_read(auprobe->insn));
>
> user_disable_single_step(current);
> return 0;
> diff --git a/arch/powerpc/lib/code-patching.c
> b/arch/powerpc/lib/code-patching.c index fa7f32adf029..3b8277a64b8f 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -24,17 +24,27 @@ static int __patch_instruction(ppc_inst *exec_addr,
> ppc_inst instr, {
> int err = 0;
>
> - __put_user_asm(instr, patch_addr, err, "stw");
> + __put_user_asm(ppc_inst_word(instr), patch_addr, err, "stw");
> if (err)
> return err;
>
> asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> "r" (exec_addr));
>
> + if (!ppc_inst_prefixed(instr))
> + return 0;
> +
> + __put_user_asm(ppc_inst_suffix(instr), patch_addr + 4, err, "stw");
> + if (err)
> + return err;
> +
> + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr + 4),
> + "r" (exec_addr + 4));
> +
> return 0;
> }
>
> -int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> +int raw_patch_instruction(void *addr, ppc_inst instr)
> {
> return __patch_instruction(addr, instr, addr);
> }
> @@ -184,7 +194,7 @@ static int do_patch_instruction(ppc_inst *addr, ppc_inst
> instr)
>
> #endif /* CONFIG_STRICT_KERNEL_RWX */
>
> -int patch_instruction(unsigned int *addr, unsigned int instr)
> +int patch_instruction(void *addr, ppc_inst instr)
> {
> /* Make sure we aren't patching a freed init section */
> if (init_mem_is_free && init_section_contains(addr, 4)) {
> @@ -195,7 +205,7 @@ int patch_instruction(unsigned int *addr, unsigned int
> instr) }
> NOKPROBE_SYMBOL(patch_instruction);
>
> -int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> +int patch_branch(void *addr, unsigned long target, int flags)
> {
> return patch_instruction(addr, create_branch(addr, target, flags));
> }
> @@ -264,7 +274,7 @@ ppc_inst create_branch(const ppc_inst *addr,
> return instruction;
> }
>
> -unsigned int create_cond_branch(const unsigned int *addr,
> +ppc_inst create_cond_branch(const void *addr,
> unsigned long target, int flags)
> {
> ppc_inst instruction;
> @@ -344,7 +354,7 @@ static unsigned long branch_bform_target(const ppc_inst
> *instr) return (unsigned long)imm;
> }
>
> -unsigned long branch_target(const ppc_inst *instr)
> +unsigned long branch_target(const void *instr)
> {
> if (instr_is_branch_iform(ppc_inst_read(instr)))
> return branch_iform_target(instr);
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index bae878a83fa5..ab4c71c43c8c 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_word(instr);
> + suffix = ppc_inst_suffix(instr);
> +
> op->type = COMPUTE;
>
> opcode = word >> 26;
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index ee084411f2f5..c5536e1a3356 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -110,7 +110,7 @@ struct bpt {
> #define BP_DABR 4
>
> #define NBPTS 256
> -#define BPT_WORDS 2
> +#define BPT_WORDS 4
> static struct bpt bpts[NBPTS];
> static struct bpt dabr;
> static struct bpt *iabr;
> @@ -118,12 +118,13 @@ static unsigned bpinstr = 0x7fe00008; /* trap */
>
> #define BP_NUM(bp) ((bp) - bpts + 1)
>
> -static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> BPT_WORDS]; +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS
> * BPT_WORDS] __aligned(64);
>
> /* Prototypes */
> 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, ppc_inst *);
> static int handle_fault(struct pt_regs *);
> static void byterev(unsigned char *, int);
> static void memex(void);
> @@ -759,8 +760,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;
> }
> @@ -862,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long
> nip, unsigned long *offp) if (off >= sizeof(bpt_table))
> return NULL;
> bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> - if (bp_off != 0 && bp_off != 4)
> + if (bp_off != 0 && bp_off != 4 && bp_off != 8)
> return NULL;
> *offp = bp_off;
> return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
> @@ -881,7 +882,6 @@ static struct bpt *new_breakpoint(unsigned long a)
> if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> bp->address = a;
> bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> - patch_instruction(bp->instr + 1, PPC_INST(bpinstr));
> return bp;
> }
> }
> @@ -900,7 +900,7 @@ static void insert_bpts(void)
> for (i = 0; i < NBPTS; ++i, ++bp) {
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
> continue;
> - if (mread(bp->address, &instr, 4) != 4) {
> + if (!mread_instr(bp->address, &instr)) {
> printf("Couldn't read instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> bp->enabled = 0;
> @@ -913,9 +913,10 @@ static void insert_bpts(void)
> continue;
> }
> patch_instruction(bp->instr, instr);
> + patch_instruction(bp->instr + ppc_inst_len(instr), PPC_INST(bpinstr));
> if (bp->enabled & BP_CIABR)
> continue;
> - if (patch_instruction((ppc_inst *)bp->address,
> + if (patch_instruction((void *)bp->address,
> PPC_INST(bpinstr)) != 0) {
> printf("Couldn't write instruction at %lx, "
> "disabling breakpoint there\n", bp->address);
> @@ -950,7 +951,7 @@ static void remove_bpts(void)
> for (i = 0; i < NBPTS; ++i, ++bp) {
> if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP)
> continue;
> - if (mread(bp->address, &instr, 4) == 4
> + if (mread_instr(bp->address, &instr)
> && ppc_inst_equal(instr, PPC_INST(bpinstr))
> && patch_instruction(
> (ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
> @@ -1166,7 +1167,7 @@ static int do_step(struct pt_regs *regs)
> 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) {
> + if (mread_instr(regs->nip, &instr)) {
> stepped = emulate_step(regs, instr);
> if (stepped < 0) {
> printf("Couldn't single-step %s instruction\n",
> @@ -1333,7 +1334,7 @@ static long check_bp_loc(unsigned long addr)
> printf("Breakpoints may only be placed at kernel addresses\n");
> return 0;
> }
> - if (!mread(addr, &instr, sizeof(instr))) {
> + if (!mread_instr(addr, &instr)) {
> printf("Can't read instruction at address %lx\n", addr);
> return 0;
> }
> @@ -2126,6 +2127,21 @@ mwrite(unsigned long adrs, void *buf, int size)
> return n;
> }
>
> +static int
> +mread_instr(unsigned long adrs, ppc_inst *instr)
> +{
> + if (setjmp(bus_error_jmp) == 0) {
> + catch_memory_errors = 1;
> + sync();
> + *instr = ppc_inst_read((void *)adrs);
> + sync();
> + /* wait a little while to see if we get a machine check */
> + __delay(200);
> + }
> + catch_memory_errors = 0;
> + return ppc_inst_len(*instr);
> +}
> +
> static int fault_type;
> static int fault_except;
> static char *fault_chars[] = { "--", "**", "##" };
^ permalink raw reply
* Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms
From: Arnd Bergmann @ 2020-03-30 8:52 UTC (permalink / raw)
To: Christian Lamparter
Cc: Geert Uytterhoeven, Paul Mackerras, Mauro Carvalho Chehab,
Sasha Levin, Jonathan Corbet, Masahiro Yamada, DTML,
Nicholas Piggin, ewald_comhaire, Rob Herring, Thomas Gleixner,
Andy Shevchenko, Enrico Weigelt, Greg Kroah-Hartman,
Nick Desaulniers, linux-kernel@vger.kernel.org, Mark Brown,
linuxppc-dev, David S. Miller
In-Reply-To: <2194609.nAEUQZTCmX@debian64>
On Sat, Mar 28, 2020 at 4:07 PM Christian Lamparter <chunkeey@gmail.com> wrote:
>
> (Sorry for the bounces, yes my smarthost mail setup breaks from time to time
> and I had to cut the CC since it complained about the length.)
>
> I guess we should think about upstreaming the MyBook Live DTS. Problem here
> is that we deviated a bit from the canyonlands.dts/bluestone.dts structure
> by having a skeleton apm82181.dtsi with most of the SoC defintition and a
> wd-mybooklive.dts for the device.
I don't see much of a problem with that. If the chips are quite
different, then this
would be the right way to do it, and it matches how most dts files are
structured
on arm and others as well.
It would be nice to move over the the bluestone .dts to the apm82181.dtsi file
when that gets added, if only to ensure they use the same description for each
node, but that shouldn't stop the addition of the new file if that is needed for
distros to make use of a popular device.
I see a couple of additional files in openwrt [1],
[obviously, I'm not actually the one in charge here, the decision is with the
powerpc maintainers].
Arnd
[1] https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/apm821xx/dts;h=d359ba4ed83547438581dd40105e5100d8240beb;hb=HEAD
^ permalink raw reply
* [PATCH v4] ocxl: control via sysfs whether the FPGA is reloaded on a link reset
From: Frederic Barrat @ 2020-03-30 8:34 UTC (permalink / raw)
To: linuxppc-dev, clombard, ajd, alastair; +Cc: felix
From: Philippe Bergheaud <felix@linux.ibm.com>
Some opencapi FPGA images allow to control if the FPGA should be reloaded
on the next adapter reset. If it is supported, the image specifies it
through a Vendor Specific DVSEC in the config space of function 0.
Signed-off-by: Philippe Bergheaud <felix@linux.ibm.com>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
Changelog:
v2:
- refine ResetReload debug message
- do not call get_function_0() if pci_dev is for function 0
v3:
- avoid get_function_0() in ocxl_config_set_reset_reload also
v4:
- simplify parsing of Vendor Specific DVSEC during AFU init
- only set/unset bit 0 of the config space register
- commonize code to fetch the right PCI function and DVSEC offset
- use kstrtoint() when parsing the sysfs buffer
Documentation/ABI/testing/sysfs-class-ocxl | 10 +++
drivers/misc/ocxl/config.c | 81 ++++++++++++++++++++--
drivers/misc/ocxl/ocxl_internal.h | 6 ++
drivers/misc/ocxl/sysfs.c | 35 ++++++++++
include/misc/ocxl-config.h | 1 +
5 files changed, 128 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-ocxl b/Documentation/ABI/testing/sysfs-class-ocxl
index b5b1fa197592..b9ea671d5805 100644
--- a/Documentation/ABI/testing/sysfs-class-ocxl
+++ b/Documentation/ABI/testing/sysfs-class-ocxl
@@ -33,3 +33,13 @@ Date: January 2018
Contact: linuxppc-dev@lists.ozlabs.org
Description: read/write
Give access the global mmio area for the AFU
+
+What: /sys/class/ocxl/<afu name>/reload_on_reset
+Date: February 2020
+Contact: linuxppc-dev@lists.ozlabs.org
+Description: read/write
+ Control whether the FPGA is reloaded on a link reset
+ 0 Do not reload FPGA image from flash
+ 1 Reload FPGA image from flash
+ unavailable
+ The device does not support this capability
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index c8e19bfb5ef9..42f7a1298775 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -71,6 +71,20 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
return 0;
}
+/**
+ * get_function_0() - Find a related PCI device (function 0)
+ * @device: PCI device to match
+ *
+ * Returns a pointer to the related device, or null if not found
+ */
+static struct pci_dev *get_function_0(struct pci_dev *dev)
+{
+ unsigned int devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
+
+ return pci_get_domain_bus_and_slot(pci_domain_nr(dev->bus),
+ dev->bus->number, devfn);
+}
+
static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
{
u16 val;
@@ -159,14 +173,15 @@ static int read_dvsec_afu_info(struct pci_dev *dev, struct ocxl_fn_config *fn)
static int read_dvsec_vendor(struct pci_dev *dev)
{
int pos;
- u32 cfg, tlx, dlx;
+ u32 cfg, tlx, dlx, reset_reload;
/*
- * vendor specific DVSEC is optional
+ * vendor specific DVSEC, for IBM images only. Some older
+ * images may not have it
*
- * It's currently only used on function 0 to specify the
- * version of some logic blocks. Some older images may not
- * even have it so we ignore any errors
+ * It's only used on function 0 to specify the version of some
+ * logic blocks and to give access to special registers to
+ * enable host-based flashing.
*/
if (PCI_FUNC(dev->devfn) != 0)
return 0;
@@ -178,11 +193,67 @@ static int read_dvsec_vendor(struct pci_dev *dev)
pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_CFG_VERS, &cfg);
pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_TLX_VERS, &tlx);
pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx);
+ pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
+ &reset_reload);
dev_dbg(&dev->dev, "Vendor specific DVSEC:\n");
dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg);
dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx);
dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx);
+ dev_dbg(&dev->dev, " ResetReload = 0x%x\n", reset_reload);
+ return 0;
+}
+
+static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0,
+ int *out_pos)
+{
+ int pos;
+
+ if (PCI_FUNC(dev->devfn) != 0) {
+ dev = get_function_0(dev);
+ if (!dev)
+ return -1;
+ }
+ pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID);
+ if (!pos)
+ return -1;
+ *dev0 = dev;
+ *out_pos = pos;
+ return 0;
+}
+
+int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val)
+{
+ struct pci_dev *dev0;
+ u32 reset_reload;
+ int pos;
+
+ if (get_dvsec_vendor0(dev, &dev0, &pos))
+ return -1;
+
+ pci_read_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
+ &reset_reload);
+ *val = !!(reset_reload & BIT(0));
+ return 0;
+}
+
+int ocxl_config_set_reset_reload(struct pci_dev *dev, int val)
+{
+ struct pci_dev *dev0;
+ u32 reset_reload;
+ int pos;
+
+ if (get_dvsec_vendor0(dev, &dev0, &pos))
+ return -1;
+
+ pci_read_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
+ &reset_reload);
+ if (val)
+ reset_reload |= BIT(0);
+ else
+ reset_reload &= ~BIT(0);
+ pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
+ reset_reload);
return 0;
}
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 345bf843a38e..af9a84aeee6f 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -112,6 +112,12 @@ void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
*/
int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+/*
+ * Control whether the FPGA is reloaded on a link reset
+ */
+int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val);
+int ocxl_config_set_reset_reload(struct pci_dev *dev, int val);
+
/*
* Check if an AFU index is valid for the given function.
*
diff --git a/drivers/misc/ocxl/sysfs.c b/drivers/misc/ocxl/sysfs.c
index 58f1ba264206..25c78df8055d 100644
--- a/drivers/misc/ocxl/sysfs.c
+++ b/drivers/misc/ocxl/sysfs.c
@@ -51,11 +51,46 @@ static ssize_t contexts_show(struct device *device,
afu->pasid_count, afu->pasid_max);
}
+static ssize_t reload_on_reset_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ocxl_afu *afu = to_afu(device);
+ struct ocxl_fn *fn = afu->fn;
+ struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
+ int val;
+
+ if (ocxl_config_get_reset_reload(pci_dev, &val))
+ return scnprintf(buf, PAGE_SIZE, "unavailable\n");
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t reload_on_reset_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ocxl_afu *afu = to_afu(device);
+ struct ocxl_fn *fn = afu->fn;
+ struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
+ int rc, val;
+
+ rc = kstrtoint(buf, 0, &val);
+ if (rc || (val != 0 && val != 1))
+ return -EINVAL;
+
+ if (ocxl_config_set_reset_reload(pci_dev, val))
+ return -ENODEV;
+
+ return count;
+}
+
static struct device_attribute afu_attrs[] = {
__ATTR_RO(global_mmio_size),
__ATTR_RO(pp_mmio_size),
__ATTR_RO(afu_version),
__ATTR_RO(contexts),
+ __ATTR_RW(reload_on_reset),
};
static ssize_t global_mmio_read(struct file *filp, struct kobject *kobj,
diff --git a/include/misc/ocxl-config.h b/include/misc/ocxl-config.h
index 3526fa996a22..ccfd3b463517 100644
--- a/include/misc/ocxl-config.h
+++ b/include/misc/ocxl-config.h
@@ -41,5 +41,6 @@
#define OCXL_DVSEC_VENDOR_CFG_VERS 0x0C
#define OCXL_DVSEC_VENDOR_TLX_VERS 0x10
#define OCXL_DVSEC_VENDOR_DLX_VERS 0x20
+#define OCXL_DVSEC_VENDOR_RESET_RELOAD 0x38
#endif /* _OCXL_CONFIG_H_ */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Baoquan He @ 2020-03-30 8:28 UTC (permalink / raw)
To: Michal Hocko
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200330081659.GA6352@MiWiFi-R3L-srv>
On 03/30/20 at 04:16pm, Baoquan He wrote:
> On 03/30/20 at 09:42am, Michal Hocko wrote:
> > On Sat 28-03-20 11:31:17, Hoan Tran wrote:
> > > In NUMA layout which nodes have memory ranges that span across other nodes,
> > > the mm driver can detect the memory node id incorrectly.
> > >
> > > For example, with layout below
> > > Node 0 address: 0000 xxxx 0000 xxxx
> > > Node 1 address: xxxx 1111 xxxx 1111
> > >
> > > Note:
> > > - Memory from low to high
> > > - 0/1: Node id
> > > - x: Invalid memory of a node
> > >
> > > When mm probes the memory map, without CONFIG_NODES_SPAN_OTHER_NODES
> > > config, mm only checks the memory validity but not the node id.
> > > Because of that, Node 1 also detects the memory from node 0 as below
> > > when it scans from the start address to the end address of node 1.
> > >
> > > Node 0 address: 0000 xxxx xxxx xxxx
> > > Node 1 address: xxxx 1111 1111 1111
> > >
> > > This layout could occur on any architecture. Most of them enables
> > > this config by default with CONFIG_NUMA. This patch, by default, enables
> > > CONFIG_NODES_SPAN_OTHER_NODES or uses early_pfn_in_nid() for NUMA.
> >
> > I am not opposed to this at all. It reduces the config space and that is
> > a good thing on its own. The history has shown that meory layout might
> > be really wild wrt NUMA. The config is only used for early_pfn_in_nid
> > which is clearly an overkill.
> >
> > Your description doesn't really explain why this is safe though. The
> > history of this config is somehow messy, though. Mike has tried
> > to remove it a94b3ab7eab4 ("[PATCH] mm: remove arch independent
> > NODES_SPAN_OTHER_NODES") just to be reintroduced by 7516795739bd
> > ("[PATCH] Reintroduce NODES_SPAN_OTHER_NODES for powerpc") without any
> > reasoning what so ever. This doesn't make it really easy see whether
> > reasons for reintroduction are still there. Maybe there are some subtle
> > dependencies. I do not see any TBH but that might be burried deep in an
> > arch specific code.
>
> Yeah, since early_pfnnid_cache was added, we do not need worry about the
> performance. But when I read the mem init code on x86 again, I do see there
> are codes to handle the node overlapping, e.g in numa_cleanup_meminfo(),
> when store node id into memblock. But the thing is if we have
> encountered the node overlapping, we just return ahead of time, leave
> something uninitialized. I am wondering if the system with node
> overlapping can still run heathily.
Ok, I didn't read code carefully. That is handling case where memblock
with different node id overlap, it needs return. In the example
Hoan gave, it has no problem, system can run well. Please ignore above
comment.
^ permalink raw reply
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Baoquan He @ 2020-03-30 8:16 UTC (permalink / raw)
To: Michal Hocko
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200330074246.GA14243@dhcp22.suse.cz>
On 03/30/20 at 09:42am, Michal Hocko wrote:
> On Sat 28-03-20 11:31:17, Hoan Tran wrote:
> > In NUMA layout which nodes have memory ranges that span across other nodes,
> > the mm driver can detect the memory node id incorrectly.
> >
> > For example, with layout below
> > Node 0 address: 0000 xxxx 0000 xxxx
> > Node 1 address: xxxx 1111 xxxx 1111
> >
> > Note:
> > - Memory from low to high
> > - 0/1: Node id
> > - x: Invalid memory of a node
> >
> > When mm probes the memory map, without CONFIG_NODES_SPAN_OTHER_NODES
> > config, mm only checks the memory validity but not the node id.
> > Because of that, Node 1 also detects the memory from node 0 as below
> > when it scans from the start address to the end address of node 1.
> >
> > Node 0 address: 0000 xxxx xxxx xxxx
> > Node 1 address: xxxx 1111 1111 1111
> >
> > This layout could occur on any architecture. Most of them enables
> > this config by default with CONFIG_NUMA. This patch, by default, enables
> > CONFIG_NODES_SPAN_OTHER_NODES or uses early_pfn_in_nid() for NUMA.
>
> I am not opposed to this at all. It reduces the config space and that is
> a good thing on its own. The history has shown that meory layout might
> be really wild wrt NUMA. The config is only used for early_pfn_in_nid
> which is clearly an overkill.
>
> Your description doesn't really explain why this is safe though. The
> history of this config is somehow messy, though. Mike has tried
> to remove it a94b3ab7eab4 ("[PATCH] mm: remove arch independent
> NODES_SPAN_OTHER_NODES") just to be reintroduced by 7516795739bd
> ("[PATCH] Reintroduce NODES_SPAN_OTHER_NODES for powerpc") without any
> reasoning what so ever. This doesn't make it really easy see whether
> reasons for reintroduction are still there. Maybe there are some subtle
> dependencies. I do not see any TBH but that might be burried deep in an
> arch specific code.
Yeah, since early_pfnnid_cache was added, we do not need worry about the
performance. But when I read the mem init code on x86 again, I do see there
are codes to handle the node overlapping, e.g in numa_cleanup_meminfo(),
when store node id into memblock. But the thing is if we have
encountered the node overlapping, we just return ahead of time, leave
something uninitialized. I am wondering if the system with node
overlapping can still run heathily.
>
> > v3:
> > * Revise the patch description
> >
> > V2:
> > * Revise the patch description
> >
> > Hoan Tran (5):
> > mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
> > powerpc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > x86: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > sparc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> > s390: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> >
> > arch/powerpc/Kconfig | 9 ---------
> > arch/s390/Kconfig | 8 --------
> > arch/sparc/Kconfig | 9 ---------
> > arch/x86/Kconfig | 9 ---------
> > mm/page_alloc.c | 2 +-
> > 5 files changed, 1 insertion(+), 36 deletions(-)
> >
> > --
> > 1.8.3.1
> >
>
> --
> Michal Hocko
> SUSE Labs
>
^ permalink raw reply
* Re: linux-next: build failure after merge of the tip tree
From: Borislav Petkov @ 2020-03-30 8:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: Stephen Rothwell, Kees Cook, H.J. Lu, Peter Zijlstra,
Linux Kernel Mailing List, Linux Next Mailing List,
H. Peter Anvin, Thomas Gleixner, PowerPC, Ingo Molnar
In-Reply-To: <87wo72uv3z.fsf@mpe.ellerman.id.au>
On Mon, Mar 30, 2020 at 07:04:16PM +1100, Michael Ellerman wrote:
> Or just squash the hunk Stephen posted into the commit, which is what I
> thought would happen to begin with.
>
> You can have my ack for it:
>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Thanks but considering how this is not really urgent stuff and it can
take its time and get some wider testing before getting upstream, I'd
prefer to delay it.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
^ permalink raw reply
* Re: [PATCH v3] powerpc xmon: use `dcbf` inplace of `dcbi` instruction for 64bit Book3S
From: Christophe Leroy @ 2020-03-30 8:13 UTC (permalink / raw)
To: Balamuruhan S, mpe
Cc: ravi.bangoria, jniethe5, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200330075954.538773-1-bala24@linux.ibm.com>
Le 30/03/2020 à 09:59, Balamuruhan S a écrit :
> Data Cache Block Invalidate (dcbi) instruction implemented back in
> PowerPC architecture version 2.03. But as per Power Processor Users Manual
> it is obsolete and not supported by POWER8/POWER9 core. Attempt to use of
> this illegal instruction results in a hypervisor emulation assistance
> interrupt. So, ifdef it out the option `i` in xmon for 64bit Book3S.
>
> 0:mon> fi
> cpu 0x0: Vector: 700 (Program Check) at [c000000003be74a0]
> pc: c000000000102030: cacheflush+0x180/0x1a0
> lr: c000000000101f3c: cacheflush+0x8c/0x1a0
> sp: c000000003be7730
> msr: 8000000000081033
> current = 0xc0000000035e5c00
> paca = 0xc000000001910000 irqmask: 0x03 irq_happened: 0x01
> pid = 1025, comm = bash
> Linux version 5.6.0-rc5-g5aa19adac (root@ltc-wspoon6) (gcc version 7.4.0
> (Ubuntu 7.4.0-1ubuntu1~18.04.1)) #1 SMP Tue Mar 10 04:38:41 CDT 2020
> cpu 0x0: Exception 700 (Program Check) in xmon, returning to main loop
> [c000000003be7c50] c00000000084abb0 __handle_sysrq+0xf0/0x2a0
> [c000000003be7d00] c00000000084b3c0 write_sysrq_trigger+0xb0/0xe0
> [c000000003be7d30] c0000000004d1edc proc_reg_write+0x8c/0x130
> [c000000003be7d60] c00000000040dc7c __vfs_write+0x3c/0x70
> [c000000003be7d80] c000000000410e70 vfs_write+0xd0/0x210
> [c000000003be7dd0] c00000000041126c ksys_write+0xdc/0x130
> [c000000003be7e20] c00000000000b9d0 system_call+0x5c/0x68
> --- Exception: c01 (System Call) at 00007fffa345e420
> SP (7ffff0b08ab0) is in userspace
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/xmon/xmon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> ---
> changes in v3:
> -------------
> Fix Christophe's review comments and rebase it on latest mpe's merge
> tree,
> * mention the dcbi obsolete statement based on Power processor
> users manual in commit message.
> * make #ifdef in a more simple way.
> changes in v2:
> -------------
> Fix review comments from Segher and Michael,
> * change incorrect architecture version 2.01 to 2.03 in commit
> message.
> * ifdef it out the option `i` for PPC_BOOK3S_64 instead to drop
> it and change the commit message accordingly.
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index ea303b7e4e29..7b62d5dcc4a1 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1808,7 +1808,7 @@ static void cacheflush(void)
> catch_memory_errors = 1;
> sync();
>
> - if (cmd != 'i') {
> + if (cmd != 'i' || IS_ENABLED(CONFIG_PPC_BOOK3S_64)) {
> for (; nflush > 0; --nflush, adrs += L1_CACHE_BYTES)
> cflush((void *) adrs);
> } else {
>
> base-commit: 1ad3b5ebe084246ad593a22707cd91ef6418c31e
>
^ permalink raw reply
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Baoquan He @ 2020-03-30 8:04 UTC (permalink / raw)
To: Michal Hocko
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200330074426.GB14243@dhcp22.suse.cz>
On 03/30/20 at 09:44am, Michal Hocko wrote:
> On Sun 29-03-20 08:19:24, Baoquan He wrote:
> > On 03/28/20 at 11:31am, Hoan Tran wrote:
> > > In NUMA layout which nodes have memory ranges that span across other nodes,
> > > the mm driver can detect the memory node id incorrectly.
> > >
> > > For example, with layout below
> > > Node 0 address: 0000 xxxx 0000 xxxx
> > > Node 1 address: xxxx 1111 xxxx 1111
> >
> > Sorry, I read this example several times, but still don't get what it
> > means. Can it be given with real hex number address as an exmaple? I
> > mean just using the memory layout you have seen from some systems. The
> > change looks interesting though.
>
> Does this make it more clear?
> physical address range and its node associaion
> [0 0 0 0 1 1 1 1 0 0 0 0 1 1 1 1 0 0 0 0 1 1 1 1]
I later read it again, have got what Hoan is trying to say, thanks.
I think the change in this patchset makes sense, still have some concern
though, let me add comment in other thread.
^ permalink raw reply
* [PATCH v3] powerpc: Make setjmp/longjmp signature standard
From: Clement Courbet @ 2020-03-30 8:03 UTC (permalink / raw)
Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
clang-built-linux, Paul Mackerras, Clement Courbet,
Nathan Chancellor, linuxppc-dev, Thomas Gleixner
In-Reply-To: <20200327100801.161671-1-courbet@google.com>
Declaring setjmp()/longjmp() as taking longs makes the signature
non-standard, and makes clang complain. In the past, this has been
worked around by adding -ffreestanding to the compile flags.
The implementation looks like it only ever propagates the value
(in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
with integer parameters.
This allows removing -ffreestanding from the compilation flags.
Context:
https://lore.kernel.org/patchwork/patch/1214060
https://lore.kernel.org/patchwork/patch/1216174
Signed-off-by: Clement Courbet <courbet@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Cc: stable@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
---
v2:
Use and array type as suggested by Segher Boessenkool
Add fix tags.
v3:
Properly place tags.
---
arch/powerpc/include/asm/setjmp.h | 6 ++++--
arch/powerpc/kexec/Makefile | 3 ---
arch/powerpc/xmon/Makefile | 3 ---
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index e9f81bb3f83b..f798e80e4106 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -7,7 +7,9 @@
#define JMP_BUF_LEN 23
-extern long setjmp(long *) __attribute__((returns_twice));
-extern void longjmp(long *, long) __attribute__((noreturn));
+typedef long jmp_buf[JMP_BUF_LEN];
+
+extern int setjmp(jmp_buf env) __attribute__((returns_twice));
+extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
#endif /* _ASM_POWERPC_SETJMP_H */
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 378f6108a414..86380c69f5ce 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,9 +3,6 @@
# Makefile for the linux kernel.
#
-# Avoid clang warnings around longjmp/setjmp declarations
-CFLAGS_crash.o += -ffreestanding
-
obj-y += core.o crash.o core_$(BITS).o
obj-$(CONFIG_PPC32) += relocate_32.o
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index c3842dbeb1b7..6f9cccea54f3 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,9 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Avoid clang warnings around longjmp/setjmp declarations
-subdir-ccflags-y := -ffreestanding
-
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply related
* Re: linux-next: build failure after merge of the tip tree
From: Michael Ellerman @ 2020-03-30 8:04 UTC (permalink / raw)
To: Borislav Petkov, Stephen Rothwell
Cc: H.J. Lu, Kees Cook, Peter Zijlstra, Linux Kernel Mailing List,
Linux Next Mailing List, H. Peter Anvin, Thomas Gleixner, PowerPC,
Ingo Molnar
In-Reply-To: <20200330074823.GA14624@zn.tnic>
Borislav Petkov <bp@suse.de> writes:
> On Mon, Mar 30, 2020 at 03:08:19PM +1100, Stephen Rothwell wrote:
>> What you really need is an Ack from the PowerPC people for the fix you
>> suggested and then tha fix should go in the same series that is now
>> causing the failure (preferably before the problematic (for PowerPC)
>> patch.
>
> I'll zap this commit from the tip lineup. There's always another merge
> window.
Or just squash the hunk Stephen posted into the commit, which is what I
thought would happen to begin with.
You can have my ack for it:
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* [PATCH v3] powerpc xmon: use `dcbf` inplace of `dcbi` instruction for 64bit Book3S
From: Balamuruhan S @ 2020-03-30 7:59 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, jniethe5, Balamuruhan S, paulus, sandipan,
naveen.n.rao, linuxppc-dev
Data Cache Block Invalidate (dcbi) instruction implemented back in
PowerPC architecture version 2.03. But as per Power Processor Users Manual
it is obsolete and not supported by POWER8/POWER9 core. Attempt to use of
this illegal instruction results in a hypervisor emulation assistance
interrupt. So, ifdef it out the option `i` in xmon for 64bit Book3S.
0:mon> fi
cpu 0x0: Vector: 700 (Program Check) at [c000000003be74a0]
pc: c000000000102030: cacheflush+0x180/0x1a0
lr: c000000000101f3c: cacheflush+0x8c/0x1a0
sp: c000000003be7730
msr: 8000000000081033
current = 0xc0000000035e5c00
paca = 0xc000000001910000 irqmask: 0x03 irq_happened: 0x01
pid = 1025, comm = bash
Linux version 5.6.0-rc5-g5aa19adac (root@ltc-wspoon6) (gcc version 7.4.0
(Ubuntu 7.4.0-1ubuntu1~18.04.1)) #1 SMP Tue Mar 10 04:38:41 CDT 2020
cpu 0x0: Exception 700 (Program Check) in xmon, returning to main loop
[c000000003be7c50] c00000000084abb0 __handle_sysrq+0xf0/0x2a0
[c000000003be7d00] c00000000084b3c0 write_sysrq_trigger+0xb0/0xe0
[c000000003be7d30] c0000000004d1edc proc_reg_write+0x8c/0x130
[c000000003be7d60] c00000000040dc7c __vfs_write+0x3c/0x70
[c000000003be7d80] c000000000410e70 vfs_write+0xd0/0x210
[c000000003be7dd0] c00000000041126c ksys_write+0xdc/0x130
[c000000003be7e20] c00000000000b9d0 system_call+0x5c/0x68
--- Exception: c01 (System Call) at 00007fffa345e420
SP (7ffff0b08ab0) is in userspace
Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
arch/powerpc/xmon/xmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
changes in v3:
-------------
Fix Christophe's review comments and rebase it on latest mpe's merge
tree,
* mention the dcbi obsolete statement based on Power processor
users manual in commit message.
* make #ifdef in a more simple way.
changes in v2:
-------------
Fix review comments from Segher and Michael,
* change incorrect architecture version 2.01 to 2.03 in commit
message.
* ifdef it out the option `i` for PPC_BOOK3S_64 instead to drop
it and change the commit message accordingly.
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ea303b7e4e29..7b62d5dcc4a1 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1808,7 +1808,7 @@ static void cacheflush(void)
catch_memory_errors = 1;
sync();
- if (cmd != 'i') {
+ if (cmd != 'i' || IS_ENABLED(CONFIG_PPC_BOOK3S_64)) {
for (; nflush > 0; --nflush, adrs += L1_CACHE_BYTES)
cflush((void *) adrs);
} else {
base-commit: 1ad3b5ebe084246ad593a22707cd91ef6418c31e
--
2.24.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc: Make setjmp/longjmp signature standard
From: Nathan Chancellor @ 2020-03-30 7:57 UTC (permalink / raw)
To: Clement Courbet
Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
clang-built-linux, Paul Mackerras, Thomas Gleixner, linuxppc-dev,
Allison Randal
In-Reply-To: <20200330064323.76162-1-courbet@google.com>
On Mon, Mar 30, 2020 at 08:43:19AM +0200, Clement Courbet wrote:
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
>
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
>
> This allows removing -ffreestanding from the compilation flags.
>
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
>
> Signed-off-by: Clement Courbet <courbet@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> ---
>
> v2:
> Use and array type as suggested by Segher Boessenkool
> Cc: stable@vger.kernel.org # v4.14+
> Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
Actual diff itself looks good but these two tags need to be above your
signoff to be included in the final changelog. Not sure if Michael will
want a v3 with that or if it can manually be done when applying.
Cheers,
Nathan
^ permalink raw reply
* Re: linux-next: build failure after merge of the tip tree
From: Borislav Petkov @ 2020-03-30 7:48 UTC (permalink / raw)
To: Stephen Rothwell
Cc: H.J. Lu, Kees Cook, Peter Zijlstra, Linux Kernel Mailing List,
Linux Next Mailing List, H. Peter Anvin, Thomas Gleixner, PowerPC,
Ingo Molnar
In-Reply-To: <20200330150819.7f0199a2@canb.auug.org.au>
On Mon, Mar 30, 2020 at 03:08:19PM +1100, Stephen Rothwell wrote:
> What you really need is an Ack from the PowerPC people for the fix you
> suggested and then tha fix should go in the same series that is now
> causing the failure (preferably before the problematic (for PowerPC)
> patch.
I'll zap this commit from the tip lineup. There's always another merge
window.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
^ permalink raw reply
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Michal Hocko @ 2020-03-30 7:44 UTC (permalink / raw)
To: Baoquan He
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran, Pavel Tatashin,
lho, Vasily Gorbik, Vlastimil Babka, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200329001924.GS3039@MiWiFi-R3L-srv>
On Sun 29-03-20 08:19:24, Baoquan He wrote:
> On 03/28/20 at 11:31am, Hoan Tran wrote:
> > In NUMA layout which nodes have memory ranges that span across other nodes,
> > the mm driver can detect the memory node id incorrectly.
> >
> > For example, with layout below
> > Node 0 address: 0000 xxxx 0000 xxxx
> > Node 1 address: xxxx 1111 xxxx 1111
>
> Sorry, I read this example several times, but still don't get what it
> means. Can it be given with real hex number address as an exmaple? I
> mean just using the memory layout you have seen from some systems. The
> change looks interesting though.
Does this make it more clear?
physical address range and its node associaion
[0 0 0 0 1 1 1 1 0 0 0 0 1 1 1 1 0 0 0 0 1 1 1 1]
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Michal Hocko @ 2020-03-30 7:42 UTC (permalink / raw)
To: Hoan Tran
Cc: mmorana, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Vlastimil Babka,
Pavel Tatashin, lho, Vasily Gorbik, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1585420282-25630-1-git-send-email-Hoan@os.amperecomputing.com>
On Sat 28-03-20 11:31:17, Hoan Tran wrote:
> In NUMA layout which nodes have memory ranges that span across other nodes,
> the mm driver can detect the memory node id incorrectly.
>
> For example, with layout below
> Node 0 address: 0000 xxxx 0000 xxxx
> Node 1 address: xxxx 1111 xxxx 1111
>
> Note:
> - Memory from low to high
> - 0/1: Node id
> - x: Invalid memory of a node
>
> When mm probes the memory map, without CONFIG_NODES_SPAN_OTHER_NODES
> config, mm only checks the memory validity but not the node id.
> Because of that, Node 1 also detects the memory from node 0 as below
> when it scans from the start address to the end address of node 1.
>
> Node 0 address: 0000 xxxx xxxx xxxx
> Node 1 address: xxxx 1111 1111 1111
>
> This layout could occur on any architecture. Most of them enables
> this config by default with CONFIG_NUMA. This patch, by default, enables
> CONFIG_NODES_SPAN_OTHER_NODES or uses early_pfn_in_nid() for NUMA.
I am not opposed to this at all. It reduces the config space and that is
a good thing on its own. The history has shown that meory layout might
be really wild wrt NUMA. The config is only used for early_pfn_in_nid
which is clearly an overkill.
Your description doesn't really explain why this is safe though. The
history of this config is somehow messy, though. Mike has tried
to remove it a94b3ab7eab4 ("[PATCH] mm: remove arch independent
NODES_SPAN_OTHER_NODES") just to be reintroduced by 7516795739bd
("[PATCH] Reintroduce NODES_SPAN_OTHER_NODES for powerpc") without any
reasoning what so ever. This doesn't make it really easy see whether
reasons for reintroduction are still there. Maybe there are some subtle
dependencies. I do not see any TBH but that might be burried deep in an
arch specific code.
> v3:
> * Revise the patch description
>
> V2:
> * Revise the patch description
>
> Hoan Tran (5):
> mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
> powerpc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> x86: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> sparc: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
> s390: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
>
> arch/powerpc/Kconfig | 9 ---------
> arch/s390/Kconfig | 8 --------
> arch/sparc/Kconfig | 9 ---------
> arch/x86/Kconfig | 9 ---------
> mm/page_alloc.c | 2 +-
> 5 files changed, 1 insertion(+), 36 deletions(-)
>
> --
> 1.8.3.1
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH] powerpc/mce: Add MCE notification chain
From: Ganesh Goudar @ 2020-03-30 7:12 UTC (permalink / raw)
To: mpe, linuxppc-dev
Cc: santosh, mahesh, npiggin, Ganesh Goudar, aneesh.kumar, arbab
From: Santosh S <santosh@fossix.org>
Introduce notification chain which lets know about uncorrected memory
errors(UE). This would help prospective users in pmem or nvdimm subsystem
to track bad blocks for better handling of persistent memory allocations.
Signed-off-by: Santosh S <santosh@fossix.org>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
arch/powerpc/include/asm/mce.h | 2 ++
arch/powerpc/kernel/mce.c | 15 +++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 6a6ddaabdb34..6e222a94a68a 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -218,6 +218,8 @@ extern void machine_check_queue_event(void);
extern void machine_check_print_event_info(struct machine_check_event *evt,
bool user_mode, bool in_guest);
unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
+int mce_register_notifier(struct notifier_block *nb);
+int mce_unregister_notifier(struct notifier_block *nb);
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 34c1001e9e8b..f50d7f56c02c 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -47,6 +47,20 @@ static struct irq_work mce_ue_event_irq_work = {
DECLARE_WORK(mce_ue_event_work, machine_process_ue_event);
+static BLOCKING_NOTIFIER_HEAD(mce_notifier_list);
+
+int mce_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_register_notifier);
+
+int mce_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&mce_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(mce_unregister_notifier);
+
static void mce_set_error_info(struct machine_check_event *mce,
struct mce_error_info *mce_err)
{
@@ -263,6 +277,7 @@ static void machine_process_ue_event(struct work_struct *work)
while (__this_cpu_read(mce_ue_count) > 0) {
index = __this_cpu_read(mce_ue_count) - 1;
evt = this_cpu_ptr(&mce_ue_event_queue[index]);
+ blocking_notifier_call_chain(&mce_notifier_list, 0, evt);
#ifdef CONFIG_MEMORY_FAILURE
/*
* This should probably queued elsewhere, but
--
2.17.2
^ permalink raw reply related
* [PATCH v2] powerpc: Make setjmp/longjmp signature standard
From: Clement Courbet @ 2020-03-30 6:43 UTC (permalink / raw)
Cc: Greg Kroah-Hartman, Nick Desaulniers, linux-kernel, stable,
clang-built-linux, Paul Mackerras, Clement Courbet,
Nathan Chancellor, linuxppc-dev, Thomas Gleixner, Allison Randal
In-Reply-To: <20200327100801.161671-1-courbet@google.com>
Declaring setjmp()/longjmp() as taking longs makes the signature
non-standard, and makes clang complain. In the past, this has been
worked around by adding -ffreestanding to the compile flags.
The implementation looks like it only ever propagates the value
(in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
with integer parameters.
This allows removing -ffreestanding from the compilation flags.
Context:
https://lore.kernel.org/patchwork/patch/1214060
https://lore.kernel.org/patchwork/patch/1216174
Signed-off-by: Clement Courbet <courbet@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
---
v2:
Use and array type as suggested by Segher Boessenkool
Cc: stable@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
---
arch/powerpc/include/asm/setjmp.h | 6 ++++--
arch/powerpc/kexec/Makefile | 3 ---
arch/powerpc/xmon/Makefile | 3 ---
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index e9f81bb3f83b..f798e80e4106 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -7,7 +7,9 @@
#define JMP_BUF_LEN 23
-extern long setjmp(long *) __attribute__((returns_twice));
-extern void longjmp(long *, long) __attribute__((noreturn));
+typedef long jmp_buf[JMP_BUF_LEN];
+
+extern int setjmp(jmp_buf env) __attribute__((returns_twice));
+extern void longjmp(jmp_buf env, int val) __attribute__((noreturn));
#endif /* _ASM_POWERPC_SETJMP_H */
diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile
index 378f6108a414..86380c69f5ce 100644
--- a/arch/powerpc/kexec/Makefile
+++ b/arch/powerpc/kexec/Makefile
@@ -3,9 +3,6 @@
# Makefile for the linux kernel.
#
-# Avoid clang warnings around longjmp/setjmp declarations
-CFLAGS_crash.o += -ffreestanding
-
obj-y += core.o crash.o core_$(BITS).o
obj-$(CONFIG_PPC32) += relocate_32.o
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index c3842dbeb1b7..6f9cccea54f3 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,9 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Avoid clang warnings around longjmp/setjmp declarations
-subdir-ccflags-y := -ffreestanding
-
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply related
* [PATCH v1] powerpc: Make setjmp/longjump signature standard
From: Clement Courbet @ 2020-03-30 6:42 UTC (permalink / raw)
Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Paul Mackerras,
Clement Courbet, Nathan Chancellor, linuxppc-dev, Thomas Gleixner,
Allison Randal
In-Reply-To: <20200327100801.161671-1-courbet@google.com>
Thanks you all for the comments. Everything addressed, plus the array vs
pointer suggestion from Segher Boessenkool on the other thread, which
is only cosmetic and does not change anything wrt behaviour.
^ permalink raw reply
* [PATCH RFC 1/1] powerpc/eeh: Synchronization for safety
From: Sam Bobroff @ 2020-03-30 6:39 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
There is currently little synchronization between EEH error detection
(eeh_dev_check_failure()), EEH error recovery
(eeh_handle_{normal,special}_event()) and the PCI subsystem (device
addition and removal), and so there are race conditions that lead to
crashes (often access to free'd memory or LIST_POISON).
However, a solution must consider:
- EEH error detection can occur in interrupt context, which prevents
the use of a mutex.
- EEH recovery may need to sleep, which prevents the use of a spinlock.
- EEH recovery uses PCI operations that may require the PCI
rescan/remove lock and/or device lock to be held
- PCI operations may hold the rescan/remove and/or device lock when
calling into EEH functions.
- Device driver callbacks may perform arbitrary PCI operations
during recovery, including device removal.
In this patch the existing mutex and spinlock are combined with the
EEH_PE_RECOVERING flag to provide some assurances that are then used
to reduce the race conditions.
The fields to be protected are the ones that provide the structure
of the trees of struct eeh_pe that are held for each PHB: the parent
pointer and child lists and the list of struct eeh_dev, as well as
the pe and pdev pointers within struct eeh_dev.
The existing way of using EEH_PE_RECOVERING is kept and slightly
extended: No struct eeh_pe will be removed while it has the flag set
on it. Additionally, when adding new PEs, they are marked
EEH_PE_RECOVERING if their parent PE is marked: this allows the
recovery thread to assume that all PEs underneath the one it's
processing will continue to exist during recovery.
Both the mutex and spinlock are held while any protected field is
changed or a PE is deleted, so holding either of them (elsewhere) will
keep them stable and safe to access. Additionally, if
EEH_PE_RECOVERING is set on a PE then the locks can be released and
re-acquired safely, as long as the protected fields aren't used while
no locks are held. This is used during recovery to release locks
for long sleeps (i.e. during eeh_wait_state() when we may sleep up to
5 minutes), or to maintain lock ordering.
The spinlock is used in error detection (which cannot use a mutex, see
above) and also where it's possible that the mutex is already held.
The mutex is used in areas that don't have that restriction, and where
blocking may be required. Care must be taken when ordering these locks
against the PCI rescan/remove lock and the device locks to avoid
deadlocking.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
Hello everyone,
Here's an attempt to bring some safety to the interactions between the various
moving parts involved in EEH recovery.
It's based on top of my recently posted set: "Release EEH device state
synchronously".
I've tried to explain it in the commit message and code comments, but I'd like
to add:
- I'm not aware of any outstanding problems with the set, but I've kept it RFC
for now becuase I'm looking for comments on the general strategy and
direction -- is this a good way to achieve some safety?
- I've only done manual testing so far. If it looks good, I'll try to
do something more thorough.
- Yes it's one big patch. I'll try to break it up if necessary.
- Good places to review carefully would be eeh_pe_report_pdev() and
eeh_reset_device().
- The mutex and spinlock need better names. Suggestions welcome.
- I'm not aiming to fix absolutely every case here, just most of them, and to
provide a decent foundation for fixing the remaining cases as they are
discovered.
And finally, a big thank you to lockdep :-)
Cheers,
Sam.
arch/powerpc/include/asm/eeh.h | 6 +-
arch/powerpc/kernel/eeh.c | 114 +++++--
arch/powerpc/kernel/eeh_driver.c | 300 ++++++++++++++-----
arch/powerpc/kernel/eeh_pe.c | 47 ++-
arch/powerpc/kernel/of_platform.c | 7 +-
arch/powerpc/kernel/pci-common.c | 4 +
arch/powerpc/kernel/pci-hotplug.c | 2 +
arch/powerpc/platforms/powernv/eeh-powernv.c | 13 +-
arch/powerpc/platforms/pseries/eeh_pseries.c | 5 +-
arch/powerpc/platforms/pseries/pci_dlpar.c | 5 +-
drivers/pci/hotplug/pnv_php.c | 5 +-
drivers/pci/hotplug/rpadlpar_core.c | 2 +
drivers/vfio/vfio_spapr_eeh.c | 10 +-
13 files changed, 404 insertions(+), 116 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 6f9b2a12540a..1d4c0b19a63c 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -273,11 +273,15 @@ static inline bool eeh_state_active(int state)
== (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
}
+void eeh_recovery_lock(void);
+void eeh_recovery_unlock(void);
+void eeh_recovery_must_be_locked(void);
+
typedef void (*eeh_edev_traverse_func)(struct eeh_dev *edev, void *flag);
typedef void *(*eeh_pe_traverse_func)(struct eeh_pe *pe, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
-int eeh_wait_state(struct eeh_pe *pe, int max_wait);
+int eeh_wait_state(struct eeh_pe *pe, int max_wait, bool unlock);
struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
struct eeh_pe *eeh_pe_next(struct eeh_pe *pe, struct eeh_pe *root);
struct eeh_pe *eeh_pe_get(struct pci_controller *phb,
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 12c248a16527..30acae0d10e5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -108,11 +108,26 @@ bool eeh_debugfs_no_recover;
/* Platform dependent EEH operations */
struct eeh_ops *eeh_ops = NULL;
-/* Lock to avoid races due to multiple reports of an error */
+/*
+ * confirm_error_lock and eeh_dev_mutex are used together to provide
+ * safety during EEH operations.
+ *
+ * Generally, the spinlock is used in error detection where it's not possible
+ * to use a mutex or where there is potential to deadlock with the mutex, and
+ * the mutex is used during recovery and other PCI related operations. One must
+ * be held when reading and both must be held when making changes to the
+ * protected fields: eeh_pe.parent, child_list, child, edevs and eeh_dev.pe,
+ * .pdev.
+ *
+ * Lock ordering:
+ * - the PCI rescan/remove mutex (see pci_lock_rescan_remove())
+ * - the struct device lock (see device_lock())
+ * - the eeh_dev_mutex mutex (see eeh_recovery_lock())
+ * - the confirm_error_lock spinlock (see eeh_serialize_lock())
+ * - the eeh_eventlist_lock spinlock
+ */
DEFINE_RAW_SPINLOCK(confirm_error_lock);
EXPORT_SYMBOL_GPL(confirm_error_lock);
-
-/* Lock to protect passed flags */
static DEFINE_MUTEX(eeh_dev_mutex);
/* Buffer for reporting pci register dumps. Its here in BSS, and
@@ -160,6 +175,23 @@ void eeh_show_enabled(void)
pr_info("EEH: No capable adapters found: recovery disabled.\n");
}
+void eeh_recovery_lock(void)
+{
+ mutex_lock(&eeh_dev_mutex);
+}
+EXPORT_SYMBOL_GPL(eeh_recovery_lock);
+
+void eeh_recovery_unlock(void)
+{
+ mutex_unlock(&eeh_dev_mutex);
+}
+EXPORT_SYMBOL_GPL(eeh_recovery_unlock);
+
+void eeh_recovery_must_be_locked(void)
+{
+ WARN_ON_ONCE(!mutex_is_locked(&eeh_dev_mutex));
+}
+EXPORT_SYMBOL_GPL(eeh_recovery_must_be_locked);
/*
* This routine captures assorted PCI configuration space data
* for the indicated PCI device, and puts them into a buffer
@@ -383,11 +415,12 @@ static inline unsigned long eeh_token_to_phys(unsigned long token)
* On PowerNV platform, we might already have fenced PHB there.
* For that case, it's meaningless to recover frozen PE. Intead,
* We have to handle fenced PHB firstly.
+ *
+ * eeh_serialize_lock must be held when calling this function.
*/
static int eeh_phb_check_failure(struct eeh_pe *pe)
{
struct eeh_pe *phb_pe;
- unsigned long flags;
int ret;
if (!eeh_has_flag(EEH_PROBE_MODE_DEV))
@@ -402,7 +435,6 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
}
/* If the PHB has been in problematic state */
- eeh_serialize_lock(&flags);
if (phb_pe->state & EEH_PE_ISOLATED) {
ret = 0;
goto out;
@@ -418,14 +450,12 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
/* Isolate the PHB and send event */
eeh_pe_mark_isolated(phb_pe);
- eeh_serialize_unlock(flags);
pr_debug("EEH: PHB#%x failure detected, location: %s\n",
phb_pe->phb->global_number, eeh_pe_loc_get(phb_pe));
eeh_send_failure_event(phb_pe);
return 1;
out:
- eeh_serialize_unlock(flags);
return ret;
}
@@ -445,12 +475,11 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
*/
int eeh_dev_check_failure(struct eeh_dev *edev)
{
- int ret;
unsigned long flags;
struct device_node *dn;
struct pci_dev *dev;
struct eeh_pe *pe, *parent_pe;
- int rc = 0;
+ int rc;
const char *location = NULL;
eeh_stats.total_mmio_ffs++;
@@ -462,6 +491,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
eeh_stats.no_dn++;
return 0;
}
+ eeh_serialize_lock(&flags);
dev = eeh_dev_to_pci_dev(edev);
pe = eeh_dev_to_pe(edev);
@@ -469,29 +499,33 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
if (!pe) {
eeh_stats.ignored_check++;
eeh_edev_dbg(edev, "Ignored check\n");
- return 0;
+ rc = 0;
+ goto dn_unlock;
}
if (!pe->addr && !pe->config_addr) {
eeh_stats.no_cfg_addr++;
- return 0;
+ rc = 0;
+ goto dn_unlock;
}
/*
* On PowerNV platform, we might already have fenced PHB
* there and we need take care of that firstly.
*/
- ret = eeh_phb_check_failure(pe);
- if (ret > 0)
- return ret;
+ rc = eeh_phb_check_failure(pe);
+ if (rc > 0)
+ goto dn_unlock;
/*
* If the PE isn't owned by us, we shouldn't check the
* state. Instead, let the owner handle it if the PE has
* been frozen.
*/
- if (eeh_pe_passed(pe))
- return 0;
+ if (eeh_pe_passed(pe)) {
+ rc = 0;
+ goto dn_unlock;
+ }
/* If we already have a pending isolation event for this
* slot, we know it's bad already, we don't need to check.
@@ -499,8 +533,6 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* in one slot might report errors simultaneously, and we
* only want one error recovery routine running.
*/
- eeh_serialize_lock(&flags);
- rc = 1;
if (pe->state & EEH_PE_ISOLATED) {
pe->check_count++;
if (pe->check_count == EEH_MAX_FAILS) {
@@ -516,6 +548,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
eeh_driver_name(dev));
dump_stack();
}
+ rc = 1;
goto dn_unlock;
}
@@ -526,7 +559,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* function zero of a multi-function device.
* In any case they must share a common PHB.
*/
- ret = eeh_ops->get_state(pe, NULL);
+ rc = eeh_ops->get_state(pe, NULL);
/* Note that config-io to empty slots may fail;
* they are empty when they don't have children.
@@ -534,8 +567,8 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* PE's state, EEH not support and Permanently unavailable
* state, PE is in good state.
*/
- if ((ret < 0) ||
- (ret == EEH_STATE_NOT_SUPPORT) || eeh_state_active(ret)) {
+ if ((rc < 0) ||
+ (rc == EEH_STATE_NOT_SUPPORT) || eeh_state_active(rc)) {
eeh_stats.false_positives++;
pe->false_positives++;
rc = 0;
@@ -554,8 +587,8 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
break;
/* Frozen parent PE ? */
- ret = eeh_ops->get_state(parent_pe, NULL);
- if (ret > 0 && !eeh_state_active(ret)) {
+ rc = eeh_ops->get_state(parent_pe, NULL);
+ if (rc > 0 && !eeh_state_active(rc)) {
pe = parent_pe;
pr_err("EEH: Failure of PHB#%x-PE#%x will be handled at parent PHB#%x-PE#%x.\n",
pe->phb->global_number, pe->addr,
@@ -573,7 +606,6 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* bridges.
*/
eeh_pe_mark_isolated(pe);
- eeh_serialize_unlock(flags);
/* Most EEH events are due to device driver bugs. Having
* a stack trace will help the device-driver authors figure
@@ -582,6 +614,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
pr_debug("EEH: %s: Frozen PHB#%x-PE#%x detected\n",
__func__, pe->phb->global_number, pe->addr);
eeh_send_failure_event(pe);
+ eeh_serialize_unlock(flags);
return 1;
@@ -685,7 +718,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
/* Check if the request is finished successfully */
if (active_flag) {
- rc = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
+ rc = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC, false);
if (rc < 0)
return rc;
@@ -942,7 +975,7 @@ int eeh_pe_reset_full(struct eeh_pe *pe, bool include_passed)
pe->phb->global_number, pe->addr, i + 1);
/* Wait until the PE is in a functioning state */
- state = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
+ state = eeh_wait_state(pe, PCI_BUS_RESET_WAIT_MSEC, false);
if (state < 0) {
pr_warn("EEH: Unrecoverable slot failure on PHB#%x-PE#%x",
pe->phb->global_number, pe->addr);
@@ -1113,7 +1146,9 @@ static int eeh_device_notifier(struct notifier_block *nb,
switch (action) {
case BUS_NOTIFY_DEL_DEVICE:
+ eeh_recovery_lock();
eeh_remove_device(to_pci_dev(dev));
+ eeh_recovery_unlock();
break;
default:
break;
@@ -1195,6 +1230,7 @@ void eeh_add_device_late(struct pci_dev *dev)
{
struct pci_dn *pdn;
struct eeh_dev *edev;
+ unsigned long flags;
if (!dev)
return;
@@ -1211,8 +1247,12 @@ void eeh_add_device_late(struct pci_dev *dev)
if (eeh_has_flag(EEH_PROBE_MODE_DEV))
eeh_ops->probe(pdn, NULL);
+ /* Both locks are required to make changes */
+ eeh_recovery_must_be_locked();
+ eeh_serialize_lock(&flags);
edev->pdev = dev;
dev->dev.archdata.edev = edev;
+ eeh_serialize_unlock(flags);
eeh_addr_cache_insert_dev(dev);
}
@@ -1278,6 +1318,7 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
void eeh_remove_device(struct pci_dev *dev)
{
struct eeh_dev *edev;
+ unsigned long flags;
if (!dev || !eeh_enabled())
return;
@@ -1297,7 +1338,11 @@ void eeh_remove_device(struct pci_dev *dev)
* a bit later. So we keep it for BAR restore and remove it
* from the parent PE during the BAR resotre.
*/
+ /* Both locks are required to make changes */
+ eeh_recovery_must_be_locked();
+ eeh_serialize_lock(&flags);
edev->pdev = NULL;
+ eeh_serialize_unlock(flags);
/*
* eeh_sysfs_remove_device() uses pci_dev_to_eeh_dev() so we need to
@@ -1323,7 +1368,11 @@ void eeh_remove_device(struct pci_dev *dev)
* for the VF EEH device.
*/
edev->in_error = false;
+ /* Both locks are required to make changes */
+ eeh_recovery_must_be_locked();
+ eeh_serialize_lock(&flags);
dev->dev.archdata.edev = NULL;
+ eeh_serialize_unlock(flags);
if (!(edev->pe->state & EEH_PE_KEEP))
eeh_rmv_from_parent_pe(edev);
else
@@ -1419,7 +1468,7 @@ int eeh_dev_open(struct pci_dev *pdev)
struct eeh_dev *edev;
int ret = -ENODEV;
- mutex_lock(&eeh_dev_mutex);
+ eeh_recovery_lock();
/* No PCI device ? */
if (!pdev)
@@ -1442,11 +1491,11 @@ int eeh_dev_open(struct pci_dev *pdev)
/* Increase PE's pass through count */
atomic_inc(&edev->pe->pass_dev_cnt);
- mutex_unlock(&eeh_dev_mutex);
+ eeh_recovery_unlock();
return 0;
out:
- mutex_unlock(&eeh_dev_mutex);
+ eeh_recovery_unlock();
return ret;
}
EXPORT_SYMBOL_GPL(eeh_dev_open);
@@ -1463,7 +1512,7 @@ void eeh_dev_release(struct pci_dev *pdev)
{
struct eeh_dev *edev;
- mutex_lock(&eeh_dev_mutex);
+ eeh_recovery_lock();
/* No PCI device ? */
if (!pdev)
@@ -1478,7 +1527,7 @@ void eeh_dev_release(struct pci_dev *pdev)
WARN_ON(atomic_dec_if_positive(&edev->pe->pass_dev_cnt) < 0);
eeh_pe_change_owner(edev->pe);
out:
- mutex_unlock(&eeh_dev_mutex);
+ eeh_recovery_unlock();
}
EXPORT_SYMBOL(eeh_dev_release);
@@ -1834,6 +1883,7 @@ static ssize_t eeh_force_recover_write(struct file *filp,
struct eeh_pe *pe;
char buf[20];
int ret;
+ unsigned long flags;
ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
if (!ret)
@@ -1870,7 +1920,9 @@ static ssize_t eeh_force_recover_write(struct file *filp,
* from an odd state (e.g. PE removed, or recovery of a
* non-isolated PE)
*/
+ eeh_serialize_lock(&flags);
__eeh_send_failure_event(pe);
+ eeh_serialize_unlock(flags);
return ret < 0 ? ret : count;
}
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7b048cee767c..f21e0910f80a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -248,38 +248,50 @@ static void eeh_set_irq_state(struct eeh_pe *root, bool enable)
}
}
-typedef enum pci_ers_result (*eeh_report_fn)(struct eeh_dev *,
- struct pci_dev *,
+typedef enum pci_ers_result (*eeh_report_fn)(struct pci_dev *,
struct pci_driver *);
-static void eeh_pe_report_edev(struct eeh_dev *edev, eeh_report_fn fn,
+static void eeh_pe_report_pdev(struct pci_dev *pdev, eeh_report_fn fn,
enum pci_ers_result *result)
{
- struct pci_dev *pdev;
+ struct eeh_dev *edev;
struct pci_driver *driver;
+ bool actionable, late, removed, passed;
enum pci_ers_result new_result;
- pci_lock_rescan_remove();
- pdev = edev->pdev;
- if (pdev)
- get_device(&pdev->dev);
- pci_unlock_rescan_remove();
- if (!pdev) {
- eeh_edev_info(edev, "no device");
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev) {
+ pci_info(pdev, "no EEH state for device");
return;
}
- device_lock(&pdev->dev);
- if (eeh_edev_actionable(edev)) {
+ /* Cache some useful values before releasing the lock: */
+ actionable = eeh_edev_actionable(edev);
+ late = edev->mode & EEH_DEV_NO_HANDLER;
+ removed = eeh_dev_removed(edev);
+ passed = eeh_pe_passed(edev->pe);
+ if (actionable) {
+ /*
+ * Driver callbacks may end up calling back into EEH functions
+ * (for example by removing a PCI device) which will deadlock
+ * unless the EEH locks are released first. Note that it may be
+ * re-acquired by the report functions, if necessary.
+ */
+ eeh_recovery_unlock();
+ device_lock(&pdev->dev);
driver = eeh_pcid_get(pdev);
if (!driver)
- eeh_edev_info(edev, "no driver");
+ pci_info(pdev, "no driver");
else if (!driver->err_handler)
- eeh_edev_info(edev, "driver not EEH aware");
- else if (edev->mode & EEH_DEV_NO_HANDLER)
- eeh_edev_info(edev, "driver bound too late");
+ pci_info(pdev, "driver not EEH aware");
+ else if (late)
+ pci_info(pdev, "driver bound too late");
else {
- new_result = fn(edev, pdev, driver);
- eeh_edev_info(edev, "%s driver reports: '%s'",
+ new_result = fn(pdev, driver);
+ /*
+ * It's not safe to use edev here, because the locks
+ * have been released and devices could have changed.
+ */
+ pci_info(pdev, "%s driver reports: '%s'",
driver->name,
pci_ers_result_name(new_result));
if (result)
@@ -288,25 +300,72 @@ static void eeh_pe_report_edev(struct eeh_dev *edev, eeh_report_fn fn,
}
if (driver)
eeh_pcid_put(pdev);
+ device_unlock(&pdev->dev);
+ eeh_recovery_lock();
} else {
- eeh_edev_info(edev, "not actionable (%d,%d,%d)", !!pdev,
- !eeh_dev_removed(edev), !eeh_pe_passed(edev->pe));
+ pci_info(pdev, "not actionable (%d,%d,%d)", !!pdev,
+ !removed, !passed);
}
- device_unlock(&pdev->dev);
- if (edev->pdev != pdev)
- eeh_edev_warn(edev, "Device changed during processing!\n");
- put_device(&pdev->dev);
}
-static void eeh_pe_report(const char *name, struct eeh_pe *root,
- eeh_report_fn fn, enum pci_ers_result *result)
+struct pci_dev **pdev_cache_list_create(struct eeh_pe *root)
{
struct eeh_pe *pe;
struct eeh_dev *edev, *tmp;
+ struct pci_dev **pdevs;
+ int i, n;
+
+ n = 0;
+ eeh_for_each_pe(root, pe) eeh_pe_for_each_dev(pe, edev, tmp) {
+ if (edev->pdev)
+ n++;
+ }
+ pdevs = kmalloc(sizeof(*pdevs) * (n + 1), GFP_KERNEL);
+ if (WARN_ON_ONCE(!pdevs))
+ return NULL;
+ i = 0;
+ eeh_for_each_pe(root, pe) eeh_pe_for_each_dev(pe, edev, tmp) {
+ if (i < n) {
+ get_device(&edev->pdev->dev);
+ pdevs[i++] = edev->pdev;
+ }
+ }
+ if (WARN_ON_ONCE(i < n))
+ n = i;
+ pdevs[n] = NULL; /* terminator */
+ return pdevs;
+}
+
+static void pdev_cache_list_destroy(struct pci_dev **pdevs)
+{
+ struct pci_dev **pdevp;
+
+ for (pdevp = pdevs; pdevp && *pdevp; pdevp++)
+ put_device(&(*pdevp)->dev);
+ kfree(pdevs);
+}
+
+static void eeh_pe_report(const char *name, struct eeh_pe *root,
+ eeh_report_fn fn, enum pci_ers_result *result)
+{
+ struct pci_dev **pdevs, **pdevp;
pr_info("EEH: Beginning: '%s'\n", name);
- eeh_for_each_pe(root, pe) eeh_pe_for_each_dev(pe, edev, tmp)
- eeh_pe_report_edev(edev, fn, result);
+ /*
+ * It would be convenient to continue to hold the recovery lock here
+ * but driver callbacks can take a very long time or never return at
+ * all.
+ */
+ pdevs = pdev_cache_list_create(root);
+ for (pdevp = pdevs; pdevp && *pdevp; pdevp++) {
+ /*
+ * NOTE! eeh_recovery_lock() is released briefly
+ * in eeh_pe_report_pdev()
+ */
+ eeh_pe_report_pdev(*pdevp, fn, result);
+ }
+ pdev_cache_list_destroy(pdevs);
+
if (result)
pr_info("EEH: Finished:'%s' with aggregate recovery state:'%s'\n",
name, pci_ers_result_name(*result));
@@ -316,50 +375,53 @@ static void eeh_pe_report(const char *name, struct eeh_pe *root,
/**
* eeh_report_error - Report pci error to each device driver
- * @edev: eeh device
+ * @pdev: eeh device
* @driver: device's PCI driver
*
* Report an EEH error to each device driver.
*/
-static enum pci_ers_result eeh_report_error(struct eeh_dev *edev,
- struct pci_dev *pdev,
+static enum pci_ers_result eeh_report_error(struct pci_dev *pdev,
struct pci_driver *driver)
{
enum pci_ers_result rc;
+ struct eeh_dev *edev;
+ unsigned long flags;
if (!driver->err_handler->error_detected)
return PCI_ERS_RESULT_NONE;
- eeh_edev_info(edev, "Invoking %s->error_detected(IO frozen)",
- driver->name);
+ pci_info(pdev, "Invoking %s->error_detected(IO frozen)", driver->name);
rc = driver->err_handler->error_detected(pdev, pci_channel_io_frozen);
- edev->in_error = true;
+ eeh_serialize_lock(&flags);
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (edev)
+ edev->in_error = true;
+ eeh_serialize_unlock(flags);
pci_uevent_ers(pdev, PCI_ERS_RESULT_NONE);
return rc;
}
/**
* eeh_report_mmio_enabled - Tell drivers that MMIO has been enabled
- * @edev: eeh device
+ * @pdev: eeh device
* @driver: device's PCI driver
*
* Tells each device driver that IO ports, MMIO and config space I/O
* are now enabled.
*/
-static enum pci_ers_result eeh_report_mmio_enabled(struct eeh_dev *edev,
- struct pci_dev *pdev,
+static enum pci_ers_result eeh_report_mmio_enabled(struct pci_dev *pdev,
struct pci_driver *driver)
{
if (!driver->err_handler->mmio_enabled)
return PCI_ERS_RESULT_NONE;
- eeh_edev_info(edev, "Invoking %s->mmio_enabled()", driver->name);
+ pci_info(pdev, "Invoking %s->mmio_enabled()", driver->name);
return driver->err_handler->mmio_enabled(pdev);
}
/**
* eeh_report_reset - Tell device that slot has been reset
- * @edev: eeh device
+ * @pdev: eeh device
* @driver: device's PCI driver
*
* This routine must be called while EEH tries to reset particular
@@ -367,13 +429,20 @@ static enum pci_ers_result eeh_report_mmio_enabled(struct eeh_dev *edev,
* some actions, usually to save data the driver needs so that the
* driver can work again while the device is recovered.
*/
-static enum pci_ers_result eeh_report_reset(struct eeh_dev *edev,
- struct pci_dev *pdev,
+static enum pci_ers_result eeh_report_reset(struct pci_dev *pdev,
struct pci_driver *driver)
{
- if (!driver->err_handler->slot_reset || !edev->in_error)
+ struct eeh_dev *edev;
+ unsigned long flags;
+
+ eeh_serialize_lock(&flags);
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!driver->err_handler->slot_reset || !edev->in_error) {
+ eeh_serialize_unlock(flags);
return PCI_ERS_RESULT_NONE;
- eeh_edev_info(edev, "Invoking %s->slot_reset()", driver->name);
+ }
+ eeh_serialize_unlock(flags);
+ pci_info(pdev, "Invoking %s->slot_reset()", driver->name);
return driver->err_handler->slot_reset(pdev);
}
@@ -406,41 +475,49 @@ static void eeh_dev_restore_state(struct eeh_dev *edev, void *userdata)
/**
* eeh_report_resume - Tell device to resume normal operations
- * @edev: eeh device
+ * @pdev: eeh device
* @driver: device's PCI driver
*
* This routine must be called to notify the device driver that it
* could resume so that the device driver can do some initialization
* to make the recovered device work again.
*/
-static enum pci_ers_result eeh_report_resume(struct eeh_dev *edev,
- struct pci_dev *pdev,
+static enum pci_ers_result eeh_report_resume(struct pci_dev *pdev,
struct pci_driver *driver)
{
- if (!driver->err_handler->resume || !edev->in_error)
+ struct eeh_dev *edev;
+ unsigned long flags;
+
+ eeh_serialize_lock(&flags);
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!driver->err_handler->resume || !edev->in_error) {
+ eeh_serialize_unlock(flags);
return PCI_ERS_RESULT_NONE;
+ }
+ eeh_serialize_unlock(flags);
- eeh_edev_info(edev, "Invoking %s->resume()", driver->name);
+ pci_info(pdev, "Invoking %s->resume()", driver->name);
driver->err_handler->resume(pdev);
- pci_uevent_ers(edev->pdev, PCI_ERS_RESULT_RECOVERED);
+ pci_uevent_ers(pdev, PCI_ERS_RESULT_RECOVERED);
#ifdef CONFIG_PCI_IOV
+ eeh_serialize_lock(&flags);
if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
+ eeh_serialize_unlock(flags);
#endif
return PCI_ERS_RESULT_NONE;
}
/**
* eeh_report_failure - Tell device driver that device is dead.
- * @edev: eeh device
+ * @pdev: eeh device
* @driver: device's PCI driver
*
* This informs the device driver that the device is permanently
* dead, and that no further recovery attempts will be made on it.
*/
-static enum pci_ers_result eeh_report_failure(struct eeh_dev *edev,
- struct pci_dev *pdev,
+static enum pci_ers_result eeh_report_failure(struct pci_dev *pdev,
struct pci_driver *driver)
{
enum pci_ers_result rc;
@@ -448,8 +525,8 @@ static enum pci_ers_result eeh_report_failure(struct eeh_dev *edev,
if (!driver->err_handler->error_detected)
return PCI_ERS_RESULT_NONE;
- eeh_edev_info(edev, "Invoking %s->error_detected(permanent failure)",
- driver->name);
+ pci_info(pdev, "Invoking %s->error_detected(permanent failure)",
+ driver->name);
rc = driver->err_handler->error_detected(pdev,
pci_channel_io_perm_failure);
@@ -477,17 +554,44 @@ static void *eeh_add_virt_device(struct eeh_dev *edev)
}
#ifdef CONFIG_PCI_IOV
- pci_iov_add_virtfn(edev->physfn, eeh_dev_to_pdn(edev)->vf_index);
+ {
+ struct pci_dev *physfn = edev->physfn;
+ int vf_index = eeh_dev_to_pdn(edev)->vf_index;
+
+ get_device(&physfn->dev);
+ eeh_recovery_unlock();
+ /*
+ * This PCI operation will call back into EEH code where the
+ * recovery lock will be acquired, so it must be released here,
+ * first:
+ */
+ pci_iov_add_virtfn(physfn, vf_index);
+ put_device(&physfn->dev);
+ eeh_recovery_lock();
+ }
#endif
return NULL;
}
-static void eeh_rmv_device(struct eeh_dev *edev, void *userdata)
+/*
+ * It's important that this function take a pdev as a parameter, because they
+ * are protected by a reference count. An edev could be lost when the recovery
+ * lock is dropped (which it must be in order to take the PCI rescan/remove
+ * lock without risking a deadlock).
+ */
+static void eeh_rmv_device(struct pci_dev *pdev, void *userdata)
{
+ struct eeh_dev *edev;
struct pci_driver *driver;
- struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
struct eeh_rmv_data *rmv_data = (struct eeh_rmv_data *)userdata;
+ unsigned long flags;
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev) {
+ pci_warn(pdev, "EEH: Device removed during processing (#%d)\n",
+ __LINE__);
+ return;
+ }
/*
* Actually, we should remove the PCI bridges as well.
* However, that's lots of complexity to do that,
@@ -496,25 +600,24 @@ static void eeh_rmv_device(struct eeh_dev *edev, void *userdata)
* simplicity here.
*/
if (!eeh_edev_actionable(edev) ||
- (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE))
+ (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE))
return;
if (rmv_data) {
- driver = eeh_pcid_get(dev);
+ driver = eeh_pcid_get(pdev);
if (driver) {
if (driver->err_handler &&
driver->err_handler->error_detected &&
driver->err_handler->slot_reset) {
- eeh_pcid_put(dev);
+ eeh_pcid_put(pdev);
return;
}
- eeh_pcid_put(dev);
+ eeh_pcid_put(pdev);
}
}
/* Remove it from PCI subsystem */
- pr_info("EEH: Removing %s without EEH sensitive driver\n",
- pci_name(dev));
+ pci_info(pdev, "EEH: Removing device without EEH sensitive driver\n");
edev->mode |= EEH_DEV_DISCONNECTED;
if (rmv_data)
rmv_data->removed_dev_count++;
@@ -523,15 +626,32 @@ static void eeh_rmv_device(struct eeh_dev *edev, void *userdata)
#ifdef CONFIG_PCI_IOV
struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+ eeh_recovery_unlock();
+ /*
+ * This PCI operation will call back into EEH code where
+ * the recovery lock will be acquired, so it must be released
+ * here, first:
+ */
pci_iov_remove_virtfn(edev->physfn, pdn->vf_index);
+ eeh_recovery_lock();
+
+ /* Both locks are required to make changes */
+ eeh_serialize_lock(&flags);
edev->pdev = NULL;
+ eeh_serialize_unlock(flags);
#endif
if (rmv_data)
list_add(&edev->rmv_entry, &rmv_data->removed_vf_list);
} else {
+ /*
+ * Lock ordering requires that the recovery lock be released
+ * before acquiring the PCI rescan/remove lock.
+ */
+ eeh_recovery_unlock();
pci_lock_rescan_remove();
- pci_stop_and_remove_bus_device(dev);
+ pci_stop_and_remove_bus_device(pdev);
pci_unlock_rescan_remove();
+ eeh_recovery_lock();
}
}
@@ -629,6 +749,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
{
time64_t tstamp;
int cnt, rc;
+ struct pci_dev **pdevs, **pdevp;
struct eeh_dev *edev;
struct eeh_pe *tmp_pe;
bool any_passed = false;
@@ -648,11 +769,22 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
*/
eeh_pe_state_mark(pe, EEH_PE_KEEP);
if (any_passed || driver_eeh_aware || (pe->type & EEH_PE_VF)) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
+ /*
+ * eeh_rmv_device() may need to release the recovery lock to
+ * remove a PCI device so we can't rely on the PE lists staying
+ * valid:
+ */
+ pdevs = pdev_cache_list_create(pe);
+ /* eeh_rmv_device() may re-acquire the recovery lock */
+ for (pdevp = pdevs; pdevp && *pdevp; pdevp++)
+ eeh_rmv_device(*pdevp, rmv_data);
+ pdev_cache_list_destroy(pdevs);
} else {
+ eeh_recovery_unlock();
pci_lock_rescan_remove();
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
+ eeh_recovery_lock();
}
/*
@@ -668,7 +800,13 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
if (rc)
return rc;
+ /*
+ * The PCI rescan/remove lock must always be taken first, but we need
+ * both here:
+ */
+ eeh_recovery_unlock();
pci_lock_rescan_remove();
+ eeh_recovery_lock();
/* Restore PE */
eeh_ops->configure_bridge(pe);
@@ -676,10 +814,9 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
/* Clear frozen state */
rc = eeh_clear_pe_frozen_state(pe, false);
- if (rc) {
- pci_unlock_rescan_remove();
+ pci_unlock_rescan_remove();
+ if (rc)
return rc;
- }
/* Give the system 5 seconds to finish running the user-space
* hotplug shutdown scripts, e.g. ifdown for ethernet. Yes,
@@ -690,7 +827,9 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
if (!driver_eeh_aware || rmv_data->removed_dev_count) {
pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
(driver_eeh_aware ? "partial" : "complete"));
+ eeh_recovery_unlock();
ssleep(5);
+ eeh_recovery_lock();
/*
* The EEH device is still connected with its parent
@@ -704,7 +843,16 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
} else {
if (!driver_eeh_aware)
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
+ /*
+ * Lock ordering requires that the recovery lock be
+ * released before acquiring the PCI rescan/remove
+ * lock.
+ */
+ eeh_recovery_unlock();
+ pci_lock_rescan_remove();
pci_hp_add_devices(bus);
+ pci_unlock_rescan_remove();
+ eeh_recovery_lock();
}
}
eeh_pe_state_clear(pe, EEH_PE_KEEP, true);
@@ -712,7 +860,6 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
pe->tstamp = tstamp;
pe->freeze_count = cnt;
- pci_unlock_rescan_remove();
return 0;
}
@@ -840,16 +987,19 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
struct pci_bus *bus;
struct eeh_dev *edev, *tmp;
struct eeh_pe *tmp_pe;
+ struct pci_dev **pdevs, **pdevp;
int rc = 0;
enum pci_ers_result result = PCI_ERS_RESULT_NONE;
struct eeh_rmv_data rmv_data =
{LIST_HEAD_INIT(rmv_data.removed_vf_list), 0};
int devices = 0;
+ eeh_recovery_lock();
bus = eeh_pe_bus_get(pe);
if (!bus) {
pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
__func__, pe->phb->global_number, pe->addr);
+ eeh_recovery_unlock();
return;
}
@@ -948,7 +1098,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* sometimes over 300 seconds for certain systems.
*/
if (result != PCI_ERS_RESULT_DISCONNECT) {
- rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
+ rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000, true);
if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
pr_warn("EEH: Permanent failure\n");
result = PCI_ERS_RESULT_DISCONNECT;
@@ -1083,12 +1233,16 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* the their PCI config any more.
*/
if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+ pdevs = pdev_cache_list_create(pe);
+ for (pdevp = pdevs; pdevp && *pdevp; pdevp++)
+ eeh_rmv_device(*pdevp, NULL);
+ pdev_cache_list_destroy(pdevs);
eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
} else {
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ eeh_recovery_unlock();
pci_lock_rescan_remove();
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
@@ -1110,6 +1264,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_clear_slot_attention(edev->pdev);
eeh_pe_state_clear(pe, EEH_PE_RECOVERING, true);
+ eeh_recovery_unlock();
}
/**
@@ -1118,6 +1273,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* Called when an EEH event is detected but can't be narrowed down to a
* specific PE. Iterates through possible failures and handles them as
* necessary.
+ * XXX TODO Needs to be checked sync work
*/
void eeh_handle_special_event(void)
{
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 177852e39a25..999b91405581 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -102,8 +102,15 @@ int eeh_phb_pe_create(struct pci_controller *phb)
*
* Wait for the state of associated PE. It might take some time
* to retrieve the PE's state.
+ *
+ * Note that if this function sleeps and unlock is true, the EEH recovery lock
+ * will be released and re-acquired.
+ * It is only safe to do this when the PE has the recovering flag set on it.
+ * This is important because the sleep may be
+ * very long (300 seconds) and device removal will be blocked by the recovery
+ * mutex. See eeh_device_notifier().
*/
-int eeh_wait_state(struct eeh_pe *pe, int max_wait)
+int eeh_wait_state(struct eeh_pe *pe, int max_wait, bool unlock)
{
int ret;
int mwait;
@@ -119,6 +126,8 @@ int eeh_wait_state(struct eeh_pe *pe, int max_wait)
#define EEH_STATE_MIN_WAIT_TIME (1000)
#define EEH_STATE_MAX_WAIT_TIME (300 * 1000)
+ WARN_ON_ONCE(unlock && !(pe->state & EEH_PE_RECOVERING));
+
while (1) {
ret = eeh_ops->get_state(pe, &mwait);
@@ -141,7 +150,11 @@ int eeh_wait_state(struct eeh_pe *pe, int max_wait)
mwait = EEH_STATE_MAX_WAIT_TIME;
}
+ if (unlock)
+ eeh_recovery_unlock();
msleep(min(mwait, max_wait));
+ if (unlock)
+ eeh_recovery_lock();
max_wait -= mwait;
}
}
@@ -363,15 +376,24 @@ static struct eeh_pe *eeh_pe_get_parent(struct eeh_dev *edev)
* exists, the PE type will be changed to EEH_PE_BUS. Otherwise,
* we have to create new PE to hold the EEH device and the new
* PE will be linked to its parent PE as well.
+ *
+ * The pci rescan/remove lock must be held when calling this function.
*/
int eeh_add_to_parent_pe(struct eeh_dev *edev)
{
+ unsigned long flags;
struct eeh_pe *pe, *parent;
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
- int config_addr = (pdn->busno << 8) | (pdn->devfn);
+ struct pci_dn *pdn;
+ int config_addr;
+ /* Both locks are required to make changes */
+ eeh_recovery_must_be_locked();
+ eeh_serialize_lock(&flags);
+ pdn = eeh_dev_to_pdn(edev);
+ config_addr = (pdn->busno << 8) | (pdn->devfn);
/* Check if the PE number is valid */
if (!eeh_has_flag(EEH_VALID_PE_ZERO) && !edev->pe_config_addr) {
+ eeh_serialize_unlock(flags);
eeh_edev_err(edev, "PE#0 is invalid for this PHB!\n");
return -EINVAL;
}
@@ -411,8 +433,14 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
list_add_tail(&edev->entry, &pe->edevs);
eeh_edev_dbg(edev, "Added to bus PE\n");
}
+ eeh_serialize_unlock(flags);
return 0;
}
+ /*
+ * Release the spinlock so that memory can be allocated without
+ * using GFP_ATOMIC
+ */
+ eeh_serialize_unlock(flags);
/* Create a new EEH PE */
if (edev->physfn)
@@ -432,6 +460,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* to PHB directly. Otherwise, we have to associate the
* PE with its parent.
*/
+ eeh_serialize_lock(&flags);
parent = eeh_pe_get_parent(edev);
if (!parent) {
parent = eeh_phb_pe_get(pdn->phb);
@@ -445,6 +474,9 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
}
pe->parent = parent;
+ if (parent->state & EEH_PE_RECOVERING)
+ pe->state |= EEH_PE_RECOVERING;
+
/*
* Put the newly created PE into the child list and
* link the EEH device accordingly.
@@ -452,6 +484,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
list_add_tail(&pe->child, &parent->child_list);
list_add_tail(&edev->entry, &pe->edevs);
edev->pe = pe;
+ eeh_serialize_unlock(flags);
eeh_edev_dbg(edev, "Added to device PE (parent: PE#%x)\n",
pe->parent->addr);
@@ -466,16 +499,23 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
* Also, the PCI devices or buses could be removed from the system
* during EEH recovery. So we have to call the function remove the
* corresponding PE accordingly if necessary.
+ *
+ * The pci rescan/remove lock must be held when calling this function.
*/
int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
{
+ unsigned long flags;
struct eeh_pe *pe, *parent, *child;
bool keep, recover;
int cnt;
+ /* Both locks are required to make changes */
+ eeh_recovery_must_be_locked();
+ eeh_serialize_lock(&flags);
pe = eeh_dev_to_pe(edev);
if (!pe) {
eeh_edev_dbg(edev, "No PE found for device.\n");
+ eeh_serialize_unlock(flags);
return -EEXIST;
}
@@ -542,6 +582,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
pe = parent;
}
+ eeh_serialize_unlock(flags);
return 0;
}
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index 427fc22f72b6..8de811804008 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -66,8 +66,11 @@ static int of_pci_phb_probe(struct platform_device *dev)
eeh_dev_phb_init_dynamic(phb);
/* Register devices with EEH */
- if (dev->dev.of_node->child)
+ if (dev->dev.of_node->child) {
+ eeh_recovery_lock();
eeh_add_device_tree_early(PCI_DN(dev->dev.of_node));
+ eeh_recovery_unlock();
+ }
/* Scan the bus */
pcibios_scan_phb(phb);
@@ -81,7 +84,9 @@ static int of_pci_phb_probe(struct platform_device *dev)
pcibios_claim_one_bus(phb->bus);
/* Finish EEH setup */
+ eeh_recovery_lock();
eeh_add_device_tree_late(phb->bus);
+ eeh_recovery_unlock();
/* Add probed PCI devices to the device model */
pci_bus_add_devices(phb->bus);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index c6c03416a151..0fb75a2bac3c 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1400,13 +1400,17 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
}
/* Fixup EEH */
+ eeh_recovery_lock();
eeh_add_device_tree_late(bus);
+ eeh_recovery_unlock();
/* Add new devices to global lists. Register in proc, sysfs. */
pci_bus_add_devices(bus);
/* sysfs files should only be added after devices are added */
+ eeh_recovery_lock();
eeh_add_sysfs_files(bus);
+ eeh_recovery_unlock();
}
EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 28e9aa274f64..4066db91ca73 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -110,7 +110,9 @@ void pci_hp_add_devices(struct pci_bus *bus)
struct pci_controller *phb;
struct device_node *dn = pci_bus_to_OF_node(bus);
+ eeh_recovery_lock();
eeh_add_device_tree_early(PCI_DN(dn));
+ eeh_recovery_unlock();
phb = pci_bus_to_host(bus);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 6f300ab7f0e9..1f6ec78ad88b 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -45,10 +45,12 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
if (!pdn || eeh_has_flag(EEH_FORCE_DISABLED))
return;
+ eeh_recovery_lock();
dev_dbg(&pdev->dev, "EEH: Setting up device\n");
eeh_add_device_early(pdn);
eeh_add_device_late(pdev);
eeh_sysfs_add_device(pdev);
+ eeh_recovery_unlock();
}
static int pnv_eeh_init(void)
@@ -126,6 +128,7 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
unsigned long addr, mask;
char buf[50];
int ret;
+ unsigned long flags;
if (!eeh_ops || !eeh_ops->err_inject)
return -ENXIO;
@@ -141,13 +144,21 @@ static ssize_t pnv_eeh_ei_write(struct file *filp,
if (ret != 5)
return -EINVAL;
+ /*
+ * Use the spinlock rather than the mutex so that errors can be
+ * injected during slow recovery operations (for testing).
+ */
+ eeh_serialize_lock(&flags);
/* Retrieve PE */
pe = eeh_pe_get(hose, pe_no, 0);
- if (!pe)
+ if (!pe) {
+ eeh_serialize_unlock(flags);
return -ENODEV;
+ }
/* Do error injection */
ret = eeh_ops->err_inject(pe, type, func, addr, mask);
+ eeh_serialize_unlock(flags);
return ret < 0 ? ret : count;
}
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index c4ef03bec0de..c89a496a210d 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -44,11 +44,13 @@ static int ibm_configure_pe;
void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
{
- struct pci_dn *pdn = pci_get_pdn(pdev);
+ struct pci_dn *pdn;
if (eeh_has_flag(EEH_FORCE_DISABLED))
return;
+ eeh_recovery_lock();
+ pdn = pci_get_pdn(pdev);
dev_dbg(&pdev->dev, "EEH: Setting up device\n");
#ifdef CONFIG_PCI_IOV
if (pdev->is_virtfn) {
@@ -79,6 +81,7 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
}
#endif
eeh_sysfs_add_device(pdev);
+ eeh_recovery_unlock();
}
/*
diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index 361986e4354e..90657bea4736 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -36,8 +36,11 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
/* Create EEH devices for the PHB */
eeh_dev_phb_init_dynamic(phb);
- if (dn->child)
+ if (dn->child) {
+ eeh_recovery_lock();
eeh_add_device_tree_early(PCI_DN(dn));
+ eeh_recovery_unlock();
+ }
pcibios_scan_phb(phb);
pcibios_finish_adding_to_bus(phb->bus);
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 04565162a449..84ae9cde7344 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -768,7 +768,6 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
u16 sts, lsts;
u8 presence;
bool added;
- unsigned long flags;
int ret;
pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &sts);
@@ -806,10 +805,10 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
edev = pchild ? pci_dev_to_eeh_dev(pchild) : NULL;
pe = edev ? edev->pe : NULL;
if (pe) {
- eeh_serialize_lock(&flags);
+ eeh_recovery_lock();
eeh_pe_mark_isolated(pe);
- eeh_serialize_unlock(flags);
eeh_pe_set_option(pe, EEH_OPT_FREEZE_PE);
+ eeh_recovery_unlock();
}
}
diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index 977946e4e613..81ac5e10aa46 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -140,7 +140,9 @@ static void dlpar_pci_add_bus(struct device_node *dn)
struct pci_controller *phb = pdn->phb;
struct pci_dev *dev = NULL;
+ eeh_recovery_lock();
eeh_add_device_tree_early(pdn);
+ eeh_recovery_unlock();
/* Add EADS device to PHB bus, adding new entry to bus->devices */
dev = of_create_pci_dev(dn, phb->bus, pdn->devfn);
diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
index 67f55ac1d459..2f184725af5a 100644
--- a/drivers/vfio/vfio_spapr_eeh.c
+++ b/drivers/vfio/vfio_spapr_eeh.c
@@ -54,6 +54,7 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
if (op.argsz < minsz || op.flags)
return -EINVAL;
+ eeh_recovery_lock();
switch (op.op) {
case VFIO_EEH_PE_DISABLE:
ret = eeh_pe_set_option(pe, EEH_OPT_DISABLE);
@@ -84,10 +85,14 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
break;
case VFIO_EEH_PE_INJECT_ERR:
minsz = offsetofend(struct vfio_eeh_pe_op, err.mask);
- if (op.argsz < minsz)
+ if (op.argsz < minsz) {
+ eeh_recovery_unlock();
return -EINVAL;
- if (copy_from_user(&op, (void __user *)arg, minsz))
+ }
+ if (copy_from_user(&op, (void __user *)arg, minsz)) {
+ eeh_recovery_unlock();
return -EFAULT;
+ }
ret = eeh_pe_inject_err(pe, op.err.type, op.err.func,
op.err.addr, op.err.mask);
@@ -95,6 +100,7 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
default:
ret = -EINVAL;
}
+ eeh_recovery_unlock();
}
return ret;
--
2.22.0.216.g00a2a96fc9
^ permalink raw reply related
* [PATCH v4 15/25] nvdimm/ocxl: Register a character device for userspace to interact with
From: Alastair D'Silva @ 2020-03-27 7:11 UTC (permalink / raw)
To: 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, Andrew Donnellan, 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: <20200327071202.2159885-1-alastair@d-silva.org>
This patch introduces a character device (/dev/ocxlpmemX) which further
patches will use to interact with userspace, such as error logs,
controller stats and card debug functionality.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/nvdimm/ocxl/main.c | 117 ++++++++++++++++++++++++++++++++-
drivers/nvdimm/ocxl/ocxlpmem.h | 2 +
2 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
index 8db573036423..9b85fcd3f1c9 100644
--- a/drivers/nvdimm/ocxl/main.c
+++ b/drivers/nvdimm/ocxl/main.c
@@ -10,6 +10,7 @@
#include <misc/ocxl.h>
#include <linux/delay.h>
#include <linux/ndctl.h>
+#include <linux/fs.h>
#include <linux/mm_types.h>
#include <linux/memory_hotplug.h>
#include "ocxlpmem.h"
@@ -356,6 +357,67 @@ static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
return device_register(&ocxlpmem->dev);
}
+static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
+{
+ put_device(&ocxlpmem->dev);
+}
+
+static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
+{
+ return (!get_device(&ocxlpmem->dev)) ? NULL : ocxlpmem;
+}
+
+static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
+{
+ struct ocxlpmem *ocxlpmem;
+ int minor = MINOR(devno);
+
+ mutex_lock(&minors_idr_lock);
+ ocxlpmem = idr_find(&minors_idr, minor);
+ if (ocxlpmem)
+ ocxlpmem_get(ocxlpmem);
+ mutex_unlock(&minors_idr_lock);
+
+ return ocxlpmem;
+}
+
+static int file_open(struct inode *inode, struct file *file)
+{
+ struct ocxlpmem *ocxlpmem;
+
+ ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
+ if (!ocxlpmem)
+ return -ENODEV;
+
+ file->private_data = ocxlpmem;
+ return 0;
+}
+
+static int file_release(struct inode *inode, struct file *file)
+{
+ struct ocxlpmem *ocxlpmem = file->private_data;
+
+ ocxlpmem_put(ocxlpmem);
+ return 0;
+}
+
+static const struct file_operations fops = {
+ .owner = THIS_MODULE,
+ .open = file_open,
+ .release = file_release,
+};
+
+/**
+ * create_cdev() - Create the chardev in /dev for the device
+ * @ocxlpmem: the SCM metadata
+ * Return: 0 on success, negative on failure
+ */
+static int create_cdev(struct ocxlpmem *ocxlpmem)
+{
+ cdev_init(&ocxlpmem->cdev, &fops);
+ return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
+}
+
/**
* ocxlpmem_remove() - Free an OpenCAPI persistent memory device
* @pdev: the PCI device information struct
@@ -376,6 +438,13 @@ static void remove(struct pci_dev *pdev)
if (ocxlpmem->nvdimm_bus)
nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
+ /*
+ * Remove the cdev early to prevent a race against userspace
+ * via the char dev
+ */
+ if (ocxlpmem->cdev.owner)
+ cdev_del(&ocxlpmem->cdev);
+
device_unregister(&ocxlpmem->dev);
}
}
@@ -527,11 +596,18 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err;
}
- if (setup_command_metadata(ocxlpmem)) {
+ rc = setup_command_metadata(ocxlpmem);
+ if (rc) {
dev_err(&pdev->dev, "Could not read command metadata\n");
goto err;
}
+ rc = create_cdev(ocxlpmem);
+ if (rc) {
+ dev_err(&pdev->dev, "Could not create character device\n");
+ goto err;
+ }
+
elapsed = 0;
timeout = ocxlpmem->readiness_timeout +
ocxlpmem->memory_available_timeout;
@@ -599,6 +675,36 @@ static struct pci_driver pci_driver = {
.shutdown = remove,
};
+static int file_init(void)
+{
+ int rc;
+
+ rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxlpmem");
+ if (rc) {
+ idr_destroy(&minors_idr);
+ pr_err("Unable to allocate OpenCAPI persistent memory major number: %d\n",
+ rc);
+ return rc;
+ }
+
+ ocxlpmem_class = class_create(THIS_MODULE, "ocxlpmem");
+ if (IS_ERR(ocxlpmem_class)) {
+ idr_destroy(&minors_idr);
+ pr_err("Unable to create ocxlpmem class\n");
+ unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
+ return PTR_ERR(ocxlpmem_class);
+ }
+
+ return 0;
+}
+
+static void file_exit(void)
+{
+ class_destroy(ocxlpmem_class);
+ unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
+ idr_destroy(&minors_idr);
+}
+
static int __init ocxlpmem_init(void)
{
int rc;
@@ -606,16 +712,23 @@ static int __init ocxlpmem_init(void)
mutex_init(&minors_idr_lock);
idr_init(&minors_idr);
- rc = pci_register_driver(&pci_driver);
+ rc = file_init();
if (rc)
return rc;
+ rc = pci_register_driver(&pci_driver);
+ if (rc) {
+ file_exit();
+ return rc;
+ }
+
return 0;
}
static void ocxlpmem_exit(void)
{
pci_unregister_driver(&pci_driver);
+ file_exit();
}
module_init(ocxlpmem_init);
diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
index b72b3f909fc3..ee3bd651f254 100644
--- a/drivers/nvdimm/ocxl/ocxlpmem.h
+++ b/drivers/nvdimm/ocxl/ocxlpmem.h
@@ -2,6 +2,7 @@
// Copyright 2020 IBM Corp.
#include <linux/pci.h>
+#include <linux/cdev.h>
#include <misc/ocxl.h>
#include <linux/libnvdimm.h>
#include <linux/mm.h>
@@ -103,6 +104,7 @@ struct command_metadata {
struct ocxlpmem {
struct device dev;
struct pci_dev *pdev;
+ struct cdev cdev;
struct ocxl_fn *ocxl_fn;
struct nd_interleave_set nd_set;
struct nvdimm_bus_descriptor bus_desc;
--
2.24.1
^ permalink raw reply related
* [PATCH v4 06/25] ocxl: Tally up the LPC memory on a link & allow it to be mapped
From: Alastair D'Silva @ 2020-03-27 7:11 UTC (permalink / raw)
To: 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, Andrew Donnellan, 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: <20200327071202.2159885-1-alastair@d-silva.org>
OpenCAPI LPC memory is allocated per link, but each link supports
multiple AFUs, and each AFU can have LPC memory assigned to it.
This patch tallys the memory for all AFUs on a link, allowing it
to be mapped in a single operation after the AFUs have been
enumerated.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/core.c | 10 ++++++
drivers/misc/ocxl/link.c | 60 +++++++++++++++++++++++++++++++
drivers/misc/ocxl/ocxl_internal.h | 33 +++++++++++++++++
3 files changed, 103 insertions(+)
diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
index b7a09b21ab36..2531c6cf19a0 100644
--- a/drivers/misc/ocxl/core.c
+++ b/drivers/misc/ocxl/core.c
@@ -230,8 +230,18 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
if (rc)
goto err_free_pasid;
+ if (afu->config.lpc_mem_size || afu->config.special_purpose_mem_size) {
+ rc = ocxl_link_add_lpc_mem(afu->fn->link, afu->config.lpc_mem_offset,
+ afu->config.lpc_mem_size +
+ afu->config.special_purpose_mem_size);
+ if (rc)
+ goto err_free_mmio;
+ }
+
return 0;
+err_free_mmio:
+ unmap_mmio_areas(afu);
err_free_pasid:
reclaim_afu_pasid(afu);
err_free_actag:
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 58d111afd9f6..af119d3ef79a 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -84,6 +84,11 @@ struct ocxl_link {
int dev;
atomic_t irq_available;
struct spa *spa;
+ struct mutex lpc_mem_lock; /* protects lpc_mem & lpc_mem_sz */
+ u64 lpc_mem_sz; /* Total amount of LPC memory presented on the link */
+ u64 lpc_mem;
+ int lpc_consumers;
+
void *platform_data;
};
static struct list_head links_list = LIST_HEAD_INIT(links_list);
@@ -396,6 +401,8 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_l
if (rc)
goto err_spa;
+ mutex_init(&link->lpc_mem_lock);
+
/* platform specific hook */
rc = pnv_ocxl_spa_setup(dev, link->spa->spa_mem, PE_mask,
&link->platform_data);
@@ -711,3 +718,56 @@ void ocxl_link_free_irq(void *link_handle, int hw_irq)
atomic_inc(&link->irq_available);
}
EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
+
+int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64 size)
+{
+ struct ocxl_link *link = (struct ocxl_link *)link_handle;
+
+ // Check for overflow
+ if (offset > (offset + size))
+ return -EINVAL;
+
+ mutex_lock(&link->lpc_mem_lock);
+ link->lpc_mem_sz = max(link->lpc_mem_sz, offset + size);
+
+ mutex_unlock(&link->lpc_mem_lock);
+
+ return 0;
+}
+
+u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
+{
+ struct ocxl_link *link = (struct ocxl_link *)link_handle;
+
+ mutex_lock(&link->lpc_mem_lock);
+
+ if (!link->lpc_mem)
+ link->lpc_mem = pnv_ocxl_platform_lpc_setup(pdev, link->lpc_mem_sz);
+
+ if (link->lpc_mem)
+ link->lpc_consumers++;
+ mutex_unlock(&link->lpc_mem_lock);
+
+ return link->lpc_mem;
+}
+
+void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
+{
+ struct ocxl_link *link = (struct ocxl_link *)link_handle;
+
+ mutex_lock(&link->lpc_mem_lock);
+
+ if (!link->lpc_mem) {
+ mutex_unlock(&link->lpc_mem_lock);
+ return;
+ }
+
+ WARN_ON(--link->lpc_consumers < 0);
+
+ if (link->lpc_consumers == 0) {
+ pnv_ocxl_platform_lpc_release(pdev);
+ link->lpc_mem = 0;
+ }
+
+ mutex_unlock(&link->lpc_mem_lock);
+}
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 198e4e4bc51d..2d7575225bd7 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -142,4 +142,37 @@ int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
+/**
+ * ocxl_link_add_lpc_mem() - Increment the amount of memory required by an OpenCAPI link
+ *
+ * @link_handle: The OpenCAPI link handle
+ * @offset: The offset of the memory to add
+ * @size: The number of bytes to increment memory on the link by
+ *
+ * Returns 0 on success, -EINVAL on overflow
+ */
+int ocxl_link_add_lpc_mem(void *link_handle, u64 offset, u64 size);
+
+/**
+ * ocxl_link_lpc_map() - Map the LPC memory for an OpenCAPI device
+ * Since LPC memory belongs to a link, the whole LPC memory available
+ * on the link must be mapped in order to make it accessible to a device.
+ * @link_handle: The OpenCAPI link handle
+ * @pdev: A device that is on the link
+ *
+ * Returns the address of the mapped LPC memory, or 0 on error
+ */
+u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
+
+/**
+ * ocxl_link_lpc_release() - Release the LPC memory device for an OpenCAPI device
+ *
+ * Offlines LPC memory on an OpenCAPI link for a device. If this is the
+ * last device on the link to release the memory, unmap it from the link.
+ *
+ * @link_handle: The OpenCAPI link handle
+ * @pdev: A device that is on the link
+ */
+void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
+
#endif /* _OCXL_INTERNAL_H_ */
--
2.24.1
^ permalink raw reply related
* [PATCH v4 12/25] nvdimm/ocxl: Add register addresses & status values to the header
From: Alastair D'Silva @ 2020-03-27 7:11 UTC (permalink / raw)
To: 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, Andrew Donnellan, 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: <20200327071202.2159885-1-alastair@d-silva.org>
These values have been taken from the device specifications.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/nvdimm/ocxl/ocxlpmem.h | 73 ++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/drivers/nvdimm/ocxl/ocxlpmem.h b/drivers/nvdimm/ocxl/ocxlpmem.h
index 03fe7a264281..322387873b4b 100644
--- a/drivers/nvdimm/ocxl/ocxlpmem.h
+++ b/drivers/nvdimm/ocxl/ocxlpmem.h
@@ -8,6 +8,79 @@
#define LABEL_AREA_SIZE BIT_ULL(PA_SECTION_SHIFT)
+#define GLOBAL_MMIO_CHI 0x000
+#define GLOBAL_MMIO_CHIC 0x008
+#define GLOBAL_MMIO_CHIE 0x010
+#define GLOBAL_MMIO_CHIEC 0x018
+#define GLOBAL_MMIO_HCI 0x020
+#define GLOBAL_MMIO_HCIC 0x028
+#define GLOBAL_MMIO_IMA0_OHP 0x040
+#define GLOBAL_MMIO_IMA0_CFP 0x048
+#define GLOBAL_MMIO_IMA1_OHP 0x050
+#define GLOBAL_MMIO_IMA1_CFP 0x058
+#define GLOBAL_MMIO_ACMA_CREQO 0x100
+#define GLOBAL_MMIO_ACMA_CRSPO 0x104
+#define GLOBAL_MMIO_ACMA_CDBO 0x108
+#define GLOBAL_MMIO_ACMA_CDBS 0x10c
+#define GLOBAL_MMIO_NSCMA_CREQO 0x120
+#define GLOBAL_MMIO_NSCMA_CRSPO 0x124
+#define GLOBAL_MMIO_NSCMA_CDBO 0x128
+#define GLOBAL_MMIO_NSCMA_CDBS 0x12c
+#define GLOBAL_MMIO_CSTS 0x140
+#define GLOBAL_MMIO_FWVER 0x148
+#define GLOBAL_MMIO_CCAP0 0x160
+#define GLOBAL_MMIO_CCAP1 0x168
+
+#define GLOBAL_MMIO_CHI_ACRA BIT_ULL(0)
+#define GLOBAL_MMIO_CHI_NSCRA BIT_ULL(1)
+#define GLOBAL_MMIO_CHI_CRDY BIT_ULL(4)
+#define GLOBAL_MMIO_CHI_CFFS BIT_ULL(5)
+#define GLOBAL_MMIO_CHI_MA BIT_ULL(6)
+#define GLOBAL_MMIO_CHI_ELA BIT_ULL(7)
+#define GLOBAL_MMIO_CHI_CDA BIT_ULL(8)
+#define GLOBAL_MMIO_CHI_CHFS BIT_ULL(9)
+
+#define GLOBAL_MMIO_CHI_ALL (GLOBAL_MMIO_CHI_ACRA | \
+ GLOBAL_MMIO_CHI_NSCRA | \
+ GLOBAL_MMIO_CHI_CRDY | \
+ GLOBAL_MMIO_CHI_CFFS | \
+ GLOBAL_MMIO_CHI_MA | \
+ GLOBAL_MMIO_CHI_ELA | \
+ GLOBAL_MMIO_CHI_CDA | \
+ GLOBAL_MMIO_CHI_CHFS)
+
+#define GLOBAL_MMIO_HCI_ACRW BIT_ULL(0) // ACRW
+#define GLOBAL_MMIO_HCI_NSCRW BIT_ULL(1) // NSCRW
+#define GLOBAL_MMIO_HCI_AFU_RESET BIT_ULL(2) // AR
+#define GLOBAL_MMIO_HCI_FW_DEBUG BIT_ULL(3) // FDE
+#define GLOBAL_MMIO_HCI_CONTROLLER_DUMP BIT_ULL(4) // CD
+#define GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED BIT_ULL(5) // CDC
+#define GLOBAL_MMIO_HCI_REQ_HEALTH_PERF BIT_ULL(6) // CHPD
+
+#define ADMIN_COMMAND_HEARTBEAT 0x00u
+#define ADMIN_COMMAND_SHUTDOWN 0x01u
+#define ADMIN_COMMAND_FW_UPDATE 0x02u
+#define ADMIN_COMMAND_FW_DEBUG 0x03u
+#define ADMIN_COMMAND_ERRLOG 0x04u
+#define ADMIN_COMMAND_SMART 0x05u
+#define ADMIN_COMMAND_CONTROLLER_STATS 0x06u
+#define ADMIN_COMMAND_CONTROLLER_DUMP 0x07u
+#define ADMIN_COMMAND_CMD_CAPS 0x08u
+#define ADMIN_COMMAND_MAX 0x08u
+
+#define STATUS_SUCCESS 0x00
+#define STATUS_MEM_UNAVAILABLE 0x20
+#define STATUS_BLOCKED_BG_TASK 0x21
+#define STATUS_BAD_OPCODE 0x50
+#define STATUS_BAD_REQUEST_PARM 0x51
+#define STATUS_BAD_DATA_PARM 0x52
+#define STATUS_DEBUG_BLOCKED 0x70
+#define STATUS_FAIL 0xFF
+
+#define STATUS_FW_UPDATE_BLOCKED STATUS_BLOCKED_BG_TASK
+#define STATUS_FW_ARG_INVALID STATUS_BAD_REQUEST_PARM
+#define STATUS_FW_INVALID STATUS_BAD_DATA_PARM
+
struct ocxlpmem {
struct device dev;
struct pci_dev *pdev;
--
2.24.1
^ permalink raw reply related
* [PATCH v4 24/25] nvdimm/ocxl: Expose the serial number & firmware version in sysfs
From: Alastair D'Silva @ 2020-03-27 7:12 UTC (permalink / raw)
To: 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, Andrew Donnellan, 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: <20200327071202.2159885-1-alastair@d-silva.org>
This patch exposes the serial number & firmware version in sysfs,
which will be used by ndctl in userspace to help users identify
the device.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/nvdimm/ocxl/main.c | 42 ++++++++++++++++++++++++++++++++++++--
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/ocxl/main.c b/drivers/nvdimm/ocxl/main.c
index 92b4389e8cbb..1f422f0d51ef 100644
--- a/drivers/nvdimm/ocxl/main.c
+++ b/drivers/nvdimm/ocxl/main.c
@@ -235,6 +235,43 @@ static int reserve_metadata(struct ocxlpmem *ocxlpmem,
return 0;
}
+static ssize_t serial_show(struct device *device, struct device_attribute *attr,
+ char *buf)
+{
+ struct nvdimm *nvdimm = to_nvdimm(device);
+ struct ocxlpmem *ocxlpmem = nvdimm_provider_data(nvdimm);
+ const struct ocxl_fn_config *fn_config = ocxl_function_config(ocxlpmem->ocxl_fn);
+
+ return scnprintf(buf, PAGE_SIZE, "%llu\n", fn_config->serial);
+}
+static DEVICE_ATTR_RO(serial);
+
+static ssize_t fw_version_show(struct device *device,
+ struct device_attribute *attr, char *buf)
+{
+ struct nvdimm *nvdimm = to_nvdimm(device);
+ struct ocxlpmem *ocxlpmem = nvdimm_provider_data(nvdimm);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", ocxlpmem->fw_version);
+}
+static DEVICE_ATTR_RO(fw_version);
+
+static struct attribute *ocxl_pmem_attrs[] = {
+ &dev_attr_serial.attr,
+ &dev_attr_fw_version.attr,
+ NULL,
+};
+
+static const struct attribute_group ocxl_pmem_attribute_group = {
+ .name = "ocxlpmem",
+ .attrs = ocxl_pmem_attrs,
+};
+
+static const struct attribute_group *ocxl_pmem_dimm_attribute_groups[] = {
+ &ocxl_pmem_attribute_group,
+ NULL,
+};
+
/**
* register_lpc_mem() - Discover persistent memory on a device and register it with the NVDIMM subsystem
* @ocxlpmem: the device metadata
@@ -291,8 +328,9 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
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);
+ ocxl_pmem_dimm_attribute_groups,
+ nvdimm_flags, nvdimm_cmd_mask, 0,
+ NULL);
if (!nd_mapping_desc.nvdimm)
return -ENOMEM;
--
2.24.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