[Qemu-devel] [PATCH v1 for-2.12 00/15] s390x/tcg: cleanup and fix program interrupts

[Qemu-devel] [PATCH v1 for-2.12 00/15] s390x/tcg: cleanup and fix program interrupts

[David Hildenbrand]

I have quite some patches on my queue for 2.12. (booting Fedora 26/27
guests, floating interrupts, machine checks, missing instructions ...)

So let's start slowly :) This series gets rid of program_interrupt() and
potential_page_fault(). We now always properly restore the cpu state when
injecting/delivering a program interrupt. So there is no need to update
the state via potential_page_fault() anymore.

In addition, handling for program interrupts comming via
s390_cpu_virt_mem_rw() was missing something for the TCG case. Also fixed.

David Hildenbrand (15):
  s390x/tcg: introduce and use program_interrupt_ra()
  s390x/tcg: get rid of runtime_exception()
  s390x/tcg: rip out dead tpi code
  s390x/ioinst: pass the retaddr to all IO instructions
  s390x/pci: pass the retaddr to all PCI instructions
  s390x/diag: pass the retaddr into handle_diag_308()
  s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)
  s390x/tcg: io instructions don't need potential_page_fault()
  s390x/tcg: use program_interrupt_ra() in SCLP Service Call
  s390x/tcg: use program_interrupt_ra() in DIAG
  s390x/tcg: use program_interrupt_ra() in per_check_exception()
  s390x/tcg: use program_interrupt_ra() in SACF
  s390x/tcg: use program_interrupt_ra() in STSI
  s390x/tcg: drop program_interrupt()
  s390x/tcg: drop potential_page_fault()

 hw/s390x/css.c               |   6 ---
 hw/s390x/s390-pci-inst.c     |  90 ++++++++++++++++++----------------
 hw/s390x/s390-pci-inst.h     |  16 ++++---
 include/hw/s390x/css.h       |   1 -
 target/s390x/cc_helper.c     |   2 +-
 target/s390x/cpu.h           |   4 +-
 target/s390x/crypto_helper.c |   7 +--
 target/s390x/diag.c          |  14 +++---
 target/s390x/excp_helper.c   |   5 +-
 target/s390x/fpu_helper.c    |   2 +-
 target/s390x/int_helper.c    |  14 +++---
 target/s390x/internal.h      |  35 +++++++-------
 target/s390x/interrupt.c     |   9 +++-
 target/s390x/ioinst.c        | 112 ++++++++++++++++++++-----------------------
 target/s390x/kvm.c           |  42 ++++++++--------
 target/s390x/mem_helper.c    |  35 +++++---------
 target/s390x/misc_helper.c   |  50 +++++++------------
 target/s390x/mmu_helper.c    |  19 +++++++-
 target/s390x/translate.c     |  27 +----------
 19 files changed, 226 insertions(+), 264 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 01/15] s390x/tcg: introduce and use program_interrupt_ra()

[David Hildenbrand]

Allows to easily convert more callers of program_interrupt() and to
easily introduce new exceptions without forgetting about the cpu state
reset.

Use program_interrupt_ra() in places where we already had the same
pattern.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h           |  2 ++
 target/s390x/crypto_helper.c |  7 ++-----
 target/s390x/excp_helper.c   |  5 +----
 target/s390x/interrupt.c     | 13 +++++++++++++
 target/s390x/mem_helper.c    | 35 +++++++++++------------------------
 target/s390x/misc_helper.c   |  3 +--
 6 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 4db8b5409e..e21d63ae04 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -720,6 +720,8 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
 /* automatically detect the instruction length */
 #define ILEN_AUTO                   0xff
 void program_interrupt(CPUS390XState *env, uint32_t code, int ilen);
+void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
+                          uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
 
diff --git a/target/s390x/crypto_helper.c b/target/s390x/crypto_helper.c
index fa360a2d6e..38f7943778 100644
--- a/target/s390x/crypto_helper.c
+++ b/target/s390x/crypto_helper.c
@@ -23,7 +23,6 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
     const uintptr_t ra = GETPC();
     const uint8_t mod = env->regs[0] & 0x80ULL;
     const uint8_t fc = env->regs[0] & 0x7fULL;
-    CPUState *cs = CPU(s390_env_get_cpu(env));
     uint8_t subfunc[16] = { 0 };
     uint64_t param_addr;
     int i;
@@ -35,8 +34,7 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
     case S390_FEAT_TYPE_PCKMO:
     case S390_FEAT_TYPE_PCC:
         if (mod) {
-            cpu_restore_state(cs, ra);
-            program_interrupt(env, PGM_SPECIFICATION, 4);
+            program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
             return 0;
         }
         break;
@@ -44,8 +42,7 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
 
     s390_get_feat_block(type, subfunc);
     if (!test_be_bit(fc, subfunc)) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return 0;
     }
 
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index e04b670663..9423bb6f36 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -554,10 +554,7 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
 
-    if (retaddr) {
-        cpu_restore_state(cs, retaddr);
-    }
-    program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO);
+    program_interrupt_ra(env, PGM_SPECIFICATION, ILEN_AUTO, retaddr);
 }
 
 #endif /* CONFIG_USER_ONLY */
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index ce6177c141..6ce06bb549 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -53,6 +53,19 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
     }
 }
 
+void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
+                          uintptr_t ra)
+{
+    S390CPU *cpu = s390_env_get_cpu(env);
+
+#ifdef CONFIG_TCG
+    if (tcg_enabled() && ra) {
+        cpu_restore_state(CPU(cpu), ra);
+    }
+#endif
+    program_interrupt(env, code, ilen);
+}
+
 #if !defined(CONFIG_USER_ONLY)
 static void cpu_inject_service(S390CPU *cpu, uint32_t param)
 {
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index a1652d4849..47f327538c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -85,9 +85,7 @@ static inline void check_alignment(CPUS390XState *env, uint64_t v,
                                    int wordsize, uintptr_t ra)
 {
     if (v % wordsize) {
-        CPUState *cs = CPU(s390_env_get_cpu(env));
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
     }
 }
 
@@ -545,8 +543,7 @@ void HELPER(srst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-55 must contain all 0.  */
     if (env->regs[0] & 0xffffff00u) {
-        cpu_restore_state(ENV_GET_CPU(env), ra);
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
     }
 
     str = get_address(env, r2);
@@ -583,8 +580,7 @@ void HELPER(srstu)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-47 of R0 must be zero.  */
     if (env->regs[0] & 0xffff0000u) {
-        cpu_restore_state(ENV_GET_CPU(env), ra);
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
     }
 
     str = get_address(env, r2);
@@ -1600,8 +1596,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
     return cc;
 
  spec_exception:
-    cpu_restore_state(ENV_GET_CPU(env), ra);
-    program_interrupt(env, PGM_SPECIFICATION, 6);
+    program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
     g_assert_not_reached();
 }
 
@@ -1865,8 +1860,7 @@ void HELPER(idte)(CPUS390XState *env, uint64_t r1, uint64_t r2, uint32_t m4)
     uint16_t entries, i, index = 0;
 
     if (r2 & 0xff000) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
     }
 
     if (!(r2 & 0x800)) {
@@ -2014,8 +2008,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
-        cpu_restore_state(cs, GETPC());
-        program_interrupt(env, PGM_SPECIAL_OP, 2);
+        program_interrupt_ra(env, PGM_SPECIAL_OP, 2, GETPC());
     }
 
     old_exc = cs->exception_index;
@@ -2185,7 +2178,6 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
     const uint8_t psw_as = (env->psw.mask & PSW_MASK_ASC) >> PSW_SHIFT_ASC;
     const uint64_t r0 = env->regs[0];
     const uintptr_t ra = GETPC();
-    CPUState *cs = CPU(s390_env_get_cpu(env));
     uint8_t dest_key, dest_as, dest_k, dest_a;
     uint8_t src_key, src_as, src_k, src_a;
     uint64_t val;
@@ -2195,8 +2187,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
                __func__, dest, src, len);
 
     if (!(env->psw.mask & PSW_MASK_DAT)) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIAL_OP, 6);
+        program_interrupt_ra(env, PGM_SPECIAL_OP, 6, ra);
     }
 
     /* OAC (operand access control) for the first operand -> dest */
@@ -2227,17 +2218,14 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 
     if (dest_a && dest_as == AS_HOME && (env->psw.mask & PSW_MASK_PSTATE)) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIAL_OP, 6);
+        program_interrupt_ra(env, PGM_SPECIAL_OP, 6, ra);
     }
     if (!(env->cregs[0] & CR0_SECONDARY) &&
         (dest_as == AS_SECONDARY || src_as == AS_SECONDARY)) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_SPECIAL_OP, 6);
+        program_interrupt_ra(env, PGM_SPECIAL_OP, 6, ra);
     }
     if (!psw_key_valid(env, dest_key) || !psw_key_valid(env, src_key)) {
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_PRIVILEGED, 6);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 6, ra);
     }
 
     len = wrap_length(env, len);
@@ -2251,8 +2239,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
         (env->psw.mask & PSW_MASK_PSTATE)) {
         qemu_log_mask(LOG_UNIMP, "%s: AR-mode and PSTATE support missing\n",
                       __func__);
-        cpu_restore_state(cs, ra);
-        program_interrupt(env, PGM_ADDRESSING, 6);
+        program_interrupt_ra(env, PGM_ADDRESSING, 6, ra);
     }
 
     /* FIXME: a) LAP
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index d272851e1c..fbb446db70 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -519,8 +519,7 @@ uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
     int i;
 
     if (addr & 0x7) {
-        cpu_restore_state(ENV_GET_CPU(env), ra);
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
     }
 
     prepare_stfl();
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 01/15] s390x/tcg: introduce and use program_interrupt_ra()

[Cornelia Huck]

On Tue, 28 Nov 2017 21:33:11 +0100
David Hildenbrand <david@redhat.com> wrote:

> Allows to easily convert more callers of program_interrupt() and to
> easily introduce new exceptions without forgetting about the cpu state
> reset.
> 
> Use program_interrupt_ra() in places where we already had the same
> pattern.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h           |  2 ++
>  target/s390x/crypto_helper.c |  7 ++-----
>  target/s390x/excp_helper.c   |  5 +----
>  target/s390x/interrupt.c     | 13 +++++++++++++
>  target/s390x/mem_helper.c    | 35 +++++++++++------------------------
>  target/s390x/misc_helper.c   |  3 +--
>  6 files changed, 30 insertions(+), 35 deletions(-)

> diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
> index ce6177c141..6ce06bb549 100644
> --- a/target/s390x/interrupt.c
> +++ b/target/s390x/interrupt.c
> @@ -53,6 +53,19 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
>      }
>  }
>  
> +void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
> +                          uintptr_t ra)
> +{
> +    S390CPU *cpu = s390_env_get_cpu(env);

Move this under the if?

> +
> +#ifdef CONFIG_TCG
> +    if (tcg_enabled() && ra) {
> +        cpu_restore_state(CPU(cpu), ra);
> +    }
> +#endif
> +    program_interrupt(env, code, ilen);
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static void cpu_inject_service(S390CPU *cpu, uint32_t param)
>  {

Re: [Qemu-devel] [PATCH v1 for-2.12 01/15] s390x/tcg: introduce and use program_interrupt_ra()

[David Hildenbrand]

On 29.11.2017 15:55, Cornelia Huck wrote:
> On Tue, 28 Nov 2017 21:33:11 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> Allows to easily convert more callers of program_interrupt() and to
>> easily introduce new exceptions without forgetting about the cpu state
>> reset.
>>
>> Use program_interrupt_ra() in places where we already had the same
>> pattern.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  target/s390x/cpu.h           |  2 ++
>>  target/s390x/crypto_helper.c |  7 ++-----
>>  target/s390x/excp_helper.c   |  5 +----
>>  target/s390x/interrupt.c     | 13 +++++++++++++
>>  target/s390x/mem_helper.c    | 35 +++++++++++------------------------
>>  target/s390x/misc_helper.c   |  3 +--
>>  6 files changed, 30 insertions(+), 35 deletions(-)
> 
>> diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
>> index ce6177c141..6ce06bb549 100644
>> --- a/target/s390x/interrupt.c
>> +++ b/target/s390x/interrupt.c
>> @@ -53,6 +53,19 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
>>      }
>>  }
>>  
>> +void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
>> +                          uintptr_t ra)
>> +{
>> +    S390CPU *cpu = s390_env_get_cpu(env);
> 
> Move this under the if?
> 

Indeed, thanks.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 01/15] s390x/tcg: introduce and use program_interrupt_ra()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> +    S390CPU *cpu = s390_env_get_cpu(env);
> +
> +#ifdef CONFIG_TCG
> +    if (tcg_enabled() && ra) {
> +        cpu_restore_state(CPU(cpu), ra);
> +    }
> +#endif

FWIW, I have a patch queued for 2.12 that removes the RA != 0 check protecting
calls to cpu_restore_state.  That condition is already handled inside and we
don't need to duplicate it.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 02/15] s390x/tcg: get rid of runtime_exception()

[David Hildenbrand]

Let's use program_interrupt_ra() instead.

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

diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index ffbeb3b2df..0ba347668c 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);
+    program_interrupt_ra(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..f384b14eab 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());
+        program_interrupt_ra(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());
+        program_interrupt_ra(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());
+        program_interrupt_ra(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());
+        program_interrupt_ra(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());
+        program_interrupt_ra(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());
+        program_interrupt_ra(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());
+            program_interrupt_ra(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 fbb446db70..6f29f92b94 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)
 {
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 02/15] s390x/tcg: get rid of runtime_exception()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Let's use program_interrupt_ra() instead.
> 
> 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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 03/15] s390x/tcg: rip out dead tpi code

[David Hildenbrand]

It is broken and not even wired up. We'll add a new handler soon, but
that will live somewhere else.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/css.c          |  6 ------
 include/hw/s390x/css.h  |  1 -
 target/s390x/internal.h |  1 -
 target/s390x/ioinst.c   | 26 --------------------------
 4 files changed, 34 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index f6b5c807cd..6bd0fedc78 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1723,12 +1723,6 @@ void css_undo_stcrw(CRW *crw)
     QTAILQ_INSERT_HEAD(&channel_subsys.pending_crws, crw_cont, sibling);
 }
 
-int css_do_tpi(IOIntCode *int_code, int lowcore)
-{
-    /* No pending interrupts for !KVM. */
-    return 0;
- }
-
 int css_collect_chp_desc(int m, uint8_t cssid, uint8_t f_chpid, uint8_t l_chpid,
                          int rfmt, void *buf)
 {
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index ab6ebe66b5..0a14f76fea 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -248,7 +248,6 @@ int css_do_tsch_get_irb(SubchDev *sch, IRB *irb, int *irb_len);
 void css_do_tsch_update_subch(SubchDev *sch);
 int css_do_stcrw(CRW *crw);
 void css_undo_stcrw(CRW *crw);
-int css_do_tpi(IOIntCode *int_code, int lowcore);
 int css_collect_chp_desc(int m, uint8_t cssid, uint8_t f_chpid, uint8_t l_chpid,
                          int rfmt, void *buf);
 void css_do_schm(uint8_t mbk, int update, int dct, uint64_t mbo);
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index db39d5bfac..603b0d7a7c 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -388,7 +388,6 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb);
 void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
 int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
 void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb);
-int ioinst_handle_tpi(S390CPU *cpu, uint32_t ipb);
 void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
                         uint32_t ipb);
 void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1);
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 23962fbebc..1d6857c14d 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -647,32 +647,6 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
     }
 }
 
-int ioinst_handle_tpi(S390CPU *cpu, uint32_t ipb)
-{
-    CPUS390XState *env = &cpu->env;
-    uint64_t addr;
-    int lowcore;
-    IOIntCode int_code;
-    hwaddr len;
-    int ret;
-    uint8_t ar;
-
-    trace_ioinst("tpi");
-    addr = decode_basedisp_s(env, ipb, &ar);
-    if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
-        return -EIO;
-    }
-
-    lowcore = addr ? 0 : 1;
-    len = lowcore ? 8 /* two words */ : 12 /* three words */;
-    ret = css_do_tpi(&int_code, lowcore);
-    if (ret == 1) {
-        s390_cpu_virt_mem_write(cpu, lowcore ? 184 : addr, ar, &int_code, len);
-    }
-    return ret;
-}
-
 #define SCHM_REG1_RES(_reg) (_reg & 0x000000000ffffffc)
 #define SCHM_REG1_MBK(_reg) ((_reg & 0x00000000f0000000) >> 28)
 #define SCHM_REG1_UPD(_reg) ((_reg & 0x0000000000000002) >> 1)
-- 
2.14.3

Re: [Qemu-devel] [qemu-s390x] [PATCH v1 for-2.12 03/15] s390x/tcg: rip out dead tpi code

[Thomas Huth]

On 28.11.2017 21:33, David Hildenbrand wrote:
> It is broken and not even wired up. We'll add a new handler soon, but
> that will live somewhere else.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/css.c          |  6 ------
>  include/hw/s390x/css.h  |  1 -
>  target/s390x/internal.h |  1 -
>  target/s390x/ioinst.c   | 26 --------------------------
>  4 files changed, 34 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v1 for-2.12 03/15] s390x/tcg: rip out dead tpi code

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> It is broken and not even wired up. We'll add a new handler soon, but
> that will live somewhere else.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/css.c          |  6 ------
>  include/hw/s390x/css.h  |  1 -
>  target/s390x/internal.h |  1 -
>  target/s390x/ioinst.c   | 26 --------------------------
>  4 files changed, 34 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[David Hildenbrand]

TCG needs the retaddr when injecting an interrupt. Let's just pass it
along and use 0 for KVM. The value will be completely ignored for KVM.

Convert program_interrupt() to program_interrupt_ra() directly, making
use of the passed address.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/internal.h    | 29 +++++++++++---------
 target/s390x/ioinst.c      | 67 +++++++++++++++++++++++-----------------------
 target/s390x/kvm.c         | 26 +++++++++---------
 target/s390x/misc_helper.c | 20 +++++++-------
 4 files changed, 73 insertions(+), 69 deletions(-)

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 603b0d7a7c..9db5f2d49d 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -379,20 +379,23 @@ void cpu_inject_stop(S390CPU *cpu);
 
 
 /* ioinst.c */
-void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1);
-void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1);
-void ioinst_handle_hsch(S390CPU *cpu, uint64_t reg1);
-void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
-void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
-void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb);
-void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
-int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb);
-void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb);
+void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
+void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
+void ioinst_handle_hsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
+void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
+                        uintptr_t ra);
+void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
+                        uintptr_t ra);
+void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra);
+void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
+                         uintptr_t ra);
+int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra);
+void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra);
 void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
-                        uint32_t ipb);
-void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1);
-void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1);
-void ioinst_handle_sal(S390CPU *cpu, uint64_t reg1);
+                        uint32_t ipb, uintptr_t ra);
+void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
+void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
+void ioinst_handle_sal(S390CPU *cpu, uint64_t reg1, uintptr_t ra);
 
 
 /* mem_helper.c */
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 1d6857c14d..4b6d38f946 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -38,13 +38,13 @@ int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int *ssid,
     return 0;
 }
 
-void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(&cpu->env, PGM_OPERAND, 4);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("xsch", cssid, ssid, schid);
@@ -56,13 +56,13 @@ void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1)
     setcc(cpu, css_do_xsch(sch));
 }
 
-void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(&cpu->env, PGM_OPERAND, 4);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("csch", cssid, ssid, schid);
@@ -74,13 +74,13 @@ void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1)
     setcc(cpu, css_do_csch(sch));
 }
 
-void ioinst_handle_hsch(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_hsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(&cpu->env, PGM_OPERAND, 4);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("hsch", cssid, ssid, schid);
@@ -105,7 +105,7 @@ static int ioinst_schib_valid(SCHIB *schib)
     return 1;
 }
 
-void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
+void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
@@ -116,7 +116,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &schib, sizeof(schib))) {
@@ -124,7 +124,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
     }
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
         !ioinst_schib_valid(&schib)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("msch", cssid, ssid, schid);
@@ -161,7 +161,7 @@ static int ioinst_orb_valid(ORB *orb)
     return 1;
 }
 
-void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
+void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
@@ -172,7 +172,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &orig_orb, sizeof(orb))) {
@@ -181,7 +181,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
     copy_orb_from_guest(&orb, &orig_orb);
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
         !ioinst_orb_valid(&orb)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("ssch", cssid, ssid, schid);
@@ -193,7 +193,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
     setcc(cpu, css_do_ssch(sch, &orb));
 }
 
-void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb)
+void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 {
     CRW crw;
     uint64_t addr;
@@ -203,7 +203,7 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return;
     }
 
@@ -218,7 +218,8 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb)
     }
 }
 
-void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
+void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
+                         uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
@@ -230,7 +231,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return;
     }
 
@@ -241,7 +242,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
          * access execption if it is not) first.
          */
         if (!s390_cpu_virt_mem_check_write(cpu, addr, ar, sizeof(schib))) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         }
         return;
     }
@@ -278,7 +279,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
     setcc(cpu, cc);
 }
 
-int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
+int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     int cssid, ssid, schid, m;
@@ -289,13 +290,13 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb)
     uint8_t ar;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return -EIO;
     }
     trace_ioinst_sch_id("tsch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return -EIO;
     }
 
@@ -585,7 +586,7 @@ static void ioinst_handle_chsc_unimplemented(ChscResp *res)
     res->param = 0;
 }
 
-void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
+void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 {
     ChscReq *req;
     ChscResp *res;
@@ -601,7 +602,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
     addr = env->regs[reg];
     /* Page boundary? */
     if (addr & 0xfff) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return;
     }
     /*
@@ -616,7 +617,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
     len = be16_to_cpu(req->len);
     /* Length field valid? */
     if ((len < 16) || (len > 4088) || (len & 7)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
     memset((char *)req + len, 0, TARGET_PAGE_SIZE - len);
@@ -653,7 +654,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
 #define SCHM_REG1_DCT(_reg) (_reg & 0x0000000000000001)
 
 void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
-                        uint32_t ipb)
+                        uint32_t ipb, uintptr_t ra)
 {
     uint8_t mbk;
     int update;
@@ -663,7 +664,7 @@ void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
     trace_ioinst("schm");
 
     if (SCHM_REG1_RES(reg1)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
 
@@ -672,20 +673,20 @@ void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
     dct = SCHM_REG1_DCT(reg1);
 
     if (update && (reg2 & 0x000000000000001f)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
 
     css_do_schm(mbk, update, dct, update ? reg2 : 0);
 }
 
-void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     int cssid, ssid, schid, m;
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(&cpu->env, PGM_OPERAND, 4);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 4, ra);
         return;
     }
     trace_ioinst_sch_id("rsch", cssid, ssid, schid);
