LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ALSA: ppc: drop if block with always false condition
From: Uwe Kleine-König @ 2020-11-26 16:59 UTC (permalink / raw)
  To: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz
  Cc: alsa-devel, linux-scsi, linux-usb, linux-fbdev, dri-devel,
	linux-block, Paul Mackerras, netdev, linuxppc-dev

The remove callback is only called for devices that were probed
successfully before. As the matching probe function cannot complete
without error if dev->match_id != PS3_MATCH_ID_SOUND, we don't have to
check this here.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/ppc/snd_ps3.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 58bb49fff184..6ab796a5d936 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1053,8 +1053,6 @@ static int snd_ps3_driver_remove(struct ps3_system_bus_device *dev)
 {
 	int ret;
 	pr_info("%s:start id=%d\n", __func__,  dev->match_id);
-	if (dev->match_id != PS3_MATCH_ID_SOUND)
-		return -ENXIO;
 
 	/*
 	 * ctl and preallocate buffer will be freed in
-- 
2.29.2


^ permalink raw reply related

* [Bug 209733] Starting new KVM virtual machines on PPC64 starts to hang after box is up for a while
From: bugzilla-daemon @ 2020-11-26 17:26 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-209733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=209733

--- Comment #4 from Cameron (cam@neo-zeon.de) ---
After enough testing, I feel confident that this issue was fixed in 5.9.9.
However, I encountered issues with XFS with 5.9.9 and 5.9.10 (mainly on POWER,
but to a lesser extent they seemed to happen for me on amd64 at least). 5.9.11
has the weird hang fixed and no other issues (XFS or otherwise) in over 2 days!

I feel confident in closing this issue.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 209733] Starting new KVM virtual machines on PPC64 starts to hang after box is up for a while
From: bugzilla-daemon @ 2020-11-26 17:26 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-209733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=209733

Cameron (cam@neo-zeon.de) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |CODE_FIX

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void
From: Uwe Kleine-König @ 2020-11-26 17:44 UTC (permalink / raw)
  To: Geoff Levand, Jaroslav Kysela, Takashi Iwai, Michael Ellerman,
	Jens Axboe, Jim Paris, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Jakub Kicinski, James E.J. Bottomley,
	Martin K. Petersen, Alan Stern, Bartlomiej Zolnierkiewicz
  Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <20201126165950.2554997-2-u.kleine-koenig@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

[dropped a few lists from Cc: that are off-topic for this mail]

Hello,

while creating this patch series I looked at ps3_system_bus_shutdown().
I think the BUG_ON(!drv) in (now) line 422 can be easily triggered when
there is a device without driver. (Try unbinding via sysfs before
shutdown.)

Also the BUG in (now) line 437 seems possible to trigger. Consider a
driver that doesn't have the two callbacks, e.g. because there is
nothing special to do on shutdown and probe only used devm_* resources.

While at it, I find it surprising that the remove callback is called if
there is no shutdown callback.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [RFC PATCH 00/14] powerpc64: Add support for ftrace direct calls
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel

This series adds support for DYNAMIC_FTRACE_WITH_DIRECT_CALLS for 
powerpc64le.

This is mostly working fine for me, except for a soft lockup I see with 
the ftrace direct selftest. It happens when irqsoff tracer is being 
tested with the ftrace direct modules. This appears to be an existing 
upstream issue since I am able to reproduce the lockup without these 
patches. I will be looking into that to see if I can figure out the 
cause of those lockups.

In the meantime, I would appreciate a review of these patches.


- Naveen


Naveen N. Rao (14):
  ftrace: Fix updating FTRACE_FL_TRAMP
  ftrace: Fix DYNAMIC_FTRACE_WITH_DIRECT_CALLS dependency
  ftrace: Fix cleanup in error path of register_ftrace_direct()
  ftrace: Remove ftrace_find_direct_func()
  ftrace: Add architectural helpers for [un]register_ftrace_direct()
  powerpc: Add support for CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
  powerpc/ftrace: Remove dead code
  powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace
    trampoline
  powerpc/ftrace: Use a hash table for tracking ftrace stubs
  powerpc/ftrace: Drop assumptions about ftrace trampoline target
  powerpc/ftrace: Use GPR save/restore macros in ftrace_graph_caller()
  powerpc/ftrace: Drop saving LR to stack save area for -mprofile-kernel
  powerpc/ftrace: Add support for register_ftrace_direct() for
    MPROFILE_KERNEL
  samples/ftrace: Add powerpc support for ftrace direct samples

 arch/powerpc/Kconfig                          |   2 +
 arch/powerpc/include/asm/ftrace.h             |  14 +
 arch/powerpc/include/asm/ptrace.h             |  31 ++
 arch/powerpc/kernel/trace/ftrace.c            | 314 +++++++++++++-----
 .../powerpc/kernel/trace/ftrace_64_mprofile.S |  70 ++--
 include/linux/ftrace.h                        |   7 +-
 kernel/trace/Kconfig                          |   2 +-
 kernel/trace/ftrace.c                         | 130 +++-----
 samples/Kconfig                               |   2 +-
 samples/ftrace/ftrace-direct-modify.c         |  58 ++++
 samples/ftrace/ftrace-direct-too.c            |  48 ++-
 samples/ftrace/ftrace-direct.c                |  45 ++-
 12 files changed, 519 insertions(+), 204 deletions(-)


base-commit: 4c202167192a77481310a3cacae9f12618b92216
-- 
2.25.4


^ permalink raw reply

* [RFC PATCH 01/14] ftrace: Fix updating FTRACE_FL_TRAMP
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

On powerpc, kprobe-direct.tc triggered FTRACE_WARN_ON() in
ftrace_get_addr_new() followed by the below message:
  Bad trampoline accounting at: 000000004222522f (wake_up_process+0xc/0x20) (f0000001)

The set of steps leading to this involved:
- modprobe ftrace-direct-too
- enable_probe
- modprobe ftrace-direct
- rmmod ftrace-direct <-- trigger

The problem turned out to be that we were not updating flags in the
ftrace record properly. From the above message about the trampoline
accounting being bad, it can be seen that the ftrace record still has
FTRACE_FL_TRAMP set though ftrace-direct module is going away. This
happens because we are checking if any ftrace_ops has the
FTRACE_FL_TRAMP flag set _before_ updating the filter hash.

The fix for this is to look for any _other_ ftrace_ops that also needs
FTRACE_FL_TRAMP.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/ftrace.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 8185f7240095f4..9c1bba8cc51b03 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1629,6 +1629,8 @@ static bool test_rec_ops_needs_regs(struct dyn_ftrace *rec)
 static struct ftrace_ops *
 ftrace_find_tramp_ops_any(struct dyn_ftrace *rec);
 static struct ftrace_ops *
+ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude);
+static struct ftrace_ops *
 ftrace_find_tramp_ops_next(struct dyn_ftrace *rec, struct ftrace_ops *ops);
 
 static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
@@ -1778,7 +1780,7 @@ static bool __ftrace_hash_rec_update(struct ftrace_ops *ops,
 			 * to it.
 			 */
 			if (ftrace_rec_count(rec) == 1 &&
-			    ftrace_find_tramp_ops_any(rec))
+			    ftrace_find_tramp_ops_any_other(rec, ops))
 				rec->flags |= FTRACE_FL_TRAMP;
 			else
 				rec->flags &= ~FTRACE_FL_TRAMP;
@@ -2244,6 +2246,24 @@ ftrace_find_tramp_ops_any(struct dyn_ftrace *rec)
 	return NULL;
 }
 
+static struct ftrace_ops *
+ftrace_find_tramp_ops_any_other(struct dyn_ftrace *rec, struct ftrace_ops *op_exclude)
+{
+	struct ftrace_ops *op;
+	unsigned long ip = rec->ip;
+
+	do_for_each_ftrace_op(op, ftrace_ops_list) {
+
+		if (op == op_exclude || !op->trampoline)
+			continue;
+
+		if (hash_contains_ip(ip, op->func_hash))
+			return op;
+	} while_for_each_ftrace_op(op);
+
+	return NULL;
+}
+
 static struct ftrace_ops *
 ftrace_find_tramp_ops_next(struct dyn_ftrace *rec,
 			   struct ftrace_ops *op)
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 02/14] ftrace: Fix DYNAMIC_FTRACE_WITH_DIRECT_CALLS dependency
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

