[PATCH] powerpc: Add user-return-notifier support

[PATCH] powerpc: Add user-return-notifier support

[Ananth N Mavinakayanahalli]

Add user return notifier support for powerpc. Similar to x86, this feature
keys off of the KVM Kconfig.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
---
 Documentation/features/debug/user-ret-profiler/arch-support.txt |    2 +-
 arch/powerpc/Kconfig                                            |    1 +
 arch/powerpc/include/asm/thread_info.h                          |    2 ++
 arch/powerpc/kernel/process.c                                   |    4 ++++
 arch/powerpc/kernel/signal.c                                    |    4 ++++
 arch/powerpc/kvm/Kconfig                                        |    1 +
 6 files changed, 13 insertions(+), 1 deletion(-)

Index: linux-4.2-rc6/Documentation/features/debug/user-ret-profiler/arch-support.txt
===================================================================
--- linux-4.2-rc6.orig/Documentation/features/debug/user-ret-profiler/arch-support.txt
+++ linux-4.2-rc6/Documentation/features/debug/user-ret-profiler/arch-support.txt
@@ -27,7 +27,7 @@
     |       nios2: | TODO |
     |    openrisc: | TODO |
     |      parisc: | TODO |
-    |     powerpc: | TODO |
+    |     powerpc: |  ok  |
     |        s390: | TODO |
     |       score: | TODO |
     |          sh: | TODO |
Index: linux-4.2-rc6/arch/powerpc/Kconfig
===================================================================
--- linux-4.2-rc6.orig/arch/powerpc/Kconfig
+++ linux-4.2-rc6/arch/powerpc/Kconfig
@@ -106,6 +106,7 @@ config PPC
 	select HAVE_ARCH_KGDB
 	select HAVE_KRETPROBES
 	select HAVE_ARCH_TRACEHOOK
+	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP
 	select HAVE_DMA_ATTRS
Index: linux-4.2-rc6/arch/powerpc/include/asm/thread_info.h
===================================================================
--- linux-4.2-rc6.orig/arch/powerpc/include/asm/thread_info.h
+++ linux-4.2-rc6/arch/powerpc/include/asm/thread_info.h
@@ -86,6 +86,7 @@ static inline struct thread_info *curren
 					   TIF_NEED_RESCHED */
 #define TIF_32BIT		4	/* 32 bit binary */
 #define TIF_RESTORE_TM		5	/* need to restore TM FP/VEC/VSX */
+#define TIF_USER_RETURN_NOTIFY	6	/* notify kernel of userspace return */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SINGLESTEP		8	/* singlestepping active */
 #define TIF_NOHZ		9	/* in adaptive nohz mode */
@@ -109,6 +110,7 @@ static inline struct thread_info *curren
 #define _TIF_POLLING_NRFLAG	(1<<TIF_POLLING_NRFLAG)
 #define _TIF_32BIT		(1<<TIF_32BIT)
 #define _TIF_RESTORE_TM		(1<<TIF_RESTORE_TM)
+#define _TIF_USER_RETURN_NOTIFY	(1<<TIF_USER_RETURN_NOTIFY)
 #define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
 #define _TIF_SINGLESTEP		(1<<TIF_SINGLESTEP)
 #define _TIF_SECCOMP		(1<<TIF_SECCOMP)
Index: linux-4.2-rc6/arch/powerpc/kernel/process.c
===================================================================
--- linux-4.2-rc6.orig/arch/powerpc/kernel/process.c
+++ linux-4.2-rc6/arch/powerpc/kernel/process.c
@@ -38,6 +38,7 @@
 #include <linux/random.h>
 #include <linux/hw_breakpoint.h>
 #include <linux/uaccess.h>
+#include <linux/user-return-notifier.h>
 
 #include <asm/pgtable.h>
 #include <asm/io.h>
@@ -894,6 +895,9 @@ struct task_struct *__switch_to(struct t
 	}
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
+	if (unlikely(task_thread_info(prev)->flags & _TIF_USER_RETURN_NOTIFY))
+		propagate_user_return_notify(prev, new);
+
 	return last;
 }
 
Index: linux-4.2-rc6/arch/powerpc/kernel/signal.c
===================================================================
--- linux-4.2-rc6.orig/arch/powerpc/kernel/signal.c
+++ linux-4.2-rc6/arch/powerpc/kernel/signal.c
@@ -14,6 +14,7 @@
 #include <linux/uprobes.h>
 #include <linux/key.h>
 #include <linux/context_tracking.h>