@@ -700,7 +701,7 @@ void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1)
 #define RCHP_REG1_RES(_reg) (_reg & 0x00000000ff00ff00)
 #define RCHP_REG1_CSSID(_reg) ((_reg & 0x0000000000ff0000) >> 16)
 #define RCHP_REG1_CHPID(_reg) (_reg & 0x00000000000000ff)
-void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     int cc;
     uint8_t cssid;
@@ -709,7 +710,7 @@ void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1)
     CPUS390XState *env = &cpu->env;
 
     if (RCHP_REG1_RES(reg1)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
 
@@ -732,17 +733,17 @@ void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1)
         break;
     default:
         /* Invalid channel subsystem. */
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return;
     }
     setcc(cpu, cc);
 }
 
 #define SAL_REG1_INVALID(_reg) (_reg & 0x0000000080000000)
-void ioinst_handle_sal(S390CPU *cpu, uint64_t reg1)
+void ioinst_handle_sal(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     /* We do not provide address limit checking, so let's suppress it. */
     if (SAL_REG1_INVALID(reg1) || reg1 & 0x000000000000ffff) {
-        program_interrupt(&cpu->env, PGM_OPERAND, 4);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 4, ra);
     }
 }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index b03f583032..7c7aa0fa5a 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1124,32 +1124,32 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 
     switch (ipa1) {
     case PRIV_B2_XSCH:
-        ioinst_handle_xsch(cpu, env->regs[1]);
+        ioinst_handle_xsch(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_CSCH:
-        ioinst_handle_csch(cpu, env->regs[1]);
+        ioinst_handle_csch(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_HSCH:
-        ioinst_handle_hsch(cpu, env->regs[1]);
+        ioinst_handle_hsch(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_MSCH:
-        ioinst_handle_msch(cpu, env->regs[1], run->s390_sieic.ipb);
+        ioinst_handle_msch(cpu, env->regs[1], run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_SSCH:
-        ioinst_handle_ssch(cpu, env->regs[1], run->s390_sieic.ipb);
+        ioinst_handle_ssch(cpu, env->regs[1], run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_STCRW:
-        ioinst_handle_stcrw(cpu, run->s390_sieic.ipb);
+        ioinst_handle_stcrw(cpu, run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_STSCH:
-        ioinst_handle_stsch(cpu, env->regs[1], run->s390_sieic.ipb);
+        ioinst_handle_stsch(cpu, env->regs[1], run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_TSCH:
         /* We should only get tsch via KVM_EXIT_S390_TSCH. */
         fprintf(stderr, "Spurious tsch intercept\n");
         break;
     case PRIV_B2_CHSC:
-        ioinst_handle_chsc(cpu, run->s390_sieic.ipb);
+        ioinst_handle_chsc(cpu, run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_TPI:
         /* This should have been handled by kvm already. */
@@ -1157,19 +1157,19 @@ static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
         break;
     case PRIV_B2_SCHM:
         ioinst_handle_schm(cpu, env->regs[1], env->regs[2],
-                           run->s390_sieic.ipb);
+                           run->s390_sieic.ipb, 0);
         break;
     case PRIV_B2_RSCH:
-        ioinst_handle_rsch(cpu, env->regs[1]);
+        ioinst_handle_rsch(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_RCHP:
-        ioinst_handle_rchp(cpu, env->regs[1]);
+        ioinst_handle_rchp(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_STCPS:
         /* We do not provide this instruction, it is suppressed. */
         break;
     case PRIV_B2_SAL:
-        ioinst_handle_sal(cpu, env->regs[1]);
+        ioinst_handle_sal(cpu, env->regs[1], 0);
         break;
     case PRIV_B2_SIGA:
         /* Not provided, set CC = 3 for subchannel not operational */
@@ -1673,7 +1673,7 @@ static int handle_tsch(S390CPU *cpu)
 
     cpu_synchronize_state(cs);
 
-    ret = ioinst_handle_tsch(cpu, cpu->env.regs[1], run->s390_tsch.ipb);
+    ret = ioinst_handle_tsch(cpu, cpu->env.regs[1], run->s390_tsch.ipb, 0);
     if (ret < 0) {
         /*
          * Failure.
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 6f29f92b94..a8a1958b06 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -323,7 +323,7 @@ void HELPER(xsch)(CPUS390XState *env, uint64_t r1)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_xsch(cpu, r1);
+    ioinst_handle_xsch(cpu, r1, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -331,7 +331,7 @@ void HELPER(csch)(CPUS390XState *env, uint64_t r1)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_csch(cpu, r1);
+    ioinst_handle_csch(cpu, r1, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -339,7 +339,7 @@ void HELPER(hsch)(CPUS390XState *env, uint64_t r1)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_hsch(cpu, r1);
+    ioinst_handle_hsch(cpu, r1, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -347,7 +347,7 @@ void HELPER(msch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_msch(cpu, r1, inst >> 16);
+    ioinst_handle_msch(cpu, r1, inst >> 16, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -355,7 +355,7 @@ void HELPER(rchp)(CPUS390XState *env, uint64_t r1)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_rchp(cpu, r1);
+    ioinst_handle_rchp(cpu, r1, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -363,7 +363,7 @@ void HELPER(rsch)(CPUS390XState *env, uint64_t r1)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_rsch(cpu, r1);
+    ioinst_handle_rsch(cpu, r1, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -371,7 +371,7 @@ void HELPER(ssch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_ssch(cpu, r1, inst >> 16);
+    ioinst_handle_ssch(cpu, r1, inst >> 16, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -379,7 +379,7 @@ void HELPER(stsch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_stsch(cpu, r1, inst >> 16);
+    ioinst_handle_stsch(cpu, r1, inst >> 16, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -387,7 +387,7 @@ void HELPER(tsch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_tsch(cpu, r1, inst >> 16);
+    ioinst_handle_tsch(cpu, r1, inst >> 16, GETPC());
     qemu_mutex_unlock_iothread();
 }
 
@@ -395,7 +395,7 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     qemu_mutex_lock_iothread();
-    ioinst_handle_chsc(cpu, inst >> 16);
+    ioinst_handle_chsc(cpu, inst >> 16, GETPC());
     qemu_mutex_unlock_iothread();
 }
 #endif
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[Cornelia Huck]

On Tue, 28 Nov 2017 21:33:14 +0100
David Hildenbrand <david@redhat.com> wrote:

> TCG needs the retaddr when injecting an interrupt. Let's just pass it
> along and use 0 for KVM. The value will be completely ignored for KVM.

Can we get a #define for that? Just to make it clear at a glance that
we're passing an ignored value and not some magic thing.

> 
> Convert program_interrupt() to program_interrupt_ra() directly, making
> use of the passed address.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/internal.h    | 29 +++++++++++---------
>  target/s390x/ioinst.c      | 67 +++++++++++++++++++++++-----------------------
>  target/s390x/kvm.c         | 26 +++++++++---------
>  target/s390x/misc_helper.c | 20 +++++++-------
>  4 files changed, 73 insertions(+), 69 deletions(-)

Seems sane.

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[David Hildenbrand]

On 29.11.2017 16:13, Cornelia Huck wrote:
> On Tue, 28 Nov 2017 21:33:14 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
>> along and use 0 for KVM. The value will be completely ignored for KVM.
> 
> Can we get a #define for that? Just to make it clear at a glance that
> we're passing an ignored value and not some magic thing.

If you can come up with a good name, I can't :)


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[Cornelia Huck]

On Wed, 29 Nov 2017 16:14:40 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 29.11.2017 16:13, Cornelia Huck wrote:
> > On Tue, 28 Nov 2017 21:33:14 +0100
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> TCG needs the retaddr when injecting an interrupt. Let's just pass it
> >> along and use 0 for KVM. The value will be completely ignored for KVM.  
> > 
> > Can we get a #define for that? Just to make it clear at a glance that
> > we're passing an ignored value and not some magic thing.  
> 
> If you can come up with a good name, I can't :)

KVM_FAKE_RA
KVM_RA_UNUSED
KVM_RA_IGNORED

?

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[David Hildenbrand]

On 29.11.2017 16:35, Cornelia Huck wrote:
> On Wed, 29 Nov 2017 16:14:40 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 29.11.2017 16:13, Cornelia Huck wrote:
>>> On Tue, 28 Nov 2017 21:33:14 +0100
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
>>>> along and use 0 for KVM. The value will be completely ignored for KVM.  
>>>
>>> Can we get a #define for that? Just to make it clear at a glance that
>>> we're passing an ignored value and not some magic thing.  
>>
>> If you can come up with a good name, I can't :)
> 
> KVM_FAKE_RA
> KVM_RA_UNUSED
> KVM_RA_IGNORED
> 
> ?
> 

KVM_RA ? (to keep lines short?)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[Cornelia Huck]

On Wed, 29 Nov 2017 16:38:03 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 29.11.2017 16:35, Cornelia Huck wrote:
> > On Wed, 29 Nov 2017 16:14:40 +0100
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> On 29.11.2017 16:13, Cornelia Huck wrote:  
> >>> On Tue, 28 Nov 2017 21:33:14 +0100
> >>> David Hildenbrand <david@redhat.com> wrote:
> >>>     
> >>>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
> >>>> along and use 0 for KVM. The value will be completely ignored for KVM.    
> >>>
> >>> Can we get a #define for that? Just to make it clear at a glance that
> >>> we're passing an ignored value and not some magic thing.    
> >>
> >> If you can come up with a good name, I can't :)  
> > 
> > KVM_FAKE_RA
> > KVM_RA_UNUSED
> > KVM_RA_IGNORED
> > 
> > ?
> >   
> 
> KVM_RA ? (to keep lines short?)
> 

That sounds too much like a magic value IMO (and not something that is
ignored).

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[David Hildenbrand]

On 29.11.2017 16:39, Cornelia Huck wrote:
> On Wed, 29 Nov 2017 16:38:03 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 29.11.2017 16:35, Cornelia Huck wrote:
>>> On Wed, 29 Nov 2017 16:14:40 +0100
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> On 29.11.2017 16:13, Cornelia Huck wrote:  
>>>>> On Tue, 28 Nov 2017 21:33:14 +0100
>>>>> David Hildenbrand <david@redhat.com> wrote:
>>>>>     
>>>>>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
>>>>>> along and use 0 for KVM. The value will be completely ignored for KVM.    
>>>>>
>>>>> Can we get a #define for that? Just to make it clear at a glance that
>>>>> we're passing an ignored value and not some magic thing.    
>>>>
>>>> If you can come up with a good name, I can't :)  
>>>
>>> KVM_FAKE_RA
>>> KVM_RA_UNUSED
>>> KVM_RA_IGNORED
>>>
>>> ?
>>>   
>>
>> KVM_RA ? (to keep lines short?)
>>
> 
> That sounds too much like a magic value IMO (and not something that is
> ignored).
> 

Passing 0 will actually also ignore it for TCG. So it is actually a
magic value :) (just that more gets ignore din case of KVM ...)

RA_NONE ?


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[Cornelia Huck]

On Wed, 29 Nov 2017 16:45:21 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 29.11.2017 16:39, Cornelia Huck wrote:
> > On Wed, 29 Nov 2017 16:38:03 +0100
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> On 29.11.2017 16:35, Cornelia Huck wrote:  
> >>> On Wed, 29 Nov 2017 16:14:40 +0100
> >>> David Hildenbrand <david@redhat.com> wrote:
> >>>     
> >>>> On 29.11.2017 16:13, Cornelia Huck wrote:    
> >>>>> On Tue, 28 Nov 2017 21:33:14 +0100
> >>>>> David Hildenbrand <david@redhat.com> wrote:
> >>>>>       
> >>>>>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
> >>>>>> along and use 0 for KVM. The value will be completely ignored for KVM.      
> >>>>>
> >>>>> Can we get a #define for that? Just to make it clear at a glance that
> >>>>> we're passing an ignored value and not some magic thing.      
> >>>>
> >>>> If you can come up with a good name, I can't :)    
> >>>
> >>> KVM_FAKE_RA
> >>> KVM_RA_UNUSED
> >>> KVM_RA_IGNORED
> >>>
> >>> ?
> >>>     
> >>
> >> KVM_RA ? (to keep lines short?)
> >>  
> > 
> > That sounds too much like a magic value IMO (and not something that is
> > ignored).
> >   
> 
> Passing 0 will actually also ignore it for TCG. So it is actually a
> magic value :) (just that more gets ignore din case of KVM ...)
> 
> RA_NONE ?
> 
> 

RA_IGNORED?

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[David Hildenbrand]

On 29.11.2017 16:57, Cornelia Huck wrote:
> On Wed, 29 Nov 2017 16:45:21 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 29.11.2017 16:39, Cornelia Huck wrote:
>>> On Wed, 29 Nov 2017 16:38:03 +0100
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> On 29.11.2017 16:35, Cornelia Huck wrote:  
>>>>> On Wed, 29 Nov 2017 16:14:40 +0100
>>>>> David Hildenbrand <david@redhat.com> wrote:
>>>>>     
>>>>>> On 29.11.2017 16:13, Cornelia Huck wrote:    
>>>>>>> On Tue, 28 Nov 2017 21:33:14 +0100
>>>>>>> David Hildenbrand <david@redhat.com> wrote:
>>>>>>>       
>>>>>>>> TCG needs the retaddr when injecting an interrupt. Let's just pass it
>>>>>>>> along and use 0 for KVM. The value will be completely ignored for KVM.      
>>>>>>>
>>>>>>> Can we get a #define for that? Just to make it clear at a glance that
>>>>>>> we're passing an ignored value and not some magic thing.      
>>>>>>
>>>>>> If you can come up with a good name, I can't :)    
>>>>>
>>>>> KVM_FAKE_RA
>>>>> KVM_RA_UNUSED
>>>>> KVM_RA_IGNORED
>>>>>
>>>>> ?
>>>>>     
>>>>
>>>> KVM_RA ? (to keep lines short?)
>>>>  
>>>
>>> That sounds too much like a magic value IMO (and not something that is
>>> ignored).
>>>   
>>
>> Passing 0 will actually also ignore it for TCG. So it is actually a
>> magic value :) (just that more gets ignore din case of KVM ...)
>>
>> RA_NONE ?
>>
>>
> 
> RA_IGNORED?
> 

jap, will use that.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 04/15] s390x/ioinst: pass the retaddr to all IO instructions

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> TCG needs the retaddr when injecting an interrupt. Let's just pass it
> along and use 0 for KVM. The value will be completely ignored for KVM.
> 
> Convert program_interrupt() to program_interrupt_ra() directly, making
> use of the passed address.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/internal.h    | 29 +++++++++++---------
>  target/s390x/ioinst.c      | 67 +++++++++++++++++++++++-----------------------
>  target/s390x/kvm.c         | 26 +++++++++---------
>  target/s390x/misc_helper.c | 20 +++++++-------
>  4 files changed, 73 insertions(+), 69 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

... with or without RA_IGNORED.  ;-)

r~

[Qemu-devel] [PATCH v1 for-2.12 05/15] s390x/pci: pass the retaddr to all PCI instructions

[David Hildenbrand]

Once we wire up TCG, we will need the retaddr to correctly inject
program interrupts. As we want to get rid of the function
program_interrupt(), convert PCI code too.

For KVM, we can simply pass a 0.

Convert program_interrupt() to program_interrupt_ra() directly, making
use of the passed address.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-pci-inst.c | 83 +++++++++++++++++++++++++-----------------------
 hw/s390x/s390-pci-inst.h | 16 ++++++----
 target/s390x/kvm.c       | 14 ++++----
 3 files changed, 59 insertions(+), 54 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 8e088f3dc9..15ba8f55ae 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -142,7 +142,7 @@ out:
     return rc;
 }
 
-int clp_service_call(S390CPU *cpu, uint8_t r2)
+int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
 {
     ClpReqHdr *reqh;
     ClpRspHdr *resh;
@@ -158,7 +158,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
     cpu_synchronize_state(CPU(cpu));
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 4);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 4, ra);
         return 0;
     }
 
@@ -168,7 +168,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
     reqh = (ClpReqHdr *)buffer;
     req_len = lduw_p(&reqh->len);
     if (req_len < 16 || req_len > 8184 || (req_len % 8 != 0)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return 0;
     }
 
@@ -179,11 +179,11 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
     resh = (ClpRspHdr *)(buffer + req_len);
     res_len = lduw_p(&resh->len);
     if (res_len < 8 || res_len > 8176 || (res_len % 8 != 0)) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return 0;
     }
     if ((req_len + res_len) > 8192) {
-        program_interrupt(env, PGM_OPERAND, 4);
+        program_interrupt_ra(env, PGM_OPERAND, 4, ra);
         return 0;
     }
 
@@ -314,7 +314,7 @@ out:
     return 0;
 }
 
-int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     S390PCIBusDevice *pbdev;
@@ -329,12 +329,12 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     cpu_synchronize_state(CPU(cpu));
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 4);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 4, ra);
         return 0;
     }
 
     if (r2 & 0x1) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return 0;
     }
 
@@ -367,19 +367,19 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
 
     if (pcias < 6) {
         if ((8 - (offset & 0x7)) < len) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
         mr = pbdev->pdev->io_regions[pcias].memory;
         result = memory_region_dispatch_read(mr, offset, &data, len,
                                              MEMTXATTRS_UNSPECIFIED);
         if (result != MEMTX_OK) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
     } else if (pcias == 15) {
         if ((4 - (offset & 0x3)) < len) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
         data =  pci_host_config_read_common(
@@ -398,7 +398,7 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
             data = bswap64(data);
             break;
         default:
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
     } else {
@@ -425,7 +425,7 @@ static int trap_msix(S390PCIBusDevice *pbdev, uint64_t offset, uint8_t pcias)
     }
 }
 
-int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     uint64_t offset, data;
@@ -439,12 +439,12 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     cpu_synchronize_state(CPU(cpu));
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 4);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 4, ra);
         return 0;
     }
 
     if (r2 & 0x1) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         return 0;
     }
 
@@ -478,7 +478,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     data = env->regs[r1];
     if (pcias < 6) {
         if ((8 - (offset & 0x7)) < len) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
 
@@ -492,12 +492,12 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
         result = memory_region_dispatch_write(mr, offset, data, len,
                                      MEMTXATTRS_UNSPECIFIED);
         if (result != MEMTX_OK) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
     } else if (pcias == 15) {
         if ((4 - (offset & 0x3)) < len) {
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
         switch (len) {
@@ -513,7 +513,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
             data = bswap64(data);
             break;
         default:
-            program_interrupt(env, PGM_OPERAND, 4);
+            program_interrupt_ra(env, PGM_OPERAND, 4, ra);
             return 0;
         }
 
@@ -531,7 +531,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     return 0;
 }
 
-int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     uint32_t fh;
@@ -545,12 +545,12 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     cpu_synchronize_state(CPU(cpu));
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 4);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 4, ra);
         goto out;
     }
 
     if (r2 & 0x1) {
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, ra);
         goto out;
     }
 
@@ -624,7 +624,7 @@ out:
 }
 
 int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
-                        uint8_t ar)
+                        uint8_t ar, uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     S390PCIBusDevice *pbdev;
@@ -637,7 +637,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     uint8_t buffer[128];
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 6);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 6, ra);
         return 0;
     }
 
@@ -659,7 +659,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     case 128:
         break;
     default:
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
         return 0;
     }
 
@@ -687,7 +687,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
 
     mr = pbdev->pdev->io_regions[pcias].memory;
     if (!memory_region_access_valid(mr, env->regs[r3], len, true)) {
-        program_interrupt(env, PGM_OPERAND, 6);
+        program_interrupt_ra(env, PGM_OPERAND, 6, ra);
         return 0;
     }
 
@@ -700,7 +700,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
                                      ldq_p(buffer + i * 8), 8,
                                      MEMTXATTRS_UNSPECIFIED);
         if (result != MEMTX_OK) {
-            program_interrupt(env, PGM_OPERAND, 6);
+            program_interrupt_ra(env, PGM_OPERAND, 6, ra);
             return 0;
         }
     }
@@ -767,7 +767,8 @@ int pci_dereg_irqs(S390PCIBusDevice *pbdev)
     return 0;
 }
 
-static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib)
+static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
+                    uintptr_t ra)
 {
     uint64_t pba = ldq_p(&fib.pba);
     uint64_t pal = ldq_p(&fib.pal);
@@ -776,14 +777,14 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib)
     uint8_t t = (g_iota >> 11) & 0x1;
 
     if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
-        program_interrupt(env, PGM_OPERAND, 6);
+        program_interrupt_ra(env, PGM_OPERAND, 6, ra);
         return -EINVAL;
     }
 
     /* currently we only support designation type 1 with translation */
     if (!(dt == ZPCI_IOTA_RTTO && t)) {
         error_report("unsupported ioat dt %d t %d", dt, t);
-        program_interrupt(env, PGM_OPERAND, 6);
+        program_interrupt_ra(env, PGM_OPERAND, 6, ra);
         return -EINVAL;
     }
 
@@ -804,7 +805,8 @@ void pci_dereg_ioat(S390PCIIOMMU *iommu)
     iommu->g_iota = 0;
 }
 
-int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
+int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
+                        uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     uint8_t oc, dmaas;
@@ -814,7 +816,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     uint64_t cc = ZPCI_PCI_LS_OK;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 6);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 6, ra);
         return 0;
     }
 
@@ -823,7 +825,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     fh = env->regs[r1] >> 32;
 
     if (fiba & 0x7) {
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
         return 0;
     }
 
@@ -850,7 +852,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     }
 
     if (fib.fmt != 0) {
-        program_interrupt(env, PGM_OPERAND, 6);
+        program_interrupt_ra(env, PGM_OPERAND, 6, ra);
         return 0;
     }
 
@@ -879,7 +881,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
         } else if (pbdev->iommu->enabled) {
             cc = ZPCI_PCI_LS_ERR;
             s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
-        } else if (reg_ioat(env, pbdev->iommu, fib)) {
+        } else if (reg_ioat(env, pbdev->iommu, fib, ra)) {
             cc = ZPCI_PCI_LS_ERR;
             s390_set_status_code(env, r1, ZPCI_MOD_ST_INSUF_RES);
         }
@@ -904,7 +906,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
             s390_set_status_code(env, r1, ZPCI_MOD_ST_SEQUENCE);
         } else {
             pci_dereg_ioat(pbdev->iommu);
-            if (reg_ioat(env, pbdev->iommu, fib)) {
+            if (reg_ioat(env, pbdev->iommu, fib, ra)) {
                 cc = ZPCI_PCI_LS_ERR;
                 s390_set_status_code(env, r1, ZPCI_MOD_ST_INSUF_RES);
             }
@@ -935,7 +937,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
         pbdev->fmb_addr = ldq_p(&fib.fmb_addr);
         break;
     default:
-        program_interrupt(&cpu->env, PGM_OPERAND, 6);
+        program_interrupt_ra(&cpu->env, PGM_OPERAND, 6, ra);
         cc = ZPCI_PCI_LS_ERR;
     }
 
@@ -943,7 +945,8 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     return 0;
 }
 
-int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
+int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
+                         uintptr_t ra)
 {
     CPUS390XState *env = &cpu->env;
     uint8_t dmaas;
@@ -954,7 +957,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     uint64_t cc = ZPCI_PCI_LS_OK;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, 6);
+        program_interrupt_ra(env, PGM_PRIVILEGED, 6, ra);
         return 0;
     }
 
@@ -968,7 +971,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
     }
 
     if (fiba & 0x7) {
-        program_interrupt(env, PGM_SPECIFICATION, 6);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 6, ra);
         return 0;
     }
 