DYNAMIC_FTRACE_WITH_DIRECT_CALLS should depend on
DYNAMIC_FTRACE_WITH_REGS since we need ftrace_regs_caller().

Fixes: 763e34e74bb7d5c ("ftrace: Add register_ftrace_direct()")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a4020c0b4508c9..e1bf5228fb692a 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -202,7 +202,7 @@ config DYNAMIC_FTRACE_WITH_REGS
 
 config DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	def_bool y
-	depends on DYNAMIC_FTRACE
+	depends on DYNAMIC_FTRACE_WITH_REGS
 	depends on HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 
 config FUNCTION_PROFILER
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 03/14] ftrace: Fix cleanup in error path of register_ftrace_direct()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

We need to remove hash entry if register_ftrace_function() fails.
Consolidate the cleanup to be done after register_ftrace_function() at
the end.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/ftrace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9c1bba8cc51b03..3844a4a1346a9c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5136,8 +5136,6 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	__add_hash_entry(direct_functions, entry);
 
 	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
-	if (ret)
-		remove_hash_entry(direct_functions, entry);
 
 	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
 		ret = register_ftrace_function(&direct_ops);
@@ -5146,6 +5144,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	}
 
 	if (ret) {
+		remove_hash_entry(direct_functions, entry);
 		kfree(entry);
 		if (!direct->count) {
 			list_del_rcu(&direct->next);
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 04/14] ftrace: Remove ftrace_find_direct_func()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

This is a revert of commit 013bf0da047481 ("ftrace: Add
ftrace_find_direct_func()")

ftrace_find_direct_func() was meant for use in the function graph tracer
by architecture specific code. However, commit ff205766dbbee0 ("ftrace:
Fix function_graph tracer interaction with BPF trampoline") disabled
function graph tracer for direct calls leaving this without any users.

In addition, modify_ftrace_direct() allowed redirecting the direct call
to a different trampoline that was never registered through
register_ftrace_direct(). This meant that ftrace_direct_funcs didn't
capture all trampolines.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h |  5 ---
 kernel/trace/ftrace.c  | 84 ++----------------------------------------
 2 files changed, 4 insertions(+), 85 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1bd3a0356ae478..46b4b7ee28c41f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -285,7 +285,6 @@ extern int ftrace_direct_func_count;
 int register_ftrace_direct(unsigned long ip, unsigned long addr);
 int unregister_ftrace_direct(unsigned long ip, unsigned long addr);
 int modify_ftrace_direct(unsigned long ip, unsigned long old_addr, unsigned long new_addr);
-struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr);
 int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 				struct dyn_ftrace *rec,
 				unsigned long old_addr,
@@ -306,10 +305,6 @@ static inline int modify_ftrace_direct(unsigned long ip,
 {
 	return -ENOTSUPP;
 }
-static inline struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
-{
-	return NULL;
-}
 static inline int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 					      struct dyn_ftrace *rec,
 					      unsigned long old_addr,
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3844a4a1346a9c..7476f2458b6d95 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5005,46 +5005,6 @@ ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
-
-struct ftrace_direct_func {
-	struct list_head	next;
-	unsigned long		addr;
-	int			count;
-};
-
-static LIST_HEAD(ftrace_direct_funcs);
-
-/**
- * ftrace_find_direct_func - test an address if it is a registered direct caller
- * @addr: The address of a registered direct caller
- *
- * This searches to see if a ftrace direct caller has been registered
- * at a specific address, and if so, it returns a descriptor for it.
- *
- * This can be used by architecture code to see if an address is
- * a direct caller (trampoline) attached to a fentry/mcount location.
- * This is useful for the function_graph tracer, as it may need to
- * do adjustments if it traced a location that also has a direct
- * trampoline attached to it.
- */
-struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
-{
-	struct ftrace_direct_func *entry;
-	bool found = false;
-
-	/* May be called by fgraph trampoline (protected by rcu tasks) */
-	list_for_each_entry_rcu(entry, &ftrace_direct_funcs, next) {
-		if (entry->addr == addr) {
-			found = true;
-			break;
-		}
-	}
-	if (found)
-		return entry;
-
-	return NULL;
-}
-
 /**
  * register_ftrace_direct - Call a custom trampoline directly
  * @ip: The address of the nop at the beginning of a function
@@ -5064,7 +5024,6 @@ struct ftrace_direct_func *ftrace_find_direct_func(unsigned long addr)
  */
 int register_ftrace_direct(unsigned long ip, unsigned long addr)
 {
-	struct ftrace_direct_func *direct;
 	struct ftrace_func_entry *entry;
 	struct ftrace_hash *free_hash = NULL;
 	struct dyn_ftrace *rec;
@@ -5118,19 +5077,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	if (!entry)
 		goto out_unlock;
 
-	direct = ftrace_find_direct_func(addr);
-	if (!direct) {
-		direct = kmalloc(sizeof(*direct), GFP_KERNEL);
-		if (!direct) {
-			kfree(entry);
-			goto out_unlock;
-		}
-		direct->addr = addr;
-		direct->count = 0;
-		list_add_rcu(&direct->next, &ftrace_direct_funcs);
-		ftrace_direct_func_count++;
-	}
-
+	ftrace_direct_func_count++;
 	entry->ip = ip;
 	entry->direct = addr;
 	__add_hash_entry(direct_functions, entry);
@@ -5145,18 +5092,8 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 
 	if (ret) {
 		remove_hash_entry(direct_functions, entry);
+		ftrace_direct_func_count--;
 		kfree(entry);
-		if (!direct->count) {
-			list_del_rcu(&direct->next);
-			synchronize_rcu_tasks();
-			kfree(direct);
-			if (free_hash)
-				free_ftrace_hash(free_hash);
-			free_hash = NULL;
-			ftrace_direct_func_count--;
-		}
-	} else {
-		direct->count++;
 	}
  out_unlock:
 	mutex_unlock(&direct_mutex);
@@ -5199,7 +5136,6 @@ static struct ftrace_func_entry *find_direct_entry(unsigned long *ip,
 
 int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 {
-	struct ftrace_direct_func *direct;
 	struct ftrace_func_entry *entry;
 	int ret = -ENODEV;
 
@@ -5217,20 +5153,8 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 	WARN_ON(ret);
 
 	remove_hash_entry(direct_functions, entry);
-
-	direct = ftrace_find_direct_func(addr);
-	if (!WARN_ON(!direct)) {
-		/* This is the good path (see the ! before WARN) */
-		direct->count--;
-		WARN_ON(direct->count < 0);
-		if (!direct->count) {
-			list_del_rcu(&direct->next);
-			synchronize_rcu_tasks();
-			kfree(direct);
-			kfree(entry);
-			ftrace_direct_func_count--;
-		}
-	}
+	ftrace_direct_func_count--;
+	kfree(entry);
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 05/14] ftrace: Add architectural helpers for [un]register_ftrace_direct()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Architectures may want to do some validation (such as to ensure that the
trampoline code is reachable from the provided ftrace location) before
accepting ftrace direct registration. Add helpers for the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/ftrace.h |  2 ++
 kernel/trace/ftrace.c  | 27 +++++++++++++++++++++------
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 46b4b7ee28c41f..3fdcb4c513bc2d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -290,6 +290,8 @@ int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
 				unsigned long old_addr,
 				unsigned long new_addr);
 unsigned long ftrace_find_rec_direct(unsigned long ip);
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr);
+void arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr);
 #else
 # define ftrace_direct_func_count 0
 static inline int register_ftrace_direct(unsigned long ip, unsigned long addr)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7476f2458b6d95..0e259b90527722 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5005,6 +5005,13 @@ ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+int __weak arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	return 0;
+}
+
+void __weak arch_unregister_ftrace_direct(unsigned long ip, unsigned long addr) { }
+
 /**
  * register_ftrace_direct - Call a custom trampoline directly
  * @ip: The address of the nop at the beginning of a function
@@ -5028,6 +5035,7 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	struct ftrace_hash *free_hash = NULL;
 	struct dyn_ftrace *rec;
 	int ret = -EBUSY;
+	int arch_ret;
 
 	mutex_lock(&direct_mutex);
 
@@ -5082,18 +5090,24 @@ int register_ftrace_direct(unsigned long ip, unsigned long addr)
 	entry->direct = addr;
 	__add_hash_entry(direct_functions, entry);
 
-	ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+	arch_ret = arch_register_ftrace_direct(ip, addr);
 
-	if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
-		ret = register_ftrace_function(&direct_ops);
-		if (ret)
-			ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+	if (!arch_ret) {
+		ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
+
+		if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
+			ret = register_ftrace_function(&direct_ops);
+			if (ret)
+				ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
+		}
 	}
 
-	if (ret) {
+	if (arch_ret || ret) {
 		remove_hash_entry(direct_functions, entry);
 		ftrace_direct_func_count--;
 		kfree(entry);
+		if (!arch_ret)
+			arch_unregister_ftrace_direct(ip, addr);
 	}
  out_unlock:
 	mutex_unlock(&direct_mutex);
@@ -5155,6 +5169,7 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
 	remove_hash_entry(direct_functions, entry);
 	ftrace_direct_func_count--;
 	kfree(entry);
+	arch_unregister_ftrace_direct(ip, addr);
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 06/14] powerpc: Add support for CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add register_get_kernel_argument() for a rudimentary way to access
kernel function arguments.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig              |  1 +
 arch/powerpc/include/asm/ptrace.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e9f13fe084929b..cfc6dd787f532c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -202,6 +202,7 @@ config PPC
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
 	select HAVE_FAST_GUP
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e2c778c176a3a6..956828c07abd70 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -62,6 +62,8 @@ struct pt_regs
 };
 #endif
 
+#define NR_REG_ARGUMENTS 8
+
 #ifdef __powerpc64__
 
 /*
@@ -85,8 +87,10 @@ struct pt_regs
 
 #ifdef PPC64_ELF_ABI_v2
 #define STACK_FRAME_MIN_SIZE	32
+#define STACK_FRAME_PARM_SAVE	32
 #else
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	48
 #endif
 
 /* Size of dummy stack frame allocated when calling signal handler. */
@@ -103,6 +107,7 @@ struct pt_regs
 #define STACK_INT_FRAME_SIZE	(sizeof(struct pt_regs) + STACK_FRAME_OVERHEAD)
 #define STACK_FRAME_MARKER	2
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
+#define STACK_FRAME_PARM_SAVE	8
 
 /* Size of stack frame allocated when calling signal handler. */
 #define __SIGNAL_FRAMESIZE	64
@@ -309,6 +314,32 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
 		return 0;
 }
 
