target/s390x/insn-data.def | 70 ++++++++++++++++++++++---------------------- target/s390x/translate.c | 15 +++++++++ 2 files changed, 50 insertions(+), 35 deletions(-)
This patch adds some gen_io_start() calls to allow execution
of s390x targets in icount mode with -smp 1.
It enables deterministic timers and record/replay features.
Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
---
v2:
- added IF_IO flag to reuse icount code in translate_one()
(suggested by Richard Henderson)
---
target/s390x/insn-data.def | 70 ++++++++++++++++++++++----------------------
target/s390x/translate.c | 15 +++++++++
2 files changed, 50 insertions(+), 35 deletions(-)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index d3bcdfd67b..b95bc98d35 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -379,7 +379,7 @@
/* EXTRACT CPU ATTRIBUTE */
C(0xeb4c, ECAG, RSY_a, GIE, 0, a2, r1, 0, ecag, 0)
/* EXTRACT CPU TIME */
- C(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0)
+ F(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0, IF_IO)
/* EXTRACT FPC */
F(0xb38c, EFPC, RRE, Z, 0, 0, new, r1_32, efpc, 0, IF_BFP)
/* EXTRACT PSW */
@@ -855,10 +855,10 @@
C(0xe32f, STRVG, RXY_a, Z, la2, r1_o, new, m1_64, rev64, 0)
/* STORE CLOCK */
- C(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0)
- C(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0)
+ F(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0, IF_IO)
+ F(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0, IF_IO)
/* STORE CLOCK EXTENDED */
- C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0)
+ F(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0, IF_IO)
/* STORE FACILITY LIST EXTENDED */
C(0xb2b0, STFLE, S, SFLE, 0, a2, 0, 0, stfle, 0)
@@ -1269,7 +1269,7 @@
E(0xb250, CSP, RRE, Z, r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL, IF_PRIV)
E(0xb98a, CSPG, RRE, DAT_ENH, r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ, IF_PRIV)
/* DIAGNOSE (KVM hypercall) */
- F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV)
+ F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV | IF_IO)
/* INSERT STORAGE KEY EXTENDED */
F(0xb229, ISKE, RRE, Z, 0, r2_o, new, r1_8, iske, 0, IF_PRIV)
/* INVALIDATE DAT TABLE ENTRY */
@@ -1301,17 +1301,17 @@
/* RESET REFERENCE BIT EXTENDED */
F(0xb22a, RRBE, RRE, Z, 0, r2_o, 0, 0, rrbe, 0, IF_PRIV)
/* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */
- F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV)
+ F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV | IF_IO)
/* SET ADDRESS SPACE CONTROL FAST */
F(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0, IF_PRIV)
/* SET CLOCK */
- F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV)
+ F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV | IF_IO)
/* SET CLOCK COMPARATOR */
- F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV)
+ F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV | IF_IO)
/* SET CLOCK PROGRAMMABLE FIELD */
F(0x0107, SCKPF, E, Z, 0, 0, 0, 0, sckpf, 0, IF_PRIV)
/* SET CPU TIMER */
- F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV)
+ F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV | IF_IO)
/* SET PREFIX */
F(0xb210, SPX, S, Z, 0, m2_32ua, 0, 0, spx, 0, IF_PRIV)
/* SET PSW KEY FROM ADDRESS */
@@ -1321,7 +1321,7 @@
/* SET SYSTEM MASK */
F(0x8000, SSM, S, Z, 0, m2_8u, 0, 0, ssm, 0, IF_PRIV)
/* SIGNAL PROCESSOR */
- F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV)
+ F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV | IF_IO)
/* STORE CLOCK COMPARATOR */
F(0xb207, STCKC, S, Z, la2, 0, new, m1_64a, stckc, 0, IF_PRIV)
/* STORE CONTROL */
@@ -1332,7 +1332,7 @@
/* STORE CPU ID */
F(0xb202, STIDP, S, Z, la2, 0, new, m1_64a, stidp, 0, IF_PRIV)
/* STORE CPU TIMER */
- F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0, IF_PRIV)
+ F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0, IF_PRIV | IF_IO)
/* STORE FACILITY LIST */
F(0xb2b1, STFL, S, Z, 0, 0, 0, 0, stfl, 0, IF_PRIV)
/* STORE PREFIX */
@@ -1352,35 +1352,35 @@
C(0xe501, TPROT, SSE, Z, la1, a2, 0, 0, tprot, 0)
/* CCW I/O Instructions */
- F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV)
- F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV)
- F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV)
- F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV)
- F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV)
- F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV)
- F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV)
- F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV)
- F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV)
- F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV)
- F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV)
- F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV)
- F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV)
- F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV)
- F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV)
+ F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV | IF_IO)
+ F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV | IF_IO)
+ F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV | IF_IO)
+ F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV | IF_IO)
+ F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV | IF_IO)
+ F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV | IF_IO)
+ F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV | IF_IO)
+ F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV | IF_IO)
+ F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV | IF_IO)
+ F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV | IF_IO)
+ F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV | IF_IO)
+ F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV | IF_IO)
+ F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV | IF_IO)
+ F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV | IF_IO)
+ F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV | IF_IO)
/* ??? Not listed in PoO ninth edition, but there's a linux driver that
uses it: "A CHSC subchannel is usually present on LPAR only." */
- F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV)
+ F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV | IF_IO)
/* zPCI Instructions */
/* None of these instructions are documented in the PoP, so this is all
based upon target/s390x/kvm.c and Linux code and likely incomplete */
- F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV)
- F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV)
- F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV)
- F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV)
- F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV)
- F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV)
- F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV)
- F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV)
+ F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV | IF_IO)
+ F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV | IF_IO)
+ F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV | IF_IO)
+ F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV | IF_IO)
+ F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV | IF_IO)
+ F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV | IF_IO)
+ F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV | IF_IO)
+ F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV | IF_IO)
#endif /* CONFIG_USER_ONLY */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ac10f42f10..7a8ff1f2de 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1214,6 +1214,7 @@ typedef struct {
#define IF_DFP 0x0010 /* decimal floating point instruction */
#define IF_PRIV 0x0020 /* privileged instruction */
#define IF_VEC 0x0040 /* vector instruction */
+#define IF_IO 0x0080 /* input/output instruction */
struct DisasInsn {
unsigned opc:16;
@@ -6369,6 +6370,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
const DisasInsn *insn;
DisasJumpType ret = DISAS_NEXT;
DisasOps o = {};
+ bool icount = false;
/* Search for the insn in the table. */
insn = extract_insn(env, s);
@@ -6435,6 +6437,14 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
return DISAS_NORETURN;
}
}
+
+ /* input/output is the special case for icount mode */
+ if (insn->flags & IF_IO) {
+ icount = tb_cflags(s->base.tb) & CF_USE_ICOUNT;
+ if (icount) {
+ gen_io_start();
+ }
+ }
}
/* Check for insn specification exceptions. */
@@ -6488,6 +6498,11 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
tcg_temp_free_i64(o.addr1);
}
+ /* io should be the last instruction in tb when icount is enabled */
+ if (icount && ret == DISAS_NEXT) {
+ ret = DISAS_PC_STALE;
+ }
+
#ifndef CONFIG_USER_ONLY
if (s->base.tb->flags & FLAG_MASK_PER) {
/* An exception might be triggered, save PSW if not already done. */
On 11/2/20 10:26 AM, Pavel Dovgalyuk wrote:
> This patch adds some gen_io_start() calls to allow execution
> of s390x targets in icount mode with -smp 1.
> It enables deterministic timers and record/replay features.
>
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
>
> ---
>
> v2:
> - added IF_IO flag to reuse icount code in translate_one()
> (suggested by Richard Henderson)
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/insn-data.def | 70 ++++++++++++++++++++++----------------------
> target/s390x/translate.c | 15 +++++++++
> 2 files changed, 50 insertions(+), 35 deletions(-)
...
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index ac10f42f10..7a8ff1f2de 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1214,6 +1214,7 @@ typedef struct {
> #define IF_DFP 0x0010 /* decimal floating point instruction */
> #define IF_PRIV 0x0020 /* privileged instruction */
> #define IF_VEC 0x0040 /* vector instruction */
> +#define IF_IO 0x0080 /* input/output instruction */
>
> struct DisasInsn {
> unsigned opc:16;
> @@ -6369,6 +6370,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> const DisasInsn *insn;
> DisasJumpType ret = DISAS_NEXT;
> DisasOps o = {};
> + bool icount = false;
>
> /* Search for the insn in the table. */
> insn = extract_insn(env, s);
> @@ -6435,6 +6437,14 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> return DISAS_NORETURN;
> }
> }
> +
> + /* input/output is the special case for icount mode */
> + if (insn->flags & IF_IO) {
> + icount = tb_cflags(s->base.tb) & CF_USE_ICOUNT;
> + if (icount) {
> + gen_io_start();
> + }
> + }
> }
>
> /* Check for insn specification exceptions. */
> @@ -6488,6 +6498,11 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> tcg_temp_free_i64(o.addr1);
> }
>
> + /* io should be the last instruction in tb when icount is enabled */
> + if (icount && ret == DISAS_NEXT) {
> + ret = DISAS_PC_STALE;
> + }
> +
> #ifndef CONFIG_USER_ONLY
> if (s->base.tb->flags & FLAG_MASK_PER) {
> /* An exception might be triggered, save PSW if not already done. */
>
>
A bit too s390x-specific to me, but the generic approach looks great :)
On 11/2/20 1:26 AM, Pavel Dovgalyuk wrote: > This patch adds some gen_io_start() calls to allow execution > of s390x targets in icount mode with -smp 1. > It enables deterministic timers and record/replay features. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Mon, 02 Nov 2020 12:26:59 +0300 Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> wrote: > This patch adds some gen_io_start() calls to allow execution > of s390x targets in icount mode with -smp 1. > It enables deterministic timers and record/replay features. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> > > --- > > v2: > - added IF_IO flag to reuse icount code in translate_one() > (suggested by Richard Henderson) > --- > target/s390x/insn-data.def | 70 ++++++++++++++++++++++---------------------- > target/s390x/translate.c | 15 +++++++++ > 2 files changed, 50 insertions(+), 35 deletions(-) <looking for s390x patches to pick> So, will there be a v3, or should I pick this one?
On 04.11.2020 20:31, Cornelia Huck wrote: > On Mon, 02 Nov 2020 12:26:59 +0300 > Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> wrote: > >> This patch adds some gen_io_start() calls to allow execution >> of s390x targets in icount mode with -smp 1. >> It enables deterministic timers and record/replay features. >> >> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> >> >> --- >> >> v2: >> - added IF_IO flag to reuse icount code in translate_one() >> (suggested by Richard Henderson) >> --- >> target/s390x/insn-data.def | 70 ++++++++++++++++++++++---------------------- >> target/s390x/translate.c | 15 +++++++++ >> 2 files changed, 50 insertions(+), 35 deletions(-) > > <looking for s390x patches to pick> > > So, will there be a v3, or should I pick this one? > Sent v3, please take that one. Pavel Dovgalyuk
On 02.11.20 10:26, Pavel Dovgalyuk wrote:
> This patch adds some gen_io_start() calls to allow execution
> of s390x targets in icount mode with -smp 1.
> It enables deterministic timers and record/replay features.
Why do we have to set it for SIGP?
>
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
>
> ---
>
> v2:
> - added IF_IO flag to reuse icount code in translate_one()
> (suggested by Richard Henderson)
> ---
> target/s390x/insn-data.def | 70 ++++++++++++++++++++++----------------------
> target/s390x/translate.c | 15 +++++++++
> 2 files changed, 50 insertions(+), 35 deletions(-)
>
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index d3bcdfd67b..b95bc98d35 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -379,7 +379,7 @@
> /* EXTRACT CPU ATTRIBUTE */
> C(0xeb4c, ECAG, RSY_a, GIE, 0, a2, r1, 0, ecag, 0)
> /* EXTRACT CPU TIME */
> - C(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0)
> + F(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0, IF_IO)
> /* EXTRACT FPC */
> F(0xb38c, EFPC, RRE, Z, 0, 0, new, r1_32, efpc, 0, IF_BFP)
> /* EXTRACT PSW */
> @@ -855,10 +855,10 @@
> C(0xe32f, STRVG, RXY_a, Z, la2, r1_o, new, m1_64, rev64, 0)
>
> /* STORE CLOCK */
> - C(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0)
> - C(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0)
> + F(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0, IF_IO)
> + F(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0, IF_IO)
> /* STORE CLOCK EXTENDED */
> - C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0)
> + F(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0, IF_IO)
>
> /* STORE FACILITY LIST EXTENDED */
> C(0xb2b0, STFLE, S, SFLE, 0, a2, 0, 0, stfle, 0)
> @@ -1269,7 +1269,7 @@
> E(0xb250, CSP, RRE, Z, r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL, IF_PRIV)
> E(0xb98a, CSPG, RRE, DAT_ENH, r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ, IF_PRIV)
> /* DIAGNOSE (KVM hypercall) */
> - F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV)
> + F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV | IF_IO)
> /* INSERT STORAGE KEY EXTENDED */
> F(0xb229, ISKE, RRE, Z, 0, r2_o, new, r1_8, iske, 0, IF_PRIV)
> /* INVALIDATE DAT TABLE ENTRY */
> @@ -1301,17 +1301,17 @@
> /* RESET REFERENCE BIT EXTENDED */
> F(0xb22a, RRBE, RRE, Z, 0, r2_o, 0, 0, rrbe, 0, IF_PRIV)
> /* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */
> - F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV)
> + F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV | IF_IO)
> /* SET ADDRESS SPACE CONTROL FAST */
> F(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0, IF_PRIV)
> /* SET CLOCK */
> - F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV)
> + F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV | IF_IO)
> /* SET CLOCK COMPARATOR */
> - F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV)
> + F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV | IF_IO)
> /* SET CLOCK PROGRAMMABLE FIELD */
> F(0x0107, SCKPF, E, Z, 0, 0, 0, 0, sckpf, 0, IF_PRIV)
> /* SET CPU TIMER */
> - F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV)
> + F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV | IF_IO)
> /* SET PREFIX */
> F(0xb210, SPX, S, Z, 0, m2_32ua, 0, 0, spx, 0, IF_PRIV)
> /* SET PSW KEY FROM ADDRESS */
> @@ -1321,7 +1321,7 @@
> /* SET SYSTEM MASK */
> F(0x8000, SSM, S, Z, 0, m2_8u, 0, 0, ssm, 0, IF_PRIV)
> /* SIGNAL PROCESSOR */
> - F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV)
> + F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV | IF_IO)
> /* STORE CLOCK COMPARATOR */
> F(0xb207, STCKC, S, Z, la2, 0, new, m1_64a, stckc, 0, IF_PRIV)
> /* STORE CONTROL */
> @@ -1332,7 +1332,7 @@
> /* STORE CPU ID */
> F(0xb202, STIDP, S, Z, la2, 0, new, m1_64a, stidp, 0, IF_PRIV)
> /* STORE CPU TIMER */
> - F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0, IF_PRIV)
> + F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0, IF_PRIV | IF_IO)
> /* STORE FACILITY LIST */
> F(0xb2b1, STFL, S, Z, 0, 0, 0, 0, stfl, 0, IF_PRIV)
> /* STORE PREFIX */
> @@ -1352,35 +1352,35 @@
> C(0xe501, TPROT, SSE, Z, la1, a2, 0, 0, tprot, 0)
>
> /* CCW I/O Instructions */
> - F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV)
> - F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV)
> - F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV)
> - F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV)
> - F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV)
> - F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV)
> - F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV)
> - F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV)
> - F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV)
> - F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV)
> - F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV)
> - F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV)
> - F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV)
> - F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV)
> - F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV)
> + F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV | IF_IO)
> + F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV | IF_IO)
> + F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV | IF_IO)
> + F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV | IF_IO)
> + F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV | IF_IO)
> + F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV | IF_IO)
> + F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV | IF_IO)
> + F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV | IF_IO)
> + F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV | IF_IO)
> + F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV | IF_IO)
> + F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV | IF_IO)
> + F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV | IF_IO)
> + F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV | IF_IO)
> + F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV | IF_IO)
> + F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV | IF_IO)
> /* ??? Not listed in PoO ninth edition, but there's a linux driver that
> uses it: "A CHSC subchannel is usually present on LPAR only." */
> - F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV)
> + F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV | IF_IO)
>
> /* zPCI Instructions */
> /* None of these instructions are documented in the PoP, so this is all
> based upon target/s390x/kvm.c and Linux code and likely incomplete */
> - F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV)
> - F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV)
> - F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV)
> - F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV)
> - F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV)
> - F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV)
> - F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV)
> - F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV)
> + F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV | IF_IO)
> + F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV | IF_IO)
> + F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV | IF_IO)
> + F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV | IF_IO)
> + F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV | IF_IO)
> + F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV | IF_IO)
> + F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV | IF_IO)
> + F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV | IF_IO)
>
> #endif /* CONFIG_USER_ONLY */
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index ac10f42f10..7a8ff1f2de 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1214,6 +1214,7 @@ typedef struct {
> #define IF_DFP 0x0010 /* decimal floating point instruction */
> #define IF_PRIV 0x0020 /* privileged instruction */
> #define IF_VEC 0x0040 /* vector instruction */
> +#define IF_IO 0x0080 /* input/output instruction */
>
> struct DisasInsn {
> unsigned opc:16;
> @@ -6369,6 +6370,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> const DisasInsn *insn;
> DisasJumpType ret = DISAS_NEXT;
> DisasOps o = {};
> + bool icount = false;
>
> /* Search for the insn in the table. */
> insn = extract_insn(env, s);
> @@ -6435,6 +6437,14 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> return DISAS_NORETURN;
> }
> }
> +
> + /* input/output is the special case for icount mode */
> + if (insn->flags & IF_IO) {
I guess this is an unlikely().
> + icount = tb_cflags(s->base.tb) & CF_USE_ICOUNT;
> + if (icount) {
> + gen_io_start();
> + }
> + }
> }
>
> /* Check for insn specification exceptions. */
> @@ -6488,6 +6498,11 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> tcg_temp_free_i64(o.addr1);
> }
>
> + /* io should be the last instruction in tb when icount is enabled */
> + if (icount && ret == DISAS_NEXT) {
> + ret = DISAS_PC_STALE;
> + }
> +
dito.
> #ifndef CONFIG_USER_ONLY
> if (s->base.tb->flags & FLAG_MASK_PER) {
> /* An exception might be triggered, save PSW if not already done. */
>
In general, looks sane to me.
--
Thanks,
David / dhildenb
On 02.11.2020 12:34, David Hildenbrand wrote:
> On 02.11.20 10:26, Pavel Dovgalyuk wrote:
>> This patch adds some gen_io_start() calls to allow execution
>> of s390x targets in icount mode with -smp 1.
>> It enables deterministic timers and record/replay features.
>
> Why do we have to set it for SIGP?
Timer-related instructions obviously needed IO.
But I'm not familiar with s390, therefore also added IO
for the instructions that lock iothread mutex, because they
probably can access some virtual hardware and change the timers.
But maybe this is not related to SIGP and it takes the mutex
for other reasons.
>
>>
>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
>>
>> ---
>>
>> v2:
>> - added IF_IO flag to reuse icount code in translate_one()
>> (suggested by Richard Henderson)
>> ---
>> target/s390x/insn-data.def | 70
>> ++++++++++++++++++++++----------------------
>> target/s390x/translate.c | 15 +++++++++
>> 2 files changed, 50 insertions(+), 35 deletions(-)
>>
>> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
>> index d3bcdfd67b..b95bc98d35 100644
>> --- a/target/s390x/insn-data.def
>> +++ b/target/s390x/insn-data.def
>> @@ -379,7 +379,7 @@
>> /* EXTRACT CPU ATTRIBUTE */
>> C(0xeb4c, ECAG, RSY_a, GIE, 0, a2, r1, 0, ecag, 0)
>> /* EXTRACT CPU TIME */
>> - C(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0)
>> + F(0xc801, ECTG, SSF, ECT, 0, 0, 0, 0, ectg, 0, IF_IO)
>> /* EXTRACT FPC */
>> F(0xb38c, EFPC, RRE, Z, 0, 0, new, r1_32, efpc, 0, IF_BFP)
>> /* EXTRACT PSW */
>> @@ -855,10 +855,10 @@
>> C(0xe32f, STRVG, RXY_a, Z, la2, r1_o, new, m1_64, rev64, 0)
>> /* STORE CLOCK */
>> - C(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0)
>> - C(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0)
>> + F(0xb205, STCK, S, Z, la2, 0, new, m1_64, stck, 0, IF_IO)
>> + F(0xb27c, STCKF, S, SCF, la2, 0, new, m1_64, stck, 0, IF_IO)
>> /* STORE CLOCK EXTENDED */
>> - C(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0)
>> + F(0xb278, STCKE, S, Z, 0, a2, 0, 0, stcke, 0, IF_IO)
>> /* STORE FACILITY LIST EXTENDED */
>> C(0xb2b0, STFLE, S, SFLE, 0, a2, 0, 0, stfle, 0)
>> @@ -1269,7 +1269,7 @@
>> E(0xb250, CSP, RRE, Z, r1_32u, ra2, r1_P, 0, csp, 0,
>> MO_TEUL, IF_PRIV)
>> E(0xb98a, CSPG, RRE, DAT_ENH, r1_o, ra2, r1_P, 0, csp, 0,
>> MO_TEQ, IF_PRIV)
>> /* DIAGNOSE (KVM hypercall) */
>> - F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV)
>> + F(0x8300, DIAG, RSI, Z, 0, 0, 0, 0, diag, 0, IF_PRIV | IF_IO)
>> /* INSERT STORAGE KEY EXTENDED */
>> F(0xb229, ISKE, RRE, Z, 0, r2_o, new, r1_8, iske, 0,
>> IF_PRIV)
>> /* INVALIDATE DAT TABLE ENTRY */
>> @@ -1301,17 +1301,17 @@
>> /* RESET REFERENCE BIT EXTENDED */
>> F(0xb22a, RRBE, RRE, Z, 0, r2_o, 0, 0, rrbe, 0, IF_PRIV)
>> /* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */
>> - F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV)
>> + F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0,
>> IF_PRIV | IF_IO)
>> /* SET ADDRESS SPACE CONTROL FAST */
>> F(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0, IF_PRIV)
>> /* SET CLOCK */
>> - F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV)
>> + F(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0, IF_PRIV |
>> IF_IO)
>> /* SET CLOCK COMPARATOR */
>> - F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV)
>> + F(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0, IF_PRIV
>> | IF_IO)
>> /* SET CLOCK PROGRAMMABLE FIELD */
>> F(0x0107, SCKPF, E, Z, 0, 0, 0, 0, sckpf, 0, IF_PRIV)
>> /* SET CPU TIMER */
>> - F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV)
>> + F(0xb208, SPT, S, Z, 0, m2_64a, 0, 0, spt, 0, IF_PRIV |
>> IF_IO)
>> /* SET PREFIX */
>> F(0xb210, SPX, S, Z, 0, m2_32ua, 0, 0, spx, 0, IF_PRIV)
>> /* SET PSW KEY FROM ADDRESS */
>> @@ -1321,7 +1321,7 @@
>> /* SET SYSTEM MASK */
>> F(0x8000, SSM, S, Z, 0, m2_8u, 0, 0, ssm, 0, IF_PRIV)
>> /* SIGNAL PROCESSOR */
>> - F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV)
>> + F(0xae00, SIGP, RS_a, Z, 0, a2, 0, 0, sigp, 0, IF_PRIV |
>> IF_IO)
>> /* STORE CLOCK COMPARATOR */
>> F(0xb207, STCKC, S, Z, la2, 0, new, m1_64a, stckc, 0,
>> IF_PRIV)
>> /* STORE CONTROL */
>> @@ -1332,7 +1332,7 @@
>> /* STORE CPU ID */
>> F(0xb202, STIDP, S, Z, la2, 0, new, m1_64a, stidp, 0,
>> IF_PRIV)
>> /* STORE CPU TIMER */
>> - F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0,
>> IF_PRIV)
>> + F(0xb209, STPT, S, Z, la2, 0, new, m1_64a, stpt, 0,
>> IF_PRIV | IF_IO)
>> /* STORE FACILITY LIST */
>> F(0xb2b1, STFL, S, Z, 0, 0, 0, 0, stfl, 0, IF_PRIV)
>> /* STORE PREFIX */
>> @@ -1352,35 +1352,35 @@
>> C(0xe501, TPROT, SSE, Z, la1, a2, 0, 0, tprot, 0)
>> /* CCW I/O Instructions */
>> - F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV)
>> - F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV)
>> - F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV)
>> - F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV)
>> - F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV)
>> - F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV)
>> - F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV)
>> - F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV)
>> - F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV)
>> - F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV)
>> - F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV)
>> - F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV)
>> - F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV)
>> - F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV)
>> - F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV)
>> + F(0xb276, XSCH, S, Z, 0, 0, 0, 0, xsch, 0, IF_PRIV | IF_IO)
>> + F(0xb230, CSCH, S, Z, 0, 0, 0, 0, csch, 0, IF_PRIV | IF_IO)
>> + F(0xb231, HSCH, S, Z, 0, 0, 0, 0, hsch, 0, IF_PRIV | IF_IO)
>> + F(0xb232, MSCH, S, Z, 0, insn, 0, 0, msch, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb23b, RCHP, S, Z, 0, 0, 0, 0, rchp, 0, IF_PRIV | IF_IO)
>> + F(0xb238, RSCH, S, Z, 0, 0, 0, 0, rsch, 0, IF_PRIV | IF_IO)
>> + F(0xb237, SAL, S, Z, 0, 0, 0, 0, sal, 0, IF_PRIV | IF_IO)
>> + F(0xb23c, SCHM, S, Z, 0, insn, 0, 0, schm, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb274, SIGA, S, Z, 0, 0, 0, 0, siga, 0, IF_PRIV | IF_IO)
>> + F(0xb23a, STCPS, S, Z, 0, 0, 0, 0, stcps, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb233, SSCH, S, Z, 0, insn, 0, 0, ssch, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb239, STCRW, S, Z, 0, insn, 0, 0, stcrw, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb234, STSCH, S, Z, 0, insn, 0, 0, stsch, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb236, TPI , S, Z, la2, 0, 0, 0, tpi, 0, IF_PRIV |
>> IF_IO)
>> + F(0xb235, TSCH, S, Z, 0, insn, 0, 0, tsch, 0, IF_PRIV |
>> IF_IO)
>> /* ??? Not listed in PoO ninth edition, but there's a linux
>> driver that
>> uses it: "A CHSC subchannel is usually present on LPAR
>> only." */
>> - F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV)
>> + F(0xb25f, CHSC, RRE, Z, 0, insn, 0, 0, chsc, 0, IF_PRIV |
>> IF_IO)
>> /* zPCI Instructions */
>> /* None of these instructions are documented in the PoP, so this
>> is all
>> based upon target/s390x/kvm.c and Linux code and likely
>> incomplete */
>> - F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV)
>> - F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV)
>> - F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV)
>> - F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV)
>> - F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV)
>> - F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV)
>> - F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV)
>> - F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV)
>> + F(0xebd0, PCISTB, RSY_a, PCI, la2, 0, 0, 0, pcistb, 0, IF_PRIV |
>> IF_IO)
>> + F(0xebd1, SIC, RSY_a, AIS, r1, r3, 0, 0, sic, 0, IF_PRIV | IF_IO)
>> + F(0xb9a0, CLP, RRF_c, PCI, 0, 0, 0, 0, clp, 0, IF_PRIV | IF_IO)
>> + F(0xb9d0, PCISTG, RRE, PCI, 0, 0, 0, 0, pcistg, 0, IF_PRIV | IF_IO)
>> + F(0xb9d2, PCILG, RRE, PCI, 0, 0, 0, 0, pcilg, 0, IF_PRIV | IF_IO)
>> + F(0xb9d3, RPCIT, RRE, PCI, 0, 0, 0, 0, rpcit, 0, IF_PRIV | IF_IO)
>> + F(0xe3d0, MPCIFC, RXY_a, PCI, la2, 0, 0, 0, mpcifc, 0, IF_PRIV |
>> IF_IO)
>> + F(0xe3d4, STPCIFC, RXY_a, PCI, la2, 0, 0, 0, stpcifc, 0, IF_PRIV
>> | IF_IO)
>> #endif /* CONFIG_USER_ONLY */
>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>> index ac10f42f10..7a8ff1f2de 100644
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -1214,6 +1214,7 @@ typedef struct {
>> #define IF_DFP 0x0010 /* decimal floating point
>> instruction */
>> #define IF_PRIV 0x0020 /* privileged instruction */
>> #define IF_VEC 0x0040 /* vector instruction */
>> +#define IF_IO 0x0080 /* input/output instruction */
>> struct DisasInsn {
>> unsigned opc:16;
>> @@ -6369,6 +6370,7 @@ static DisasJumpType translate_one(CPUS390XState
>> *env, DisasContext *s)
>> const DisasInsn *insn;
>> DisasJumpType ret = DISAS_NEXT;
>> DisasOps o = {};
>> + bool icount = false;
>> /* Search for the insn in the table. */
>> insn = extract_insn(env, s);
>> @@ -6435,6 +6437,14 @@ static DisasJumpType
>> translate_one(CPUS390XState *env, DisasContext *s)
>> return DISAS_NORETURN;
>> }
>> }
>> +
>> + /* input/output is the special case for icount mode */
>> + if (insn->flags & IF_IO) {
>
> I guess this is an unlikely().
ok
>
>> + icount = tb_cflags(s->base.tb) & CF_USE_ICOUNT;
>> + if (icount) {
>> + gen_io_start();
>> + }
>> + }
>> }
>> /* Check for insn specification exceptions. */
>> @@ -6488,6 +6498,11 @@ static DisasJumpType
>> translate_one(CPUS390XState *env, DisasContext *s)
>> tcg_temp_free_i64(o.addr1);
>> }
>> + /* io should be the last instruction in tb when icount is enabled */
>> + if (icount && ret == DISAS_NEXT) {
>> + ret = DISAS_PC_STALE;
>> + }
>> +
>
> dito.
>
>> #ifndef CONFIG_USER_ONLY
>> if (s->base.tb->flags & FLAG_MASK_PER) {
>> /* An exception might be triggered, save PSW if not already
>> done. */
>>
>
> In general, looks sane to me.
>
On 02.11.20 10:41, Pavel Dovgalyuk wrote: > On 02.11.2020 12:34, David Hildenbrand wrote: >> On 02.11.20 10:26, Pavel Dovgalyuk wrote: >>> This patch adds some gen_io_start() calls to allow execution >>> of s390x targets in icount mode with -smp 1. >>> It enables deterministic timers and record/replay features. >> >> Why do we have to set it for SIGP? > > Timer-related instructions obviously needed IO. > But I'm not familiar with s390, therefore also added IO > for the instructions that lock iothread mutex, because they > probably can access some virtual hardware and change the timers. > But maybe this is not related to SIGP and it takes the mutex > for other reasons. SIGP does IPI but also triggers CPU reset, so it deals with timers in some sense (reset). So looks like the right thing to do. Thanks! Acked-by: David Hildenbrand <david@redhat.com> -- Thanks, David / dhildenb
© 2016 - 2024 Red Hat, Inc.