diff --git a/hw/s390x/s390-pci-inst.h b/hw/s390x/s390-pci-inst.h
index 94a959f91c..93ef290101 100644
--- a/hw/s390x/s390-pci-inst.h
+++ b/hw/s390x/s390-pci-inst.h
@@ -293,13 +293,15 @@ typedef struct ZpciFib {
 
 int pci_dereg_irqs(S390PCIBusDevice *pbdev);
 void pci_dereg_ioat(S390PCIIOMMU *iommu);
-int clp_service_call(S390CPU *cpu, uint8_t r2);
-int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
-int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
-int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2);
+int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra);
+int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra);
+int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra);
+int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra);
 int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
-                        uint8_t ar);
-int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar);
-int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar);
+                        uint8_t ar, uintptr_t ra);
+int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
+                        uintptr_t ra);
+int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
+                         uintptr_t ra);
 
 #endif
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 7c7aa0fa5a..061abf9d4b 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1230,7 +1230,7 @@ static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
     if (s390_has_feat(S390_FEAT_ZPCI)) {
-        return clp_service_call(cpu, r2);
+        return clp_service_call(cpu, r2, 0);
     } else {
         return -1;
     }
@@ -1242,7 +1242,7 @@ static int kvm_pcilg_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
     if (s390_has_feat(S390_FEAT_ZPCI)) {
-        return pcilg_service_call(cpu, r1, r2);
+        return pcilg_service_call(cpu, r1, r2, 0);
     } else {
         return -1;
     }
@@ -1254,7 +1254,7 @@ static int kvm_pcistg_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
     if (s390_has_feat(S390_FEAT_ZPCI)) {
-        return pcistg_service_call(cpu, r1, r2);
+        return pcistg_service_call(cpu, r1, r2, 0);
     } else {
         return -1;
     }
@@ -1270,7 +1270,7 @@ static int kvm_stpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
         cpu_synchronize_state(CPU(cpu));
         fiba = get_base_disp_rxy(cpu, run, &ar);
 
-        return stpcifc_service_call(cpu, r1, fiba, ar);
+        return stpcifc_service_call(cpu, r1, fiba, ar, 0);
     } else {
         return -1;
     }
@@ -1302,7 +1302,7 @@ static int kvm_rpcit_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
     if (s390_has_feat(S390_FEAT_ZPCI)) {
-        return rpcit_service_call(cpu, r1, r2);
+        return rpcit_service_call(cpu, r1, r2, 0);
     } else {
         return -1;
     }
@@ -1319,7 +1319,7 @@ static int kvm_pcistb_service_call(S390CPU *cpu, struct kvm_run *run)
         cpu_synchronize_state(CPU(cpu));
         gaddr = get_base_disp_rsy(cpu, run, &ar);
 
-        return pcistb_service_call(cpu, r1, r3, gaddr, ar);
+        return pcistb_service_call(cpu, r1, r3, gaddr, ar, 0);
     } else {
         return -1;
     }
@@ -1335,7 +1335,7 @@ static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
         cpu_synchronize_state(CPU(cpu));
         fiba = get_base_disp_rxy(cpu, run, &ar);
 
-        return mpcifc_service_call(cpu, r1, fiba, ar);
+        return mpcifc_service_call(cpu, r1, fiba, ar, 0);
     } else {
         return -1;
     }
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 05/15] s390x/pci: pass the retaddr to all PCI instructions

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Once we wire up TCG, we will need the retaddr to correctly inject
> program interrupts. As we want to get rid of the function
> program_interrupt(), convert PCI code too.
> 
> For KVM, we can simply pass a 0.
> 
> Convert program_interrupt() to program_interrupt_ra() directly, making
> use of the passed address.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-pci-inst.c | 83 +++++++++++++++++++++++++-----------------------
>  hw/s390x/s390-pci-inst.h | 16 ++++++----
>  target/s390x/kvm.c       | 14 ++++----
>  3 files changed, 59 insertions(+), 54 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

with or without RA_IGNORED.


r~

[Qemu-devel] [PATCH v1 for-2.12 06/15] s390x/diag: pass the retaddr into handle_diag_308()

[David Hildenbrand]

Needed to later drop potential_page_fault() from the diag TCG translate
function.

Convert program_interrupt() to program_interrupt_ra() directly, making
use of the passed address.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/diag.c        | 14 +++++++-------
 target/s390x/internal.h    |  3 ++-
 target/s390x/kvm.c         |  2 +-
 target/s390x/misc_helper.c |  2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index dbbb9e886f..425a74efcd 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -99,19 +99,19 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
 #define DIAG_308_RC_NO_CONF         0x0102
 #define DIAG_308_RC_INVALID         0x0402
 
-void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3)
+void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
 {
     uint64_t addr =  env->regs[r1];
     uint64_t subcode = env->regs[r3];
     IplParameterBlock *iplb;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        program_interrupt(env, PGM_PRIVILEGED, ILEN_AUTO);
+        program_interrupt_ra(env, PGM_PRIVILEGED, ILEN_AUTO, ra);
         return;
     }
 
     if ((subcode & ~0x0ffffULL) || (subcode > 6)) {
-        program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO);
+        program_interrupt_ra(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
         return;
     }
 
@@ -136,12 +136,12 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3)
         break;
     case 5:
         if ((r1 & 1) || (addr & 0x0fffULL)) {
-            program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO);
+            program_interrupt_ra(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
                                         sizeof(IplParameterBlock), false)) {
-            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
+            program_interrupt_ra(env, PGM_ADDRESSING, ILEN_AUTO, ra);
             return;
         }
         iplb = g_new0(IplParameterBlock, 1);
@@ -165,12 +165,12 @@ out:
         return;
     case 6:
         if ((r1 & 1) || (addr & 0x0fffULL)) {
-            program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO);
+            program_interrupt_ra(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
                                         sizeof(IplParameterBlock), true)) {
-            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
+            program_interrupt_ra(env, PGM_ADDRESSING, ILEN_AUTO, ra);
             return;
         }
         iplb = s390_ipl_get_iplb();
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 9db5f2d49d..6817b2c432 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -411,7 +411,8 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
 
 /* misc_helper.c */
 int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
-void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3);
+void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
+                     uintptr_t ra);
 
 
 /* translate.c */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 061abf9d4b..7bd0679fda 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1451,7 +1451,7 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run)
     cpu_synchronize_state(CPU(cpu));
     r1 = (run->s390_sieic.ipa & 0x00f0) >> 4;
     r3 = run->s390_sieic.ipa & 0x000f;
-    handle_diag_308(&cpu->env, r1, r3);
+    handle_diag_308(&cpu->env, r1, r3, 0);
 }
 
 static int handle_sw_breakpoint(S390CPU *cpu, struct kvm_run *run)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index a8a1958b06..e50ea480be 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -88,7 +88,7 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
     case 0x308:
         /* ipl */
         qemu_mutex_lock_iothread();
-        handle_diag_308(env, r1, r3);
+        handle_diag_308(env, r1, r3, GETPC());
         qemu_mutex_unlock_iothread();
         r = 0;
         break;
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 06/15] s390x/diag: pass the retaddr into handle_diag_308()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Needed to later drop potential_page_fault() from the diag TCG translate
> function.
> 
> Convert program_interrupt() to program_interrupt_ra() directly, making
> use of the passed address.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/diag.c        | 14 +++++++-------
>  target/s390x/internal.h    |  3 ++-
>  target/s390x/kvm.c         |  2 +-
>  target/s390x/misc_helper.c |  2 +-
>  4 files changed, 11 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Likewise wrt RA_IGNORED.

r~

[Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[David Hildenbrand]

s390_cpu_virt_mem_rw() must always return, so callers can react on
an exception (e.g. see ioinst_handle_stcrw()). For TCG, there was one
case where a cpu loop exit was triggered. Fix that up.

However, for TCG we always have to exit the cpu loop (and restore the
cpu state before that) if we injected a program interrupt. So let's
introduce and use s390_cpu_virt_mem_handle_exc() in code that is not
purely KVM.

Directly pass the retaddr we already have available in these functions.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-pci-inst.c  |  7 +++++++
 target/s390x/cpu.h        |  1 +
 target/s390x/ioinst.c     | 21 ++++++++++++++++++---
 target/s390x/mmu_helper.c | 19 ++++++++++++++++++-
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 15ba8f55ae..3671432a29 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -163,6 +163,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
     }
 
     if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer, sizeof(*reqh))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
     reqh = (ClpReqHdr *)buffer;
@@ -174,6 +175,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
 
     if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer,
                                req_len + sizeof(*resh))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
     resh = (ClpRspHdr *)(buffer + req_len);
@@ -189,6 +191,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
 
     if (s390_cpu_virt_mem_read(cpu, env->regs[r2], r2, buffer,
                                req_len + res_len)) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
 
@@ -308,6 +311,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
 out:
     if (s390_cpu_virt_mem_write(cpu, env->regs[r2], r2, buffer,
                                 req_len + res_len)) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
     setcc(cpu, cc);
@@ -692,6 +696,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     }
 
     if (s390_cpu_virt_mem_read(cpu, gaddr, ar, buffer, len)) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
 
@@ -848,6 +853,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     }
 
     if (s390_cpu_virt_mem_read(cpu, fiba, ar, (uint8_t *)&fib, sizeof(fib))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
 
@@ -1029,6 +1035,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
 
 out:
     if (s390_cpu_virt_mem_write(cpu, fiba, ar, (uint8_t *)&fib, sizeof(fib))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return 0;
     }
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index e21d63ae04..617f9b0148 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -735,6 +735,7 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
         s390_cpu_virt_mem_rw(cpu, laddr, ar, dest, len, true)
 #define s390_cpu_virt_mem_check_write(cpu, laddr, ar, len)   \
         s390_cpu_virt_mem_rw(cpu, laddr, ar, NULL, len, true)
+void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra);
 
 
 /* sigp.c */
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 4b6d38f946..0851bf6fef 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -120,6 +120,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &schib, sizeof(schib))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return;
     }
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
@@ -176,6 +177,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &orig_orb, sizeof(orb))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return;
     }
     copy_orb_from_guest(&orb, &orig_orb);
@@ -212,9 +214,12 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 
     if (s390_cpu_virt_mem_write(cpu, addr, ar, &crw, sizeof(crw)) == 0) {
         setcc(cpu, cc);
-    } else if (cc == 0) {
-        /* Write failed: requeue CRW since STCRW is a suppressing instruction */
-        css_undo_stcrw(&crw);
+    } else {
+        if (cc == 0) {
+            /* Write failed: requeue CRW since STCRW is suppressing */
+            css_undo_stcrw(&crw);
+        }
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
     }
 }
 
@@ -243,6 +248,8 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
          */
         if (!s390_cpu_virt_mem_check_write(cpu, addr, ar, sizeof(schib))) {
             program_interrupt_ra(env, PGM_OPERAND, 4, ra);
+        } else {
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
         }
         return;
     }
@@ -268,11 +275,13 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
     if (cc != 3) {
         if (s390_cpu_virt_mem_write(cpu, addr, ar, &schib,
                                     sizeof(schib)) != 0) {
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
             return;
         }
     } else {
         /* Access exceptions have a higher priority than cc3 */
         if (s390_cpu_virt_mem_check_write(cpu, addr, ar, sizeof(schib)) != 0) {
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
             return;
         }
     }
@@ -309,6 +318,7 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     /* 0 - status pending, 1 - not status pending, 3 - not operational */
     if (cc != 3) {
         if (s390_cpu_virt_mem_write(cpu, addr, ar, &irb, irb_len) != 0) {
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
             return -EFAULT;
         }
         css_do_tsch_update_subch(sch);
@@ -316,6 +326,7 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
         irb_len = sizeof(irb) - sizeof(irb.emw);
         /* Access exceptions have a higher priority than cc3 */
         if (s390_cpu_virt_mem_check_write(cpu, addr, ar, irb_len) != 0) {
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
             return -EFAULT;
         }
     }
@@ -611,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
      * care of req->len here first.
      */
     if (s390_cpu_virt_mem_read(cpu, addr, reg, buf, sizeof(ChscReq))) {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
         return;
     }
     req = (ChscReq *)buf;
@@ -645,9 +657,12 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
     if (!s390_cpu_virt_mem_write(cpu, addr + len, reg, res,
                                  be16_to_cpu(res->len))) {
         setcc(cpu, 0);    /* Command execution complete */
+    } else {
+        s390_cpu_virt_mem_handle_exc(cpu, ra);
     }
 }
 
+
 #define SCHM_REG1_RES(_reg) (_reg & 0x000000000ffffffc)
 #define SCHM_REG1_MBK(_reg) ((_reg & 0x00000000f0000000) >> 28)
 #define SCHM_REG1_UPD(_reg) ((_reg & 0x0000000000000002) >> 1)
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 31e3f3f415..39da9aeef4 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -22,6 +22,7 @@
 #include "internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
+#include "exec/exec-all.h"
 #include "trace.h"
 #include "hw/s390x/storage-keys.h"
 
@@ -458,7 +459,7 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
         }
         if (!address_space_access_valid(&address_space_memory, pages[i],
                                         TARGET_PAGE_SIZE, is_write)) {
-            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
+            trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);
             return -EFAULT;
         }
         addr += TARGET_PAGE_SIZE;
@@ -478,6 +479,9 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
  *
  * Copy from/to guest memory using logical addresses. Note that we inject a
  * program interrupt in case there is an error while accessing the memory.
+ *
+ * This function will always return (also for TCG), make sure to call
+ * s390_cpu_virt_mem_handle_exc() to properly exit the CPU loop.
  */
 int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
                          int len, bool is_write)
@@ -514,6 +518,19 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
     return ret;
 }
 
+void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
+{
+    /* KVM will handle the interrupt automatically, TCG has to exit the TB */
+    #ifdef CONFIG_TCG
+    if (tcg_enabled()) {
+        if (ra) {
+            cpu_restore_state(CPU(cpu), ra);
+        }
+        cpu_loop_exit(CPU(cpu));
+    }
+    #endif
+}
+
 /**
  * Translate a real address into a physical (absolute) address.
  * @param raddr  the real address
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[Cornelia Huck]

On Tue, 28 Nov 2017 21:33:17 +0100
David Hildenbrand <david@redhat.com> wrote:

> s390_cpu_virt_mem_rw() must always return, so callers can react on
> an exception (e.g. see ioinst_handle_stcrw()). For TCG, there was one
> case where a cpu loop exit was triggered. Fix that up.
> 
> However, for TCG we always have to exit the cpu loop (and restore the
> cpu state before that) if we injected a program interrupt. So let's
> introduce and use s390_cpu_virt_mem_handle_exc() in code that is not
> purely KVM.
> 
> Directly pass the retaddr we already have available in these functions.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-pci-inst.c  |  7 +++++++
>  target/s390x/cpu.h        |  1 +
>  target/s390x/ioinst.c     | 21 ++++++++++++++++++---
>  target/s390x/mmu_helper.c | 19 ++++++++++++++++++-
>  4 files changed, 44 insertions(+), 4 deletions(-)
> 

> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index 4b6d38f946..0851bf6fef 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c

> @@ -243,6 +248,8 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
>           */
>          if (!s390_cpu_virt_mem_check_write(cpu, addr, ar, sizeof(schib))) {
>              program_interrupt_ra(env, PGM_OPERAND, 4, ra);
> +        } else {
> +            s390_cpu_virt_mem_handle_exc(cpu, ra);

This looks a bit odd if you don't realize that kvm already handled the
exception. But I don't really have a better idea.

>          }
>          return;
>      }

> @@ -645,9 +657,12 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
>      if (!s390_cpu_virt_mem_write(cpu, addr + len, reg, res,
>                                   be16_to_cpu(res->len))) {
>          setcc(cpu, 0);    /* Command execution complete */
> +    } else {
> +        s390_cpu_virt_mem_handle_exc(cpu, ra);
>      }
>  }
>  
> +

unrelated whitespace

>  #define SCHM_REG1_RES(_reg) (_reg & 0x000000000ffffffc)
>  #define SCHM_REG1_MBK(_reg) ((_reg & 0x00000000f0000000) >> 28)
>  #define SCHM_REG1_UPD(_reg) ((_reg & 0x0000000000000002) >> 1)
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 31e3f3f415..39da9aeef4 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -22,6 +22,7 @@
>  #include "internal.h"
>  #include "kvm_s390x.h"
>  #include "sysemu/kvm.h"
> +#include "exec/exec-all.h"
>  #include "trace.h"
>  #include "hw/s390x/storage-keys.h"
>  
> @@ -458,7 +459,7 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
>          }
>          if (!address_space_access_valid(&address_space_memory, pages[i],
>                                          TARGET_PAGE_SIZE, is_write)) {
> -            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
> +            trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);

Why did you change this?

>              return -EFAULT;
>          }
>          addr += TARGET_PAGE_SIZE;
> @@ -478,6 +479,9 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
>   *
>   * Copy from/to guest memory using logical addresses. Note that we inject a
>   * program interrupt in case there is an error while accessing the memory.
> + *
> + * This function will always return (also for TCG), make sure to call
> + * s390_cpu_virt_mem_handle_exc() to properly exit the CPU loop.
>   */
>  int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>                           int len, bool is_write)
> @@ -514,6 +518,19 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>      return ret;
>  }
>  
> +void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
> +{
> +    /* KVM will handle the interrupt automatically, TCG has to exit the TB */
> +    #ifdef CONFIG_TCG

Please move the #ifdef/#endif to the beginning of the line.

> +    if (tcg_enabled()) {
> +        if (ra) {
> +            cpu_restore_state(CPU(cpu), ra);
> +        }
> +        cpu_loop_exit(CPU(cpu));
> +    }
> +    #endif
> +}
> +
>  /**
>   * Translate a real address into a physical (absolute) address.
>   * @param raddr  the real address

Re: [Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[David Hildenbrand]

> This looks a bit odd if you don't realize that kvm already handled the
> exception. But I don't really have a better idea.
> 
>>          }
>>          return;
>>      }
> 
>> @@ -645,9 +657,12 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
>>      if (!s390_cpu_virt_mem_write(cpu, addr + len, reg, res,
>>                                   be16_to_cpu(res->len))) {
>>          setcc(cpu, 0);    /* Command execution complete */
>> +    } else {
>> +        s390_cpu_virt_mem_handle_exc(cpu, ra);
>>      }
>>  }
>>  
>> +
> 
> unrelated whitespace

ack, will drop.

> 
>>  #define SCHM_REG1_RES(_reg) (_reg & 0x000000000ffffffc)
>>  #define SCHM_REG1_MBK(_reg) ((_reg & 0x00000000f0000000) >> 28)
>>  #define SCHM_REG1_UPD(_reg) ((_reg & 0x0000000000000002) >> 1)
>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>> index 31e3f3f415..39da9aeef4 100644
>> --- a/target/s390x/mmu_helper.c
>> +++ b/target/s390x/mmu_helper.c
>> @@ -22,6 +22,7 @@
>>  #include "internal.h"
>>  #include "kvm_s390x.h"
>>  #include "sysemu/kvm.h"
>> +#include "exec/exec-all.h"
>>  #include "trace.h"
>>  #include "hw/s390x/storage-keys.h"
>>  
>> @@ -458,7 +459,7 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
>>          }
>>          if (!address_space_access_valid(&address_space_memory, pages[i],
>>                                          TARGET_PAGE_SIZE, is_write)) {
>> -            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
>> +            trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);
> 
> Why did you change this?

"For TCG, there was one case where a cpu loop exit was triggered. Fix
that up."

Wasn't worth a separate patch :)

> 
>>              return -EFAULT;
>>          }
>>          addr += TARGET_PAGE_SIZE;
>> @@ -478,6 +479,9 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
>>   *
>>   * Copy from/to guest memory using logical addresses. Note that we inject a
>>   * program interrupt in case there is an error while accessing the memory.
>> + *
>> + * This function will always return (also for TCG), make sure to call
>> + * s390_cpu_virt_mem_handle_exc() to properly exit the CPU loop.
>>   */
>>  int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>>                           int len, bool is_write)
>> @@ -514,6 +518,19 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>>      return ret;
>>  }
>>  
>> +void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
>> +{
>> +    /* KVM will handle the interrupt automatically, TCG has to exit the TB */
>> +    #ifdef CONFIG_TCG
> 
> Please move the #ifdef/#endif to the beginning of the line.

Oh, yes.

> 
>> +    if (tcg_enabled()) {
>> +        if (ra) {
>> +            cpu_restore_state(CPU(cpu), ra);
>> +        }
>> +        cpu_loop_exit(CPU(cpu));
>> +    }
>> +    #endif
>> +}
>> +
>>  /**
>>   * Translate a real address into a physical (absolute) address.
>>   * @param raddr  the real address
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[Cornelia Huck]

On Wed, 29 Nov 2017 16:54:30 +0100
David Hildenbrand <david@redhat.com> wrote:


> >> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> >> index 31e3f3f415..39da9aeef4 100644
> >> --- a/target/s390x/mmu_helper.c
> >> +++ b/target/s390x/mmu_helper.c
> >> @@ -22,6 +22,7 @@
> >>  #include "internal.h"
> >>  #include "kvm_s390x.h"
> >>  #include "sysemu/kvm.h"
> >> +#include "exec/exec-all.h"
> >>  #include "trace.h"
> >>  #include "hw/s390x/storage-keys.h"
> >>  
> >> @@ -458,7 +459,7 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
> >>          }
> >>          if (!address_space_access_valid(&address_space_memory, pages[i],
> >>                                          TARGET_PAGE_SIZE, is_write)) {
> >> -            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
> >> +            trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);  
> > 
> > Why did you change this?  
> 
> "For TCG, there was one case where a cpu loop exit was triggered. Fix
> that up."
> 
> Wasn't worth a separate patch :)

Well, maybe it is, to avoid dumb questions :)

> 
> >   
> >>              return -EFAULT;
> >>          }
> >>          addr += TARGET_PAGE_SIZE;

Re: [Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[David Hildenbrand]

On 29.11.2017 16:59, Cornelia Huck wrote:
> On Wed, 29 Nov 2017 16:54:30 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
> 
>>>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>>>> index 31e3f3f415..39da9aeef4 100644
>>>> --- a/target/s390x/mmu_helper.c
>>>> +++ b/target/s390x/mmu_helper.c
>>>> @@ -22,6 +22,7 @@
>>>>  #include "internal.h"
>>>>  #include "kvm_s390x.h"
>>>>  #include "sysemu/kvm.h"
>>>> +#include "exec/exec-all.h"
>>>>  #include "trace.h"
>>>>  #include "hw/s390x/storage-keys.h"
>>>>  
>>>> @@ -458,7 +459,7 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
>>>>          }
>>>>          if (!address_space_access_valid(&address_space_memory, pages[i],
>>>>                                          TARGET_PAGE_SIZE, is_write)) {
>>>> -            program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO);
>>>> +            trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);  
>>>
>>> Why did you change this?  
>>
>> "For TCG, there was one case where a cpu loop exit was triggered. Fix
>> that up."
>>
>> Wasn't worth a separate patch :)
> 
> Well, maybe it is, to avoid dumb questions :)

... splitting it up :) Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 07/15] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> +void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
> +{
> +    /* KVM will handle the interrupt automatically, TCG has to exit the TB */
> +    #ifdef CONFIG_TCG
> +    if (tcg_enabled()) {
> +        if (ra) {
> +            cpu_restore_state(CPU(cpu), ra);
> +        }
> +        cpu_loop_exit(CPU(cpu));
> +    }