+/**
+ * regs_get_kernel_argument() - get Nth function argument in kernel
+ * @regs:	pt_regs of that context
+ * @n:		function argument number (start from 0)
+ *
+ * regs_get_argument() returns @n th argument of the function call.
+ * Note that this chooses most probable assignment, and is incorrect
+ * in scenarios where double or fp/vector parameters are involved.
+ * This also doesn't take into account stack alignment requirements.
+ *
+ * This is expected to be called from kprobes or ftrace with regs
+ * at function entry, so the current function has not setup its stack.
+ */
+static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
+						     unsigned int n)
+{
+	if (n >= NR_REG_ARGUMENTS) {
+#ifndef __powerpc64__
+		n -= NR_REG_ARGUMENTS;
+#endif
+		n += STACK_FRAME_PARM_SAVE / sizeof(unsigned long);
+		return regs_get_kernel_stack_nth(regs, n);
+	} else {
+		return regs_get_register(regs, offsetof(struct pt_regs, gpr[n + 3]));
+	}
+}
 #endif /* __ASSEMBLY__ */
 
 #ifndef __powerpc64__
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 07/14] powerpc/ftrace: Remove dead code
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

ftrace_plt_tramps[] was intended to speed up skipping plt branches, but
the code wasn't completed. It is also not significantly better than
reading and decoding the instruction. Remove the same.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 42761ebec9f755..4fe5f373172fd2 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -332,7 +332,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	struct ppc_inst op;
 	unsigned long ptr;
 	struct ppc_inst instr;
-	static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
 
 	/* Is this a known long jump tramp? */
 	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
@@ -341,13 +340,6 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		else if (ftrace_tramps[i] == tramp)
 			return 0;
 
-	/* Is this a known plt tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_plt_tramps[i])
-			break;
-		else if (ftrace_plt_tramps[i] == tramp)
-			return -1;
-
 	/* New trampoline -- read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)tramp)) {
 		pr_debug("Fetching opcode failed.\n");
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 08/14] powerpc/ftrace: Use FTRACE_REGS_ADDR to identify the correct ftrace trampoline
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Use FTRACE_REGS_ADDR instead of keying off
CONFIG_DYNAMIC_FTRACE_WITH_REGS to identify the proper ftrace trampoline
address to use.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4fe5f373172fd2..14b39f7797d455 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -361,11 +361,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	ptr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	ptr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
 				(void *)ptr);
@@ -885,11 +881,7 @@ int __init ftrace_dyn_arch_init(void)
 		0x7d8903a6,		/* mtctr   r12			*/
 		0x4e800420,		/* bctr				*/
 	};
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_regs_caller);
-#else
-	unsigned long addr = ppc_global_function_entry((void *)ftrace_caller);
-#endif
+	unsigned long addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 	long reladdr = addr - kernel_toc_addr();
 
 	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 12/14] powerpc/ftrace: Drop saving LR to stack save area for -mprofile-kernel
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

In -mprofile-kernel variant of ftrace_graph_caller(), we save the
optionally-updated LR address into the stack save area at the end. This
is likely an offshoot of the initial -mprofile-kernel implementation in
gcc emitting the same as part of the -mprofile-kernel instruction
sequence. However, this is not required. Drop it.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index bbe871b47ade58..c5602e9b07faa3 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -310,7 +310,5 @@ _GLOBAL(ftrace_graph_caller)
 	ld	r2, 24(r1)
 
 	addi	r1, r1, SWITCH_FRAME_SIZE
-	mflr	r0
-	std	r0, LRSAVE(r1)
 	bctr
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 10/14] powerpc/ftrace: Drop assumptions about ftrace trampoline target
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

We currently assume that ftrace locations are patched to go to either
ftrace_caller or ftrace_regs_caller. Drop this assumption in preparation
for supporting ftrace direct calls.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 107 +++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 7ddb6e4b527c39..fcb21a9756e456 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -322,14 +322,15 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
+	int i;
 	struct ppc_inst op;
 	struct ppc_inst instr;
 	struct ppc_ftrace_stub_data *stub;
 	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