+#include <linux/user-return-notifier.h>
 #include <asm/hw_breakpoint.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -159,6 +160,9 @@ void do_notify_resume(struct pt_regs *re
 		tracehook_notify_resume(regs);
 	}
 
+	if (thread_info_flags & _TIF_USER_RETURN_NOTIFY)
+		fire_user_return_notifiers();
+
 	user_enter();
 }
 
Index: linux-4.2-rc6/arch/powerpc/kvm/Kconfig
===================================================================
--- linux-4.2-rc6.orig/arch/powerpc/kvm/Kconfig
+++ linux-4.2-rc6/arch/powerpc/kvm/Kconfig
@@ -22,6 +22,7 @@ config KVM
 	select ANON_INODES
 	select HAVE_KVM_EVENTFD
 	select SRCU
+	select USER_RETURN_NOTIFIER
 
 config KVM_BOOK3S_HANDLER
 	bool

Re: powerpc: Add user-return-notifier support

[Michael Ellerman]

On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:
> Add user return notifier support for powerpc. Similar to x86, this feature
> keys off of the KVM Kconfig.

Please flesh this out.

What is it, why do we want it, why is your implementation correct.

cheers

Re: powerpc: Add user-return-notifier support

[Ananth N Mavinakayanahalli]

On Mon, Aug 31, 2015 at 08:35:17PM +1000, Michael Ellerman wrote:
> On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:
> > Add user return notifier support for powerpc. Similar to x86, this feature
> > keys off of the KVM Kconfig.
> 
> Please flesh this out.
> 
> What is it, why do we want it, why is your implementation correct.

The only current in-kernel user of the infrastructure is KVM on x86. It
is useful for optimizations when MSR values can continue to be used
between the host and guest. Commit log for 18863bdd60f8 upstream has a
more complete explanation.

I do not know the inner details of the KVM implementation on Power,
perhaps Paul/Gautham can comment on if a similar optimization will
benefit Power systems too?

We could certainly make this a generic config option.. but I am yet to
see a real usecase outside of the KVM thingy. We do use TIF_UPROBE for
something very similar, though that needs to fire much before the
outstanding signals get handled. The user-return notifier is the last
thing executed before return to userspace -- the two cannot be merged.

Ananth

PS: I am not sure having an 'ok' against the Documentation/features/ for
powerpc is a valid enough argument for this :-)

Re: powerpc: Add user-return-notifier support

[Scott Wood]

On Tue, 2015-09-01 at 12:11 +0530, Ananth N Mavinakayanahalli wrote:
> On Mon, Aug 31, 2015 at 08:35:17PM +1000, Michael Ellerman wrote:
> > On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:
> > > Add user return notifier support for powerpc. Similar to x86, this 
> > > feature
> > > keys off of the KVM Kconfig.
> > 
> > Please flesh this out.
> > 
> > What is it, why do we want it, why is your implementation correct.
> 
> The only current in-kernel user of the infrastructure is KVM on x86. It
> is useful for optimizations when MSR values can continue to be used
> between the host and guest. Commit log for 18863bdd60f8 upstream has a
> more complete explanation.
>
> I do not know the inner details of the KVM implementation on Power,
> perhaps Paul/Gautham can comment on if a similar optimization will
> benefit Power systems too?

"MSR" is x86-specific terminology and is pretty vague.  What specifically is 
the functionality being optimized, in terms of things that actually exist on 
PPC?

In any case, that commit log doesn't explain what user-return-notifier is or 
how it works, just that it's being used.

> We could certainly make this a generic config option.. but I am yet to
> see a real usecase outside of the KVM thingy. We do use TIF_UPROBE for
> something very similar, though that needs to fire much before the
> outstanding signals get handled. The user-return notifier is the last
> thing executed before return to userspace -- the two cannot be merged.
> 
> Ananth
> 
> PS: I am not sure having an 'ok' against the Documentation/features/ for
> powerpc is a valid enough argument for this :-)

So how did you test this?  What platforms did you test it on?  What hardware 
support does it need?

-Scott

Re: powerpc: Add user-return-notifier support

