[PATCH] powerpc: Add support for userspace P9 copy paste

[PATCH] powerpc: Add support for userspace P9 copy paste

[Chris Smart]

The copy paste facility introduced in POWER9 provides an optimised
mechanism for a userspace application to copy a cacheline. This is
provided by a pair of instructions, copy and paste, while a third,
cp_abort (copy paste abort), provides a clean up of the state in case of
a failure.

The copy instruction will read a 128 byte cacheline and store it in an
internal buffer. The subsequent paste instruction will store this
internal buffer to memory and set a CR field if the paste succeeds.

Since the state of the copy paste buffer is internal (and not
architecturally visible), in the unlikely event of a context switch, the
state cannot be stored and the paste should therefore fail.

The cp_abort instruction exists to fail and clean up any such
interrupted copy paste sequence and is to be called by the kernel as
part of the context switch. Doing so prevents data from a preceding copy
in one process leaking into the paste of another.

This code enables use of the cp_abort instruction if a supported
processor is detected.

NOTE: this is for userspace only, not in kernel, and does not deal
with KVM guests.

Patch created with much assistance from Michael Neuling
<mikey@neuling.org>

Signed-off-by: Chris Smart <chris@distroguy.com>
---

Note: A follow-up patch is expected soon with a working self-test.

 arch/powerpc/include/asm/ppc-opcode.h | 2 ++
 arch/powerpc/kernel/entry_64.S        | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 7ab04fc59e24..1d035c1cc889 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -131,6 +131,7 @@
 /* sorted alphabetically */
 #define PPC_INST_BHRBE			0x7c00025c
 #define PPC_INST_CLRBHRB		0x7c00035c
+#define PPC_INST_CP_ABORT		0x7c00068c
 #define PPC_INST_DCBA			0x7c0005ec
 #define PPC_INST_DCBA_MASK		0xfc0007fe
 #define PPC_INST_DCBAL			0x7c2005ec
@@ -285,6 +286,7 @@
 #endif
 
 /* Deal with instructions that older assemblers aren't aware of */
+#define	PPC_CP_ABORT		stringify_in_c(.long PPC_INST_CP_ABORT)
 #define	PPC_DCBAL(a, b)		stringify_in_c(.long PPC_INST_DCBAL | \
 					__PPC_RA(a) | __PPC_RB(b))
 #define	PPC_DCBZL(a, b)		stringify_in_c(.long PPC_INST_DCBZL | \
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 8b9d68676d2b..ab1457c3f1d1 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -36,6 +36,7 @@
 #include <asm/hw_irq.h>
 #include <asm/context_tracking.h>
 #include <asm/tm.h>