-	/* Is this a known long jump tramp? */
-	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
-		if (stub->target == ftrace_target && stub->addr == tramp)
+	/* Is this a known tramp? */
+	hash_for_each(ppc_ftrace_stubs, i, stub, hentry)
+		if (stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -608,23 +609,16 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 {
 	struct ppc_inst op;
 	void *ip = (void *)rec->ip;
-	unsigned long tramp, entry, ptr;
+	unsigned long tramp, ptr;
 
-	/* Make sure we're being asked to patch branch to a known ftrace addr */
-	entry = ppc_global_function_entry((void *)ftrace_caller);
 	ptr = ppc_global_function_entry((void *)addr);
 
-	if (ptr != entry) {
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		entry = ppc_global_function_entry((void *)ftrace_regs_caller);
-		if (ptr != entry) {
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	tramp = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == tramp)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
-			pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
-			return -EINVAL;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-		}
-#endif
-	}
 
 	/* Make sure we have a nop */
 	if (probe_kernel_read_inst(&op, ip)) {
@@ -637,7 +631,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
+	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -783,6 +777,81 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 }
 #endif
 
+static int
+__ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr)
+{
+	struct ppc_inst op;
+	unsigned long ip = rec->ip;
+	unsigned long entry, ptr, tramp;
+
+	/* read where this goes */
+	if (probe_kernel_read_inst(&op, (void *)ip)) {
+		pr_err("Fetching opcode failed.\n");
+		return -EFAULT;
+	}
+
+	/* Make sure that this is still a 24bit jump */
+	if (!is_bl_op(op)) {
+		pr_err("Not expected bl: opcode is %s\n", ppc_inst_as_str(op));
+		return -EINVAL;
+	}
+
+	/* lets find where the pointer goes */
+	tramp = find_bl_target(ip, op);
+	entry = ppc_global_function_entry((void *)old_addr);
+
+	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+
+	if (tramp != entry) {
+		/* old_addr is not within range, so we must have used a trampoline */
+		struct ppc_ftrace_stub_data *stub;
+
+		hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, entry)
+			if (stub->target == entry && stub->addr == tramp)
+				break;
+
+		if (stub->target != entry || stub->addr != tramp) {
+			pr_err("we don't know about the tramp at %lx!\n", tramp);
+			return -EFAULT;
+		}
+	}
+
+	/* The new target may be within range */
+	if (test_24bit_addr(ip, addr)) {
+		/* within range */
+		if (patch_branch((struct ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
+			pr_err("REL24 out of range!\n");
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	ptr = ppc_global_function_entry((void *)addr);
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	/* Make sure we branch to ftrace_regs_caller since we only setup stubs for that */
+	entry = ppc_global_function_entry((void *)ftrace_caller);
+	if (ptr == entry)
+		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
+#endif
+
+	tramp = find_ftrace_tramp(ip, ptr);
+
+	if (!tramp) {
+		pr_err("Couldn't find a trampoline\n");
+		return -EFAULT;
+	}
+
+	pr_devel("trampoline %lx target %lx", tramp, ptr);
+
+	if (patch_branch((struct ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
+		pr_err("REL24 out of range!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -800,11 +869,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 		new = ftrace_call_replace(ip, addr, 1);
 		return ftrace_modify_code(ip, old, new);
 	} else if (core_kernel_text(ip)) {
-		/*
-		 * We always patch out of range locations to go to the regs
-		 * variant, so there is nothing to do here
-		 */
-		return 0;
+		return __ftrace_modify_call_kernel(rec, old_addr, addr);
 	}
 
 #ifdef CONFIG_MODULES
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 13/14] powerpc/ftrace: Add support for register_ftrace_direct() for MPROFILE_KERNEL
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add support for register_ftrace_direct() for MPROFILE_KERNEL, as it
depends on DYNAMIC_FTRACE_WITH_REGS.

Since powerpc only provides a branch range of 32MB, we set aside a 64k
area within kernel text for creating stubs that can be used to branch to
the provided trampoline, which can be located in the module area. This
is limited to kernel text, and as such, ftrace direct calls are not
supported for functions in kernel modules at this time.

We use orig_gpr3 to stash the address of the direct call trampoline in
arch_ftrace_set_direct_caller().  ftrace_regs_caller() is updated to
check for this to determine if we need to redirect to a direct call
trampoline. As the direct call trampoline has to work as an alternative
for the ftrace trampoline, we setup LR and r0 appropriately, and update
ctr to the trampoline address.  Finally, ftrace_graph_caller() is
updated to save/restore r0.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                          |   1 +
 arch/powerpc/include/asm/ftrace.h             |  14 ++
 arch/powerpc/kernel/trace/ftrace.c            | 140 +++++++++++++++++-
 .../powerpc/kernel/trace/ftrace_64_mprofile.S |  40 ++++-
 4 files changed, 182 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index cfc6dd787f532c..a87ac2e403196e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -197,6 +197,7 @@ config PPC
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_DYNAMIC_FTRACE
+	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS	if MPROFILE_KERNEL
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS	if MPROFILE_KERNEL
 	select HAVE_EBPF_JIT			if PPC64
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS	if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index bc76970b6ee532..2f1c46e9f5d416 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -10,6 +10,8 @@
 
 #define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
 
+#define FTRACE_STUBS_SIZE	65536
+
 #ifdef __ASSEMBLY__
 
 /* Based off of objdump optput from glibc */
@@ -59,6 +61,18 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 struct dyn_arch_ftrace {
 	struct module *mod;
 };
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+/*
+ * When there is a direct caller registered, we use regs->orig_gpr3 (similar to
+ * how x86 uses orig_ax) to let ftrace_{regs_}_caller know that we should go
+ * there instead of returning to the function
+ */
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+	regs->orig_gpr3 = addr;
+}
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index fcb21a9756e456..815b14ae45a71f 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -37,6 +37,7 @@ static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
 struct ppc_ftrace_stub_data {
 	unsigned long addr;
 	unsigned long target;
+	refcount_t refs;
 	struct hlist_node hentry;
 };
 
@@ -299,7 +300,7 @@ static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target, int lock)
 {
 	struct ppc_ftrace_stub_data *stub;
 
@@ -309,11 +310,123 @@ static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 
 	stub->addr = tramp;
 	stub->target = target;
+	refcount_set(&stub->refs, 1);
+	if (lock)
+		refcount_inc(&stub->refs);
 	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
 
 	return 0;
 }
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+static u32 ftrace_direct_stub_insns[] = {
+	PPC_RAW_LIS(12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_SLDI(12, 12, 32),
+	PPC_RAW_ORIS(12, 12, 0),
+	PPC_RAW_ORI(12, 12, 0),
+	PPC_RAW_MTCTR(12),
+	PPC_RAW_BCTR(),
+};
+#define FTRACE_NUM_STUBS	(FTRACE_STUBS_SIZE / sizeof(ftrace_direct_stub_insns))
+static DECLARE_BITMAP(stubs_bitmap, FTRACE_NUM_STUBS);
+extern unsigned int ftrace_stubs[];
+
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	struct ppc_ftrace_stub_data *stub_data;
+	struct ppc_inst instr;
+	unsigned int *stub;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub_data, hentry, target) {
+		if (stub_data->target == target &&
+				!create_branch(&instr, (void *)ip, stub_data->addr, 0)) {
+			refcount_inc(&stub_data->refs);
+			return stub_data->addr;
+		}
+	}
+
+	/* Allocate a stub */
+	do {
+		index = find_first_zero_bit(stubs_bitmap, FTRACE_NUM_STUBS);
+		if (index >= FTRACE_NUM_STUBS) {
+			pr_err("No stubs available\n");
+			return 0;
+		}
+	} while (test_and_set_bit(index, stubs_bitmap));
+	stub = &ftrace_stubs[index * sizeof(ftrace_direct_stub_insns) / 4];
+
+	if (create_branch(&instr, (void *)ip, (unsigned long)stub, 0)) {
+		/* Stub is not reachable from the ftrace location */
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	memcpy(stub, ftrace_direct_stub_insns, sizeof(ftrace_direct_stub_insns));
+	stub[0] |= IMM_L(target >> 48);
+	stub[1] |= IMM_L(target >> 32);
+	stub[3] |= IMM_L(target >> 16);
+	stub[4] |= IMM_L(target);
+	if (add_ftrace_tramp((unsigned long)stub, target, 0)) {
+		pr_err("Error allocating ftrace stub");
+		clear_bit(index, stubs_bitmap);
+		return 0;
+	}
+
+	return (unsigned long)stub;
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr)
+{
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long tramp = 0;
+	struct ppc_inst instr;
+	int index;
+
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target) {
+		if (stub->target == target && stub->addr == stub_addr &&
+				!create_branch(&instr, (void *)ip, stub->addr, 0)) {
+			if (refcount_dec_and_test(&stub->refs)) {
+				tramp = stub->addr;
+				hash_del(&stub->hentry);
+				kfree(stub);
+				break;
+			}
+			return;
+		}
+	}
+
+	if (tramp) {
+		synchronize_rcu_tasks();
+		index = (tramp - (unsigned long)ftrace_stubs) / sizeof(ftrace_direct_stub_insns);
+		clear_bit(index, stubs_bitmap);
+	}
+}
+
+int arch_register_ftrace_direct(unsigned long ip, unsigned long addr)
+{
+	if (addr & 0x03) {
+		pr_err("Target address is not at instruction boundary: 0x%lx\n", addr);
+		return -EINVAL;
+	}
+
+	if (is_module_text_address(ip)) {
+		pr_err("Kernel modules are not supported for direct calls\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+#else
+static unsigned long get_ftrace_tramp(unsigned long ip, unsigned long target)
+{
+	return find_ftrace_tramp(ip, target);
+}
+
+static void remove_ftrace_tramp(unsigned long ip, unsigned long target, unsigned long stub_addr) { }
+#endif
+
 /*
  * If this is a compiler generated long_branch trampoline (essentially, a
  * trampoline that has a branch to _mcount()), we re-write the branch to
@@ -365,7 +478,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp, ftrace_target)) {
+	if (add_ftrace_tramp(tramp, ftrace_target, 1)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -409,6 +522,8 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EPERM;
 	}
 
+	remove_ftrace_tramp(ip, addr, tramp);
+
 	return 0;
 }
 
@@ -631,7 +746,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip, ptr);
+	tramp = get_ftrace_tramp((unsigned long)ip, ptr);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -782,7 +897,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 {
 	struct ppc_inst op;
 	unsigned long ip = rec->ip;
-	unsigned long entry, ptr, tramp;
+	unsigned long entry, ptr, tramp, tramp_old = 0;
 
 	/* read where this goes */
 	if (probe_kernel_read_inst(&op, (void *)ip)) {
@@ -814,6 +929,8 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			pr_err("we don't know about the tramp at %lx!\n", tramp);
 			return -EFAULT;
 		}
+
+		tramp_old = tramp;
 	}
 
 	/* The new target may be within range */
@@ -824,7 +941,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 			return -EINVAL;
 		}
 
-		return 0;
+		goto out;
 	}
 
 	ptr = ppc_global_function_entry((void *)addr);
@@ -836,7 +953,7 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 #endif
 
-	tramp = find_ftrace_tramp(ip, ptr);
+	tramp = get_ftrace_tramp(ip, ptr);
 
 	if (!tramp) {
 		pr_err("Couldn't find a trampoline\n");
@@ -850,8 +967,13 @@ __ftrace_modify_call_kernel(struct dyn_ftrace *rec, unsigned long old_addr, unsi
 		return -EINVAL;
 	}
 
+out:
+	if (tramp_old)
+		remove_ftrace_tramp(ip, old_addr, tramp_old);
+
 	return 0;
 }
+
 int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			unsigned long addr)
 {
@@ -950,9 +1072,13 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i], addr);
+		add_ftrace_tramp((unsigned long)tramp[i], addr, 1);
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	bitmap_zero(stubs_bitmap, FTRACE_NUM_STUBS);
+#endif
+
 	return 0;
 }
 #else
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index c5602e9b07faa3..ffd2e33ff979bc 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -13,6 +13,13 @@
 #include <asm/bug.h>
 #include <asm/ptrace.h>
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	.balign	4
+.global ftrace_stubs
+ftrace_stubs:
+	.space FTRACE_STUBS_SIZE
+#endif
+
 /*
  *
  * ftrace_caller()/ftrace_regs_caller() is the function that replaces _mcount()
@@ -91,6 +98,10 @@ _GLOBAL(ftrace_regs_caller)
 	std     r10, _XER(r1)
 	std     r11, _CCR(r1)
 
+	/* Clear out orig_gpr3 */
+	li	r6, 0
+	std	r6, ORIG_GPR3(r1)
+
 	/* Load &pt_regs in r6 for call below */
 	addi    r6, r1 ,STACK_FRAME_OVERHEAD
 
