* Re: [PATCH V2] crypto/NX: Set receive window credits to max number of CRBs in RxFIFO
From: Herbert Xu @ 2019-06-27 6:26 UTC (permalink / raw)
To: Haren Myneni; +Cc: linuxppc-dev, linux-crypto, stable
In-Reply-To: <1560884962.22818.9.camel@hbabu-laptop>
Haren Myneni <haren@linux.vnet.ibm.com> wrote:
>
> System gets checkstop if RxFIFO overruns with more requests than the
> maximum possible number of CRBs in FIFO at the same time. The max number
> of requests per window is controlled by window credits. So find max
> CRBs from FIFO size and set it to receive window credits.
>
> Fixes: b0d6c9bab5e4 ("crypto/nx: Add P9 NX support for 842 compression engine")
> CC: stable@vger.kernel.org # v4.14+
> Signed-off-by:Haren Myneni <haren@us.ibm.com>
I presume this is being picked up by the powerpc tree?
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/powernv: remove unused NPU DMA code
From: Christoph Hellwig @ 2019-06-27 7:22 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, linux-kernel, Oliver O'Halloran,
Frederic Barrat, Paul Mackerras, linuxppc-dev, Christoph Hellwig
In-Reply-To: <027a5095-a22c-2799-8ff6-42d0bc4d2bc9@ozlabs.ru>
On Thu, Jun 27, 2019 at 10:21:55AM +1000, Alexey Kardashevskiy wrote:
> > Which comment? Last time I asked you complaint "it is still used in
> > exactly the same way as before" which you later clarified that you
> > have a hidden out of tree user somewhere, and you only objected to
>
> It is not hidden, anyone can download and inspect that GPL driver.
For one no one has ever posted a link. And second as mentioned
countless times it doesn't matter, it only matters if it is in mainline,
or as a special exception actively trying to go mainline.
> > the word "dead". That has been fixed and there were no further
> > comments.
>
> You still have it in the cover letter so at very least 3/4 is not a part
> of this patchset then.
>
> And I still want to see a formal statement about out-of-tree drivers
> support/tolerance. If you manage to remove this code, I'll have to post
> a revert (again and again) but I would rather know the exact list of
> what we do and what we do not do about such drivers and if the list 1)
> exists 2) is reasonable then I could try to come up with a better
> solution or point others to the policy and push them to do the right
> thing. Right now it is just you pretending that the nVidia driver does
> not exist, this is not helping. Thanks,
We had that discussion at kernel summit and it was reported. Anyway,
adding Greg, who usually has some pretty good prewritten letters for
this kind of thing.
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Fix maximum memory value
From: Aravinda Prasad @ 2019-06-27 8:38 UTC (permalink / raw)
To: Nathan Lynch; +Cc: naveen.n.rao, linuxppc-dev
In-Reply-To: <874l4ct1mg.fsf@linux.ibm.com>
On Thursday 27 June 2019 04:06 AM, Nathan Lynch wrote:
> Aravinda Prasad <aravinda@linux.vnet.ibm.com> writes:
>> Calculating the maximum memory based on the number of lmbs
>> and lmb size does not account for the RMA region. Hence
>> use drmem_lmb_memory_max(), which already accounts for the
>> RMA region, to fetch the maximum memory value.
>>
>> Fixes: 772b039fd9a7: ("powerpc/pseries: Export maximum memory value")
>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/lparcfg.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
>> index e33e8bc..f425842 100644
>> --- a/arch/powerpc/platforms/pseries/lparcfg.c
>> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
>> @@ -435,7 +435,7 @@ static void maxmem_data(struct seq_file *m)
>> {
>> unsigned long maxmem = 0;
>>
>> - maxmem += drmem_info->n_lmbs * drmem_info->lmb_size;
>> + maxmem += drmem_lmb_memory_max();
>
> Would memory_hotplug_max() be better here? There's no guarantee an LPAR
> will have the device tree node/properties that populate drmem.
I see memory_hotplug_max() calling drmem_lmb_memory_max() if "/rtas"
node is not available or else it fetches from "ibm,lrdr-capacity". So I
think memory_hotplug_max() can be used.
Regards,
Aravinda
>
--
Regards,
Aravinda
^ permalink raw reply
* [PATCH] powerpc/xmon: Fix disabling tracing while in xmon
From: Naveen N. Rao @ 2019-06-27 9:59 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
Commit ed49f7fd6438d ("powerpc/xmon: Disable tracing when entering
xmon") added code to disable recording trace entries while in xmon. The
commit introduced a variable 'tracing_enabled' to record if tracing was
enabled on xmon entry, and used this to conditionally enable tracing
during exit from xmon.
However, we are not checking the value of 'fromipi' variable in
xmon_core() when setting 'tracing_enabled'. Due to this, when secondary
cpus enter xmon, they will see tracing as being disabled already and
tracing won't be re-enabled on exit. Fix the same.
Fixes: ed49f7fd6438d ("powerpc/xmon: Disable tracing when entering xmon")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/xmon/xmon.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index d0620d762a5a..4a721fd62406 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -465,8 +465,10 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
local_irq_save(flags);
hard_irq_disable();
- tracing_enabled = tracing_is_on();
- tracing_off();
+ if (!fromipi) {
+ tracing_enabled = tracing_is_on();
+ tracing_off();
+ }
bp = in_breakpoint_table(regs->nip, &offset);
if (bp != NULL) {
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] recordmcount: Fix spurious mcount entries on powerpc
From: Satheesh Rajendran @ 2019-06-27 10:23 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: linuxppc-dev, linux-kernel, Steven Rostedt
In-Reply-To: <20190626183801.31247-1-naveen.n.rao@linux.vnet.ibm.com>
On Thu, Jun 27, 2019 at 12:08:01AM +0530, Naveen N. Rao wrote:
> The recent change enabling HAVE_C_RECORDMCOUNT on powerpc started
> showing the following issue:
>
> # modprobe kprobe_example
> ftrace-powerpc: Not expected bl: opcode is 3c4c0001
> WARNING: CPU: 0 PID: 227 at kernel/trace/ftrace.c:2001 ftrace_bug+0x90/0x318
> Modules linked in:
> CPU: 0 PID: 227 Comm: modprobe Not tainted 5.2.0-rc6-00678-g1c329100b942 #2
> NIP: c000000000264318 LR: c00000000025d694 CTR: c000000000f5cd30
> REGS: c000000001f2b7b0 TRAP: 0700 Not tainted (5.2.0-rc6-00678-g1c329100b942)
> MSR: 900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]> CR: 28228222 XER: 00000000
> CFAR: c0000000002642fc IRQMASK: 0
> <snip>
> NIP [c000000000264318] ftrace_bug+0x90/0x318
> LR [c00000000025d694] ftrace_process_locs+0x4f4/0x5e0
> Call Trace:
> [c000000001f2ba40] [0000000000000004] 0x4 (unreliable)
> [c000000001f2bad0] [c00000000025d694] ftrace_process_locs+0x4f4/0x5e0
> [c000000001f2bb90] [c00000000020ff10] load_module+0x25b0/0x30c0
> [c000000001f2bd00] [c000000000210cb0] sys_finit_module+0xc0/0x130
> [c000000001f2be20] [c00000000000bda4] system_call+0x5c/0x70
> Instruction dump:
> 419e0018 2f83ffff 419e00bc 2f83ffea 409e00cc 4800001c 0fe00000 3c62ff96
> 39000001 39400000 386386d0 480000c4 <0fe00000> 3ce20003 39000001 3c62ff96
> ---[ end trace 4c438d5cebf78381 ]---
> ftrace failed to modify
> [<c0080000012a0008>] 0xc0080000012a0008
> actual: 01:00:4c:3c
> Initializing ftrace call sites
> ftrace record flags: 2000000
> (0)
> expected tramp: c00000000006af4c
>
> Looking at the relocation records in __mcount_loc showed a few spurious
> entries:
> RELOCATION RECORDS FOR [__mcount_loc]:
> OFFSET TYPE VALUE
> 0000000000000000 R_PPC64_ADDR64 .text.unlikely+0x0000000000000008
> 0000000000000008 R_PPC64_ADDR64 .text.unlikely+0x0000000000000014
> 0000000000000010 R_PPC64_ADDR64 .text.unlikely+0x0000000000000060
> 0000000000000018 R_PPC64_ADDR64 .text.unlikely+0x00000000000000b4
> 0000000000000020 R_PPC64_ADDR64 .init.text+0x0000000000000008
> 0000000000000028 R_PPC64_ADDR64 .init.text+0x0000000000000014
>
> The first entry in each section is incorrect. Looking at the relocation
> records, the spurious entries correspond to the R_PPC64_ENTRY records:
> RELOCATION RECORDS FOR [.text.unlikely]:
> OFFSET TYPE VALUE
> 0000000000000000 R_PPC64_REL64 .TOC.-0x0000000000000008
> 0000000000000008 R_PPC64_ENTRY *ABS*
> 0000000000000014 R_PPC64_REL24 _mcount
> <snip>
>
> The problem is that we are not validating the return value from
> get_mcountsym() in sift_rel_mcount(). With this entry, mcountsym is 0,
> but Elf_r_sym(relp) also ends up being 0. Fix this by ensuring mcountsym
> is valid before processing the entry.
>
> Fixes: c7d64b560ce80 ("powerpc/ftrace: Enable C Version of recordmcount")
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> scripts/recordmcount.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Hi,
linuxppc/merge branch latest commit 850f6274c5b5 was failing to boot IBM
Power8 Box, and the failure got resolved with this patch.
https://github.com/linuxppc/issues/issues/254
# git log -2 --oneline
0e0f55b31ea8 (HEAD -> issue_254) recordmcount: Fix spurious mcount entries on powerpc
850f6274c5b5 (origin/merge, merge) Automatic merge of branches 'master', 'next' and 'fixes' into merge
# uname -r
5.2.0-rc6-00123-g0e0f55b31ea8
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Regards,
-Satheesh
>
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 13c5e6c8829c..47fca2c69a73 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -325,7 +325,8 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
> if (!mcountsym)
> mcountsym = get_mcountsym(sym0, relp, str0);
>
> - if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> + if (mcountsym && mcountsym == Elf_r_sym(relp) &&
> + !is_fake_mcount(relp)) {
> uint_t const addend =
> _w(_w(relp->r_offset) - recval + mcount_adjust);
> mrelp->r_offset = _w(offbase
> --
> 2.22.0
>
^ permalink raw reply
* [PATCH v2 0/7] powerpc/ftrace: Patch out -mprofile-kernel instructions
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
Changes since v1
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=114556):
- Patches 1,2,3 and 6: No changes
- Patch 4: Add smp_call_function() to flush icache on all cpus after
patching in the 'mflr r0' instruction.
- Patch 5: Changes as per Steven Rostedt's suggestions.
- Patch 7: Changes as per Masami and Joe Perches.
--
On powerpc64, -mprofile-kernel results in two instructions being
emitted: 'mflr r0' and 'bl _mcount'. So far, we were only nop'ing out
the branch to _mcount(). This series implements an approach to also nop
out the preceding mflr.
- Naveen
Naveen N. Rao (7):
ftrace: Expose flags used for ftrace_replace_code()
x86/ftrace: Fix use of flags in ftrace_replace_code()
ftrace: Expose __ftrace_replace_code()
powerpc/ftrace: Additionally nop out the preceding mflr with
-mprofile-kernel
ftrace: Update ftrace_location() for powerpc -mprofile-kernel
kprobes/ftrace: Use ftrace_location() when [dis]arming probes
powerpc/kprobes: Allow probing on any ftrace address
arch/powerpc/include/asm/ftrace.h | 8 +
arch/powerpc/kernel/kprobes-ftrace.c | 32 +++-
arch/powerpc/kernel/trace/ftrace.c | 258 ++++++++++++++++++++++++---
arch/x86/kernel/ftrace.c | 3 +-
include/linux/ftrace.h | 15 ++
kernel/kprobes.c | 10 +-
kernel/trace/ftrace.c | 15 +-
7 files changed, 302 insertions(+), 39 deletions(-)
--
2.22.0
^ permalink raw reply
* Re: [PATCH 0/5] Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Aaron Lindsay OS @ 2019-06-27 11:17 UTC (permalink / raw)
To: Linus Torvalds
Cc: Michal Hocko, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H . Peter Anvin,
sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, x86@kernel.org, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Vlastimil Babka,
Open Source Submission, Pavel Tatashin, Vasily Gorbik,
Will Deacon, Borislav Petkov, Thomas Gleixner, Hoan Tran OS,
Oscar Salvador, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <CAADWXX8wdEPNZ26SFJUfwrhQson3HPTrZ7D2jju3RhEeMuc+QQ@mail.gmail.com>
On Jun 26 08:28, Linus Torvalds wrote:
> This is not a comment on the patch series itself, it is a comment on the emails.
>
> Your email is mis-configured and ends up all being marked as spam for
> me, because you go through the wrong smtp server (or maybe your smtp
> server itself is miconfigured)
>
> All your emails fail dmarc, because the "From" header is
> os.amperecomputing.com, but the DKIM signature is for
> amperemail.onmicrosoft.com.
It appears Microsoft enables DKIM by default, but does so using keys
advertised at *.onmicrosoft.com domains, and our IT folks didn't
initially notice this. We believe we've rectified the situation, so
please let us know if our emails (including this one) continue to be an
issue.
> End result: it wil all go into the spam box of anybody who checks DKIM.
Interestingly, *some* receiving mail servers (at least gmail.com) were
reporting DKIM/DMARC pass for emails sent directly from our domain
(though not those sent through a mailing list), which I believe allowed
our IT to falsely think they had everything configured correctly.
-Aaron
^ permalink raw reply
* [PATCH v2 1/7] ftrace: Expose flags used for ftrace_replace_code()
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
Since ftrace_replace_code() is a __weak function and can be overridden,
we need to expose the flags that can be set. So, move the flags enum to
the header file.
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
include/linux/ftrace.h | 5 +++++
kernel/trace/ftrace.c | 5 -----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 25e2995d4a4c..e97789c95c4e 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -162,6 +162,11 @@ enum {
FTRACE_OPS_FL_TRACE_ARRAY = 1 << 15,
};
+enum {
+ FTRACE_MODIFY_ENABLE_FL = (1 << 0),
+ FTRACE_MODIFY_MAY_SLEEP_FL = (1 << 1),
+};
+
#ifdef CONFIG_DYNAMIC_FTRACE
/* The hash used to know what functions callbacks trace */
struct ftrace_ops_hash {
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 38277af44f5c..5710a6b3edc1 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -75,11 +75,6 @@
#define INIT_OPS_HASH(opsname)
#endif
-enum {
- FTRACE_MODIFY_ENABLE_FL = (1 << 0),
- FTRACE_MODIFY_MAY_SLEEP_FL = (1 << 1),
-};
-
struct ftrace_ops ftrace_list_end __read_mostly = {
.func = ftrace_stub,
.flags = FTRACE_OPS_FL_RECURSION_SAFE | FTRACE_OPS_FL_STUB,
--
2.22.0
^ permalink raw reply related
* [PATCH v2 2/7] x86/ftrace: Fix use of flags in ftrace_replace_code()
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be
schedulable), the generic ftrace_replace_code() function was modified to
accept a flags argument in place of a single 'enable' flag. However, the
x86 version of this function was not updated. Fix the same.
Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/x86/kernel/ftrace.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 0927bb158ffc..f34005a17051 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -573,8 +573,9 @@ static void run_sync(void)
local_irq_disable();
}
-void ftrace_replace_code(int enable)
+void ftrace_replace_code(int mod_flags)
{
+ int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
struct ftrace_rec_iter *iter;
struct dyn_ftrace *rec;
const char *report = "adding breakpoints";
--
2.22.0
^ permalink raw reply related
* [PATCH v2 3/7] ftrace: Expose __ftrace_replace_code()
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
While over-riding ftrace_replace_code(), we still want to reuse the
existing __ftrace_replace_code() function. Rename the function and
make it available for other kernel code.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c | 8 ++++----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index e97789c95c4e..fa653a561da5 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -456,6 +456,7 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char *buf, int enable);
/* defined in arch */
extern int ftrace_ip_converted(unsigned long ip);
extern int ftrace_dyn_arch_init(void);
+extern int ftrace_replace_code_rec(struct dyn_ftrace *rec, int enable);
extern void ftrace_replace_code(int enable);
extern int ftrace_update_ftrace_func(ftrace_func_t func);
extern void ftrace_caller(void);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5710a6b3edc1..21d8e201ee80 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2351,8 +2351,8 @@ unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec)
return (unsigned long)FTRACE_ADDR;
}
-static int
-__ftrace_replace_code(struct dyn_ftrace *rec, int enable)
+int
+ftrace_replace_code_rec(struct dyn_ftrace *rec, int enable)
{
unsigned long ftrace_old_addr;
unsigned long ftrace_addr;
@@ -2403,7 +2403,7 @@ void __weak ftrace_replace_code(int mod_flags)
if (rec->flags & FTRACE_FL_DISABLED)
continue;
- failed = __ftrace_replace_code(rec, enable);
+ failed = ftrace_replace_code_rec(rec, enable);
if (failed) {
ftrace_bug(failed, rec);
/* Stop processing */
@@ -5827,7 +5827,7 @@ void ftrace_module_enable(struct module *mod)
rec->flags = cnt;
if (ftrace_start_up && cnt) {
- int failed = __ftrace_replace_code(rec, 1);
+ int failed = ftrace_replace_code_rec(rec, 1);
if (failed) {
ftrace_bug(failed, rec);
goto out_loop;
--
2.22.0
^ permalink raw reply related
* [PATCH v2 4/7] powerpc/ftrace: Additionally nop out the preceding mflr with -mprofile-kernel
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
With -mprofile-kernel, gcc emits 'mflr r0', followed by 'bl _mcount' to
enable function tracing and profiling. So far, with dynamic ftrace, we
used to only patch out the branch to _mcount(). However, mflr is
executed by the branch unit that can only execute one per cycle on
POWER9 and shared with branches, so it would be nice to avoid it where
possible.
We cannot simply nop out the mflr either. When enabling function
tracing, there can be a race if tracing is enabled when some thread was
interrupted after executing a nop'ed out mflr. In this case, the thread
would execute the now-patched-in branch to _mcount() without having
executed the preceding mflr.
To solve this, we now enable function tracing in 2 steps: patch in the
mflr instruction, use 'smp_call_function(isync);
synchronize_rcu_tasks()' to ensure all existing threads make progress,
and then patch in the branch to _mcount(). We override
ftrace_replace_code() with a powerpc64 variant for this purpose.
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/trace/ftrace.c | 258 ++++++++++++++++++++++++++---
1 file changed, 236 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 517662a56bdc..86c2b50dcaa9 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -125,7 +125,7 @@ __ftrace_make_nop(struct module *mod,
{
unsigned long entry, ptr, tramp;
unsigned long ip = rec->ip;
- unsigned int op, pop;
+ unsigned int op;
/* read where this goes */
if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
@@ -160,8 +160,6 @@ __ftrace_make_nop(struct module *mod,
#ifdef CONFIG_MPROFILE_KERNEL
/* When using -mkernel_profile there is no load to jump over */
- pop = PPC_INST_NOP;
-
if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
pr_err("Fetching instruction at %lx failed.\n", ip - 4);
return -EFAULT;
@@ -169,26 +167,23 @@ __ftrace_make_nop(struct module *mod,
/* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
- pr_err("Unexpected instruction %08x around bl _mcount\n", op);
+ pr_err("Unexpected instruction %08x before bl _mcount\n", op);
return -EINVAL;
}
-#else
- /*
- * Our original call site looks like:
- *
- * bl <tramp>
- * ld r2,XX(r1)
- *
- * Milton Miller pointed out that we can not simply nop the branch.
- * If a task was preempted when calling a trace function, the nops
- * will remove the way to restore the TOC in r2 and the r2 TOC will
- * get corrupted.
- *
- * Use a b +8 to jump over the load.
- */
- pop = PPC_INST_BRANCH | 8; /* b +8 */
+ /* We should patch out the bl to _mcount first */
+ if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
+ pr_err("Patching NOP failed.\n");
+ return -EPERM;
+ }
+ /* then, nop out the preceding 'mflr r0' as an optimization */
+ if (op == PPC_INST_MFLR &&
+ patch_instruction((unsigned int *)(ip - 4), PPC_INST_NOP)) {
+ pr_err("Patching NOP failed.\n");
+ return -EPERM;
+ }
+#else
/*
* Check what is in the next instruction. We can see ld r2,40(r1), but
* on first pass after boot we will see mflr r0.
@@ -202,12 +197,25 @@ __ftrace_make_nop(struct module *mod,
pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
return -EINVAL;
}
-#endif /* CONFIG_MPROFILE_KERNEL */
- if (patch_instruction((unsigned int *)ip, pop)) {
+ /*
+ * Our original call site looks like:
+ *
+ * bl <tramp>
+ * ld r2,XX(r1)
+ *
+ * Milton Miller pointed out that we can not simply nop the branch.
+ * If a task was preempted when calling a trace function, the nops
+ * will remove the way to restore the TOC in r2 and the r2 TOC will
+ * get corrupted.
+ *
+ * Use a b +8 to jump over the load.
+ */
+ if (patch_instruction((unsigned int *)ip, PPC_INST_BRANCH | 8)) {
pr_err("Patching NOP failed.\n");
return -EPERM;
}
+#endif /* CONFIG_MPROFILE_KERNEL */
return 0;
}
@@ -421,6 +429,26 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
return -EPERM;
}
+#ifdef CONFIG_MPROFILE_KERNEL
+ /* Nop out the preceding 'mflr r0' as an optimization */
+ if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
+ pr_err("Fetching instruction at %lx failed.\n", ip - 4);
+ return -EFAULT;
+ }
+
+ /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
+ if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
+ pr_err("Unexpected instruction %08x before bl _mcount\n", op);
+ return -EINVAL;
+ }
+
+ if (op == PPC_INST_MFLR &&
+ patch_instruction((unsigned int *)(ip - 4), PPC_INST_NOP)) {
+ pr_err("Patching NOP failed.\n");
+ return -EPERM;
+ }
+#endif
+
return 0;
}
@@ -429,6 +457,7 @@ int ftrace_make_nop(struct module *mod,
{
unsigned long ip = rec->ip;
unsigned int old, new;
+ int rc;
/*
* If the calling address is more that 24 bits away,
@@ -439,7 +468,34 @@ int ftrace_make_nop(struct module *mod,
/* within range */
old = ftrace_call_replace(ip, addr, 1);
new = PPC_INST_NOP;
- return ftrace_modify_code(ip, old, new);
+ rc = ftrace_modify_code(ip, old, new);
+#ifdef CONFIG_MPROFILE_KERNEL
+ if (rc)
+ return rc;
+
+ /*
+ * For -mprofile-kernel, we patch out the preceding 'mflr r0'
+ * instruction, as an optimization. It is important to nop out
+ * the branch to _mcount() first, as a lone 'mflr r0' is
+ * harmless.
+ */
+ if (probe_kernel_read(&old, (void *)(ip - 4), 4)) {
+ pr_err("Fetching instruction at %lx failed.\n", ip - 4);
+ return -EFAULT;
+ }
+
+ /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
+ if (old != PPC_INST_MFLR && old != PPC_INST_STD_LR) {
+ pr_err("Unexpected instruction %08x before bl _mcount\n",
+ old);
+ return -EINVAL;
+ }
+
+ if (old == PPC_INST_MFLR)
+ rc = patch_instruction((unsigned int *)(ip - 4),
+ PPC_INST_NOP);
+#endif
+ return rc;
} else if (core_kernel_text(ip))
return __ftrace_make_nop_kernel(rec, addr);
@@ -567,6 +623,37 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
return -EINVAL;
}
+#ifdef CONFIG_MPROFILE_KERNEL
+ /*
+ * We could end up here without having called __ftrace_make_call_prep()
+ * if function tracing is enabled before a module is loaded.
+ *
+ * ftrace_module_enable() --> ftrace_replace_code_rec() -->
+ * ftrace_make_call() --> __ftrace_make_call()
+ *
+ * In this scenario, the previous instruction will be a NOP. It is
+ * safe to patch it with a 'mflr r0' since we know for a fact that
+ * this code is not yet being run.
+ */
+ ip -= MCOUNT_INSN_SIZE;
+
+ /* read where this goes */
+ if (probe_kernel_read(op, ip, MCOUNT_INSN_SIZE))
+ return -EFAULT;
+
+ /*
+ * nothing to do if this is using the older -mprofile-kernel
+ * instruction sequence
+ */
+ if (op[0] != PPC_INST_NOP)
+ return 0;
+
+ if (patch_instruction((unsigned int *)ip, PPC_INST_MFLR)) {
+ pr_err("Patching MFLR failed.\n");
+ return -EPERM;
+ }
+#endif
+
return 0;
}
@@ -863,6 +950,133 @@ void arch_ftrace_update_code(int command)
ftrace_modify_all_code(command);
}
+#ifdef CONFIG_MPROFILE_KERNEL
+/* Returns 1 if we patched in the mflr */
+static int __ftrace_make_call_prep(struct dyn_ftrace *rec)
+{
+ void *ip = (void *)rec->ip - MCOUNT_INSN_SIZE;
+ unsigned int op[2];
+
+ /* read where this goes */
+ if (probe_kernel_read(op, ip, sizeof(op)))
+ return -EFAULT;
+
+ if (op[1] != PPC_INST_NOP) {
+ pr_err("Unexpected call sequence at %p: %x %x\n",
+ ip, op[0], op[1]);
+ return -EINVAL;
+ }
+
+ /*
+ * nothing to do if this is using the older -mprofile-kernel
+ * instruction sequence
+ */
+ if (op[0] != PPC_INST_NOP)
+ return 0;
+
+ if (patch_instruction((unsigned int *)ip, PPC_INST_MFLR)) {
+ pr_err("Patching MFLR failed.\n");
+ return -EPERM;
+ }
+
+ return 1;
+}
+
+static void do_isync(void *info __maybe_unused)
+{
+ isync();
+}
+
+/*
+ * When enabling function tracing for -mprofile-kernel that uses a
+ * 2-instruction sequence of 'mflr r0, bl _mcount()', we use a 2 step process:
+ * 1. Patch in the 'mflr r0' instruction
+ * 1a. flush icache on all cpus, so that the updated instruction is seen
+ * 1b. synchronize_rcu_tasks() to ensure that any cpus that had executed
+ * the earlier nop there make progress (and hence do not branch into
+ * ftrace without executing the preceding mflr)
+ * 2. Patch in the branch to ftrace
+ */
+void ftrace_replace_code(int mod_flags)
+{
+ int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
+ int schedulable = mod_flags & FTRACE_MODIFY_MAY_SLEEP_FL;
+ int ret, failed, make_call = 0;
+ struct ftrace_rec_iter *iter;
+ struct dyn_ftrace *rec;
+
+ if (unlikely(!ftrace_enabled))
+ return;
+
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
+
+ if (rec->flags & FTRACE_FL_DISABLED)
+ continue;
+
+ failed = 0;
+ ret = ftrace_test_record(rec, enable);
+ if (ret == FTRACE_UPDATE_MAKE_CALL) {
+ failed = __ftrace_make_call_prep(rec);
+ if (failed < 0) {
+ ftrace_bug(failed, rec);
+ return;
+ } else if (failed == 1)
+ make_call++;
+ }
+
+ if (!failed) {
+ /* We can patch the record directly */
+ failed = ftrace_replace_code_rec(rec, enable);
+ if (failed) {
+ ftrace_bug(failed, rec);
+ return;
+ }
+ }
+
+ if (schedulable)
+ cond_resched();
+ }
+
+ if (!make_call)
+ /* No records needed patching a preceding mflr */
+ return;
+
+ /* Make sure all cpus see the new instruction */
+ smp_call_function(do_isync, NULL, 1);
+
+ /*
+ * We also need to ensure that all cpus make progress:
+ * - With !CONFIG_PREEMPT, we want to be sure that cpus return from
+ * any interrupts they may be handling, and make progress.
+ * - With CONFIG_PREEMPT, we want to be additionally sure that there
+ * are no pre-empted tasks that have executed the earlier nop, and
+ * might end up executing the subsequently patched branch to ftrace.
+ */
+ synchronize_rcu_tasks();
+
+ for_ftrace_rec_iter(iter) {
+ rec = ftrace_rec_iter_record(iter);
+
+ if (rec->flags & FTRACE_FL_DISABLED)
+ continue;
+
+ ret = ftrace_test_record(rec, enable);
+ if (ret == FTRACE_UPDATE_MAKE_CALL)
+ failed = ftrace_replace_code_rec(rec, enable);
+
+ if (failed) {
+ ftrace_bug(failed, rec);
+ return;
+ }
+
+ if (schedulable)
+ cond_resched();
+ }
+
+}
+#endif
+
#ifdef CONFIG_PPC64
#define PACATOC offsetof(struct paca_struct, kernel_toc)
--
2.22.0
^ permalink raw reply related
* [PATCH v2 5/7] ftrace: Update ftrace_location() for powerpc -mprofile-kernel
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
Now that we are patching the preceding 'mflr r0' instruction with
-mprofile-kernel, we need to update ftrace_location() to recognise that
as being part of ftrace.
To do this, we introduce FTRACE_IP_EXTENSION to denote the length (in
bytes) of the mcount caller. By default, this is set to 0. For powerpc
with CONFIG_MPROFILE_KERNEL, we set this to MCOUNT_INSN_SIZE so that
this works if ftrace_location() is called with the address of the actual
ftrace entry call, or with the address of the preceding 'mflr r0'
instruction.
Note that we do not check if the preceding instruction is indeed the
'mflr r0'. Earlier -mprofile-kernel ABI included a 'std r0,stack'
instruction between the 'mflr r0' and the 'bl _mcount'. This is harmless
as the 'std r0,stack' instruction is inconsequential and is not relied
upon.
Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/ftrace.h | 8 ++++++++
include/linux/ftrace.h | 9 +++++++++
kernel/trace/ftrace.c | 2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 3dfb80b86561..510a8ac8ac8d 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -61,6 +61,14 @@ struct dyn_arch_ftrace {
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
#define ARCH_SUPPORTS_FTRACE_OPS 1
+
+#ifdef CONFIG_MPROFILE_KERNEL
+/*
+ * We consider two instructions -- 'mflr r0', 'bl _mcount' -- to be part
+ * of ftrace with -mprofile-kernel
+ */
+#define FTRACE_IP_EXTENSION MCOUNT_INSN_SIZE
+#endif
#endif
#endif /* CONFIG_FUNCTION_TRACER */
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index fa653a561da5..310b514438cb 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -29,6 +29,15 @@
#define ARCH_SUPPORTS_FTRACE_OPS 0
#endif
+/*
+ * This denotes the number of instructions (in bytes) that is used by the
+ * arch mcount caller. All instructions in this range will be owned by
+ * ftrace.
+ */
+#ifndef FTRACE_IP_EXTENSION
+#define FTRACE_IP_EXTENSION 0
+#endif
+
/*
* If the arch's mcount caller does not support all of ftrace's
* features, then it must call an indirect function that
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 21d8e201ee80..308555925b81 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1575,7 +1575,7 @@ unsigned long ftrace_location_range(unsigned long start, unsigned long end)
*/
unsigned long ftrace_location(unsigned long ip)
{
- return ftrace_location_range(ip, ip);
+ return ftrace_location_range(ip, ip + FTRACE_IP_EXTENSION);
}
/**
--
2.22.0
^ permalink raw reply related
* [PATCH v2 7/7] powerpc/kprobes: Allow probing on any ftrace address
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
With KPROBES_ON_FTRACE, kprobe is allowed to be inserted on instructions
that branch to _mcount (referred to as ftrace location). With
-mprofile-kernel, we now include the preceding 'mflr r0' as being part
of the ftrace location.
However, by default, probing on an instruction that is not actually the
branch to _mcount() is prohibited, as that is considered to not be at an
instruction boundary. This is not the case on powerpc, so allow the same
by overriding arch_check_ftrace_location()
In addition, we update kprobe_ftrace_handler() to detect this scenarios
and to pass the proper nip to the pre and post probe handlers.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/kprobes-ftrace.c | 32 +++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kprobes-ftrace.c b/arch/powerpc/kernel/kprobes-ftrace.c
index 972cb28174b2..23c840748183 100644
--- a/arch/powerpc/kernel/kprobes-ftrace.c
+++ b/arch/powerpc/kernel/kprobes-ftrace.c
@@ -12,15 +12,35 @@
#include <linux/preempt.h>
#include <linux/ftrace.h>
+/*
+ * With -mprofile-kernel, we patch two instructions -- the branch to _mcount
+ * as well as the preceding 'mflr r0'. Both these instructions are claimed
+ * by ftrace and we should allow probing on either instruction.
+ */
+int arch_check_ftrace_location(struct kprobe *p)
+{
+ if (ftrace_location((unsigned long)p->addr))
+ p->flags |= KPROBE_FLAG_FTRACE;
+ return 0;
+}
+
/* Ftrace callback handler for kprobes */
void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
struct ftrace_ops *ops, struct pt_regs *regs)
{
struct kprobe *p;
+ int mflr_kprobe = 0;
struct kprobe_ctlblk *kcb;
p = get_kprobe((kprobe_opcode_t *)nip);
- if (unlikely(!p) || kprobe_disabled(p))
+ if (!p) {
+ p = get_kprobe((kprobe_opcode_t *)(nip - MCOUNT_INSN_SIZE));
+ if (unlikely(!p))
+ return;
+ mflr_kprobe = 1;
+ }
+
+ if (kprobe_disabled(p))
return;
kcb = get_kprobe_ctlblk();
@@ -33,6 +53,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
*/
regs->nip -= MCOUNT_INSN_SIZE;
+ if (mflr_kprobe)
+ regs->nip -= MCOUNT_INSN_SIZE;
+
__this_cpu_write(current_kprobe, p);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
if (!p->pre_handler || !p->pre_handler(p, regs)) {
@@ -45,6 +68,8 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
kcb->kprobe_status = KPROBE_HIT_SSDONE;
p->post_handler(p, regs, 0);
}
+ if (mflr_kprobe)
+ regs->nip += MCOUNT_INSN_SIZE;
}
/*
* If pre_handler returns !0, it changes regs->nip. We have to
@@ -57,6 +82,11 @@ NOKPROBE_SYMBOL(kprobe_ftrace_handler);
int arch_prepare_kprobe_ftrace(struct kprobe *p)
{
+ if ((unsigned long)p->addr & 0x03) {
+ pr_err("Attempt to register kprobe at an unaligned address\n");
+ return -EILSEQ;
+ }
+
p->ainsn.insn = NULL;
p->ainsn.boostable = -1;
return 0;
--
2.22.0
^ permalink raw reply related
* [PATCH v2 6/7] kprobes/ftrace: Use ftrace_location() when [dis]arming probes
From: Naveen N. Rao @ 2019-06-27 11:23 UTC (permalink / raw)
To: Michael Ellerman, Steven Rostedt, Masami Hiramatsu, Ingo Molnar,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
Ftrace location could include more than a single instruction in case of
some architectures (powerpc64, for now). In this case, kprobe is
permitted on any of those instructions, and uses ftrace infrastructure
for functioning.
However, [dis]arm_kprobe_ftrace() uses the kprobe address when setting
up ftrace filter IP. This won't work if the address points to any
instruction apart from the one that has a branch to _mcount(). To
resolve this, have [dis]arm_kprobe_ftrace() use ftrace_function() to
identify the filter IP.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
kernel/kprobes.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 445337c107e0..282ee704e2d8 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -978,10 +978,10 @@ static int prepare_kprobe(struct kprobe *p)
/* Caller must lock kprobe_mutex */
static int arm_kprobe_ftrace(struct kprobe *p)
{
+ unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
int ret = 0;
- ret = ftrace_set_filter_ip(&kprobe_ftrace_ops,
- (unsigned long)p->addr, 0, 0);
+ ret = ftrace_set_filter_ip(&kprobe_ftrace_ops, ftrace_ip, 0, 0);
if (ret) {
pr_debug("Failed to arm kprobe-ftrace at %pS (%d)\n",
p->addr, ret);
@@ -1005,13 +1005,14 @@ static int arm_kprobe_ftrace(struct kprobe *p)
* non-empty filter_hash for IPMODIFY ops, we're safe from an accidental
* empty filter_hash which would undesirably trace all functions.
*/
- ftrace_set_filter_ip(&kprobe_ftrace_ops, (unsigned long)p->addr, 1, 0);
+ ftrace_set_filter_ip(&kprobe_ftrace_ops, ftrace_ip, 1, 0);
return ret;
}
/* Caller must lock kprobe_mutex */
static int disarm_kprobe_ftrace(struct kprobe *p)
{
+ unsigned long ftrace_ip = ftrace_location((unsigned long)p->addr);
int ret = 0;
if (kprobe_ftrace_enabled == 1) {
@@ -1022,8 +1023,7 @@ static int disarm_kprobe_ftrace(struct kprobe *p)
kprobe_ftrace_enabled--;
- ret = ftrace_set_filter_ip(&kprobe_ftrace_ops,
- (unsigned long)p->addr, 1, 0);
+ ret = ftrace_set_filter_ip(&kprobe_ftrace_ops, ftrace_ip, 1, 0);
WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (%d)\n",
p->addr, ret);
return ret;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 2/7] x86/ftrace: Fix use of flags in ftrace_replace_code()
From: Naveen N. Rao @ 2019-06-27 11:27 UTC (permalink / raw)
To: Masami Hiramatsu, Ingo Molnar, Michael Ellerman, Nicholas Piggin,
Steven Rostedt
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <abc56ad177f370ec423edcfc538d35b418c1808e.1561634177.git.naveen.n.rao@linux.v net.ibm.com>
Naveen N. Rao wrote:
> In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be
> schedulable), the generic ftrace_replace_code() function was modified to
> accept a flags argument in place of a single 'enable' flag. However, the
> x86 version of this function was not updated. Fix the same.
>
> Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable")
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/ftrace.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
If the rest of this series is ok, and if Ingo and Steven are ok to have
this series go through the powerpc tree, then I can re-post this
particular patch for x86 after -rc1.
Thanks,
Naveen
^ permalink raw reply
* Re: [PATCH] recordmcount: Fix spurious mcount entries on powerpc
From: Steven Rostedt @ 2019-06-27 13:17 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Naveen N. Rao, linuxppc-dev, linux-kernel
In-Reply-To: <8736jvtvvg.fsf@concordia.ellerman.id.au>
On Thu, 27 Jun 2019 15:55:47 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Steve are you OK if I merge this via the powerpc tree? I'll reword the
> commit message so that it makes sense coming prior to the commit
> mentioned above.
Yes, please add:
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v2 2/7] x86/ftrace: Fix use of flags in ftrace_replace_code()
From: Steven Rostedt @ 2019-06-27 13:29 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, Nicholas Piggin, Masami Hiramatsu, linuxppc-dev,
Ingo Molnar
In-Reply-To: <abc56ad177f370ec423edcfc538d35b418c1808e.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
On Thu, 27 Jun 2019 16:53:50 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be
> schedulable), the generic ftrace_replace_code() function was modified to
> accept a flags argument in place of a single 'enable' flag. However, the
> x86 version of this function was not updated. Fix the same.
>
> Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable")
I don't mind this change, but it's not a bug, and I'm not sure it
should have the fixes tag. The reason being, the
FTRACE_MODIFY_ENABLE_FL is only set when ftrace is called by with the
command flag FTRACE_MAY_SLEEP, which is never done on x86.
That said, I'm fine with the change as it makes it more robust, but by
adding the fixes tag, you're going to get this into all the stable
code, and I'm not sure that's really necessary.
-- Steve
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/ftrace.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 0927bb158ffc..f34005a17051 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -573,8 +573,9 @@ static void run_sync(void)
> local_irq_disable();
> }
>
> -void ftrace_replace_code(int enable)
> +void ftrace_replace_code(int mod_flags)
> {
> + int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
> struct ftrace_rec_iter *iter;
> struct dyn_ftrace *rec;
> const char *report = "adding breakpoints";
^ permalink raw reply
* Re: [PATCH v3 1/2] pid: add pidfd_open()
From: Konstantin Khlebnikov @ 2019-06-27 13:57 UTC (permalink / raw)
To: Christian Brauner, jannh, oleg, viro, torvalds, linux-kernel,
arnd
Cc: linux-ia64, linux-sh, linux-mips, dhowells, joel, linux-kselftest,
sparclinux, elena.reshetova, linux-arch, linux-s390, dancol,
kernel-team, serge, linux-xtensa, keescook, linux-m68k, luto,
tglx, surenb, linux-arm-kernel, linux-parisc, linux-api, cyphar,
luto, ebiederm, linux-alpha, akpm, linuxppc-dev
In-Reply-To: <20190520155630.21684-1-christian@brauner.io>
On 20.05.2019 18:56, Christian Brauner wrote:
> This adds the pidfd_open() syscall. It allows a caller to retrieve pollable
> pidfds for a process which did not get created via CLONE_PIDFD, i.e. for a
> process that is created via traditional fork()/clone() calls that is only
> referenced by a PID:
>
> int pidfd = pidfd_open(1234, 0);
> ret = pidfd_send_signal(pidfd, SIGSTOP, NULL, 0);
>
> With the introduction of pidfds through CLONE_PIDFD it is possible to
> created pidfds at process creation time.
> However, a lot of processes get created with traditional PID-based calls
> such as fork() or clone() (without CLONE_PIDFD). For these processes a
> caller can currently not create a pollable pidfd. This is a problem for
> Android's low memory killer (LMK) and service managers such as systemd.
> Both are examples of tools that want to make use of pidfds to get reliable
> notification of process exit for non-parents (pidfd polling) and race-free
> signal sending (pidfd_send_signal()). They intend to switch to this API for
> process supervision/management as soon as possible. Having no way to get
> pollable pidfds from PID-only processes is one of the biggest blockers for
> them in adopting this api. With pidfd_open() making it possible to retrieve
> pidfds for PID-based processes we enable them to adopt this api.
>
> In line with Arnd's recent changes to consolidate syscall numbers across
> architectures, I have added the pidfd_open() syscall to all architectures
> at the same time.
As I see pidfd_open() works only within current pid-namespace.
Have you considered separate argument for pidns-fd or flag for opening pid in
pid-ns referred by nsproxy->pid_ns_for_children set by setns.
This could be used for use cases I've tried to cover by syscall "translate_pid"
https://lkml.org/lkml/2018/6/1/788
>
> Signed-off-by: Christian Brauner <christian@brauner.io>
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jann Horn <jannh@google.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Andy Lutomirsky <luto@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-api@vger.kernel.org
> ---
> v1:
> - kbuild test robot <lkp@intel.com>:
> - add missing entry for pidfd_open to arch/arm/tools/syscall.tbl
> - Oleg Nesterov <oleg@redhat.com>:
> - use simpler thread-group leader check
> v2:
> - Oleg Nesterov <oleg@redhat.com>:
> - avoid using additional variable
> - remove unneeded comment
> - Arnd Bergmann <arnd@arndb.de>:
> - switch from 428 to 434 since the new mount api has taken it
> - bump syscall numbers in arch/arm64/include/asm/unistd.h
> - Joel Fernandes (Google) <joel@joelfernandes.org>:
> - switch from ESRCH to EINVAL when the passed-in pid does not refer to a
> thread-group leader
> - Christian Brauner <christian@brauner.io>:
> - rebase on v5.2-rc1
> - adapt syscall number to account for new mount api syscalls
> v3:
> - Arnd Bergmann <arnd@arndb.de>:
> - add missing syscall entries for mips-o32 and mips-n64
> ---
> arch/alpha/kernel/syscalls/syscall.tbl | 1 +
> arch/arm/tools/syscall.tbl | 1 +
> arch/arm64/include/asm/unistd.h | 2 +-
> arch/arm64/include/asm/unistd32.h | 2 +
> arch/ia64/kernel/syscalls/syscall.tbl | 1 +
> arch/m68k/kernel/syscalls/syscall.tbl | 1 +
> arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
> arch/parisc/kernel/syscalls/syscall.tbl | 1 +
> arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
> arch/s390/kernel/syscalls/syscall.tbl | 1 +
> arch/sh/kernel/syscalls/syscall.tbl | 1 +
> arch/sparc/kernel/syscalls/syscall.tbl | 1 +
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
> include/linux/pid.h | 1 +
> include/linux/syscalls.h | 1 +
> include/uapi/asm-generic/unistd.h | 4 +-
> kernel/fork.c | 2 +-
> kernel/pid.c | 43 +++++++++++++++++++++
> 23 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index 9e7704e44f6d..1db9bbcfb84e 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -473,3 +473,4 @@
> 541 common fsconfig sys_fsconfig
> 542 common fsmount sys_fsmount
> 543 common fspick sys_fspick
> +544 common pidfd_open sys_pidfd_open
> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index aaf479a9e92d..81e6e1817c45 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -447,3 +447,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h
> index 70e6882853c0..e8f7d95a1481 100644
> --- a/arch/arm64/include/asm/unistd.h
> +++ b/arch/arm64/include/asm/unistd.h
> @@ -44,7 +44,7 @@
> #define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5)
> #define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800)
>
> -#define __NR_compat_syscalls 434
> +#define __NR_compat_syscalls 435
> #endif
>
> #define __ARCH_WANT_SYS_CLONE
> diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> index c39e90600bb3..7a3158ccd68e 100644
> --- a/arch/arm64/include/asm/unistd32.h
> +++ b/arch/arm64/include/asm/unistd32.h
> @@ -886,6 +886,8 @@ __SYSCALL(__NR_fsconfig, sys_fsconfig)
> __SYSCALL(__NR_fsmount, sys_fsmount)
> #define __NR_fspick 433
> __SYSCALL(__NR_fspick, sys_fspick)
> +#define __NR_pidfd_open 434
> +__SYSCALL(__NR_pidfd_open, sys_pidfd_open)
>
> /*
> * Please add new compat syscalls above this comment and update
> diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
> index e01df3f2f80d..ecc44926737b 100644
> --- a/arch/ia64/kernel/syscalls/syscall.tbl
> +++ b/arch/ia64/kernel/syscalls/syscall.tbl
> @@ -354,3 +354,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> index 7e3d0734b2f3..9a3eb2558568 100644
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -433,3 +433,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
> index 26339e417695..ad706f83c755 100644
> --- a/arch/microblaze/kernel/syscalls/syscall.tbl
> +++ b/arch/microblaze/kernel/syscalls/syscall.tbl
> @@ -439,3 +439,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> index 0e2dd68ade57..97035e19ad03 100644
> --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> @@ -372,3 +372,4 @@
> 431 n32 fsconfig sys_fsconfig
> 432 n32 fsmount sys_fsmount
> 433 n32 fspick sys_fspick
> +434 n32 pidfd_open sys_pidfd_open
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index 5eebfa0d155c..d7292722d3b0 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -348,3 +348,4 @@
> 431 n64 fsconfig sys_fsconfig
> 432 n64 fsmount sys_fsmount
> 433 n64 fspick sys_fspick
> +434 n64 pidfd_open sys_pidfd_open
> diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
> index 3cc1374e02d0..dba084c92f14 100644
> --- a/arch/mips/kernel/syscalls/syscall_o32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
> @@ -421,3 +421,4 @@
> 431 o32 fsconfig sys_fsconfig
> 432 o32 fsmount sys_fsmount
> 433 o32 fspick sys_fspick
> +434 o32 pidfd_open sys_pidfd_open
> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> index c9e377d59232..5022b9e179c2 100644
> --- a/arch/parisc/kernel/syscalls/syscall.tbl
> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> @@ -430,3 +430,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index 103655d84b4b..f2c3bda2d39f 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -515,3 +515,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> index e822b2964a83..6ebacfeaf853 100644
> --- a/arch/s390/kernel/syscalls/syscall.tbl
> +++ b/arch/s390/kernel/syscalls/syscall.tbl
> @@ -436,3 +436,4 @@
> 431 common fsconfig sys_fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount sys_fsmount
> 433 common fspick sys_fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open sys_pidfd_open
> diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> index 016a727d4357..834c9c7d79fa 100644
> --- a/arch/sh/kernel/syscalls/syscall.tbl
> +++ b/arch/sh/kernel/syscalls/syscall.tbl
> @@ -436,3 +436,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> index e047480b1605..c58e71f21129 100644
> --- a/arch/sparc/kernel/syscalls/syscall.tbl
> +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> @@ -479,3 +479,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index ad968b7bac72..43e4429a5272 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -438,3 +438,4 @@
> 431 i386 fsconfig sys_fsconfig __ia32_sys_fsconfig
> 432 i386 fsmount sys_fsmount __ia32_sys_fsmount
> 433 i386 fspick sys_fspick __ia32_sys_fspick
> +434 i386 pidfd_open sys_pidfd_open __ia32_sys_pidfd_open
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index b4e6f9e6204a..1bee0a77fdd3 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -355,6 +355,7 @@
> 431 common fsconfig __x64_sys_fsconfig
> 432 common fsmount __x64_sys_fsmount
> 433 common fspick __x64_sys_fspick
> +434 common pidfd_open __x64_sys_pidfd_open
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
> index 5fa0ee1c8e00..782b81945ccc 100644
> --- a/arch/xtensa/kernel/syscalls/syscall.tbl
> +++ b/arch/xtensa/kernel/syscalls/syscall.tbl
> @@ -404,3 +404,4 @@
> 431 common fsconfig sys_fsconfig
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> +434 common pidfd_open sys_pidfd_open
> diff --git a/include/linux/pid.h b/include/linux/pid.h
> index 3c8ef5a199ca..c938a92eab99 100644
> --- a/include/linux/pid.h
> +++ b/include/linux/pid.h
> @@ -67,6 +67,7 @@ struct pid
> extern struct pid init_struct_pid;
>
> extern const struct file_operations pidfd_fops;
> +extern int pidfd_create(struct pid *pid);
>
> static inline struct pid *get_pid(struct pid *pid)
> {
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e2870fe1be5b..989055e0b501 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -929,6 +929,7 @@ asmlinkage long sys_clock_adjtime32(clockid_t which_clock,
> struct old_timex32 __user *tx);
> asmlinkage long sys_syncfs(int fd);
> asmlinkage long sys_setns(int fd, int nstype);
> +asmlinkage long sys_pidfd_open(pid_t pid, unsigned int flags);
> asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
> unsigned int vlen, unsigned flags);
> asmlinkage long sys_process_vm_readv(pid_t pid,
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index a87904daf103..e5684a4512c0 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -844,9 +844,11 @@ __SYSCALL(__NR_fsconfig, sys_fsconfig)
> __SYSCALL(__NR_fsmount, sys_fsmount)
> #define __NR_fspick 433
> __SYSCALL(__NR_fspick, sys_fspick)
> +#define __NR_pidfd_open 434
> +__SYSCALL(__NR_pidfd_open, sys_pidfd_open)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 434
> +#define __NR_syscalls 435
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b4cba953040a..c3df226f47a1 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1724,7 +1724,7 @@ const struct file_operations pidfd_fops = {
> * Return: On success, a cloexec pidfd is returned.
> * On error, a negative errno number will be returned.
> */
> -static int pidfd_create(struct pid *pid)
> +int pidfd_create(struct pid *pid)
> {
> int fd;
>
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 89548d35eefb..8fc9d94f6ac1 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -37,6 +37,7 @@
> #include <linux/syscalls.h>
> #include <linux/proc_ns.h>
> #include <linux/proc_fs.h>
> +#include <linux/sched/signal.h>
> #include <linux/sched/task.h>
> #include <linux/idr.h>
>
> @@ -450,6 +451,48 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
> return idr_get_next(&ns->idr, &nr);
> }
>
> +/**
> + * pidfd_open() - Open new pid file descriptor.
> + *
> + * @pid: pid for which to retrieve a pidfd
> + * @flags: flags to pass
> + *
> + * This creates a new pid file descriptor with the O_CLOEXEC flag set for
> + * the process identified by @pid. Currently, the process identified by
> + * @pid must be a thread-group leader. This restriction currently exists
> + * for all aspects of pidfds including pidfd creation (CLONE_PIDFD cannot
> + * be used with CLONE_THREAD) and pidfd polling (only supports thread group
> + * leaders).
> + *
> + * Return: On success, a cloexec pidfd is returned.
> + * On error, a negative errno number will be returned.
> + */
> +SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags)
> +{
> + int fd, ret;
> + struct pid *p;
> +
> + if (flags)
> + return -EINVAL;
> +
> + if (pid <= 0)
> + return -EINVAL;
> +
> + p = find_get_pid(pid);
> + if (!p)
> + return -ESRCH;
> +
> + ret = 0;
> + rcu_read_lock();
> + if (!pid_task(p, PIDTYPE_TGID))
> + ret = -EINVAL;
> + rcu_read_unlock();
> +
> + fd = ret ?: pidfd_create(p);
> + put_pid(p);
> + return fd;
> +}
> +
> void __init pid_idr_init(void)
> {
> /* Verify no one has done anything silly: */
>
^ permalink raw reply
* Re: [PATCH v2 7/7] powerpc/kprobes: Allow probing on any ftrace address
From: Masami Hiramatsu @ 2019-06-27 14:19 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, Nicholas Piggin, Steven Rostedt, linuxppc-dev,
Ingo Molnar
In-Reply-To: <da4200e511f4863b153b0721f3913a75aa3bf80d.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
On Thu, 27 Jun 2019 16:53:55 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> With KPROBES_ON_FTRACE, kprobe is allowed to be inserted on instructions
> that branch to _mcount (referred to as ftrace location). With
> -mprofile-kernel, we now include the preceding 'mflr r0' as being part
> of the ftrace location.
>
> However, by default, probing on an instruction that is not actually the
> branch to _mcount() is prohibited, as that is considered to not be at an
> instruction boundary. This is not the case on powerpc, so allow the same
> by overriding arch_check_ftrace_location()
>
> In addition, we update kprobe_ftrace_handler() to detect this scenarios
> and to pass the proper nip to the pre and post probe handlers.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> ---
> arch/powerpc/kernel/kprobes-ftrace.c | 32 +++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kprobes-ftrace.c b/arch/powerpc/kernel/kprobes-ftrace.c
> index 972cb28174b2..23c840748183 100644
> --- a/arch/powerpc/kernel/kprobes-ftrace.c
> +++ b/arch/powerpc/kernel/kprobes-ftrace.c
> @@ -12,15 +12,35 @@
> #include <linux/preempt.h>
> #include <linux/ftrace.h>
>
> +/*
> + * With -mprofile-kernel, we patch two instructions -- the branch to _mcount
> + * as well as the preceding 'mflr r0'. Both these instructions are claimed
> + * by ftrace and we should allow probing on either instruction.
> + */
> +int arch_check_ftrace_location(struct kprobe *p)
> +{
> + if (ftrace_location((unsigned long)p->addr))
> + p->flags |= KPROBE_FLAG_FTRACE;
> + return 0;
> +}
> +
> /* Ftrace callback handler for kprobes */
> void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> struct ftrace_ops *ops, struct pt_regs *regs)
> {
> struct kprobe *p;
> + int mflr_kprobe = 0;
> struct kprobe_ctlblk *kcb;
>
> p = get_kprobe((kprobe_opcode_t *)nip);
> - if (unlikely(!p) || kprobe_disabled(p))
> + if (!p) {
> + p = get_kprobe((kprobe_opcode_t *)(nip - MCOUNT_INSN_SIZE));
> + if (unlikely(!p))
> + return;
> + mflr_kprobe = 1;
> + }
> +
> + if (kprobe_disabled(p))
> return;
>
> kcb = get_kprobe_ctlblk();
> @@ -33,6 +53,9 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> */
> regs->nip -= MCOUNT_INSN_SIZE;
>
> + if (mflr_kprobe)
> + regs->nip -= MCOUNT_INSN_SIZE;
> +
> __this_cpu_write(current_kprobe, p);
> kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> if (!p->pre_handler || !p->pre_handler(p, regs)) {
> @@ -45,6 +68,8 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> kcb->kprobe_status = KPROBE_HIT_SSDONE;
> p->post_handler(p, regs, 0);
> }
> + if (mflr_kprobe)
> + regs->nip += MCOUNT_INSN_SIZE;
> }
> /*
> * If pre_handler returns !0, it changes regs->nip. We have to
> @@ -57,6 +82,11 @@ NOKPROBE_SYMBOL(kprobe_ftrace_handler);
>
> int arch_prepare_kprobe_ftrace(struct kprobe *p)
> {
> + if ((unsigned long)p->addr & 0x03) {
> + pr_err("Attempt to register kprobe at an unaligned address\n");
> + return -EILSEQ;
> + }
> +
> p->ainsn.insn = NULL;
> p->ainsn.boostable = -1;
> return 0;
> --
> 2.22.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 3/7] ftrace: Expose __ftrace_replace_code()
From: Steven Rostedt @ 2019-06-27 14:36 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, Nicholas Piggin, Masami Hiramatsu, linuxppc-dev,
Ingo Molnar
In-Reply-To: <0d09c94f19332b13707109f41cd15b6e0c45d120.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
On Thu, 27 Jun 2019 16:53:51 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> While over-riding ftrace_replace_code(), we still want to reuse the
> existing __ftrace_replace_code() function. Rename the function and
> make it available for other kernel code.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> include/linux/ftrace.h | 1 +
> kernel/trace/ftrace.c | 8 ++++----
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index e97789c95c4e..fa653a561da5 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -456,6 +456,7 @@ ftrace_set_early_filter(struct ftrace_ops *ops, char *buf, int enable);
> /* defined in arch */
> extern int ftrace_ip_converted(unsigned long ip);
> extern int ftrace_dyn_arch_init(void);
> +extern int ftrace_replace_code_rec(struct dyn_ftrace *rec, int enable);
> extern void ftrace_replace_code(int enable);
> extern int ftrace_update_ftrace_func(ftrace_func_t func);
> extern void ftrace_caller(void);
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5710a6b3edc1..21d8e201ee80 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2351,8 +2351,8 @@ unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec)
> return (unsigned long)FTRACE_ADDR;
> }
>
> -static int
> -__ftrace_replace_code(struct dyn_ftrace *rec, int enable)
> +int
> +ftrace_replace_code_rec(struct dyn_ftrace *rec, int enable)
Make this a single line, as it removes static and "__" which should
keep it normal.
Other than that,
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> {
> unsigned long ftrace_old_addr;
> unsigned long ftrace_addr;
> @@ -2403,7 +2403,7 @@ void __weak ftrace_replace_code(int mod_flags)
> if (rec->flags & FTRACE_FL_DISABLED)
> continue;
>
> - failed = __ftrace_replace_code(rec, enable);
> + failed = ftrace_replace_code_rec(rec, enable);
> if (failed) {
> ftrace_bug(failed, rec);
> /* Stop processing */
> @@ -5827,7 +5827,7 @@ void ftrace_module_enable(struct module *mod)
> rec->flags = cnt;
>
> if (ftrace_start_up && cnt) {
> - int failed = __ftrace_replace_code(rec, 1);
> + int failed = ftrace_replace_code_rec(rec, 1);
> if (failed) {
> ftrace_bug(failed, rec);
> goto out_loop;
^ permalink raw reply
* Re: [PATCH v3 2/3] powerpc/papr_scm: Update drc_pmem_unbind() to use H_SCM_UNBIND_ALL
From: Vaibhav Jain @ 2019-06-27 14:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev
Cc: Laurent Dufour, Oliver O'Halloran, David Gibson
In-Reply-To: <4a460995-f7c5-22bf-028e-628d984dce96@linux.ibm.com>
Thanks for reviewing this patch Aneesh,
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On 6/26/19 7:34 PM, Vaibhav Jain wrote:
>> The new hcall named H_SCM_UNBIND_ALL has been introduce that can
>> unbind all or specific scm memory assigned to an lpar. This is
>> more efficient than using H_SCM_UNBIND_MEM as currently we don't
>> support partial unbind of scm memory.
>>
>> Hence this patch proposes following changes to drc_pmem_unbind():
>>
>> * Update drc_pmem_unbind() to replace hcall H_SCM_UNBIND_MEM to
>> H_SCM_UNBIND_ALL.
>>
>> * Update drc_pmem_unbind() to handles cases when PHYP asks the guest
>> kernel to wait for specific amount of time before retrying the
>> hcall via the 'LONG_BUSY' return value.
>>
>> * Ensure appropriate error code is returned back from the function
>> in case of an error.
>>
>> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Change-log:
>>
>> v3:
>> * Fixed a build warning reported by kbuild-robot.
>> * Updated patch description to put emphasis on 'scm memory' instead of
>> 'scm drc memory blocks' as for phyp there is a stark difference
>> between how drc are managed for scm memory v/s regular memory. [Oliver]
>>
>> v2:
>> * Added a dev_dbg when unbind operation succeeds [Oliver]
>> * Changed type of variable 'rc' to int64_t [Oliver]
>> * Removed the code that was logging a warning in case bind operation
>> takes >1-seconds [Oliver]
>> * Spinned off changes to hvcall.h as a separate patch. [Oliver]
>> ---
>> arch/powerpc/platforms/pseries/papr_scm.c | 29 +++++++++++++++++------
>> 1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 96c53b23e58f..c01a03fd3ee7 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -11,6 +11,7 @@
>> #include <linux/sched.h>
>> #include <linux/libnvdimm.h>
>> #include <linux/platform_device.h>
>> +#include <linux/delay.h>
>>
>> #include <asm/plpar_wrappers.h>
>>
>> @@ -77,22 +78,36 @@ static int drc_pmem_bind(struct papr_scm_priv *p)
>> static int drc_pmem_unbind(struct papr_scm_priv *p)
>> {
>> unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> - uint64_t rc, token;
>> + uint64_t token = 0;
>> + int64_t rc;
>>
>> - token = 0;
>> + dev_dbg(&p->pdev->dev, "unbind drc %x\n", p->drc_index);
>>
>> - /* NB: unbind has the same retry requirements mentioned above */
>> + /* NB: unbind has the same retry requirements as drc_pmem_bind() */
>> do {
>> - rc = plpar_hcall(H_SCM_UNBIND_MEM, ret, p->drc_index,
>> - p->bound_addr, p->blocks, token);
>> +
>> + /* Unbind of all SCM resources associated with drcIndex */
>> + rc = plpar_hcall(H_SCM_UNBIND_ALL, ret, H_UNBIND_SCOPE_DRC,
>> + p->drc_index, token);
>> token = ret[0];
>> - cond_resched();
>> +
>> + /* Check if we are stalled for some time */
>> + if (H_IS_LONG_BUSY(rc)) {
>> + msleep(get_longbusy_msecs(rc));
>> + rc = H_BUSY;
>> + } else if (rc == H_BUSY) {
>> + cond_resched();
>> + }
>> +
>> } while (rc == H_BUSY);
>>
>> if (rc)
>> dev_err(&p->pdev->dev, "unbind error: %lld\n", rc);
>> + else
>> + dev_dbg(&p->pdev->dev, "unbind drc %x complete\n",
>> + p->drc_index);
>>
> Can we add p->drc_index as part of these messages? Also s/%x/0x%x ?
the scm platform device has the name of the form
"ibm,persistent-memory:ibm,pmemory@44100002" which also contains the
drc_index. So i think printing it again in this message would be
redundant.
>
>
>> - return !!rc;
>> + return rc == H_SUCCESS ? 0 : -ENXIO;
>> }
>>
> The error -ENXIO is confusing. Can we keep the HCALL error as return for
> this? We don't check error from this. If we can't take any action based
> on the return. Then may be make it void?
>
Wanted to keep the behaviour of this function symmetric to
drc_pmem_bind() which when updated in the next patch returns a kernel
error code instead of hcall error.
Agree that we arent using the return
value of this function right now but this may change in the future.
>
>> static int papr_scm_meta_get(struct papr_scm_priv *p,
>>
>
>
> -aneesh
--
Vaibhav Jain <vaibhav@linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
^ permalink raw reply
* Re: [PATCH v2 2/7] x86/ftrace: Fix use of flags in ftrace_replace_code()
From: Naveen N. Rao @ 2019-06-27 14:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Nicholas Piggin, Masami Hiramatsu, linuxppc-dev,
Ingo Molnar
In-Reply-To: <20190627092902.253971d2@gandalf.local.home>
Steven Rostedt wrote:
> On Thu, 27 Jun 2019 16:53:50 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be
>> schedulable), the generic ftrace_replace_code() function was modified to
>> accept a flags argument in place of a single 'enable' flag. However, the
>> x86 version of this function was not updated. Fix the same.
>>
>> Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable")
>
> I don't mind this change, but it's not a bug, and I'm not sure it
> should have the fixes tag. The reason being, the
> FTRACE_MODIFY_ENABLE_FL is only set when ftrace is called by with the
> command flag FTRACE_MAY_SLEEP, which is never done on x86.
I guess you meant to say that *FTRACE_MODIFY_MAY_SLEEP_FL* is only set
with FTRACE_MAY_SLEEP.
>
> That said, I'm fine with the change as it makes it more robust, but by
> adding the fixes tag, you're going to get this into all the stable
> code, and I'm not sure that's really necessary.
Agreed. Thanks for pointing this out. We can drop this patch from this
series and I will re-post this as a simpler cleanup later on.
Thanks,
Naveen
^ permalink raw reply
* Re: [PATCH v2 4/7] powerpc/ftrace: Additionally nop out the preceding mflr with -mprofile-kernel
From: Naveen N. Rao @ 2019-06-27 14:50 UTC (permalink / raw)
To: Masami Hiramatsu, Ingo Molnar, Michael Ellerman, Nicholas Piggin,
Steven Rostedt
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <841386feda429a1f0d4b7442c3ede1ed91466f92.1561634177.git.naveen.n.rao@linux.v net.ibm.com>
Naveen N. Rao wrote:
> With -mprofile-kernel, gcc emits 'mflr r0', followed by 'bl _mcount' to
> enable function tracing and profiling. So far, with dynamic ftrace, we
> used to only patch out the branch to _mcount(). However, mflr is
> executed by the branch unit that can only execute one per cycle on
> POWER9 and shared with branches, so it would be nice to avoid it where
> possible.
>
> We cannot simply nop out the mflr either. When enabling function
> tracing, there can be a race if tracing is enabled when some thread was
> interrupted after executing a nop'ed out mflr. In this case, the thread
> would execute the now-patched-in branch to _mcount() without having
> executed the preceding mflr.
>
> To solve this, we now enable function tracing in 2 steps: patch in the
> mflr instruction, use 'smp_call_function(isync);
> synchronize_rcu_tasks()' to ensure all existing threads make progress,
> and then patch in the branch to _mcount(). We override
> ftrace_replace_code() with a powerpc64 variant for this purpose.
>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/trace/ftrace.c | 258 ++++++++++++++++++++++++++---
> 1 file changed, 236 insertions(+), 22 deletions(-)
>
I missed adding a comment here to explain the changes. As discussed in
the previous series, I think we are ok with this patch from a CMODX
perspective. For smp_call_function(), I decided to have it included in
this patch since we know that we need it here for sure. I am not
entirely sure we want to do that in patch_instruction() since ftrace
doesn't seem to need it elsewhere. As Nick Piggin pointed out, we may
want to have users of patch_instruction() (kprobes) add the necessary
synchronization.
Thanks,
Naveen
^ permalink raw reply
* Re: [PATCH v2 4/7] powerpc/ftrace: Additionally nop out the preceding mflr with -mprofile-kernel
From: Steven Rostedt @ 2019-06-27 15:08 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, Nicholas Piggin, Masami Hiramatsu, linuxppc-dev,
Ingo Molnar
In-Reply-To: <841386feda429a1f0d4b7442c3ede1ed91466f92.1561634177.git.naveen.n.rao@linux.vnet.ibm.com>
On Thu, 27 Jun 2019 16:53:52 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> With -mprofile-kernel, gcc emits 'mflr r0', followed by 'bl _mcount' to
> enable function tracing and profiling. So far, with dynamic ftrace, we
> used to only patch out the branch to _mcount(). However, mflr is
> executed by the branch unit that can only execute one per cycle on
> POWER9 and shared with branches, so it would be nice to avoid it where
> possible.
>
> We cannot simply nop out the mflr either. When enabling function
> tracing, there can be a race if tracing is enabled when some thread was
> interrupted after executing a nop'ed out mflr. In this case, the thread
> would execute the now-patched-in branch to _mcount() without having
> executed the preceding mflr.
>
> To solve this, we now enable function tracing in 2 steps: patch in the
> mflr instruction, use 'smp_call_function(isync);
> synchronize_rcu_tasks()' to ensure all existing threads make progress,
> and then patch in the branch to _mcount(). We override
> ftrace_replace_code() with a powerpc64 variant for this purpose.
You may want to explain that you do the reverse when patching it out.
That is, patch out the "bl _mcount" into a nop and then patch out the
"mflr r0". But interesting, I don't see a synchronize_rcu_tasks() call
there.
>
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/trace/ftrace.c | 258 ++++++++++++++++++++++++++---
> 1 file changed, 236 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 517662a56bdc..86c2b50dcaa9 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -125,7 +125,7 @@ __ftrace_make_nop(struct module *mod,
> {
> unsigned long entry, ptr, tramp;
> unsigned long ip = rec->ip;
> - unsigned int op, pop;
> + unsigned int op;
>
> /* read where this goes */
> if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> @@ -160,8 +160,6 @@ __ftrace_make_nop(struct module *mod,
>
> #ifdef CONFIG_MPROFILE_KERNEL
> /* When using -mkernel_profile there is no load to jump over */
> - pop = PPC_INST_NOP;
> -
> if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
> pr_err("Fetching instruction at %lx failed.\n", ip - 4);
> return -EFAULT;
> @@ -169,26 +167,23 @@ __ftrace_make_nop(struct module *mod,
>
> /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
> - pr_err("Unexpected instruction %08x around bl _mcount\n", op);
> + pr_err("Unexpected instruction %08x before bl _mcount\n", op);
> return -EINVAL;
> }
> -#else
> - /*
> - * Our original call site looks like:
> - *
> - * bl <tramp>
> - * ld r2,XX(r1)
> - *
> - * Milton Miller pointed out that we can not simply nop the branch.
> - * If a task was preempted when calling a trace function, the nops
> - * will remove the way to restore the TOC in r2 and the r2 TOC will
> - * get corrupted.
> - *
> - * Use a b +8 to jump over the load.
> - */
>
> - pop = PPC_INST_BRANCH | 8; /* b +8 */
> + /* We should patch out the bl to _mcount first */
> + if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> + pr_err("Patching NOP failed.\n");
> + return -EPERM;
> + }
>
> + /* then, nop out the preceding 'mflr r0' as an optimization */
> + if (op == PPC_INST_MFLR &&
> + patch_instruction((unsigned int *)(ip - 4), PPC_INST_NOP)) {
> + pr_err("Patching NOP failed.\n");
> + return -EPERM;
> + }
> +#else
> /*
> * Check what is in the next instruction. We can see ld r2,40(r1), but
> * on first pass after boot we will see mflr r0.
> @@ -202,12 +197,25 @@ __ftrace_make_nop(struct module *mod,
> pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, op);
> return -EINVAL;
> }
> -#endif /* CONFIG_MPROFILE_KERNEL */
>
> - if (patch_instruction((unsigned int *)ip, pop)) {
> + /*
> + * Our original call site looks like:
> + *
> + * bl <tramp>
> + * ld r2,XX(r1)
> + *
> + * Milton Miller pointed out that we can not simply nop the branch.
> + * If a task was preempted when calling a trace function, the nops
> + * will remove the way to restore the TOC in r2 and the r2 TOC will
> + * get corrupted.
> + *
> + * Use a b +8 to jump over the load.
> + */
> + if (patch_instruction((unsigned int *)ip, PPC_INST_BRANCH | 8)) {
> pr_err("Patching NOP failed.\n");
> return -EPERM;
> }
> +#endif /* CONFIG_MPROFILE_KERNEL */
>
> return 0;
> }
> @@ -421,6 +429,26 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
> return -EPERM;
> }
>
> +#ifdef CONFIG_MPROFILE_KERNEL
I would think you need to break this up into two parts as well, with a
synchronize_rcu_tasks() in between.
Imagine this scenario:
<func>:
nop <-- interrupt comes here, and preempts the task
nop
First change.
<func>:
mflr r0
nop
Second change.
<func>:
mflr r0
bl _mcount
Task returns from interrupt
<func>:
mflr r0
bl _mcount <-- executes here
It never did the mflr r0, because the last command that was executed
was a nop before it was interrupted. And yes, it can be interrupted
on a nop!
-- Steve
> + /* Nop out the preceding 'mflr r0' as an optimization */
> + if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
> + pr_err("Fetching instruction at %lx failed.\n", ip - 4);
> + return -EFAULT;
> + }
> +
> + /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> + if (op != PPC_INST_MFLR && op != PPC_INST_STD_LR) {
> + pr_err("Unexpected instruction %08x before bl _mcount\n", op);
> + return -EINVAL;
> + }
> +
> + if (op == PPC_INST_MFLR &&
> + patch_instruction((unsigned int *)(ip - 4), PPC_INST_NOP)) {
> + pr_err("Patching NOP failed.\n");
> + return -EPERM;
> + }
> +#endif
> +
> return 0;
> }
>
> @@ -429,6 +457,7 @@ int ftrace_make_nop(struct module *mod,
> {
> unsigned long ip = rec->ip;
> unsigned int old, new;
> + int rc;
>
> /*
> * If the calling address is more that 24 bits away,
> @@ -439,7 +468,34 @@ int ftrace_make_nop(struct module *mod,
> /* within range */
> old = ftrace_call_replace(ip, addr, 1);
> new = PPC_INST_NOP;
> - return ftrace_modify_code(ip, old, new);
> + rc = ftrace_modify_code(ip, old, new);
> +#ifdef CONFIG_MPROFILE_KERNEL
> + if (rc)
> + return rc;
> +
> + /*
> + * For -mprofile-kernel, we patch out the preceding 'mflr r0'
> + * instruction, as an optimization. It is important to nop out
> + * the branch to _mcount() first, as a lone 'mflr r0' is
> + * harmless.
> + */
> + if (probe_kernel_read(&old, (void *)(ip - 4), 4)) {
> + pr_err("Fetching instruction at %lx failed.\n", ip - 4);
> + return -EFAULT;
> + }
> +
> + /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */
> + if (old != PPC_INST_MFLR && old != PPC_INST_STD_LR) {
> + pr_err("Unexpected instruction %08x before bl _mcount\n",
> + old);
> + return -EINVAL;
> + }
> +
> + if (old == PPC_INST_MFLR)
> + rc = patch_instruction((unsigned int *)(ip - 4),
> + PPC_INST_NOP);
> +#endif
> + return rc;
> } else if (core_kernel_text(ip))
> return __ftrace_make_nop_kernel(rec, addr);
>
> @@ -567,6 +623,37 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> return -EINVAL;
> }
>
> +#ifdef CONFIG_MPROFILE_KERNEL
> + /*
> + * We could end up here without having called __ftrace_make_call_prep()
> + * if function tracing is enabled before a module is loaded.
> + *
> + * ftrace_module_enable() --> ftrace_replace_code_rec() -->
> + * ftrace_make_call() --> __ftrace_make_call()
> + *
> + * In this scenario, the previous instruction will be a NOP. It is
> + * safe to patch it with a 'mflr r0' since we know for a fact that
> + * this code is not yet being run.
> + */
> + ip -= MCOUNT_INSN_SIZE;
> +
> + /* read where this goes */
> + if (probe_kernel_read(op, ip, MCOUNT_INSN_SIZE))
> + return -EFAULT;
> +
> + /*
> + * nothing to do if this is using the older -mprofile-kernel
> + * instruction sequence
> + */
> + if (op[0] != PPC_INST_NOP)
> + return 0;
> +
> + if (patch_instruction((unsigned int *)ip, PPC_INST_MFLR)) {
> + pr_err("Patching MFLR failed.\n");
> + return -EPERM;
> + }
> +#endif
> +
> return 0;
> }
>
> @@ -863,6 +950,133 @@ void arch_ftrace_update_code(int command)
> ftrace_modify_all_code(command);
> }
>
> +#ifdef CONFIG_MPROFILE_KERNEL
> +/* Returns 1 if we patched in the mflr */
> +static int __ftrace_make_call_prep(struct dyn_ftrace *rec)
> +{
> + void *ip = (void *)rec->ip - MCOUNT_INSN_SIZE;
> + unsigned int op[2];
> +
> + /* read where this goes */
> + if (probe_kernel_read(op, ip, sizeof(op)))
> + return -EFAULT;
> +
> + if (op[1] != PPC_INST_NOP) {
> + pr_err("Unexpected call sequence at %p: %x %x\n",
> + ip, op[0], op[1]);
> + return -EINVAL;
> + }
> +
> + /*
> + * nothing to do if this is using the older -mprofile-kernel
> + * instruction sequence
> + */
> + if (op[0] != PPC_INST_NOP)
> + return 0;
> +
> + if (patch_instruction((unsigned int *)ip, PPC_INST_MFLR)) {
> + pr_err("Patching MFLR failed.\n");
> + return -EPERM;
> + }
> +
> + return 1;
> +}
> +
> +static void do_isync(void *info __maybe_unused)
> +{
> + isync();
> +}
> +
> +/*
> + * When enabling function tracing for -mprofile-kernel that uses a
> + * 2-instruction sequence of 'mflr r0, bl _mcount()', we use a 2 step process:
> + * 1. Patch in the 'mflr r0' instruction
> + * 1a. flush icache on all cpus, so that the updated instruction is seen
> + * 1b. synchronize_rcu_tasks() to ensure that any cpus that had executed
> + * the earlier nop there make progress (and hence do not branch into
> + * ftrace without executing the preceding mflr)
> + * 2. Patch in the branch to ftrace
> + */
> +void ftrace_replace_code(int mod_flags)
> +{
> + int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
> + int schedulable = mod_flags & FTRACE_MODIFY_MAY_SLEEP_FL;
> + int ret, failed, make_call = 0;
> + struct ftrace_rec_iter *iter;
> + struct dyn_ftrace *rec;
> +
> + if (unlikely(!ftrace_enabled))
> + return;
> +
> + for_ftrace_rec_iter(iter) {
> + rec = ftrace_rec_iter_record(iter);
> +
> + if (rec->flags & FTRACE_FL_DISABLED)
> + continue;
> +
> + failed = 0;
> + ret = ftrace_test_record(rec, enable);
> + if (ret == FTRACE_UPDATE_MAKE_CALL) {
> + failed = __ftrace_make_call_prep(rec);
> + if (failed < 0) {
> + ftrace_bug(failed, rec);
> + return;
> + } else if (failed == 1)
> + make_call++;
> + }
> +
> + if (!failed) {
> + /* We can patch the record directly */
> + failed = ftrace_replace_code_rec(rec, enable);
> + if (failed) {
> + ftrace_bug(failed, rec);
> + return;
> + }
> + }
> +
> + if (schedulable)
> + cond_resched();
> + }
> +
> + if (!make_call)
> + /* No records needed patching a preceding mflr */
> + return;
> +
> + /* Make sure all cpus see the new instruction */
> + smp_call_function(do_isync, NULL, 1);
> +
> + /*
> + * We also need to ensure that all cpus make progress:
> + * - With !CONFIG_PREEMPT, we want to be sure that cpus return from
> + * any interrupts they may be handling, and make progress.
> + * - With CONFIG_PREEMPT, we want to be additionally sure that there
> + * are no pre-empted tasks that have executed the earlier nop, and
> + * might end up executing the subsequently patched branch to ftrace.
> + */
> + synchronize_rcu_tasks();
> +
> + for_ftrace_rec_iter(iter) {
> + rec = ftrace_rec_iter_record(iter);
> +
> + if (rec->flags & FTRACE_FL_DISABLED)
> + continue;
> +
> + ret = ftrace_test_record(rec, enable);
> + if (ret == FTRACE_UPDATE_MAKE_CALL)
> + failed = ftrace_replace_code_rec(rec, enable);
> +
> + if (failed) {
> + ftrace_bug(failed, rec);
> + return;
> + }
> +
> + if (schedulable)
> + cond_resched();
> + }
> +
> +}
> +#endif
> +
> #ifdef CONFIG_PPC64
> #define PACATOC offsetof(struct paca_struct, kernel_toc)
>
^ permalink raw reply
* [PATCH] soc/fsl/qbman: Use index when accessing device tree properties
From: Roy Pledge @ 2019-06-27 15:17 UTC (permalink / raw)
To: Leo Li, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: Roy Pledge, stable@vger.kernel.org
The index value should be passed to the of_parse_phandle()
function to ensure the correct property is read.
Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
---
drivers/soc/fsl/qbman/dpaa_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c
index 3e0a7f3..0b901a8 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.c
+++ b/drivers/soc/fsl/qbman/dpaa_sys.c
@@ -49,7 +49,7 @@ int qbman_init_private_mem(struct device *dev, int idx, dma_addr_t *addr,
idx, ret);
return -ENODEV;
}
- mem_node = of_parse_phandle(dev->of_node, "memory-region", 0);
+ mem_node = of_parse_phandle(dev->of_node, "memory-region", idx);
if (mem_node) {
ret = of_property_read_u64(mem_node, "size", &size64);
if (ret) {
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox