s390x/tcg: cleanup and fix program interrupts
v2 cover s390x/tcg: cleanup and fix program interrupts
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.
v1 -> v2:
- renamed program_interrupt_ra() to s390_program_interrupt()
- introduce and use RA_IGNORED for KVM.
- switch to cpu_exit_loop_restore() and don't check if ra is set
- "s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)" -> split out "s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()"
- "s390x/tcg: use program_interrupt_ra() in SCLP Service Call" -> moved qemu_mutex_lock_iothread(); further up
- "s390x/tcg: drop program_interrupt()"
-> move restore to tcg_s390_program_interrupt() via cpu_loop_exit_restore()
- smaller requested cleanups
David Hildenbrand (16):
s390x/tcg: introduce and use s390_program_interrupt() 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: don't exit the cpu loop in s390_cpu_virt_mem_rw() s390x/tcg: io instructions don't need potential_page_fault() s390x/tcg: use s390_program_interrupt() in SCLP Service Call s390x/tcg: use s390_program_interrupt() in DIAG
s390x/tcg: use s390_program_interrupt() in per_check_exception() s390x/tcg: use s390_program_interrupt() in SACF
s390x/tcg: use s390_program_interrupt() 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 | 5 +- 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 | 113 +++++++++++++++++++------------------------ target/s390x/kvm.c | 43 ++++++++-------- target/s390x/mem_helper.c | 35 +++++--------- target/s390x/misc_helper.c | 52 +++++++------------- target/s390x/mmu_helper.c | 16 +++++- target/s390x/translate.c | 27 +---------- 19 files changed, 223 insertions(+), 269 deletions(-)
Signature · 1 line
2.14.3
PATCH v2 1/16 s390x/tcg: introduce and use s390_program_interrupt()
Allows to easily convert more callers of program_interrupt() and to easily introduce new exceptions without forgetting about the cpu state reset.
Use s390_program_interrupt() in places where we already had the same pattern. We will later get rid of program_interrupt().
RA != 0 checks are already done behind the scenes.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- 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(-)
target/s390x/cpu.h +2 -0
@@ -720,6 +720,8 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,/* automatically detect the instruction length */ #define ILEN_AUTO 0xffvoid program_interrupt (CPUS390XState *env,uint32_t code,int ilen); +void s390_program_interrupt (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);
target/s390x/crypto_helper.c +2 -5
@@ -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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,4 , ra);return 0 ; }
target/s390x/excp_helper.c +1 -4
@@ -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); + s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, retaddr); }#endif /* CONFIG_USER_ONLY */
target/s390x/interrupt.c +13 -0
@@ -53,6 +53,19 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen) } } +void s390_program_interrupt (CPUS390XState *env,uint32_t code,int ilen, +uintptr_t ra) +{ +#ifdef CONFIG_TCG +S390CPU *cpu =s390_env_get_cpu (env); + +if (tcg_enabled ()) { +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) {
target/s390x/mem_helper.c +11 -24
@@ -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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_ADDRESSING ,6 , ra); }/* FIXME: a) LAP
target/s390x/misc_helper.c +1 -2
@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,4 , ra); }prepare_stfl ();
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 01/16] s390x/tcg: introduce and use s390_program_interrupt()
Quoted 11 lines · 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
s390_program_interrupt()in places where we already had the same pattern. We will later get rid ofprogram_interrupt().RA != 0 checks are already done behind the scenes.
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 8 lines
--- 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(-)
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 2/16 s390x/tcg: get rid of runtime_exception()
In reply to David HildenbrandLet's use s390_program_interrupt() instead.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- 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(-)
target/s390x/fpu_helper.c +1 -1
@@ -44,7 +44,7 @@ static void ieee_exception(CPUS390XState *env, uint32_t dxc, uintptr_t retaddr)/* Install the DXC code. */ env->fpc = (env->fpc & ~0xff00 ) | (dxc <<8 );/* Trap. */ -runtime_exception (env,PGM_DATA , retaddr); +s390_program_interrupt (env,PGM_DATA ,ILEN_AUTO , retaddr); }/* Should be called after any operation that may raise IEEE exceptions. */
target/s390x/int_helper.c +7 -7
@@ -39,7 +39,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)int64_t q;if (b ==0 ) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); } ret = q = a / b;
@@ -47,7 +47,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)/* Catch non-representable quotient. */ if (ret != q) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); }return ret;
@@ -60,7 +60,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)uint64_t q;if (b ==0 ) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); } ret = q = a / b;
@@ -68,7 +68,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)/* Catch non-representable quotient. */ if (ret != q) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); }return ret;
@@ -79,7 +79,7 @@ int64_t HELPER(divs64)(CPUS390XState *env, int64_t a, int64_t b) {/* Catch divide by zero, and non-representable quotient (MIN / -1). */ if (b ==0 || (b ==-1 && a == (1ll <<63 ))) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); } env->retxl = a % b;return a / b;
@@ -92,7 +92,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,uint64_t ret;/* Signal divide by zero. */ if (b ==0 ) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); }if (ah ==0 ) {/* 64 -> 64/64 case */
@@ -106,7 +106,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al, env->retxl = a % b; ret = q;if (ret != q) { -runtime_exception (env,PGM_FIXPT_DIVIDE ,GETPC ()); +s390_program_interrupt (env,PGM_FIXPT_DIVIDE ,ILEN_AUTO ,GETPC ()); }#else S390CPU *cpu =s390_env_get_cpu (env);
target/s390x/internal.h +0 -2
@@ -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);
target/s390x/misc_helper.c +0 -16
@@ -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) {
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 02/16] s390x/tcg: get rid of runtime_exception()
Quoted 10 lines · David Hildenbrand
Let's use
s390_program_interrupt()instead.Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/fpu_helper.c | 2 +- target/s390x/int_helper.c | 14 +++++++------- target/s390x/internal.h | 2 -- target/s390x/misc_helper.c | 16 ---------------- 4 files changed, 8 insertions(+), 26 deletions(-)
Is it a disadvantage that runtime_exception() was declared as QEMU_NORETURN, and s390_program_interrupt() is not declared as QEMU_NORETURN?
At a first glance, I guess it's ok, and in that case:
Reviewed-by Thomas Huth <[email protected]>
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 02/16] s390x/tcg: get rid of runtime_exception()
Quoted 12 lines · Thomas Huth
Quoted 10 lines · David Hildenbrand
Let's use
s390_program_interrupt()instead.Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- 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(-)
Quoted 3 lines
Is it a disadvantage that
runtime_exception()was declared asQEMU_NORETURN, ands390_program_interrupt()is not declared asQEMU_NORETURN?
We could add that to trigger_pgm_exception instead. But if cpu_loop_exit() would really return, we would be in more trouble AKA nothing would work.
So I don't see a problem dropping this.
Quoted 5 lines
At a first glance, I guess it's ok, and in that case:
Reviewed-by Thomas Huth <[email protected]>
Signature · 3 lines
Thanks, David / dhildenb
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 02/16] s390x/tcg: get rid of runtime_exception()
Quoted 13 lines · David Hildenbrand
Quoted 12 lines · Thomas Huth
Quoted 10 lines · David Hildenbrand
Let's use
s390_program_interrupt()instead.Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- 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(-)
Quoted 7 lines
Quoted 3 lines
Is it a disadvantage that
runtime_exception()was declared asQEMU_NORETURN, ands390_program_interrupt()is not declared asQEMU_NORETURN?We could add that to
trigger_pgm_exceptioninstead. But ifcpu_loop_exit()would really return, we would be in more trouble AKA nothing would work.
s390_program_interrupt and trigger_pgm_exception do return (kvm).
We could export tcg_s390_program_interrupt as NORETURN, and use it directly in places that are tcg-only. That might provide some minor advantages to the compiler, but I don't think it's critical.
r~
PATCH v2 3/16 s390x/tcg: rip out dead tpi code
In reply to David HildenbrandIt is broken and not even wired up. We'll add a new handler soon, but that will live somewhere else.
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- 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(-)
hw/s390x/css.c +0 -6
@@ -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) {
include/hw/s390x/css.h +0 -1
@@ -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);
target/s390x/internal.h +0 -1
@@ -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);
target/s390x/ioinst.c +0 -26
@@ -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)
Signature · 1 line
2.14.3
PATCH v2 4/16 s390x/ioinst: pass the retaddr to all IO instructions
In reply to David HildenbrandTCG needs the retaddr when injecting an interrupt. Let's just pass it along and use RA_IGNORED for KVM. The value will be completely ignored for KVM.
Convert program_interrupt() to s390_program_interrupt() directly, making use of the passed address.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/cpu.h | 1 + target/s390x/internal.h | 29 +++++++++++--------- target/s390x/ioinst.c | 67 +++++++++++++++++++++++----------------------- target/s390x/kvm.c | 27 ++++++++++--------- target/s390x/misc_helper.c | 20 +++++++------- 5 files changed, 75 insertions(+), 69 deletions(-)
target/s390x/cpu.h +1 -0
@@ -720,6 +720,7 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,/* automatically detect the instruction length */ #define ILEN_AUTO 0xffvoid program_interrupt (CPUS390XState *env,uint32_t code,int ilen); +#define RA_IGNORED 0void s390_program_interrupt (CPUS390XState *env,uint32_t code,int ilen,uintptr_t ra);/* service interrupts are floating therefore we must not pass an cpustate */
target/s390x/internal.h +16 -13
@@ -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 */
target/s390x/ioinst.c +34 -33
256 lines not on this page · raw message
target/s390x/kvm.c +14 -13
@@ -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 ],RA_IGNORED );break ;case PRIV_B2_CSCH : -ioinst_handle_csch (cpu, env->regs[1 ]); +ioinst_handle_csch (cpu, env->regs[1 ],RA_IGNORED );break ;case PRIV_B2_HSCH : -ioinst_handle_hsch (cpu, env->regs[1 ]); +ioinst_handle_hsch (cpu, env->regs[1 ],RA_IGNORED );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,RA_IGNORED );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,RA_IGNORED );break ;case PRIV_B2_STCRW : -ioinst_handle_stcrw (cpu, run->s390_sieic.ipb); +ioinst_handle_stcrw (cpu, run->s390_sieic.ipb,RA_IGNORED );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,RA_IGNORED );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,RA_IGNORED );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,RA_IGNORED );break ;case PRIV_B2_RSCH : -ioinst_handle_rsch (cpu, env->regs[1 ]); +ioinst_handle_rsch (cpu, env->regs[1 ],RA_IGNORED );break ;case PRIV_B2_RCHP : -ioinst_handle_rchp (cpu, env->regs[1 ]); +ioinst_handle_rchp (cpu, env->regs[1 ],RA_IGNORED );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 ],RA_IGNORED );break ;case PRIV_B2_SIGA :/* Not provided, set CC = 3 for subchannel not operational */
@@ -1673,7 +1673,8 @@ 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, +RA_IGNORED );if (ret <0 ) {/* * Failure.
target/s390x/misc_helper.c +10 -10
@@ -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
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 04/16] s390x/ioinst: pass the retaddr to all IO instructions
Quoted 9 lines · David Hildenbrand
TCG needs the retaddr when injecting an interrupt. Let's just pass it along and use
RA_IGNOREDfor KVM. The value will be completely ignored for KVM.Convert
program_interrupt()tos390_program_interrupt()directly, making use of the passed address.Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 7 lines
--- target/s390x/cpu.h | 1 + target/s390x/internal.h | 29 +++++++++++--------- target/s390x/ioinst.c | 67 +++++++++++++++++++++++----------------------- target/s390x/kvm.c | 27 ++++++++++--------- target/s390x/misc_helper.c | 20 +++++++------- 5 files changed, 75 insertions(+), 69 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 5/16 s390x/pci: pass the retaddr to all PCI instructions
In reply to David HildenbrandOnce 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 use RA_IGNORED.
Convert program_interrupt() to s390_program_interrupt() directly, making use of the passed address.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- 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(-)
hw/s390x/s390-pci-inst.c +43 -40
@@ -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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_OPERAND ,4 , ra);return 0 ; }if ((req_len + res_len) >8192 ) { -program_interrupt (env,PGM_OPERAND ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_PRIVILEGED ,4 , ra);return 0 ; }if (r2 &0x1 ) { -program_interrupt (env,PGM_SPECIFICATION ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_OPERAND ,4 , ra);return 0 ; } }else if (pcias ==15 ) {if ((4 - (offset &0x3 )) < len) { -program_interrupt (env,PGM_OPERAND ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_PRIVILEGED ,4 , ra);return 0 ; }if (r2 &0x1 ) { -program_interrupt (env,PGM_SPECIFICATION ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_OPERAND ,4 , ra);return 0 ; } }else if (pcias ==15 ) {if ((4 - (offset &0x3 )) < len) { -program_interrupt (env,PGM_OPERAND ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_PRIVILEGED ,4 , ra); goto out; }if (r2 &0x1 ) { -program_interrupt (env,PGM_SPECIFICATION ,4 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (&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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,6 , ra);return 0 ; }
hw/s390x/s390-pci-inst.h +9 -7
@@ -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
target/s390x/kvm.c +7 -7
@@ -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,RA_IGNORED ); }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,RA_IGNORED ); }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,RA_IGNORED ); }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,RA_IGNORED ); }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,RA_IGNORED ); }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,RA_IGNORED ); }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,RA_IGNORED ); }else {return -1 ; }
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 05/16] s390x/pci: pass the retaddr to all PCI instructions
Quoted 9 lines · 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 use
RA_IGNORED.Convert
program_interrupt()tos390_program_interrupt()directly, making use of the passed address.
Quoted 7 lines
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- 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 Thomas Huth <[email protected]>
PATCH v2 6/16 s390x/diag: pass the retaddr into handle_diag_308()
In reply to David HildenbrandNeeded to later drop potential_page_fault() from the diag TCG translate function.
Convert program_interrupt() to s390_program_interrupt() directly, making use of the passed address.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- 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(-)
target/s390x/diag.c +7 -7
@@ -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 ); +s390_program_interrupt (env,PGM_PRIVILEGED ,ILEN_AUTO , ra);return ; }if ((subcode & ~0x0ffffULL ) || (subcode >6 )) { -program_interrupt (env,PGM_SPECIFICATION ,ILEN_AUTO ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (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 ); +s390_program_interrupt (env,PGM_ADDRESSING ,ILEN_AUTO , ra);return ; } iplb =s390_ipl_get_iplb ();
target/s390x/internal.h +2 -1
@@ -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 */
target/s390x/kvm.c +1 -1
@@ -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,RA_IGNORED ); }static int handle_sw_breakpoint (S390CPU *cpu,struct kvm_run *run)
target/s390x/misc_helper.c +1 -1
@@ -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 ;
Signature · 1 line
2.14.3
PATCH v2 7/16 s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)
In reply to David Hildenbrands390_cpu_virt_mem_rw() must always return, so callers can react on an exception (e.g. see ioinst_handle_stcrw()).
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 <[email protected]>
--- hw/s390x/s390-pci-inst.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/ioinst.c | 20 +++++++++++++++++--- target/s390x/mmu_helper.c | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)
hw/s390x/s390-pci-inst.c +7 -0
@@ -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 ; }
target/s390x/cpu.h +1 -0
@@ -736,6 +736,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 */
target/s390x/ioinst.c +17 -3
@@ -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))) {s390_program_interrupt (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,6 +657,8 @@ 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); } }
target/s390x/mmu_helper.c +14 -0
@@ -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"
@@ -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,16 @@ 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 ()) { +cpu_loop_exit_restore (CPU (cpu), ra); + } +#endif +} +/** * Translate a real address into a physical (absolute) address. * @param raddr the real address
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 07/16] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)
Quoted 10 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).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.
Quoted 7 lines
Signed-off-by David Hildenbrand <[email protected]>--- hw/s390x/s390-pci-inst.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/ioinst.c | 20 +++++++++++++++++--- target/s390x/mmu_helper.c | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)
Reviewed-by Richard Henderson <[email protected]>
r~
Re: [Qemu-devel] [PATCH v2 for-2.12 07/16] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)
In reply to David HildenbrandQuoted 39 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).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 <[email protected]>--- hw/s390x/s390-pci-inst.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/ioinst.c | 20 +++++++++++++++++--- target/s390x/mmu_helper.c | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)hw/s390x/s390-pci-inst.c +2
-0@@ -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);
hw/s390x/s390-pci-inst.c int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra) · line 196
Quoted 6 lines
hw/s390x/s390-pci-inst.c +1
-0@@ -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 ; }
[...]
Having to change all these spots is kind of ugly ... and I guess it will also be quite error-prone in the future (if someone is developing new code under KVM only and then forgets to add these handlers for TCG).
Can't you do that simply always at the end of s390_cpu_virt_mem_rw() instead, when the function has been called in non-checking mode (i.e. hostbuf != NULL)? Then we only need the extra-dance in places where we call the _check function?
Quoted 5 lines
hw/s390x/s390-pci-inst.c +0
-1@@ -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 ) {
hw/s390x/s390-pci-inst.c void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) · line 224
Quoted 10 lines
hw/s390x/s390-pci-inst.c +6
-2-/* 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); } }
That hunk would then need to do _check first and in case there is a failure, call css_undo_stcrw() before the s390_cpu_virt_mem_handle_exc() function, I think.
Alternatively, the s390_cpu_virt_mem_rw() function could get an additional parameter that points to a callback function which is used for "clean-up" right before the cpu_loop_exit ... and in this case the callback function would contain the css_undo_stcrw().
What do you think?
Thomas
Re: [Qemu-devel] [PATCH v2 for-2.12 07/16] s390x: handle exceptions during s390_cpu_virt_mem_rw() correctly (TCG)
Quoted 48 lines · Thomas Huth
Quoted 45 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).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 <[email protected]>--- hw/s390x/s390-pci-inst.c | 7 +++++++ target/s390x/cpu.h | 1 + target/s390x/ioinst.c | 20 +++++++++++++++++--- target/s390x/mmu_helper.c | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-)hw/s390x/s390-pci-inst.c +3
-0@@ -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 ; }[...]
Quoted 3 lines
Having to change all these spots is kind of ugly ... and I guess it will also be quite error-prone in the future (if someone is developing new code under KVM only and then forgets to add these handlers for TCG).
The good thing is, it won't break KVM but only TCG. And it has been broken in TCG for years now without anybody noticing it.
Quoted 4 lines
Can't you do that simply always at the end of
s390_cpu_virt_mem_rw()instead, when the function has been called in non-checking mode (i.e. hostbuf != NULL)? Then we only need the extra-dance in places where we call the_checkfunction?
Oh god no, no such strange calling conventions from the same function.
Quoted 19 lines
Quoted 6 lines
hw/s390x/s390-pci-inst.c +0
-1@@ -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 ) {hw/s390x/s390-pci-inst.c void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra) · line 224
Quoted 10 lines
hw/s390x/s390-pci-inst.c +6
-2-/* 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); } }That hunk would then need to do
_checkfirst and in case there is a failure, callcss_undo_stcrw()before thes390_cpu_virt_mem_handle_exc()function, I think.
I don't really like that. And there would be a possibility for a race. So a no from my side.
Quoted 7 lines
Alternatively, the
s390_cpu_virt_mem_rw()function could get an additional parameter that points to a callback function which is used for "clean-up" right before thecpu_loop_exit... and in this case the callback function would contain thecss_undo_stcrw().What do you think?
I also thought about this, but adding two parameters to every call is even uglier. Not talking about having to construct special structs to pass through the data. Please not that the TPI handler I am about to introduce will also require to revert stuff in case there was an exception.
Another option I thought about was checking in the caller if EXC_PGM has been set. But missing to add such a check is even easier.
I'd prefer to keep it as is, but if there are other opinions/ideas, please speak up.
Thanks for having a look!
Quoted 3 lines
Thomas
Signature · 3 lines
Thanks, David / dhildenb
PATCH v2 8/16 s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()
In reply to David Hildenbrands390_cpu_virt_mem_rw() must always return, so callers can react on an exception (e.g. see ioinst_handle_stcrw()).
Therefore, using program_interrupt() is wrong. Fix that up.
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
target/s390x/mmu_helper.c +1 -1
@@ -459,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 ;
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 08/16] s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()
Quoted 9 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).Therefore, using
program_interrupt()is wrong. Fix that up.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Richard Henderson <[email protected]>
r~
Re: [Qemu-devel] [PATCH v2 for-2.12 08/16] s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()
In reply to David HildenbrandQuoted 15 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).Therefore, using
program_interrupt()is wrong. Fix that up.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)target/s390x/mmu_helper.c +0
-0
target/s390x/mmu_helper.c static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages, · line 465
Quoted 8 lines
target/s390x/mmu_helper.c +1
-1@@ -459,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 ;
Is that still right when running with KVM? I think the exception will then silently be ignored instead?
Thomas
Re: [Qemu-devel] [PATCH v2 for-2.12 08/16] s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()
Quoted 11 lines · Thomas Huth
Quoted 10 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).Therefore, using
program_interrupt()is wrong. Fix that up.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 16 lines
target/s390x/mmu_helper.c static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages, · line 465
Quoted 13 lines
target/s390x/mmu_helper.c +1
-1@@ -459,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 ;Is that still right when running with KVM? I think the exception will then silently be ignored instead?
Good point (for older KVM). And ugly.
if (kvm_enabled()) {
kvm_s390_program_interrupt...
} else {
trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);
}Quoted 3 lines
Thomas
Signature · 3 lines
Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v2 for-2.12 08/16] s390x/tcg: don't exit the cpu loop in s390_cpu_virt_mem_rw()
Quoted 29 lines · On Thu, 30 Nov 2017 12:57:00 +0100 David Hildenbrand <[email protected]> wrote:
Quoted 11 lines · Thomas Huth
Quoted 10 lines · David Hildenbrand
s390_cpu_virt_mem_rw()must always return, so callers can react on an exception (e.g. seeioinst_handle_stcrw()).Therefore, using
program_interrupt()is wrong. Fix that up.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 16 lines
target/s390x/mmu_helper.c static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages, · line 465
Quoted 13 lines
target/s390x/mmu_helper.c +1
-1@@ -459,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 ;Is that still right when running with KVM? I think the exception will then silently be ignored instead?
Quoted 7 lines
Good point (for older KVM). And ugly.
if (kvm_enabled()) { kvm_s390_program_interrupt... } else { trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO); }
Maybe add a comment that this is only for old kernels that do not support the mem op interface?
PATCH v2 9/16 s390x/tcg: io instructions don't need potential_page_fault()
In reply to David HildenbrandAs we handle the retaddr in all cases properly now, we can drop it.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/translate.c | 10 ---------- 1 file changed, 10 deletions(-)
target/s390x/translate.c +0 -10
@@ -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 ;
Signature · 1 line
2.14.3
PATCH v2 10/16 s390x/tcg: use s390_program_interrupt() in SCLP Service Call
In reply to David HildenbrandNow we can drop potential_page_fault(). While at it, move the unlock further up, looks cleaner.
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/misc_helper.c | 5 ++--- target/s390x/translate.c | 1 - 2 files changed, 2 insertions(+), 4 deletions(-)
target/s390x/misc_helper.c +2 -3
@@ -62,11 +62,10 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2) {qemu_mutex_lock_iothread ();int r =sclp_service_call (env, r1, r2); +qemu_mutex_unlock_iothread ();if (r <0 ) { -program_interrupt (env, -r,4 ); - r =0 ; +s390_program_interrupt (env, -r,4 ,GETPC ()); } -qemu_mutex_unlock_iothread ();return r; }
target/s390x/translate.c +0 -1
@@ -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 ;
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 10/16] s390x/tcg: use s390_program_interrupt() in SCLP Service Call
Quoted 8 lines · David Hildenbrand
Now we can drop
potential_page_fault(). While at it, move the unlock further up, looks cleaner.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/misc_helper.c | 5 ++--- target/s390x/translate.c | 1 - 2 files changed, 2 insertions(+), 4 deletions(-)
Reviewed-by Richard Henderson <[email protected]>
r~
Re: [Qemu-devel] [PATCH v2 for-2.12 10/16] s390x/tcg: use s390_program_interrupt() in SCLP Service Call
In reply to David HildenbrandQuoted 32 lines · David Hildenbrand
Now we can drop
potential_page_fault(). While at it, move the unlock further up, looks cleaner.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/misc_helper.c | 5 ++--- target/s390x/translate.c | 1 - 2 files changed, 2 insertions(+), 4 deletions(-)target/s390x/misc_helper.c +2
-3@@ -62,11 +62,10 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2) {qemu_mutex_lock_iothread ();int r =sclp_service_call (env, r1, r2); +qemu_mutex_unlock_iothread ();if (r <0 ) { -program_interrupt (env, -r,4 ); - r =0 ; +s390_program_interrupt (env, -r,4 ,GETPC ()); } -qemu_mutex_unlock_iothread ();return r; }target/s390x/translate.c +0
-0
target/s390x/translate.c static ExitStatus op_sqxb(DisasContext *s, DisasOps *o) · line 3709
Quoted 7 lines
target/s390x/translate.c +0
-1@@ -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 ;
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 11/16 s390x/tcg: use s390_program_interrupt() in DIAG
In reply to David HildenbrandNow we can drop the two save statements in the translate function.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-)
target/s390x/misc_helper.c +1 -1
@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,ILEN_AUTO ,GETPC ()); } }
target/s390x/translate.c +0 -3
@@ -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);
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 11/16] s390x/tcg: use s390_program_interrupt() in DIAG
Quoted 28 lines · David Hildenbrand
Now we can drop the two save statements in the translate function.
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-)target/s390x/misc_helper.c +1
-1@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,ILEN_AUTO ,GETPC ()); } }target/s390x/translate.c +0
-0
target/s390x/translate.c static ExitStatus op_diag(DisasContext *s, DisasOps *o) · line 2129
Quoted 9 lines
target/s390x/translate.c +0
-3@@ -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);
Maybe merge it with patch 06/16 ?
Thomas
Re: [Qemu-devel] [PATCH v2 for-2.12 11/16] s390x/tcg: use s390_program_interrupt() in DIAG
Quoted 36 lines · Thomas Huth
Quoted 35 lines · David Hildenbrand
Now we can drop the two save statements in the translate function.
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-)target/s390x/misc_helper.c +1
-1@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,ILEN_AUTO ,GETPC ()); } }target/s390x/translate.c +0
-3@@ -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);
Quoted 5 lines
target/s390x/translate.c static ExitStatus op_diag(DisasContext *s, DisasOps *o) · line 2129
Quoted 2 lines
target/s390x/translate.c +0
-0tcg_temp_free_i32 (func_code);Maybe merge it with patch 06/16 ?
No, because this is tcg specific.
Quoted 2 lines
Thomas
Signature · 3 lines
Thanks, David / dhildenb
PATCH v2 12/16 s390x/tcg: use s390_program_interrupt() in per_check_exception()
In reply to David HildenbrandWe can now drop updating the cc.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-)
target/s390x/misc_helper.c +1 -1
@@ -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); +s390_program_interrupt (env,PGM_PER , ilen,GETPC ()); } }
target/s390x/translate.c +0 -3
@@ -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); }
Signature · 1 line
2.14.3
PATCH v2 13/16 s390x/tcg: use s390_program_interrupt() in SACF
In reply to David HildenbrandConvert this user, too.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/cc_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
target/s390x/cc_helper.c +1 -1
@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,2 ,GETPC ());break ; } }
Signature · 1 line
2.14.3
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 13/16] s390x/tcg: use s390_program_interrupt() in SACF
Quoted 14 lines · David Hildenbrand
Convert this user, too.
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/cc_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)target/s390x/cc_helper.c +0
-0
target/s390x/cc_helper.c void HELPER(sacf)(CPUS390XState *env, uint64_t a1) · line 570
Quoted 8 lines
target/s390x/cc_helper.c +1
-1@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,2 ,GETPC ());break ; } }
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 14/16 s390x/tcg: use s390_program_interrupt() in STSI
In reply to David HildenbrandSTSI needs some more love, but let's do one step at a time. We can now drop potential_page_fault().
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
target/s390x/misc_helper.c +1 -1
@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,4 ,GETPC ()); } sel1 = r0 &STSI_R0_SEL1_MASK ;
target/s390x/translate.c +0 -1
@@ -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 ;
Signature · 1 line
2.14.3
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 14/16] s390x/tcg: use s390_program_interrupt() in STSI
Quoted 28 lines · David Hildenbrand
STSI needs some more love, but let's do one step at a time. We can now drop
potential_page_fault().Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/misc_helper.c | 2 +- target/s390x/translate.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)target/s390x/misc_helper.c +1
-1@@ -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 ); +s390_program_interrupt (env,PGM_SPECIFICATION ,4 ,GETPC ()); } sel1 = r0 &STSI_R0_SEL1_MASK ;target/s390x/translate.c +0
-0
target/s390x/translate.c static ExitStatus op_stpt(DisasContext *s, DisasOps *o) · line 3993
Quoted 7 lines
target/s390x/translate.c +0
-1@@ -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 ;
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 15/16 s390x/tcg: drop program_interrupt()
In reply to David HildenbrandAll users are gone, we can finally drop it and make sure that all new program interrupt injections are reminded of the retaddr - as they have to use s390_program_interrupt() now.
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/cpu.h | 1 - target/s390x/interrupt.c | 22 +++++----------------- 2 files changed, 5 insertions(+), 18 deletions(-)
target/s390x/cpu.h +0 -1
@@ -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);#define RA_IGNORED 0void s390_program_interrupt (CPUS390XState *env,uint32_t code,int ilen,uintptr_t ra);
target/s390x/interrupt.c +5 -17
@@ -27,17 +27,18 @@ void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen) }static void tcg_s390_program_interrupt (CPUS390XState *env,uint32_t code, -int ilen) +int ilen,uintptr_t ra) {#ifdef CONFIG_TCG trigger_pgm_exception (env, code, ilen); -cpu_loop_exit (CPU (s390_env_get_cpu (env))); +cpu_loop_exit_restore (CPU (s390_env_get_cpu (env)), ra);#else g_assert_not_reached ();#endif } -void program_interrupt (CPUS390XState *env,uint32_t code,int ilen) +void s390_program_interrupt (CPUS390XState *env,uint32_t code,int ilen, +uintptr_t ra) {S390CPU *cpu =s390_env_get_cpu (env);
@@ -47,25 +48,12 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)if (kvm_enabled ()) {kvm_s390_program_interrupt (cpu, code); }else if (tcg_enabled ()) { -tcg_s390_program_interrupt (env, code, ilen); +tcg_s390_program_interrupt (env, code, ilen, ra); }else {g_assert_not_reached (); } } -void s390_program_interrupt (CPUS390XState *env,uint32_t code,int ilen, -uintptr_t ra) -{ -#ifdef CONFIG_TCG -S390CPU *cpu =s390_env_get_cpu (env); - -if (tcg_enabled ()) { -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) {
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 for-2.12 15/16] s390x/tcg: drop program_interrupt()
Quoted 9 lines · David Hildenbrand
All users are gone, we can finally drop it and make sure that all new program interrupt injections are reminded of the retaddr - as they have to use
s390_program_interrupt()now.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/cpu.h | 1 - target/s390x/interrupt.c | 22 +++++----------------- 2 files changed, 5 insertions(+), 18 deletions(-)
Reviewed-by Richard Henderson <[email protected]>
r~
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 15/16] s390x/tcg: drop program_interrupt()
In reply to David HildenbrandQuoted 9 lines · David Hildenbrand
All users are gone, we can finally drop it and make sure that all new program interrupt injections are reminded of the retaddr - as they have to use
s390_program_interrupt()now.Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/cpu.h | 1 - target/s390x/interrupt.c | 22 +++++----------------- 2 files changed, 5 insertions(+), 18 deletions(-)
Reviewed-by Thomas Huth <[email protected]>
PATCH v2 16/16 s390x/tcg: drop potential_page_fault()
In reply to David HildenbrandOnly one user left, get rid of it so we don't get any new users.
Reviewed-by Richard Henderson <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
--- target/s390x/translate.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
target/s390x/translate.c +2 -7
@@ -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 ; }
Signature · 1 line
2.14.3
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 for-2.12 16/16] s390x/tcg: drop potential_page_fault()
Quoted 27 lines · David Hildenbrand
Only one user left, get rid of it so we don't get any new users.
Reviewed-by Richard Henderson <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- target/s390x/translate.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)target/s390x/translate.c +0
-6@@ -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);
target/s390x/translate.c static ExitStatus op_lpd(DisasContext *s, DisasOps *o) · line 2940
Quoted 9 lines
target/s390x/translate.c +2
-1@@ -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 ; }
Reviewed-by Thomas Huth <[email protected]>