@@ -103,20 +114,34 @@ ftrace_regs_call:
 	/* Load ctr with the possibly modified NIP */
 	ld	r3, _NIP(r1)
 	mtctr	r3
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+	/* Check if we should go to a direct call next */
+	ld	r4, ORIG_GPR3(r1)
+	cmpdi	r4, 0
+	beq+	1f
+	/* r4 has the direct call target, setup LR and r0 as on our entry, reset cr0 */
+	mtctr	r4
+	mtlr	r3
+	ld	r0, _LINK(r1)
+	cmpd	r3, r3
+	b	2f
+#endif
+
+1:
 #ifdef CONFIG_LIVEPATCH
 	cmpd	r14, r3		/* has NIP been altered? */
 #endif
 
-	/* Restore gprs */
-	REST_GPR(0,r1)
-	REST_10GPRS(2,r1)
-	REST_10GPRS(12,r1)
-	REST_10GPRS(22,r1)
-
 	/* Restore possibly modified LR */
 	ld	r0, _LINK(r1)
 	mtlr	r0
 
+	/* Restore gprs */
+2:	REST_10GPRS(2,r1)
+	REST_10GPRS(12,r1)
+	REST_10GPRS(22,r1)
+
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
@@ -282,6 +307,7 @@ _GLOBAL(ftrace_graph_caller)
 	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
 	SAVE_8GPRS(3, r1)
+	SAVE_GPR(0, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
@@ -304,6 +330,8 @@ _GLOBAL(ftrace_graph_caller)
 
 	ld	r0, _NIP(r1)
 	mtctr	r0
+
+	REST_GPR(0, r1)
 	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 09/14] powerpc/ftrace: Use a hash table for tracking ftrace stubs
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

In preparation for having to deal with large number of ftrace stubs in
support of ftrace direct calls, convert existing stubs to use a hash
table. The hash table is key'ed off the target address for the stubs
since there could be multiple stubs for the same target to cover the
full kernel text.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 75 +++++++++++++-----------------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 14b39f7797d455..7ddb6e4b527c39 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) "ftrace-powerpc: " fmt
 
+#include <linux/hashtable.h>
 #include <linux/spinlock.h>
 #include <linux/hardirq.h>
 #include <linux/uaccess.h>
@@ -32,14 +33,12 @@
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 
-/*
- * We generally only have a single long_branch tramp and at most 2 or 3 plt
- * tramps generated. But, we don't use the plt tramps currently. We also allot
- * 2 tramps after .text and .init.text. So, we only end up with around 3 usable
- * tramps in total. Set aside 8 just to be sure.
- */
-#define	NUM_FTRACE_TRAMPS	8
-static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
+static DEFINE_HASHTABLE(ppc_ftrace_stubs, 8);
+struct ppc_ftrace_stub_data {
+	unsigned long addr;
+	unsigned long target;
+	struct hlist_node hentry;
+};
 
 static struct ppc_inst
 ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
@@ -288,36 +287,31 @@ __ftrace_make_nop(struct module *mod,
 #endif /* PPC64 */
 #endif /* CONFIG_MODULES */
 
-static unsigned long find_ftrace_tramp(unsigned long ip)
+static unsigned long find_ftrace_tramp(unsigned long ip, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 	struct ppc_inst instr;
 
-	/*
-	 * We have the compiler generated long_branch tramps at the end
-	 * and we prefer those
-	 */
-	for (i = NUM_FTRACE_TRAMPS - 1; i >= 0; i--)
-		if (!ftrace_tramps[i])
-			continue;
-		else if (create_branch(&instr, (void *)ip,
-				       ftrace_tramps[i], 0) == 0)
-			return ftrace_tramps[i];
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, target)
+		if (stub->target == target && !create_branch(&instr, (void *)ip, stub->addr, 0))
+			return stub->addr;
 
 	return 0;
 }
 
-static int add_ftrace_tramp(unsigned long tramp)
+static int add_ftrace_tramp(unsigned long tramp, unsigned long target)
 {
-	int i;
+	struct ppc_ftrace_stub_data *stub;
 
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i]) {
-			ftrace_tramps[i] = tramp;
-			return 0;
-		}
+	stub = kmalloc(sizeof(*stub), GFP_KERNEL);
+	if (!stub)
+		return -1;
 
-	return -1;
+	stub->addr = tramp;
+	stub->target = target;
+	hash_add(ppc_ftrace_stubs, &stub->hentry, target);
+
+	return 0;
 }
 
 /*
@@ -328,16 +322,14 @@ static int add_ftrace_tramp(unsigned long tramp)
  */
 static int setup_mcount_compiler_tramp(unsigned long tramp)
 {
-	int i;
 	struct ppc_inst op;
-	unsigned long ptr;
 	struct ppc_inst instr;
+	struct ppc_ftrace_stub_data *stub;
+	unsigned long ptr, ftrace_target = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
 
 	/* Is this a known long jump tramp? */
-	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
-		if (!ftrace_tramps[i])
-			break;
-		else if (ftrace_tramps[i] == tramp)
+	hash_for_each_possible(ppc_ftrace_stubs, stub, hentry, ftrace_target)
+		if (stub->target == ftrace_target && stub->addr == tramp)
 			return 0;
 
 	/* New trampoline -- read where this goes */
@@ -361,19 +353,18 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	}
 
 	/* Let's re-write the tramp to go to ftrace_[regs_]caller */
-	ptr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR);
-	if (create_branch(&instr, (void *)tramp, ptr, 0)) {
+	if (create_branch(&instr, (void *)tramp, ftrace_target, 0)) {
 		pr_debug("%ps is not reachable from existing mcount tramp\n",
-				(void *)ptr);
+				(void *)ftrace_target);
 		return -1;
 	}
 
