Patchwork Emulate sync instruction variants

login
register
mail settings
Submitter James Yang
Date July 3, 2013, 9:26 p.m.
Message ID <1372886807-12109-1-git-send-email-James.Yang@freescale.com>
Download mbox | patch
Permalink /patch/256747/
State Accepted, archived
Delegated to: Scott Wood
Headers show

Comments

James Yang - July 3, 2013, 9:26 p.m.
From: James Yang <James.Yang@freescale.com>

Reserved fields of the sync instruction have been used for other
instructions (e.g. lwsync).  On processors that do not support variants
of the sync instruction, emulate it by executing a sync to subsume the
effect of the intended instruction.

Signed-off-by: James Yang <James.Yang@freescale.com>
---
 arch/powerpc/include/asm/ppc-opcode.h |    2 ++
 arch/powerpc/kernel/traps.c           |    7 +++++++
 2 files changed, 9 insertions(+), 0 deletions(-)
David Laight - July 4, 2013, 8:31 a.m.
> Reserved fields of the sync instruction have been used for other
> instructions (e.g. lwsync).  On processors that do not support variants
> of the sync instruction, emulate it by executing a sync to subsume the
> effect of the intended instruction.
...
> +	/* Emulate sync instruction variants */
> +	if ((instword & PPC_INST_SYNC_MASK) == PPC_INST_SYNC) {
> +		PPC_WARN_EMULATED(sync, regs);
> +		asm volatile ("sync");
> +		return 0;
> +	}

Do you need to execute 'sync' here?
It is worth checking whether the trap entry/exit doesn't
do an implicit one for you.

	David
Benjamin Herrenschmidt - July 4, 2013, 9:29 a.m.
On Thu, 2013-07-04 at 09:31 +0100, David Laight wrote:
> Do you need to execute 'sync' here?
> It is worth checking whether the trap entry/exit doesn't
> do an implicit one for you.

Not really. It does an implicit isync (more than one even) but
not a sync.

Cheers,
Ben.
James Yang - July 4, 2013, 9:56 p.m.
On Thu, 4 Jul 2013, Benjamin Herrenschmidt wrote:

> On Thu, 2013-07-04 at 09:31 +0100, David Laight wrote:
> > Do you need to execute 'sync' here?
> > It is worth checking whether the trap entry/exit doesn't
> > do an implicit one for you.
> 
> Not really. It does an implicit isync (more than one even) but not a 
> sync.

The execution of a sync is required because the original sync variant 
instruction is not actually executed, and, per Ben, no other explicit 
sync exists in the exception handler.  If the ISA extends the sync 
instruction using its reserved bits, the intent would also be that 
executing a heavyweight sync is suitably compatible, otherwise they 
should have modified some other synchronization instruction.  This 
patch ensures at least one sync instruction is executed for each sync 
variant instruction executed that triggers a program exception.

Patch

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index eccfc16..0142eb2 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -96,6 +96,8 @@ 
 #define PPC_INST_LSWX			0x7c00042a
 #define PPC_INST_LWARX			0x7c000028
 #define PPC_INST_LWSYNC			0x7c2004ac
+#define PPC_INST_SYNC			0x7c0004ac
+#define PPC_INST_SYNC_MASK		0xfc0007fe
 #define PPC_INST_LXVD2X			0x7c000698
 #define PPC_INST_MCRXR			0x7c000400
 #define PPC_INST_MCRXR_MASK		0xfc0007fe
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a98adc7..c3ceaa2 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1018,6 +1018,13 @@  static int emulate_instruction(struct pt_regs *regs)
 		return emulate_isel(regs, instword);
 	}
 
+	/* Emulate sync instruction variants */
+	if ((instword & PPC_INST_SYNC_MASK) == PPC_INST_SYNC) {
+		PPC_WARN_EMULATED(sync, regs);
+		asm volatile ("sync");
+		return 0;
+	}
+
 #ifdef CONFIG_PPC64
 	/* Emulate the mfspr rD, DSCR. */
 	if ((((instword & PPC_INST_MFSPR_DSCR_USER_MASK) ==