[Ananth N Mavinakayanahalli]

On Tue, Sep 01, 2015 at 07:03:12PM -0500, Scott Wood wrote:
> On Tue, 2015-09-01 at 12:11 +0530, Ananth N Mavinakayanahalli wrote:
> > On Mon, Aug 31, 2015 at 08:35:17PM +1000, Michael Ellerman wrote:
> > > On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:
> > > > Add user return notifier support for powerpc. Similar to x86, this 
> > > > feature
> > > > keys off of the KVM Kconfig.
> > > 
> > > Please flesh this out.
> > > 
> > > What is it, why do we want it, why is your implementation correct.
> > 
> > The only current in-kernel user of the infrastructure is KVM on x86. It
> > is useful for optimizations when MSR values can continue to be used
> > between the host and guest. Commit log for 18863bdd60f8 upstream has a
> > more complete explanation.
> >
> > I do not know the inner details of the KVM implementation on Power,
> > perhaps Paul/Gautham can comment on if a similar optimization will
> > benefit Power systems too?
> 
> "MSR" is x86-specific terminology and is pretty vague.  What specifically is 
> the functionality being optimized, in terms of things that actually exist on 
> PPC?
>
> In any case, that commit log doesn't explain what user-return-notifier is or 
> how it works, just that it's being used.

User return notifier is a simple mechanism to fire a one-shot custom handler in
the context of the thread its registered against, just before it returns to
userspace. It works by setting a TIF flag to indicate that a handler needs to
run, and on the userspace return path (do_notify_resume()), this flag is checked
to call any registered handlers.

> > We could certainly make this a generic config option.. but I am yet to
> > see a real usecase outside of the KVM thingy. We do use TIF_UPROBE for
> > something very similar, though that needs to fire much before the
> > outstanding signals get handled. The user-return notifier is the last
> > thing executed before return to userspace -- the two cannot be merged.
> > 
> > Ananth
> > 
> > PS: I am not sure having an 'ok' against the Documentation/features/ for
> > powerpc is a valid enough argument for this :-)
> 
> So how did you test this?  What platforms did you test it on?  What hardware 
> support does it need?

Simple kernel module below... Its fairly evident from the patch that the
feature is a software only construct and no specific hardware support is
needed. I've tested this on a P7.

Ananth

---
/* uret test module */

#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/user-return-notifier.h>

struct user_return_notifier urn;
static int count;

static void test_on_user_return(struct user_return_notifier *urn)
{
	count++;
}


static int init_urn(void)
{
	urn.on_user_return = test_on_user_return;
	user_return_notifier_register(&urn);
	printk("urn registered\n");
	return 0;
}
	
static void cleanup_urn(void)
{
	user_return_notifier_unregister(&urn);
	printk("urn unregistered; count = %d\n", count);
}

module_init(init_urn);
module_exit(cleanup_urn);
MODULE_LICENSE("GPL");

Re: powerpc: Add user-return-notifier support

[Scott Wood]

On Wed, 2015-09-02 at 08:07 +0530, Ananth N Mavinakayanahalli wrote:
> On Tue, Sep 01, 2015 at 07:03:12PM -0500, Scott Wood wrote:
> > On Tue, 2015-09-01 at 12:11 +0530, Ananth N Mavinakayanahalli wrote:
> > > On Mon, Aug 31, 2015 at 08:35:17PM +1000, Michael Ellerman wrote:
> > > > On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:
> > > > > Add user return notifier support for powerpc. Similar to x86, this 
> > > > > feature
> > > > > keys off of the KVM Kconfig.
> > > > 
> > > > Please flesh this out.
> > > > 
> > > > What is it, why do we want it, why is your implementation correct.
> > > 
> > > The only current in-kernel user of the infrastructure is KVM on x86. It
> > > is useful for optimizations when MSR values can continue to be used
> > > between the host and guest. Commit log for 18863bdd60f8 upstream has a
> > > more complete explanation.
> > > 
> > > I do not know the inner details of the KVM implementation on Power,
> > > perhaps Paul/Gautham can comment on if a similar optimization will
> > > benefit Power systems too?
> > 
> > "MSR" is x86-specific terminology and is pretty vague.  What specifically 
> > is 
> > the functionality being optimized, in terms of things that actually exist 
> > on 
> > PPC?
> > 
> > In any case, that commit log doesn't explain what user-return-notifier is 
> > or 
> > how it works, just that it's being used.
> 
> User return notifier is a simple mechanism to fire a one-shot custom 
> handler in
> the context of the thread its registered against, just before it returns to
> userspace. It works by setting a TIF flag to indicate that a handler needs 
> to
> run, and on the userspace return path (do_notify_resume()), this flag is 
> checked
> to call any registered handlers.