-	if (patch_branch((struct ppc_inst *)tramp, ptr, 0)) {
+	if (patch_branch((struct ppc_inst *)tramp, ftrace_target, 0)) {
 		pr_debug("REL24 out of range!\n");
 		return -1;
 	}
 
-	if (add_ftrace_tramp(tramp)) {
+	if (add_ftrace_tramp(tramp, ftrace_target)) {
 		pr_debug("No tramp locations left\n");
 		return -1;
 	}
@@ -405,7 +396,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 
 	if (setup_mcount_compiler_tramp(tramp)) {
 		/* Are other trampolines reachable? */
-		if (!find_ftrace_tramp(ip)) {
+		if (!find_ftrace_tramp(ip, FTRACE_REGS_ADDR)) {
 			pr_err("No ftrace trampolines reachable from %ps\n",
 					(void *)ip);
 			return -EINVAL;
@@ -646,7 +637,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	tramp = find_ftrace_tramp((unsigned long)ip);
+	tramp = find_ftrace_tramp((unsigned long)ip, FTRACE_REGS_ADDR);
 	if (!tramp) {
 		pr_err("No ftrace trampolines reachable from %ps\n", ip);
 		return -EINVAL;
@@ -894,7 +885,7 @@ int __init ftrace_dyn_arch_init(void)
 		memcpy(tramp[i], stub_insns, sizeof(stub_insns));
 		tramp[i][1] |= PPC_HA(reladdr);
 		tramp[i][2] |= PPC_LO(reladdr);
-		add_ftrace_tramp((unsigned long)tramp[i]);
+		add_ftrace_tramp((unsigned long)tramp[i], addr);
 	}
 
 	return 0;
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 11/14] powerpc/ftrace: Use GPR save/restore macros in ftrace_graph_caller()
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Use SAVE_8GPRS(), REST_8GPRS() and _NIP(), along with using the standard
SWITCH_FRAME_SIZE for the stack frame in ftrace_graph_caller() to
simplify code. This increases the stack frame size, but it is unlikely
to be an issue since ftrace_[regs_]caller() have just used a similar
stack frame size, and it isn't evident that the graph caller has too
deep a call stack to cause issues.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 .../powerpc/kernel/trace/ftrace_64_mprofile.S | 28 +++++--------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index f9fd5f743eba34..bbe871b47ade58 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -279,24 +279,17 @@ livepatch_handler:
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 _GLOBAL(ftrace_graph_caller)
-	stdu	r1, -112(r1)
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
 	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
-	std	r10, 104(r1)
-	std	r9, 96(r1)
-	std	r8, 88(r1)
-	std	r7, 80(r1)
-	std	r6, 72(r1)
-	std	r5, 64(r1)
-	std	r4, 56(r1)
-	std	r3, 48(r1)
+	SAVE_8GPRS(3, r1)
 
 	/* Save callee's TOC in the ABI compliant location */
 	std	r2, 24(r1)
 	ld	r2, PACATOC(r13)	/* get kernel TOC in r2 */
 
-	addi	r5, r1, 112
+	addi	r5, r1, SWITCH_FRAME_SIZE
 	mfctr	r4		/* ftrace_caller has moved local addr here */
-	std	r4, 40(r1)
+	std	r4, _NIP(r1)
 	mflr	r3		/* ftrace_caller has restored LR from stack */
 	subi	r4, r4, MCOUNT_INSN_SIZE
 
@@ -309,21 +302,14 @@ _GLOBAL(ftrace_graph_caller)
 	 */
 	mtlr	r3
 
-	ld	r0, 40(r1)
+	ld	r0, _NIP(r1)
 	mtctr	r0
-	ld	r10, 104(r1)
-	ld	r9, 96(r1)
-	ld	r8, 88(r1)
-	ld	r7, 80(r1)
-	ld	r6, 72(r1)
-	ld	r5, 64(r1)
-	ld	r4, 56(r1)
-	ld	r3, 48(r1)
+	REST_8GPRS(3, r1)
 
 	/* Restore callee's TOC */
 	ld	r2, 24(r1)
 
-	addi	r1, r1, 112
+	addi	r1, r1, SWITCH_FRAME_SIZE
 	mflr	r0
 	std	r0, LRSAVE(r1)
 	bctr
-- 
2.25.4


^ permalink raw reply related

* [RFC PATCH 14/14] samples/ftrace: Add powerpc support for ftrace direct samples
From: Naveen N. Rao @ 2020-11-26 18:08 UTC (permalink / raw)
  To: Steven Rostedt, Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1606412433.git.naveen.n.rao@linux.vnet.ibm.com>

Add a simple powerpc trampoline to demonstrate use of ftrace direct on
powerpc.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 samples/Kconfig                       |  2 +-
 samples/ftrace/ftrace-direct-modify.c | 58 +++++++++++++++++++++++++++
 samples/ftrace/ftrace-direct-too.c    | 48 ++++++++++++++++++++--
 samples/ftrace/ftrace-direct.c        | 45 +++++++++++++++++++--
 4 files changed, 144 insertions(+), 9 deletions(-)

diff --git a/samples/Kconfig b/samples/Kconfig
index 0ed6e4d71d87b1..fdc9e44dba3b95 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -26,7 +26,7 @@ config SAMPLE_TRACE_PRINTK
 config SAMPLE_FTRACE_DIRECT
 	tristate "Build register_ftrace_direct() example"
 	depends on DYNAMIC_FTRACE_WITH_DIRECT_CALLS && m
-	depends on X86_64 # has x86_64 inlined asm
+	depends on X86_64 || PPC64 # has inlined asm
 	help
 	  This builds an ftrace direct function example
 	  that hooks to wake_up_process and prints the parameters.
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index c13a5bc5095bea..89d66a12d300e1 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -2,6 +2,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func1(void)
 {
@@ -18,6 +19,7 @@ extern void my_tramp2(void *);
 
 static unsigned long my_ip = (unsigned long)schedule;
 
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp1, @function\n"
@@ -38,6 +40,58 @@ asm (
 "	.size		my_tramp2, .-my_tramp2\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp1, @function\n"
+"	.global		my_tramp1\n"
+"   my_tramp1:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (3f - 1b)(12)\n"
+"	bl	my_direct_func1\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp1, .-my_tramp1\n"
+"	nop\n"
+"	.type		my_tramp2, @function\n"
+"	.global		my_tramp2\n"
+"   my_tramp2:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 2f\n"
+"2:	mflr	12\n"
+"	ld	2, (3f - 2b)(12)\n"
+"	bl	my_direct_func2\n"
+"	nop\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp2, .-my_tramp2\n"
+"3:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
+
 
 static unsigned long my_tramp = (unsigned long)my_tramp1;
 static unsigned long tramps[2] = {
@@ -72,6 +126,10 @@ static int __init ftrace_direct_init(void)
 {
 	int ret;
 
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+
 	ret = register_ftrace_direct(my_ip, my_tramp);
 	if (!ret)
 		simple_tsk = kthread_run(simple_thread, NULL, "event-sample-fn");
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index d5c5022be66429..9a82abecbe0dcc 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -3,6 +3,7 @@
 
 #include <linux/mm.h> /* for handle_mm_fault() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h>
 
 void my_direct_func(struct vm_area_struct *vma,
 			unsigned long address, unsigned int flags)
@@ -13,6 +14,9 @@ void my_direct_func(struct vm_area_struct *vma,
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)handle_mm_fault;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -31,18 +35,54 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	std	4, 144(1)\n"
+"	std	5, 152(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	5, 152(1)\n"
+"	ld	4, 144(1)\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)handle_mm_fault,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)handle_mm_fault,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index 63ca06d42c803f..da67b6217f91d2 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h> /* for wake_up_process() */
 #include <linux/ftrace.h>
+#include <linux/kprobes.h> /* for ppc_function_entry() */
 
 void my_direct_func(struct task_struct *p)
 {
@@ -11,6 +12,9 @@ void my_direct_func(struct task_struct *p)
 
 extern void my_tramp(void *);
 
+static unsigned long my_ip = (unsigned long)wake_up_process;
+
+#ifdef CONFIG_X86
 asm (
 "	.pushsection    .text, \"ax\", @progbits\n"
 "	.type		my_tramp, @function\n"
@@ -25,18 +29,51 @@ asm (
 "	.size		my_tramp, .-my_tramp\n"
 "	.popsection\n"
 );
+#elif CONFIG_PPC64
+asm (
+"	.pushsection	.text, \"ax\", @progbits\n"
+"	.type		my_tramp, @function\n"
+"	.global		my_tramp\n"
+"   my_tramp:\n"
+"	std	0, 16(1)\n"
+"	stdu	1, -480(1)\n"
+"	std	2, 24(1)\n"
+"	std	3, 136(1)\n"
+"	mflr	7\n"
+"	std	7, 368(1)\n"
+"	bcl	20, 31, 1f\n"
+"1:	mflr	12\n"
+"	ld	2, (2f - 1b)(12)\n"
+"	bl	my_direct_func\n"
+"	nop\n"
+"	ld	3, 136(1)\n"
+"	ld	2, 24(1)\n"
+"	ld	7, 368(1)\n"
+"	mtctr	7\n"
+"	addi	1, 1, 480\n"
+"	ld	0, 16(1)\n"
+"	mtlr	0\n"
+"	bctr\n"
+"	.size		my_tramp, .-my_tramp\n"
+"2:\n"
+"	.quad		.TOC.@tocbase\n"
+"	.popsection\n"
+);
+#endif
 
 
 static int __init ftrace_direct_init(void)
 {
-	return register_ftrace_direct((unsigned long)wake_up_process,
-				     (unsigned long)my_tramp);
+#ifdef CONFIG_PPC64
+	/* Ftrace location is (usually) the second instruction at a function's local entry point */
+	my_ip = ppc_function_entry((void *)my_ip) + 4;
+#endif
+	return register_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 static void __exit ftrace_direct_exit(void)
 {
-	unregister_ftrace_direct((unsigned long)wake_up_process,
-				 (unsigned long)my_tramp);
+	unregister_ftrace_direct(my_ip, (unsigned long)my_tramp);
 }
 
 module_init(ftrace_direct_init);
-- 
2.25.4


^ permalink raw reply related

* Re: [PATCH] ASoC: fsl: Fix config name of CONFIG_ARCH_MXC
From: Mark Brown @ 2020-11-26 20:05 UTC (permalink / raw)
  To: tiwai, timur, perex, festevam, Xiubo.Lee, Shengjiu Wang,
	alsa-devel, nicoleotsuka
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1606371293-29099-1-git-send-email-shengjiu.wang@nxp.com>

On Thu, 26 Nov 2020 14:14:53 +0800, Shengjiu Wang wrote:
> CONFIG_ARCH_MXC should be ARCH_MXC

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: fsl: Fix config name of CONFIG_ARCH_MXC
      commit: c61d1142cfd45f58b63bf9d2d59523f91096e873

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* Re: [PATCH v2 00/19] Add generic vdso_base tracking
From: Dmitry Safonov @ 2020-11-26 20:41 UTC (permalink / raw)
  To: Christophe Leroy, linux-kernel
  Cc: Thomas Bogendoerfer, Arnd Bergmann, Catalin Marinas, x86,
	Dmitry Safonov, Oleg Nesterov, Russell King,
	linuxppc-dev@lists.ozlabs.org, Ingo Molnar, Borislav Petkov,
	Alexander Viro, Andy Lutomirski, H. Peter Anvin, Guo Ren,
	Andrew Morton, Vincenzo Frascino, Will Deacon, Thomas Gleixner
In-Reply-To: <5e315bf6-b03d-e66e-9557-22ece397080e@csgroup.eu>

Hi Christophe,

On 11/24/20 6:53 AM, Christophe Leroy wrote:
> 
> 
> Le 24/11/2020 à 01:29, Dmitry Safonov a écrit :
>> v2 Changes:
>> - Rename user_landing to vdso_base as it tracks vDSO VMA start address,
>>    rather than the explicit address to land (Andy)
>> - Reword and don't use "new-execed" and "new-born" task (Andy)
>> - Fix failures reported by build robot
>>
>> Started from discussion [1], where was noted that currently a couple of
>> architectures support mremap() for vdso/sigpage, but not munmap().
>> If an application maps something on the ex-place of vdso/sigpage,
>> later after processing signal it will land there (good luck!)
>>
>> Patches set is based on linux-next (next-20201123) and it depends on
>> changes in x86/cleanups (those reclaim TIF_IA32/TIF_X32) and also
>> on my changes in akpm (fixing several mremap() issues).
> 
> I have a series that cleans up VDSO init on powerpc and migrates powerpc
> to _install_special_mapping() (patch 10 of the series).
> 
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=204396&state=%2A&archive=both
> 
> 
> I'm wondering how we should coordinate with your series for merging.
> 
> I guess your series will also imply removal of arch_unmap() ? see
> https://elixir.bootlin.com/linux/v5.10-rc4/source/arch/powerpc/include/asm/mmu_context.h#L262

I think our series intersect only in that moment where I re-introduce
arch_setup_additional_pages() parameters. So, in theory we could
minimize the conflicts by merging both series in parallel and cleanup
the result by moving to generic vdso_base on the top, what do you think?

Thanks,
          Dmitry

^ permalink raw reply

* [Bug 209733] Starting new KVM virtual machines on PPC64 starts to hang after box is up for a while
From: bugzilla-daemon @ 2020-11-26 23:16 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-209733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=209733

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |michael@ellerman.id.au

--- Comment #5 from Michael Ellerman (michael@ellerman.id.au) ---
Thanks for persisting with the testing.

I wonder if it was fixed by:

c4629e4e7e09 ("mm/compaction: stop isolation if too many pages are isolated and
we have pages to migrate")
or
38935861d85a ("mm/compaction: count pages and stop correctly during page
isolation")

They fix a potential infinte loop in a path that's used by the HTAB allocation.

Those landed in v5.9.9, and fix a commit that was introduced in v5.7 (which
doesn't match your observation that v5.7.x was OK).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH net 1/2] ibmvnic: Ensure that SCRQ entry reads are correctly ordered
From: Michael Ellerman @ 2020-11-26 23:57 UTC (permalink / raw)
  To: Thomas Falcon, netdev
  Cc: cforno12, ljp, ricklind, dnbanerg, drt, brking, sukadev,
	linuxppc-dev
In-Reply-To: <2da3e517-f1dd-95c9-11db-a6c62bf61978@linux.ibm.com>

Thomas Falcon <tlfalcon@linux.ibm.com> writes:
> On 11/24/20 11:43 PM, Michael Ellerman wrote:
>> Thomas Falcon <tlfalcon@linux.ibm.com> writes:
>>> Ensure that received Subordinate Command-Response Queue (SCRQ)
>>> entries are properly read in order by the driver. These queues
>>> are used in the ibmvnic device to process RX buffer and TX completion
>>> descriptors. dma_rmb barriers have been added after checking for a
>>> pending descriptor to ensure the correct descriptor entry is checked
>>> and after reading the SCRQ descriptor to ensure the entire
>>> descriptor is read before processing.
>>>
>>> Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
>>> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
>>> ---
>>>   drivers/net/ethernet/ibm/ibmvnic.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>>> index 2aa40b2..489ed5e 100644
>>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>>> @@ -2403,6 +2403,8 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
>>>   
>>>   		if (!pending_scrq(adapter, adapter->rx_scrq[scrq_num]))
>>>   			break;
>>> +		/* ensure that we do not prematurely exit the polling loop */
>>> +		dma_rmb();
>> I'd be happier if these comments were more specific about which read(s)
>> they are ordering vs which other read(s).
>>
>> I'm sure it's obvious to you, but it may not be to a future author,
>> and/or after the code has been refactored over time.
>
> Thank you for reviewing! I will submit a v2 soon with clearer comments 
> on the reads being ordered here.

Thanks.

cheers

^ permalink raw reply

* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Michael Ellerman @ 2020-11-27  1:03 UTC (permalink / raw)
  To: Bill Wendling; +Cc: Nick Desaulniers, linuxppc-dev
In-Reply-To: <CAGG=3QUSF4UwcZQHhFE-PW6As7GVJknsyGkgVMENDXghABzy5A@mail.gmail.com>

Bill Wendling <morbo@google.com> writes:
> On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Bill Wendling <morbo@google.com> writes:
>> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
>> > <segher@kernel.crashing.org> wrote:
>> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
>> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
>> >> > <segher@kernel.crashing.org> wrote:
>> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
>> >> > > > <segher@kernel.crashing.org> wrote:
>> >> > > > > "true" (as a result of a comparison) in as is -1, not 1.
>> >> > >
>> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote:
>> >> > > > What Segher said. :-) Also, if you reverse the comparison, you'll get
>> >> > > > a build error.
>> >> > >
>> >> > > But that means your patch is the wrong way around?
>> >> > >
>> >> > > -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
>> >> > > -       .error "Feature section else case larger than body";    \
>> >> > > -       .endif;                                                 \
>> >> > > +       .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
>> >> > >
>> >> > > It should be a + in that last line, not a -.
>> >> >
>> >> > I said so in a follow up email.
>> >>
>> >> Yeah, and that arrived a second after I pressed "send" :-)
>> >>
>> > Michael, I apologize for the churn with these patches. I believe the
>> > policy is to resend the match as "v4", correct?
>> >
>> > I ran tests with the change above. It compiled with no error. If I
>> > switch the labels around to ".org . + ((label##2b-label##1b) >
>> > (label##4b-label##3b))", then it fails as expected.
>>
>> I wanted to retain the nicer error reporting for gcc builds, so I did it
>> like this:
>>
>> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
>> index b0af97add751..c4ad33074df5 100644
>> --- a/arch/powerpc/include/asm/feature-fixups.h
>> +++ b/arch/powerpc/include/asm/feature-fixups.h
>> @@ -36,6 +36,24 @@ label##2:                                            \
>>         .align 2;                                       \
>>  label##3:
>>
>> +
>> +#ifndef CONFIG_CC_IS_CLANG
>> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
>> +       .ifgt (else_size) - (body_size);                        \
>> +       .error "Feature section else case larger than body";    \
>> +       .endif;
>> +#else
>> +/*
>> + * If we use the ifgt syntax above, clang's assembler complains about the
>> + * expression being non-absolute when the code appears in an inline assembly
>> + * statement.
>> + * As a workaround use an .org directive that has no effect if the else case
>> + * instructions are smaller than the body, but fails otherwise.
>> + */
>> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
>> +       .org . + ((else_size) > (body_size));
>> +#endif
>> +
>>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)          \
>>  label##4:                                                      \
>>         .popsection;                                            \
>> @@ -48,9 +66,7 @@ label##5:                                                     \
>>         FTR_ENTRY_OFFSET label##2b-label##5b;                   \
>>         FTR_ENTRY_OFFSET label##3b-label##5b;                   \
>>         FTR_ENTRY_OFFSET label##4b-label##5b;                   \
>> -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
>> -       .error "Feature section else case larger than body";    \
>> -       .endif;                                                 \
>> +       CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \
>>         .popsection;
>>
>>
>>
>> I've pushed a branch with all your patches applied to:
>>
>>   https://github.com/linuxppc/linux/commits/next-test
>>
> This works for me. Thanks!

Great.

>> Are you able to give that a quick test? It builds clean with clang for
>> me, but we must be using different versions of clang because my branch
>> already builds clean for me even without your patches.
>>
> You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That
> turns on clang's integrated assembler, which I think is disabled by
> default.

Yep that does it.

But then I get:
  clang: error: unsupported argument '-mpower4' to option 'Wa,'
  clang: error: unsupported argument '-many' to option 'Wa,'

So I guess I'm still missing something?

> Note that with clang's integrated assembler, arch/powerpc/boot/util.S
> fails to compile. Alan Modra mentioned that he sent you a patch to
> "modernize" the file so that clang can compile it.

Ah you're right he did, it didn't go to patchwork so I missed it. Have
grabbed it now.

cheers

^ permalink raw reply

* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Bill Wendling @ 2020-11-27  1:59 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev
In-Reply-To: <87ft4vy5jp.fsf@mpe.ellerman.id.au>

On Thu, Nov 26, 2020 at 5:03 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> >> > <segher@kernel.crashing.org> wrote:
> >> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
> >> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
> >> >> > <segher@kernel.crashing.org> wrote:
> >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> >> > > > <segher@kernel.crashing.org> wrote:
> >> >> > > > > "true" (as a result of a comparison) in as is -1, not 1.
> >> >> > >
> >> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote:
> >> >> > > > What Segher said. :-) Also, if you reverse the comparison, you'll get
> >> >> > > > a build error.
> >> >> > >
> >> >> > > But that means your patch is the wrong way around?
> >> >> > >
> >> >> > > -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> >> > > -       .error "Feature section else case larger than body";    \
> >> >> > > -       .endif;                                                 \
> >> >> > > +       .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
> >> >> > >
> >> >> > > It should be a + in that last line, not a -.
> >> >> >
> >> >> > I said so in a follow up email.
> >> >>
> >> >> Yeah, and that arrived a second after I pressed "send" :-)
> >> >>
> >> > Michael, I apologize for the churn with these patches. I believe the
> >> > policy is to resend the match as "v4", correct?
> >> >
> >> > I ran tests with the change above. It compiled with no error. If I
> >> > switch the labels around to ".org . + ((label##2b-label##1b) >
> >> > (label##4b-label##3b))", then it fails as expected.
> >>
> >> I wanted to retain the nicer error reporting for gcc builds, so I did it
> >> like this:
> >>
> >> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
> >> index b0af97add751..c4ad33074df5 100644
> >> --- a/arch/powerpc/include/asm/feature-fixups.h
> >> +++ b/arch/powerpc/include/asm/feature-fixups.h
> >> @@ -36,6 +36,24 @@ label##2:                                            \
> >>         .align 2;                                       \
> >>  label##3:
> >>
> >> +
> >> +#ifndef CONFIG_CC_IS_CLANG
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .ifgt (else_size) - (body_size);                        \
> >> +       .error "Feature section else case larger than body";    \
> >> +       .endif;
> >> +#else
> >> +/*
> >> + * If we use the ifgt syntax above, clang's assembler complains about the
> >> + * expression being non-absolute when the code appears in an inline assembly
> >> + * statement.
> >> + * As a workaround use an .org directive that has no effect if the else case
> >> + * instructions are smaller than the body, but fails otherwise.
> >> + */
> >> +#define CHECK_ALT_SIZE(else_size, body_size)                   \
> >> +       .org . + ((else_size) > (body_size));
> >> +#endif
> >> +
> >>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)          \
> >>  label##4:                                                      \
> >>         .popsection;                                            \
> >> @@ -48,9 +66,7 @@ label##5:                                                     \
> >>         FTR_ENTRY_OFFSET label##2b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##3b-label##5b;                   \
> >>         FTR_ENTRY_OFFSET label##4b-label##5b;                   \
> >> -       .ifgt (label##4b- label##3b)-(label##2b- label##1b);    \
> >> -       .error "Feature section else case larger than body";    \
> >> -       .endif;                                                 \
> >> +       CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \
> >>         .popsection;
> >>
> >>
> >>
> >> I've pushed a branch with all your patches applied to:
> >>
> >>   https://github.com/linuxppc/linux/commits/next-test
> >>
> > This works for me. Thanks!
>
> Great.
>
> >> Are you able to give that a quick test? It builds clean with clang for
> >> me, but we must be using different versions of clang because my branch
> >> already builds clean for me even without your patches.
> >>
> > You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That
> > turns on clang's integrated assembler, which I think is disabled by
> > default.
>
> Yep that does it.
>
> But then I get:
>   clang: error: unsupported argument '-mpower4' to option 'Wa,'
>   clang: error: unsupported argument '-many' to option 'Wa,'
>
> So I guess I'm still missing something?
>
[Resent, because my previous email went out as non-plain text.]

I've seen that too. I'm not entirely sure what's causing it, but I'll
look into it. I've got a backlog of things to work on still. :-) It's
probably a clang issue. There's another one that came up having to do
with the format of some PPC instructions. I have a clang fix on review
for those.

> > Note that with clang's integrated assembler, arch/powerpc/boot/util.S
> > fails to compile. Alan Modra mentioned that he sent you a patch to
> > "modernize" the file so that clang can compile it.
>
> Ah you're right he did, it didn't go to patchwork so I missed it. Have
> grabbed it now.
>
Thanks!

-bw

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox