On 2017-06-14 22:53, Richard Henderson wrote: > Signed-off-by: Richard Henderson <r...@twiddle.net> > --- > target/s390x/translate.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/target/s390x/translate.c b/target/s390x/translate.c > index af18ffb..48cee25 100644 > --- a/target/s390x/translate.c > +++ b/target/s390x/translate.c > @@ -55,6 +55,7 @@ typedef struct DisasFields DisasFields; > > struct DisasContext { > struct TranslationBlock *tb; > + const unsigned long *features; > const DisasInsn *insn; > DisasFields *fields; > uint64_t ex_value; > @@ -5600,6 +5601,12 @@ static ExitStatus translate_one(CPUS390XState *env, > DisasContext *s) > } > #endif > > + /* Check for insn feature enabled. */ > + if (!test_bit(insn->fac, s->features)) { > + gen_program_exception(s, PGM_OPERATION); > + return EXIT_NORETURN; > + } > + > /* Check for insn specification exceptions. */ > if (insn->spec) { > int spec = insn->spec, excp = 0, r; > @@ -5726,6 +5733,7 @@ void gen_intermediate_code(CPUS390XState *env, struct > TranslationBlock *tb) > } > > dc.tb = tb; > + dc.features = cpu->model->features; > dc.pc = pc_start; > dc.cc_op = CC_OP_DYNAMIC; > dc.ex_value = tb->cs_base;
It looks correct technically. Now I am not sure we want to do that, without at least provided a user to enable those instructions. There are a lot facilities that are partially implemented, usually because only the really used instructions are implemented, but also because some facilities are grouped in a single feature bit. This includes for example FPE, LPP, MIE, LAT, IEEEE_SIM and more. We could maybe provide a way to override the check, or only enable enforcement for fully implemented facilities. Failing to do so means we just introduce a regression from the user point of view (many binaries will stop working) and also that we change thousand of lines of code into dead code. Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net