[PATCH v2] target/s390x: Enforce instruction features

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2] target/s390x: Enforce instruction features

Richard Henderson
Introduce a synthetic feature (type MISC) to handle disabling of
the enforcing of features at translation time.

Signed-off-by: Richard Henderson <[hidden email]>
---
 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"),
 };
 
 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;
--
2.9.4


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] target/s390x: Enforce instruction features

no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [hidden email]
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 [hidden email]
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] target/s390x: Enforce instruction features

David Hildenbrand-3
In reply to this post by Richard Henderson
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 <[hidden email]>
> ---
>  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?


--

Thanks,

David

Loading...