[Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Chris Smart]

Calling ISA 3.0 instructions copy, copy_first, paste and paste_last
generates an alignment fault when copying or pasting unaligned
data (128 byte). We catch this and send SIGBUS to the userspace
process that caused it.

We do not emulate these because paste may contain additional metadata
when pasting to a co-processor and paste_last is the synchronisation
point for preceding copy/paste sequences.

Thanks to Michael Neuling <mikey@neuling.org> for his help.

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

Changes since v1:
 - define and use instruction for mask test

 arch/powerpc/include/asm/ppc-opcode.h |  4 ++++
 arch/powerpc/kernel/align.c           | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 1d035c1cc889..7921d3e5704d 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -131,6 +131,8 @@
 /* sorted alphabetically */
 #define PPC_INST_BHRBE			0x7c00025c
 #define PPC_INST_CLRBHRB		0x7c00035c
+#define PPC_INST_COPY			0x7c00060c
+#define PPC_INST_COPY_FIRST		0x7c20060c
 #define PPC_INST_CP_ABORT		0x7c00068c
 #define PPC_INST_DCBA			0x7c0005ec
 #define PPC_INST_DCBA_MASK		0xfc0007fe
@@ -159,6 +161,8 @@
 #define PPC_INST_MSGSNDP		0x7c00011c
 #define PPC_INST_MTTMR			0x7c0003dc
 #define PPC_INST_NOP			0x60000000
+#define PPC_INST_PASTE			0x7c00070c
+#define PPC_INST_PASTE_LAST		0x7c20070d
 #define PPC_INST_POPCNTB		0x7c0000f4
 #define PPC_INST_POPCNTB_MASK		0xfc0007fe
 #define PPC_INST_POPCNTD		0x7c0003f4
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 8e7cb8e2b21a..6e0a1f8495f2 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -875,6 +875,20 @@ int fix_alignment(struct pt_regs *regs)
 		return emulate_vsx(addr, reg, areg, regs, flags, nb, elsize);
 	}
 #endif
+
+	/*
+	 * ISA 3.0 (such as P9) copy, copy_first, paste and paste_last alignment
+	 * check.
+	 *
+	 * Send a SIGBUS to the process that caused the fault.
+	 *
+	 * We do not emulate these because paste may contain additional metadata
+	 * when pasting to a co-processor. Furthermore, paste_last is the
+	 * synchronisation point for preceding copy/paste sequences.
+	 */
+	if ((instruction & 0xfc0006fe) == PPC_INST_COPY)
+		return -EIO;
+
 	/* A size of 0 indicates an instruction we don't support, with
 	 * the exception of DCBZ which is handled as a special case here
 	 */
-- 
2.7.4

Re: [Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Segher Boessenkool]

On Fri, Jun 17, 2016 at 09:33:45AM +1000, Chris Smart wrote:
> +#define PPC_INST_COPY			0x7c00060c
> +#define PPC_INST_COPY_FIRST		0x7c20060c

> +#define PPC_INST_PASTE			0x7c00070c
> +#define PPC_INST_PASTE_LAST		0x7c20070d

That's not quite right I think.

copy is       7c00060c mask fc2007fe (or ffe007fe)
copy_first is 7c20060c mask fc2007fe
paste is      7c00070c mask fc2007fe
paste_last is 7c20070c mask fc2007fe

(this includes record form for paste; the low bit).


Segher

Re: [Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Chris Smart]

On Thu, Jun 16, 2016 at 11:04:12PM -0500, Segher Boessenkool wrote:
>On Fri, Jun 17, 2016 at 09:33:45AM +1000, Chris Smart wrote:
>> +#define PPC_INST_COPY			0x7c00060c
>> +#define PPC_INST_COPY_FIRST		0x7c20060c
>
>> +#define PPC_INST_PASTE			0x7c00070c
>> +#define PPC_INST_PASTE_LAST		0x7c20070d
>
>That's not quite right I think.
>

Hi Segher,

Thanks for checking that for me, it's good to make sure it's correct.

Just to be sure, I've gone back and compared them all with the ISA. I
think that the only one that differs is the paste_last. Am I missing
something?

>copy is       7c00060c mask fc2007fe (or ffe007fe)

COPY = copy RA,RB,L (L=0)

  31   //// L  RA    RB      774     /
011111 0000 0 00000 00000 1100000110 0 = 0x7c00060c (instruction)
111111 0000 1 00000 00000 1111111111 0 = 0xfc2007fe (specific mask)

>copy_first is 7c20060c mask fc2007fe

COPY_FIRST = copy RA,RB,L (L=1)
If L=1, the instruction identifies the beginning of a move group.

  31   //// L   RA   RB      774     /
011111 0000 1 00000 00000 1100000110 0 = 0x7c20060c (instruction)
111111 0000 1 00000 00000 1111111111 0 = 0xfc2007fe (specific mask)

>paste is      7c00070c mask fc2007fe

PASTE = paste RA,RB,L (L=0 Rc=0)

  31   //// L  RA    RB      902     Rc
011111 0000 0 00000 00000 1110000110 0 = 0x7c00070c (instruction)
111111 0000 1 00000 00000 1111111111 1 = 0xfc2007ff (specific mask)

>paste_last is 7c20070c mask fc2007fe
>

PASTE_LAST = paste. RA,RB,L (L=1 Rc=1)
If L=1, the instruction identifies the end of a move group.
If L≠Rc, the instruction form is invalid.

  31   //// L  RA    RB      902     Rc
011111 0000 1 00000 00000 1110000110 1 = 0x7c20070d (instruction)
111111 0000 1 00000 00000 1111111111 1 = 0xfc2007ff (specific mask)

>(this includes record form for paste; the low bit).
>

To make the test simple I use a combined copy, copy_first, paste and
paste_last mask to compare just against copy. So that excluded:
 - L
 - bit 24 of 32
 - Rc
111111 0000 0 00000 00000 1101111111 0 = 0xfc0006fe

Would it be better and more clear to check each instruction with its
mask? Something like:

#define PPC_INST_COPY_MASK 0xfc2007fe
#define PPC_INST_PASTE_MASK 0xfc2007ff

if (cpu_has_feature(CPU_FTR_ARCH_300)) {
	unsigned int masked_instruction = instruction & PPC_INST_COPY_MASK;

	if (masked_instruction == PPC_INST_COPY || \
			masked_instruction == PPC_INST_COPY_FIRST)
		return -EIO;

	masked_instruction = instruction & PPC_INST_PASTE_MASK;

	if (masked_instruction == PPC_INST_PASTE || \
			masked_instruction == PPC_INST_PASTE_LAST)
		return -EIO;
}

Thanks!
-c

Re: [Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Balbir Singh]

On 17/06/16 09:33, Chris Smart wrote:
> Calling ISA 3.0 instructions copy, copy_first, paste and paste_last
> generates an alignment fault when copying or pasting unaligned
> data (128 byte). We catch this and send SIGBUS to the userspace
> process that caused it.
> 
> We do not emulate these because paste may contain additional metadata
> when pasting to a co-processor and paste_last is the synchronisation
> point for preceding copy/paste sequences.
> 
> Thanks to Michael Neuling <mikey@neuling.org> for his help.
> 
> Signed-off-by: Chris Smart <chris@distroguy.com>
> ---
> 
> Changes since v1:
> - define and use instruction for mask test
> 
> arch/powerpc/include/asm/ppc-opcode.h |  4 ++++
> arch/powerpc/kernel/align.c           | 14 ++++++++++++++
> 2 files changed, 18 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 1d035c1cc889..7921d3e5704d 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -131,6 +131,8 @@
> /* sorted alphabetically */
> #define PPC_INST_BHRBE            0x7c00025c
> #define PPC_INST_CLRBHRB        0x7c00035c
> +#define PPC_INST_COPY            0x7c00060c
> +#define PPC_INST_COPY_FIRST        0x7c20060c
> #define PPC_INST_CP_ABORT        0x7c00068c
> #define PPC_INST_DCBA            0x7c0005ec
> #define PPC_INST_DCBA_MASK        0xfc0007fe
> @@ -159,6 +161,8 @@
> #define PPC_INST_MSGSNDP        0x7c00011c
> #define PPC_INST_MTTMR            0x7c0003dc
> #define PPC_INST_NOP            0x60000000
> +#define PPC_INST_PASTE            0x7c00070c
> +#define PPC_INST_PASTE_LAST        0x7c20070d
> #define PPC_INST_POPCNTB        0x7c0000f4
> #define PPC_INST_POPCNTB_MASK        0xfc0007fe
> #define PPC_INST_POPCNTD        0x7c0003f4
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index 8e7cb8e2b21a..6e0a1f8495f2 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -875,6 +875,20 @@ int fix_alignment(struct pt_regs *regs)
>         return emulate_vsx(addr, reg, areg, regs, flags, nb, elsize);
>     }
> #endif
> +
> +    /*
> +     * ISA 3.0 (such as P9) copy, copy_first, paste and paste_last alignment
> +     * check.
> +     *
> +     * Send a SIGBUS to the process that caused the fault.
> +     *
> +     * We do not emulate these because paste may contain additional metadata
> +     * when pasting to a co-processor. Furthermore, paste_last is the
> +     * synchronisation point for preceding copy/paste sequences.
> +     */
> +    if ((instruction & 0xfc0006fe) == PPC_INST_COPY)
> +        return -EIO;

Should this all be under cpu_has_feature(CPU_FTR_ARCH_300)?

Balbir Singh.

Re: [Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Chris Smart]

On Fri, Jun 17, 2016 at 05:20:05PM +1000, Balbir Singh wrote:
>
>
>On 17/06/16 09:33, Chris Smart wrote:

[snip]

>> +
>> +    /*
>> +     * ISA 3.0 (such as P9) copy, copy_first, paste and paste_last alignment
>> +     * check.
>> +     *
>> +     * Send a SIGBUS to the process that caused the fault.
>> +     *
>> +     * We do not emulate these because paste may contain additional metadata
>> +     * when pasting to a co-processor. Furthermore, paste_last is the
>> +     * synchronisation point for preceding copy/paste sequences.
>> +     */
>> +    if ((instruction & 0xfc0006fe) == PPC_INST_COPY)
>> +        return -EIO;
>
>Should this all be under cpu_has_feature(CPU_FTR_ARCH_300)?
>

I'm not sure we should or not. That instruction only exists on those
machines so is it worth adding an additional check when the next check
will fail anyway?

I guess it would reduce non ISA 3.0 machines to just a single check rather
than two, but increases ISO 3.0 machines to three.

I defer to the wisdom of others.

-c

Re: [Patch v2 1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Balbir Singh]

On 20/06/16 09:48, Chris Smart wrote:
> On Fri, Jun 17, 2016 at 05:20:05PM +1000, Balbir Singh wrote:
>>
>>
>> On 17/06/16 09:33, Chris Smart wrote:
> 
> [snip]
> 
>>> +
>>> +    /*
>>> +     * ISA 3.0 (such as P9) copy, copy_first, paste and paste_last alignment
>>> +     * check.
>>> +     *
>>> +     * Send a SIGBUS to the process that caused the fault.
>>> +     *
>>> +     * We do not emulate these because paste may contain additional metadata
>>> +     * when pasting to a co-processor. Furthermore, paste_last is the
>>> +     * synchronisation point for preceding copy/paste sequences.
>>> +     */
>>> +    if ((instruction & 0xfc0006fe) == PPC_INST_COPY)
>>> +        return -EIO;
>>
>> Should this all be under cpu_has_feature(CPU_FTR_ARCH_300)?
>>
> 
> I'm not sure we should or not. That instruction only exists on those
> machines so is it worth adding an additional check when the next check
> will fail anyway?
> 
> I guess it would reduce non ISA 3.0 machines to just a single check rather
> than two, but increases ISO 3.0 machines to three.
> 

Ideally we can do features and fixups, but that today works mostly for the
assembly code we have. In this case we might get away by doing the if and "&",
I would recommend adding a branch hint

if (unlikely(...))

Balbir Singh

Re: [v2,1/2] powerpc: Send SIGBUS on unaligned copy and paste

[Michael Ellerman]

On Thu, 2016-16-06 at 23:33:45 UTC, Chris Smart wrote:
> Calling ISA 3.0 instructions copy, copy_first, paste and paste_last
> generates an alignment fault when copying or pasting unaligned
> data (128 byte). We catch this and send SIGBUS to the userspace
> process that caused it.
> 
> We do not emulate these because paste may contain additional metadata
> when pasting to a co-processor and paste_last is the synchronisation
> point for preceding copy/paste sequences.
> 
> Thanks to Michael Neuling <mikey@neuling.org> for his help.
> 
> Signed-off-by: Chris Smart <chris@distroguy.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ae26b36f8098c793a754549662

cheers