In addition to everything else,

  cpu_loop_exit_restore(CPU(cpu), ra);


r~

[Qemu-devel] [PATCH v1 for-2.12 08/15] s390x/tcg: io instructions don't need potential_page_fault()

[David Hildenbrand]

As we handle the retaddr in all cases properly now, we can drop it.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 85d0a6c3af..d0859c4bc7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4008,7 +4008,6 @@ static ExitStatus op_spx(DisasContext *s, DisasOps *o)
 static ExitStatus op_xsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_xsch(cpu_env, regs[1]);
     set_cc_static(s);
     return NO_EXIT;
@@ -4017,7 +4016,6 @@ static ExitStatus op_xsch(DisasContext *s, DisasOps *o)
 static ExitStatus op_csch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_csch(cpu_env, regs[1]);
     set_cc_static(s);
     return NO_EXIT;
@@ -4026,7 +4024,6 @@ static ExitStatus op_csch(DisasContext *s, DisasOps *o)
 static ExitStatus op_hsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_hsch(cpu_env, regs[1]);
     set_cc_static(s);
     return NO_EXIT;
@@ -4035,7 +4032,6 @@ static ExitStatus op_hsch(DisasContext *s, DisasOps *o)
 static ExitStatus op_msch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_msch(cpu_env, regs[1], o->in2);
     set_cc_static(s);
     return NO_EXIT;
@@ -4044,7 +4040,6 @@ static ExitStatus op_msch(DisasContext *s, DisasOps *o)
 static ExitStatus op_rchp(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_rchp(cpu_env, regs[1]);
     set_cc_static(s);
     return NO_EXIT;
@@ -4053,7 +4048,6 @@ static ExitStatus op_rchp(DisasContext *s, DisasOps *o)
 static ExitStatus op_rsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_rsch(cpu_env, regs[1]);
     set_cc_static(s);
     return NO_EXIT;
@@ -4062,7 +4056,6 @@ static ExitStatus op_rsch(DisasContext *s, DisasOps *o)
 static ExitStatus op_ssch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_ssch(cpu_env, regs[1], o->in2);
     set_cc_static(s);
     return NO_EXIT;
@@ -4071,7 +4064,6 @@ static ExitStatus op_ssch(DisasContext *s, DisasOps *o)
 static ExitStatus op_stsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_stsch(cpu_env, regs[1], o->in2);
     set_cc_static(s);
     return NO_EXIT;
@@ -4080,7 +4072,6 @@ static ExitStatus op_stsch(DisasContext *s, DisasOps *o)
 static ExitStatus op_tsch(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_tsch(cpu_env, regs[1], o->in2);
     set_cc_static(s);
     return NO_EXIT;
@@ -4089,7 +4080,6 @@ static ExitStatus op_tsch(DisasContext *s, DisasOps *o)
 static ExitStatus op_chsc(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_chsc(cpu_env, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 08/15] s390x/tcg: io instructions don't need potential_page_fault()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> As we handle the retaddr in all cases properly now, we can drop it.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/translate.c | 10 ----------
>  1 file changed, 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 09/15] s390x/tcg: use program_interrupt_ra() in SCLP Service Call

[David Hildenbrand]

Now we can drop potential_page_fault().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/misc_helper.c | 3 +--
 target/s390x/translate.c   | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index e50ea480be..3c2b589f7b 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -63,8 +63,7 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
     qemu_mutex_lock_iothread();
     int r = sclp_service_call(env, r1, r2);
     if (r < 0) {
-        program_interrupt(env, -r, 4);
-        r = 0;
+        program_interrupt_ra(env, -r, 4, GETPC());
     }
     qemu_mutex_unlock_iothread();
     return r;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d0859c4bc7..76b222b0ce 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3704,7 +3704,6 @@ static ExitStatus op_sqxb(DisasContext *s, DisasOps *o)
 static ExitStatus op_servc(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_servc(cc_op, cpu_env, o->in2, o->in1);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 09/15] s390x/tcg: use program_interrupt_ra() in SCLP Service Call

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
>      qemu_mutex_lock_iothread();
>      int r = sclp_service_call(env, r1, r2);
>      if (r < 0) {
> -        program_interrupt(env, -r, 4);
> -        r = 0;
> +        program_interrupt_ra(env, -r, 4, GETPC());
>      }
>      qemu_mutex_unlock_iothread();
>      return r;

Incidentally, shouldn't the unlock happen before throwing the exception?
We will unlock after a longjmp, but it does look odd to write it this way.


r~

Re: [Qemu-devel] [PATCH v1 for-2.12 09/15] s390x/tcg: use program_interrupt_ra() in SCLP Service Call

[David Hildenbrand]

On 29.11.2017 19:10, Richard Henderson wrote:
> On 11/28/2017 08:33 PM, David Hildenbrand wrote:
>>      qemu_mutex_lock_iothread();
>>      int r = sclp_service_call(env, r1, r2);
>>      if (r < 0) {
>> -        program_interrupt(env, -r, 4);
>> -        r = 0;
>> +        program_interrupt_ra(env, -r, 4, GETPC());
>>      }
>>      qemu_mutex_unlock_iothread();
>>      return r;
> 
> Incidentally, shouldn't the unlock happen before throwing the exception?
> We will unlock after a longjmp, but it does look odd to write it this way.
> 

Yes, but we have that all over the place, no? The unlock here could be
moved up.

> 
> r~
> 


-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH v1 for-2.12 10/15] s390x/tcg: use program_interrupt_ra() in DIAG

[David Hildenbrand]

Now we can drop the two save statements in the translate function.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/misc_helper.c | 2 +-
 target/s390x/translate.c   | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 3c2b589f7b..154bde0481 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -101,7 +101,7 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
     }
 
     if (r) {
-        program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO);
+        program_interrupt_ra(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 76b222b0ce..cf8ffa217e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2124,9 +2124,6 @@ static ExitStatus op_diag(DisasContext *s, DisasOps *o)
     TCGv_i32 func_code = tcg_const_i32(get_field(s->fields, i2));
 
     check_privileged(s);
-    update_psw_addr(s);
-    gen_op_calc_cc(s);
-
     gen_helper_diag(cpu_env, r1, r3, func_code);
 
     tcg_temp_free_i32(func_code);
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 10/15] s390x/tcg: use program_interrupt_ra() in DIAG

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Now we can drop the two save statements in the translate function.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 2 +-
>  target/s390x/translate.c   | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 11/15] s390x/tcg: use program_interrupt_ra() in per_check_exception()

[David Hildenbrand]

We can now drop updating the cc.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/misc_helper.c | 2 +-
 target/s390x/translate.c   | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 154bde0481..6ae8e8a220 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -412,7 +412,7 @@ void HELPER(per_check_exception)(CPUS390XState *env)
          * of EXECUTE, while per_address contains the target of EXECUTE.
          */
         ilen = get_ilen(cpu_ldub_code(env, env->per_address));
-        program_interrupt(env, PGM_PER, ilen);
+        program_interrupt_ra(env, PGM_PER, ilen, GETPC());
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index cf8ffa217e..f26fa64a78 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5837,9 +5837,6 @@ static ExitStatus translate_one(CPUS390XState *env, DisasContext *s)
             tcg_gen_movi_i64(psw_addr, s->next_pc);
         }
 
-        /* Save off cc.  */
-        update_cc_op(s);
-
         /* Call the helper to check for a possible PER exception.  */
         gen_helper_per_check_exception(cpu_env);
     }
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 11/15] s390x/tcg: use program_interrupt_ra() in per_check_exception()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> We can now drop updating the cc.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 2 +-
>  target/s390x/translate.c   | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 12/15] s390x/tcg: use program_interrupt_ra() in SACF

[David Hildenbrand]

Convert this user, too.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cc_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index f008897e84..5ead3abbc1 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -564,7 +564,7 @@ void HELPER(sacf)(CPUS390XState *env, uint64_t a1)
         break;
     default:
         HELPER_LOG("unknown sacf mode: %" PRIx64 "\n", a1);
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 2, GETPC());
         break;
     }
 }
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 12/15] s390x/tcg: use program_interrupt_ra() in SACF

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Convert this user, too.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cc_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 13/15] s390x/tcg: use program_interrupt_ra() in STSI

[David Hildenbrand]

STSI needs some more love, but let's do one step at a time.
We can now drop potential_page_fault().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/misc_helper.c | 2 +-
 target/s390x/translate.c   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 6ae8e8a220..aa0430dcac 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -184,7 +184,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
     if ((r0 & STSI_LEVEL_MASK) <= STSI_LEVEL_3 &&
         ((r0 & STSI_R0_RESERVED_MASK) || (r1 & STSI_R1_RESERVED_MASK))) {
         /* valid function code, invalid reserved bits */
-        program_interrupt(env, PGM_SPECIFICATION, 4);
+        program_interrupt_ra(env, PGM_SPECIFICATION, 4, GETPC());
     }
 
     sel1 = r0 & STSI_R0_SEL1_MASK;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f26fa64a78..1ce1390699 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3988,7 +3988,6 @@ static ExitStatus op_stpt(DisasContext *s, DisasOps *o)
 static ExitStatus op_stsi(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_stsi(cc_op, cpu_env, o->in2, regs[0], regs[1]);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 13/15] s390x/tcg: use program_interrupt_ra() in STSI

[Cornelia Huck]

On Tue, 28 Nov 2017 21:33:23 +0100
David Hildenbrand <david@redhat.com> wrote:

> STSI needs some more love, but let's do one step at a time.

Out of curiousity: What else do you want to do?

> We can now drop potential_page_fault().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 2 +-
>  target/s390x/translate.c   | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)