Thanks.

> > > We could certainly make this a generic config option.. but I am yet to
> > > see a real usecase outside of the KVM thingy. We do use TIF_UPROBE for
> > > something very similar, though that needs to fire much before the
> > > outstanding signals get handled. The user-return notifier is the last
> > > thing executed before return to userspace -- the two cannot be merged.
> > > 
> > > Ananth
> > > 
> > > PS: I am not sure having an 'ok' against the Documentation/features/ for
> > > powerpc is a valid enough argument for this :-)
> > 
> > So how did you test this?  What platforms did you test it on?  What 
> > hardware 
> > support does it need?
> 
> Simple kernel module below... Its fairly evident from the patch that the
> feature is a software only construct and no specific hardware support is
> needed.

It's evident now that I'm aware of what the purpose is... It wasn't 100% 

clear before, at least from a quick glance, given that the explanation was 

"look at this hardware-specific KVM patch", whether there was an assumption 
of something else going on -- and even things that are implemented in 
software often work differently on book3s versus book3e, ppc32 versus ppc64, 
etc.

Why is this selected by KVM on PPC if KVM on PPC doesn't use it?  What is the 
user you're trying to enable?

Where is the "profiler" that Documentation/features/debug/user-ret-
profiler/arch-support.txt hints at?

-Scott

Re: powerpc: Add user-return-notifier support

[Ananth N Mavinakayanahalli]

On Tue, Sep 01, 2015 at 10:29:12PM -0500, Scott Wood wrote:
> On Wed, 2015-09-02 at 08:07 +0530, Ananth N Mavinakayanahalli wrote:
> > On Tue, Sep 01, 2015 at 07:03:12PM -0500, Scott Wood wrote:
> > > On Tue, 2015-09-01 at 12:11 +0530, Ananth N Mavinakayanahalli wrote:
> > > > On Mon, Aug 31, 2015 at 08:35:17PM +1000, Michael Ellerman wrote:
> > > > > On Tue, 2015-25-08 at 05:41:10 UTC, Ananth N Mavinakayanahalli wrote:

...

> > > > We could certainly make this a generic config option.. but I am yet to
> > > > see a real usecase outside of the KVM thingy. We do use TIF_UPROBE for
> > > > something very similar, though that needs to fire much before the
> > > > outstanding signals get handled. The user-return notifier is the last
> > > > thing executed before return to userspace -- the two cannot be merged.
> > > > 
> > > > Ananth
> > > > 
> > > > PS: I am not sure having an 'ok' against the Documentation/features/ for
> > > > powerpc is a valid enough argument for this :-)
> > > 
> > > So how did you test this?  What platforms did you test it on?  What 
> > > hardware 
> > > support does it need?
> > 
> > Simple kernel module below... Its fairly evident from the patch that the
> > feature is a software only construct and no specific hardware support is
> > needed.
> 
> It's evident now that I'm aware of what the purpose is... It wasn't 100% 
> 
> clear before, at least from a quick glance, given that the explanation was 
> 
> "look at this hardware-specific KVM patch", whether there was an assumption 
> of something else going on -- and even things that are implemented in 
> software often work differently on book3s versus book3e, ppc32 versus ppc64, 
> etc.

I meant the infrastructure is agnostic of the underlying Power platform.
Sorry if I wasn't clear.

> Why is this selected by KVM on PPC if KVM on PPC doesn't use it?  What is the 
> user you're trying to enable?

I copied Paul and Gautham to get their thoughts on whether it is
something they could use on Power. At this time, apart from enabling the
feature, I do not have a specific usecase for it. We can park this patch
pending a real user on powerpc.

> Where is the "profiler" that Documentation/features/debug/user-ret-
> profiler/arch-support.txt hints at?

I guess profiler is a misnomer. Not sure why it was named that and not a
notifier.

Ananth

Re: powerpc: Add user-return-notifier support

[Michael Ellerman]

On Wed, 2015-09-02 at 10:39 +0530, Ananth N Mavinakayanahalli wrote:
> On Tue, Sep 01, 2015 at 10:29:12PM -0500, Scott Wood wrote:
> 
> > Why is this selected by KVM on PPC if KVM on PPC doesn't use it?  What is the 
> > user you're trying to enable?
> 
> I copied Paul and Gautham to get their thoughts on whether it is
> something they could use on Power. At this time, apart from enabling the
> feature, I do not have a specific usecase for it. We can park this patch
> pending a real user on powerpc.

OK, thanks for hashing that out guys. I think parking it is the best solution
for now, we could merge it and disable it somehow, but that would be ugly and
prone to breaking.

Do we want to do something like this?

cheers

diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
index d22a1095e661..3e2cf6161cf4 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
     | ok |  # feature supported by the architecture
     |TODO|  # feature not yet supported by the architecture
     | .. |  # feature cannot be supported by the hardware
+    |n/a |  # feature is not needed/wanted/meaningful for the architecture
 
diff --git a/Documentation/features/debug/user-ret-profiler/arch-support.txt b/Documentation/features/debug/user-ret-profiler/arch-support.txt
index 44cc1ff3f603..669813180dc5 100644
--- a/Documentation/features/debug/user-ret-profiler/arch-support.txt
+++ b/Documentation/features/debug/user-ret-profiler/arch-support.txt
@@ -27,7 +27,7 @@
     |       nios2: | TODO |
     |    openrisc: | TODO |
     |      parisc: | TODO |
-    |     powerpc: | TODO |
+    |     powerpc: | n/a  |    # http://lkml.kernel.org/r/20150825054110.GD3815@in.ibm.com
     |        s390: | TODO |
     |       score: | TODO |
     |          sh: | TODO |

Re: powerpc: Add user-return-notifier support

[Ananth N Mavinakayanahalli]

On Tue, Sep 08, 2015 at 07:24:39PM +1000, Michael Ellerman wrote:
> On Wed, 2015-09-02 at 10:39 +0530, Ananth N Mavinakayanahalli wrote:
> > On Tue, Sep 01, 2015 at 10:29:12PM -0500, Scott Wood wrote:
> > 
> > > Why is this selected by KVM on PPC if KVM on PPC doesn't use it?  What is the 
> > > user you're trying to enable?
> > 
> > I copied Paul and Gautham to get their thoughts on whether it is
> > something they could use on Power. At this time, apart from enabling the
> > feature, I do not have a specific usecase for it. We can park this patch
> > pending a real user on powerpc.
> 
> OK, thanks for hashing that out guys. I think parking it is the best solution
> for now, we could merge it and disable it somehow, but that would be ugly and
> prone to breaking.
> 
> Do we want to do something like this?

Yes. This looks fine -- folks who want it down the line know where to look.

Thanks,
Ananth

> 
> cheers
> 
> diff --git a/Documentation/features/arch-support.txt b/Documentation/features/arch-support.txt
> index d22a1095e661..3e2cf6161cf4 100644
> --- a/Documentation/features/arch-support.txt
> +++ b/Documentation/features/arch-support.txt
> @@ -8,4 +8,5 @@ The meaning of entries in the tables is:
>      | ok |  # feature supported by the architecture
>      |TODO|  # feature not yet supported by the architecture
>      | .. |  # feature cannot be supported by the hardware
> +    |n/a |  # feature is not needed/wanted/meaningful for the architecture
> 
> diff --git a/Documentation/features/debug/user-ret-profiler/arch-support.txt b/Documentation/features/debug/user-ret-profiler/arch-support.txt
> index 44cc1ff3f603..669813180dc5 100644
> --- a/Documentation/features/debug/user-ret-profiler/arch-support.txt
> +++ b/Documentation/features/debug/user-ret-profiler/arch-support.txt
> @@ -27,7 +27,7 @@
>      |       nios2: | TODO |
>      |    openrisc: | TODO |
>      |      parisc: | TODO |
> -    |     powerpc: | TODO |
> +    |     powerpc: | n/a  |    # http://lkml.kernel.org/r/20150825054110.GD3815@in.ibm.com
>      |        s390: | TODO |
>      |       score: | TODO |
>      |          sh: | TODO |
> 
>