Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 19.09.2012, at 02:14, Richard Henderson wrote:
On 09/18/2012 01:18 PM, Alexander Graf wrote:
-/* remember what pgm exeption this was */
+/* Remember what pgm exeption this was. */
tmp = tcg_const_i32(code);
tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code));
tcg_temp_free_i32(tmp);
-tmp = tcg_const_i32(ilc);
+tmp = tcg_const_i32(s-next_pc - s-pc);
Mind to explain this one?
ILC = the size of the insn. Rather than passing ILC around into
gen_program_exception, get it back from the s-next_pc value that
we stored into DisasContext.
Ah, makes sense. Maybe create a small helper that makes it more obvious:
static int current_ilc(DisasContext *s)
{
/* Next pc - current pc = current instruction length. */
return s-next_pc - s-pc;
}
Alex
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 19 September 2012 01:14, Richard Henderson r...@twiddle.net wrote: On 09/18/2012 01:18 PM, Alexander Graf wrote: -/* remember what pgm exeption this was */ +/* Remember what pgm exeption this was. */ ...if you're changing this comment then at least fix the typo (exception)... tmp = tcg_const_i32(code); tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code)); tcg_temp_free_i32(tmp); -tmp = tcg_const_i32(ilc); +tmp = tcg_const_i32(s-next_pc - s-pc); Mind to explain this one? ILC = the size of the insn. Rather than passing ILC around into gen_program_exception, get it back from the s-next_pc value that we stored into DisasContext. ...but isn't (s-next_pc - s-pc) ilc*2, not ilc? Compare this hunk from elsewhere in the patch: -tmp32_2 = tcg_const_i32(ilc * 2); -tmp32_3 = tcg_const_i32(EXCP_SVC); +tmp32_2 = tcg_const_i32(s-next_pc - s-pc); -- PMM
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 19.09.2012, at 13:29, Peter Maydell wrote: On 19 September 2012 01:14, Richard Henderson r...@twiddle.net wrote: On 09/18/2012 01:18 PM, Alexander Graf wrote: -/* remember what pgm exeption this was */ +/* Remember what pgm exeption this was. */ ...if you're changing this comment then at least fix the typo (exception)... tmp = tcg_const_i32(code); tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code)); tcg_temp_free_i32(tmp); -tmp = tcg_const_i32(ilc); +tmp = tcg_const_i32(s-next_pc - s-pc); Mind to explain this one? ILC = the size of the insn. Rather than passing ILC around into gen_program_exception, get it back from the s-next_pc value that we stored into DisasContext. ...but isn't (s-next_pc - s-pc) ilc*2, not ilc? Compare this hunk from elsewhere in the patch: -tmp32_2 = tcg_const_i32(ilc * 2); -tmp32_3 = tcg_const_i32(EXCP_SVC); +tmp32_2 = tcg_const_i32(s-next_pc - s-pc); Yes, it is, and hence it's correct. The exception field encodes the real instruction length, not the ILC bits in the opcode. Maybe the naming is unfortunate... Alex
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 19.09.2012, at 13:34, Alexander Graf wrote: On 19.09.2012, at 13:29, Peter Maydell wrote: On 19 September 2012 01:14, Richard Henderson r...@twiddle.net wrote: On 09/18/2012 01:18 PM, Alexander Graf wrote: -/* remember what pgm exeption this was */ +/* Remember what pgm exeption this was. */ ...if you're changing this comment then at least fix the typo (exception)... tmp = tcg_const_i32(code); tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code)); tcg_temp_free_i32(tmp); -tmp = tcg_const_i32(ilc); +tmp = tcg_const_i32(s-next_pc - s-pc); Mind to explain this one? ILC = the size of the insn. Rather than passing ILC around into gen_program_exception, get it back from the s-next_pc value that we stored into DisasContext. ...but isn't (s-next_pc - s-pc) ilc*2, not ilc? Compare this hunk from elsewhere in the patch: -tmp32_2 = tcg_const_i32(ilc * 2); -tmp32_3 = tcg_const_i32(EXCP_SVC); +tmp32_2 = tcg_const_i32(s-next_pc - s-pc); Yes, it is, and hence it's correct. The exception field encodes the real instruction length, not the ILC bits in the opcode. Maybe the naming is unfortunate... Or maybe not. For SVC we seem to pass in the full instruction length, while PGM only gets ILC. So the above hunk is wrong. Richard, please double-check all the ilc passing again with the spec. Alex
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 09/19/2012 06:02 AM, Alexander Graf wrote: Richard, please double-check all the ilc passing again with the spec. I'll look again. But I do recall that the spec is consistent in placing a 3-bit field (values 1-3) shifted one bit left into the larger 32-bit field, so that in essence the value is always the real instruction length. However, qemu was not consistent in performing that multiplication. So there will be some place that gets changed because of this. Frankly I think it would be least confusing to always manipulate the full instruction length and never explicitly perform the multiply. r~
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 19.09.2012, at 16:34, Richard Henderson wrote: On 09/19/2012 06:02 AM, Alexander Graf wrote: Richard, please double-check all the ilc passing again with the spec. I'll look again. But I do recall that the spec is consistent in placing a 3-bit field (values 1-3) shifted one bit left into the larger 32-bit field, so that in essence the value is always the real instruction length. However, qemu was not consistent in performing that multiplication. So there will be some place that gets changed because of this. Frankly I think it would be least confusing to always manipulate the full instruction length and never explicitly perform the multiply. I agree. How about we rename the whole thing to inst_len and then just modify it accordingly when we move it into the exception area? Alex
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 09/09/2012 11:04 PM, Richard Henderson wrote:
Make the user path more like the system path. Prepare for more kinds
of runtime exceptions. Compute ILC from S-NEXT_PC, rather than
passing it around.
Signed-off-by: Richard Henderson r...@twiddle.net
---
linux-user/main.c | 133 +--
target-s390x/cpu.h | 11 ---
target-s390x/misc_helper.c | 10 ++-
target-s390x/translate.c | 220 +
4 files changed, 174 insertions(+), 200 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 1a1c661..8f6f39b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2935,71 +2935,108 @@ void cpu_loop(CPUAlphaState *env)
#ifdef TARGET_S390X
void cpu_loop(CPUS390XState *env)
{
-int trapnr;
+int trapnr, n, sig;
target_siginfo_t info;
+target_ulong addr;
while (1) {
-trapnr = cpu_s390x_exec (env);
-
+trapnr = cpu_s390x_exec(env);
switch (trapnr) {
case EXCP_INTERRUPT:
-/* just indicate that signals should be handled asap */
+/* Just indicate that signals should be handled asap. */
break;
-case EXCP_DEBUG:
-{
-int sig;
-sig = gdb_handlesig (env, TARGET_SIGTRAP);
-if (sig) {
-info.si_signo = sig;
-info.si_errno = 0;
-info.si_code = TARGET_TRAP_BRKPT;
-queue_signal(env, info.si_signo, info);
-}
+case EXCP_SVC:
+n = env-int_svc_code;
+if (!n) {
+/* syscalls 255 */
+n = env-regs[1];
}
+env-psw.addr += env-int_svc_ilc;
+env-regs[2] = do_syscall(env, n, env-regs[2], env-regs[3],
+ env-regs[4], env-regs[5],
+ env-regs[6], env-regs[7], 0, 0);
break;
-case EXCP_SVC:
-{
-int n = env-int_svc_code;
-if (!n) {
-/* syscalls 255 */
-n = env-regs[1];
-}
-env-psw.addr += env-int_svc_ilc;
-env-regs[2] = do_syscall(env, n,
- env-regs[2],
- env-regs[3],
- env-regs[4],
- env-regs[5],
- env-regs[6],
- env-regs[7],
- 0, 0);
+
+case EXCP_DEBUG:
+sig = gdb_handlesig(env, TARGET_SIGTRAP);
+if (sig) {
+n = TARGET_TRAP_BRKPT;
+goto do_signal_pc;
}
break;
-case EXCP_ADDR:
-{
-info.si_signo = SIGSEGV;
-info.si_errno = 0;
+case EXCP_PGM:
+n = env-int_pgm_code;
+switch (n) {
+case PGM_OPERATION:
+case PGM_PRIVILEGED:
+sig = SIGILL;
+n = TARGET_ILL_ILLOPC;
+goto do_signal_pc;
+case PGM_PROTECTION:
+case PGM_ADDRESSING:
+sig = SIGSEGV;
/* XXX: check env-error_code */
-info.si_code = TARGET_SEGV_MAPERR;
-info._sifields._sigfault._addr = env-__excp_addr;
-queue_signal(env, info.si_signo, info);
+n = TARGET_SEGV_MAPERR;
+addr = env-__excp_addr;
+goto do_signal;
+case PGM_EXECUTE:
+case PGM_SPECIFICATION:
+case PGM_DATA:
+case PGM_SPECIAL_OP:
+case PGM_OPERAND:
+sig = SIGILL;
+n = TARGET_ILL_ILLOPN;
+goto do_signal_pc;
+case PGM_FIXPT_OVERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_INTOVF;
+goto do_signal_pc;
+case PGM_FIXPT_DIVIDE:
+sig = SIGFPE;
+n = TARGET_FPE_INTDIV;
+goto do_signal_pc;
+case PGM_HFP_EXP_OVERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_FLTOVF;
+goto do_signal_pc;
+case PGM_HFP_EXP_UNDERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_FLTUND;
+goto do_signal_pc;
+case PGM_HFP_SIGNIFICANCE:
+sig = SIGFPE;
+n = TARGET_FPE_FLTRES;
+goto do_signal_pc;
+case PGM_HFP_DIVIDE:
+sig = SIGFPE;
+n = TARGET_FPE_FLTDIV;
+goto do_signal_pc;
+case PGM_HFP_SQRT:
+sig = SIGFPE;
+n = TARGET_FPE_FLTINV;
+goto do_signal_pc;
+default:
+fprintf(stderr, Unhandled
Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
On 09/18/2012 01:18 PM, Alexander Graf wrote: -/* remember what pgm exeption this was */ +/* Remember what pgm exeption this was. */ tmp = tcg_const_i32(code); tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code)); tcg_temp_free_i32(tmp); -tmp = tcg_const_i32(ilc); +tmp = tcg_const_i32(s-next_pc - s-pc); Mind to explain this one? ILC = the size of the insn. Rather than passing ILC around into gen_program_exception, get it back from the s-next_pc value that we stored into DisasContext. r~
[Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling
Make the user path more like the system path. Prepare for more kinds
of runtime exceptions. Compute ILC from S-NEXT_PC, rather than
passing it around.
Signed-off-by: Richard Henderson r...@twiddle.net
---
linux-user/main.c | 133 +--
target-s390x/cpu.h | 11 ---
target-s390x/misc_helper.c | 10 ++-
target-s390x/translate.c | 220 +
4 files changed, 174 insertions(+), 200 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 1a1c661..8f6f39b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2935,71 +2935,108 @@ void cpu_loop(CPUAlphaState *env)
#ifdef TARGET_S390X
void cpu_loop(CPUS390XState *env)
{
-int trapnr;
+int trapnr, n, sig;
target_siginfo_t info;
+target_ulong addr;
while (1) {
-trapnr = cpu_s390x_exec (env);
-
+trapnr = cpu_s390x_exec(env);
switch (trapnr) {
case EXCP_INTERRUPT:
-/* just indicate that signals should be handled asap */
+/* Just indicate that signals should be handled asap. */
break;
-case EXCP_DEBUG:
-{
-int sig;
-sig = gdb_handlesig (env, TARGET_SIGTRAP);
-if (sig) {
-info.si_signo = sig;
-info.si_errno = 0;
-info.si_code = TARGET_TRAP_BRKPT;
-queue_signal(env, info.si_signo, info);
-}
+case EXCP_SVC:
+n = env-int_svc_code;
+if (!n) {
+/* syscalls 255 */
+n = env-regs[1];
}
+env-psw.addr += env-int_svc_ilc;
+env-regs[2] = do_syscall(env, n, env-regs[2], env-regs[3],
+ env-regs[4], env-regs[5],
+ env-regs[6], env-regs[7], 0, 0);
break;
-case EXCP_SVC:
-{
-int n = env-int_svc_code;
-if (!n) {
-/* syscalls 255 */
-n = env-regs[1];
-}
-env-psw.addr += env-int_svc_ilc;
-env-regs[2] = do_syscall(env, n,
- env-regs[2],
- env-regs[3],
- env-regs[4],
- env-regs[5],
- env-regs[6],
- env-regs[7],
- 0, 0);
+
+case EXCP_DEBUG:
+sig = gdb_handlesig(env, TARGET_SIGTRAP);
+if (sig) {
+n = TARGET_TRAP_BRKPT;
+goto do_signal_pc;
}
break;
-case EXCP_ADDR:
-{
-info.si_signo = SIGSEGV;
-info.si_errno = 0;
+case EXCP_PGM:
+n = env-int_pgm_code;
+switch (n) {
+case PGM_OPERATION:
+case PGM_PRIVILEGED:
+sig = SIGILL;
+n = TARGET_ILL_ILLOPC;
+goto do_signal_pc;
+case PGM_PROTECTION:
+case PGM_ADDRESSING:
+sig = SIGSEGV;
/* XXX: check env-error_code */
-info.si_code = TARGET_SEGV_MAPERR;
-info._sifields._sigfault._addr = env-__excp_addr;
-queue_signal(env, info.si_signo, info);
+n = TARGET_SEGV_MAPERR;
+addr = env-__excp_addr;
+goto do_signal;
+case PGM_EXECUTE:
+case PGM_SPECIFICATION:
+case PGM_DATA:
+case PGM_SPECIAL_OP:
+case PGM_OPERAND:
+sig = SIGILL;
+n = TARGET_ILL_ILLOPN;
+goto do_signal_pc;
+case PGM_FIXPT_OVERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_INTOVF;
+goto do_signal_pc;
+case PGM_FIXPT_DIVIDE:
+sig = SIGFPE;
+n = TARGET_FPE_INTDIV;
+goto do_signal_pc;
+case PGM_HFP_EXP_OVERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_FLTOVF;
+goto do_signal_pc;
+case PGM_HFP_EXP_UNDERFLOW:
+sig = SIGFPE;
+n = TARGET_FPE_FLTUND;
+goto do_signal_pc;
+case PGM_HFP_SIGNIFICANCE:
+sig = SIGFPE;
+n = TARGET_FPE_FLTRES;
+goto do_signal_pc;
+case PGM_HFP_DIVIDE:
+sig = SIGFPE;
+n = TARGET_FPE_FLTDIV;
+goto do_signal_pc;
+case PGM_HFP_SQRT:
+sig = SIGFPE;
+n = TARGET_FPE_FLTINV;
+goto do_signal_pc;
+default:
+fprintf(stderr, Unhandled program exception: %#x\n, n);
+cpu_dump_state(env, stderr,