Re: [Qemu-devel] [PATCH v1 for-2.12 13/15] s390x/tcg: use program_interrupt_ra() in STSI

[David Hildenbrand]

On 29.11.2017 16:53, Cornelia Huck wrote:
> On Tue, 28 Nov 2017 21:33:23 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> STSI needs some more love, but let's do one step at a time.
> 
> Out of curiousity: What else do you want to do?
> 

Will post the patch as soon as this series has been picked up.

Here is a preview:

commit a1a86f4db56e307d22918117515184fde540c14b
Author: David Hildenbrand <david@redhat.com>
Date:   Wed Nov 22 18:18:52 2017 +0100

    s390x/tcg: fix size + content of STSI blocks
    
    All blocks are 4k in size, which is only true for one of them right now.
    Also some reserved fields were wrong, fix it and convert all reserved
    fields to speficy u8.
    
    This also fixes the LPAR part output in /proc/sysinfo under LPAR.
    
    Signed-off-by: David Hildenbrand <david@redhat.com>


commit 41192e74109532d4c106c7b1a6bcf7f989b2f4fd
Author: David Hildenbrand <david@redhat.com>
Date:   Wed Nov 22 18:21:14 2017 +0100

    s390x/tcg: STSI overhaul
    
    Current STSI implementation is a mess, so let's rewrite it.
    
    Problems fixed by this patch:
    1) The order of exceptions/when recognized is wrong.
    2) We have to store to virtual address space, not absolute.
    3) Alignment check of the block is missing.
    3) The SMP information is not indicated.
    
    While at it:
    a) Make the code look nicer
        - get rid of nesting levels
        - use struct initialization instead of initializing to zero
        - rename a misspelled field and rename function code defines
        - use a union and have only one write statement
        - use cpu_to_beX()
    b) Indicate the VM name/extended name + UUID just like KVM does
    c) Indicate that all LPAR CPUs we fake are dedicated
    d) Add a comment why we fake being a KVM guest
    e) Give our guest as default the name "TCGguest"
    f) Fake the same CPU information we have in our Guest for all layers
    
    While at it, get rid of "potential_page_fault()" by forwarding the
    retaddr properly.
    
    The result is best verified by looking at "/proc/sysinfo" in the guest
    when specifying on the qemu command line
        -uuid "74738ff5-5367-5958-9aee-98fffdcd1876" \
        -name "extra long guest name"
    
    Signed-off-by: David Hildenbrand <david@redhat.com>


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1 for-2.12 13/15] s390x/tcg: use program_interrupt_ra() in STSI

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> STSI needs some more love, but let's do one step at a time.
> We can now drop potential_page_fault().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 2 +-
>  target/s390x/translate.c   | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[Qemu-devel] [PATCH v1 for-2.12 14/15] s390x/tcg: drop program_interrupt()

[David Hildenbrand]

All users are gone, we can finally drop it and make sure that all new
new program interrupt injections are reminded of the retaddr - as they
have to use program_interrupt_ra() now.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h       |  1 -
 target/s390x/interrupt.c | 22 ++++++++--------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 617f9b0148..890a25c1d6 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -719,7 +719,6 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
                        uint32_t io_int_parm, uint32_t io_int_word);
 /* automatically detect the instruction length */
 #define ILEN_AUTO                   0xff
-void program_interrupt(CPUS390XState *env, uint32_t code, int ilen);
 void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
                           uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 6ce06bb549..c0996187ef 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -37,13 +37,20 @@ static void tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
 #endif
 }
 
-void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
+void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
+                          uintptr_t ra)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
 
     qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
                   env->psw.addr);
 
+#ifdef CONFIG_TCG
+    if (tcg_enabled() && ra) {
+        cpu_restore_state(CPU(cpu), ra);
+    }
+#endif
+
     if (kvm_enabled()) {
         kvm_s390_program_interrupt(cpu, code);
     } else if (tcg_enabled()) {
@@ -53,19 +60,6 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
     }
 }
 
-void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
-                          uintptr_t ra)
-{
-    S390CPU *cpu = s390_env_get_cpu(env);
-
-#ifdef CONFIG_TCG
-    if (tcg_enabled() && ra) {
-        cpu_restore_state(CPU(cpu), ra);
-    }
-#endif
-    program_interrupt(env, code, ilen);
-}
-
 #if !defined(CONFIG_USER_ONLY)
 static void cpu_inject_service(S390CPU *cpu, uint32_t param)
 {
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 14/15] s390x/tcg: drop program_interrupt()

[Cornelia Huck]

On Tue, 28 Nov 2017 21:33:24 +0100
David Hildenbrand <david@redhat.com> wrote:

> All users are gone, we can finally drop it and make sure that all new
> new program interrupt injections are reminded of the retaddr - as they

s/new new/new/

> have to use program_interrupt_ra() now.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h       |  1 -
>  target/s390x/interrupt.c | 22 ++++++++--------------
>  2 files changed, 8 insertions(+), 15 deletions(-)
> 

Re: [Qemu-devel] [PATCH v1 for-2.12 14/15] s390x/tcg: drop program_interrupt()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> -void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
> +void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
> +                          uintptr_t ra)
>  {
>      S390CPU *cpu = s390_env_get_cpu(env);
>  
>      qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
>                    env->psw.addr);
>  
> +#ifdef CONFIG_TCG
> +    if (tcg_enabled() && ra) {
> +        cpu_restore_state(CPU(cpu), ra);
> +    }
> +#endif

Please move this into tcg_s390_program_interrupt by using cpu_loop_exit_restore
there.


r~

Re: [Qemu-devel] [PATCH v1 for-2.12 14/15] s390x/tcg: drop program_interrupt()

[David Hildenbrand]

On 29.11.2017 19:18, Richard Henderson wrote:
> On 11/28/2017 08:33 PM, David Hildenbrand wrote:
>> -void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
>> +void program_interrupt_ra(CPUS390XState *env, uint32_t code, int ilen,
>> +                          uintptr_t ra)
>>  {
>>      S390CPU *cpu = s390_env_get_cpu(env);
>>  
>>      qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
>>                    env->psw.addr);
>>  
>> +#ifdef CONFIG_TCG
>> +    if (tcg_enabled() && ra) {
>> +        cpu_restore_state(CPU(cpu), ra);
>> +    }
>> +#endif
> 
> Please move this into tcg_s390_program_interrupt by using cpu_loop_exit_restore
> there.
> 

Indeed, good idea, thanks!

> 
> r~
> 


-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH v1 for-2.12 15/15] s390x/tcg: drop potential_page_fault()

[David Hildenbrand]

Only one user left, get rid of it so we don't get any new users.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 1ce1390699..26cf993405 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -240,12 +240,6 @@ static void update_cc_op(DisasContext *s)
     }
 }
 
-static void potential_page_fault(DisasContext *s)
-{
-    update_psw_addr(s);
-    update_cc_op(s);
-}
-
 static inline uint64_t ld_code2(CPUS390XState *env, uint64_t pc)
 {
     return (uint64_t)cpu_lduw_code(env, pc);
@@ -2939,7 +2933,8 @@ static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
 
     /* In a parallel context, stop the world and single step.  */
     if (tb_cflags(s->tb) & CF_PARALLEL) {
-        potential_page_fault(s);
+        update_psw_addr(s);
+        update_cc_op(s);
         gen_exception(EXCP_ATOMIC);
         return EXIT_NORETURN;
     }
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 15/15] s390x/tcg: drop potential_page_fault()

[Richard Henderson]

On 11/28/2017 08:33 PM, David Hildenbrand wrote:
> Only one user left, get rid of it so we don't get any new users.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/translate.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH v1 for-2.12 00/15] s390x/tcg: cleanup and fix program interrupts

[Thomas Huth]

On 28.11.2017 21:33, David Hildenbrand wrote:
> I have quite some patches on my queue for 2.12. (booting Fedora 26/27
> guests, floating interrupts, machine checks, missing instructions ...)
> 
> So let's start slowly :) This series gets rid of program_interrupt() and
> potential_page_fault(). We now always properly restore the cpu state when
> injecting/delivering a program interrupt. So there is no need to update
> the state via potential_page_fault() anymore.

I think this series is basically a very good idea! But...
OK, this is kind of bike-shed-painting now, but since we're currently in
hard freeze anyway and got plenty of time for discussion:
Something that bothers me a little bit is the name of the new function
"program_interrupt_ra()" ... that would IMHO be OK if the old function
"program_interrupt" would still stay, but since that is removed and the
_ra function is the only generic way that is left to inject a program
interrupt, could we maybe name the new function somewhat nicer right
from the start? Something like "s390_program_interrupt" maybe (which is
similar to tcg_s390_program_interrupt and kvm_s390_program_interrupt
that we have already)?

 Thomas

Re: [Qemu-devel] [qemu-s390x] [PATCH v1 for-2.12 00/15] s390x/tcg: cleanup and fix program interrupts

[David Hildenbrand]

On 29.11.2017 14:51, Thomas Huth wrote:
> On 28.11.2017 21:33, David Hildenbrand wrote:
>> I have quite some patches on my queue for 2.12. (booting Fedora 26/27
>> guests, floating interrupts, machine checks, missing instructions ...)
>>
>> So let's start slowly :) This series gets rid of program_interrupt() and
>> potential_page_fault(). We now always properly restore the cpu state when
>> injecting/delivering a program interrupt. So there is no need to update
>> the state via potential_page_fault() anymore.
> 
> I think this series is basically a very good idea! But...
> OK, this is kind of bike-shed-painting now, but since we're currently in
> hard freeze anyway and got plenty of time for discussion:
> Something that bothers me a little bit is the name of the new function
> "program_interrupt_ra()" ... that would IMHO be OK if the old function
> "program_interrupt" would still stay, but since that is removed and the
> _ra function is the only generic way that is left to inject a program
> interrupt, could we maybe name the new function somewhat nicer right
> from the start? Something like "s390_program_interrupt" maybe (which is
> similar to tcg_s390_program_interrupt and kvm_s390_program_interrupt
> that we have already)?

Sure I can do that, other opinions?

> 
>  Thomas
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH v1 for-2.12 00/15] s390x/tcg: cleanup and fix program interrupts

[Cornelia Huck]

On Wed, 29 Nov 2017 15:06:33 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 29.11.2017 14:51, Thomas Huth wrote:
> > On 28.11.2017 21:33, David Hildenbrand wrote:  
> >> I have quite some patches on my queue for 2.12. (booting Fedora 26/27
> >> guests, floating interrupts, machine checks, missing instructions ...)
> >>
> >> So let's start slowly :) This series gets rid of program_interrupt() and
> >> potential_page_fault(). We now always properly restore the cpu state when
> >> injecting/delivering a program interrupt. So there is no need to update
> >> the state via potential_page_fault() anymore.  
> > 
> > I think this series is basically a very good idea! But...
> > OK, this is kind of bike-shed-painting now, but since we're currently in
> > hard freeze anyway and got plenty of time for discussion:
> > Something that bothers me a little bit is the name of the new function
> > "program_interrupt_ra()" ... that would IMHO be OK if the old function
> > "program_interrupt" would still stay, but since that is removed and the
> > _ra function is the only generic way that is left to inject a program
> > interrupt, could we maybe name the new function somewhat nicer right
> > from the start? Something like "s390_program_interrupt" maybe (which is
> > similar to tcg_s390_program_interrupt and kvm_s390_program_interrupt
> > that we have already)?  
> 
> Sure I can do that, other opinions?

Fine with me as well.