| Message ID | 20170616051526.6814-1-rth@twiddle.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20170616051526.6814-1-rth@twiddle.net
Subject: [Qemu-devel] [PATCH v2] target/s390x: Enforce instruction features
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a2131fd target/s390x: Enforce instruction features
=== OUTPUT BEGIN ===
Checking PATCH 1/1: target/s390x: Enforce instruction features...
ERROR: line over 90 characters
#30: FILE: target/s390x/cpu_features.c:255:
+ FEAT_INIT("tcg-all-insns", S390_FEAT_TYPE_MISC, 0, "Enable all insns supported by TCG"),
total: 1 errors, 0 warnings, 61 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
On 16.06.2017 07:15, Richard Henderson wrote: > Introduce a synthetic feature (type MISC) to handle disabling of > the enforcing of features at translation time. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > target/s390x/cpu_features.c | 4 +++- > target/s390x/cpu_features_def.h | 1 + > target/s390x/cpu_models.c | 5 +++++ > target/s390x/translate.c | 9 +++++++++ > 4 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c > index 42fd9d7..8d542c0 100644 > --- a/target/s390x/cpu_features.c > +++ b/target/s390x/cpu_features.c > @@ -24,7 +24,7 @@ > } > > /* indexed by feature number for easy lookup */ > -static const S390FeatDef s390_features[] = { > +static const S390FeatDef s390_features[S390_FEAT_MAX] = { > FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"), > FEAT_INIT("zarch", S390_FEAT_TYPE_STFL, 1, "z/Architecture architectural mode"), > FEAT_INIT("dateh", S390_FEAT_TYPE_STFL, 3, "DAT-enhancement facility"), > @@ -251,6 +251,8 @@ static const S390FeatDef s390_features[] = { > FEAT_INIT("pcc-xts-eaes-256", S390_FEAT_TYPE_PCC, 60, "PCC Compute-XTS-Parameter-Using-Encrypted-AES-256"), > > FEAT_INIT("ppno-sha-512-drng", S390_FEAT_TYPE_PPNO, 3, "PPNO SHA-512-DRNG"), > + > + FEAT_INIT("tcg-all-insns", S390_FEAT_TYPE_MISC, 0, "Enable all insns supported by TCG"), Very mixed feelings about fake features. Will return strange/wrong/misleading information on baselining/comparing..... I don't like this. Basically also because this is a temporary hack and will - in theory - be dropped at one point. nack from my side. Can't we just handle the qemu model special as proposed?
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c index 42fd9d7..8d542c0 100644 --- a/target/s390x/cpu_features.c +++ b/target/s390x/cpu_features.c @@ -24,7 +24,7 @@ } /* indexed by feature number for easy lookup */ -static const S390FeatDef s390_features[] = { +static const S390FeatDef s390_features[S390_FEAT_MAX] = { FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"), FEAT_INIT("zarch", S390_FEAT_TYPE_STFL, 1, "z/Architecture architectural mode"), FEAT_INIT("dateh", S390_FEAT_TYPE_STFL, 3, "DAT-enhancement facility"), @@ -251,6 +251,8 @@ static const S390FeatDef s390_features[] = { FEAT_INIT("pcc-xts-eaes-256", S390_FEAT_TYPE_PCC, 60, "PCC Compute-XTS-Parameter-Using-Encrypted-AES-256"), FEAT_INIT("ppno-sha-512-drng", S390_FEAT_TYPE_PPNO, 3, "PPNO SHA-512-DRNG"), + + FEAT_INIT("tcg-all-insns", S390_FEAT_TYPE_MISC, 0, "Enable all insns supported by TCG"), }; const S390FeatDef *s390_feat_def(S390Feat feat) diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h index aa5ab8d..0726c54 100644 --- a/target/s390x/cpu_features_def.h +++ b/target/s390x/cpu_features_def.h @@ -225,6 +225,7 @@ typedef enum { S390_FEAT_PCC_XTS_EAES_128, S390_FEAT_PCC_XTS_EAES_256, S390_FEAT_PPNO_SHA_512_DRNG, + S390_FEAT_TCG_ALL_INSNS, S390_FEAT_MAX, } S390Feat; diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 478bcc6..641552e 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -685,6 +685,11 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm) S390_FEAT_GENERAL_INSTRUCTIONS_EXT, S390_FEAT_EXECUTE_EXT, S390_FEAT_STFLE_45, + + /* There are other features that are only partially implemented. + We do not advertise those above. Indicate that we should not + enforce PGM_OPERATION for insns without the feature bit set. */ + S390_FEAT_TCG_ALL_INSNS, }; int i; diff --git a/target/s390x/translate.c b/target/s390x/translate.c index af18ffb..69c1f94 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 (s->features && !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,8 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb) } dc.tb = tb; + dc.features = (s390_has_feat(S390_FEAT_TCG_ALL_INSNS) + ? NULL : cpu->model->features); dc.pc = pc_start; dc.cc_op = CC_OP_DYNAMIC; dc.ex_value = tb->cs_base;
Introduce a synthetic feature (type MISC) to handle disabling of the enforcing of features at translation time. Signed-off-by: Richard Henderson <rth@twiddle.net> --- target/s390x/cpu_features.c | 4 +++- target/s390x/cpu_features_def.h | 1 + target/s390x/cpu_models.c | 5 +++++ target/s390x/translate.c | 9 +++++++++ 4 files changed, 18 insertions(+), 1 deletion(-)