+#include <asm/ppc-opcode.h>
 
 /*
  * System calls.
@@ -508,6 +509,14 @@ BEGIN_FTR_SECTION
 	ldarx	r6,0,r1
 END_FTR_SECTION_IFSET(CPU_FTR_STCX_CHECKS_ADDRESS)
 
+BEGIN_FTR_SECTION
+/*
+ * A cp_abort (copy paste abort) here ensures that when context switching, a
+ * copy from one process can't leak into the paste of another.
+ */
+        PPC_CP_ABORT
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
+
 #ifdef CONFIG_PPC_BOOK3S
 /* Cancel all explict user streams as they will have no use after context
  * switch and will stop the HW from creating streams itself
-- 
2.5.5


-- 
  _
 °v°
/(_)\
 ^ ^

Re: [PATCH] powerpc: Add support for userspace P9 copy paste

[Cyril Bur]

On Tue, 26 Apr 2016 10:28:50 +1000
Chris Smart <chris@distroguy.com> wrote:

> The copy paste facility introduced in POWER9 provides an optimised
> mechanism for a userspace application to copy a cacheline. This is
> provided by a pair of instructions, copy and paste, while a third,
> cp_abort (copy paste abort), provides a clean up of the state in case of
> a failure.
> 
> The copy instruction will read a 128 byte cacheline and store it in an
> internal buffer. The subsequent paste instruction will store this
> internal buffer to memory and set a CR field if the paste succeeds.
> 
> Since the state of the copy paste buffer is internal (and not
> architecturally visible), in the unlikely event of a context switch, the
> state cannot be stored and the paste should therefore fail.
> 
> The cp_abort instruction exists to fail and clean up any such
> interrupted copy paste sequence and is to be called by the kernel as
> part of the context switch. Doing so prevents data from a preceding copy
> in one process leaking into the paste of another.
> 
> This code enables use of the cp_abort instruction if a supported
> processor is detected.
> 
> NOTE: this is for userspace only, not in kernel, and does not deal
> with KVM guests.
> 
> Patch created with much assistance from Michael Neuling
> <mikey@neuling.org>
> 

Hi Chris,

Patch looks good. Looks like you've put 8 spaces (instead of a tab) on the
PPC_CP_ABORT line.

Apart from that,

Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

> Signed-off-by: Chris Smart <chris@distroguy.com>
> ---
> 
> Note: A follow-up patch is expected soon with a working self-test.
> 
>  arch/powerpc/include/asm/ppc-opcode.h | 2 ++
>  arch/powerpc/kernel/entry_64.S        | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 7ab04fc59e24..1d035c1cc889 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -131,6 +131,7 @@
>  /* sorted alphabetically */
>  #define PPC_INST_BHRBE			0x7c00025c
>  #define PPC_INST_CLRBHRB		0x7c00035c
> +#define PPC_INST_CP_ABORT		0x7c00068c
>  #define PPC_INST_DCBA			0x7c0005ec
>  #define PPC_INST_DCBA_MASK		0xfc0007fe
>  #define PPC_INST_DCBAL			0x7c2005ec
> @@ -285,6 +286,7 @@
>  #endif
>  
>  /* Deal with instructions that older assemblers aren't aware of */
> +#define	PPC_CP_ABORT		stringify_in_c(.long PPC_INST_CP_ABORT)
>  #define	PPC_DCBAL(a, b)		stringify_in_c(.long PPC_INST_DCBAL | \
>  					__PPC_RA(a) | __PPC_RB(b))
>  #define	PPC_DCBZL(a, b)		stringify_in_c(.long PPC_INST_DCBZL | \
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index 8b9d68676d2b..ab1457c3f1d1 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -36,6 +36,7 @@
>  #include <asm/hw_irq.h>
>  #include <asm/context_tracking.h>
>  #include <asm/tm.h>
> +#include <asm/ppc-opcode.h>
>  
>  /*
>   * System calls.
> @@ -508,6 +509,14 @@ BEGIN_FTR_SECTION
>  	ldarx	r6,0,r1
>  END_FTR_SECTION_IFSET(CPU_FTR_STCX_CHECKS_ADDRESS)
>  
> +BEGIN_FTR_SECTION
> +/*
> + * A cp_abort (copy paste abort) here ensures that when context switching, a
> + * copy from one process can't leak into the paste of another.
> + */
> +        PPC_CP_ABORT
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +
>  #ifdef CONFIG_PPC_BOOK3S
>  /* Cancel all explict user streams as they will have no use after context
>   * switch and will stop the HW from creating streams itself

Re: [PATCH] powerpc: Add support for userspace P9 copy paste

[Chris Smart]

On Tue, Apr 26, 2016 at 04:42:09PM +1000, Cyril Bur wrote:
>Hi Chris,
>
>Patch looks good. Looks like you've put 8 spaces (instead of a tab) on the
>PPC_CP_ABORT line.
>

Argh. I'm not sure how that happened, thanks.

>Apart from that,
>
>Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

Cheers,
-c

Re: [PATCH] powerpc: Add support for userspace P9 copy paste

[Balbir Singh]

> /*
>  * System calls.
> @@ -508,6 +509,14 @@ BEGIN_FTR_SECTION
>     ldarx    r6,0,r1
> END_FTR_SECTION_IFSET(CPU_FTR_STCX_CHECKS_ADDRESS)
> 
> +BEGIN_FTR_SECTION
> +/*
> + * A cp_abort (copy paste abort) here ensures that when context switching, a
> + * copy from one process can't leak into the paste of another.
> + */
> +        PPC_CP_ABORT

I think the alignment issue has been called out, but it is not clear from the changelog that
we do this during syscall_exit/syscalls.

And also, do we need to care about preemptions, etc by the scheduler?

> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +
> #ifdef CONFIG_PPC_BOOK3S
> /* Cancel all explict user streams as they will have no use after context
>  * switch and will stop the HW from creating streams itself

Re: [PATCH] powerpc: Add support for userspace P9 copy paste

[Michael Neuling]

On Tue, 2016-04-26 at 17:45 +1000, Balbir Singh wrote:
> >=C2=A0
> > /*
> > =C2=A0* System calls.
> > @@ -508,6 +509,14 @@ BEGIN_FTR_SECTION
> > =C2=A0=C2=A0=C2=A0=C2=A0ldarx=C2=A0=C2=A0=C2=A0=C2=A0r6,0,r1
> > END_FTR_SECTION_IFSET(CPU_FTR_STCX_CHECKS_ADDRESS)
> >=C2=A0
> > +BEGIN_FTR_SECTION
> > +/*
> > + * A cp_abort (copy paste abort) here ensures that when context
> > switching, a
> > + * copy from one process can't leak into the paste of another.
> > + */
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0PPC_CP_ABORT
> I think the alignment issue has been called out, but it is not clear from
> the changelog that
> we do this during syscall_exit/syscalls.

I don't think we need this during syscall entry exit until we use this
in the kernel.

> And also, do we need to care about preemptions, etc by the scheduler?

This handles the userspace preemption case.

kernel support is not handled here, so kernel preemption is TBD.

Mikey

>=C2=A0
> >=C2=A0
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> > +
> > #ifdef CONFIG_PPC_BOOK3S
> > /* Cancel all explict user streams as they will have no use after conte=
xt
> > =C2=A0* switch and will stop the HW from creating streams itself
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH] powerpc: Add support for userspace P9 copy paste

[David Laight]

RnJvbTogQ2hyaXMgU21hcnQNCj4gU2VudDogMjYgQXByaWwgMjAxNiAwMToyOQ0KPiBUaGUgY29w
eSBwYXN0ZSBmYWNpbGl0eSBpbnRyb2R1Y2VkIGluIFBPV0VSOSBwcm92aWRlcyBhbiBvcHRpbWlz
ZWQNCj4gbWVjaGFuaXNtIGZvciBhIHVzZXJzcGFjZSBhcHBsaWNhdGlvbiB0byBjb3B5IGEgY2Fj
aGVsaW5lLiBUaGlzIGlzDQo+IHByb3ZpZGVkIGJ5IGEgcGFpciBvZiBpbnN0cnVjdGlvbnMsIGNv
cHkgYW5kIHBhc3RlLCB3aGlsZSBhIHRoaXJkLA0KPiBjcF9hYm9ydCAoY29weSBwYXN0ZSBhYm9y
dCksIHByb3ZpZGVzIGEgY2xlYW4gdXAgb2YgdGhlIHN0YXRlIGluIGNhc2Ugb2YNCj4gYSBmYWls
dXJlLg0KPiANCj4gVGhlIGNvcHkgaW5zdHJ1Y3Rpb24gd2lsbCByZWFkIGEgMTI4IGJ5dGUgY2Fj
aGVsaW5lIGFuZCBzdG9yZSBpdCBpbiBhbg0KPiBpbnRlcm5hbCBidWZmZXIuIFRoZSBzdWJzZXF1
ZW50IHBhc3RlIGluc3RydWN0aW9uIHdpbGwgc3RvcmUgdGhpcw0KPiBpbnRlcm5hbCBidWZmZXIg
dG8gbWVtb3J5IGFuZCBzZXQgYSBDUiBmaWVsZCBpZiB0aGUgcGFzdGUgc3VjY2VlZHMuDQo+IA0K
PiBTaW5jZSB0aGUgc3RhdGUgb2YgdGhlIGNvcHkgcGFzdGUgYnVmZmVyIGlzIGludGVybmFsIChh
bmQgbm90DQo+IGFyY2hpdGVjdHVyYWxseSB2aXNpYmxlKSwgaW4gdGhlIHVubGlrZWx5IGV2ZW50
IG9mIGEgY29udGV4dCBzd2l0Y2gsIHRoZQ0KPiBzdGF0ZSBjYW5ub3QgYmUgc3RvcmVkIGFuZCB0
aGUgcGFzdGUgc2hvdWxkIHRoZXJlZm9yZSBmYWlsLg0KPiANCj4gVGhlIGNwX2Fib3J0IGluc3Ry
dWN0aW9uIGV4aXN0cyB0byBmYWlsIGFuZCBjbGVhbiB1cCBhbnkgc3VjaA0KPiBpbnRlcnJ1cHRl
ZCBjb3B5IHBhc3RlIHNlcXVlbmNlIGFuZCBpcyB0byBiZSBjYWxsZWQgYnkgdGhlIGtlcm5lbCBh
cw0KPiBwYXJ0IG9mIHRoZSBjb250ZXh0IHN3aXRjaC4gRG9pbmcgc28gcHJldmVudHMgZGF0YSBm
cm9tIGEgcHJlY2VkaW5nIGNvcHkNCj4gaW4gb25lIHByb2Nlc3MgbGVha2luZyBpbnRvIHRoZSBw
YXN0ZSBvZiBhbm90aGVyLg0KPiANCj4gVGhpcyBjb2RlIGVuYWJsZXMgdXNlIG9mIHRoZSBjcF9h
Ym9ydCBpbnN0cnVjdGlvbiBpZiBhIHN1cHBvcnRlZA0KPiBwcm9jZXNzb3IgaXMgZGV0ZWN0ZWQu
DQoNCkluIHRoYXQgY2FzZSB3aGF0IGFjdHVhbGx5IGNvbXBsZXRlcyB0aGUgY29weT8NCkkgdGhp
bmsgeW91J2QgbmVlZCB0byBiZSBpbnNpZGUgYSAncmVzdGFydGFibGUgYXRvbWljIHNlcXVlbmNl
Jw0KaW4gd2hpY2ggY2FzZSB0aGUgY3BfYWJvcnQgbmVlZCBvbmx5IGJlIGRvbmUgd2hlbiB0aGUv
YSBSQVMNCmJsb2NrIGlzIGRldGVjdGVkLg0KDQpPciBoYXZlIEkgbWlzc2VkIHNvbWV0aGluZz8/
DQoNCglEYXZpZA0KDQo=

Re: [PATCH] powerpc: Add support for userspace P9 copy paste

[Chris Smart]

On Wed, Apr 27, 2016 at 03:25:59PM +0000, David Laight wrote:
>From: Chris Smart
>> Sent: 26 April 2016 01:29
>> The copy paste facility introduced in POWER9 provides an optimised
>> mechanism for a userspace application to copy a cacheline. This is
>> provided by a pair of instructions, copy and paste, while a third,
>> cp_abort (copy paste abort), provides a clean up of the state in case of
>> a failure.
>>
>> The copy instruction will read a 128 byte cacheline and store it in an
>> internal buffer. The subsequent paste instruction will store this
>> internal buffer to memory and set a CR field if the paste succeeds.
>>
>> Since the state of the copy paste buffer is internal (and not
>> architecturally visible), in the unlikely event of a context switch, the
>> state cannot be stored and the paste should therefore fail.
>>
>> The cp_abort instruction exists to fail and clean up any such
>> interrupted copy paste sequence and is to be called by the kernel as
>> part of the context switch. Doing so prevents data from a preceding copy
>> in one process leaking into the paste of another.
>>
>> This code enables use of the cp_abort instruction if a supported
>> processor is detected.
>
>In that case what actually completes the copy?
>I think you'd need to be inside a 'restartable atomic sequence'
>in which case the cp_abort need only be done when the/a RAS
>block is detected.
>
>Or have I missed something??

It's up to the userspace process that's doing the copy paste to check
the CR field after the paste to see whether it has succeeded. If it has
not succeeded, then the process can choose to re-do the copy and paste.

This is very similar to a lwarx stwcx where the user checks the CR after
the stwcx to verify whether it has succeeded.

Hope that makes sense.

-c

Re: powerpc: Add support for userspace P9 copy paste

[Michael Ellerman]

On Tue, 2016-26-04 at 00:28:50 UTC, Chris Smart wrote:
> The copy paste facility introduced in POWER9 provides an optimised
> mechanism for a userspace application to copy a cacheline. This is
> provided by a pair of instructions, copy and paste, while a third,
> cp_abort (copy paste abort), provides a clean up of the state in case of
> a failure.
...
> Signed-off-by: Chris Smart <chris@distroguy.com>
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8a649045e75a4b9091ea9d041f

cheers

RE: [PATCH] powerpc: Add support for userspace P9 copy paste

[David Laight]

RnJvbTogQ2hyaXMgU21hcnQNCj4gU2VudDogMjggQXByaWwgMjAxNiAwMDo1Mg0KLi4uDQo+ID5J
biB0aGF0IGNhc2Ugd2hhdCBhY3R1YWxseSBjb21wbGV0ZXMgdGhlIGNvcHk/DQo+ID5JIHRoaW5r
IHlvdSdkIG5lZWQgdG8gYmUgaW5zaWRlIGEgJ3Jlc3RhcnRhYmxlIGF0b21pYyBzZXF1ZW5jZScN
Cj4gPmluIHdoaWNoIGNhc2UgdGhlIGNwX2Fib3J0IG5lZWQgb25seSBiZSBkb25lIHdoZW4gdGhl
L2EgUkFTDQo+ID5ibG9jayBpcyBkZXRlY3RlZC4NCj4gPg0KPiA+T3IgaGF2ZSBJIG1pc3NlZCBz
b21ldGhpbmc/Pw0KPiANCj4gSXQncyB1cCB0byB0aGUgdXNlcnNwYWNlIHByb2Nlc3MgdGhhdCdz
IGRvaW5nIHRoZSBjb3B5IHBhc3RlIHRvIGNoZWNrDQo+IHRoZSBDUiBmaWVsZCBhZnRlciB0aGUg
cGFzdGUgdG8gc2VlIHdoZXRoZXIgaXQgaGFzIHN1Y2NlZWRlZC4gSWYgaXQgaGFzDQo+IG5vdCBz
dWNjZWVkZWQsIHRoZW4gdGhlIHByb2Nlc3MgY2FuIGNob29zZSB0byByZS1kbyB0aGUgY29weSBh
bmQgcGFzdGUuDQo+DQo+IEhvcGUgdGhhdCBtYWtlcyBzZW5zZS4NCg0KWWVzLCB0aGFua3MuDQoN
CglEYXZpZA0KDQo=