Message ID | 20171129202701.16117-3-david@redhat.com |
---|---|
State | New |
Headers | show |
Series | s390x/tcg: cleanup and fix program interrupts | expand |
On 29.11.2017 21:26, David Hildenbrand wrote: > Let's use s390_program_interrupt() instead. > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/fpu_helper.c | 2 +- > target/s390x/int_helper.c | 14 +++++++------- > target/s390x/internal.h | 2 -- > target/s390x/misc_helper.c | 16 ---------------- > 4 files changed, 8 insertions(+), 26 deletions(-) Is it a disadvantage that runtime_exception() was declared as QEMU_NORETURN, and s390_program_interrupt() is not declared as QEMU_NORETURN? At a first glance, I guess it's ok, and in that case: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 30.11.2017 10:10, Thomas Huth wrote: > On 29.11.2017 21:26, David Hildenbrand wrote: >> Let's use s390_program_interrupt() instead. >> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> target/s390x/fpu_helper.c | 2 +- >> target/s390x/int_helper.c | 14 +++++++------- >> target/s390x/internal.h | 2 -- >> target/s390x/misc_helper.c | 16 ---------------- >> 4 files changed, 8 insertions(+), 26 deletions(-) > > Is it a disadvantage that runtime_exception() was declared as > QEMU_NORETURN, and s390_program_interrupt() is not declared as > QEMU_NORETURN? We could add that to trigger_pgm_exception instead. But if cpu_loop_exit() would really return, we would be in more trouble AKA nothing would work. So I don't see a problem dropping this. > > At a first glance, I guess it's ok, and in that case: > > Reviewed-by: Thomas Huth <thuth@redhat.com> >
On 11/30/2017 03:56 PM, David Hildenbrand wrote: > On 30.11.2017 10:10, Thomas Huth wrote: >> On 29.11.2017 21:26, David Hildenbrand wrote: >>> Let's use s390_program_interrupt() instead. >>> >>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> target/s390x/fpu_helper.c | 2 +- >>> target/s390x/int_helper.c | 14 +++++++------- >>> target/s390x/internal.h | 2 -- >>> target/s390x/misc_helper.c | 16 ---------------- >>> 4 files changed, 8 insertions(+), 26 deletions(-) >> >> Is it a disadvantage that runtime_exception() was declared as >> QEMU_NORETURN, and s390_program_interrupt() is not declared as >> QEMU_NORETURN? > > We could add that to trigger_pgm_exception instead. But if > cpu_loop_exit() would really return, we would be in more trouble AKA > nothing would work. s390_program_interrupt and trigger_pgm_exception do return (kvm). We could export tcg_s390_program_interrupt as NORETURN, and use it directly in places that are tcg-only. That might provide some minor advantages to the compiler, but I don't think it's critical. r~
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c index ffbeb3b2df..334159119f 100644 --- a/target/s390x/fpu_helper.c +++ b/target/s390x/fpu_helper.c @@ -44,7 +44,7 @@ static void ieee_exception(CPUS390XState *env, uint32_t dxc, uintptr_t retaddr) /* Install the DXC code. */ env->fpc = (env->fpc & ~0xff00) | (dxc << 8); /* Trap. */ - runtime_exception(env, PGM_DATA, retaddr); + s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, retaddr); } /* Should be called after any operation that may raise IEEE exceptions. */ diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c index 0076bea047..abf77a94e6 100644 --- a/target/s390x/int_helper.c +++ b/target/s390x/int_helper.c @@ -39,7 +39,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64) int64_t q; if (b == 0) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } ret = q = a / b; @@ -47,7 +47,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64) /* Catch non-representable quotient. */ if (ret != q) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } return ret; @@ -60,7 +60,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64) uint64_t q; if (b == 0) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } ret = q = a / b; @@ -68,7 +68,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64) /* Catch non-representable quotient. */ if (ret != q) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } return ret; @@ -79,7 +79,7 @@ int64_t HELPER(divs64)(CPUS390XState *env, int64_t a, int64_t b) { /* Catch divide by zero, and non-representable quotient (MIN / -1). */ if (b == 0 || (b == -1 && a == (1ll << 63))) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } env->retxl = a % b; return a / b; @@ -92,7 +92,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al, uint64_t ret; /* Signal divide by zero. */ if (b == 0) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } if (ah == 0) { /* 64 -> 64/64 case */ @@ -106,7 +106,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al, env->retxl = a % b; ret = q; if (ret != q) { - runtime_exception(env, PGM_FIXPT_DIVIDE, GETPC()); + s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC()); } #else S390CPU *cpu = s390_env_get_cpu(env); diff --git a/target/s390x/internal.h b/target/s390x/internal.h index 3aff54ada4..db39d5bfac 100644 --- a/target/s390x/internal.h +++ b/target/s390x/internal.h @@ -408,8 +408,6 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw, /* misc_helper.c */ -void QEMU_NORETURN runtime_exception(CPUS390XState *env, int excp, - uintptr_t retaddr); int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3); void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3); diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c index 1ccbafbb7d..67628e690d 100644 --- a/target/s390x/misc_helper.c +++ b/target/s390x/misc_helper.c @@ -45,22 +45,6 @@ #define HELPER_LOG(x...) #endif -/* Raise an exception dynamically from a helper function. */ -void QEMU_NORETURN runtime_exception(CPUS390XState *env, int excp, - uintptr_t retaddr) -{ - CPUState *cs = CPU(s390_env_get_cpu(env)); - - cs->exception_index = EXCP_PGM; - env->int_pgm_code = excp; - env->int_pgm_ilen = ILEN_AUTO; - - /* Use the (ultimate) callers address to find the insn that trapped. */ - cpu_restore_state(cs, retaddr); - - cpu_loop_exit(cs); -} - /* Raise an exception statically from a TB. */ void HELPER(exception)(CPUS390XState *env, uint32_t excp) {