diff mbox series

[v2,for-2.12,02/16] s390x/tcg: get rid of runtime_exception()

Message ID 20171129202701.16117-3-david@redhat.com
State New
Headers show
Series s390x/tcg: cleanup and fix program interrupts | expand

Commit Message

David Hildenbrand Nov. 29, 2017, 8:26 p.m. UTC 
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(-)

Comments

Thomas Huth Nov. 30, 2017, 9:10 a.m. UTC | #1 
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>
David Hildenbrand Nov. 30, 2017, 3:56 p.m. UTC | #2 
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>
>
Richard Henderson Dec. 1, 2017, 12:48 p.m. UTC | #3 
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 mbox series

Patch

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)
 {