* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Arnd Bergmann @ 2020-04-15 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jeremy Kerr, linux-kernel@vger.kernel.org, Eric W . Biederman,
Linux FS-devel Mailing List, Andrew Morton, linuxppc-dev,
Alexander Viro
In-Reply-To: <20200415074514.GA1393@lst.de>
On Wed, Apr 15, 2020 at 9:45 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Apr 14, 2020 at 03:15:09PM +0200, Arnd Bergmann wrote:
> > I don't think you are changing the behavior here, but I still wonder if it
> > is in fact correct for x32: is in_x32_syscall() true here when dumping an
> > x32 compat elf process, or should this rather be set according to which
> > binfmt_elf copy is being used?
>
> The infrastructure could enable that, although it would require more
> arch hooks I think.
I was more interested in whether you can tell if it's currently broken
or not. If my feeling is right that the current code does the wrong thing
here, it would be good to at least put a FIXME comment in there.
> I'd rather keep it out of this series and to
> an interested party. Then again x32 doesn't seem to have a whole lot
> of interested parties..
Fine with me. It's on my mental list of things that we want to kill off
eventually as soon as the remaining users stop replying to questions
about it.
In fact I should really turn that into a properly maintained list in
Documentation/... that contains any options that someone has
asked about removing in the past, along with the reasons for keeping
it around and a time at which we should ask about it again.
Arnd
^ permalink raw reply
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Balamuruhan S @ 2020-04-15 8:14 UTC (permalink / raw)
To: Jordan Niethe
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <CACzsE9rc+g5eWko0RQdhbvWpPXUwORdKVU8Za2650ROoZAjBdg@mail.gmail.com>
On Wed, 2020-04-15 at 14:40 +1000, Jordan Niethe wrote:
> On Mon, Apr 13, 2020 at 10:04 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > > For powerpc64, redefine the ppc_inst type so both word and prefixed
> > > instructions can be represented. On powerpc32 the type will remain the
> > > same. Update places which had assumed instructions to be 4 bytes long.
> > >
> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > ---
> > > v4: New to series
> > > v5: - Distinguish normal instructions from prefixed instructions with a
> > > 0xff marker for the suffix.
> > > - __patch_instruction() using std for prefixed instructions
> > > ---
> > > arch/powerpc/include/asm/inst.h | 71 ++++++++++++++++++++++++++--
> > > arch/powerpc/include/asm/kprobes.h | 2 +-
> > > arch/powerpc/include/asm/uaccess.h | 31 ++++++++++--
> > > arch/powerpc/include/asm/uprobes.h | 2 +-
> > > arch/powerpc/kernel/optprobes.c | 42 ++++++++--------
> > > arch/powerpc/kernel/optprobes_head.S | 3 ++
> > > arch/powerpc/kernel/trace/ftrace.c | 26 +++++++++-
> > > arch/powerpc/lib/code-patching.c | 19 +++++---
> > > arch/powerpc/lib/feature-fixups.c | 5 +-
> > > arch/powerpc/lib/sstep.c | 4 +-
> > > arch/powerpc/xmon/xmon.c | 6 +--
> > > arch/powerpc/xmon/xmon_bpts.S | 4 +-
> > > 12 files changed, 171 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/inst.h
> > > b/arch/powerpc/include/asm/inst.h
> > > index 70b37a35a91a..7e23e7146c66 100644
> > > --- a/arch/powerpc/include/asm/inst.h
> > > +++ b/arch/powerpc/include/asm/inst.h
> > > @@ -8,23 +8,67 @@
> > >
> > > struct ppc_inst {
> > > u32 val;
> > > +#ifdef __powerpc64__
> > > + u32 suffix;
> > > +#endif /* __powerpc64__ */
> > > } __packed;
> > >
> > > -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > > +static inline int ppc_inst_opcode(struct ppc_inst x)
> > > +{
> > > + return x.val >> 26;
> >
> > why don't we wrap here and in `ppc_inst_opcode()` in patch 9 using
> > `ppc_inst_val()` ?
> Will do.
> >
> > > +}
> > >
> > > static inline u32 ppc_inst_val(struct ppc_inst x)
> >
> > There is another same definition below for the same function in
> > #else part of __powerpc64__ ifdef.
> Thanks
> >
> > > {
> > > return x.val;
> > > }
> > >
> > > -static inline bool ppc_inst_len(struct ppc_inst x)
> > > +#ifdef __powerpc64__
> > > +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> > > +
> > > +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix =
> > > (y)
> > > })
> > > +
> > > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > > {
> > > - return sizeof(struct ppc_inst);
> > > + return x.suffix;
> > > }
> > >
> > > -static inline int ppc_inst_opcode(struct ppc_inst x)
> > > +static inline bool ppc_inst_prefixed(struct ppc_inst x) {
> > > + return ((ppc_inst_val(x) >> 26) == 1) && ppc_inst_suffix(x) !=
> > > 0xff;
> > > +}
> > > +
> > > +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > > {
> > > - return x.val >> 26;
> > > + return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> > > + swab32(ppc_inst_suffix(x)));
> > > +}
> > > +
> > > +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > > +{
> > > + u32 val, suffix = 0xff;
> > > + val = *(u32 *)ptr;
> > > + if ((val >> 26) == 1)
> > > + suffix = *((u32 *)ptr + 1);
> > > + return ppc_inst_prefix(val, suffix);
> > > +}
> > > +
> > > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> > > x)
> > > +{
> > > + if (ppc_inst_prefixed(x)) {
> > > + *(u32 *)ptr = x.val;
> > > + *((u32 *)ptr + 1) = x.suffix;
> > > + } else {
> > > + *(u32 *)ptr = x.val;
> >
> > can we wrap here as well with `ppc_inst_val()` and `ppc_inst_suffix()` ?
> Yeah no reason not too.
> >
> > > + }
> > > +}
> > > +
> > > +#else
> > > +
> > > +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > > +
> > > +static inline bool ppc_inst_prefixed(ppc_inst x)
> > > +{
> > > + return 0;
> >
> > Is it return !!0 or return false ?
> False probably will make more sense.
> >
> > > }
> > >
> > > static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > > @@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
> > > ppc_inst x)
> > > return ppc_inst(swab32(ppc_inst_val(x)));
> > > }
> > >
> > > +static inline u32 ppc_inst_val(struct ppc_inst x)
> >
> > [...] duplicate definition that is defined outside __powerpc64__ above.
> >
> >
> > > +{
> > > + return x.val;
> > > +}
> > > +
> > > static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > > {
> > > return *ptr;
> > > }
> > >
> > > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> > > x)
> > > +{
> > > + *ptr = x;
> > > +}
> > > +
> > > +#endif /* __powerpc64__ */
> > > +
> > > static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> > > {
> > > return !memcmp(&x, &y, sizeof(struct ppc_inst));
> > > }
> > >
> > > +static inline int ppc_inst_len(struct ppc_inst x)
> > > +{
> > > + return (ppc_inst_prefixed(x)) ? 8 : 4;
> > > +}
> > > +
> > > #endif /* _ASM_INST_H */
> > > diff --git a/arch/powerpc/include/asm/kprobes.h
> > > b/arch/powerpc/include/asm/kprobes.h
> > > index 66b3f2983b22..4fc0e15e23a5 100644
> > > --- a/arch/powerpc/include/asm/kprobes.h
> > > +++ b/arch/powerpc/include/asm/kprobes.h
> > > @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> > > extern kprobe_opcode_t optprobe_template_end[];
> > >
> > > /* Fixed instruction size for powerpc */
> > > -#define MAX_INSN_SIZE 1
> > > +#define MAX_INSN_SIZE 2
> > > #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> > > #define MAX_OPTINSN_SIZE (optprobe_template_end -
> > > optprobe_template_entry)
> > > #define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
> > > diff --git a/arch/powerpc/include/asm/uaccess.h
> > > b/arch/powerpc/include/asm/uaccess.h
> > > index c0a35e4586a5..5a3f486ddf02 100644
> > > --- a/arch/powerpc/include/asm/uaccess.h
> > > +++ b/arch/powerpc/include/asm/uaccess.h
> > > @@ -105,11 +105,34 @@ static inline int __access_ok(unsigned long addr,
> > > unsigned long size,
> > > #define __put_user_inatomic(x, ptr) \
> > > __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> > >
> > > -#define __get_user_instr(x, ptr) \
> > > - __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> > > +#define __get_user_instr(x, ptr) \
> > > +({ \
> > > + long __gui_ret = 0; \
> > > + unsigned int prefix, suffix; \
> > > + __gui_ret = __get_user(prefix, (unsigned int __user *)ptr);
> > > \
> > > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > > + __gui_ret = __get_user(suffix, (unsigned int __user *)ptr +
> > > 1);
> > > \
> > > + (x) = ppc_inst_prefix(prefix, suffix); \
> > > + } else { \
> > > + (x) = ppc_inst(prefix); \
> > > + } \
> > > + __gui_ret; \
> > > +})
> > > +
> > > +#define __get_user_instr_inatomic(x, ptr) \
> > > +({ \
> > > + long __gui_ret = 0; \
> > > + unsigned int prefix, suffix; \
> > > + __gui_ret = __get_user_inatomic(prefix, (unsigned int __user
> > > *)ptr);
> > > \
> > > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > > + __gui_ret = __get_user_inatomic(suffix, (unsigned int
> > > __user
> > > *)ptr + 1); \
> > > + (x) = ppc_inst_prefix(prefix, suffix); \
> > > + } else { \
> > > + (x) = ppc_inst(prefix); \
> > > + } \
> > > + __gui_ret; \
> > > +})
> > >
> > > -#define __get_user_instr_inatomic(x, ptr) \
> > > - __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
> > > extern long __put_user_bad(void);
> > >
> > > /*
> > > diff --git a/arch/powerpc/include/asm/uprobes.h
> > > b/arch/powerpc/include/asm/uprobes.h
> > > index 7e3b329ba2d3..5bf65f5d44a9 100644
> > > --- a/arch/powerpc/include/asm/uprobes.h
> > > +++ b/arch/powerpc/include/asm/uprobes.h
> > > @@ -15,7 +15,7 @@
> > >
> > > typedef ppc_opcode_t uprobe_opcode_t;
> > >
> > > -#define MAX_UINSN_BYTES 4
> > > +#define MAX_UINSN_BYTES 8
> > > #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
> > >
> > > /* The following alias is needed for reference from arch-agnostic code
> > > */
> > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > b/arch/powerpc/kernel/optprobes.c
> > > index 684640b8fa2e..689daf430161 100644
> > > --- a/arch/powerpc/kernel/optprobes.c
> > > +++ b/arch/powerpc/kernel/optprobes.c
> > > @@ -159,38 +159,38 @@ void patch_imm32_load_insns(unsigned int val,
> > > kprobe_opcode_t *addr)
> > >
> > > /*
> > > * Generate instructions to load provided immediate 64-bit value
> > > - * to register 'r3' and patch these instructions at 'addr'.
> > > + * to register 'reg' and patch these instructions at 'addr'.
> > > */
> > > -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> > > +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t
> > > *addr)
> > > {
> > > - /* lis r3,(op)@highest */
> > > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> > > |
> > > ___PPC_RT(3) |
> > > + /* lis reg,(op)@highest */
> > > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> > > |
> > > ___PPC_RT(reg) |
> > > ((val >> 48) & 0xffff)));
> > > addr++;
> > >
> > > - /* ori r3,r3,(op)@higher */
> > > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(3) |
> > > - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> > > + /* ori reg,reg,(op)@higher */
> > > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(reg) |
> > > + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> > > addr++;
> > >
> > > - /* rldicr r3,r3,32,31 */
> > > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR
> > > |
> > > ___PPC_RA(3) |
> > > - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> > > + /* rldicr reg,reg,32,31 */
> > > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR
> > > |
> > > ___PPC_RA(reg) |
> > > + ___PPC_RS(reg) | __PPC_SH64(32) |
> > > __PPC_ME64(31)));
> > > addr++;
> > >
> > > - /* oris r3,r3,(op)@h */
> > > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > > ___PPC_RA(3) |
> > > - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> > > + /* oris reg,reg,(op)@h */
> > > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > > ___PPC_RA(reg) |
> > > + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> > > addr++;
> > >
> > > - /* ori r3,r3,(op)@l */
> > > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(3) |
> > > - ___PPC_RS(3) | (val & 0xffff)));
> > > + /* ori reg,reg,(op)@l */
> > > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(reg) |
> > > + ___PPC_RS(reg) | (val & 0xffff)));
> > > }
> > >
> > > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct
> > > kprobe
> > > *p)
> > > {
> > > - struct ppc_inst branch_op_callback, branch_emulate_step;
> > > + struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> > > kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > > long b_offset;
> > > unsigned long nip, size;
> > > @@ -240,7 +240,7 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe
> > > *op, struct kprobe *p)
> > > * Fixup the template with instructions to:
> > > * 1. load the address of the actual probepoint
> > > */
> > > - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> > > + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> > >
> > > /*
> > > * 2. branch to optimized_callback() and emulate_step()
> > > @@ -271,7 +271,11 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p)
> > > /*
> > > * 3. load instruction to be emulated into relevant register, and
> > > */
> > > - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
> > > + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> > > + patch_imm64_load_insns(ppc_inst_val(temp) |
> > > + ((u64)ppc_inst_suffix(temp) << 32),
> >
> > did we check building for ppc32 ?
> Yeah I need to do so.
> > I doubt we might hit build failure as `ppc_inst_suffix()` macro is not
> > defined.
> >
> >
> > > + 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 e78742613b36..16041a5c86d5 100644
> > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > @@ -41,11 +41,35 @@
> > > #define NUM_FTRACE_TRAMPS 8
> > > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> > >
> > > +#ifdef __powerpc64__
> > > static long
> > > probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > > {
> > > - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> > > + u32 val, suffix = 0;
> >
> > don't we need to initialize suffix with 0xff ?
> Good catch, that was causing problems.
Also we can do,
if ((val >> 26) == 1)
{
err = probe_kernel_read((void *)&suffix,
src + 4, MCOUNT_INSN_SIZE);
if (err)
return err;
*inst = ppc_inst_prefix(val, suffix);
}
else
*inst = ppc_inst(val);
to explicitly show a non-prefixed instruction and we do not have to
initialize suffix to 0xff, as ppc_inst() would take care.
-- Bala
> >
> > > + long err;
> > > +
> > > + err = probe_kernel_read((void *)&val,
> > > + src, sizeof(val));
> > > + if (err)
> > > + return err;
> > > +
> > > + if ((val >> 26) == 1)
> > > + err = probe_kernel_read((void *)&suffix,
> > > + src + 4, MCOUNT_INSN_SIZE);
> > > + if (err)
> > > + return err;
> > > +
> > > + *inst = ppc_inst_prefix(val, suffix);
> > > +
> > > + return 0;
> > > }
> > > +#else
> > > +static long
> > > +probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > > +{
> > > + return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE)
> > > +}
> > > +#endif
> > >
> > > static struct ppc_inst
> > > ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > > patching.c
> > > index c329ad657302..b4007e03d8fa 100644
> > > --- a/arch/powerpc/lib/code-patching.c
> > > +++ b/arch/powerpc/lib/code-patching.c
> > > @@ -24,12 +24,19 @@ static int __patch_instruction(struct ppc_inst
> > > *exec_addr, struct ppc_inst instr
> > > {
> > > int err = 0;
> > >
> > > - __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> > > - if (err)
> > > - return err;
> > > -
> > > - asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > > (patch_addr),
> > > - "r"
> > > (exec_addr));
> > > + if (!ppc_inst_prefixed(instr)) {
> > > + __put_user_asm(ppc_inst_val(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));
> > > + } else {
> > > + __put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
> > > ppc_inst_val(instr), patch_addr, err, "std");
> > > + if (err)
> > > + return err;
> > > + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > > (patch_addr),
> > > + "r"
> > > (exec_addr));
> > > + }
> >
> > can we keep these 2 lines out of conditions as it remains to be the same ?
> True.
> > if (err)
> > return err;
> > asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr), "r"
> > (exec_addr));
> >
> >
> > > return 0;
> > > }
> > > diff --git a/arch/powerpc/lib/feature-fixups.c
> > > b/arch/powerpc/lib/feature-
> > > fixups.c
> > > index f00dd13b1c3c..5519cec83cc8 100644
> > > --- a/arch/powerpc/lib/feature-fixups.c
> > > +++ b/arch/powerpc/lib/feature-fixups.c
> > > @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value,
> > > struct fixup_entry *fcur)
> > > src = alt_start;
> > > dest = start;
> > >
> > > - for (; src < alt_end; src++, dest++) {
> > > + for (; src < alt_end; src = (void *)src +
> > > ppc_inst_len(ppc_inst_read(src)),
> > > + (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
> > > if (patch_alt_instruction(src, dest, alt_start, alt_end))
> > > return 1;
> > > }
> > >
> > > - for (; dest < end; dest++)
> > > + for (; dest < end; dest = (void *)dest +
> > > ppc_inst_len(ppc_inst(PPC_INST_NOP)))
> > > raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
> > >
> > > return 0;
> > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > > index 52ddd3122dc8..8b285bf11218 100644
> > > --- a/arch/powerpc/lib/sstep.c
> > > +++ b/arch/powerpc/lib/sstep.c
> > > @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op,
> > > const
> > > struct pt_regs *regs,
> > > unsigned long int imm;
> > > unsigned long int val, val2;
> > > unsigned int mb, me, sh;
> > > - unsigned int word;
> > > + unsigned int word, suffix;
> > > long ival;
> > >
> > > word = ppc_inst_val(instr);
> > > + suffix = ppc_inst_suffix(instr);
> >
> > same here, I doubt it might break for ppc32.
> Yeah I need to work on ppc32.
> >
> > -- Bala
> > > +
> > > op->type = COMPUTE;
> > >
> > > opcode = word >> 26;
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index 6f3bcdcfc9c7..b704aebb099a 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -761,8 +761,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;
> > > }
> > > @@ -863,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long
> > > nip,
> > > unsigned long *offp)
> > > if (off >= sizeof(bpt_table))
> > > return NULL;
> > > *offp = off % BPT_SIZE;
> > > - if (*offp != 0 && *offp != 4)
> > > + if (*offp != 0 && *offp != 4 && *offp != 8)
> > > return NULL;
> > > return bpts + (off / BPT_SIZE);
> > > }
> > > diff --git a/arch/powerpc/xmon/xmon_bpts.S
> > > b/arch/powerpc/xmon/xmon_bpts.S
> > > index ebb2dbc70ca8..09058eb6abbd 100644
> > > --- a/arch/powerpc/xmon/xmon_bpts.S
> > > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > > @@ -3,6 +3,8 @@
> > > #include <asm/asm-compat.h>
> > > #include "xmon_bpts.h"
> > >
> > > +/* Prefixed instructions can not cross 64 byte boundaries */
> > > +.align 6
> > > .global bpt_table
> > > bpt_table:
> > > - .space NBPTS * 8
> > > + .space NBPTS * 16
^ permalink raw reply
* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Christoph Hellwig @ 2020-04-15 7:45 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel@vger.kernel.org, Alexander Viro, Jeremy Kerr,
Linux FS-devel Mailing List, Andrew Morton, linuxppc-dev,
Christoph Hellwig, Eric W . Biederman
In-Reply-To: <CAK8P3a3HvbPKTkwfWr6PbZ96koO_NrJP1qgk8H1mgk=qUScGkQ@mail.gmail.com>
On Tue, Apr 14, 2020 at 03:15:09PM +0200, Arnd Bergmann wrote:
> I don't think you are changing the behavior here, but I still wonder if it
> is in fact correct for x32: is in_x32_syscall() true here when dumping an
> x32 compat elf process, or should this rather be set according to which
> binfmt_elf copy is being used?
The infrastructure cold enable that, although it would require more
arch hooks I think. I'd rather keep it out of this series and to
an interested party. Then again x32 doesn't seem to have a whole lot
of interested parties..
^ permalink raw reply
* Re: [EXTERNAL] [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts
From: Cédric Le Goater @ 2020-04-15 6:49 UTC (permalink / raw)
To: Nicholas Piggin, qemu-devel
Cc: qemu-stable, qemu-ppc, Nathan Chancellor, linuxppc-dev,
David Gibson
In-Reply-To: <20200414111131.465560-1-npiggin@gmail.com>
On 4/14/20 1:11 PM, Nicholas Piggin wrote:
> If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending,
> it does not cause an interrupt. This causes the test case to hang:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Dppc_2019-2D10_msg00826.html&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=XHJsZhhuWSw9713Fp0ciew&m=TQfi_v-8XYgz7MiMDAZ_CjkyalSh9-EXhQ3oDesUm74&s=pFoesXbioVBh5wCuzEnzwgfze6X7e-a9unkfUgsRwiw&e=
>
> More recently, Linux reduced the occurance of operations (e.g., rfi)
> which stop translation and allow pending interrupts to be processed.
> This started causing hangs in Linux boot in long-running kernel tests,
> running with '-d int' shows the decrementer stops firing despite DEC
> wrapping and MSR[EE]=1.
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.ozlabs.org_pipermail_linuxppc-2Ddev_2020-2DApril_208301.html&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=XHJsZhhuWSw9713Fp0ciew&m=TQfi_v-8XYgz7MiMDAZ_CjkyalSh9-EXhQ3oDesUm74&s=EhkRfxvQMomvneYweWDEIUktCkKykgIqEmdhA0PtiwU&e=
>
> The cause is the broken mtmsr L=1 behaviour, which is contrary to the
> architecture. From Power ISA v3.0B, p.977, Move To Machine State Register,
> Programming Note states:
>
> If MSR[EE]=0 and an External, Decrementer, or Performance Monitor
> exception is pending, executing an mtmsrd instruction that sets
> MSR[EE] to 1 will cause the interrupt to occur before the next
> instruction is executed, if no higher priority exception exists
>
> Fix this by handling L=1 exactly the same way as L=0, modulo the MSR
> bits altered.
>
> The confusion arises from L=0 being "context synchronizing" whereas L=1
> is "execution synchronizing", which is a weaker semantic. However this
> is not a relaxation of the requirement that these exceptions cause
> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
> TCG is doing here), rather it specifies how a pipelined processor can
> have multiple instructions in flight where one may influence how another
> behaves.
I was expecting more changes but this looks fine.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Cc: qemu-stable@nongnu.org
> Reported-by: Anton Blanchard <anton@ozlabs.org>
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
I gave it a try on PowerNV, pseries and mac99. All good.
Tested-by: Cédric Le Goater <clg@kaod.org>
I don't know how we could include tests in QEMU such as the one Anton
sent. These are good exercisers for our exception model.
Thanks,
C.
> ---
> Thanks very much to Nathan for reporting and testing it, I added his
> Tested-by tag despite a more polished patch, as the the basics are
> still the same (and still fixes his test case here).
>
> This bug possibly goes back to early v2.04 / mtmsrd L=1 support around
> 2007, and the code has been changed several times since then so may
> require some backporting.
>
> 32-bit / mtmsr untested at the moment, I don't have an environment
> handy.
>
>
> target/ppc/translate.c | 46 +++++++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..9959259dba 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> if (ctx->opcode & 0x00010000) {
> - /* Special form that does not need any synchronisation */
> + /* L=1 form only updates EE and RI */
> TCGv t0 = tcg_temp_new();
> + TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(cpu_msr, cpu_msr,
> + tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> + tcg_gen_or_tl(t1, t1, t0);
> +
> + gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> + tcg_temp_free(t1);
> +
> } else {
> /*
> * XXX: we need to update nip before the store if we enter
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> - if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> - gen_io_start();
> - }
> gen_update_nip(ctx, ctx->base.pc_next);
> gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
> - /* Must stop the translation as machine state (may have) changed */
> - /* Note that mtmsr is not always defined as context-synchronizing */
> - gen_stop_exception(ctx);
> }
> + /* Must stop the translation as machine state (may have) changed */
> + gen_stop_exception(ctx);
> #endif /* !defined(CONFIG_USER_ONLY) */
> }
> #endif /* defined(TARGET_PPC64) */
> @@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> - if (ctx->opcode & 0x00010000) {
> - /* Special form that does not need any synchronisation */
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> + if (ctx->opcode & 0x00010000) {
> + /* L=1 form only updates EE and RI */
> TCGv t0 = tcg_temp_new();
> + TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(cpu_msr, cpu_msr,
> + tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> + tcg_gen_or_tl(t1, t1, t0);
> +
> + gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> + tcg_temp_free(t1);
> +
> } else {
> TCGv msr = tcg_temp_new();
>
> @@ -4411,9 +4423,6 @@ static void gen_mtmsr(DisasContext *ctx)
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> - if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> - gen_io_start();
> - }
> gen_update_nip(ctx, ctx->base.pc_next);
> #if defined(TARGET_PPC64)
> tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
> @@ -4422,10 +4431,9 @@ static void gen_mtmsr(DisasContext *ctx)
> #endif
> gen_helper_store_msr(cpu_env, msr);
> tcg_temp_free(msr);
> - /* Must stop the translation as machine state (may have) changed */
> - /* Note that mtmsr is not always defined as context-synchronizing */
> - gen_stop_exception(ctx);
> }
> + /* Must stop the translation as machine state (may have) changed */
> + gen_stop_exception(ctx);
> #endif
> }
>
>
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/eeh: Remove workaround from eeh_add_device_late()
From: Sam Bobroff @ 2020-04-15 6:44 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <CAOSf1CHA+ZzWpLtuPrvkOvWO13Dikv86UjiBdppyG4GrexGSpA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]
On Wed, Apr 08, 2020 at 04:53:36PM +1000, Oliver O'Halloran wrote:
> On Wed, Apr 8, 2020 at 4:22 PM Sam Bobroff <sbobroff@linux.ibm.com> wrote:
> >
> > On Fri, Apr 03, 2020 at 05:08:32PM +1100, Oliver O'Halloran wrote:
> > > On Mon, 2020-03-30 at 15:56 +1100, Sam Bobroff wrote:
> > > > When EEH device state was released asynchronously by the device
> > > > release handler, it was possible for an outstanding reference to
> > > > prevent it's release and it was necessary to work around that if a
> > > > device was re-discovered at the same PCI location.
> > >
> > > I think this is a bit misleading. The main situation where you'll hit
> > > this hack is when recovering a device with a driver that doesn't
> > > implement the error handling callbacks. In that case the device is
> > > removed, reset, then re-probed by the PCI core, but we assume it's the
> > > same physical device so the eeh_device state remains active.
> > >
> > > If you actually changed the underlying device I suspect something bad
> > > would happen.
> >
> > I'm not sure I understand. Isn't the case you're talking about caught by
> > the earlier check (just above the patch)?
> >
> > if (edev->pdev == dev) {
> > eeh_edev_dbg(edev, "Device already referenced!\n");
> > return;
> > }
>
> No, in the case I'm talking about the pci_dev is torn down and
> freed(). After the PE is reset we re-probe the device and create a new
> pci_dev. If the release of the old pci_dev is delayed we need the
> hack this patch is removing.
Oh, yes, that is the case I was intending to change here. But I must be
missing something, isn't it also the case that's changed by patch 2/4?
What I intended was, after patch 2, eeh_remove_device() is called from
the bus notifier so it happens imediately when recovery calls
pci_stop_and_remove_bus_device(). Once it returns, edev->pdev has
already been set to NULL by eeh_remove_device() so this case can't be
hit anymore, and we should clean it up (this patch).
(There is a slight difference in the way EEH_PE_KEEP is handled between
the code removed here and the body of eeh_remove_device(), but checking
and explaining that is already on my list for v2.)
(I did test recovery on an unaware device and didn't hit the
WARN_ON_ONCE().)
> The check above should probably be a WARN_ON() since we should never
> be re-running the EEH probe on the same device. I think there is a
> case where that can happen, but I don't remember the details.
Yeah, I also certainly see the "Device already referenced!" message
while debugging, and it would be good to track down.
> Oliver
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Christoph Hellwig @ 2020-04-15 6:19 UTC (permalink / raw)
To: Michael Ellerman
Cc: Arnd Bergmann, linux-kernel, Alexander Viro, Jeremy Kerr,
linux-fsdevel, Andrew Morton, linuxppc-dev, Christoph Hellwig,
Eric W . Biederman
In-Reply-To: <87y2qxih94.fsf@mpe.ellerman.id.au>
On Wed, Apr 15, 2020 at 01:01:59PM +1000, Michael Ellerman wrote:
> > + to_compat_siginfo(csigdata, siginfo, compat_siginfo_flags()); \
> > + fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata); \
> > +} while (0)
>
> This doesn't build on ppc (cell_defconfig):
>
> ../fs/binfmt_elf.c: In function 'fill_note_info':
> ../fs/compat_binfmt_elf.c:44:39: error: implicit declaration of function 'compat_siginfo_flags'; did you mean 'to_compat_siginfo'? [-Werror=implicit-function-d
> eclaration]
> to_compat_siginfo(csigdata, siginfo, compat_siginfo_flags()); \
> ^~~~~~~~~~~~~~~~~~~~
> ../fs/binfmt_elf.c:1846:2: note: in expansion of macro 'fill_siginfo_note'
> fill_siginfo_note(&info->signote, &info->csigdata, siginfo);
> ^~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make[2]: *** [../scripts/Makefile.build:266: fs/compat_binfmt_elf.o] Error 1
>
>
> I guess the empty version from kernel/signal.c needs to move into a
> header somewhere.
Yes, it should. Odd that the buildbut hasn't complained yet so far..
^ permalink raw reply
* Re: [PATCH 1/4] dma-mapping: move the remaining DMA API calls out of line
From: Christoph Hellwig @ 2020-04-15 6:18 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, linuxppc-dev, Christoph Hellwig, Lu Baolu
In-Reply-To: <c2572d30-f03c-450d-e257-3a8673b42d44@ozlabs.ru>
On Wed, Apr 15, 2020 at 12:26:04PM +1000, Alexey Kardashevskiy wrote:
> May be this is correct and allowed (no idea) but removing exported
> symbols at least deserves a mention in the commit log, does not it?
>
> The rest of the series is fine and works. Thanks,
Maybe I can throw in a line, but the point is that dma_direct_*
was exported as dma_* called them inline. Now dma_* is out of line
and exported instead, which always was the actual API.
^ permalink raw reply
* Re: [RFC PATCH] powerpc/lib: Fixing use a temporary mm for code patching
From: Christopher M Riedl @ 2020-04-15 5:16 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c88b13ede49744d81fdab32e037a7ae10f0b241f.1585233657.git.christophe.leroy@c-s.fr>
> On March 26, 2020 9:42 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
> This patch fixes the RFC series identified below.
> It fixes three points:
> - Failure with CONFIG_PPC_KUAP
> - Failure to write do to lack of DIRTY bit set on the 8xx
> - Inadequaly complex WARN post verification
>
> However, it has an impact on the CPU load. Here is the time
> needed on an 8xx to run the ftrace selftests without and
> with this series:
> - Without CONFIG_STRICT_KERNEL_RWX ==> 38 seconds
> - With CONFIG_STRICT_KERNEL_RWX ==> 40 seconds
> - With CONFIG_STRICT_KERNEL_RWX + this series ==> 43 seconds
>
> Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=166003
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/lib/code-patching.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index f156132e8975..4ccff427592e 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -97,6 +97,7 @@ static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
> }
>
> pte = mk_pte(page, pgprot);
> + pte = pte_mkdirty(pte);
> set_pte_at(patching_mm, patching_addr, ptep, pte);
>
> init_temp_mm(&patch_mapping->temp_mm, patching_mm);
> @@ -168,7 +169,9 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> (offset_in_page((unsigned long)addr) /
> sizeof(unsigned int));
>
> + allow_write_to_user(patch_addr, sizeof(instr));
> __patch_instruction(addr, instr, patch_addr);
> + prevent_write_to_user(patch_addr, sizeof(instr));
>
On radix we can map the page with PAGE_KERNEL protection which ends up
setting EAA[0] in the radix PTE. This means the KUAP (AMR) protection is
ignored (ISA v3.0b Fig. 35) since we are accessing the page from MSR[PR]=0.
Can we employ a similar approach on the 8xx? I would prefer *not* to wrap
the __patch_instruction() with the allow_/prevent_write_to_user() KUAP things
because this is a temporary kernel mapping which really isn't userspace in
the usual sense.
> err = unmap_patch(&patch_mapping);
> if (err)
> @@ -179,7 +182,7 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> * think we just wrote.
> * XXX: BUG_ON() instead?
> */
> - WARN_ON(memcmp(addr, &instr, sizeof(instr)));
> + WARN_ON(*addr != instr);
>
> out:
> local_irq_restore(flags);
> --
> 2.25.0
^ permalink raw reply
* Re: [RFC PATCH 3/3] powerpc/lib: Use a temporary mm for code patching
From: Christopher M Riedl @ 2020-04-15 5:11 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <db40ec6a-1d81-91e3-00d8-cd86fd5262d5@c-s.fr>
> On March 24, 2020 11:25 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
> Le 23/03/2020 à 05:52, Christopher M. Riedl a écrit :
> > Currently, code patching a STRICT_KERNEL_RWX exposes the temporary
> > mappings to other CPUs. These mappings should be kept local to the CPU
> > doing the patching. Use the pre-initialized temporary mm and patching
> > address for this purpose. Also add a check after patching to ensure the
> > patch succeeded.
> >
> > Based on x86 implementation:
> >
> > commit b3fd8e83ada0
> > ("x86/alternatives: Use temporary mm for text poking")
> >
> > Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> > ---
> > arch/powerpc/lib/code-patching.c | 128 ++++++++++++++-----------------
> > 1 file changed, 57 insertions(+), 71 deletions(-)
> >
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> > index 18b88ecfc5a8..f156132e8975 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -19,6 +19,7 @@
> > #include <asm/page.h>
> > #include <asm/code-patching.h>
> > #include <asm/setup.h>
> > +#include <asm/mmu_context.h>
> >
> > static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
> > unsigned int *patch_addr)
> > @@ -65,99 +66,79 @@ void __init poking_init(void)
> > pte_unmap_unlock(ptep, ptl);
> > }
> >
> > -static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
> > -
> > -static int text_area_cpu_up(unsigned int cpu)
> > -{
> > - struct vm_struct *area;
> > -
> > - area = get_vm_area(PAGE_SIZE, VM_ALLOC);
> > - if (!area) {
> > - WARN_ONCE(1, "Failed to create text area for cpu %d\n",
> > - cpu);
> > - return -1;
> > - }
> > - this_cpu_write(text_poke_area, area);
> > -
> > - return 0;
> > -}
> > -
> > -static int text_area_cpu_down(unsigned int cpu)
> > -{
> > - free_vm_area(this_cpu_read(text_poke_area));
> > - return 0;
> > -}
> > -
> > -/*
> > - * Run as a late init call. This allows all the boot time patching to be done
> > - * simply by patching the code, and then we're called here prior to
> > - * mark_rodata_ro(), which happens after all init calls are run. Although
> > - * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
> > - * it as being preferable to a kernel that will crash later when someone tries
> > - * to use patch_instruction().
> > - */
> > -static int __init setup_text_poke_area(void)
> > -{
> > - BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > - "powerpc/text_poke:online", text_area_cpu_up,
> > - text_area_cpu_down));
> > -
> > - return 0;
> > -}
> > -late_initcall(setup_text_poke_area);
> > +struct patch_mapping {
> > + spinlock_t *ptl; /* for protecting pte table */
> > + struct temp_mm temp_mm;
> > +};
> >
> > /*
> > * This can be called for kernel text or a module.
> > */
> > -static int map_patch_area(void *addr, unsigned long text_poke_addr)
> > +static int map_patch(const void *addr, struct patch_mapping *patch_mapping)
>
> Why change the name ?
>
It's not really an "area" anymore.
> > {
> > - unsigned long pfn;
> > - int err;
> > + struct page *page;
> > + pte_t pte, *ptep;
> > + pgprot_t pgprot;
> >
> > if (is_vmalloc_addr(addr))
> > - pfn = vmalloc_to_pfn(addr);
> > + page = vmalloc_to_page(addr);
> > else
> > - pfn = __pa_symbol(addr) >> PAGE_SHIFT;
> > + page = virt_to_page(addr);
> >
> > - err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL);
> > + if (radix_enabled())
> > + pgprot = __pgprot(pgprot_val(PAGE_KERNEL));
> > + else
> > + pgprot = PAGE_SHARED;
>
> Can you explain the difference between radix and non radix ?
>
> Why PAGE_KERNEL for a page that is mapped in userspace ?
>
> Why do you need to do __pgprot(pgprot_val(PAGE_KERNEL)) instead of just
> using PAGE_KERNEL ?
>
On hash there is a manual check which prevents setting _PAGE_PRIVILEGED for
kernel to userspace access in __hash_page - hence we cannot access the mapping
if the page is mapped PAGE_KERNEL on hash. However, I would like to use
PAGE_KERNEL here as well and am working on understanding why this check is
done in hash and if this can change. On radix this works just fine.
The page is mapped PAGE_KERNEL because the address is technically a userspace
address - but only to keep the mapping local to this CPU doing the patching.
PAGE_KERNEL makes it clear both in intent and protection that this is a kernel
mapping.
I think the correct way is pgprot_val(PAGE_KERNEL) since PAGE_KERNEL is defined
as:
#define PAGE_KERNEL __pgprot(_PAGE_BASE | _PAGE_KERNEL_RW)
and __pgprot() is defined as:
typedef struct { unsigned long pgprot; } pgprot_t;
#define pgprot_val(x) ((x).pgprot)
#define __pgprot(x) ((pgprot_t) { (x) })
> >
> > - pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err);
> > - if (err)
> > + ptep = get_locked_pte(patching_mm, patching_addr, &patch_mapping->ptl);
> > + if (unlikely(!ptep)) {
> > + pr_warn("map patch: failed to allocate pte for patching\n");
> > return -1;
> > + }
> > +
> > + pte = mk_pte(page, pgprot);
> > + set_pte_at(patching_mm, patching_addr, ptep, pte);
> > +
> > + init_temp_mm(&patch_mapping->temp_mm, patching_mm);
> > + use_temporary_mm(&patch_mapping->temp_mm);
> >
> > return 0;
> > }
> >
> > -static inline int unmap_patch_area(unsigned long addr)
> > +static int unmap_patch(struct patch_mapping *patch_mapping)
> > {
> > pte_t *ptep;
> > pmd_t *pmdp;
> > pud_t *pudp;
> > pgd_t *pgdp;
> >
> > - pgdp = pgd_offset_k(addr);
> > + pgdp = pgd_offset(patching_mm, patching_addr);
> > if (unlikely(!pgdp))
> > return -EINVAL;
> >
> > - pudp = pud_offset(pgdp, addr);
> > + pudp = pud_offset(pgdp, patching_addr);
> > if (unlikely(!pudp))
> > return -EINVAL;
> >
> > - pmdp = pmd_offset(pudp, addr);
> > + pmdp = pmd_offset(pudp, patching_addr);
> > if (unlikely(!pmdp))
> > return -EINVAL;
> >
> > - ptep = pte_offset_kernel(pmdp, addr);
> > + ptep = pte_offset_kernel(pmdp, patching_addr);
>
> ptep should be stored in the patch_mapping struct instead of walking
> again the page tables.
>
Oh yes - this will be in the next version.
> > if (unlikely(!ptep))
> > return -EINVAL;
> >
> > - pr_devel("clearing mm %p, pte %p, addr %lx\n", &init_mm, ptep, addr);
> > + /*
> > + * In hash, pte_clear flushes the tlb
> > + */
> > + pte_clear(patching_mm, patching_addr, ptep);
> > + unuse_temporary_mm(&patch_mapping->temp_mm);
> >
> > /*
> > - * In hash, pte_clear flushes the tlb, in radix, we have to
> > + * In radix, we have to explicitly flush the tlb (no-op in hash)
> > */
> > - pte_clear(&init_mm, addr, ptep);
> > - flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> > + local_flush_tlb_mm(patching_mm);
> > + pte_unmap_unlock(ptep, patch_mapping->ptl);
> >
> > return 0;
> > }
> > @@ -167,33 +148,38 @@ static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > int err;
> > unsigned int *patch_addr = NULL;
> > unsigned long flags;
> > - unsigned long text_poke_addr;
> > - unsigned long kaddr = (unsigned long)addr;
> > + struct patch_mapping patch_mapping;
> >
> > /*
> > - * During early early boot patch_instruction is called
> > - * when text_poke_area is not ready, but we still need
> > - * to allow patching. We just do the plain old patching
> > + * The patching_mm is initialized before calling mark_rodata_ro. Prior
> > + * to this, patch_instruction is called when we don't have (and don't
> > + * need) the patching_mm so just do plain old patching.
> > */
> > - if (!this_cpu_read(text_poke_area))
> > + if (!patching_mm)
> > return raw_patch_instruction(addr, instr);
> >
> > local_irq_save(flags);
> >
> > - text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
> > - if (map_patch_area(addr, text_poke_addr)) {
> > - err = -1;
> > + err = map_patch(addr, &patch_mapping);
> > + if (err)
> > goto out;
> > - }
> >
> > - patch_addr = (unsigned int *)(text_poke_addr) +
> > - ((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
> > + patch_addr = (unsigned int *)(patching_addr) +
> > + (offset_in_page((unsigned long)addr) /
> > + sizeof(unsigned int));
> >
> > __patch_instruction(addr, instr, patch_addr);
>
> The error returned by __patch_instruction() should be managed.
>
Agreed, will do something in the next spin.
> >
> > - err = unmap_patch_area(text_poke_addr);
> > + err = unmap_patch(&patch_mapping);
> > if (err)
> > - pr_warn("failed to unmap %lx\n", text_poke_addr);
> > + pr_warn("unmap patch: failed to unmap patch\n");
> > +
> > + /*
> > + * Something is wrong if what we just wrote doesn't match what we
> > + * think we just wrote.
> > + * XXX: BUG_ON() instead?
>
> No, not a BUG_ON(). If patching fails, that's no a vital fault, we can
> fail gracefully. You should return a fault instead.
>
Yup - will make these changes in the next version.
> > + */
> > + WARN_ON(memcmp(addr, &instr, sizeof(instr)));
>
> Come on. addr is an *int, instr is an int. By doing a memcmp() on
> &instr, you for the compiler to write instr into the stack whereas local
> vars are mainly in registers on RISC processors like powerpc. Following
> should do it:
>
> WARN_ON(*addr != instr);
>
Oh man - I agree, that's just embarrassing.
Appreciate your feedback on this RFC series, thanks!
> >
> > out:
> > local_irq_restore(flags);
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH 1/2] mm, treewide: Rename kzfree() to kfree_sensitive()
From: Johannes Weiner @ 2020-04-15 5:01 UTC (permalink / raw)
To: Waiman Long
Cc: linux-btrfs, Jarkko Sakkinen, virtualization, David Howells,
linux-mm, linux-sctp, keyrings, kasan-dev, samba-technical,
linux-stm32, devel, linux-s390, linux-scsi, x86, James Morris,
Matthew Wilcox, cocci, linux-wpan, intel-wired-lan,
David Rientjes, Serge E. Hallyn, linux-pm, ecryptfs, linux-nfs,
linux-fscrypt, linux-mediatek, linux-amlogic, linux-integrity,
linux-arm-kernel, linux-cifs, Linus Torvalds, linux-wireless,
linux-kernel, linux-bluetooth, linux-security-module,
target-devel, tipc-discussion, linux-crypto, netdev, Joe Perches,
Andrew Morton, linuxppc-dev, wireguard, linux-ppp
In-Reply-To: <20200413211550.8307-2-longman@redhat.com>
On Mon, Apr 13, 2020 at 05:15:49PM -0400, Waiman Long wrote:
> As said by Linus:
>
> A symmetric naming is only helpful if it implies symmetries in use.
> Otherwise it's actively misleading.
As the btrfs example proves - people can be tempted by this false
symmetry to pair kzalloc with kzfree, which isn't what we wanted.
> In "kzalloc()", the z is meaningful and an important part of what the
> caller wants.
>
> In "kzfree()", the z is actively detrimental, because maybe in the
> future we really _might_ want to use that "memfill(0xdeadbeef)" or
> something. The "zero" part of the interface isn't even _relevant_.
>
> The main reason that kzfree() exists is to clear sensitive information
> that should not be leaked to other future users of the same memory
> objects.
>
> Rename kzfree() to kfree_sensitive() to follow the example of the
> recently added kvfree_sensitive() and make the intention of the API
> more explicit. In addition, memzero_explicit() is used to clear the
> memory to make sure that it won't get optimized away by the compiler.
>
> The renaming is done by using the command sequence:
>
> git grep -w --name-only kzfree |\
> xargs sed -i 's/\bkzfree\b/kfree_sensitive/'
>
> followed by some editing of the kfree_sensitive() kerneldoc and the
> use of memzero_explicit() instead of memset().
>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Waiman Long <longman@redhat.com>
Looks good to me. Thanks for fixing this very old mistake.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply
* [PATCH] ASoC: fsl_micfil: Omit superfluous error message in fsl_micfil_probe()
From: Tang Bin @ 2020-04-15 4:45 UTC (permalink / raw)
To: broonie, timur, nicoleotsuka, Xiubo.Lee, perex, tiwai
Cc: alsa-devel, Shengju Zhang, linuxppc-dev, linux-kernel, Tang Bin
In the function fsl_micfil_probe(), when get irq failed, the function
platform_get_irq() logs an error message, so remove redundant message here.
Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
Signed-off-by: Shengju Zhang <zhangshengju@cmss.chinamobile.com>
---
sound/soc/fsl/fsl_micfil.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index f7f2d29f1..e73bd6570 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -702,10 +702,8 @@ static int fsl_micfil_probe(struct platform_device *pdev)
for (i = 0; i < MICFIL_IRQ_LINES; i++) {
micfil->irq[i] = platform_get_irq(pdev, i);
dev_err(&pdev->dev, "GET IRQ: %d\n", micfil->irq[i]);
- if (micfil->irq[i] < 0) {
- dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+ if (micfil->irq[i] < 0)
return micfil->irq[i];
- }
}
if (of_property_read_bool(np, "fsl,shared-interrupt"))
--
2.20.1.windows.1
^ permalink raw reply related
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-04-15 4:40 UTC (permalink / raw)
To: Balamuruhan S
Cc: Alistair Popple, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <435c17d3180d560a723efcae42de351ae7152cb3.camel@linux.ibm.com>
On Mon, Apr 13, 2020 at 10:04 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > For powerpc64, redefine the ppc_inst type so both word and prefixed
> > instructions can be represented. On powerpc32 the type will remain the
> > same. Update places which had assumed instructions to be 4 bytes long.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > v5: - Distinguish normal instructions from prefixed instructions with a
> > 0xff marker for the suffix.
> > - __patch_instruction() using std for prefixed instructions
> > ---
> > arch/powerpc/include/asm/inst.h | 71 ++++++++++++++++++++++++++--
> > arch/powerpc/include/asm/kprobes.h | 2 +-
> > arch/powerpc/include/asm/uaccess.h | 31 ++++++++++--
> > arch/powerpc/include/asm/uprobes.h | 2 +-
> > arch/powerpc/kernel/optprobes.c | 42 ++++++++--------
> > arch/powerpc/kernel/optprobes_head.S | 3 ++
> > arch/powerpc/kernel/trace/ftrace.c | 26 +++++++++-
> > arch/powerpc/lib/code-patching.c | 19 +++++---
> > arch/powerpc/lib/feature-fixups.c | 5 +-
> > arch/powerpc/lib/sstep.c | 4 +-
> > arch/powerpc/xmon/xmon.c | 6 +--
> > arch/powerpc/xmon/xmon_bpts.S | 4 +-
> > 12 files changed, 171 insertions(+), 44 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > index 70b37a35a91a..7e23e7146c66 100644
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -8,23 +8,67 @@
> >
> > struct ppc_inst {
> > u32 val;
> > +#ifdef __powerpc64__
> > + u32 suffix;
> > +#endif /* __powerpc64__ */
> > } __packed;
> >
> > -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > +static inline int ppc_inst_opcode(struct ppc_inst x)
> > +{
> > + return x.val >> 26;
>
>
> why don't we wrap here and in `ppc_inst_opcode()` in patch 9 using
> `ppc_inst_val()` ?
Will do.
>
>
> > +}
> >
> > static inline u32 ppc_inst_val(struct ppc_inst x)
>
>
> There is another same definition below for the same function in
> #else part of __powerpc64__ ifdef.
Thanks
>
>
> > {
> > return x.val;
> > }
> >
> > -static inline bool ppc_inst_len(struct ppc_inst x)
> > +#ifdef __powerpc64__
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> > +
> > +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y)
> > })
> > +
> > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > {
> > - return sizeof(struct ppc_inst);
> > + return x.suffix;
> > }
> >
> > -static inline int ppc_inst_opcode(struct ppc_inst x)
> > +static inline bool ppc_inst_prefixed(struct ppc_inst x) {
> > + return ((ppc_inst_val(x) >> 26) == 1) && ppc_inst_suffix(x) != 0xff;
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > {
> > - return x.val >> 26;
> > + return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> > + swab32(ppc_inst_suffix(x)));
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > +{
> > + u32 val, suffix = 0xff;
> > + val = *(u32 *)ptr;
> > + if ((val >> 26) == 1)
> > + suffix = *((u32 *)ptr + 1);
> > + return ppc_inst_prefix(val, suffix);
> > +}
> > +
> > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst x)
> > +{
> > + if (ppc_inst_prefixed(x)) {
> > + *(u32 *)ptr = x.val;
> > + *((u32 *)ptr + 1) = x.suffix;
> > + } else {
> > + *(u32 *)ptr = x.val;
>
>
> can we wrap here as well with `ppc_inst_val()` and `ppc_inst_suffix()` ?
Yeah no reason not too.
>
>
> > + }
> > +}
> > +
> > +#else
> > +
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > +
> > +static inline bool ppc_inst_prefixed(ppc_inst x)
> > +{
> > + return 0;
>
>
> Is it return !!0 or return false ?
False probably will make more sense.
>
>
> > }
> >
> > static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > @@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
> > ppc_inst x)
> > return ppc_inst(swab32(ppc_inst_val(x)));
> > }
> >
> > +static inline u32 ppc_inst_val(struct ppc_inst x)
>
>
> [...] duplicate definition that is defined outside __powerpc64__ above.
>
>
> > +{
> > + return x.val;
> > +}
> > +
> > static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > {
> > return *ptr;
> > }
> >
> > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst x)
> > +{
> > + *ptr = x;
> > +}
> > +
> > +#endif /* __powerpc64__ */
> > +
> > static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> > {
> > return !memcmp(&x, &y, sizeof(struct ppc_inst));
> > }
> >
> > +static inline int ppc_inst_len(struct ppc_inst x)
> > +{
> > + return (ppc_inst_prefixed(x)) ? 8 : 4;
> > +}
> > +
> > #endif /* _ASM_INST_H */
> > diff --git a/arch/powerpc/include/asm/kprobes.h
> > b/arch/powerpc/include/asm/kprobes.h
> > index 66b3f2983b22..4fc0e15e23a5 100644
> > --- a/arch/powerpc/include/asm/kprobes.h
> > +++ b/arch/powerpc/include/asm/kprobes.h
> > @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> > extern kprobe_opcode_t optprobe_template_end[];
> >
> > /* Fixed instruction size for powerpc */
> > -#define MAX_INSN_SIZE 1
> > +#define MAX_INSN_SIZE 2
> > #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> > #define MAX_OPTINSN_SIZE (optprobe_template_end -
> > optprobe_template_entry)
> > #define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
> > diff --git a/arch/powerpc/include/asm/uaccess.h
> > b/arch/powerpc/include/asm/uaccess.h
> > index c0a35e4586a5..5a3f486ddf02 100644
> > --- a/arch/powerpc/include/asm/uaccess.h
> > +++ b/arch/powerpc/include/asm/uaccess.h
> > @@ -105,11 +105,34 @@ static inline int __access_ok(unsigned long addr,
> > unsigned long size,
> > #define __put_user_inatomic(x, ptr) \
> > __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> >
> > -#define __get_user_instr(x, ptr) \
> > - __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> > +#define __get_user_instr(x, ptr) \
> > +({ \
> > + long __gui_ret = 0; \
> > + unsigned int prefix, suffix; \
> > + __gui_ret = __get_user(prefix, (unsigned int __user *)ptr);
> > \
> > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > + __gui_ret = __get_user(suffix, (unsigned int __user *)ptr + 1);
> > \
> > + (x) = ppc_inst_prefix(prefix, suffix); \
> > + } else { \
> > + (x) = ppc_inst(prefix); \
> > + } \
> > + __gui_ret; \
> > +})
> > +
> > +#define __get_user_instr_inatomic(x, ptr) \
> > +({ \
> > + long __gui_ret = 0; \
> > + unsigned int prefix, suffix; \
> > + __gui_ret = __get_user_inatomic(prefix, (unsigned int __user *)ptr);
> > \
> > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > + __gui_ret = __get_user_inatomic(suffix, (unsigned int __user
> > *)ptr + 1); \
> > + (x) = ppc_inst_prefix(prefix, suffix); \
> > + } else { \
> > + (x) = ppc_inst(prefix); \
> > + } \
> > + __gui_ret; \
> > +})
> >
> > -#define __get_user_instr_inatomic(x, ptr) \
> > - __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
> > extern long __put_user_bad(void);
> >
> > /*
> > diff --git a/arch/powerpc/include/asm/uprobes.h
> > b/arch/powerpc/include/asm/uprobes.h
> > index 7e3b329ba2d3..5bf65f5d44a9 100644
> > --- a/arch/powerpc/include/asm/uprobes.h
> > +++ b/arch/powerpc/include/asm/uprobes.h
> > @@ -15,7 +15,7 @@
> >
> > typedef ppc_opcode_t uprobe_opcode_t;
> >
> > -#define MAX_UINSN_BYTES 4
> > +#define MAX_UINSN_BYTES 8
> > #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
> >
> > /* The following alias is needed for reference from arch-agnostic code */
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c
> > index 684640b8fa2e..689daf430161 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -159,38 +159,38 @@ void patch_imm32_load_insns(unsigned int val,
> > kprobe_opcode_t *addr)
> >
> > /*
> > * Generate instructions to load provided immediate 64-bit value
> > - * to register 'r3' and patch these instructions at 'addr'.
> > + * to register 'reg' and patch these instructions at 'addr'.
> > */
> > -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> > +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t
> > *addr)
> > {
> > - /* lis r3,(op)@highest */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS |
> > ___PPC_RT(3) |
> > + /* lis reg,(op)@highest */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS |
> > ___PPC_RT(reg) |
> > ((val >> 48) & 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@higher */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 32) & 0xffff)));
> > + /* ori reg,reg,(op)@higher */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
> > addr++;
> >
> > - /* rldicr r3,r3,32,31 */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR |
> > ___PPC_RA(3) |
> > - ___PPC_RS(3) | __PPC_SH64(32) | __PPC_ME64(31)));
> > + /* rldicr reg,reg,32,31 */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_RLDICR |
> > ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
> > addr++;
> >
> > - /* oris r3,r3,(op)@h */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > ___PPC_RA(3) |
> > - ___PPC_RS(3) | ((val >> 16) & 0xffff)));
> > + /* oris reg,reg,(op)@h */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@l */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(3) |
> > - ___PPC_RS(3) | (val & 0xffff)));
> > + /* ori reg,reg,(op)@l */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(reg) |
> > + ___PPC_RS(reg) | (val & 0xffff)));
> > }
> >
> > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe
> > *p)
> > {
> > - struct ppc_inst branch_op_callback, branch_emulate_step;
> > + struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> > kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > long b_offset;
> > unsigned long nip, size;
> > @@ -240,7 +240,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe
> > *op, struct kprobe *p)
> > * Fixup the template with instructions to:
> > * 1. load the address of the actual probepoint
> > */
> > - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> > + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> >
> > /*
> > * 2. branch to optimized_callback() and emulate_step()
> > @@ -271,7 +271,11 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p)
> > /*
> > * 3. load instruction to be emulated into relevant register, and
> > */
> > - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
> > + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> > + patch_imm64_load_insns(ppc_inst_val(temp) |
> > + ((u64)ppc_inst_suffix(temp) << 32),
>
>
> did we check building for ppc32 ?
Yeah I need to do so.
>
> I doubt we might hit build failure as `ppc_inst_suffix()` macro is not defined.
>
>
> > + 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 e78742613b36..16041a5c86d5 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -41,11 +41,35 @@
> > #define NUM_FTRACE_TRAMPS 8
> > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> >
> > +#ifdef __powerpc64__
> > static long
> > probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > {
> > - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> > + u32 val, suffix = 0;
>
>
> don't we need to initialize suffix with 0xff ?
Good catch, that was causing problems.
>
>
> > + long err;
> > +
> > + err = probe_kernel_read((void *)&val,
> > + src, sizeof(val));
> > + if (err)
> > + return err;
> > +
> > + if ((val >> 26) == 1)
> > + err = probe_kernel_read((void *)&suffix,
> > + src + 4, MCOUNT_INSN_SIZE);
> > + if (err)
> > + return err;
> > +
> > + *inst = ppc_inst_prefix(val, suffix);
> > +
> > + return 0;
> > }
> > +#else
> > +static long
> > +probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > +{
> > + return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE)
> > +}
> > +#endif
> >
> > static struct ppc_inst
> > ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > patching.c
> > index c329ad657302..b4007e03d8fa 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -24,12 +24,19 @@ static int __patch_instruction(struct ppc_inst
> > *exec_addr, struct ppc_inst instr
> > {
> > int err = 0;
> >
> > - __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> > - if (err)
> > - return err;
> > -
> > - asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
> > - "r" (exec_addr));
> > + if (!ppc_inst_prefixed(instr)) {
> > + __put_user_asm(ppc_inst_val(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));
> > + } else {
> > + __put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
> > ppc_inst_val(instr), patch_addr, err, "std");
> > + if (err)
> > + return err;
> > + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > (patch_addr),
> > + "r"
> > (exec_addr));
> > + }
>
>
> can we keep these 2 lines out of conditions as it remains to be the same ?
True.
>
> if (err)
> return err;
> asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr), "r"
> (exec_addr));
>
>
> >
> > return 0;
> > }
> > diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-
> > fixups.c
> > index f00dd13b1c3c..5519cec83cc8 100644
> > --- a/arch/powerpc/lib/feature-fixups.c
> > +++ b/arch/powerpc/lib/feature-fixups.c
> > @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value,
> > struct fixup_entry *fcur)
> > src = alt_start;
> > dest = start;
> >
> > - for (; src < alt_end; src++, dest++) {
> > + for (; src < alt_end; src = (void *)src +
> > ppc_inst_len(ppc_inst_read(src)),
> > + (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
> > if (patch_alt_instruction(src, dest, alt_start, alt_end))
> > return 1;
> > }
> >
> > - for (; dest < end; dest++)
> > + for (; dest < end; dest = (void *)dest +
> > ppc_inst_len(ppc_inst(PPC_INST_NOP)))
> > raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
> >
> > return 0;
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index 52ddd3122dc8..8b285bf11218 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> > unsigned long int imm;
> > unsigned long int val, val2;
> > unsigned int mb, me, sh;
> > - unsigned int word;
> > + unsigned int word, suffix;
> > long ival;
> >
> > word = ppc_inst_val(instr);
> > + suffix = ppc_inst_suffix(instr);
>
>
> same here, I doubt it might break for ppc32.
Yeah I need to work on ppc32.
>
>
> -- Bala
> > +
> > op->type = COMPUTE;
> >
> > opcode = word >> 26;
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 6f3bcdcfc9c7..b704aebb099a 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -761,8 +761,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;
> > }
> > @@ -863,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long nip,
> > unsigned long *offp)
> > if (off >= sizeof(bpt_table))
> > return NULL;
> > *offp = off % BPT_SIZE;
> > - if (*offp != 0 && *offp != 4)
> > + if (*offp != 0 && *offp != 4 && *offp != 8)
> > return NULL;
> > return bpts + (off / BPT_SIZE);
> > }
> > diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> > index ebb2dbc70ca8..09058eb6abbd 100644
> > --- a/arch/powerpc/xmon/xmon_bpts.S
> > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > @@ -3,6 +3,8 @@
> > #include <asm/asm-compat.h>
> > #include "xmon_bpts.h"
> >
> > +/* Prefixed instructions can not cross 64 byte boundaries */
> > +.align 6
> > .global bpt_table
> > bpt_table:
> > - .space NBPTS * 8
> > + .space NBPTS * 16
>
^ permalink raw reply
* Re: [RFC PATCH 2/3] powerpc/lib: Initialize a temporary mm for code patching
From: Christopher M Riedl @ 2020-04-15 4:39 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <8ff6d279-fdd9-4e4d-b4e0-f5c5cba480a4@c-s.fr>
> On April 8, 2020 6:01 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
> Le 31/03/2020 à 05:19, Christopher M Riedl a écrit :
> >> On March 24, 2020 11:10 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> >>
> >>
> >> Le 23/03/2020 à 05:52, Christopher M. Riedl a écrit :
> >>> When code patching a STRICT_KERNEL_RWX kernel the page containing the
> >>> address to be patched is temporarily mapped with permissive memory
> >>> protections. Currently, a per-cpu vmalloc patch area is used for this
> >>> purpose. While the patch area is per-cpu, the temporary page mapping is
> >>> inserted into the kernel page tables for the duration of the patching.
> >>> The mapping is exposed to CPUs other than the patching CPU - this is
> >>> undesirable from a hardening perspective.
> >>>
> >>> Use the `poking_init` init hook to prepare a temporary mm and patching
> >>> address. Initialize the temporary mm by copying the init mm. Choose a
> >>> randomized patching address inside the temporary mm userspace address
> >>> portion. The next patch uses the temporary mm and patching address for
> >>> code patching.
> >>>
> >>> Based on x86 implementation:
> >>>
> >>> commit 4fc19708b165
> >>> ("x86/alternatives: Initialize temporary mm for patching")
> >>>
> >>> Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf>
> >>> ---
> >>> arch/powerpc/lib/code-patching.c | 26 ++++++++++++++++++++++++++
> >>> 1 file changed, 26 insertions(+)
> >>>
> >>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> >>> index 3345f039a876..18b88ecfc5a8 100644
> >>> --- a/arch/powerpc/lib/code-patching.c
> >>> +++ b/arch/powerpc/lib/code-patching.c
> >>> @@ -11,6 +11,8 @@
> >>> #include <linux/cpuhotplug.h>
> >>> #include <linux/slab.h>
> >>> #include <linux/uaccess.h>
> >>> +#include <linux/sched/task.h>
> >>> +#include <linux/random.h>
> >>>
> >>> #include <asm/pgtable.h>
> >>> #include <asm/tlbflush.h>
> >>> @@ -39,6 +41,30 @@ int raw_patch_instruction(unsigned int *addr, unsigned int instr)
> >>> }
> >>>
> >>> #ifdef CONFIG_STRICT_KERNEL_RWX
> >>> +
> >>> +__ro_after_init struct mm_struct *patching_mm;
> >>> +__ro_after_init unsigned long patching_addr;
> >>
> >> Can we make those those static ?
> >>
> >
> > Yes, makes sense to me.
> >
> >>> +
> >>> +void __init poking_init(void)
> >>> +{
> >>> + spinlock_t *ptl; /* for protecting pte table */
> >>> + pte_t *ptep;
> >>> +
> >>> + patching_mm = copy_init_mm();
> >>> + BUG_ON(!patching_mm);
> >>
> >> Does it needs to be a BUG_ON() ? Can't we fail gracefully with just a
> >> WARN_ON ?
> >>
> >
> > I'm not sure what failing gracefully means here? The main reason this could
> > fail is if there is not enough memory to allocate the patching_mm. The
> > previous implementation had this justification for BUG_ON():
>
> But the system can continue running just fine after this failure.
> Only the things that make use of code patching will fail (ftrace, kgdb, ...)
>
> Checkpatch tells: "Avoid crashing the kernel - try using WARN_ON &
> recovery code rather than BUG() or BUG_ON()"
>
> All vital code patching has already been done previously, so I think a
> WARN_ON() should be enough, plus returning non 0 to indicate that the
> late_initcall failed.
>
>
Got it, makes sense to me. I will make these changes in the next version.
Thanks!
> >
> > /*
> > * Run as a late init call. This allows all the boot time patching to be done
> > * simply by patching the code, and then we're called here prior to
> > * mark_rodata_ro(), which happens after all init calls are run. Although
> > * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
> > * it as being preferable to a kernel that will crash later when someone tries
> > * to use patch_instruction().
> > */
> > static int __init setup_text_poke_area(void)
> > {
> > BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > "powerpc/text_poke:online", text_area_cpu_up,
> > text_area_cpu_down));
> >
> > return 0;
> > }
> > late_initcall(setup_text_poke_area);
> >
> > I think the BUG_ON() is appropriate even if only to adhere to the previous
> > judgement call. I can add a similar comment explaining the reasoning if
> > that helps.
> >
> >>> +
> >>> + /*
> >>> + * In hash we cannot go above DEFAULT_MAP_WINDOW easily.
> >>> + * XXX: Do we want additional bits of entropy for radix?
> >>> + */
> >>> + patching_addr = (get_random_long() & PAGE_MASK) %
> >>> + (DEFAULT_MAP_WINDOW - PAGE_SIZE);
> >>> +
> >>> + ptep = get_locked_pte(patching_mm, patching_addr, &ptl);
> >>> + BUG_ON(!ptep);
> >>
> >> Same here, can we fail gracefully instead ?
> >>
> >
> > Same reasoning as above.
>
> Here as well, a WARN_ON() should be enough, the system will continue
> running after that.
>
> >
> >>> + pte_unmap_unlock(ptep, ptl);
> >>> +}
> >>> +
> >>> static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
> >>>
> >>> static int text_area_cpu_up(unsigned int cpu)
> >>>
> >>
> >> Christophe
>
> Christophe
^ permalink raw reply
* Re: [PATCH 4/8] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Michael Ellerman @ 2020-04-15 3:01 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200414070142.288696-5-hch@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> Instead of messing with the address limit just open code the trivial
> memcpy + memset logic for the native version, and a call to
> to_compat_siginfo for the compat version.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/binfmt_elf.c | 9 +++++----
> fs/compat_binfmt_elf.c | 6 +++++-
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 13f25e241ac4..607c5a5f855e 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1553,15 +1553,16 @@ static void fill_auxv_note(struct memelfnote *note, struct mm_struct *mm)
> fill_note(note, "CORE", NT_AUXV, i * sizeof(elf_addr_t), auxv);
> }
>
> +#ifndef fill_siginfo_note
> static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
> const kernel_siginfo_t *siginfo)
> {
> - mm_segment_t old_fs = get_fs();
> - set_fs(KERNEL_DS);
> - copy_siginfo_to_user((user_siginfo_t __user *) csigdata, siginfo);
> - set_fs(old_fs);
> + memcpy(csigdata, siginfo, sizeof(struct kernel_siginfo));
> + memset((char *)csigdata + sizeof(struct kernel_siginfo), 0,
> + SI_EXPANSION_SIZE);
> fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
> }
> +#endif
>
> #define MAX_FILE_NOTE_SIZE (4*1024*1024)
> /*
> diff --git a/fs/compat_binfmt_elf.c b/fs/compat_binfmt_elf.c
> index aaad4ca1217e..ab84e095618b 100644
> --- a/fs/compat_binfmt_elf.c
> +++ b/fs/compat_binfmt_elf.c
> @@ -39,7 +39,11 @@
> */
> #define user_long_t compat_long_t
> #define user_siginfo_t compat_siginfo_t
> -#define copy_siginfo_to_user copy_siginfo_to_user32
> +#define fill_siginfo_note(note, csigdata, siginfo) \
> +do { \
> + to_compat_siginfo(csigdata, siginfo, compat_siginfo_flags()); \
> + fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata); \
> +} while (0)
This doesn't build on ppc (cell_defconfig):
../fs/binfmt_elf.c: In function 'fill_note_info':
../fs/compat_binfmt_elf.c:44:39: error: implicit declaration of function 'compat_siginfo_flags'; did you mean 'to_compat_siginfo'? [-Werror=implicit-function-d
eclaration]
to_compat_siginfo(csigdata, siginfo, compat_siginfo_flags()); \
^~~~~~~~~~~~~~~~~~~~
../fs/binfmt_elf.c:1846:2: note: in expansion of macro 'fill_siginfo_note'
fill_siginfo_note(&info->signote, &info->csigdata, siginfo);
^~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [../scripts/Makefile.build:266: fs/compat_binfmt_elf.o] Error 1
I guess the empty version from kernel/signal.c needs to move into a
header somewhere.
cheers
^ permalink raw reply
* [PATCH kernel] powerpc/dma: Call indirect dma_ops when persistent memory present
From: Alexey Kardashevskiy @ 2020-04-15 2:58 UTC (permalink / raw)
To: linuxppc-dev
Cc: Brian J King, Alexey Kardashevskiy, Sam Bobroff,
Aneesh Kumar K . V, Wen Xiong, Christoph Hellwig, David Gibson
Unlike normal memory ("memory" compatible type in the FDT),
the persistent memory ("ibm,pmemory" in the FDT) can be mapped anywhere
in the guest physical space and it can be used for DMA.
In order to maintain 1:1 mapping via the huge DMA window, we need to
know the maximum physical address at the time of the window setup.
So far we've been looking at "memory" nodes but "ibm,pmemory" does not
have fixed addresses and the persistent memory may be mapped afterwards.
When the maximum window size is not big enough to fit persistent memory,
this clears the dma_ops_bypass flag to tell the generic code that indirect
dma_ops call is needed. This lets the platform code check the DMA
boundaries and call direct DMA API if DMA-ing to/from generic RAM
or call IOMMU API otherwise.
This adds dma_max to device::archdata to tell the direct DMA mapping
limit. At the moment only pseries sets the limit so powernv is
unaffected by this change.
As persistent memory is backed with page structs, this uses
MAX_PHYSMEM_BITS as the upper limit (rather than simple 64bit).
This should not change the existing behaviour when no persistent memory.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
This is based on
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=170152
---
arch/powerpc/include/asm/device.h | 2 +
arch/powerpc/kernel/dma-iommu.c | 68 +++++++++++++++++++++++++-
arch/powerpc/platforms/pseries/iommu.c | 23 ++++++++-
3 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 452402215e12..380e92684a16 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -18,6 +18,8 @@ struct iommu_table;
* drivers/macintosh/macio_asic.c
*/
struct dev_archdata {
+ /* Maximum DMA address for 1:1 mapping (when enabled) */
+ dma_addr_t dma_max;
/*
* These two used to be a union. However, with the hybrid ops we need
* both so here we store both a DMA offset for direct mappings and
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 569fecd7b5b2..8c67bfffdef6 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -10,6 +10,16 @@
#include <linux/pci.h>
#include <asm/iommu.h>
+static inline bool can_map_direct(struct device *dev, phys_addr_t addr)
+{
+ return dev->archdata.dma_max >= phys_to_dma(dev, addr);
+}
+
+static inline bool dma_handle_direct(struct device *dev, dma_addr_t dma_handle)
+{
+ return dma_handle >= dev->archdata.dma_offset;
+}
+
/*
* Generic iommu implementation
*/
@@ -44,6 +54,12 @@ static dma_addr_t dma_iommu_map_page(struct device *dev, struct page *page,
enum dma_data_direction direction,
unsigned long attrs)
{
+ if (dev->archdata.dma_max &&
+ can_map_direct(dev, (phys_addr_t) page_to_phys(page) +
+ offset + size))
+ return dma_direct_map_page(dev, page, offset, size, direction,
+ attrs);
+
return iommu_map_page(dev, get_iommu_table_base(dev), page, offset,
size, dma_get_mask(dev), direction, attrs);
}
@@ -53,6 +69,12 @@ static void dma_iommu_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction direction,
unsigned long attrs)
{
+ if (dev->archdata.dma_max &&
+ dma_handle_direct(dev, dma_handle + size)) {
+ dma_direct_unmap_page(dev, dma_handle, size, direction, attrs);
+ return;
+ }
+
iommu_unmap_page(get_iommu_table_base(dev), dma_handle, size, direction,
attrs);
}
@@ -62,6 +84,22 @@ static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
+ if (dev->archdata.dma_max) {
+ struct scatterlist *s;
+ bool direct = true;
+ int i;
+
+ for_each_sg(sglist, s, nelems, i) {
+ direct = can_map_direct(dev,
+ sg_phys(s) + s->offset + s->length);
+ if (!direct)
+ break;
+ }
+ if (direct)
+ return dma_direct_map_sg(dev, sglist, nelems, direction,
+ attrs);
+ }
+
return ppc_iommu_map_sg(dev, get_iommu_table_base(dev), sglist, nelems,
dma_get_mask(dev), direction, attrs);
}
@@ -70,6 +108,24 @@ static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
+ if (dev->archdata.dma_max) {
+ struct scatterlist *s;
+ bool direct = true;
+ int i;
+
+ for_each_sg(sglist, s, nelems, i) {
+ direct = dma_handle_direct(dev,
+ s->dma_address + s->length);
+ if (!direct)
+ break;
+ }
+ if (direct) {
+ dma_direct_unmap_sg(dev, sglist, nelems, direction,
+ attrs);
+ return;
+ }
+ }
+
ppc_iommu_unmap_sg(get_iommu_table_base(dev), sglist, nelems,
direction, attrs);
}
@@ -90,8 +146,16 @@ int dma_iommu_dma_supported(struct device *dev, u64 mask)
struct iommu_table *tbl = get_iommu_table_base(dev);
if (dev_is_pci(dev) && dma_iommu_bypass_supported(dev, mask)) {
- dev->dma_ops_bypass = true;
- dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
+ /*
+ * dma_iommu_bypass_supported() sets dma_max when there is
+ * 1:1 mapping but it is somehow limited.
+ * ibm,pmemory is one example.
+ */
+ dev->dma_ops_bypass = dev->archdata.dma_max == 0;
+ if (!dev->dma_ops_bypass)
+ dev_warn(dev, "iommu: 64-bit OK but using default ops\n");
+ else
+ dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
return 1;
}
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 6d47b4a3ce39..227055a4fbe2 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -994,7 +994,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
struct ddw_query_response query;
struct ddw_create_response create;
int page_shift;
- u64 dma_addr, max_addr;
+ u64 dma_addr, max_addr = 0, dma_max_sz = 0;
struct device_node *dn;
u32 ddw_avail[3];
struct direct_window *window;
@@ -1066,7 +1066,23 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
}
/* verify the window * number of ptes will map the partition */
/* check largest block * page size > max memory hotplug addr */
- max_addr = ddw_memory_hotplug_max();
+ /*
+ * The "ibm,pmemory" can appear anywhere in the address space.
+ * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS
+ * for the upper limit and fallback to max RAM otherwise but this
+ * disables device::dma_ops_bypass.
+ */
+ if (of_find_node_by_type(NULL, "ibm,pmemory")) {
+ max_addr = 1ULL << (MAX_PHYSMEM_BITS - page_shift);
+ if (query.largest_available_block < max_addr) {
+ dev_info(&dev->dev, "Skipping ibm,pmemory");
+ max_addr = ddw_memory_hotplug_max();
+ dma_max_sz = max_addr;
+ }
+ } else {
+ max_addr = ddw_memory_hotplug_max();
+ }
+
if (query.largest_available_block < (max_addr >> page_shift)) {
dev_dbg(&dev->dev, "can't map partition max 0x%llx with %u "
"%llu-sized pages\n", max_addr, query.largest_available_block,
@@ -1151,6 +1167,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
out_unlock:
mutex_unlock(&direct_window_init_mutex);
+ if (!dev->dev.archdata.dma_max && dma_addr && dma_max_sz)
+ dev->dev.archdata.dma_max = dma_addr + dma_max_sz;
+
return dma_addr;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v6 6/7] ASoC: dt-bindings: fsl_easrc: Add document for EASRC
From: Shengjiu Wang @ 2020-04-15 2:55 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Nicolin Chen, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <20200414154643.GA29098@bogus>
Hi Rob
On Tue, Apr 14, 2020 at 11:49 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 01, 2020 at 04:45:39PM +0800, Shengjiu Wang wrote:
> > EASRC (Enhanced Asynchronous Sample Rate Converter) is a new
> > IP module found on i.MX8MN.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > .../devicetree/bindings/sound/fsl,easrc.yaml | 101 ++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > new file mode 100644
> > index 000000000000..14ea60084420
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> > @@ -0,0 +1,101 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,easrc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP Asynchronous Sample Rate Converter (ASRC) Controller
> > +
> > +maintainers:
> > + - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^easrc@.*"
> > +
> > + compatible:
> > + const: fsl,imx8mn-easrc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: Peripheral clock
> > +
> > + clock-names:
> > + items:
> > + - const: mem
> > +
> > + dmas:
> > + maxItems: 8
> > +
> > + dma-names:
> > + items:
> > + - const: ctx0_rx
> > + - const: ctx0_tx
> > + - const: ctx1_rx
> > + - const: ctx1_tx
> > + - const: ctx2_rx
> > + - const: ctx2_tx
> > + - const: ctx3_rx
> > + - const: ctx3_tx
> > +
> > + firmware-name:
> > + allOf:
> > + - $ref: /schemas/types.yaml#/definitions/string
> > + - const: imx/easrc/easrc-imx8mn.bin
> > + description: The coefficient table for the filters
> > +
> > + fsl,asrc-rate:
>
> fsl,asrc-rate-hz
Can we keep "fsl,asrc-rate", because I want this property
align with the one in fsl,asrc.txt. These two asrc modules
can share same property name.
best regards
wang shengjiu
^ permalink raw reply
* [PATCH V2] vhost: do not enable VHOST_MENU by default
From: Jason Wang @ 2020-04-15 2:43 UTC (permalink / raw)
To: jasowang, mst
Cc: linux-s390, tsbogend, gor, kvm, linux-kernel, heiko.carstens,
linux-mips, virtualization, borntraeger, geert, netdev, paulus,
linuxppc-dev
We try to keep the defconfig untouched after decoupling CONFIG_VHOST
out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a
("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by
default. Then the defconfigs can keep enabling CONFIG_VHOST_NET
without the caring of CONFIG_VHOST.
But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even
for the ones that doesn't want vhost. So it actually shifts the
burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is
not set". So this patch tries to enable CONFIG_VHOST explicitly in
defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK.
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Change since V1:
- depends on EVENTFD for VHOST
---
arch/mips/configs/malta_kvm_defconfig | 1 +
arch/powerpc/configs/powernv_defconfig | 1 +
arch/powerpc/configs/ppc64_defconfig | 1 +
arch/powerpc/configs/pseries_defconfig | 1 +
arch/s390/configs/debug_defconfig | 1 +
arch/s390/configs/defconfig | 1 +
drivers/vhost/Kconfig | 26 +++++++++-----------------
7 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/arch/mips/configs/malta_kvm_defconfig b/arch/mips/configs/malta_kvm_defconfig
index 8ef612552a19..06f0c7a0ca87 100644
--- a/arch/mips/configs/malta_kvm_defconfig
+++ b/arch/mips/configs/malta_kvm_defconfig
@@ -18,6 +18,7 @@ CONFIG_PCI=y
CONFIG_VIRTUALIZATION=y
CONFIG_KVM=m
CONFIG_KVM_MIPS_DEBUG_COP0_COUNTERS=y
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index 71749377d164..404245b4594d 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -346,5 +346,6 @@ CONFIG_CRYPTO_DEV_VMX=y
CONFIG_VIRTUALIZATION=y
CONFIG_KVM_BOOK3S_64=m
CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_PRINTK_TIME=y
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index 7e68cb222c7b..4599fc7be285 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -61,6 +61,7 @@ CONFIG_ELECTRA_CF=y
CONFIG_VIRTUALIZATION=y
CONFIG_KVM_BOOK3S_64=m
CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_OPROFILE=m
CONFIG_KPROBES=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index 6b68109e248f..4cad3901b5de 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -321,5 +321,6 @@ CONFIG_CRYPTO_DEV_VMX=y
CONFIG_VIRTUALIZATION=y
CONFIG_KVM_BOOK3S_64=m
CONFIG_KVM_BOOK3S_64_HV=m
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_PRINTK_TIME=y
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index 0c86ba19fa2b..6ec6e69630d1 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y
CONFIG_CMM=m
CONFIG_APPLDATA_BASE=y
CONFIG_KVM=m
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_VHOST_VSOCK=m
CONFIG_OPROFILE=m
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 6b27d861a9a3..d1b3bf83d687 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y
CONFIG_CMM=m
CONFIG_APPLDATA_BASE=y
CONFIG_KVM=m
+CONFIG_VHOST=m
CONFIG_VHOST_NET=m
CONFIG_VHOST_VSOCK=m
CONFIG_OPROFILE=m
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index e79cbbdfea45..29f171a53d8a 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -12,23 +12,19 @@ config VHOST_RING
This option is selected by any driver which needs to access
the host side of a virtio ring.
-config VHOST
- tristate
+menuconfig VHOST
+ tristate "Vhost Devices"
+ depends on EVENTFD
select VHOST_IOTLB
help
- This option is selected by any driver which needs to access
- the core of vhost.
+ Enable option to support host kernel or hardware accelerator
+ for virtio device.
-menuconfig VHOST_MENU
- bool "VHOST drivers"
- default y
-
-if VHOST_MENU
+if VHOST
config VHOST_NET
tristate "Host kernel accelerator for virtio net"
- depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP)
- select VHOST
+ depends on NET && (TUN || !TUN) && (TAP || !TAP)
---help---
This kernel module can be loaded in host kernel to accelerate
guest networking with virtio_net. Not to be confused with virtio_net
@@ -39,8 +35,7 @@ config VHOST_NET
config VHOST_SCSI
tristate "VHOST_SCSI TCM fabric driver"
- depends on TARGET_CORE && EVENTFD
- select VHOST
+ depends on TARGET_CORE
default n
---help---
Say M here to enable the vhost_scsi TCM fabric module
@@ -48,8 +43,7 @@ config VHOST_SCSI
config VHOST_VSOCK
tristate "vhost virtio-vsock driver"
- depends on VSOCKETS && EVENTFD
- select VHOST
+ depends on VSOCKETS
select VIRTIO_VSOCKETS_COMMON
default n
---help---
@@ -62,8 +56,6 @@ config VHOST_VSOCK
config VHOST_VDPA
tristate "Vhost driver for vDPA-based backend"
- depends on EVENTFD
- select VHOST
depends on VDPA
help
This kernel module can be loaded in host kernel to accelerate
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] vhost: do not enable VHOST_MENU by default
From: Jason Wang @ 2020-04-15 2:40 UTC (permalink / raw)
To: kbuild test robot, mst, linux-mips, linux-kernel, linuxppc-dev,
linux-s390, kvm, virtualization, netdev, geert,
Thomas Bogendoerfer, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger
Cc: kbuild-all
In-Reply-To: <202004150530.wxnpDMSc%lkp@intel.com>
On 2020/4/15 上午5:15, kbuild test robot wrote:
> Hi Jason,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on vhost/linux-next]
> [also build test ERROR on next-20200414]
> [cannot apply to powerpc/next s390/features v5.7-rc1]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Jason-Wang/vhost-do-not-enable-VHOST_MENU-by-default/20200414-110807
> base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
> config: ia64-randconfig-a001-20200415 (attached as .config)
> compiler: ia64-linux-gcc (GCC) 9.3.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=9.3.0 make.cross ARCH=ia64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All error/warnings (new ones prefixed by >>):
>
> drivers/vhost/vhost.c: In function 'vhost_vring_ioctl':
>>> drivers/vhost/vhost.c:1577:33: error: implicit declaration of function 'eventfd_fget'; did you mean 'eventfd_signal'? [-Werror=implicit-function-declaration]
> 1577 | eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> | ^~~~~~~~~~~~
> | eventfd_signal
>>> drivers/vhost/vhost.c:1577:31: warning: pointer/integer type mismatch in conditional expression
Forget to make VHOST depend on EVENTFD.
Will send v2.
Thanks
> 1577 | eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> | ^
> cc1: some warnings being treated as errors
>
> vim +1577 drivers/vhost/vhost.c
>
> feebcaeac79ad8 Jason Wang 2019-05-24 1493
> feebcaeac79ad8 Jason Wang 2019-05-24 1494 static long vhost_vring_set_num_addr(struct vhost_dev *d,
> feebcaeac79ad8 Jason Wang 2019-05-24 1495 struct vhost_virtqueue *vq,
> feebcaeac79ad8 Jason Wang 2019-05-24 1496 unsigned int ioctl,
> feebcaeac79ad8 Jason Wang 2019-05-24 1497 void __user *argp)
> feebcaeac79ad8 Jason Wang 2019-05-24 1498 {
> feebcaeac79ad8 Jason Wang 2019-05-24 1499 long r;
> feebcaeac79ad8 Jason Wang 2019-05-24 1500
> feebcaeac79ad8 Jason Wang 2019-05-24 1501 mutex_lock(&vq->mutex);
> feebcaeac79ad8 Jason Wang 2019-05-24 1502
> feebcaeac79ad8 Jason Wang 2019-05-24 1503 switch (ioctl) {
> feebcaeac79ad8 Jason Wang 2019-05-24 1504 case VHOST_SET_VRING_NUM:
> feebcaeac79ad8 Jason Wang 2019-05-24 1505 r = vhost_vring_set_num(d, vq, argp);
> feebcaeac79ad8 Jason Wang 2019-05-24 1506 break;
> feebcaeac79ad8 Jason Wang 2019-05-24 1507 case VHOST_SET_VRING_ADDR:
> feebcaeac79ad8 Jason Wang 2019-05-24 1508 r = vhost_vring_set_addr(d, vq, argp);
> feebcaeac79ad8 Jason Wang 2019-05-24 1509 break;
> feebcaeac79ad8 Jason Wang 2019-05-24 1510 default:
> feebcaeac79ad8 Jason Wang 2019-05-24 1511 BUG();
> feebcaeac79ad8 Jason Wang 2019-05-24 1512 }
> feebcaeac79ad8 Jason Wang 2019-05-24 1513
> feebcaeac79ad8 Jason Wang 2019-05-24 1514 mutex_unlock(&vq->mutex);
> feebcaeac79ad8 Jason Wang 2019-05-24 1515
> feebcaeac79ad8 Jason Wang 2019-05-24 1516 return r;
> feebcaeac79ad8 Jason Wang 2019-05-24 1517 }
> 26b36604523f4a Sonny Rao 2018-03-14 1518 long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1519 {
> cecb46f194460d Al Viro 2012-08-27 1520 struct file *eventfp, *filep = NULL;
> cecb46f194460d Al Viro 2012-08-27 1521 bool pollstart = false, pollstop = false;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1522 struct eventfd_ctx *ctx = NULL;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1523 u32 __user *idxp = argp;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1524 struct vhost_virtqueue *vq;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1525 struct vhost_vring_state s;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1526 struct vhost_vring_file f;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1527 u32 idx;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1528 long r;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1529
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1530 r = get_user(idx, idxp);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1531 if (r < 0)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1532 return r;
> 0f3d9a17469d71 Krishna Kumar 2010-05-25 1533 if (idx >= d->nvqs)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1534 return -ENOBUFS;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1535
> ff002269a4ee9c Jason Wang 2018-10-30 1536 idx = array_index_nospec(idx, d->nvqs);
> 3ab2e420ec1caf Asias He 2013-04-27 1537 vq = d->vqs[idx];
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1538
> feebcaeac79ad8 Jason Wang 2019-05-24 1539 if (ioctl == VHOST_SET_VRING_NUM ||
> feebcaeac79ad8 Jason Wang 2019-05-24 1540 ioctl == VHOST_SET_VRING_ADDR) {
> feebcaeac79ad8 Jason Wang 2019-05-24 1541 return vhost_vring_set_num_addr(d, vq, ioctl, argp);
> feebcaeac79ad8 Jason Wang 2019-05-24 1542 }
> feebcaeac79ad8 Jason Wang 2019-05-24 1543
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1544 mutex_lock(&vq->mutex);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1545
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1546 switch (ioctl) {
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1547 case VHOST_SET_VRING_BASE:
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1548 /* Moving base with an active backend?
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1549 * You don't want to do that. */
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1550 if (vq->private_data) {
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1551 r = -EBUSY;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1552 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1553 }
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1554 if (copy_from_user(&s, argp, sizeof s)) {
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1555 r = -EFAULT;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1556 break;
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1557 }
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1558 if (s.num > 0xffff) {
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1559 r = -EINVAL;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1560 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1561 }
> 8d65843c44269c Jason Wang 2017-07-27 1562 vq->last_avail_idx = s.num;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1563 /* Forget the cached index value. */
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1564 vq->avail_idx = vq->last_avail_idx;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1565 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1566 case VHOST_GET_VRING_BASE:
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1567 s.index = idx;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1568 s.num = vq->last_avail_idx;
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1569 if (copy_to_user(argp, &s, sizeof s))
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1570 r = -EFAULT;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1571 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1572 case VHOST_SET_VRING_KICK:
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1573 if (copy_from_user(&f, argp, sizeof f)) {
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1574 r = -EFAULT;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1575 break;
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1576 }
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 @1577 eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1578 if (IS_ERR(eventfp)) {
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1579 r = PTR_ERR(eventfp);
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1580 break;
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1581 }
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1582 if (eventfp != vq->kick) {
> cecb46f194460d Al Viro 2012-08-27 1583 pollstop = (filep = vq->kick) != NULL;
> cecb46f194460d Al Viro 2012-08-27 1584 pollstart = (vq->kick = eventfp) != NULL;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1585 } else
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1586 filep = eventfp;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1587 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1588 case VHOST_SET_VRING_CALL:
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1589 if (copy_from_user(&f, argp, sizeof f)) {
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1590 r = -EFAULT;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1591 break;
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1592 }
> e050c7d93f4adb Eric Biggers 2018-01-06 1593 ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);
> e050c7d93f4adb Eric Biggers 2018-01-06 1594 if (IS_ERR(ctx)) {
> e050c7d93f4adb Eric Biggers 2018-01-06 1595 r = PTR_ERR(ctx);
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1596 break;
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1597 }
> e050c7d93f4adb Eric Biggers 2018-01-06 1598 swap(ctx, vq->call_ctx);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1599 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1600 case VHOST_SET_VRING_ERR:
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1601 if (copy_from_user(&f, argp, sizeof f)) {
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1602 r = -EFAULT;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1603 break;
> 7ad9c9d2704854 Takuya Yoshikawa 2010-05-27 1604 }
> 09f332a589232f Eric Biggers 2018-01-06 1605 ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);
> 09f332a589232f Eric Biggers 2018-01-06 1606 if (IS_ERR(ctx)) {
> 09f332a589232f Eric Biggers 2018-01-06 1607 r = PTR_ERR(ctx);
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1608 break;
> 535297a6ae4c3b Michael S. Tsirkin 2010-03-17 1609 }
> 09f332a589232f Eric Biggers 2018-01-06 1610 swap(ctx, vq->error_ctx);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1611 break;
> 2751c9882b9472 Greg Kurz 2015-04-24 1612 case VHOST_SET_VRING_ENDIAN:
> 2751c9882b9472 Greg Kurz 2015-04-24 1613 r = vhost_set_vring_endian(vq, argp);
> 2751c9882b9472 Greg Kurz 2015-04-24 1614 break;
> 2751c9882b9472 Greg Kurz 2015-04-24 1615 case VHOST_GET_VRING_ENDIAN:
> 2751c9882b9472 Greg Kurz 2015-04-24 1616 r = vhost_get_vring_endian(vq, idx, argp);
> 2751c9882b9472 Greg Kurz 2015-04-24 1617 break;
> 03088137246065 Jason Wang 2016-03-04 1618 case VHOST_SET_VRING_BUSYLOOP_TIMEOUT:
> 03088137246065 Jason Wang 2016-03-04 1619 if (copy_from_user(&s, argp, sizeof(s))) {
> 03088137246065 Jason Wang 2016-03-04 1620 r = -EFAULT;
> 03088137246065 Jason Wang 2016-03-04 1621 break;
> 03088137246065 Jason Wang 2016-03-04 1622 }
> 03088137246065 Jason Wang 2016-03-04 1623 vq->busyloop_timeout = s.num;
> 03088137246065 Jason Wang 2016-03-04 1624 break;
> 03088137246065 Jason Wang 2016-03-04 1625 case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
> 03088137246065 Jason Wang 2016-03-04 1626 s.index = idx;
> 03088137246065 Jason Wang 2016-03-04 1627 s.num = vq->busyloop_timeout;
> 03088137246065 Jason Wang 2016-03-04 1628 if (copy_to_user(argp, &s, sizeof(s)))
> 03088137246065 Jason Wang 2016-03-04 1629 r = -EFAULT;
> 03088137246065 Jason Wang 2016-03-04 1630 break;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1631 default:
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1632 r = -ENOIOCTLCMD;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1633 }
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1634
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1635 if (pollstop && vq->handle_kick)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1636 vhost_poll_stop(&vq->poll);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1637
> e050c7d93f4adb Eric Biggers 2018-01-06 1638 if (!IS_ERR_OR_NULL(ctx))
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1639 eventfd_ctx_put(ctx);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1640 if (filep)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1641 fput(filep);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1642
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1643 if (pollstart && vq->handle_kick)
> 2b8b328b61c799 Jason Wang 2013-01-28 1644 r = vhost_poll_start(&vq->poll, vq->kick);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1645
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1646 mutex_unlock(&vq->mutex);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1647
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1648 if (pollstop && vq->handle_kick)
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1649 vhost_poll_flush(&vq->poll);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1650 return r;
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1651 }
> 6ac1afbf6132df Asias He 2013-05-06 1652 EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
> 3a4d5c94e95935 Michael S. Tsirkin 2010-01-14 1653
>
> :::::: The code at line 1577 was first introduced by commit
> :::::: 3a4d5c94e959359ece6d6b55045c3f046677f55c vhost_net: a kernel-level virtio server
>
> :::::: TO: Michael S. Tsirkin <mst@redhat.com>
> :::::: CC: David S. Miller <davem@davemloft.net>
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 1/3] kexec: Prevent removal of memory in use by a loaded kexec image
From: Baoquan He @ 2020-04-15 2:35 UTC (permalink / raw)
To: David Hildenbrand
Cc: piliu, Anshuman Khandual, Catalin Marinas, Bhupesh Sharma,
linuxppc-dev, kexec, Russell King - ARM Linux admin, linux-mm,
James Morse, Eric W. Biederman, Andrew Morton, Will Deacon,
linux-arm-kernel
In-Reply-To: <0085f460-b0c7-b25f-36a7-fa3bafaab6fe@redhat.com>
On 04/14/20 at 04:49pm, David Hildenbrand wrote:
> >>>>> The root cause is kexec-ed kernel is targeted at hotpluggable memory
> >>>>> region. Just avoiding the movable area can fix it. In kexec_file_load(),
> >>>>> just checking or picking those unmovable region to put kernel/initrd in
> >>>>> function locate_mem_hole_callback() can fix it. The page or pageblock's
> >>>>> zone is movable or not, it's easy to know. This fix doesn't need to
> >>>>> bother other component.
> >>>>
> >>>> I don't fully agree. E.g., just because memory is onlined to ZONE_NORMAL
> >>>> does not imply that it cannot get offlined and removed e.g., this is
> >>>> heavily used on ppc64, with 16MB sections.
> >>>
> >>> Really? I just know there are two kinds of mem hoplug in ppc, but don't
> >>> know the details. So in this case, is there any flag or a way to know
> >>> those memory block are hotpluggable? I am curious how those kernel data
> >>> is avoided to be put in this area. Or ppc just freely uses it for kernel
> >>> data or user space data, then try to migrate when hot remove?
> >>
> >> See
> >> arch/powerpc/platforms/pseries/hotplug-memory.c:dlpar_memory_remove_by_count()
> >>
> >> Under DLAPR, it can remove memory in LMB granularity, which is usually
> >> 16MB (== single section on ppc64). DLPAR will directly online all
> >> hotplugged memory (LMBs) from the kernel using device_online(), which
> >> will go to ZONE_NORMAL.
> >>
> >> When trying to remove memory, it simply scans for offlineable 16MB
> >> memory blocks (==section == LMB), offlines and removes them. No need for
> >> the movable zone and all the involved issues.
> >
> > Yes, this is a different one, thanks for pointing it out. It sounds like
> > balloon driver in virt platform, doesn't it?
>
> With DLPAR there is a hypervisor involved (which manages the actual HW
> DIMMs), so yes.
>
> >
> > Avoiding to put kexec kernel into movable zone can't solve this DLPAR
> > case as you said.
> >
> >>
> >> Now, the interesting question is, can we have LMBs added during boot
> >> (not via add_memory()), that will later be removed via remove_memory().
> >> IIRC, we had BUGs related to that, so I think yes. If a section contains
> >> no unmovable allocations (after boot), it can get removed.
> >
> > I do want to ask this question. If we can add LMB into system RAM, then
> > reload kexec can solve it.
> >
> > Another better way is adding a common function to filter out the
> > movable zone when search position for kexec kernel, use a arch specific
> > funciton to filter out DLPAR memory blocks for ppc only. Over there,
> > we can simply use for_each_drmem_lmb() to do that.
>
> I was thinking about something similar. Maybe something like a notifier
> that can be used to test if selected memory can be used for kexec
Not sure if I get the notifier idea clearly. If you mean
1) Add a common function to pick memory in unmovable zone;
2) Let DLPAR, balloon register with notifier;
3) In the common function, ask notified part to check if the picked
unmovable memory is available for locating kexec kernel;
Sounds doable to me, and not complicated.
> images. It would apply to
>
> - arm64 and filter out all hotadded memory (IIRC, only boot memory can
> be used).
Do you mean hot added memory after boot can't be recognized and added
into system RAM on arm64?
> - powerpc to filter out all LMBs that can be removed (assuming not all
> memory corresponds to LMBs that can be removed, otherwise we're in
> trouble ... :) )
> - virtio-mem to filter out all memory it added.
> - hyper-v to filter out partially backed memory blocks (esp. the last
> memory block it added and only partially backed it by memory).
>
> This would make it work for kexec_file_load(), however, I do wonder how
> we would want to approach that from userspace kexec-tools when handling
> it from kexec_load().
Let's make kexec_file_load work firstly. Since this work is only first
step to make kexec-ed kernel not break memory hotplug. After kexec
rebooting, the KASLR may locate kernel into hotpluggable area too.
^ permalink raw reply
* Re: [PATCH 1/4] dma-mapping: move the remaining DMA API calls out of line
From: Alexey Kardashevskiy @ 2020-04-15 2:26 UTC (permalink / raw)
To: Christoph Hellwig, iommu
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
linuxppc-dev, Lu Baolu
In-Reply-To: <20200414122506.438134-2-hch@lst.de>
On 14/04/2020 22:25, Christoph Hellwig wrote:
> For a long time the DMA API has been implemented inline in dma-mapping.h,
> but the function bodies can be quite large. Move them all out of line.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/dma-direct.h | 58 +++++++++
> include/linux/dma-mapping.h | 247 ++++--------------------------------
> kernel/dma/direct.c | 9 --
> kernel/dma/mapping.c | 164 ++++++++++++++++++++++++
> 4 files changed, 244 insertions(+), 234 deletions(-)
>
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index 24b8684aa21d..da689ad5fffd 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -85,4 +85,62 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
> void *cpu_addr, dma_addr_t dma_addr, size_t size,
> unsigned long attrs);
> int dma_direct_supported(struct device *dev, u64 mask);
> +dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> + unsigned long offset, size_t size, enum dma_data_direction dir,
> + unsigned long attrs);
> +int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> + enum dma_data_direction dir, unsigned long attrs);
> +dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +
> +#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> + defined(CONFIG_SWIOTLB)
> +void dma_direct_sync_single_for_device(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir);
> +void dma_direct_sync_sg_for_device(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> +#else
> +static inline void dma_direct_sync_single_for_device(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir)
> +{
> +}
> +static inline void dma_direct_sync_sg_for_device(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> +{
> +}
> +#endif
> +
> +#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
> + defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) || \
> + defined(CONFIG_SWIOTLB)
> +void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sgl,
> + int nents, enum dma_data_direction dir, unsigned long attrs);
> +void dma_direct_sync_single_for_cpu(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir);
> +void dma_direct_sync_sg_for_cpu(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> +#else
> +static inline void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> +}
> +static inline void dma_direct_unmap_sg(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir,
> + unsigned long attrs)
> +{
> +}
> +static inline void dma_direct_sync_single_for_cpu(struct device *dev,
> + dma_addr_t addr, size_t size, enum dma_data_direction dir)
> +{
> +}
> +static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
> + struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> +{
> +}
> +#endif
> +
> +size_t dma_direct_max_mapping_size(struct device *dev);
> +
> #endif /* _LINUX_DMA_DIRECT_H */
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 330ad58fbf4d..793ad775cd54 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -188,73 +188,6 @@ static inline int dma_mmap_from_global_coherent(struct vm_area_struct *vma,
> }
> #endif /* CONFIG_DMA_DECLARE_COHERENT */
>
> -static inline bool dma_is_direct(const struct dma_map_ops *ops)
> -{
> - return likely(!ops);
> -}
> -
> -/*
> - * All the dma_direct_* declarations are here just for the indirect call bypass,
> - * and must not be used directly drivers!
> - */
> -dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
> - unsigned long offset, size_t size, enum dma_data_direction dir,
> - unsigned long attrs);
> -int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
> - enum dma_data_direction dir, unsigned long attrs);
> -dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs);
> -
> -#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> - defined(CONFIG_SWIOTLB)
> -void dma_direct_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir);
> -void dma_direct_sync_sg_for_device(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> -#else
> -static inline void dma_direct_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir)
> -{
> -}
> -static inline void dma_direct_sync_sg_for_device(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> -{
> -}
> -#endif
> -
> -#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
> - defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) || \
> - defined(CONFIG_SWIOTLB)
> -void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs);
> -void dma_direct_unmap_sg(struct device *dev, struct scatterlist *sgl,
> - int nents, enum dma_data_direction dir, unsigned long attrs);
> -void dma_direct_sync_single_for_cpu(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir);
> -void dma_direct_sync_sg_for_cpu(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir);
> -#else
> -static inline void dma_direct_unmap_page(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs)
> -{
> -}
> -static inline void dma_direct_unmap_sg(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> -}
> -static inline void dma_direct_sync_single_for_cpu(struct device *dev,
> - dma_addr_t addr, size_t size, enum dma_data_direction dir)
> -{
> -}
> -static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
> - struct scatterlist *sgl, int nents, enum dma_data_direction dir)
> -{
> -}
> -#endif
> -
> -size_t dma_direct_max_mapping_size(struct device *dev);
> -
> #ifdef CONFIG_HAS_DMA
> #include <asm/dma-mapping.h>
>
> @@ -271,164 +204,6 @@ static inline void set_dma_ops(struct device *dev,
> dev->dma_ops = dma_ops;
> }
>
> -static inline dma_addr_t dma_map_page_attrs(struct device *dev,
> - struct page *page, size_t offset, size_t size,
> - enum dma_data_direction dir, unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - dma_addr_t addr;
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
> - else
> - addr = ops->map_page(dev, page, offset, size, dir, attrs);
> - debug_dma_map_page(dev, page, offset, size, dir, addr);
> -
> - return addr;
> -}
> -
> -static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir, unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_unmap_page(dev, addr, size, dir, attrs);
> - else if (ops->unmap_page)
> - ops->unmap_page(dev, addr, size, dir, attrs);
> - debug_dma_unmap_page(dev, addr, size, dir);
> -}
> -
> -/*
> - * dma_maps_sg_attrs returns 0 on error and > 0 on success.
> - * It should never return a value < 0.
> - */
> -static inline int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - int ents;
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
> - else
> - ents = ops->map_sg(dev, sg, nents, dir, attrs);
> - BUG_ON(ents < 0);
> - debug_dma_map_sg(dev, sg, nents, ents, dir);
> -
> - return ents;
> -}
> -
> -static inline void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - debug_dma_unmap_sg(dev, sg, nents, dir);
> - if (dma_is_direct(ops))
> - dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
> - else if (ops->unmap_sg)
> - ops->unmap_sg(dev, sg, nents, dir, attrs);
> -}
> -
> -static inline dma_addr_t dma_map_resource(struct device *dev,
> - phys_addr_t phys_addr,
> - size_t size,
> - enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> - dma_addr_t addr = DMA_MAPPING_ERROR;
> -
> - BUG_ON(!valid_dma_direction(dir));
> -
> - /* Don't allow RAM to be mapped */
> - if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
> - return DMA_MAPPING_ERROR;
> -
> - if (dma_is_direct(ops))
> - addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
> - else if (ops->map_resource)
> - addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
> -
> - debug_dma_map_resource(dev, phys_addr, size, dir, addr);
> - return addr;
> -}
> -
> -static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir,
> - unsigned long attrs)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (!dma_is_direct(ops) && ops->unmap_resource)
> - ops->unmap_resource(dev, addr, size, dir, attrs);
> - debug_dma_unmap_resource(dev, addr, size, dir);
> -}
> -
> -static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
> - size_t size,
> - enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_single_for_cpu(dev, addr, size, dir);
> - else if (ops->sync_single_for_cpu)
> - ops->sync_single_for_cpu(dev, addr, size, dir);
> - debug_dma_sync_single_for_cpu(dev, addr, size, dir);
> -}
> -
> -static inline void dma_sync_single_for_device(struct device *dev,
> - dma_addr_t addr, size_t size,
> - enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_single_for_device(dev, addr, size, dir);
> - else if (ops->sync_single_for_device)
> - ops->sync_single_for_device(dev, addr, size, dir);
> - debug_dma_sync_single_for_device(dev, addr, size, dir);
> -}
> -
> -static inline void
> -dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
> - int nelems, enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
> - else if (ops->sync_sg_for_cpu)
> - ops->sync_sg_for_cpu(dev, sg, nelems, dir);
> - debug_dma_sync_sg_for_cpu(dev, sg, nelems, dir);
> -}
> -
> -static inline void
> -dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
> - int nelems, enum dma_data_direction dir)
> -{
> - const struct dma_map_ops *ops = get_dma_ops(dev);
> -
> - BUG_ON(!valid_dma_direction(dir));
> - if (dma_is_direct(ops))
> - dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
> - else if (ops->sync_sg_for_device)
> - ops->sync_sg_for_device(dev, sg, nelems, dir);
> - debug_dma_sync_sg_for_device(dev, sg, nelems, dir);
> -
> -}
>
> static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
> {
> @@ -439,6 +214,28 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
> return 0;
> }
>
> +dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
> + size_t offset, size_t size, enum dma_data_direction dir,
> + unsigned long attrs);
> +void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir, unsigned long attrs);
> +int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg, int nents,
> + enum dma_data_direction dir, unsigned long attrs);
> +void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
> + int nents, enum dma_data_direction dir,
> + unsigned long attrs);
> +dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
> + size_t size, enum dma_data_direction dir, unsigned long attrs);
> +void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir, unsigned long attrs);
> +void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr, size_t size,
> + enum dma_data_direction dir);
> +void dma_sync_single_for_device(struct device *dev, dma_addr_t addr,
> + size_t size, enum dma_data_direction dir);
> +void dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
> + int nelems, enum dma_data_direction dir);
> +void dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
> + int nelems, enum dma_data_direction dir);
> void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp_t flag, unsigned long attrs);
> void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8f4bbdaf965e..f1a9099a4b5b 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -260,7 +260,6 @@ void dma_direct_sync_single_for_device(struct device *dev,
> if (!dev_is_dma_coherent(dev))
> arch_sync_dma_for_device(paddr, size, dir);
> }
> -EXPORT_SYMBOL(dma_direct_sync_single_for_device);
May be this is correct and allowed (no idea) but removing exported
symbols at least deserves a mention in the commit log, does not it?
The rest of the series is fine and works. Thanks,
--
Alexey
^ permalink raw reply
* [PATCH 1/2] ocxl: Remove unnecessary externs
From: Alastair D'Silva @ 2020-04-15 1:23 UTC (permalink / raw)
To: alastair
Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
Paul Mackerras, Frederic Barrat, linuxppc-dev
In-Reply-To: <20200415012343.919255-1-alastair@d-silva.org>
Function declarations don't need externs, remove the existing ones
so they are consistent with newer code
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
arch/powerpc/include/asm/pnv-ocxl.h | 40 ++++++++++++++---------------
include/misc/ocxl.h | 6 ++---
2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index 7de82647e761..ee79d2cd9fb6 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -9,28 +9,26 @@
#define PNV_OCXL_TL_BITS_PER_RATE 4
#define PNV_OCXL_TL_RATE_BUF_SIZE ((PNV_OCXL_TL_MAX_TEMPLATE+1) * PNV_OCXL_TL_BITS_PER_RATE / 8)
-extern int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
- u16 *supported);
-extern int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count);
+int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled, u16 *supported);
+int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count);
-extern int pnv_ocxl_get_tl_cap(struct pci_dev *dev, long *cap,
+int pnv_ocxl_get_tl_cap(struct pci_dev *dev, long *cap,
char *rate_buf, int rate_buf_size);
-extern int pnv_ocxl_set_tl_conf(struct pci_dev *dev, long cap,
- uint64_t rate_buf_phys, int rate_buf_size);
-
-extern int pnv_ocxl_get_xsl_irq(struct pci_dev *dev, int *hwirq);
-extern void pnv_ocxl_unmap_xsl_regs(void __iomem *dsisr, void __iomem *dar,
- void __iomem *tfc, void __iomem *pe_handle);
-extern int pnv_ocxl_map_xsl_regs(struct pci_dev *dev, void __iomem **dsisr,
- void __iomem **dar, void __iomem **tfc,
- void __iomem **pe_handle);
-
-extern int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask,
- void **platform_data);
-extern void pnv_ocxl_spa_release(void *platform_data);
-extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
-
-extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
-extern void pnv_ocxl_free_xive_irq(u32 irq);
+int pnv_ocxl_set_tl_conf(struct pci_dev *dev, long cap,
+ uint64_t rate_buf_phys, int rate_buf_size);
+
+int pnv_ocxl_get_xsl_irq(struct pci_dev *dev, int *hwirq);
+void pnv_ocxl_unmap_xsl_regs(void __iomem *dsisr, void __iomem *dar,
+ void __iomem *tfc, void __iomem *pe_handle);
+int pnv_ocxl_map_xsl_regs(struct pci_dev *dev, void __iomem **dsisr,
+ void __iomem **dar, void __iomem **tfc,
+ void __iomem **pe_handle);
+
+int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask, void **platform_data);
+void pnv_ocxl_spa_release(void *platform_data);
+int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
+
+int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
+void pnv_ocxl_free_xive_irq(u32 irq);
#endif /* _ASM_PNV_OCXL_H */
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 06dd5839e438..0a762e387418 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -173,7 +173,7 @@ int ocxl_context_detach(struct ocxl_context *ctx);
*
* Returns 0 on success, negative on failure
*/
-extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
/**
* Frees an IRQ associated with an AFU context
@@ -182,7 +182,7 @@ extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
*
* Returns 0 on success, negative on failure
*/
-extern int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
/**
* Gets the address of the trigger page for an IRQ
@@ -193,7 +193,7 @@ extern int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
*
* returns the trigger page address, or 0 if the IRQ is not valid
*/
-extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
/**
* Provide a callback to be called when an IRQ is triggered
--
2.25.1
^ permalink raw reply related
* [PATCH 2/2] ocxl: Address kernel doc errors & warnings
From: Alastair D'Silva @ 2020-04-15 1:23 UTC (permalink / raw)
To: alastair
Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
Paul Mackerras, Frederic Barrat, linuxppc-dev
In-Reply-To: <20200415012343.919255-1-alastair@d-silva.org>
This patch addresses warnings and errors from the kernel doc scripts for
the OpenCAPI driver.
It also makes minor tweaks to make the docs more consistent.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
---
drivers/misc/ocxl/config.c | 24 ++++----
drivers/misc/ocxl/ocxl_internal.h | 9 +--
include/misc/ocxl.h | 96 ++++++++++++-------------------
3 files changed, 55 insertions(+), 74 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index c8e19bfb5ef9..a62e3d7db2bf 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -273,16 +273,16 @@ static int read_afu_info(struct pci_dev *dev, struct ocxl_fn_config *fn,
}
/**
- * Read the template version from the AFU
- * dev: the device for the AFU
- * fn: the AFU offsets
- * len: outputs the template length
- * version: outputs the major<<8,minor version
+ * read_template_version() - Read the template version from the AFU
+ * @dev: the device for the AFU
+ * @fn: the AFU offsets
+ * @len: outputs the template length
+ * @version: outputs the major<<8,minor version
*
* Returns 0 on success, negative on failure
*/
static int read_template_version(struct pci_dev *dev, struct ocxl_fn_config *fn,
- u16 *len, u16 *version)
+ u16 *len, u16 *version)
{
u32 val32;
u8 major, minor;
@@ -476,16 +476,16 @@ static int validate_afu(struct pci_dev *dev, struct ocxl_afu_config *afu)
}
/**
- * Populate AFU metadata regarding LPC memory
- * dev: the device for the AFU
- * fn: the AFU offsets
- * afu: the AFU struct to populate the LPC metadata into
+ * read_afu_lpc_memory_info() - Populate AFU metadata regarding LPC memory
+ * @dev: the device for the AFU
+ * @fn: the AFU offsets
+ * @afu: the AFU struct to populate the LPC metadata into
*
* Returns 0 on success, negative on failure
*/
static int read_afu_lpc_memory_info(struct pci_dev *dev,
- struct ocxl_fn_config *fn,
- struct ocxl_afu_config *afu)
+ struct ocxl_fn_config *fn,
+ struct ocxl_afu_config *afu)
{
int rc;
u32 val32;
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 345bf843a38e..198e4e4bc51d 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -122,11 +122,12 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
struct ocxl_fn_config *fn, int afu_idx);
/**
- * Update values within a Process Element
+ * ocxl_link_update_pe() - Update values within a Process Element
+ * @link_handle: the link handle associated with the process element
+ * @pasid: the PASID for the AFU context
+ * @tid: the new thread id for the process element
*
- * link_handle: the link handle associated with the process element
- * pasid: the PASID for the AFU context
- * tid: the new thread id for the process element
+ * Returns 0 on success
*/
int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 0a762e387418..357ef1aadbc0 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -62,8 +62,7 @@ struct ocxl_context;
// Device detection & initialisation
/**
- * Open an OpenCAPI function on an OpenCAPI device
- *
+ * ocxl_function_open() - Open an OpenCAPI function on an OpenCAPI device
* @dev: The PCI device that contains the function
*
* Returns an opaque pointer to the function, or an error pointer (check with IS_ERR)
@@ -71,8 +70,7 @@ struct ocxl_context;
struct ocxl_fn *ocxl_function_open(struct pci_dev *dev);
/**
- * Get the list of AFUs associated with a PCI function device
- *
+ * ocxl_function_afu_list() - Get the list of AFUs associated with a PCI function device
* Returns a list of struct ocxl_afu *
*
* @fn: The OpenCAPI function containing the AFUs
@@ -80,8 +78,7 @@ struct ocxl_fn *ocxl_function_open(struct pci_dev *dev);
struct list_head *ocxl_function_afu_list(struct ocxl_fn *fn);
/**
- * Fetch an AFU instance from an OpenCAPI function
- *
+ * ocxl_function_fetch_afu() - Fetch an AFU instance from an OpenCAPI function
* @fn: The OpenCAPI function to get the AFU from
* @afu_idx: The index of the AFU to get
*
@@ -92,23 +89,20 @@ struct list_head *ocxl_function_afu_list(struct ocxl_fn *fn);
struct ocxl_afu *ocxl_function_fetch_afu(struct ocxl_fn *fn, u8 afu_idx);
/**
- * Take a reference to an AFU
- *
+ * ocxl_afu_get() - Take a reference to an AFU
* @afu: The AFU to increment the reference count on
*/
void ocxl_afu_get(struct ocxl_afu *afu);
/**
- * Release a reference to an AFU
- *
+ * ocxl_afu_put() - Release a reference to an AFU
* @afu: The AFU to decrement the reference count on
*/
void ocxl_afu_put(struct ocxl_afu *afu);
/**
- * Get the configuration information for an OpenCAPI function
- *
+ * ocxl_function_config() - Get the configuration information for an OpenCAPI function
* @fn: The OpenCAPI function to get the config for
*
* Returns the function config, or NULL on error
@@ -116,8 +110,7 @@ void ocxl_afu_put(struct ocxl_afu *afu);
const struct ocxl_fn_config *ocxl_function_config(struct ocxl_fn *fn);
/**
- * Close an OpenCAPI function
- *
+ * ocxl_function_close() - Close an OpenCAPI function
* This will free any AFUs previously retrieved from the function, and
* detach and associated contexts. The contexts must by freed by the caller.
*
@@ -129,8 +122,7 @@ void ocxl_function_close(struct ocxl_fn *fn);
// Context allocation
/**
- * Allocate an OpenCAPI context
- *
+ * ocxl_context_alloc() - Allocate an OpenCAPI context
* @context: The OpenCAPI context to allocate, must be freed with ocxl_context_free
* @afu: The AFU the context belongs to
* @mapping: The mapping to unmap when the context is closed (may be NULL)
@@ -139,14 +131,13 @@ int ocxl_context_alloc(struct ocxl_context **context, struct ocxl_afu *afu,
struct address_space *mapping);
/**
- * Free an OpenCAPI context
- *
+ * ocxl_context_free() - Free an OpenCAPI context
* @ctx: The OpenCAPI context to free
*/
void ocxl_context_free(struct ocxl_context *ctx);
/**
- * Grant access to an MM to an OpenCAPI context
+ * ocxl_context_attach() - Grant access to an MM to an OpenCAPI context
* @ctx: The OpenCAPI context to attach
* @amr: The value of the AMR register to restrict access
* @mm: The mm to attach to the context
@@ -157,7 +148,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
struct mm_struct *mm);
/**
- * Detach an MM from an OpenCAPI context
+ * ocxl_context_detach() - Detach an MM from an OpenCAPI context
* @ctx: The OpenCAPI context to attach
*
* Returns 0 on success, negative on failure
@@ -167,7 +158,7 @@ int ocxl_context_detach(struct ocxl_context *ctx);
// AFU IRQs
/**
- * Allocate an IRQ associated with an AFU context
+ * ocxl_afu_irq_alloc() - Allocate an IRQ associated with an AFU context
* @ctx: the AFU context
* @irq_id: out, the IRQ ID
*
@@ -176,7 +167,7 @@ int ocxl_context_detach(struct ocxl_context *ctx);
int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
/**
- * Frees an IRQ associated with an AFU context
+ * ocxl_afu_irq_free() - Frees an IRQ associated with an AFU context
* @ctx: the AFU context
* @irq_id: the IRQ ID
*
@@ -185,7 +176,7 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
/**
- * Gets the address of the trigger page for an IRQ
+ * ocxl_afu_irq_get_addr() - Gets the address of the trigger page for an IRQ
* This can then be provided to an AFU which will write to that
* page to trigger the IRQ.
* @ctx: The AFU context that the IRQ is associated with
@@ -196,7 +187,7 @@ int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
/**
- * Provide a callback to be called when an IRQ is triggered
+ * ocxl_irq_set_handler() - Provide a callback to be called when an IRQ is triggered
* @ctx: The AFU context that the IRQ is associated with
* @irq_id: The IRQ ID
* @handler: the callback to be called when the IRQ is triggered
@@ -213,8 +204,7 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
// AFU Metadata
/**
- * Get a pointer to the config for an AFU
- *
+ * ocxl_afu_config() - Get a pointer to the config for an AFU
* @afu: a pointer to the AFU to get the config for
*
* Returns a pointer to the AFU config
@@ -222,27 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
struct ocxl_afu_config *ocxl_afu_config(struct ocxl_afu *afu);
/**
- * Assign opaque hardware specific information to an OpenCAPI AFU.
- *
- * @dev: The PCI device associated with the OpenCAPI device
+ * ocxl_afu_set_private() - Assign opaque hardware specific information to an OpenCAPI AFU.
+ * @afu: The OpenCAPI AFU
* @private: the opaque hardware specific information to assign to the driver
*/
void ocxl_afu_set_private(struct ocxl_afu *afu, void *private);
/**
- * Fetch the hardware specific information associated with an external OpenCAPI
- * AFU. This may be consumed by an external OpenCAPI driver.
- *
- * @afu: The AFU
+ * ocxl_afu_get_private() - Fetch the hardware specific information associated with
+ * an external OpenCAPI AFU. This may be consumed by an external OpenCAPI driver.
+ * @afu: The OpenCAPI AFU
*
* Returns the opaque pointer associated with the device, or NULL if not set
*/
-void *ocxl_afu_get_private(struct ocxl_afu *dev);
+void *ocxl_afu_get_private(struct ocxl_afu *afu);
// Global MMIO
/**
- * Read a 32 bit value from global MMIO
- *
+ * ocxl_global_mmio_read32() - Read a 32 bit value from global MMIO
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -251,11 +238,10 @@ void *ocxl_afu_get_private(struct ocxl_afu *dev);
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u32 *val);
+ enum ocxl_endian endian, u32 *val);
/**
- * Read a 64 bit value from global MMIO
- *
+ * ocxl_global_mmio_read64() - Read a 64 bit value from global MMIO
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -264,11 +250,10 @@ int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u64 *val);
+ enum ocxl_endian endian, u64 *val);
/**
- * Write a 32 bit value to global MMIO
- *
+ * ocxl_global_mmio_write32() - Write a 32 bit value to global MMIO
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -277,11 +262,10 @@ int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u32 val);
+ enum ocxl_endian endian, u32 val);
/**
- * Write a 64 bit value to global MMIO
- *
+ * ocxl_global_mmio_write64() - Write a 64 bit value to global MMIO
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -290,11 +274,10 @@ int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u64 val);
+ enum ocxl_endian endian, u64 val);
/**
- * Set bits in a 32 bit global MMIO register
- *
+ * ocxl_global_mmio_set32() - Set bits in a 32 bit global MMIO register
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -303,11 +286,10 @@ int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u32 mask);
+ enum ocxl_endian endian, u32 mask);
/**
- * Set bits in a 64 bit global MMIO register
- *
+ * ocxl_global_mmio_set64() - Set bits in a 64 bit global MMIO register
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -316,11 +298,10 @@ int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u64 mask);
+ enum ocxl_endian endian, u64 mask);
/**
- * Set bits in a 32 bit global MMIO register
- *
+ * ocxl_global_mmio_clear32() - Set bits in a 32 bit global MMIO register
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -329,11 +310,10 @@ int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u32 mask);
+ enum ocxl_endian endian, u32 mask);
/**
- * Set bits in a 64 bit global MMIO register
- *
+ * ocxl_global_mmio_clear64() - Set bits in a 64 bit global MMIO register
* @afu: The AFU
* @offset: The Offset from the start of MMIO
* @endian: the endianness that the MMIO data is in
@@ -342,7 +322,7 @@ int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
* Returns 0 for success, negative on error
*/
int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
- enum ocxl_endian endian, u64 mask);
+ enum ocxl_endian endian, u64 mask);
// Functions left here are for compatibility with the cxlflash driver
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] powerpc: OpenCAPI Cleanup
From: Alastair D'Silva @ 2020-04-15 1:23 UTC (permalink / raw)
To: alastair
Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
Paul Mackerras, Frederic Barrat, linuxppc-dev
These patches address checkpatch & kernel doc warnings
in the OpenCAPI infrastructure.
Alastair D'Silva (2):
ocxl: Remove unnecessary externs
ocxl: Address kernel doc errors & warnings
arch/powerpc/include/asm/pnv-ocxl.h | 40 ++++++-----
drivers/misc/ocxl/config.c | 24 +++----
drivers/misc/ocxl/ocxl_internal.h | 9 +--
include/misc/ocxl.h | 102 +++++++++++-----------------
4 files changed, 77 insertions(+), 98 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2 25/33] docs: powerpc: cxl.rst: mark two section titles as such
From: Andrew Donnellan @ 2020-04-15 0:16 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Doc Mailing List
Cc: Jonathan Corbet, linux-kernel, Paul Mackerras, Frederic Barrat,
linuxppc-dev
In-Reply-To: <190d67397cd63e419de8d85b92e8018d48e8c345.1586881715.git.mchehab+huawei@kernel.org>
On 15/4/20 2:48 am, Mauro Carvalho Chehab wrote:
> The User API chapter contains two sub-chapters. Mark them as
> such.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts
From: Nathan Chancellor @ 2020-04-14 23:35 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-stable, qemu-devel, Cédric Le Goater, qemu-ppc,
linuxppc-dev, David Gibson
In-Reply-To: <20200414111131.465560-1-npiggin@gmail.com>
On Tue, Apr 14, 2020 at 09:11:31PM +1000, Nicholas Piggin wrote:
> If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending,
> it does not cause an interrupt. This causes the test case to hang:
>
> https://lists.gnu.org/archive/html/qemu-ppc/2019-10/msg00826.html
>
> More recently, Linux reduced the occurance of operations (e.g., rfi)
> which stop translation and allow pending interrupts to be processed.
> This started causing hangs in Linux boot in long-running kernel tests,
> running with '-d int' shows the decrementer stops firing despite DEC
> wrapping and MSR[EE]=1.
>
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-April/208301.html
>
> The cause is the broken mtmsr L=1 behaviour, which is contrary to the
> architecture. From Power ISA v3.0B, p.977, Move To Machine State Register,
> Programming Note states:
>
> If MSR[EE]=0 and an External, Decrementer, or Performance Monitor
> exception is pending, executing an mtmsrd instruction that sets
> MSR[EE] to 1 will cause the interrupt to occur before the next
> instruction is executed, if no higher priority exception exists
>
> Fix this by handling L=1 exactly the same way as L=0, modulo the MSR
> bits altered.
>
> The confusion arises from L=0 being "context synchronizing" whereas L=1
> is "execution synchronizing", which is a weaker semantic. However this
> is not a relaxation of the requirement that these exceptions cause
> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
> TCG is doing here), rather it specifies how a pipelined processor can
> have multiple instructions in flight where one may influence how another
> behaves.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Anton Blanchard <anton@ozlabs.org>
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Thanks very much to Nathan for reporting and testing it, I added his
> Tested-by tag despite a more polished patch, as the the basics are
> still the same (and still fixes his test case here).
I did re-run the test with the updated version of your patch and it
passed still so that tag can still stand without any controversy :)
Thank you for the fix again!
Nathan
> This bug possibly goes back to early v2.04 / mtmsrd L=1 support around
> 2007, and the code has been changed several times since then so may
> require some backporting.
>
> 32-bit / mtmsr untested at the moment, I don't have an environment
> handy.
>
> target/ppc/translate.c | 46 +++++++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..9959259dba 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> if (ctx->opcode & 0x00010000) {
> - /* Special form that does not need any synchronisation */
> + /* L=1 form only updates EE and RI */
> TCGv t0 = tcg_temp_new();
> + TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(cpu_msr, cpu_msr,
> + tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> + tcg_gen_or_tl(t1, t1, t0);
> +
> + gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> + tcg_temp_free(t1);
> +
> } else {
> /*
> * XXX: we need to update nip before the store if we enter
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> - if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> - gen_io_start();
> - }
> gen_update_nip(ctx, ctx->base.pc_next);
> gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
> - /* Must stop the translation as machine state (may have) changed */
> - /* Note that mtmsr is not always defined as context-synchronizing */
> - gen_stop_exception(ctx);
> }
> + /* Must stop the translation as machine state (may have) changed */
> + gen_stop_exception(ctx);
> #endif /* !defined(CONFIG_USER_ONLY) */
> }
> #endif /* defined(TARGET_PPC64) */
> @@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> - if (ctx->opcode & 0x00010000) {
> - /* Special form that does not need any synchronisation */
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> + if (ctx->opcode & 0x00010000) {
> + /* L=1 form only updates EE and RI */
> TCGv t0 = tcg_temp_new();
> + TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> - tcg_gen_andi_tl(cpu_msr, cpu_msr,
> + tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> - tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> + tcg_gen_or_tl(t1, t1, t0);
> +
> + gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> + tcg_temp_free(t1);
> +
> } else {
> TCGv msr = tcg_temp_new();
>
> @@ -4411,9 +4423,6 @@ static void gen_mtmsr(DisasContext *ctx)
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> - if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> - gen_io_start();
> - }
> gen_update_nip(ctx, ctx->base.pc_next);
> #if defined(TARGET_PPC64)
> tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
> @@ -4422,10 +4431,9 @@ static void gen_mtmsr(DisasContext *ctx)
> #endif
> gen_helper_store_msr(cpu_env, msr);
> tcg_temp_free(msr);
> - /* Must stop the translation as machine state (may have) changed */
> - /* Note that mtmsr is not always defined as context-synchronizing */
> - gen_stop_exception(ctx);
> }
> + /* Must stop the translation as machine state (may have) changed */
> + gen_stop_exception(ctx);
> #endif
> }
>
> --
> 2.23.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox