* [kvm-unit-tests PATCH 0/5] Check emulation
@ 2016-03-16 15:12 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:12 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
This series adds exception handler to be able to test instruction
emulation. We test then invalid instruction, lswx instruction, and
check that lswx instruction triggers an alignment interrupt in
little-endian mode. During the development of this series, I've seen
that processor is not running in 64bit mode, so I've added a test
to check that is the case now.
This has been tested with KVM PR, HV, TCG in big and little-endian mode.
KVM-PR needs a fix to be able to manage correctly invalid instruction.
http://patchwork.ozlabs.org/patch/597855/
QEMU needs a fix to initialize correctly the 64bit mode:
http://patchwork.ozlabs.org/patch/598198/
With TCG, some lswx tests fail because it needs some fixes... I'm working
on this.
Laurent Vivier (5):
powerpc: add exception handler
powerpc: add test to check invalid instruction trap
powerpc: check 64bit mode
powerpc: check lswx
powerpc: Check lswx in little-endian mode.
lib/powerpc/asm/hcall.h | 1 +
lib/powerpc/asm/ppc_asm.h | 5 +
lib/powerpc/asm/processor.h | 11 +++
lib/powerpc/processor.c | 38 ++++++++
lib/powerpc/setup.c | 19 ++++
lib/ppc64/asm-offsets.c | 42 ++++++++
lib/ppc64/asm/processor.h | 1 +
lib/ppc64/asm/ptrace.h | 24 +++++
powerpc/Makefile.common | 6 +-
powerpc/cstart64.S | 136 ++++++++++++++++++++++++++
powerpc/emulator.c | 230 ++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 3 +
12 files changed, 515 insertions(+), 1 deletion(-)
create mode 100644 lib/powerpc/asm/processor.h
create mode 100644 lib/powerpc/processor.c
create mode 100644 lib/ppc64/asm/processor.h
create mode 100644 lib/ppc64/asm/ptrace.h
create mode 100644 powerpc/emulator.c
--
2.5.0
^ permalink raw reply [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 0/5] Check emulation
@ 2016-03-16 15:12 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:12 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
This series adds exception handler to be able to test instruction
emulation. We test then invalid instruction, lswx instruction, and
check that lswx instruction triggers an alignment interrupt in
little-endian mode. During the development of this series, I've seen
that processor is not running in 64bit mode, so I've added a test
to check that is the case now.
This has been tested with KVM PR, HV, TCG in big and little-endian mode.
KVM-PR needs a fix to be able to manage correctly invalid instruction.
http://patchwork.ozlabs.org/patch/597855/
QEMU needs a fix to initialize correctly the 64bit mode:
http://patchwork.ozlabs.org/patch/598198/
With TCG, some lswx tests fail because it needs some fixes... I'm working
on this.
Laurent Vivier (5):
powerpc: add exception handler
powerpc: add test to check invalid instruction trap
powerpc: check 64bit mode
powerpc: check lswx
powerpc: Check lswx in little-endian mode.
lib/powerpc/asm/hcall.h | 1 +
lib/powerpc/asm/ppc_asm.h | 5 +
lib/powerpc/asm/processor.h | 11 +++
lib/powerpc/processor.c | 38 ++++++++
lib/powerpc/setup.c | 19 ++++
lib/ppc64/asm-offsets.c | 42 ++++++++
lib/ppc64/asm/processor.h | 1 +
lib/ppc64/asm/ptrace.h | 24 +++++
powerpc/Makefile.common | 6 +-
powerpc/cstart64.S | 136 ++++++++++++++++++++++++++
powerpc/emulator.c | 230 ++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 3 +
12 files changed, 515 insertions(+), 1 deletion(-)
create mode 100644 lib/powerpc/asm/processor.h
create mode 100644 lib/powerpc/processor.c
create mode 100644 lib/ppc64/asm/processor.h
create mode 100644 lib/ppc64/asm/ptrace.h
create mode 100644 powerpc/emulator.c
--
2.5.0
^ permalink raw reply [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-16 15:12 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:12 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
lib/powerpc/asm/hcall.h | 1 +
lib/powerpc/asm/ppc_asm.h | 5 ++
lib/powerpc/asm/processor.h | 11 ++++
lib/powerpc/processor.c | 38 +++++++++++++
lib/powerpc/setup.c | 19 +++++++
lib/ppc64/asm-offsets.c | 42 ++++++++++++++
lib/ppc64/asm/processor.h | 1 +
lib/ppc64/asm/ptrace.h | 24 ++++++++
powerpc/Makefile.common | 1 +
powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 278 insertions(+)
create mode 100644 lib/powerpc/asm/processor.h
create mode 100644 lib/powerpc/processor.c
create mode 100644 lib/ppc64/asm/processor.h
create mode 100644 lib/ppc64/asm/ptrace.h
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index f6f9ea8..99bce79 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -20,6 +20,7 @@
#define H_PAGE_INIT 0x2c
#define H_PUT_TERM_CHAR 0x58
#define H_RANDOM 0x300
+#define H_SET_MODE 0x31C
#ifndef __ASSEMBLY__
/*
diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index f18100e..39620a3 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -1,6 +1,11 @@
#ifndef _ASMPOWERPC_PPC_ASM_H
#define _ASMPOWERPC_PPC_ASM_H
+#include <asm/asm-offsets.h>
+
+#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
+#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
+
#define LOAD_REG_IMMEDIATE(reg,expr) \
lis reg,(expr)@highest; \
ori reg,reg,(expr)@higher; \
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
new file mode 100644
index 0000000..09692bd
--- /dev/null
+++ b/lib/powerpc/asm/processor.h
@@ -0,0 +1,11 @@
+#ifndef _ASMPOWERPC_PROCESSOR_H_
+#define _ASMPOWERPC_PROCESSOR_H_
+
+#include <asm/ptrace.h>
+
+#ifndef __ASSEMBLY__
+void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
+void do_handle_exception(struct pt_regs *regs);
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
new file mode 100644
index 0000000..a78bc3c
--- /dev/null
+++ b/lib/powerpc/processor.c
@@ -0,0 +1,38 @@
+/*
+ * processor control and status function
+ */
+
+#include <libcflat.h>
+#include <asm/processor.h>
+#include <asm/ptrace.h>
+
+static struct {
+ void (*func)(struct pt_regs *, void *data);
+ void *data;
+} handlers[16];
+
+void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
+ void * data)
+{
+ trap >>= 8;
+
+ if (trap < 16) {
+ handlers[trap].func = func;
+ handlers[trap].data = data;
+ }
+}
+
+void do_handle_exception(struct pt_regs *regs)
+{
+ unsigned char v;
+
+ v = regs->trap >> 8;
+
+ if (v < 16 && handlers[v].func) {
+ handlers[v].func(regs, handlers[v].data);
+ return;
+ }
+
+ printf("unhandled cpu exception 0x%lx\n", regs->trap);
+ abort();
+}
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 0c0c882..afe7fbc 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -16,6 +16,8 @@
#include <alloc.h>
#include <asm/setup.h>
#include <asm/page.h>
+#include <asm/ppc_asm.h>
+#include <asm/hcall.h>
extern unsigned long stacktop;
extern void io_init(void);
@@ -33,6 +35,10 @@ struct cpu_set_params {
unsigned dcache_bytes;
};
+#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
+
+static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
+
static void cpu_set(int fdtnode, u32 regval, void *info)
{
static bool read_common_info = false;
@@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
}
cpus[cpu] = regval;
+ /* set exception stack address for this CPU (in SPGR0) */
+
+ asm volatile ("mtsprg0 %[addr]" ::
+ [addr] "r" (exception_stack + cpu + 1));
+
if (!read_common_info) {
const struct fdt_property *prop;
u32 *data;
@@ -76,6 +87,14 @@ static void cpu_init(void)
assert(ret == 0);
__icache_bytes = params.icache_bytes;
__dcache_bytes = params.dcache_bytes;
+
+ /* Interrupt Endianness */
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ hcall(H_SET_MODE, 1, 4, 0, 0);
+#else
+ hcall(H_SET_MODE, 0, 4, 0, 0);
+#endif
}
static void mem_init(phys_addr_t freemem_start)
diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
index 2d38a71..7843a20 100644
--- a/lib/ppc64/asm-offsets.c
+++ b/lib/ppc64/asm-offsets.c
@@ -5,8 +5,50 @@
*/
#include <libcflat.h>
#include <kbuild.h>
+#include <asm/ptrace.h>
int main(void)
{
+ DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
+
+ DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
+ DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
+ DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
+ DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
+ DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
+ DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
+ DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
+ DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
+ DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
+ DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
+ DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
+ DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
+ DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
+ DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
+ DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
+ DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
+ DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
+ DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
+ DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
+ DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
+ DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
+ DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
+ DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
+ DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
+ DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
+ DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
+ DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
+ DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
+ DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
+ DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
+ DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
+ DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
+ DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
+ DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
+ DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
+ DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
+ DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
+ DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
+ DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
return 0;
}
diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
new file mode 100644
index 0000000..066a51a
--- /dev/null
+++ b/lib/ppc64/asm/processor.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/processor.h"
diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
new file mode 100644
index 0000000..076c9d9
--- /dev/null
+++ b/lib/ppc64/asm/ptrace.h
@@ -0,0 +1,24 @@
+#ifndef _ASMPPC64_PTRACE_H_
+#define _ASMPPC64_PTRACE_H_
+
+#define KERNEL_REDZONE_SIZE 288
+#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
+
+#ifndef __ASSEMBLY__
+struct pt_regs {
+ unsigned long gpr[32];
+ unsigned long nip;
+ unsigned long msr;
+ unsigned long ctr;
+ unsigned long link;
+ unsigned long xer;
+ unsigned long ccr;
+ unsigned long trap;
+};
+
+#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
+ STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASMPPC64_PTRACE_H_ */
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 424983e..ab2caf6 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
cflatobjs += lib/powerpc/hcall.o
cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
+cflatobjs += lib/powerpc/processor.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index c87e3d6..bc5aeac 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -9,6 +9,7 @@
#include <asm/hcall.h>
#include <asm/ppc_asm.h>
#include <asm/rtas.h>
+#include <asm/ptrace.h>
.section .init
@@ -45,6 +46,34 @@ start:
add r4, r4, r31
bl relocate
+ /* relocate vector table to base address 0x0 (MSR_IP = 0) */
+
+ /* source: r4, dest end: r5, destination: r6 */
+
+ LOAD_REG_ADDR(r4, __start_interrupts)
+ LOAD_REG_ADDR(r5, __end_interrupts)
+ sub r5,r5,r4
+ li r6,0x100
+
+ sub r4,r4,r6
+ add r5,r5,r6
+ addi r6,r6,-8
+2: li r0,8
+ mtctr r0
+ /* copy a cache line size */
+3: addi r6,r6,8
+ ldx r0,r6,r4
+ stdx r0,0,r6
+ bdnz 3b
+ dcbst 0,r6
+ /* flush icache */
+ sync
+ icbi 0,r6
+ cmpld 0,r6,r5
+ blt 2b
+ sync
+ isync
+
/* patch sc1 if needed */
bl hcall_have_broken_sc1
cmpwi r3, 0
@@ -105,3 +134,110 @@ rtas_return_loc:
ld r0, 16(r1)
mtlr r0
blr
+
+call_handler:
+ /* save context */
+
+ /* GPRs */
+
+ .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
+ 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
+ SAVE_GPR(\i, r1)
+ .endr
+ mfsprg1 r0
+ std r0,GPR1(r1)
+
+ /* lr, xer, ccr */
+
+ mflr r0
+ std r0,_LINK(r1)
+
+ mfxer r0
+ std r0,_XER(r1)
+
+ mfcr r0
+ std r0,_CCR(r1)
+
+ /* nip and msr */
+
+ mfsrr0 r0
+ std r0, _NIP(r1)
+
+ mfsrr1 r0
+ std r0, _MSR(r1)
+
+ /* FIXME: build stack frame */
+
+ /* call generic handler */
+
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl do_handle_exception
+
+ /* restore context */
+
+ ld r0,_CTR(r1)
+ mtctr r0
+
+ ld r0,_LINK(r1)
+ mtlr r0
+
+ ld r0,_XER(r1)
+ mtxer r0
+
+ ld r0,_CCR(r1)
+ mtcr r0
+
+ ld r0, _NIP(r1)
+ mtsrr0 r0
+
+ ld r0, _MSR(r1)
+ mtsrr1 r0
+
+ .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
+ 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
+ REST_GPR(\i, r1)
+ .endr
+
+ rfid
+ b .
+
+.section .text.ex
+
+.macro VECTOR vec
+ . = \vec
+
+ mtsprg1 r1 /* save r1 */
+ mfsprg0 r1 /* get exception stack address */
+ subi r1,r1, INT_FRAME_SIZE
+
+ /* save r0 and ctr to call generic handler */
+
+ SAVE_GPR(0,r1)
+
+ mfctr r0
+ std r0,_CTR(r1)
+
+ LOAD_REG_ADDR(r0, call_handler)
+ mtctr r0
+
+ li r0,\vec
+ std r0,_TRAP(r1)
+
+ bctr
+.endm
+
+ . = 0x100
+ .globl __start_interrupts
+__start_interrupts:
+
+VECTOR(0x300)
+VECTOR(0x400)
+VECTOR(0x500)
+VECTOR(0x600)
+VECTOR(0x700)
+VECTOR(0x800)
+VECTOR(0x900)
+
+ .align 7
+ .globl __end_interrupts
+__end_interrupts:
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
@ 2016-03-16 15:12 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:12 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
lib/powerpc/asm/hcall.h | 1 +
lib/powerpc/asm/ppc_asm.h | 5 ++
lib/powerpc/asm/processor.h | 11 ++++
lib/powerpc/processor.c | 38 +++++++++++++
lib/powerpc/setup.c | 19 +++++++
lib/ppc64/asm-offsets.c | 42 ++++++++++++++
lib/ppc64/asm/processor.h | 1 +
lib/ppc64/asm/ptrace.h | 24 ++++++++
powerpc/Makefile.common | 1 +
powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 278 insertions(+)
create mode 100644 lib/powerpc/asm/processor.h
create mode 100644 lib/powerpc/processor.c
create mode 100644 lib/ppc64/asm/processor.h
create mode 100644 lib/ppc64/asm/ptrace.h
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index f6f9ea8..99bce79 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -20,6 +20,7 @@
#define H_PAGE_INIT 0x2c
#define H_PUT_TERM_CHAR 0x58
#define H_RANDOM 0x300
+#define H_SET_MODE 0x31C
#ifndef __ASSEMBLY__
/*
diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
index f18100e..39620a3 100644
--- a/lib/powerpc/asm/ppc_asm.h
+++ b/lib/powerpc/asm/ppc_asm.h
@@ -1,6 +1,11 @@
#ifndef _ASMPOWERPC_PPC_ASM_H
#define _ASMPOWERPC_PPC_ASM_H
+#include <asm/asm-offsets.h>
+
+#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
+#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
+
#define LOAD_REG_IMMEDIATE(reg,expr) \
lis reg,(expr)@highest; \
ori reg,reg,(expr)@higher; \
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
new file mode 100644
index 0000000..09692bd
--- /dev/null
+++ b/lib/powerpc/asm/processor.h
@@ -0,0 +1,11 @@
+#ifndef _ASMPOWERPC_PROCESSOR_H_
+#define _ASMPOWERPC_PROCESSOR_H_
+
+#include <asm/ptrace.h>
+
+#ifndef __ASSEMBLY__
+void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
+void do_handle_exception(struct pt_regs *regs);
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
new file mode 100644
index 0000000..a78bc3c
--- /dev/null
+++ b/lib/powerpc/processor.c
@@ -0,0 +1,38 @@
+/*
+ * processor control and status function
+ */
+
+#include <libcflat.h>
+#include <asm/processor.h>
+#include <asm/ptrace.h>
+
+static struct {
+ void (*func)(struct pt_regs *, void *data);
+ void *data;
+} handlers[16];
+
+void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
+ void * data)
+{
+ trap >>= 8;
+
+ if (trap < 16) {
+ handlers[trap].func = func;
+ handlers[trap].data = data;
+ }
+}
+
+void do_handle_exception(struct pt_regs *regs)
+{
+ unsigned char v;
+
+ v = regs->trap >> 8;
+
+ if (v < 16 && handlers[v].func) {
+ handlers[v].func(regs, handlers[v].data);
+ return;
+ }
+
+ printf("unhandled cpu exception 0x%lx\n", regs->trap);
+ abort();
+}
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index 0c0c882..afe7fbc 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -16,6 +16,8 @@
#include <alloc.h>
#include <asm/setup.h>
#include <asm/page.h>
+#include <asm/ppc_asm.h>
+#include <asm/hcall.h>
extern unsigned long stacktop;
extern void io_init(void);
@@ -33,6 +35,10 @@ struct cpu_set_params {
unsigned dcache_bytes;
};
+#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
+
+static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
+
static void cpu_set(int fdtnode, u32 regval, void *info)
{
static bool read_common_info = false;
@@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
}
cpus[cpu] = regval;
+ /* set exception stack address for this CPU (in SPGR0) */
+
+ asm volatile ("mtsprg0 %[addr]" ::
+ [addr] "r" (exception_stack + cpu + 1));
+
if (!read_common_info) {
const struct fdt_property *prop;
u32 *data;
@@ -76,6 +87,14 @@ static void cpu_init(void)
assert(ret = 0);
__icache_bytes = params.icache_bytes;
__dcache_bytes = params.dcache_bytes;
+
+ /* Interrupt Endianness */
+
+#if __BYTE_ORDER__ = __ORDER_LITTLE_ENDIAN__
+ hcall(H_SET_MODE, 1, 4, 0, 0);
+#else
+ hcall(H_SET_MODE, 0, 4, 0, 0);
+#endif
}
static void mem_init(phys_addr_t freemem_start)
diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
index 2d38a71..7843a20 100644
--- a/lib/ppc64/asm-offsets.c
+++ b/lib/ppc64/asm-offsets.c
@@ -5,8 +5,50 @@
*/
#include <libcflat.h>
#include <kbuild.h>
+#include <asm/ptrace.h>
int main(void)
{
+ DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
+
+ DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
+ DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
+ DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
+ DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
+ DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
+ DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
+ DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
+ DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
+ DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
+ DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
+ DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
+ DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
+ DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
+ DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
+ DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
+ DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
+ DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
+ DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
+ DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
+ DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
+ DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
+ DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
+ DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
+ DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
+ DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
+ DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
+ DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
+ DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
+ DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
+ DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
+ DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
+ DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
+ DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
+ DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
+ DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
+ DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
+ DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
+ DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
+ DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
return 0;
}
diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
new file mode 100644
index 0000000..066a51a
--- /dev/null
+++ b/lib/ppc64/asm/processor.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/processor.h"
diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
new file mode 100644
index 0000000..076c9d9
--- /dev/null
+++ b/lib/ppc64/asm/ptrace.h
@@ -0,0 +1,24 @@
+#ifndef _ASMPPC64_PTRACE_H_
+#define _ASMPPC64_PTRACE_H_
+
+#define KERNEL_REDZONE_SIZE 288
+#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
+
+#ifndef __ASSEMBLY__
+struct pt_regs {
+ unsigned long gpr[32];
+ unsigned long nip;
+ unsigned long msr;
+ unsigned long ctr;
+ unsigned long link;
+ unsigned long xer;
+ unsigned long ccr;
+ unsigned long trap;
+};
+
+#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
+ STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASMPPC64_PTRACE_H_ */
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 424983e..ab2caf6 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
cflatobjs += lib/powerpc/hcall.o
cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
+cflatobjs += lib/powerpc/processor.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index c87e3d6..bc5aeac 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -9,6 +9,7 @@
#include <asm/hcall.h>
#include <asm/ppc_asm.h>
#include <asm/rtas.h>
+#include <asm/ptrace.h>
.section .init
@@ -45,6 +46,34 @@ start:
add r4, r4, r31
bl relocate
+ /* relocate vector table to base address 0x0 (MSR_IP = 0) */
+
+ /* source: r4, dest end: r5, destination: r6 */
+
+ LOAD_REG_ADDR(r4, __start_interrupts)
+ LOAD_REG_ADDR(r5, __end_interrupts)
+ sub r5,r5,r4
+ li r6,0x100
+
+ sub r4,r4,r6
+ add r5,r5,r6
+ addi r6,r6,-8
+2: li r0,8
+ mtctr r0
+ /* copy a cache line size */
+3: addi r6,r6,8
+ ldx r0,r6,r4
+ stdx r0,0,r6
+ bdnz 3b
+ dcbst 0,r6
+ /* flush icache */
+ sync
+ icbi 0,r6
+ cmpld 0,r6,r5
+ blt 2b
+ sync
+ isync
+
/* patch sc1 if needed */
bl hcall_have_broken_sc1
cmpwi r3, 0
@@ -105,3 +134,110 @@ rtas_return_loc:
ld r0, 16(r1)
mtlr r0
blr
+
+call_handler:
+ /* save context */
+
+ /* GPRs */
+
+ .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
+ 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
+ SAVE_GPR(\i, r1)
+ .endr
+ mfsprg1 r0
+ std r0,GPR1(r1)
+
+ /* lr, xer, ccr */
+
+ mflr r0
+ std r0,_LINK(r1)
+
+ mfxer r0
+ std r0,_XER(r1)
+
+ mfcr r0
+ std r0,_CCR(r1)
+
+ /* nip and msr */
+
+ mfsrr0 r0
+ std r0, _NIP(r1)
+
+ mfsrr1 r0
+ std r0, _MSR(r1)
+
+ /* FIXME: build stack frame */
+
+ /* call generic handler */
+
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ bl do_handle_exception
+
+ /* restore context */
+
+ ld r0,_CTR(r1)
+ mtctr r0
+
+ ld r0,_LINK(r1)
+ mtlr r0
+
+ ld r0,_XER(r1)
+ mtxer r0
+
+ ld r0,_CCR(r1)
+ mtcr r0
+
+ ld r0, _NIP(r1)
+ mtsrr0 r0
+
+ ld r0, _MSR(r1)
+ mtsrr1 r0
+
+ .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
+ 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
+ REST_GPR(\i, r1)
+ .endr
+
+ rfid
+ b .
+
+.section .text.ex
+
+.macro VECTOR vec
+ . = \vec
+
+ mtsprg1 r1 /* save r1 */
+ mfsprg0 r1 /* get exception stack address */
+ subi r1,r1, INT_FRAME_SIZE
+
+ /* save r0 and ctr to call generic handler */
+
+ SAVE_GPR(0,r1)
+
+ mfctr r0
+ std r0,_CTR(r1)
+
+ LOAD_REG_ADDR(r0, call_handler)
+ mtctr r0
+
+ li r0,\vec
+ std r0,_TRAP(r1)
+
+ bctr
+.endm
+
+ . = 0x100
+ .globl __start_interrupts
+__start_interrupts:
+
+VECTOR(0x300)
+VECTOR(0x400)
+VECTOR(0x500)
+VECTOR(0x600)
+VECTOR(0x700)
+VECTOR(0x800)
+VECTOR(0x900)
+
+ .align 7
+ .globl __end_interrupts
+__end_interrupts:
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-16 15:13 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/Makefile.common | 5 ++++-
powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 3 +++
3 files changed, 53 insertions(+), 1 deletion(-)
create mode 100644 powerpc/emulator.c
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index ab2caf6..257e3fb 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -7,7 +7,8 @@
tests-common = \
$(TEST_DIR)/selftest.elf \
$(TEST_DIR)/spapr_hcall.elf \
- $(TEST_DIR)/rtas.elf
+ $(TEST_DIR)/rtas.elf \
+ $(TEST_DIR)/emulator.elf
all: $(TEST_DIR)/boot_rom.bin test_cases
@@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
$(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
+
+$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
new file mode 100644
index 0000000..1215c4f
--- /dev/null
+++ b/powerpc/emulator.c
@@ -0,0 +1,46 @@
+/*
+ * Test some powerpc instructions
+ */
+
+#include <libcflat.h>
+#include <asm/processor.h>
+
+static int volatile is_invalid;
+
+static void program_check_handler(struct pt_regs *regs, void *opaque)
+{
+ int *data = opaque;
+
+ printf("Detected invalid instruction 0x%016lx: %08x\n",
+ regs->nip, *(uint32_t*)regs->nip);
+
+ *data = 1;
+
+ regs->nip += 4;
+}
+
+static void test_illegal(void)
+{
+ report_prefix_push("invalid");
+
+ is_invalid = 0;
+
+ asm volatile (".long 0");
+
+ report("exception", is_invalid);
+
+ report_prefix_pop();
+}
+
+int main(void)
+{
+ handle_exception(0x700, program_check_handler, (void *)&is_invalid);
+
+ report_prefix_push("emulator");
+
+ test_illegal();
+
+ report_prefix_pop();
+
+ return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 02b21c7..ed4fdbe 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -47,3 +47,6 @@ file = rtas.elf
extra_params = -append "set-time-of-day"
timeout = 5
groups = rtas
+
+[emulator]
+file = emulator.elf
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
@ 2016-03-16 15:13 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/Makefile.common | 5 ++++-
powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 3 +++
3 files changed, 53 insertions(+), 1 deletion(-)
create mode 100644 powerpc/emulator.c
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index ab2caf6..257e3fb 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -7,7 +7,8 @@
tests-common = \
$(TEST_DIR)/selftest.elf \
$(TEST_DIR)/spapr_hcall.elf \
- $(TEST_DIR)/rtas.elf
+ $(TEST_DIR)/rtas.elf \
+ $(TEST_DIR)/emulator.elf
all: $(TEST_DIR)/boot_rom.bin test_cases
@@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
$(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
+
+$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
new file mode 100644
index 0000000..1215c4f
--- /dev/null
+++ b/powerpc/emulator.c
@@ -0,0 +1,46 @@
+/*
+ * Test some powerpc instructions
+ */
+
+#include <libcflat.h>
+#include <asm/processor.h>
+
+static int volatile is_invalid;
+
+static void program_check_handler(struct pt_regs *regs, void *opaque)
+{
+ int *data = opaque;
+
+ printf("Detected invalid instruction 0x%016lx: %08x\n",
+ regs->nip, *(uint32_t*)regs->nip);
+
+ *data = 1;
+
+ regs->nip += 4;
+}
+
+static void test_illegal(void)
+{
+ report_prefix_push("invalid");
+
+ is_invalid = 0;
+
+ asm volatile (".long 0");
+
+ report("exception", is_invalid);
+
+ report_prefix_pop();
+}
+
+int main(void)
+{
+ handle_exception(0x700, program_check_handler, (void *)&is_invalid);
+
+ report_prefix_push("emulator");
+
+ test_illegal();
+
+ report_prefix_pop();
+
+ return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 02b21c7..ed4fdbe 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -47,3 +47,6 @@ file = rtas.elf
extra_params = -append "set-time-of-day"
timeout = 5
groups = rtas
+
+[emulator]
+file = emulator.elf
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-16 15:13 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Check in MSR if the SF bit is set (64bit mode is enabled)
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 1215c4f..b66c1d7 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -32,12 +32,26 @@ static void test_illegal(void)
report_prefix_pop();
}
+static void test_64bit(void)
+{
+ uint64_t msr;
+
+ report_prefix_push("64bit");
+
+ asm("mfmsr %[msr]": [msr] "=r" (msr));
+
+ report("detected", msr & 0x8000000000000000UL);
+
+ report_prefix_pop();
+}
+
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
report_prefix_push("emulator");
+ test_64bit();
test_illegal();
report_prefix_pop();
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
@ 2016-03-16 15:13 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Check in MSR if the SF bit is set (64bit mode is enabled)
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 1215c4f..b66c1d7 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -32,12 +32,26 @@ static void test_illegal(void)
report_prefix_pop();
}
+static void test_64bit(void)
+{
+ uint64_t msr;
+
+ report_prefix_push("64bit");
+
+ asm("mfmsr %[msr]": [msr] "=r" (msr));
+
+ report("detected", msr & 0x8000000000000000UL);
+
+ report_prefix_pop();
+}
+
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
report_prefix_push("emulator");
+ test_64bit();
test_illegal();
report_prefix_pop();
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-16 15:13 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 148 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index b66c1d7..dfe5859 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -45,6 +45,153 @@ static void test_64bit(void)
report_prefix_pop();
}
+/*
+ * lswx: Load String Word Indexed X-form
+ *
+ * lswx RT,RA,RB
+ *
+ * EA = (RA|0) + RB
+ * n = XER
+ *
+ * Load n bytes from address EA into (n / 4) consecutive registers,
+ * throught RT -> RT + (n / 4) - 1.
+ * - Data are loaded into 4 low order bytes of registers (Word).
+ * - The unfilled bytes are set to 0.
+ * - The sequence of registers wraps around to GPR0.
+ * - if n == 0, content of RT is undefined
+ * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
+ * - RT == RA == 0 is invalid
+ *
+ */
+
+#define SPR_XER 1
+
+static void test_lswx(void)
+{
+ int i;
+ char addr[128];
+ uint64_t regs[32];
+
+ report_prefix_push("lswx");
+
+ /* fill memory with sequence */
+
+ for (i = 0; i < 128; i++)
+ addr[i] = 1 + i;
+
+ /* check incomplete register filling */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r12,-1;"
+ "mr r11, r12;"
+ "lswx r11, 0, %[addr];"
+ "std r11, 0*8(%[regs]);"
+ "std r12, 1*8(%[regs]);"
+ ::
+ [len] "r" (3),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r11", "r12");
+
+ report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
+
+ /* check an old know bug: the number of bytes is used as
+ * the number of registers, so try 32 bytes.
+ */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r19,-1;"
+ "mr r11, r19; mr r12, r19; mr r13, r19;"
+ "mr r14, r19; mr r15, r19; mr r16, r19;"
+ "mr r17, r19; mr r18, r19;"
+ "lswx r11, 0, %[addr];"
+ "std r11, 0*8(%[regs]);"
+ "std r12, 1*8(%[regs]);"
+ "std r13, 2*8(%[regs]);"
+ "std r14, 3*8(%[regs]);"
+ "std r15, 4*8(%[regs]);"
+ "std r16, 5*8(%[regs]);"
+ "std r17, 6*8(%[regs]);"
+ "std r18, 7*8(%[regs]);"
+ "std r19, 8*8(%[regs]);"
+ ::
+ [len] "r" (32),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
+ "r18", "r19");
+
+ report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
+ regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
+ regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
+ regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
+ regs[8] == (uint64_t)-1);
+
+ /* check wrap around to r0 */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r31,-1;"
+ "mr r0, r31;"
+ "lswx r31, 0, %[addr];"
+ "std r31, 0*8(%[regs]);"
+ "std r0, 1*8(%[regs]);"
+ ::
+ [len] "r" (8),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r31", "r0");
+
+ report("wrap around to r0", regs[0] == 0x01020304 &&
+ regs[1] == 0x05060708);
+
+ /* check wrap around to r0 over RB doesn't break RB */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ /* adding r1 in the clobber list doesn't protect it... */
+ "mr r29,r1;"
+ "li r31,-1;"
+ "mr r1,r31;"
+ "mr r0, %[addr];"
+ "lswx r31, 0, r0;"
+ "std r31, 0*8(%[regs]);"
+ "std r0, 1*8(%[regs]);"
+ "std r1, 2*8(%[regs]);"
+ "mr r1,r29;"
+ ::
+ [len] "r" (12),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r31", "r0", "r29");
+
+ /* doc says it is invalid, real proc stops when it comes to
+ * overwrite the register.
+ * In all the cases, the register must stay untouched
+ */
+ report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
+
+ report_prefix_pop();
+}
+
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
@@ -53,6 +200,7 @@ int main(void)
test_64bit();
test_illegal();
+ test_lswx();
report_prefix_pop();
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-16 15:13 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 148 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index b66c1d7..dfe5859 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -45,6 +45,153 @@ static void test_64bit(void)
report_prefix_pop();
}
+/*
+ * lswx: Load String Word Indexed X-form
+ *
+ * lswx RT,RA,RB
+ *
+ * EA = (RA|0) + RB
+ * n = XER
+ *
+ * Load n bytes from address EA into (n / 4) consecutive registers,
+ * throught RT -> RT + (n / 4) - 1.
+ * - Data are loaded into 4 low order bytes of registers (Word).
+ * - The unfilled bytes are set to 0.
+ * - The sequence of registers wraps around to GPR0.
+ * - if n = 0, content of RT is undefined
+ * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
+ * - RT = RA = 0 is invalid
+ *
+ */
+
+#define SPR_XER 1
+
+static void test_lswx(void)
+{
+ int i;
+ char addr[128];
+ uint64_t regs[32];
+
+ report_prefix_push("lswx");
+
+ /* fill memory with sequence */
+
+ for (i = 0; i < 128; i++)
+ addr[i] = 1 + i;
+
+ /* check incomplete register filling */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r12,-1;"
+ "mr r11, r12;"
+ "lswx r11, 0, %[addr];"
+ "std r11, 0*8(%[regs]);"
+ "std r12, 1*8(%[regs]);"
+ ::
+ [len] "r" (3),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r11", "r12");
+
+ report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
+
+ /* check an old know bug: the number of bytes is used as
+ * the number of registers, so try 32 bytes.
+ */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r19,-1;"
+ "mr r11, r19; mr r12, r19; mr r13, r19;"
+ "mr r14, r19; mr r15, r19; mr r16, r19;"
+ "mr r17, r19; mr r18, r19;"
+ "lswx r11, 0, %[addr];"
+ "std r11, 0*8(%[regs]);"
+ "std r12, 1*8(%[regs]);"
+ "std r13, 2*8(%[regs]);"
+ "std r14, 3*8(%[regs]);"
+ "std r15, 4*8(%[regs]);"
+ "std r16, 5*8(%[regs]);"
+ "std r17, 6*8(%[regs]);"
+ "std r18, 7*8(%[regs]);"
+ "std r19, 8*8(%[regs]);"
+ ::
+ [len] "r" (32),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
+ "r18", "r19");
+
+ report("length", regs[0] = 0x01020304 && regs[1] = 0x05060708 &&
+ regs[2] = 0x090a0b0c && regs[3] = 0x0d0e0f10 &&
+ regs[4] = 0x11121314 && regs[5] = 0x15161718 &&
+ regs[6] = 0x191a1b1c && regs[7] = 0x1d1e1f20 &&
+ regs[8] = (uint64_t)-1);
+
+ /* check wrap around to r0 */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ "li r31,-1;"
+ "mr r0, r31;"
+ "lswx r31, 0, %[addr];"
+ "std r31, 0*8(%[regs]);"
+ "std r0, 1*8(%[regs]);"
+ ::
+ [len] "r" (8),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r31", "r0");
+
+ report("wrap around to r0", regs[0] = 0x01020304 &&
+ regs[1] = 0x05060708);
+
+ /* check wrap around to r0 over RB doesn't break RB */
+
+ asm volatile ("mtspr %[XER], %[len];"
+ /* adding r1 in the clobber list doesn't protect it... */
+ "mr r29,r1;"
+ "li r31,-1;"
+ "mr r1,r31;"
+ "mr r0, %[addr];"
+ "lswx r31, 0, r0;"
+ "std r31, 0*8(%[regs]);"
+ "std r0, 1*8(%[regs]);"
+ "std r1, 2*8(%[regs]);"
+ "mr r1,r29;"
+ ::
+ [len] "r" (12),
+ [XER] "i" (SPR_XER),
+ [addr] "r" (addr),
+ [regs] "r" (regs)
+ :
+ /* as 32 is the number of bytes,
+ * we should modify 32/4 = 8 regs, from r1
+ */
+ "xer", "r31", "r0", "r29");
+
+ /* doc says it is invalid, real proc stops when it comes to
+ * overwrite the register.
+ * In all the cases, the register must stay untouched
+ */
+ report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
+
+ report_prefix_pop();
+}
+
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
@@ -53,6 +200,7 @@ int main(void)
test_64bit();
test_illegal();
+ test_lswx();
report_prefix_pop();
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 5/5] powerpc: Check lswx in little-endian mode.
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-16 15:13 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
For lswx in little-endian mode, an alignment interrupt occurs.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index dfe5859..3a8f17e 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -6,6 +6,7 @@
#include <asm/processor.h>
static int volatile is_invalid;
+static int volatile alignment;
static void program_check_handler(struct pt_regs *regs, void *opaque)
{
@@ -19,6 +20,18 @@ static void program_check_handler(struct pt_regs *regs, void *opaque)
regs->nip += 4;
}
+static void alignment_handler(struct pt_regs *regs, void *opaque)
+{
+ int *data = opaque;
+
+ printf("Detected alignment exception 0x%016lx: %08x\n",
+ regs->nip, *(uint32_t*)regs->nip);
+
+ *data = 1;
+
+ regs->nip += 4;
+}
+
static void test_illegal(void)
{
report_prefix_push("invalid");
@@ -62,6 +75,8 @@ static void test_64bit(void)
* - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
* - RT == RA == 0 is invalid
*
+ * For lswx in little-endian mode, an alignment interrupt always occurs.
+ *
*/
#define SPR_XER 1
@@ -81,6 +96,7 @@ static void test_lswx(void)
/* check incomplete register filling */
+ alignment = 0;
asm volatile ("mtspr %[XER], %[len];"
"li r12,-1;"
"mr r11, r12;"
@@ -98,7 +114,12 @@ static void test_lswx(void)
*/
"xer", "r11", "r12");
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ report("alignment", alignment);
+ return;
+#else
report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
+#endif
/* check an old know bug: the number of bytes is used as
* the number of registers, so try 32 bytes.
@@ -195,6 +216,7 @@ static void test_lswx(void)
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
+ handle_exception(0x600, alignment_handler, (void *)&alignment);
report_prefix_push("emulator");
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [kvm-unit-tests PATCH 5/5] powerpc: Check lswx in little-endian mode.
@ 2016-03-16 15:13 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-16 15:13 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
For lswx in little-endian mode, an alignment interrupt occurs.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/emulator.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index dfe5859..3a8f17e 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -6,6 +6,7 @@
#include <asm/processor.h>
static int volatile is_invalid;
+static int volatile alignment;
static void program_check_handler(struct pt_regs *regs, void *opaque)
{
@@ -19,6 +20,18 @@ static void program_check_handler(struct pt_regs *regs, void *opaque)
regs->nip += 4;
}
+static void alignment_handler(struct pt_regs *regs, void *opaque)
+{
+ int *data = opaque;
+
+ printf("Detected alignment exception 0x%016lx: %08x\n",
+ regs->nip, *(uint32_t*)regs->nip);
+
+ *data = 1;
+
+ regs->nip += 4;
+}
+
static void test_illegal(void)
{
report_prefix_push("invalid");
@@ -62,6 +75,8 @@ static void test_64bit(void)
* - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
* - RT = RA = 0 is invalid
*
+ * For lswx in little-endian mode, an alignment interrupt always occurs.
+ *
*/
#define SPR_XER 1
@@ -81,6 +96,7 @@ static void test_lswx(void)
/* check incomplete register filling */
+ alignment = 0;
asm volatile ("mtspr %[XER], %[len];"
"li r12,-1;"
"mr r11, r12;"
@@ -98,7 +114,12 @@ static void test_lswx(void)
*/
"xer", "r11", "r12");
+#if __BYTE_ORDER__ = __ORDER_LITTLE_ENDIAN__
+ report("alignment", alignment);
+ return;
+#else
report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
+#endif
/* check an old know bug: the number of bytes is used as
* the number of registers, so try 32 bytes.
@@ -195,6 +216,7 @@ static void test_lswx(void)
int main(void)
{
handle_exception(0x700, program_check_handler, (void *)&is_invalid);
+ handle_exception(0x600, alignment_handler, (void *)&alignment);
report_prefix_push("emulator");
--
2.5.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-18 3:59 ` David Gibson
-1 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-03-18 3:59 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 13164 bytes --]
On Wed, 16 Mar 2016 16:12:59 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> lib/powerpc/asm/hcall.h | 1 +
> lib/powerpc/asm/ppc_asm.h | 5 ++
> lib/powerpc/asm/processor.h | 11 ++++
> lib/powerpc/processor.c | 38 +++++++++++++
> lib/powerpc/setup.c | 19 +++++++
> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
> lib/ppc64/asm/processor.h | 1 +
> lib/ppc64/asm/ptrace.h | 24 ++++++++
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 278 insertions(+)
> create mode 100644 lib/powerpc/asm/processor.h
> create mode 100644 lib/powerpc/processor.c
> create mode 100644 lib/ppc64/asm/processor.h
> create mode 100644 lib/ppc64/asm/ptrace.h
>
> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> index f6f9ea8..99bce79 100644
> --- a/lib/powerpc/asm/hcall.h
> +++ b/lib/powerpc/asm/hcall.h
> @@ -20,6 +20,7 @@
> #define H_PAGE_INIT 0x2c
> #define H_PUT_TERM_CHAR 0x58
> #define H_RANDOM 0x300
> +#define H_SET_MODE 0x31C
>
> #ifndef __ASSEMBLY__
> /*
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index f18100e..39620a3 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -1,6 +1,11 @@
> #ifndef _ASMPOWERPC_PPC_ASM_H
> #define _ASMPOWERPC_PPC_ASM_H
>
> +#include <asm/asm-offsets.h>
> +
> +#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
> +#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
> +
> #define LOAD_REG_IMMEDIATE(reg,expr) \
> lis reg,(expr)@highest; \
> ori reg,reg,(expr)@higher; \
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> new file mode 100644
> index 0000000..09692bd
> --- /dev/null
> +++ b/lib/powerpc/asm/processor.h
> @@ -0,0 +1,11 @@
> +#ifndef _ASMPOWERPC_PROCESSOR_H_
> +#define _ASMPOWERPC_PROCESSOR_H_
> +
> +#include <asm/ptrace.h>
> +
> +#ifndef __ASSEMBLY__
> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> +void do_handle_exception(struct pt_regs *regs);
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> new file mode 100644
> index 0000000..a78bc3c
> --- /dev/null
> +++ b/lib/powerpc/processor.c
> @@ -0,0 +1,38 @@
> +/*
> + * processor control and status function
> + */
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +#include <asm/ptrace.h>
> +
> +static struct {
> + void (*func)(struct pt_regs *, void *data);
> + void *data;
> +} handlers[16];
> +
> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
> + void * data)
> +{
> + trap >>= 8;
I'm assuming trap starts out as the vector address (e.g. 0x300 for
DSI). Using trap >> 8 is going to break when we want to handle, say,
the SLB miss exceptions at 0x380 and 0x480, which we'll probably want
to do.
I think there are a few exceptions with even smaller spacing, but we
might be able to ignore those for a while.
> + if (trap < 16) {
> + handlers[trap].func = func;
> + handlers[trap].data = data;
> + }
> +}
> +
> +void do_handle_exception(struct pt_regs *regs)
> +{
> + unsigned char v;
> +
> + v = regs->trap >> 8;
> +
> + if (v < 16 && handlers[v].func) {
> + handlers[v].func(regs, handlers[v].data);
> + return;
> + }
> +
> + printf("unhandled cpu exception 0x%lx\n", regs->trap);
> + abort();
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 0c0c882..afe7fbc 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -16,6 +16,8 @@
> #include <alloc.h>
> #include <asm/setup.h>
> #include <asm/page.h>
> +#include <asm/ppc_asm.h>
> +#include <asm/hcall.h>
>
> extern unsigned long stacktop;
> extern void io_init(void);
> @@ -33,6 +35,10 @@ struct cpu_set_params {
> unsigned dcache_bytes;
> };
>
> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
> +
> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
> +
> static void cpu_set(int fdtnode, u32 regval, void *info)
> {
> static bool read_common_info = false;
> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> }
> cpus[cpu] = regval;
>
> + /* set exception stack address for this CPU (in SPGR0) */
> +
> + asm volatile ("mtsprg0 %[addr]" ::
> + [addr] "r" (exception_stack + cpu + 1));
> +
> if (!read_common_info) {
> const struct fdt_property *prop;
> u32 *data;
> @@ -76,6 +87,14 @@ static void cpu_init(void)
> assert(ret == 0);
> __icache_bytes = params.icache_bytes;
> __dcache_bytes = params.dcache_bytes;
> +
> + /* Interrupt Endianness */
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> + hcall(H_SET_MODE, 1, 4, 0, 0);
> +#else
> + hcall(H_SET_MODE, 0, 4, 0, 0);
> +#endif
> }
>
> static void mem_init(phys_addr_t freemem_start)
> diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
> index 2d38a71..7843a20 100644
> --- a/lib/ppc64/asm-offsets.c
> +++ b/lib/ppc64/asm-offsets.c
> @@ -5,8 +5,50 @@
> */
> #include <libcflat.h>
> #include <kbuild.h>
> +#include <asm/ptrace.h>
>
> int main(void)
> {
> + DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
> +
> + DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
> + DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
> + DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
> + DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
> + DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
> + DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
> + DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
> + DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
> + DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
> + DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
> + DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
> + DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
> + DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
> + DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
> + DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
> + DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
> + DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
> + DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
> + DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
> + DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
> + DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
> + DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
> + DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
> + DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
> + DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
> + DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
> + DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
> + DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
> + DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
> + DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
> + DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
> + DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
> + DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
> + DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
> + DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
> + DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
> + DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
> + DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
> + DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
> return 0;
> }
> diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
> new file mode 100644
> index 0000000..066a51a
> --- /dev/null
> +++ b/lib/ppc64/asm/processor.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/processor.h"
> diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
> new file mode 100644
> index 0000000..076c9d9
> --- /dev/null
> +++ b/lib/ppc64/asm/ptrace.h
> @@ -0,0 +1,24 @@
> +#ifndef _ASMPPC64_PTRACE_H_
> +#define _ASMPPC64_PTRACE_H_
> +
> +#define KERNEL_REDZONE_SIZE 288
> +#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
> +
> +#ifndef __ASSEMBLY__
> +struct pt_regs {
> + unsigned long gpr[32];
> + unsigned long nip;
> + unsigned long msr;
> + unsigned long ctr;
> + unsigned long link;
> + unsigned long xer;
> + unsigned long ccr;
> + unsigned long trap;
> +};
> +
> +#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
> + STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPPC64_PTRACE_H_ */
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 424983e..ab2caf6 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
> cflatobjs += lib/powerpc/hcall.o
> cflatobjs += lib/powerpc/setup.o
> cflatobjs += lib/powerpc/rtas.o
> +cflatobjs += lib/powerpc/processor.o
>
> FLATLIBS = $(libcflat) $(LIBFDT_archive)
> %.elf: CFLAGS += $(arch_CFLAGS)
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index c87e3d6..bc5aeac 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -9,6 +9,7 @@
> #include <asm/hcall.h>
> #include <asm/ppc_asm.h>
> #include <asm/rtas.h>
> +#include <asm/ptrace.h>
>
> .section .init
>
> @@ -45,6 +46,34 @@ start:
> add r4, r4, r31
> bl relocate
>
> + /* relocate vector table to base address 0x0 (MSR_IP = 0) */
> +
> + /* source: r4, dest end: r5, destination: r6 */
> +
> + LOAD_REG_ADDR(r4, __start_interrupts)
> + LOAD_REG_ADDR(r5, __end_interrupts)
> + sub r5,r5,r4
> + li r6,0x100
> +
> + sub r4,r4,r6
> + add r5,r5,r6
> + addi r6,r6,-8
> +2: li r0,8
> + mtctr r0
> + /* copy a cache line size */
> +3: addi r6,r6,8
> + ldx r0,r6,r4
> + stdx r0,0,r6
> + bdnz 3b
> + dcbst 0,r6
> + /* flush icache */
> + sync
> + icbi 0,r6
> + cmpld 0,r6,r5
> + blt 2b
> + sync
> + isync
> +
> /* patch sc1 if needed */
> bl hcall_have_broken_sc1
> cmpwi r3, 0
> @@ -105,3 +134,110 @@ rtas_return_loc:
> ld r0, 16(r1)
> mtlr r0
> blr
> +
> +call_handler:
> + /* save context */
> +
> + /* GPRs */
> +
> + .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
> + SAVE_GPR(\i, r1)
> + .endr
> + mfsprg1 r0
> + std r0,GPR1(r1)
> +
> + /* lr, xer, ccr */
> +
> + mflr r0
> + std r0,_LINK(r1)
> +
> + mfxer r0
> + std r0,_XER(r1)
> +
> + mfcr r0
> + std r0,_CCR(r1)
> +
> + /* nip and msr */
> +
> + mfsrr0 r0
> + std r0, _NIP(r1)
> +
> + mfsrr1 r0
> + std r0, _MSR(r1)
> +
> + /* FIXME: build stack frame */
> +
> + /* call generic handler */
> +
> + addi r3,r1,STACK_FRAME_OVERHEAD
> + bl do_handle_exception
> +
> + /* restore context */
> +
> + ld r0,_CTR(r1)
> + mtctr r0
> +
> + ld r0,_LINK(r1)
> + mtlr r0
> +
> + ld r0,_XER(r1)
> + mtxer r0
> +
> + ld r0,_CCR(r1)
> + mtcr r0
> +
> + ld r0, _NIP(r1)
> + mtsrr0 r0
> +
> + ld r0, _MSR(r1)
> + mtsrr1 r0
> +
> + .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
The fact that '1' has to go last in this list is pretty subtle. I
wonder if it might be clearer to split that out explicitly rather than
including it in the .irp
> + REST_GPR(\i, r1)
> + .endr
> +
> + rfid
> + b .
> +
> +.section .text.ex
> +
> +.macro VECTOR vec
> + . = \vec
> +
> + mtsprg1 r1 /* save r1 */
> + mfsprg0 r1 /* get exception stack address */
> + subi r1,r1, INT_FRAME_SIZE
> +
> + /* save r0 and ctr to call generic handler */
> +
> + SAVE_GPR(0,r1)
> +
> + mfctr r0
> + std r0,_CTR(r1)
> +
> + LOAD_REG_ADDR(r0, call_handler)
> + mtctr r0
> +
> + li r0,\vec
> + std r0,_TRAP(r1)
> +
> + bctr
> +.endm
> +
> + . = 0x100
> + .globl __start_interrupts
> +__start_interrupts:
> +
> +VECTOR(0x300)
> +VECTOR(0x400)
> +VECTOR(0x500)
> +VECTOR(0x600)
> +VECTOR(0x700)
> +VECTOR(0x800)
> +VECTOR(0x900)
> +
> + .align 7
> + .globl __end_interrupts
> +__end_interrupts:
> --
> 2.5.0
>
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
@ 2016-03-18 3:59 ` David Gibson
0 siblings, 0 replies; 42+ messages in thread
From: David Gibson @ 2016-03-18 3:59 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 13164 bytes --]
On Wed, 16 Mar 2016 16:12:59 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> lib/powerpc/asm/hcall.h | 1 +
> lib/powerpc/asm/ppc_asm.h | 5 ++
> lib/powerpc/asm/processor.h | 11 ++++
> lib/powerpc/processor.c | 38 +++++++++++++
> lib/powerpc/setup.c | 19 +++++++
> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
> lib/ppc64/asm/processor.h | 1 +
> lib/ppc64/asm/ptrace.h | 24 ++++++++
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 278 insertions(+)
> create mode 100644 lib/powerpc/asm/processor.h
> create mode 100644 lib/powerpc/processor.c
> create mode 100644 lib/ppc64/asm/processor.h
> create mode 100644 lib/ppc64/asm/ptrace.h
>
> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> index f6f9ea8..99bce79 100644
> --- a/lib/powerpc/asm/hcall.h
> +++ b/lib/powerpc/asm/hcall.h
> @@ -20,6 +20,7 @@
> #define H_PAGE_INIT 0x2c
> #define H_PUT_TERM_CHAR 0x58
> #define H_RANDOM 0x300
> +#define H_SET_MODE 0x31C
>
> #ifndef __ASSEMBLY__
> /*
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index f18100e..39620a3 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -1,6 +1,11 @@
> #ifndef _ASMPOWERPC_PPC_ASM_H
> #define _ASMPOWERPC_PPC_ASM_H
>
> +#include <asm/asm-offsets.h>
> +
> +#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
> +#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
> +
> #define LOAD_REG_IMMEDIATE(reg,expr) \
> lis reg,(expr)@highest; \
> ori reg,reg,(expr)@higher; \
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> new file mode 100644
> index 0000000..09692bd
> --- /dev/null
> +++ b/lib/powerpc/asm/processor.h
> @@ -0,0 +1,11 @@
> +#ifndef _ASMPOWERPC_PROCESSOR_H_
> +#define _ASMPOWERPC_PROCESSOR_H_
> +
> +#include <asm/ptrace.h>
> +
> +#ifndef __ASSEMBLY__
> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> +void do_handle_exception(struct pt_regs *regs);
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> new file mode 100644
> index 0000000..a78bc3c
> --- /dev/null
> +++ b/lib/powerpc/processor.c
> @@ -0,0 +1,38 @@
> +/*
> + * processor control and status function
> + */
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +#include <asm/ptrace.h>
> +
> +static struct {
> + void (*func)(struct pt_regs *, void *data);
> + void *data;
> +} handlers[16];
> +
> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
> + void * data)
> +{
> + trap >>= 8;
I'm assuming trap starts out as the vector address (e.g. 0x300 for
DSI). Using trap >> 8 is going to break when we want to handle, say,
the SLB miss exceptions at 0x380 and 0x480, which we'll probably want
to do.
I think there are a few exceptions with even smaller spacing, but we
might be able to ignore those for a while.
> + if (trap < 16) {
> + handlers[trap].func = func;
> + handlers[trap].data = data;
> + }
> +}
> +
> +void do_handle_exception(struct pt_regs *regs)
> +{
> + unsigned char v;
> +
> + v = regs->trap >> 8;
> +
> + if (v < 16 && handlers[v].func) {
> + handlers[v].func(regs, handlers[v].data);
> + return;
> + }
> +
> + printf("unhandled cpu exception 0x%lx\n", regs->trap);
> + abort();
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 0c0c882..afe7fbc 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -16,6 +16,8 @@
> #include <alloc.h>
> #include <asm/setup.h>
> #include <asm/page.h>
> +#include <asm/ppc_asm.h>
> +#include <asm/hcall.h>
>
> extern unsigned long stacktop;
> extern void io_init(void);
> @@ -33,6 +35,10 @@ struct cpu_set_params {
> unsigned dcache_bytes;
> };
>
> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
> +
> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
> +
> static void cpu_set(int fdtnode, u32 regval, void *info)
> {
> static bool read_common_info = false;
> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> }
> cpus[cpu] = regval;
>
> + /* set exception stack address for this CPU (in SPGR0) */
> +
> + asm volatile ("mtsprg0 %[addr]" ::
> + [addr] "r" (exception_stack + cpu + 1));
> +
> if (!read_common_info) {
> const struct fdt_property *prop;
> u32 *data;
> @@ -76,6 +87,14 @@ static void cpu_init(void)
> assert(ret == 0);
> __icache_bytes = params.icache_bytes;
> __dcache_bytes = params.dcache_bytes;
> +
> + /* Interrupt Endianness */
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> + hcall(H_SET_MODE, 1, 4, 0, 0);
> +#else
> + hcall(H_SET_MODE, 0, 4, 0, 0);
> +#endif
> }
>
> static void mem_init(phys_addr_t freemem_start)
> diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
> index 2d38a71..7843a20 100644
> --- a/lib/ppc64/asm-offsets.c
> +++ b/lib/ppc64/asm-offsets.c
> @@ -5,8 +5,50 @@
> */
> #include <libcflat.h>
> #include <kbuild.h>
> +#include <asm/ptrace.h>
>
> int main(void)
> {
> + DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
> +
> + DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
> + DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
> + DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
> + DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
> + DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
> + DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
> + DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
> + DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
> + DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
> + DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
> + DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
> + DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
> + DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
> + DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
> + DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
> + DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
> + DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
> + DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
> + DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
> + DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
> + DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
> + DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
> + DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
> + DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
> + DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
> + DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
> + DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
> + DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
> + DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
> + DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
> + DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
> + DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
> + DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
> + DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
> + DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
> + DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
> + DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
> + DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
> + DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
> return 0;
> }
> diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
> new file mode 100644
> index 0000000..066a51a
> --- /dev/null
> +++ b/lib/ppc64/asm/processor.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/processor.h"
> diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
> new file mode 100644
> index 0000000..076c9d9
> --- /dev/null
> +++ b/lib/ppc64/asm/ptrace.h
> @@ -0,0 +1,24 @@
> +#ifndef _ASMPPC64_PTRACE_H_
> +#define _ASMPPC64_PTRACE_H_
> +
> +#define KERNEL_REDZONE_SIZE 288
> +#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
> +
> +#ifndef __ASSEMBLY__
> +struct pt_regs {
> + unsigned long gpr[32];
> + unsigned long nip;
> + unsigned long msr;
> + unsigned long ctr;
> + unsigned long link;
> + unsigned long xer;
> + unsigned long ccr;
> + unsigned long trap;
> +};
> +
> +#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
> + STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPPC64_PTRACE_H_ */
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 424983e..ab2caf6 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
> cflatobjs += lib/powerpc/hcall.o
> cflatobjs += lib/powerpc/setup.o
> cflatobjs += lib/powerpc/rtas.o
> +cflatobjs += lib/powerpc/processor.o
>
> FLATLIBS = $(libcflat) $(LIBFDT_archive)
> %.elf: CFLAGS += $(arch_CFLAGS)
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index c87e3d6..bc5aeac 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -9,6 +9,7 @@
> #include <asm/hcall.h>
> #include <asm/ppc_asm.h>
> #include <asm/rtas.h>
> +#include <asm/ptrace.h>
>
> .section .init
>
> @@ -45,6 +46,34 @@ start:
> add r4, r4, r31
> bl relocate
>
> + /* relocate vector table to base address 0x0 (MSR_IP = 0) */
> +
> + /* source: r4, dest end: r5, destination: r6 */
> +
> + LOAD_REG_ADDR(r4, __start_interrupts)
> + LOAD_REG_ADDR(r5, __end_interrupts)
> + sub r5,r5,r4
> + li r6,0x100
> +
> + sub r4,r4,r6
> + add r5,r5,r6
> + addi r6,r6,-8
> +2: li r0,8
> + mtctr r0
> + /* copy a cache line size */
> +3: addi r6,r6,8
> + ldx r0,r6,r4
> + stdx r0,0,r6
> + bdnz 3b
> + dcbst 0,r6
> + /* flush icache */
> + sync
> + icbi 0,r6
> + cmpld 0,r6,r5
> + blt 2b
> + sync
> + isync
> +
> /* patch sc1 if needed */
> bl hcall_have_broken_sc1
> cmpwi r3, 0
> @@ -105,3 +134,110 @@ rtas_return_loc:
> ld r0, 16(r1)
> mtlr r0
> blr
> +
> +call_handler:
> + /* save context */
> +
> + /* GPRs */
> +
> + .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
> + SAVE_GPR(\i, r1)
> + .endr
> + mfsprg1 r0
> + std r0,GPR1(r1)
> +
> + /* lr, xer, ccr */
> +
> + mflr r0
> + std r0,_LINK(r1)
> +
> + mfxer r0
> + std r0,_XER(r1)
> +
> + mfcr r0
> + std r0,_CCR(r1)
> +
> + /* nip and msr */
> +
> + mfsrr0 r0
> + std r0, _NIP(r1)
> +
> + mfsrr1 r0
> + std r0, _MSR(r1)
> +
> + /* FIXME: build stack frame */
> +
> + /* call generic handler */
> +
> + addi r3,r1,STACK_FRAME_OVERHEAD
> + bl do_handle_exception
> +
> + /* restore context */
> +
> + ld r0,_CTR(r1)
> + mtctr r0
> +
> + ld r0,_LINK(r1)
> + mtlr r0
> +
> + ld r0,_XER(r1)
> + mtxer r0
> +
> + ld r0,_CCR(r1)
> + mtcr r0
> +
> + ld r0, _NIP(r1)
> + mtsrr0 r0
> +
> + ld r0, _MSR(r1)
> + mtsrr1 r0
> +
> + .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
The fact that '1' has to go last in this list is pretty subtle. I
wonder if it might be clearer to split that out explicitly rather than
including it in the .irp
> + REST_GPR(\i, r1)
> + .endr
> +
> + rfid
> + b .
> +
> +.section .text.ex
> +
> +.macro VECTOR vec
> + . = \vec
> +
> + mtsprg1 r1 /* save r1 */
> + mfsprg0 r1 /* get exception stack address */
> + subi r1,r1, INT_FRAME_SIZE
> +
> + /* save r0 and ctr to call generic handler */
> +
> + SAVE_GPR(0,r1)
> +
> + mfctr r0
> + std r0,_CTR(r1)
> +
> + LOAD_REG_ADDR(r0, call_handler)
> + mtctr r0
> +
> + li r0,\vec
> + std r0,_TRAP(r1)
> +
> + bctr
> +.endm
> +
> + . = 0x100
> + .globl __start_interrupts
> +__start_interrupts:
> +
> +VECTOR(0x300)
> +VECTOR(0x400)
> +VECTOR(0x500)
> +VECTOR(0x600)
> +VECTOR(0x700)
> +VECTOR(0x800)
> +VECTOR(0x900)
> +
> + .align 7
> + .globl __end_interrupts
> +__end_interrupts:
> --
> 2.5.0
>
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
2016-03-16 15:13 ` Laurent Vivier
@ 2016-03-18 8:28 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 8:28 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Check in MSR if the SF bit is set (64bit mode is enabled)
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index 1215c4f..b66c1d7 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -32,12 +32,26 @@ static void test_illegal(void)
> report_prefix_pop();
> }
>
> +static void test_64bit(void)
> +{
> + uint64_t msr;
> +
> + report_prefix_push("64bit");
> +
> + asm("mfmsr %[msr]": [msr] "=r" (msr));
> +
> + report("detected", msr & 0x8000000000000000UL);
> +
> + report_prefix_pop();
> +}
> +
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>
> report_prefix_push("emulator");
>
> + test_64bit();
> test_illegal();
>
> report_prefix_pop();
Reviewed-by: Thomas Huth <thuth@redhat.com>
But I still wonder how kvm-unit-tests worked at all before the 64-bit
mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
64-bit code, but it's still working when run in 32-bit? Something really
strange was going on here...
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
@ 2016-03-18 8:28 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 8:28 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Check in MSR if the SF bit is set (64bit mode is enabled)
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index 1215c4f..b66c1d7 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -32,12 +32,26 @@ static void test_illegal(void)
> report_prefix_pop();
> }
>
> +static void test_64bit(void)
> +{
> + uint64_t msr;
> +
> + report_prefix_push("64bit");
> +
> + asm("mfmsr %[msr]": [msr] "=r" (msr));
> +
> + report("detected", msr & 0x8000000000000000UL);
> +
> + report_prefix_pop();
> +}
> +
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>
> report_prefix_push("emulator");
>
> + test_64bit();
> test_illegal();
>
> report_prefix_pop();
Reviewed-by: Thomas Huth <thuth@redhat.com>
But I still wonder how kvm-unit-tests worked at all before the 64-bit
mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
64-bit code, but it's still working when run in 32-bit? Something really
strange was going on here...
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
2016-03-18 8:28 ` Thomas Huth
@ 2016-03-18 8:38 ` Paul Mackerras
-1 siblings, 0 replies; 42+ messages in thread
From: Paul Mackerras @ 2016-03-18 8:38 UTC (permalink / raw)
To: Thomas Huth; +Cc: Laurent Vivier, kvm, kvm-ppc, drjones, dgibson, pbonzini
On Fri, Mar 18, 2016 at 09:28:33AM +0100, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
> > Check in MSR if the SF bit is set (64bit mode is enabled)
> >
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> > powerpc/emulator.c | 14 ++++++++++++++f958ee745f70
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> > index 1215c4f..b66c1d7 100644
> > --- a/powerpc/emulator.c
> > +++ b/powerpc/emulator.c
> > @@ -32,12 +32,26 @@ static void test_illegal(void)
> > report_prefix_pop();
> > }
> >
> > +static void test_64bit(void)
> > +{
> > + uint64_t msr;
> > +
> > + report_prefix_push("64bit");
> > +
> > + asm("mfmsr %[msr]": [msr] "=r" (msr));
> > +
> > + report("detected", msr & 0x8000000000000000UL);
> > +
> > + report_prefix_pop();
> > +}
> > +
> > int main(void)
> > {
> > handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> >
> > report_prefix_push("emulator");
> >
> > + test_64bit();
> > test_illegal();
> >
> > report_prefix_pop();
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> But I still wonder how kvm-unit-tests worked at all before the 64-bit
> mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
> 64-bit code, but it's still working when run in 32-bit? Something really
> strange was going on here...
On Power processors, there's not as much difference between 32-bit and
64-bit mode as you might expect. In 32-bit mode on a 64-bit
processor, you can use all 64 bits of the registers and execute 64-bit
instructions. The only things that change in 32-bit mode are that (a)
effective addresses are truncated to 32 bits and (b) dot-form
instructions (e.g. "add.") set CR0 based on the 32-bit result rather
than the 64-bit result.
Paul.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
@ 2016-03-18 8:38 ` Paul Mackerras
0 siblings, 0 replies; 42+ messages in thread
From: Paul Mackerras @ 2016-03-18 8:38 UTC (permalink / raw)
To: Thomas Huth; +Cc: Laurent Vivier, kvm, kvm-ppc, drjones, dgibson, pbonzini
On Fri, Mar 18, 2016 at 09:28:33AM +0100, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
> > Check in MSR if the SF bit is set (64bit mode is enabled)
> >
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> > powerpc/emulator.c | 14 ++++++++++++++f958ee745f70
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> > index 1215c4f..b66c1d7 100644
> > --- a/powerpc/emulator.c
> > +++ b/powerpc/emulator.c
> > @@ -32,12 +32,26 @@ static void test_illegal(void)
> > report_prefix_pop();
> > }
> >
> > +static void test_64bit(void)
> > +{
> > + uint64_t msr;
> > +
> > + report_prefix_push("64bit");
> > +
> > + asm("mfmsr %[msr]": [msr] "=r" (msr));
> > +
> > + report("detected", msr & 0x8000000000000000UL);
> > +
> > + report_prefix_pop();
> > +}
> > +
> > int main(void)
> > {
> > handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> >
> > report_prefix_push("emulator");
> >
> > + test_64bit();
> > test_illegal();
> >
> > report_prefix_pop();
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> But I still wonder how kvm-unit-tests worked at all before the 64-bit
> mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
> 64-bit code, but it's still working when run in 32-bit? Something really
> strange was going on here...
On Power processors, there's not as much difference between 32-bit and
64-bit mode as you might expect. In 32-bit mode on a 64-bit
processor, you can use all 64 bits of the registers and execute 64-bit
instructions. The only things that change in 32-bit mode are that (a)
effective addresses are truncated to 32 bits and (b) dot-form
instructions (e.g. "add.") set CR0 based on the 32-bit result rather
than the 64-bit result.
Paul.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
2016-03-18 8:38 ` Paul Mackerras
@ 2016-03-18 8:41 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 8:41 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Laurent Vivier, kvm, kvm-ppc, drjones, dgibson, pbonzini
On 18.03.2016 09:38, Paul Mackerras wrote:
> On Fri, Mar 18, 2016 at 09:28:33AM +0100, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Check in MSR if the SF bit is set (64bit mode is enabled)
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/emulator.c | 14 ++++++++++++++f958ee745f70
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> index 1215c4f..b66c1d7 100644
>>> --- a/powerpc/emulator.c
>>> +++ b/powerpc/emulator.c
>>> @@ -32,12 +32,26 @@ static void test_illegal(void)
>>> report_prefix_pop();
>>> }
>>>
>>> +static void test_64bit(void)
>>> +{
>>> + uint64_t msr;
>>> +
>>> + report_prefix_push("64bit");
>>> +
>>> + asm("mfmsr %[msr]": [msr] "=r" (msr));
>>> +
>>> + report("detected", msr & 0x8000000000000000UL);
>>> +
>>> + report_prefix_pop();
>>> +}
>>> +
>>> int main(void)
>>> {
>>> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>>>
>>> report_prefix_push("emulator");
>>>
>>> + test_64bit();
>>> test_illegal();
>>>
>>> report_prefix_pop();
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> But I still wonder how kvm-unit-tests worked at all before the 64-bit
>> mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
>> 64-bit code, but it's still working when run in 32-bit? Something really
>> strange was going on here...
>
> On Power processors, there's not as much difference between 32-bit and
> 64-bit mode as you might expect. In 32-bit mode on a 64-bit
> processor, you can use all 64 bits of the registers and execute 64-bit
> instructions. The only things that change in 32-bit mode are that (a)
> effective addresses are truncated to 32 bits and (b) dot-form
> instructions (e.g. "add.") set CR0 based on the 32-bit result rather
> than the 64-bit result.
Ah, ok, ... and since kvm-unit-tests normally does not use the RAM > 4
GB, we didn't notice the truncation. Makes sense.
Thanks for the clarification!
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode
@ 2016-03-18 8:41 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 8:41 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Laurent Vivier, kvm, kvm-ppc, drjones, dgibson, pbonzini
On 18.03.2016 09:38, Paul Mackerras wrote:
> On Fri, Mar 18, 2016 at 09:28:33AM +0100, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Check in MSR if the SF bit is set (64bit mode is enabled)
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/emulator.c | 14 ++++++++++++++f958ee745f70
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> index 1215c4f..b66c1d7 100644
>>> --- a/powerpc/emulator.c
>>> +++ b/powerpc/emulator.c
>>> @@ -32,12 +32,26 @@ static void test_illegal(void)
>>> report_prefix_pop();
>>> }
>>>
>>> +static void test_64bit(void)
>>> +{
>>> + uint64_t msr;
>>> +
>>> + report_prefix_push("64bit");
>>> +
>>> + asm("mfmsr %[msr]": [msr] "=r" (msr));
>>> +
>>> + report("detected", msr & 0x8000000000000000UL);
>>> +
>>> + report_prefix_pop();
>>> +}
>>> +
>>> int main(void)
>>> {
>>> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>>>
>>> report_prefix_push("emulator");
>>>
>>> + test_64bit();
>>> test_illegal();
>>>
>>> report_prefix_pop();
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> But I still wonder how kvm-unit-tests worked at all before the 64-bit
>> mode has been fixed in QEMU ... I mean kvm-unit-tests is compiled with
>> 64-bit code, but it's still working when run in 32-bit? Something really
>> strange was going on here...
>
> On Power processors, there's not as much difference between 32-bit and
> 64-bit mode as you might expect. In 32-bit mode on a 64-bit
> processor, you can use all 64 bits of the registers and execute 64-bit
> instructions. The only things that change in 32-bit mode are that (a)
> effective addresses are truncated to 32 bits and (b) dot-form
> instructions (e.g. "add.") set CR0 based on the 32-bit result rather
> than the 64-bit result.
Ah, ok, ... and since kvm-unit-tests normally does not use the RAM > 4
GB, we didn't notice the truncation. Makes sense.
Thanks for the clarification!
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
2016-03-18 3:59 ` David Gibson
@ 2016-03-18 8:53 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 8:53 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
On 18/03/2016 04:59, David Gibson wrote:
> On Wed, 16 Mar 2016 16:12:59 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>
>
>> ---
>> lib/powerpc/asm/hcall.h | 1 +
>> lib/powerpc/asm/ppc_asm.h | 5 ++
>> lib/powerpc/asm/processor.h | 11 ++++
>> lib/powerpc/processor.c | 38 +++++++++++++
>> lib/powerpc/setup.c | 19 +++++++
>> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
>> lib/ppc64/asm/processor.h | 1 +
>> lib/ppc64/asm/ptrace.h | 24 ++++++++
>> powerpc/Makefile.common | 1 +
>> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
>> 10 files changed, 278 insertions(+)
>> create mode 100644 lib/powerpc/asm/processor.h
>> create mode 100644 lib/powerpc/processor.c
>> create mode 100644 lib/ppc64/asm/processor.h
>> create mode 100644 lib/ppc64/asm/ptrace.h
>>
>> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
>> index f6f9ea8..99bce79 100644
>> --- a/lib/powerpc/asm/hcall.h
>> +++ b/lib/powerpc/asm/hcall.h
>> @@ -20,6 +20,7 @@
>> #define H_PAGE_INIT 0x2c
>> #define H_PUT_TERM_CHAR 0x58
>> #define H_RANDOM 0x300
>> +#define H_SET_MODE 0x31C
>>
>> #ifndef __ASSEMBLY__
>> /*
>> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
>> index f18100e..39620a3 100644
>> --- a/lib/powerpc/asm/ppc_asm.h
>> +++ b/lib/powerpc/asm/ppc_asm.h
>> @@ -1,6 +1,11 @@
>> #ifndef _ASMPOWERPC_PPC_ASM_H
>> #define _ASMPOWERPC_PPC_ASM_H
>>
>> +#include <asm/asm-offsets.h>
>> +
>> +#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
>> +#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
>> +
>> #define LOAD_REG_IMMEDIATE(reg,expr) \
>> lis reg,(expr)@highest; \
>> ori reg,reg,(expr)@higher; \
>> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
>> new file mode 100644
>> index 0000000..09692bd
>> --- /dev/null
>> +++ b/lib/powerpc/asm/processor.h
>> @@ -0,0 +1,11 @@
>> +#ifndef _ASMPOWERPC_PROCESSOR_H_
>> +#define _ASMPOWERPC_PROCESSOR_H_
>> +
>> +#include <asm/ptrace.h>
>> +
>> +#ifndef __ASSEMBLY__
>> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
>> +void do_handle_exception(struct pt_regs *regs);
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* _ASMPOWERPC_PROCESSOR_H_ */
>> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
>> new file mode 100644
>> index 0000000..a78bc3c
>> --- /dev/null
>> +++ b/lib/powerpc/processor.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * processor control and status function
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/processor.h>
>> +#include <asm/ptrace.h>
>> +
>> +static struct {
>> + void (*func)(struct pt_regs *, void *data);
>> + void *data;
>> +} handlers[16];
>> +
>> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
>> + void * data)
>> +{
>> + trap >>= 8;
>
> I'm assuming trap starts out as the vector address (e.g. 0x300 for
> DSI). Using trap >> 8 is going to break when we want to handle, say,
> the SLB miss exceptions at 0x380 and 0x480, which we'll probably want
> to do.
>
> I think there are a few exceptions with even smaller spacing, but we
> might be able to ignore those for a while.
In fact, the handler is registered on (trap >> 8) (for instance 0x3) but
in regs->trap we have the full value (for instance 0x380), so we can
have sub-handler for these particular values.
>
>> + if (trap < 16) {
>> + handlers[trap].func = func;
>> + handlers[trap].data = data;
>> + }
>> +}
>> +
>> +void do_handle_exception(struct pt_regs *regs)
>> +{
>> + unsigned char v;
>> +
>> + v = regs->trap >> 8;
>> +
>> + if (v < 16 && handlers[v].func) {
>> + handlers[v].func(regs, handlers[v].data);
>> + return;
>> + }
>> +
>> + printf("unhandled cpu exception 0x%lx\n", regs->trap);
>> + abort();
>> +}
>> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
>> index 0c0c882..afe7fbc 100644
>> --- a/lib/powerpc/setup.c
>> +++ b/lib/powerpc/setup.c
>> @@ -16,6 +16,8 @@
>> #include <alloc.h>
>> #include <asm/setup.h>
>> #include <asm/page.h>
>> +#include <asm/ppc_asm.h>
>> +#include <asm/hcall.h>
>>
>> extern unsigned long stacktop;
>> extern void io_init(void);
>> @@ -33,6 +35,10 @@ struct cpu_set_params {
>> unsigned dcache_bytes;
>> };
>>
>> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
>> +
>> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
>> +
>> static void cpu_set(int fdtnode, u32 regval, void *info)
>> {
>> static bool read_common_info = false;
>> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
>> }
>> cpus[cpu] = regval;
>>
>> + /* set exception stack address for this CPU (in SPGR0) */
>> +
>> + asm volatile ("mtsprg0 %[addr]" ::
>> + [addr] "r" (exception_stack + cpu + 1));
>> +
>> if (!read_common_info) {
>> const struct fdt_property *prop;
>> u32 *data;
>> @@ -76,6 +87,14 @@ static void cpu_init(void)
>> assert(ret == 0);
>> __icache_bytes = params.icache_bytes;
>> __dcache_bytes = params.dcache_bytes;
>> +
>> + /* Interrupt Endianness */
>> +
>> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> + hcall(H_SET_MODE, 1, 4, 0, 0);
>> +#else
>> + hcall(H_SET_MODE, 0, 4, 0, 0);
>> +#endif
>> }
>>
>> static void mem_init(phys_addr_t freemem_start)
>> diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
>> index 2d38a71..7843a20 100644
>> --- a/lib/ppc64/asm-offsets.c
>> +++ b/lib/ppc64/asm-offsets.c
>> @@ -5,8 +5,50 @@
>> */
>> #include <libcflat.h>
>> #include <kbuild.h>
>> +#include <asm/ptrace.h>
>>
>> int main(void)
>> {
>> + DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
>> +
>> + DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
>> + DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
>> + DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
>> + DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
>> + DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
>> + DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
>> + DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
>> + DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
>> + DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
>> + DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
>> + DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
>> + DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
>> + DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
>> + DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
>> + DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
>> + DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
>> + DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
>> + DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
>> + DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
>> + DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
>> + DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
>> + DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
>> + DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
>> + DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
>> + DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
>> + DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
>> + DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
>> + DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
>> + DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
>> + DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
>> + DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
>> + DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
>> + DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
>> + DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
>> + DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
>> + DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
>> + DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
>> + DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
>> + DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
>> return 0;
>> }
>> diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
>> new file mode 100644
>> index 0000000..066a51a
>> --- /dev/null
>> +++ b/lib/ppc64/asm/processor.h
>> @@ -0,0 +1 @@
>> +#include "../../powerpc/asm/processor.h"
>> diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
>> new file mode 100644
>> index 0000000..076c9d9
>> --- /dev/null
>> +++ b/lib/ppc64/asm/ptrace.h
>> @@ -0,0 +1,24 @@
>> +#ifndef _ASMPPC64_PTRACE_H_
>> +#define _ASMPPC64_PTRACE_H_
>> +
>> +#define KERNEL_REDZONE_SIZE 288
>> +#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
>> +
>> +#ifndef __ASSEMBLY__
>> +struct pt_regs {
>> + unsigned long gpr[32];
>> + unsigned long nip;
>> + unsigned long msr;
>> + unsigned long ctr;
>> + unsigned long link;
>> + unsigned long xer;
>> + unsigned long ccr;
>> + unsigned long trap;
>> +};
>> +
>> +#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
>> + STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* _ASMPPC64_PTRACE_H_ */
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index 424983e..ab2caf6 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
>> cflatobjs += lib/powerpc/hcall.o
>> cflatobjs += lib/powerpc/setup.o
>> cflatobjs += lib/powerpc/rtas.o
>> +cflatobjs += lib/powerpc/processor.o
>>
>> FLATLIBS = $(libcflat) $(LIBFDT_archive)
>> %.elf: CFLAGS += $(arch_CFLAGS)
>> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
>> index c87e3d6..bc5aeac 100644
>> --- a/powerpc/cstart64.S
>> +++ b/powerpc/cstart64.S
>> @@ -9,6 +9,7 @@
>> #include <asm/hcall.h>
>> #include <asm/ppc_asm.h>
>> #include <asm/rtas.h>
>> +#include <asm/ptrace.h>
>>
>> .section .init
>>
>> @@ -45,6 +46,34 @@ start:
>> add r4, r4, r31
>> bl relocate
>>
>> + /* relocate vector table to base address 0x0 (MSR_IP = 0) */
>> +
>> + /* source: r4, dest end: r5, destination: r6 */
>> +
>> + LOAD_REG_ADDR(r4, __start_interrupts)
>> + LOAD_REG_ADDR(r5, __end_interrupts)
>> + sub r5,r5,r4
>> + li r6,0x100
>> +
>> + sub r4,r4,r6
>> + add r5,r5,r6
>> + addi r6,r6,-8
>> +2: li r0,8
>> + mtctr r0
>> + /* copy a cache line size */
>> +3: addi r6,r6,8
>> + ldx r0,r6,r4
>> + stdx r0,0,r6
>> + bdnz 3b
>> + dcbst 0,r6
>> + /* flush icache */
>> + sync
>> + icbi 0,r6
>> + cmpld 0,r6,r5
>> + blt 2b
>> + sync
>> + isync
>> +
>> /* patch sc1 if needed */
>> bl hcall_have_broken_sc1
>> cmpwi r3, 0
>> @@ -105,3 +134,110 @@ rtas_return_loc:
>> ld r0, 16(r1)
>> mtlr r0
>> blr
>> +
>> +call_handler:
>> + /* save context */
>> +
>> + /* GPRs */
>> +
>> + .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
>> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
>> + SAVE_GPR(\i, r1)
>> + .endr
>> + mfsprg1 r0
>> + std r0,GPR1(r1)
>> +
>> + /* lr, xer, ccr */
>> +
>> + mflr r0
>> + std r0,_LINK(r1)
>> +
>> + mfxer r0
>> + std r0,_XER(r1)
>> +
>> + mfcr r0
>> + std r0,_CCR(r1)
>> +
>> + /* nip and msr */
>> +
>> + mfsrr0 r0
>> + std r0, _NIP(r1)
>> +
>> + mfsrr1 r0
>> + std r0, _MSR(r1)
>> +
>> + /* FIXME: build stack frame */
>> +
>> + /* call generic handler */
>> +
>> + addi r3,r1,STACK_FRAME_OVERHEAD
>> + bl do_handle_exception
>> +
>> + /* restore context */
>> +
>> + ld r0,_CTR(r1)
>> + mtctr r0
>> +
>> + ld r0,_LINK(r1)
>> + mtlr r0
>> +
>> + ld r0,_XER(r1)
>> + mtxer r0
>> +
>> + ld r0,_CCR(r1)
>> + mtcr r0
>> +
>> + ld r0, _NIP(r1)
>> + mtsrr0 r0
>> +
>> + ld r0, _MSR(r1)
>> + mtsrr1 r0
>> +
>> + .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
>> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
>
> The fact that '1' has to go last in this list is pretty subtle. I
> wonder if it might be clearer to split that out explicitly rather than
> including it in the .irp
>
>> + REST_GPR(\i, r1)
If I have to resend the series, I will.
>
>
>> + .endr
>> +
>> + rfid
>> + b .
>> +
>> +.section .text.ex
>> +
>> +.macro VECTOR vec
>> + . = \vec
>> +
>> + mtsprg1 r1 /* save r1 */
>> + mfsprg0 r1 /* get exception stack address */
>> + subi r1,r1, INT_FRAME_SIZE
>> +
>> + /* save r0 and ctr to call generic handler */
>> +
>> + SAVE_GPR(0,r1)
>> +
>> + mfctr r0
>> + std r0,_CTR(r1)
>> +
>> + LOAD_REG_ADDR(r0, call_handler)
>> + mtctr r0
>> +
>> + li r0,\vec
>> + std r0,_TRAP(r1)
>> +
>> + bctr
>> +.endm
>> +
>> + . = 0x100
>> + .globl __start_interrupts
>> +__start_interrupts:
>> +
>> +VECTOR(0x300)
>> +VECTOR(0x400)
>> +VECTOR(0x500)
>> +VECTOR(0x600)
>> +VECTOR(0x700)
>> +VECTOR(0x800)
>> +VECTOR(0x900)
>> +
>> + .align 7
>> + .globl __end_interrupts
>> +__end_interrupts:
>> --
>> 2.5.0
>>
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
@ 2016-03-18 8:53 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 8:53 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
On 18/03/2016 04:59, David Gibson wrote:
> On Wed, 16 Mar 2016 16:12:59 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>
>
>> ---
>> lib/powerpc/asm/hcall.h | 1 +
>> lib/powerpc/asm/ppc_asm.h | 5 ++
>> lib/powerpc/asm/processor.h | 11 ++++
>> lib/powerpc/processor.c | 38 +++++++++++++
>> lib/powerpc/setup.c | 19 +++++++
>> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
>> lib/ppc64/asm/processor.h | 1 +
>> lib/ppc64/asm/ptrace.h | 24 ++++++++
>> powerpc/Makefile.common | 1 +
>> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
>> 10 files changed, 278 insertions(+)
>> create mode 100644 lib/powerpc/asm/processor.h
>> create mode 100644 lib/powerpc/processor.c
>> create mode 100644 lib/ppc64/asm/processor.h
>> create mode 100644 lib/ppc64/asm/ptrace.h
>>
>> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
>> index f6f9ea8..99bce79 100644
>> --- a/lib/powerpc/asm/hcall.h
>> +++ b/lib/powerpc/asm/hcall.h
>> @@ -20,6 +20,7 @@
>> #define H_PAGE_INIT 0x2c
>> #define H_PUT_TERM_CHAR 0x58
>> #define H_RANDOM 0x300
>> +#define H_SET_MODE 0x31C
>>
>> #ifndef __ASSEMBLY__
>> /*
>> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
>> index f18100e..39620a3 100644
>> --- a/lib/powerpc/asm/ppc_asm.h
>> +++ b/lib/powerpc/asm/ppc_asm.h
>> @@ -1,6 +1,11 @@
>> #ifndef _ASMPOWERPC_PPC_ASM_H
>> #define _ASMPOWERPC_PPC_ASM_H
>>
>> +#include <asm/asm-offsets.h>
>> +
>> +#define SAVE_GPR(n, base) std n,GPR0+8*(n)(base)
>> +#define REST_GPR(n, base) ld n,GPR0+8*(n)(base)
>> +
>> #define LOAD_REG_IMMEDIATE(reg,expr) \
>> lis reg,(expr)@highest; \
>> ori reg,reg,(expr)@higher; \
>> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
>> new file mode 100644
>> index 0000000..09692bd
>> --- /dev/null
>> +++ b/lib/powerpc/asm/processor.h
>> @@ -0,0 +1,11 @@
>> +#ifndef _ASMPOWERPC_PROCESSOR_H_
>> +#define _ASMPOWERPC_PROCESSOR_H_
>> +
>> +#include <asm/ptrace.h>
>> +
>> +#ifndef __ASSEMBLY__
>> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
>> +void do_handle_exception(struct pt_regs *regs);
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* _ASMPOWERPC_PROCESSOR_H_ */
>> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
>> new file mode 100644
>> index 0000000..a78bc3c
>> --- /dev/null
>> +++ b/lib/powerpc/processor.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * processor control and status function
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/processor.h>
>> +#include <asm/ptrace.h>
>> +
>> +static struct {
>> + void (*func)(struct pt_regs *, void *data);
>> + void *data;
>> +} handlers[16];
>> +
>> +void handle_exception(int trap, void (*func)(struct pt_regs *, void *),
>> + void * data)
>> +{
>> + trap >>= 8;
>
> I'm assuming trap starts out as the vector address (e.g. 0x300 for
> DSI). Using trap >> 8 is going to break when we want to handle, say,
> the SLB miss exceptions at 0x380 and 0x480, which we'll probably want
> to do.
>
> I think there are a few exceptions with even smaller spacing, but we
> might be able to ignore those for a while.
In fact, the handler is registered on (trap >> 8) (for instance 0x3) but
in regs->trap we have the full value (for instance 0x380), so we can
have sub-handler for these particular values.
>
>> + if (trap < 16) {
>> + handlers[trap].func = func;
>> + handlers[trap].data = data;
>> + }
>> +}
>> +
>> +void do_handle_exception(struct pt_regs *regs)
>> +{
>> + unsigned char v;
>> +
>> + v = regs->trap >> 8;
>> +
>> + if (v < 16 && handlers[v].func) {
>> + handlers[v].func(regs, handlers[v].data);
>> + return;
>> + }
>> +
>> + printf("unhandled cpu exception 0x%lx\n", regs->trap);
>> + abort();
>> +}
>> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
>> index 0c0c882..afe7fbc 100644
>> --- a/lib/powerpc/setup.c
>> +++ b/lib/powerpc/setup.c
>> @@ -16,6 +16,8 @@
>> #include <alloc.h>
>> #include <asm/setup.h>
>> #include <asm/page.h>
>> +#include <asm/ppc_asm.h>
>> +#include <asm/hcall.h>
>>
>> extern unsigned long stacktop;
>> extern void io_init(void);
>> @@ -33,6 +35,10 @@ struct cpu_set_params {
>> unsigned dcache_bytes;
>> };
>>
>> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
>> +
>> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
>> +
>> static void cpu_set(int fdtnode, u32 regval, void *info)
>> {
>> static bool read_common_info = false;
>> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
>> }
>> cpus[cpu] = regval;
>>
>> + /* set exception stack address for this CPU (in SPGR0) */
>> +
>> + asm volatile ("mtsprg0 %[addr]" ::
>> + [addr] "r" (exception_stack + cpu + 1));
>> +
>> if (!read_common_info) {
>> const struct fdt_property *prop;
>> u32 *data;
>> @@ -76,6 +87,14 @@ static void cpu_init(void)
>> assert(ret = 0);
>> __icache_bytes = params.icache_bytes;
>> __dcache_bytes = params.dcache_bytes;
>> +
>> + /* Interrupt Endianness */
>> +
>> +#if __BYTE_ORDER__ = __ORDER_LITTLE_ENDIAN__
>> + hcall(H_SET_MODE, 1, 4, 0, 0);
>> +#else
>> + hcall(H_SET_MODE, 0, 4, 0, 0);
>> +#endif
>> }
>>
>> static void mem_init(phys_addr_t freemem_start)
>> diff --git a/lib/ppc64/asm-offsets.c b/lib/ppc64/asm-offsets.c
>> index 2d38a71..7843a20 100644
>> --- a/lib/ppc64/asm-offsets.c
>> +++ b/lib/ppc64/asm-offsets.c
>> @@ -5,8 +5,50 @@
>> */
>> #include <libcflat.h>
>> #include <kbuild.h>
>> +#include <asm/ptrace.h>
>>
>> int main(void)
>> {
>> + DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
>> +
>> + DEFINE(GPR0, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[0]));
>> + DEFINE(GPR1, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[1]));
>> + DEFINE(GPR2, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[2]));
>> + DEFINE(GPR3, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[3]));
>> + DEFINE(GPR4, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[4]));
>> + DEFINE(GPR5, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[5]));
>> + DEFINE(GPR6, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[6]));
>> + DEFINE(GPR7, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[7]));
>> + DEFINE(GPR8, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[8]));
>> + DEFINE(GPR9, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[9]));
>> + DEFINE(GPR10, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[10]));
>> + DEFINE(GPR11, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[11]));
>> + DEFINE(GPR12, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[12]));
>> + DEFINE(GPR13, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[13]));
>> + DEFINE(GPR14, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[14]));
>> + DEFINE(GPR15, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[15]));
>> + DEFINE(GPR16, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[16]));
>> + DEFINE(GPR17, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[17]));
>> + DEFINE(GPR18, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[18]));
>> + DEFINE(GPR19, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[19]));
>> + DEFINE(GPR20, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[20]));
>> + DEFINE(GPR21, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[21]));
>> + DEFINE(GPR22, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[22]));
>> + DEFINE(GPR23, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[23]));
>> + DEFINE(GPR24, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[24]));
>> + DEFINE(GPR25, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[25]));
>> + DEFINE(GPR26, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[26]));
>> + DEFINE(GPR27, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[27]));
>> + DEFINE(GPR28, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[28]));
>> + DEFINE(GPR29, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[29]));
>> + DEFINE(GPR30, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[30]));
>> + DEFINE(GPR31, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, gpr[31]));
>> + DEFINE(_NIP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, nip));
>> + DEFINE(_MSR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, msr));
>> + DEFINE(_CTR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ctr));
>> + DEFINE(_LINK, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, link));
>> + DEFINE(_XER, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, xer));
>> + DEFINE(_CCR, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, ccr));
>> + DEFINE(_TRAP, STACK_FRAME_OVERHEAD+offsetof(struct pt_regs, trap));
>> return 0;
>> }
>> diff --git a/lib/ppc64/asm/processor.h b/lib/ppc64/asm/processor.h
>> new file mode 100644
>> index 0000000..066a51a
>> --- /dev/null
>> +++ b/lib/ppc64/asm/processor.h
>> @@ -0,0 +1 @@
>> +#include "../../powerpc/asm/processor.h"
>> diff --git a/lib/ppc64/asm/ptrace.h b/lib/ppc64/asm/ptrace.h
>> new file mode 100644
>> index 0000000..076c9d9
>> --- /dev/null
>> +++ b/lib/ppc64/asm/ptrace.h
>> @@ -0,0 +1,24 @@
>> +#ifndef _ASMPPC64_PTRACE_H_
>> +#define _ASMPPC64_PTRACE_H_
>> +
>> +#define KERNEL_REDZONE_SIZE 288
>> +#define STACK_FRAME_OVERHEAD 112 /* size of minimum stack frame */
>> +
>> +#ifndef __ASSEMBLY__
>> +struct pt_regs {
>> + unsigned long gpr[32];
>> + unsigned long nip;
>> + unsigned long msr;
>> + unsigned long ctr;
>> + unsigned long link;
>> + unsigned long xer;
>> + unsigned long ccr;
>> + unsigned long trap;
>> +};
>> +
>> +#define STACK_INT_FRAME_SIZE (sizeof(struct pt_regs) + \
>> + STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* _ASMPPC64_PTRACE_H_ */
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index 424983e..ab2caf6 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
>> cflatobjs += lib/powerpc/hcall.o
>> cflatobjs += lib/powerpc/setup.o
>> cflatobjs += lib/powerpc/rtas.o
>> +cflatobjs += lib/powerpc/processor.o
>>
>> FLATLIBS = $(libcflat) $(LIBFDT_archive)
>> %.elf: CFLAGS += $(arch_CFLAGS)
>> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
>> index c87e3d6..bc5aeac 100644
>> --- a/powerpc/cstart64.S
>> +++ b/powerpc/cstart64.S
>> @@ -9,6 +9,7 @@
>> #include <asm/hcall.h>
>> #include <asm/ppc_asm.h>
>> #include <asm/rtas.h>
>> +#include <asm/ptrace.h>
>>
>> .section .init
>>
>> @@ -45,6 +46,34 @@ start:
>> add r4, r4, r31
>> bl relocate
>>
>> + /* relocate vector table to base address 0x0 (MSR_IP = 0) */
>> +
>> + /* source: r4, dest end: r5, destination: r6 */
>> +
>> + LOAD_REG_ADDR(r4, __start_interrupts)
>> + LOAD_REG_ADDR(r5, __end_interrupts)
>> + sub r5,r5,r4
>> + li r6,0x100
>> +
>> + sub r4,r4,r6
>> + add r5,r5,r6
>> + addi r6,r6,-8
>> +2: li r0,8
>> + mtctr r0
>> + /* copy a cache line size */
>> +3: addi r6,r6,8
>> + ldx r0,r6,r4
>> + stdx r0,0,r6
>> + bdnz 3b
>> + dcbst 0,r6
>> + /* flush icache */
>> + sync
>> + icbi 0,r6
>> + cmpld 0,r6,r5
>> + blt 2b
>> + sync
>> + isync
>> +
>> /* patch sc1 if needed */
>> bl hcall_have_broken_sc1
>> cmpwi r3, 0
>> @@ -105,3 +134,110 @@ rtas_return_loc:
>> ld r0, 16(r1)
>> mtlr r0
>> blr
>> +
>> +call_handler:
>> + /* save context */
>> +
>> + /* GPRs */
>> +
>> + .irp i, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
>> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31
>> + SAVE_GPR(\i, r1)
>> + .endr
>> + mfsprg1 r0
>> + std r0,GPR1(r1)
>> +
>> + /* lr, xer, ccr */
>> +
>> + mflr r0
>> + std r0,_LINK(r1)
>> +
>> + mfxer r0
>> + std r0,_XER(r1)
>> +
>> + mfcr r0
>> + std r0,_CCR(r1)
>> +
>> + /* nip and msr */
>> +
>> + mfsrr0 r0
>> + std r0, _NIP(r1)
>> +
>> + mfsrr1 r0
>> + std r0, _MSR(r1)
>> +
>> + /* FIXME: build stack frame */
>> +
>> + /* call generic handler */
>> +
>> + addi r3,r1,STACK_FRAME_OVERHEAD
>> + bl do_handle_exception
>> +
>> + /* restore context */
>> +
>> + ld r0,_CTR(r1)
>> + mtctr r0
>> +
>> + ld r0,_LINK(r1)
>> + mtlr r0
>> +
>> + ld r0,_XER(r1)
>> + mtxer r0
>> +
>> + ld r0,_CCR(r1)
>> + mtcr r0
>> +
>> + ld r0, _NIP(r1)
>> + mtsrr0 r0
>> +
>> + ld r0, _MSR(r1)
>> + mtsrr1 r0
>> +
>> + .irp i, 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 \
>> + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 1
>
> The fact that '1' has to go last in this list is pretty subtle. I
> wonder if it might be clearer to split that out explicitly rather than
> including it in the .irp
>
>> + REST_GPR(\i, r1)
If I have to resend the series, I will.
>
>
>> + .endr
>> +
>> + rfid
>> + b .
>> +
>> +.section .text.ex
>> +
>> +.macro VECTOR vec
>> + . = \vec
>> +
>> + mtsprg1 r1 /* save r1 */
>> + mfsprg0 r1 /* get exception stack address */
>> + subi r1,r1, INT_FRAME_SIZE
>> +
>> + /* save r0 and ctr to call generic handler */
>> +
>> + SAVE_GPR(0,r1)
>> +
>> + mfctr r0
>> + std r0,_CTR(r1)
>> +
>> + LOAD_REG_ADDR(r0, call_handler)
>> + mtctr r0
>> +
>> + li r0,\vec
>> + std r0,_TRAP(r1)
>> +
>> + bctr
>> +.endm
>> +
>> + . = 0x100
>> + .globl __start_interrupts
>> +__start_interrupts:
>> +
>> +VECTOR(0x300)
>> +VECTOR(0x400)
>> +VECTOR(0x500)
>> +VECTOR(0x600)
>> +VECTOR(0x700)
>> +VECTOR(0x800)
>> +VECTOR(0x900)
>> +
>> + .align 7
>> + .globl __end_interrupts
>> +__end_interrupts:
>> --
>> 2.5.0
>>
>
>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-16 15:13 ` Laurent Vivier
@ 2016-03-18 9:09 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:09 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 148 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index b66c1d7..dfe5859 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -45,6 +45,153 @@ static void test_64bit(void)
> report_prefix_pop();
> }
>
> +/*
> + * lswx: Load String Word Indexed X-form
> + *
> + * lswx RT,RA,RB
> + *
> + * EA = (RA|0) + RB
> + * n = XER
> + *
> + * Load n bytes from address EA into (n / 4) consecutive registers,
> + * throught RT -> RT + (n / 4) - 1.
> + * - Data are loaded into 4 low order bytes of registers (Word).
> + * - The unfilled bytes are set to 0.
> + * - The sequence of registers wraps around to GPR0.
> + * - if n == 0, content of RT is undefined
> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
> + * - RT == RA == 0 is invalid
> + *
> + */
> +
> +#define SPR_XER 1
> +
> +static void test_lswx(void)
> +{
> + int i;
> + char addr[128];
> + uint64_t regs[32];
> +
> + report_prefix_push("lswx");
> +
> + /* fill memory with sequence */
> +
> + for (i = 0; i < 128; i++)
> + addr[i] = 1 + i;
> +
> + /* check incomplete register filling */
> +
> + asm volatile ("mtspr %[XER], %[len];"
It's maybe simpler to use the "mtxer" opcode alias here, then you don't
have to pass SPR_XER via the parameters.
> + "li r12,-1;"
> + "mr r11, r12;"
> + "lswx r11, 0, %[addr];"
> + "std r11, 0*8(%[regs]);"
> + "std r12, 1*8(%[regs]);"
> + ::
> + [len] "r" (3),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
Is that comment a copy-n-paste leftover? It seems not to make much sense
here!?
> + "xer", "r11", "r12");
I think you need "memory" in the clobber list, since you write to the
regs buffer.
> + report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
> +
> + /* check an old know bug: the number of bytes is used as
> + * the number of registers, so try 32 bytes.
> + */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + "li r19,-1;"
> + "mr r11, r19; mr r12, r19; mr r13, r19;"
> + "mr r14, r19; mr r15, r19; mr r16, r19;"
> + "mr r17, r19; mr r18, r19;"
> + "lswx r11, 0, %[addr];"
> + "std r11, 0*8(%[regs]);"
> + "std r12, 1*8(%[regs]);"
> + "std r13, 2*8(%[regs]);"
> + "std r14, 3*8(%[regs]);"
> + "std r15, 4*8(%[regs]);"
> + "std r16, 5*8(%[regs]);"
> + "std r17, 6*8(%[regs]);"
> + "std r18, 7*8(%[regs]);"
> + "std r19, 8*8(%[regs]);"
> + ::
> + [len] "r" (32),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
... from r11 instead of r1 ?
> + */
> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
> + "r18", "r19");
Please also add "memory" here.
> + report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
> + regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
> + regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
> + regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
> + regs[8] == (uint64_t)-1);
> +
> + /* check wrap around to r0 */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + "li r31,-1;"
> + "mr r0, r31;"
> + "lswx r31, 0, %[addr];"
> + "std r31, 0*8(%[regs]);"
> + "std r0, 1*8(%[regs]);"
> + ::
> + [len] "r" (8),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
Comment also needs to be fixed?
> + "xer", "r31", "r0");
"memory" missing again
> + report("wrap around to r0", regs[0] == 0x01020304 &&
> + regs[1] == 0x05060708);
> +
> + /* check wrap around to r0 over RB doesn't break RB */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + /* adding r1 in the clobber list doesn't protect it... */
> + "mr r29,r1;"
> + "li r31,-1;"
> + "mr r1,r31;"
> + "mr r0, %[addr];"
> + "lswx r31, 0, r0;"
> + "std r31, 0*8(%[regs]);"
> + "std r0, 1*8(%[regs]);"
> + "std r1, 2*8(%[regs]);"
> + "mr r1,r29;"
> + ::
> + [len] "r" (12),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
That comment needs some update, too.
> + "xer", "r31", "r0", "r29");
"memory"
> + /* doc says it is invalid, real proc stops when it comes to
> + * overwrite the register.
> + * In all the cases, the register must stay untouched
> + */
> + report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
Huh, how can this KVM unit test ever finish successfully if real
processor stops? Should this last test maybe be optional and only be
triggered if this kvm-unit-test is run with certain parameters?
> + report_prefix_pop();
> +}
> +
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> @@ -53,6 +200,7 @@ int main(void)
>
> test_64bit();
> test_illegal();
> + test_lswx();
>
> report_prefix_pop();
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-18 9:09 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:09 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 148 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index b66c1d7..dfe5859 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -45,6 +45,153 @@ static void test_64bit(void)
> report_prefix_pop();
> }
>
> +/*
> + * lswx: Load String Word Indexed X-form
> + *
> + * lswx RT,RA,RB
> + *
> + * EA = (RA|0) + RB
> + * n = XER
> + *
> + * Load n bytes from address EA into (n / 4) consecutive registers,
> + * throught RT -> RT + (n / 4) - 1.
> + * - Data are loaded into 4 low order bytes of registers (Word).
> + * - The unfilled bytes are set to 0.
> + * - The sequence of registers wraps around to GPR0.
> + * - if n = 0, content of RT is undefined
> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
> + * - RT = RA = 0 is invalid
> + *
> + */
> +
> +#define SPR_XER 1
> +
> +static void test_lswx(void)
> +{
> + int i;
> + char addr[128];
> + uint64_t regs[32];
> +
> + report_prefix_push("lswx");
> +
> + /* fill memory with sequence */
> +
> + for (i = 0; i < 128; i++)
> + addr[i] = 1 + i;
> +
> + /* check incomplete register filling */
> +
> + asm volatile ("mtspr %[XER], %[len];"
It's maybe simpler to use the "mtxer" opcode alias here, then you don't
have to pass SPR_XER via the parameters.
> + "li r12,-1;"
> + "mr r11, r12;"
> + "lswx r11, 0, %[addr];"
> + "std r11, 0*8(%[regs]);"
> + "std r12, 1*8(%[regs]);"
> + ::
> + [len] "r" (3),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
Is that comment a copy-n-paste leftover? It seems not to make much sense
here!?
> + "xer", "r11", "r12");
I think you need "memory" in the clobber list, since you write to the
regs buffer.
> + report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
> +
> + /* check an old know bug: the number of bytes is used as
> + * the number of registers, so try 32 bytes.
> + */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + "li r19,-1;"
> + "mr r11, r19; mr r12, r19; mr r13, r19;"
> + "mr r14, r19; mr r15, r19; mr r16, r19;"
> + "mr r17, r19; mr r18, r19;"
> + "lswx r11, 0, %[addr];"
> + "std r11, 0*8(%[regs]);"
> + "std r12, 1*8(%[regs]);"
> + "std r13, 2*8(%[regs]);"
> + "std r14, 3*8(%[regs]);"
> + "std r15, 4*8(%[regs]);"
> + "std r16, 5*8(%[regs]);"
> + "std r17, 6*8(%[regs]);"
> + "std r18, 7*8(%[regs]);"
> + "std r19, 8*8(%[regs]);"
> + ::
> + [len] "r" (32),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
... from r11 instead of r1 ?
> + */
> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
> + "r18", "r19");
Please also add "memory" here.
> + report("length", regs[0] = 0x01020304 && regs[1] = 0x05060708 &&
> + regs[2] = 0x090a0b0c && regs[3] = 0x0d0e0f10 &&
> + regs[4] = 0x11121314 && regs[5] = 0x15161718 &&
> + regs[6] = 0x191a1b1c && regs[7] = 0x1d1e1f20 &&
> + regs[8] = (uint64_t)-1);
> +
> + /* check wrap around to r0 */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + "li r31,-1;"
> + "mr r0, r31;"
> + "lswx r31, 0, %[addr];"
> + "std r31, 0*8(%[regs]);"
> + "std r0, 1*8(%[regs]);"
> + ::
> + [len] "r" (8),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
Comment also needs to be fixed?
> + "xer", "r31", "r0");
"memory" missing again
> + report("wrap around to r0", regs[0] = 0x01020304 &&
> + regs[1] = 0x05060708);
> +
> + /* check wrap around to r0 over RB doesn't break RB */
> +
> + asm volatile ("mtspr %[XER], %[len];"
> + /* adding r1 in the clobber list doesn't protect it... */
> + "mr r29,r1;"
> + "li r31,-1;"
> + "mr r1,r31;"
> + "mr r0, %[addr];"
> + "lswx r31, 0, r0;"
> + "std r31, 0*8(%[regs]);"
> + "std r0, 1*8(%[regs]);"
> + "std r1, 2*8(%[regs]);"
> + "mr r1,r29;"
> + ::
> + [len] "r" (12),
> + [XER] "i" (SPR_XER),
> + [addr] "r" (addr),
> + [regs] "r" (regs)
> + :
> + /* as 32 is the number of bytes,
> + * we should modify 32/4 = 8 regs, from r1
> + */
That comment needs some update, too.
> + "xer", "r31", "r0", "r29");
"memory"
> + /* doc says it is invalid, real proc stops when it comes to
> + * overwrite the register.
> + * In all the cases, the register must stay untouched
> + */
> + report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
Huh, how can this KVM unit test ever finish successfully if real
processor stops? Should this last test maybe be optional and only be
triggered if this kvm-unit-test is run with certain parameters?
> + report_prefix_pop();
> +}
> +
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> @@ -53,6 +200,7 @@ int main(void)
>
> test_64bit();
> test_illegal();
> + test_lswx();
>
> report_prefix_pop();
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
2016-03-16 15:12 ` Laurent Vivier
@ 2016-03-18 9:42 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:42 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:12, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> lib/powerpc/asm/hcall.h | 1 +
> lib/powerpc/asm/ppc_asm.h | 5 ++
> lib/powerpc/asm/processor.h | 11 ++++
> lib/powerpc/processor.c | 38 +++++++++++++
> lib/powerpc/setup.c | 19 +++++++
> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
> lib/ppc64/asm/processor.h | 1 +
> lib/ppc64/asm/ptrace.h | 24 ++++++++
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 278 insertions(+)
> create mode 100644 lib/powerpc/asm/processor.h
> create mode 100644 lib/powerpc/processor.c
> create mode 100644 lib/ppc64/asm/processor.h
> create mode 100644 lib/ppc64/asm/ptrace.h
...
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 0c0c882..afe7fbc 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -16,6 +16,8 @@
> #include <alloc.h>
> #include <asm/setup.h>
> #include <asm/page.h>
> +#include <asm/ppc_asm.h>
> +#include <asm/hcall.h>
>
> extern unsigned long stacktop;
> extern void io_init(void);
> @@ -33,6 +35,10 @@ struct cpu_set_params {
> unsigned dcache_bytes;
> };
>
> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
> +
> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
> +
> static void cpu_set(int fdtnode, u32 regval, void *info)
> {
> static bool read_common_info = false;
> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> }
> cpus[cpu] = regval;
>
> + /* set exception stack address for this CPU (in SPGR0) */
> +
> + asm volatile ("mtsprg0 %[addr]" ::
> + [addr] "r" (exception_stack + cpu + 1));
Maybe use "exception_stack[cpu + 1]" instead? That's easier to read.
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] powerpc: add exception handler
@ 2016-03-18 9:42 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:42 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:12, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> lib/powerpc/asm/hcall.h | 1 +
> lib/powerpc/asm/ppc_asm.h | 5 ++
> lib/powerpc/asm/processor.h | 11 ++++
> lib/powerpc/processor.c | 38 +++++++++++++
> lib/powerpc/setup.c | 19 +++++++
> lib/ppc64/asm-offsets.c | 42 ++++++++++++++
> lib/ppc64/asm/processor.h | 1 +
> lib/ppc64/asm/ptrace.h | 24 ++++++++
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 136 ++++++++++++++++++++++++++++++++++++++++++++
> 10 files changed, 278 insertions(+)
> create mode 100644 lib/powerpc/asm/processor.h
> create mode 100644 lib/powerpc/processor.c
> create mode 100644 lib/ppc64/asm/processor.h
> create mode 100644 lib/ppc64/asm/ptrace.h
...
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index 0c0c882..afe7fbc 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -16,6 +16,8 @@
> #include <alloc.h>
> #include <asm/setup.h>
> #include <asm/page.h>
> +#include <asm/ppc_asm.h>
> +#include <asm/hcall.h>
>
> extern unsigned long stacktop;
> extern void io_init(void);
> @@ -33,6 +35,10 @@ struct cpu_set_params {
> unsigned dcache_bytes;
> };
>
> +#define EXCEPTION_STACK_SIZE (32*1024) /* 32kB */
> +
> +static char exception_stack[NR_CPUS][EXCEPTION_STACK_SIZE];
> +
> static void cpu_set(int fdtnode, u32 regval, void *info)
> {
> static bool read_common_info = false;
> @@ -46,6 +52,11 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> }
> cpus[cpu] = regval;
>
> + /* set exception stack address for this CPU (in SPGR0) */
> +
> + asm volatile ("mtsprg0 %[addr]" ::
> + [addr] "r" (exception_stack + cpu + 1));
Maybe use "exception_stack[cpu + 1]" instead? That's easier to read.
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
2016-03-16 15:13 ` Laurent Vivier
@ 2016-03-18 9:51 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:51 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/Makefile.common | 5 ++++-
> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 3 +++
> 3 files changed, 53 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/emulator.c
>
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index ab2caf6..257e3fb 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -7,7 +7,8 @@
> tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> - $(TEST_DIR)/rtas.elf
> + $(TEST_DIR)/rtas.elf \
> + $(TEST_DIR)/emulator.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>
> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
> +
> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> new file mode 100644
> index 0000000..1215c4f
> --- /dev/null
> +++ b/powerpc/emulator.c
> @@ -0,0 +1,46 @@
> +/*
> + * Test some powerpc instructions
> + */
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +
> +static int volatile is_invalid;
> +
> +static void program_check_handler(struct pt_regs *regs, void *opaque)
> +{
> + int *data = opaque;
> +
> + printf("Detected invalid instruction 0x%016lx: %08x\n",
> + regs->nip, *(uint32_t*)regs->nip);
Should the test always print this, or is this rather confusing for the
users? Then it should maybe be commented out by default, I think.
> + *data = 1;
> +
> + regs->nip += 4;
> +}
> +
> +static void test_illegal(void)
> +{
> + report_prefix_push("invalid");
> +
> + is_invalid = 0;
> +
> + asm volatile (".long 0");
> +
> + report("exception", is_invalid);
> +
> + report_prefix_pop();
> +}
> +
> +int main(void)
> +{
> + handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> +
> + report_prefix_push("emulator");
> +
> + test_illegal();
> +
> + report_prefix_pop();
> +
> + return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 02b21c7..ed4fdbe 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -47,3 +47,6 @@ file = rtas.elf
> extra_params = -append "set-time-of-day"
> timeout = 5
> groups = rtas
> +
> +[emulator]
> +file = emulator.elf
Reviewed-by: Thomas Huth <thuth@redhat.com>
BTW: I think you could optionally also check the contents of SRR1 for
sane values in test_illegal(), in case you want to improve the test a
little bit.
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
@ 2016-03-18 9:51 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:51 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/Makefile.common | 5 ++++-
> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 3 +++
> 3 files changed, 53 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/emulator.c
>
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index ab2caf6..257e3fb 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -7,7 +7,8 @@
> tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> - $(TEST_DIR)/rtas.elf
> + $(TEST_DIR)/rtas.elf \
> + $(TEST_DIR)/emulator.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>
> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
> +
> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> new file mode 100644
> index 0000000..1215c4f
> --- /dev/null
> +++ b/powerpc/emulator.c
> @@ -0,0 +1,46 @@
> +/*
> + * Test some powerpc instructions
> + */
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +
> +static int volatile is_invalid;
> +
> +static void program_check_handler(struct pt_regs *regs, void *opaque)
> +{
> + int *data = opaque;
> +
> + printf("Detected invalid instruction 0x%016lx: %08x\n",
> + regs->nip, *(uint32_t*)regs->nip);
Should the test always print this, or is this rather confusing for the
users? Then it should maybe be commented out by default, I think.
> + *data = 1;
> +
> + regs->nip += 4;
> +}
> +
> +static void test_illegal(void)
> +{
> + report_prefix_push("invalid");
> +
> + is_invalid = 0;
> +
> + asm volatile (".long 0");
> +
> + report("exception", is_invalid);
> +
> + report_prefix_pop();
> +}
> +
> +int main(void)
> +{
> + handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> +
> + report_prefix_push("emulator");
> +
> + test_illegal();
> +
> + report_prefix_pop();
> +
> + return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 02b21c7..ed4fdbe 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -47,3 +47,6 @@ file = rtas.elf
> extra_params = -append "set-time-of-day"
> timeout = 5
> groups = rtas
> +
> +[emulator]
> +file = emulator.elf
Reviewed-by: Thomas Huth <thuth@redhat.com>
BTW: I think you could optionally also check the contents of SRR1 for
sane values in test_illegal(), in case you want to improve the test a
little bit.
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 5/5] powerpc: Check lswx in little-endian mode.
2016-03-16 15:13 ` Laurent Vivier
@ 2016-03-18 9:54 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:54 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> For lswx in little-endian mode, an alignment interrupt occurs.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index dfe5859..3a8f17e 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -6,6 +6,7 @@
> #include <asm/processor.h>
>
> static int volatile is_invalid;
> +static int volatile alignment;
>
> static void program_check_handler(struct pt_regs *regs, void *opaque)
> {
> @@ -19,6 +20,18 @@ static void program_check_handler(struct pt_regs *regs, void *opaque)
> regs->nip += 4;
> }
>
> +static void alignment_handler(struct pt_regs *regs, void *opaque)
> +{
> + int *data = opaque;
> +
> + printf("Detected alignment exception 0x%016lx: %08x\n",
> + regs->nip, *(uint32_t*)regs->nip);
> +
> + *data = 1;
> +
> + regs->nip += 4;
> +}
> +
> static void test_illegal(void)
> {
> report_prefix_push("invalid");
> @@ -62,6 +75,8 @@ static void test_64bit(void)
> * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
> * - RT == RA == 0 is invalid
> *
> + * For lswx in little-endian mode, an alignment interrupt always occurs.
> + *
> */
>
> #define SPR_XER 1
> @@ -81,6 +96,7 @@ static void test_lswx(void)
>
> /* check incomplete register filling */
>
> + alignment = 0;
> asm volatile ("mtspr %[XER], %[len];"
> "li r12,-1;"
> "mr r11, r12;"
> @@ -98,7 +114,12 @@ static void test_lswx(void)
> */
> "xer", "r11", "r12");
>
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> + report("alignment", alignment);
> + return;
> +#else
> report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
> +#endif
>
> /* check an old know bug: the number of bytes is used as
> * the number of registers, so try 32 bytes.
> @@ -195,6 +216,7 @@ static void test_lswx(void)
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> + handle_exception(0x600, alignment_handler, (void *)&alignment);
>
> report_prefix_push("emulator");
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 5/5] powerpc: Check lswx in little-endian mode.
@ 2016-03-18 9:54 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 9:54 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 16.03.2016 16:13, Laurent Vivier wrote:
> For lswx in little-endian mode, an alignment interrupt occurs.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/emulator.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index dfe5859..3a8f17e 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -6,6 +6,7 @@
> #include <asm/processor.h>
>
> static int volatile is_invalid;
> +static int volatile alignment;
>
> static void program_check_handler(struct pt_regs *regs, void *opaque)
> {
> @@ -19,6 +20,18 @@ static void program_check_handler(struct pt_regs *regs, void *opaque)
> regs->nip += 4;
> }
>
> +static void alignment_handler(struct pt_regs *regs, void *opaque)
> +{
> + int *data = opaque;
> +
> + printf("Detected alignment exception 0x%016lx: %08x\n",
> + regs->nip, *(uint32_t*)regs->nip);
> +
> + *data = 1;
> +
> + regs->nip += 4;
> +}
> +
> static void test_illegal(void)
> {
> report_prefix_push("invalid");
> @@ -62,6 +75,8 @@ static void test_64bit(void)
> * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
> * - RT = RA = 0 is invalid
> *
> + * For lswx in little-endian mode, an alignment interrupt always occurs.
> + *
> */
>
> #define SPR_XER 1
> @@ -81,6 +96,7 @@ static void test_lswx(void)
>
> /* check incomplete register filling */
>
> + alignment = 0;
> asm volatile ("mtspr %[XER], %[len];"
> "li r12,-1;"
> "mr r11, r12;"
> @@ -98,7 +114,12 @@ static void test_lswx(void)
> */
> "xer", "r11", "r12");
>
> +#if __BYTE_ORDER__ = __ORDER_LITTLE_ENDIAN__
> + report("alignment", alignment);
> + return;
> +#else
> report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
> +#endif
>
> /* check an old know bug: the number of bytes is used as
> * the number of registers, so try 32 bytes.
> @@ -195,6 +216,7 @@ static void test_lswx(void)
> int main(void)
> {
> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
> + handle_exception(0x600, alignment_handler, (void *)&alignment);
>
> report_prefix_push("emulator");
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-18 9:09 ` Thomas Huth
@ 2016-03-18 10:01 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:01 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 10:09, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 148 insertions(+)
>>
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> index b66c1d7..dfe5859 100644
>> --- a/powerpc/emulator.c
>> +++ b/powerpc/emulator.c
>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>> report_prefix_pop();
>> }
>>
>> +/*
>> + * lswx: Load String Word Indexed X-form
>> + *
>> + * lswx RT,RA,RB
>> + *
>> + * EA = (RA|0) + RB
>> + * n = XER
>> + *
>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>> + * throught RT -> RT + (n / 4) - 1.
>> + * - Data are loaded into 4 low order bytes of registers (Word).
>> + * - The unfilled bytes are set to 0.
>> + * - The sequence of registers wraps around to GPR0.
>> + * - if n == 0, content of RT is undefined
>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>> + * - RT == RA == 0 is invalid
>> + *
>> + */
>> +
>> +#define SPR_XER 1
>> +
>> +static void test_lswx(void)
>> +{
>> + int i;
>> + char addr[128];
>> + uint64_t regs[32];
>> +
>> + report_prefix_push("lswx");
>> +
>> + /* fill memory with sequence */
>> +
>> + for (i = 0; i < 128; i++)
>> + addr[i] = 1 + i;
>> +
>> + /* check incomplete register filling */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>
> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
> have to pass SPR_XER via the parameters.
OK.
>> + "li r12,-1;"
>> + "mr r11, r12;"
>> + "lswx r11, 0, %[addr];"
>> + "std r11, 0*8(%[regs]);"
>> + "std r12, 1*8(%[regs]);"
>> + ::
>> + [len] "r" (3),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> Is that comment a copy-n-paste leftover? It seems not to make much sense
> here!?
Yes, it's completely wrong...
>> + "xer", "r11", "r12");
>
> I think you need "memory" in the clobber list, since you write to the
> regs buffer.
ok
>> + report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
>> +
>> + /* check an old know bug: the number of bytes is used as
>> + * the number of registers, so try 32 bytes.
>> + */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + "li r19,-1;"
>> + "mr r11, r19; mr r12, r19; mr r13, r19;"
>> + "mr r14, r19; mr r15, r19; mr r16, r19;"
>> + "mr r17, r19; mr r18, r19;"
>> + "lswx r11, 0, %[addr];"
>> + "std r11, 0*8(%[regs]);"
>> + "std r12, 1*8(%[regs]);"
>> + "std r13, 2*8(%[regs]);"
>> + "std r14, 3*8(%[regs]);"
>> + "std r15, 4*8(%[regs]);"
>> + "std r16, 5*8(%[regs]);"
>> + "std r17, 6*8(%[regs]);"
>> + "std r18, 7*8(%[regs]);"
>> + "std r19, 8*8(%[regs]);"
>> + ::
>> + [len] "r" (32),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>
> ... from r11 instead of r1 ?
always a bad cut'n'paste...
>> + */
>> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>> + "r18", "r19");
>
> Please also add "memory" here.
ok
>> + report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
>> + regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
>> + regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
>> + regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
>> + regs[8] == (uint64_t)-1);
>> +
>> + /* check wrap around to r0 */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + "li r31,-1;"
>> + "mr r0, r31;"
>> + "lswx r31, 0, %[addr];"
>> + "std r31, 0*8(%[regs]);"
>> + "std r0, 1*8(%[regs]);"
>> + ::
>> + [len] "r" (8),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> Comment also needs to be fixed?
yes,
>
>> + "xer", "r31", "r0");
>
> "memory" missing again
ok,
>
>> + report("wrap around to r0", regs[0] == 0x01020304 &&
>> + regs[1] == 0x05060708);
>> +
>> + /* check wrap around to r0 over RB doesn't break RB */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + /* adding r1 in the clobber list doesn't protect it... */
>> + "mr r29,r1;"
>> + "li r31,-1;"
>> + "mr r1,r31;"
>> + "mr r0, %[addr];"
>> + "lswx r31, 0, r0;"
>> + "std r31, 0*8(%[regs]);"
>> + "std r0, 1*8(%[regs]);"
>> + "std r1, 2*8(%[regs]);"
>> + "mr r1,r29;"
>> + ::
>> + [len] "r" (12),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> That comment needs some update, too.
yes,
>
>> + "xer", "r31", "r0", "r29");
>
> "memory"
ok
>> + /* doc says it is invalid, real proc stops when it comes to
>> + * overwrite the register.
>> + * In all the cases, the register must stay untouched
>> + */
>> + report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
>
> Huh, how can this KVM unit test ever finish successfully if real
> processor stops? Should this last test maybe be optional and only be
> triggered if this kvm-unit-test is run with certain parameters?
Well, my bad, I've not been accurate: the processor doesn't stop, but
the processing of the instruction is stopped. Only registers until Rb
(not included) are updated, and then the processor continue with the
next instruction. So we have just to test Rb is not modified. I'll
update the comment.
>
>> + report_prefix_pop();
>> +}
>> +
>> int main(void)
>> {
>> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>> @@ -53,6 +200,7 @@ int main(void)
>>
>> test_64bit();
>> test_illegal();
>> + test_lswx();
>>
>> report_prefix_pop();
>
> Thomas
>
Thanks,
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-18 10:01 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:01 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 10:09, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 148 insertions(+)
>>
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> index b66c1d7..dfe5859 100644
>> --- a/powerpc/emulator.c
>> +++ b/powerpc/emulator.c
>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>> report_prefix_pop();
>> }
>>
>> +/*
>> + * lswx: Load String Word Indexed X-form
>> + *
>> + * lswx RT,RA,RB
>> + *
>> + * EA = (RA|0) + RB
>> + * n = XER
>> + *
>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>> + * throught RT -> RT + (n / 4) - 1.
>> + * - Data are loaded into 4 low order bytes of registers (Word).
>> + * - The unfilled bytes are set to 0.
>> + * - The sequence of registers wraps around to GPR0.
>> + * - if n = 0, content of RT is undefined
>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>> + * - RT = RA = 0 is invalid
>> + *
>> + */
>> +
>> +#define SPR_XER 1
>> +
>> +static void test_lswx(void)
>> +{
>> + int i;
>> + char addr[128];
>> + uint64_t regs[32];
>> +
>> + report_prefix_push("lswx");
>> +
>> + /* fill memory with sequence */
>> +
>> + for (i = 0; i < 128; i++)
>> + addr[i] = 1 + i;
>> +
>> + /* check incomplete register filling */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>
> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
> have to pass SPR_XER via the parameters.
OK.
>> + "li r12,-1;"
>> + "mr r11, r12;"
>> + "lswx r11, 0, %[addr];"
>> + "std r11, 0*8(%[regs]);"
>> + "std r12, 1*8(%[regs]);"
>> + ::
>> + [len] "r" (3),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> Is that comment a copy-n-paste leftover? It seems not to make much sense
> here!?
Yes, it's completely wrong...
>> + "xer", "r11", "r12");
>
> I think you need "memory" in the clobber list, since you write to the
> regs buffer.
ok
>> + report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
>> +
>> + /* check an old know bug: the number of bytes is used as
>> + * the number of registers, so try 32 bytes.
>> + */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + "li r19,-1;"
>> + "mr r11, r19; mr r12, r19; mr r13, r19;"
>> + "mr r14, r19; mr r15, r19; mr r16, r19;"
>> + "mr r17, r19; mr r18, r19;"
>> + "lswx r11, 0, %[addr];"
>> + "std r11, 0*8(%[regs]);"
>> + "std r12, 1*8(%[regs]);"
>> + "std r13, 2*8(%[regs]);"
>> + "std r14, 3*8(%[regs]);"
>> + "std r15, 4*8(%[regs]);"
>> + "std r16, 5*8(%[regs]);"
>> + "std r17, 6*8(%[regs]);"
>> + "std r18, 7*8(%[regs]);"
>> + "std r19, 8*8(%[regs]);"
>> + ::
>> + [len] "r" (32),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>
> ... from r11 instead of r1 ?
always a bad cut'n'paste...
>> + */
>> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>> + "r18", "r19");
>
> Please also add "memory" here.
ok
>> + report("length", regs[0] = 0x01020304 && regs[1] = 0x05060708 &&
>> + regs[2] = 0x090a0b0c && regs[3] = 0x0d0e0f10 &&
>> + regs[4] = 0x11121314 && regs[5] = 0x15161718 &&
>> + regs[6] = 0x191a1b1c && regs[7] = 0x1d1e1f20 &&
>> + regs[8] = (uint64_t)-1);
>> +
>> + /* check wrap around to r0 */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + "li r31,-1;"
>> + "mr r0, r31;"
>> + "lswx r31, 0, %[addr];"
>> + "std r31, 0*8(%[regs]);"
>> + "std r0, 1*8(%[regs]);"
>> + ::
>> + [len] "r" (8),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> Comment also needs to be fixed?
yes,
>
>> + "xer", "r31", "r0");
>
> "memory" missing again
ok,
>
>> + report("wrap around to r0", regs[0] = 0x01020304 &&
>> + regs[1] = 0x05060708);
>> +
>> + /* check wrap around to r0 over RB doesn't break RB */
>> +
>> + asm volatile ("mtspr %[XER], %[len];"
>> + /* adding r1 in the clobber list doesn't protect it... */
>> + "mr r29,r1;"
>> + "li r31,-1;"
>> + "mr r1,r31;"
>> + "mr r0, %[addr];"
>> + "lswx r31, 0, r0;"
>> + "std r31, 0*8(%[regs]);"
>> + "std r0, 1*8(%[regs]);"
>> + "std r1, 2*8(%[regs]);"
>> + "mr r1,r29;"
>> + ::
>> + [len] "r" (12),
>> + [XER] "i" (SPR_XER),
>> + [addr] "r" (addr),
>> + [regs] "r" (regs)
>> + :
>> + /* as 32 is the number of bytes,
>> + * we should modify 32/4 = 8 regs, from r1
>> + */
>
> That comment needs some update, too.
yes,
>
>> + "xer", "r31", "r0", "r29");
>
> "memory"
ok
>> + /* doc says it is invalid, real proc stops when it comes to
>> + * overwrite the register.
>> + * In all the cases, the register must stay untouched
>> + */
>> + report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
>
> Huh, how can this KVM unit test ever finish successfully if real
> processor stops? Should this last test maybe be optional and only be
> triggered if this kvm-unit-test is run with certain parameters?
Well, my bad, I've not been accurate: the processor doesn't stop, but
the processing of the instruction is stopped. Only registers until Rb
(not included) are updated, and then the processor continue with the
next instruction. So we have just to test Rb is not modified. I'll
update the comment.
>
>> + report_prefix_pop();
>> +}
>> +
>> int main(void)
>> {
>> handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>> @@ -53,6 +200,7 @@ int main(void)
>>
>> test_64bit();
>> test_illegal();
>> + test_lswx();
>>
>> report_prefix_pop();
>
> Thomas
>
Thanks,
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-18 10:01 ` Laurent Vivier
@ 2016-03-18 10:06 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:06 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:01, Laurent Vivier wrote:
>
>
> On 18/03/2016 10:09, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 148 insertions(+)
>>>
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> index b66c1d7..dfe5859 100644
>>> --- a/powerpc/emulator.c
>>> +++ b/powerpc/emulator.c
>>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>>> report_prefix_pop();
>>> }
>>>
>>> +/*
>>> + * lswx: Load String Word Indexed X-form
>>> + *
>>> + * lswx RT,RA,RB
>>> + *
>>> + * EA = (RA|0) + RB
>>> + * n = XER
>>> + *
>>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>>> + * throught RT -> RT + (n / 4) - 1.
>>> + * - Data are loaded into 4 low order bytes of registers (Word).
>>> + * - The unfilled bytes are set to 0.
>>> + * - The sequence of registers wraps around to GPR0.
>>> + * - if n == 0, content of RT is undefined
>>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>>> + * - RT == RA == 0 is invalid
>>> + *
>>> + */
>>> +
>>> +#define SPR_XER 1
>>> +
>>> +static void test_lswx(void)
>>> +{
>>> + int i;
>>> + char addr[128];
>>> + uint64_t regs[32];
>>> +
>>> + report_prefix_push("lswx");
>>> +
>>> + /* fill memory with sequence */
>>> +
>>> + for (i = 0; i < 128; i++)
>>> + addr[i] = 1 + i;
>>> +
>>> + /* check incomplete register filling */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>
>> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
>> have to pass SPR_XER via the parameters.
>
> OK.
>
>>> + "li r12,-1;"
>>> + "mr r11, r12;"
>>> + "lswx r11, 0, %[addr];"
>>> + "std r11, 0*8(%[regs]);"
>>> + "std r12, 1*8(%[regs]);"
>>> + ::
>>> + [len] "r" (3),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> Is that comment a copy-n-paste leftover? It seems not to make much sense
>> here!?
>
> Yes, it's completely wrong...
>
>>> + "xer", "r11", "r12");
>>
>> I think you need "memory" in the clobber list, since you write to the
>> regs buffer.
>
> ok
>
>>> + report("partial", regs[0] == 0x01020300 && regs[1] == (uint64_t)-1);
>>> +
>>> + /* check an old know bug: the number of bytes is used as
>>> + * the number of registers, so try 32 bytes.
>>> + */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + "li r19,-1;"
>>> + "mr r11, r19; mr r12, r19; mr r13, r19;"
>>> + "mr r14, r19; mr r15, r19; mr r16, r19;"
>>> + "mr r17, r19; mr r18, r19;"
>>> + "lswx r11, 0, %[addr];"
>>> + "std r11, 0*8(%[regs]);"
>>> + "std r12, 1*8(%[regs]);"
>>> + "std r13, 2*8(%[regs]);"
>>> + "std r14, 3*8(%[regs]);"
>>> + "std r15, 4*8(%[regs]);"
>>> + "std r16, 5*8(%[regs]);"
>>> + "std r17, 6*8(%[regs]);"
>>> + "std r18, 7*8(%[regs]);"
>>> + "std r19, 8*8(%[regs]);"
>>> + ::
>>> + [len] "r" (32),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>
>> ... from r11 instead of r1 ?
>
> always a bad cut'n'paste...
>
>>> + */
>>> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>>> + "r18", "r19");
>>
>> Please also add "memory" here.
>
>
> ok
>
>>> + report("length", regs[0] == 0x01020304 && regs[1] == 0x05060708 &&
>>> + regs[2] == 0x090a0b0c && regs[3] == 0x0d0e0f10 &&
>>> + regs[4] == 0x11121314 && regs[5] == 0x15161718 &&
>>> + regs[6] == 0x191a1b1c && regs[7] == 0x1d1e1f20 &&
>>> + regs[8] == (uint64_t)-1);
>>> +
>>> + /* check wrap around to r0 */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + "li r31,-1;"
>>> + "mr r0, r31;"
>>> + "lswx r31, 0, %[addr];"
>>> + "std r31, 0*8(%[regs]);"
>>> + "std r0, 1*8(%[regs]);"
>>> + ::
>>> + [len] "r" (8),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> Comment also needs to be fixed?
>
> yes,
>
>>
>>> + "xer", "r31", "r0");
>>
>> "memory" missing again
>
> ok,
>
>>
>>> + report("wrap around to r0", regs[0] == 0x01020304 &&
>>> + regs[1] == 0x05060708);
>>> +
>>> + /* check wrap around to r0 over RB doesn't break RB */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + /* adding r1 in the clobber list doesn't protect it... */
>>> + "mr r29,r1;"
>>> + "li r31,-1;"
>>> + "mr r1,r31;"
>>> + "mr r0, %[addr];"
>>> + "lswx r31, 0, r0;"
>>> + "std r31, 0*8(%[regs]);"
>>> + "std r0, 1*8(%[regs]);"
>>> + "std r1, 2*8(%[regs]);"
>>> + "mr r1,r29;"
>>> + ::
>>> + [len] "r" (12),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> That comment needs some update, too.
>
> yes,
>
>>
>>> + "xer", "r31", "r0", "r29");
>>
>> "memory"
>
> ok
>
>>> + /* doc says it is invalid, real proc stops when it comes to
>>> + * overwrite the register.
>>> + * In all the cases, the register must stay untouched
>>> + */
>>> + report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
>>
>> Huh, how can this KVM unit test ever finish successfully if real
>> processor stops? Should this last test maybe be optional and only be
>> triggered if this kvm-unit-test is run with certain parameters?
>
> Well, my bad, I've not been accurate: the processor doesn't stop, but
> the processing of the instruction is stopped. Only registers until Rb
> (not included) are updated, and then the processor continue with the
> next instruction. So we have just to test Rb is not modified. I'll
> update the comment.
Ok, now I've got it, I think. Maybe you should also check the
"is_invalid" variable here? And it would also be interesting to see
whether regs[0] (i.e. r31) has been changed or not?
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-18 10:06 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:06 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:01, Laurent Vivier wrote:
>
>
> On 18/03/2016 10:09, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/emulator.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 148 insertions(+)
>>>
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> index b66c1d7..dfe5859 100644
>>> --- a/powerpc/emulator.c
>>> +++ b/powerpc/emulator.c
>>> @@ -45,6 +45,153 @@ static void test_64bit(void)
>>> report_prefix_pop();
>>> }
>>>
>>> +/*
>>> + * lswx: Load String Word Indexed X-form
>>> + *
>>> + * lswx RT,RA,RB
>>> + *
>>> + * EA = (RA|0) + RB
>>> + * n = XER
>>> + *
>>> + * Load n bytes from address EA into (n / 4) consecutive registers,
>>> + * throught RT -> RT + (n / 4) - 1.
>>> + * - Data are loaded into 4 low order bytes of registers (Word).
>>> + * - The unfilled bytes are set to 0.
>>> + * - The sequence of registers wraps around to GPR0.
>>> + * - if n = 0, content of RT is undefined
>>> + * - RT <= RA or RB < RT + (n + 4) is invalid or result is undefined
>>> + * - RT = RA = 0 is invalid
>>> + *
>>> + */
>>> +
>>> +#define SPR_XER 1
>>> +
>>> +static void test_lswx(void)
>>> +{
>>> + int i;
>>> + char addr[128];
>>> + uint64_t regs[32];
>>> +
>>> + report_prefix_push("lswx");
>>> +
>>> + /* fill memory with sequence */
>>> +
>>> + for (i = 0; i < 128; i++)
>>> + addr[i] = 1 + i;
>>> +
>>> + /* check incomplete register filling */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>
>> It's maybe simpler to use the "mtxer" opcode alias here, then you don't
>> have to pass SPR_XER via the parameters.
>
> OK.
>
>>> + "li r12,-1;"
>>> + "mr r11, r12;"
>>> + "lswx r11, 0, %[addr];"
>>> + "std r11, 0*8(%[regs]);"
>>> + "std r12, 1*8(%[regs]);"
>>> + ::
>>> + [len] "r" (3),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> Is that comment a copy-n-paste leftover? It seems not to make much sense
>> here!?
>
> Yes, it's completely wrong...
>
>>> + "xer", "r11", "r12");
>>
>> I think you need "memory" in the clobber list, since you write to the
>> regs buffer.
>
> ok
>
>>> + report("partial", regs[0] = 0x01020300 && regs[1] = (uint64_t)-1);
>>> +
>>> + /* check an old know bug: the number of bytes is used as
>>> + * the number of registers, so try 32 bytes.
>>> + */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + "li r19,-1;"
>>> + "mr r11, r19; mr r12, r19; mr r13, r19;"
>>> + "mr r14, r19; mr r15, r19; mr r16, r19;"
>>> + "mr r17, r19; mr r18, r19;"
>>> + "lswx r11, 0, %[addr];"
>>> + "std r11, 0*8(%[regs]);"
>>> + "std r12, 1*8(%[regs]);"
>>> + "std r13, 2*8(%[regs]);"
>>> + "std r14, 3*8(%[regs]);"
>>> + "std r15, 4*8(%[regs]);"
>>> + "std r16, 5*8(%[regs]);"
>>> + "std r17, 6*8(%[regs]);"
>>> + "std r18, 7*8(%[regs]);"
>>> + "std r19, 8*8(%[regs]);"
>>> + ::
>>> + [len] "r" (32),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>
>> ... from r11 instead of r1 ?
>
> always a bad cut'n'paste...
>
>>> + */
>>> + "xer", "r11", "r12", "r13", "r14", "r15", "r16", "r17",
>>> + "r18", "r19");
>>
>> Please also add "memory" here.
>
>
> ok
>
>>> + report("length", regs[0] = 0x01020304 && regs[1] = 0x05060708 &&
>>> + regs[2] = 0x090a0b0c && regs[3] = 0x0d0e0f10 &&
>>> + regs[4] = 0x11121314 && regs[5] = 0x15161718 &&
>>> + regs[6] = 0x191a1b1c && regs[7] = 0x1d1e1f20 &&
>>> + regs[8] = (uint64_t)-1);
>>> +
>>> + /* check wrap around to r0 */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + "li r31,-1;"
>>> + "mr r0, r31;"
>>> + "lswx r31, 0, %[addr];"
>>> + "std r31, 0*8(%[regs]);"
>>> + "std r0, 1*8(%[regs]);"
>>> + ::
>>> + [len] "r" (8),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> Comment also needs to be fixed?
>
> yes,
>
>>
>>> + "xer", "r31", "r0");
>>
>> "memory" missing again
>
> ok,
>
>>
>>> + report("wrap around to r0", regs[0] = 0x01020304 &&
>>> + regs[1] = 0x05060708);
>>> +
>>> + /* check wrap around to r0 over RB doesn't break RB */
>>> +
>>> + asm volatile ("mtspr %[XER], %[len];"
>>> + /* adding r1 in the clobber list doesn't protect it... */
>>> + "mr r29,r1;"
>>> + "li r31,-1;"
>>> + "mr r1,r31;"
>>> + "mr r0, %[addr];"
>>> + "lswx r31, 0, r0;"
>>> + "std r31, 0*8(%[regs]);"
>>> + "std r0, 1*8(%[regs]);"
>>> + "std r1, 2*8(%[regs]);"
>>> + "mr r1,r29;"
>>> + ::
>>> + [len] "r" (12),
>>> + [XER] "i" (SPR_XER),
>>> + [addr] "r" (addr),
>>> + [regs] "r" (regs)
>>> + :
>>> + /* as 32 is the number of bytes,
>>> + * we should modify 32/4 = 8 regs, from r1
>>> + */
>>
>> That comment needs some update, too.
>
> yes,
>
>>
>>> + "xer", "r31", "r0", "r29");
>>
>> "memory"
>
> ok
>
>>> + /* doc says it is invalid, real proc stops when it comes to
>>> + * overwrite the register.
>>> + * In all the cases, the register must stay untouched
>>> + */
>>> + report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
>>
>> Huh, how can this KVM unit test ever finish successfully if real
>> processor stops? Should this last test maybe be optional and only be
>> triggered if this kvm-unit-test is run with certain parameters?
>
> Well, my bad, I've not been accurate: the processor doesn't stop, but
> the processing of the instruction is stopped. Only registers until Rb
> (not included) are updated, and then the processor continue with the
> next instruction. So we have just to test Rb is not modified. I'll
> update the comment.
Ok, now I've got it, I think. Maybe you should also check the
"is_invalid" variable here? And it would also be interesting to see
whether regs[0] (i.e. r31) has been changed or not?
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
2016-03-18 9:51 ` Thomas Huth
@ 2016-03-18 10:08 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:08 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 10:51, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> powerpc/Makefile.common | 5 ++++-
>> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 3 +++
>> 3 files changed, 53 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/emulator.c
>>
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index ab2caf6..257e3fb 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -7,7 +7,8 @@
>> tests-common = \
>> $(TEST_DIR)/selftest.elf \
>> $(TEST_DIR)/spapr_hcall.elf \
>> - $(TEST_DIR)/rtas.elf
>> + $(TEST_DIR)/rtas.elf \
>> + $(TEST_DIR)/emulator.elf
>>
>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>
>> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>>
>> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>> +
>> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> new file mode 100644
>> index 0000000..1215c4f
>> --- /dev/null
>> +++ b/powerpc/emulator.c
>> @@ -0,0 +1,46 @@
>> +/*
>> + * Test some powerpc instructions
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/processor.h>
>> +
>> +static int volatile is_invalid;
>> +
>> +static void program_check_handler(struct pt_regs *regs, void *opaque)
>> +{
>> + int *data = opaque;
>> +
>> + printf("Detected invalid instruction 0x%016lx: %08x\n",
>> + regs->nip, *(uint32_t*)regs->nip);
>
> Should the test always print this, or is this rather confusing for the
> users? Then it should maybe be commented out by default, I think.
In fact, it was really helpful to debug (for instance with kvm-pr), it's
why I let it. But as I agree with you, I'll remove it.
>
>> + *data = 1;
>> +
>> + regs->nip += 4;
>> +}
>> +
>> +static void test_illegal(void)
>> +{
>> + report_prefix_push("invalid");
>> +
>> + is_invalid = 0;
>> +
>> + asm volatile (".long 0");
>> +
>> + report("exception", is_invalid);
>> +
>> + report_prefix_pop();
>> +}
>> +
>> +int main(void)
>> +{
>> + handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>> +
>> + report_prefix_push("emulator");
>> +
>> + test_illegal();
>> +
>> + report_prefix_pop();
>> +
>> + return report_summary();
>> +}
>> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
>> index 02b21c7..ed4fdbe 100644
>> --- a/powerpc/unittests.cfg
>> +++ b/powerpc/unittests.cfg
>> @@ -47,3 +47,6 @@ file = rtas.elf
>> extra_params = -append "set-time-of-day"
>> timeout = 5
>> groups = rtas
>> +
>> +[emulator]
>> +file = emulator.elf
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> BTW: I think you could optionally also check the contents of SRR1 for
> sane values in test_illegal(), in case you want to improve the test a
> little bit.
OK,
Thanks,
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
@ 2016-03-18 10:08 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:08 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 10:51, Thomas Huth wrote:
> On 16.03.2016 16:13, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> powerpc/Makefile.common | 5 ++++-
>> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 3 +++
>> 3 files changed, 53 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/emulator.c
>>
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index ab2caf6..257e3fb 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -7,7 +7,8 @@
>> tests-common = \
>> $(TEST_DIR)/selftest.elf \
>> $(TEST_DIR)/spapr_hcall.elf \
>> - $(TEST_DIR)/rtas.elf
>> + $(TEST_DIR)/rtas.elf \
>> + $(TEST_DIR)/emulator.elf
>>
>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>
>> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>>
>> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>> +
>> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>> new file mode 100644
>> index 0000000..1215c4f
>> --- /dev/null
>> +++ b/powerpc/emulator.c
>> @@ -0,0 +1,46 @@
>> +/*
>> + * Test some powerpc instructions
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/processor.h>
>> +
>> +static int volatile is_invalid;
>> +
>> +static void program_check_handler(struct pt_regs *regs, void *opaque)
>> +{
>> + int *data = opaque;
>> +
>> + printf("Detected invalid instruction 0x%016lx: %08x\n",
>> + regs->nip, *(uint32_t*)regs->nip);
>
> Should the test always print this, or is this rather confusing for the
> users? Then it should maybe be commented out by default, I think.
In fact, it was really helpful to debug (for instance with kvm-pr), it's
why I let it. But as I agree with you, I'll remove it.
>
>> + *data = 1;
>> +
>> + regs->nip += 4;
>> +}
>> +
>> +static void test_illegal(void)
>> +{
>> + report_prefix_push("invalid");
>> +
>> + is_invalid = 0;
>> +
>> + asm volatile (".long 0");
>> +
>> + report("exception", is_invalid);
>> +
>> + report_prefix_pop();
>> +}
>> +
>> +int main(void)
>> +{
>> + handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>> +
>> + report_prefix_push("emulator");
>> +
>> + test_illegal();
>> +
>> + report_prefix_pop();
>> +
>> + return report_summary();
>> +}
>> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
>> index 02b21c7..ed4fdbe 100644
>> --- a/powerpc/unittests.cfg
>> +++ b/powerpc/unittests.cfg
>> @@ -47,3 +47,6 @@ file = rtas.elf
>> extra_params = -append "set-time-of-day"
>> timeout = 5
>> groups = rtas
>> +
>> +[emulator]
>> +file = emulator.elf
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> BTW: I think you could optionally also check the contents of SRR1 for
> sane values in test_illegal(), in case you want to improve the test a
> little bit.
OK,
Thanks,
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
2016-03-18 10:08 ` Laurent Vivier
@ 2016-03-18 10:12 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:12 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:08, Laurent Vivier wrote:
>
>
> On 18/03/2016 10:51, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/Makefile.common | 5 ++++-
>>> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>> powerpc/unittests.cfg | 3 +++
>>> 3 files changed, 53 insertions(+), 1 deletion(-)
>>> create mode 100644 powerpc/emulator.c
>>>
>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>>> index ab2caf6..257e3fb 100644
>>> --- a/powerpc/Makefile.common
>>> +++ b/powerpc/Makefile.common
>>> @@ -7,7 +7,8 @@
>>> tests-common = \
>>> $(TEST_DIR)/selftest.elf \
>>> $(TEST_DIR)/spapr_hcall.elf \
>>> - $(TEST_DIR)/rtas.elf
>>> + $(TEST_DIR)/rtas.elf \
>>> + $(TEST_DIR)/emulator.elf
>>>
>>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>>
>>> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>>> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>>>
>>> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>>> +
>>> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> new file mode 100644
>>> index 0000000..1215c4f
>>> --- /dev/null
>>> +++ b/powerpc/emulator.c
>>> @@ -0,0 +1,46 @@
>>> +/*
>>> + * Test some powerpc instructions
>>> + */
>>> +
>>> +#include <libcflat.h>
>>> +#include <asm/processor.h>
>>> +
>>> +static int volatile is_invalid;
>>> +
>>> +static void program_check_handler(struct pt_regs *regs, void *opaque)
>>> +{
>>> + int *data = opaque;
>>> +
>>> + printf("Detected invalid instruction 0x%016lx: %08x\n",
>>> + regs->nip, *(uint32_t*)regs->nip);
>>
>> Should the test always print this, or is this rather confusing for the
>> users? Then it should maybe be commented out by default, I think.
>
> In fact, it was really helpful to debug (for instance with kvm-pr), it's
> why I let it. But as I agree with you, I'll remove it.
You could also add a CLI option to this unit test, so that it e.g. only
printed when the test is started with a "-v" parameter or so?
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap
@ 2016-03-18 10:12 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:12 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:08, Laurent Vivier wrote:
>
>
> On 18/03/2016 10:51, Thomas Huth wrote:
>> On 16.03.2016 16:13, Laurent Vivier wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>> powerpc/Makefile.common | 5 ++++-
>>> powerpc/emulator.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>> powerpc/unittests.cfg | 3 +++
>>> 3 files changed, 53 insertions(+), 1 deletion(-)
>>> create mode 100644 powerpc/emulator.c
>>>
>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>>> index ab2caf6..257e3fb 100644
>>> --- a/powerpc/Makefile.common
>>> +++ b/powerpc/Makefile.common
>>> @@ -7,7 +7,8 @@
>>> tests-common = \
>>> $(TEST_DIR)/selftest.elf \
>>> $(TEST_DIR)/spapr_hcall.elf \
>>> - $(TEST_DIR)/rtas.elf
>>> + $(TEST_DIR)/rtas.elf \
>>> + $(TEST_DIR)/emulator.elf
>>>
>>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>>
>>> @@ -70,3 +71,5 @@ $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>>> $(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>>>
>>> $(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>>> +
>>> +$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>>> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
>>> new file mode 100644
>>> index 0000000..1215c4f
>>> --- /dev/null
>>> +++ b/powerpc/emulator.c
>>> @@ -0,0 +1,46 @@
>>> +/*
>>> + * Test some powerpc instructions
>>> + */
>>> +
>>> +#include <libcflat.h>
>>> +#include <asm/processor.h>
>>> +
>>> +static int volatile is_invalid;
>>> +
>>> +static void program_check_handler(struct pt_regs *regs, void *opaque)
>>> +{
>>> + int *data = opaque;
>>> +
>>> + printf("Detected invalid instruction 0x%016lx: %08x\n",
>>> + regs->nip, *(uint32_t*)regs->nip);
>>
>> Should the test always print this, or is this rather confusing for the
>> users? Then it should maybe be commented out by default, I think.
>
> In fact, it was really helpful to debug (for instance with kvm-pr), it's
> why I let it. But as I agree with you, I'll remove it.
You could also add a CLI option to this unit test, so that it e.g. only
printed when the test is started with a "-v" parameter or so?
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-18 10:06 ` Thomas Huth
@ 2016-03-18 10:18 ` Laurent Vivier
-1 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:18 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 11:06, Thomas Huth wrote:
> On 18.03.2016 11:01, Laurent Vivier wrote:
...
>>>> + /* doc says it is invalid, real proc stops when it comes to
>>>> + * overwrite the register.
>>>> + * In all the cases, the register must stay untouched
>>>> + */
>>>> + report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
>>>
>>> Huh, how can this KVM unit test ever finish successfully if real
>>> processor stops? Should this last test maybe be optional and only be
>>> triggered if this kvm-unit-test is run with certain parameters?
>>
>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>> the processing of the instruction is stopped. Only registers until Rb
>> (not included) are updated, and then the processor continue with the
>> next instruction. So we have just to test Rb is not modified. I'll
>> update the comment.
>
> Ok, now I've got it, I think. Maybe you should also check the
> "is_invalid" variable here? And it would also be interesting to see
> whether regs[0] (i.e. r31) has been changed or not?
In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
there is never an illegal exception. It's why I don't check that.
Checking regs[1] is correct in the case of the doc (illegal instruction)
and of the real processor (it doesn't update it).
Checking regs[0] can be tricky: in the case of the doc (illegal) it
should not be updated, in the case of the real processor it is updated.
I'd like this test checks only if TCG behaves like a real processor,
it's why I prefer to check this result against the result of a real
processor than against the result described in the doc.
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-18 10:18 ` Laurent Vivier
0 siblings, 0 replies; 42+ messages in thread
From: Laurent Vivier @ 2016-03-18 10:18 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18/03/2016 11:06, Thomas Huth wrote:
> On 18.03.2016 11:01, Laurent Vivier wrote:
...
>>>> + /* doc says it is invalid, real proc stops when it comes to
>>>> + * overwrite the register.
>>>> + * In all the cases, the register must stay untouched
>>>> + */
>>>> + report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
>>>
>>> Huh, how can this KVM unit test ever finish successfully if real
>>> processor stops? Should this last test maybe be optional and only be
>>> triggered if this kvm-unit-test is run with certain parameters?
>>
>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>> the processing of the instruction is stopped. Only registers until Rb
>> (not included) are updated, and then the processor continue with the
>> next instruction. So we have just to test Rb is not modified. I'll
>> update the comment.
>
> Ok, now I've got it, I think. Maybe you should also check the
> "is_invalid" variable here? And it would also be interesting to see
> whether regs[0] (i.e. r31) has been changed or not?
In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
there is never an illegal exception. It's why I don't check that.
Checking regs[1] is correct in the case of the doc (illegal instruction)
and of the real processor (it doesn't update it).
Checking regs[0] can be tricky: in the case of the doc (illegal) it
should not be updated, in the case of the real processor it is updated.
I'd like this test checks only if TCG behaves like a real processor,
it's why I prefer to check this result against the result of a real
processor than against the result described in the doc.
Laurent
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
2016-03-18 10:18 ` Laurent Vivier
@ 2016-03-18 10:34 ` Thomas Huth
-1 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:34 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:18, Laurent Vivier wrote:
>
>
> On 18/03/2016 11:06, Thomas Huth wrote:
>> On 18.03.2016 11:01, Laurent Vivier wrote:
> ...
>>>>> + /* doc says it is invalid, real proc stops when it comes to
>>>>> + * overwrite the register.
>>>>> + * In all the cases, the register must stay untouched
>>>>> + */
>>>>> + report("Don't overwrite Rb", regs[1] == (uint64_t)addr);
>>>>
>>>> Huh, how can this KVM unit test ever finish successfully if real
>>>> processor stops? Should this last test maybe be optional and only be
>>>> triggered if this kvm-unit-test is run with certain parameters?
>>>
>>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>>> the processing of the instruction is stopped. Only registers until Rb
>>> (not included) are updated, and then the processor continue with the
>>> next instruction. So we have just to test Rb is not modified. I'll
>>> update the comment.
>>
>> Ok, now I've got it, I think. Maybe you should also check the
>> "is_invalid" variable here? And it would also be interesting to see
>> whether regs[0] (i.e. r31) has been changed or not?
>
> In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
> there is never an illegal exception. It's why I don't check that.
>
> Checking regs[1] is correct in the case of the doc (illegal instruction)
> and of the real processor (it doesn't update it).
>
> Checking regs[0] can be tricky: in the case of the doc (illegal) it
> should not be updated, in the case of the real processor it is updated.
>
> I'd like this test checks only if TCG behaves like a real processor,
> it's why I prefer to check this result against the result of a real
> processor than against the result described in the doc.
Ah, ok ... I just saw that there is even a comment in the QEMU sources
about this in front of the helper_lswx() function ... then better keep
the unit test code in its current shape!
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] powerpc: check lswx
@ 2016-03-18 10:34 ` Thomas Huth
0 siblings, 0 replies; 42+ messages in thread
From: Thomas Huth @ 2016-03-18 10:34 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 18.03.2016 11:18, Laurent Vivier wrote:
>
>
> On 18/03/2016 11:06, Thomas Huth wrote:
>> On 18.03.2016 11:01, Laurent Vivier wrote:
> ...
>>>>> + /* doc says it is invalid, real proc stops when it comes to
>>>>> + * overwrite the register.
>>>>> + * In all the cases, the register must stay untouched
>>>>> + */
>>>>> + report("Don't overwrite Rb", regs[1] = (uint64_t)addr);
>>>>
>>>> Huh, how can this KVM unit test ever finish successfully if real
>>>> processor stops? Should this last test maybe be optional and only be
>>>> triggered if this kvm-unit-test is run with certain parameters?
>>>
>>> Well, my bad, I've not been accurate: the processor doesn't stop, but
>>> the processing of the instruction is stopped. Only registers until Rb
>>> (not included) are updated, and then the processor continue with the
>>> next instruction. So we have just to test Rb is not modified. I'll
>>> update the comment.
>>
>> Ok, now I've got it, I think. Maybe you should also check the
>> "is_invalid" variable here? And it would also be interesting to see
>> whether regs[0] (i.e. r31) has been changed or not?
>
> In fact, I've tested this with kvm PR and HV, on POWER8 and ppc970, and
> there is never an illegal exception. It's why I don't check that.
>
> Checking regs[1] is correct in the case of the doc (illegal instruction)
> and of the real processor (it doesn't update it).
>
> Checking regs[0] can be tricky: in the case of the doc (illegal) it
> should not be updated, in the case of the real processor it is updated.
>
> I'd like this test checks only if TCG behaves like a real processor,
> it's why I prefer to check this result against the result of a real
> processor than against the result described in the doc.
Ah, ok ... I just saw that there is even a comment in the QEMU sources
about this in front of the helper_lswx() function ... then better keep
the unit test code in its current shape!
Thomas
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2016-03-18 10:34 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-16 15:12 [kvm-unit-tests PATCH 0/5] Check emulation Laurent Vivier
2016-03-16 15:12 ` Laurent Vivier
2016-03-16 15:12 ` [kvm-unit-tests PATCH 1/5] powerpc: add exception handler Laurent Vivier
2016-03-16 15:12 ` Laurent Vivier
2016-03-18 3:59 ` David Gibson
2016-03-18 3:59 ` David Gibson
2016-03-18 8:53 ` Laurent Vivier
2016-03-18 8:53 ` Laurent Vivier
2016-03-18 9:42 ` Thomas Huth
2016-03-18 9:42 ` Thomas Huth
2016-03-16 15:13 ` [kvm-unit-tests PATCH 2/5] powerpc: add test to check invalid instruction trap Laurent Vivier
2016-03-16 15:13 ` Laurent Vivier
2016-03-18 9:51 ` Thomas Huth
2016-03-18 9:51 ` Thomas Huth
2016-03-18 10:08 ` Laurent Vivier
2016-03-18 10:08 ` Laurent Vivier
2016-03-18 10:12 ` Thomas Huth
2016-03-18 10:12 ` Thomas Huth
2016-03-16 15:13 ` [kvm-unit-tests PATCH 3/5] powerpc: check 64bit mode Laurent Vivier
2016-03-16 15:13 ` Laurent Vivier
2016-03-18 8:28 ` Thomas Huth
2016-03-18 8:28 ` Thomas Huth
2016-03-18 8:38 ` Paul Mackerras
2016-03-18 8:38 ` Paul Mackerras
2016-03-18 8:41 ` Thomas Huth
2016-03-18 8:41 ` Thomas Huth
2016-03-16 15:13 ` [kvm-unit-tests PATCH 4/5] powerpc: check lswx Laurent Vivier
2016-03-16 15:13 ` Laurent Vivier
2016-03-18 9:09 ` Thomas Huth
2016-03-18 9:09 ` Thomas Huth
2016-03-18 10:01 ` Laurent Vivier
2016-03-18 10:01 ` Laurent Vivier
2016-03-18 10:06 ` Thomas Huth
2016-03-18 10:06 ` Thomas Huth
2016-03-18 10:18 ` Laurent Vivier
2016-03-18 10:18 ` Laurent Vivier
2016-03-18 10:34 ` Thomas Huth
2016-03-18 10:34 ` Thomas Huth
2016-03-16 15:13 ` [kvm-unit-tests PATCH 5/5] powerpc: Check lswx in little-endian mode Laurent Vivier
2016-03-16 15:13 ` Laurent Vivier
2016-03-18 9:54 ` Thomas Huth
2016-03-18 9:54 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.