[Qemu-devel] [PATCH/RFC 0/3] s390x/kvm: implement new hardware/firmware features

[Qemu-devel] [PATCH/RFC 0/3] s390x/kvm: implement new hardware/firmware features

[Christian Borntraeger]

We want to provide more hw features to guests, namely the new bpb
control as well as other transparent facilities that might be
introduced by firmware updates (e.g. the stfle facility 81).

See the kernel discussion for the KVM side
https://www.spinics.net/lists/kernel/msg2700551.html

Christian Borntraeger (2):
  header sync
  s390x/kvm: Handle bpb feature

Halil Pasic (1):
  s390x/cpumodel: fix transparency for non-hyp STFL features

 linux-headers/asm-s390/kvm.h    |   9 ++--
 linux-headers/linux/kvm.h       |   5 +-
 target/s390x/cpu.c              |   1 +
 target/s390x/cpu.h              |   1 +
 target/s390x/cpu_features.c     |  55 +++++++++++++++++++++
 target/s390x/cpu_features_def.h |  55 +++++++++++++++++++++
 target/s390x/gen-features.c     | 104 ++++++++++++++++++++++++++++++++++++++++
 target/s390x/kvm.c              |  16 +++++++
 target/s390x/machine.c          |  17 +++++++
 9 files changed, 256 insertions(+), 7 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/3] header sync

[Christian Borntraeger]

replace with proper header sync

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 9 ++++-----
 linux-headers/linux/kvm.h    | 5 +++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 32d372e..11def14 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -6,10 +6,6 @@
  *
  * Copyright IBM Corp. 2008
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License (version 2 only)
- * as published by the Free Software Foundation.
- *
  *    Author(s): Carsten Otte <cotte@de.ibm.com>
  *               Christian Borntraeger <borntraeger@de.ibm.com>
  */
@@ -228,6 +224,7 @@ struct kvm_guest_debug_arch {
 #define KVM_SYNC_RICCB  (1UL << 7)
 #define KVM_SYNC_FPRS   (1UL << 8)
 #define KVM_SYNC_GSCB   (1UL << 9)
+#define KVM_SYNC_BPBC   (1UL << 10)
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8
 #define SDNXL (1UL << SDNXC)
@@ -251,7 +248,9 @@ struct kvm_sync_regs {
 	};
 	__u8  reserved[512];	/* for future vector expansion */
 	__u32 fpc;		/* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */
-	__u8 padding1[52];	/* riccb needs to be 64byte aligned */
+	__u8 bpbc : 1;		/* bp mode */
+	__u8 reserved2 : 7;
+	__u8 padding1[51];	/* riccb needs to be 64byte aligned */
 	__u8 riccb[64];		/* runtime instrumentation controls block */
 	__u8 padding2[192];	/* sdnx needs to be 256byte aligned */
 	union {
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ce6c2f1..b4503d8 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -630,9 +630,9 @@ struct kvm_s390_irq {
 
 struct kvm_s390_irq_state {
 	__u64 buf;
-	__u32 flags;
+	__u32 flags;        /* will stay unused for compatibility reasons */
 	__u32 len;
-	__u32 reserved[4];
+	__u32 reserved[4];  /* will stay unused for compatibility reasons */
 };
 
 /* for KVM_SET_GUEST_DEBUG */
@@ -932,6 +932,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_HYPERV_SYNIC2 148
 #define KVM_CAP_HYPERV_VP_INDEX 149
 #define KVM_CAP_S390_AIS_MIGRATION 150
+#define KVM_CAP_S390_BPB 151
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.9.4

[Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Christian Borntraeger]

We need to handle the bpb control on reset and migration. Normally
stfle.82 is transparent (and the normal guest part works without
hypervisor activity). To prevent any issues we require full
host kernel support for this feature.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target/s390x/cpu.c              |  1 +
 target/s390x/cpu.h              |  1 +
 target/s390x/cpu_features.c     |  1 +
 target/s390x/cpu_features_def.h |  1 +
 target/s390x/gen-features.c     |  1 +
 target/s390x/kvm.c              | 16 ++++++++++++++++
 target/s390x/machine.c          | 17 +++++++++++++++++
 7 files changed, 38 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index ae3cee9..1577b2c 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -89,6 +89,7 @@ static void s390_cpu_reset(CPUState *s)
     CPUS390XState *env = &cpu->env;
 
     env->pfault_token = -1UL;
+    env->bpbc = 0;
     scc->parent_reset(s);
     cpu->env.sigp_order = 0;
     s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 1a8b6b9..8514905 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -93,6 +93,7 @@ struct CPUS390XState {
 
     uint32_t fpc;          /* floating-point control register */
     uint32_t cc_op;
+    uint8_t bpbc;          /* branch prediction blocking */
 
     float_status fpu_status; /* passed to softfloat lib */
 
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 31a4676..5d1c210 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -89,6 +89,7 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
     FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
     FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
+    FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
     FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
     FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
     FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 4b6d4e9..4487cfd 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -80,6 +80,7 @@ typedef enum {
     S390_FEAT_MSA_EXT_4,
     S390_FEAT_EDAT_2,
     S390_FEAT_DFP_PACKED_CONVERSION,
+    S390_FEAT_BPB,
     S390_FEAT_VECTOR,
     S390_FEAT_INSTRUCTION_EXEC_PROT,
     S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index b24f6ad..95ee870 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -357,6 +357,7 @@ static uint16_t full_GEN7_GA1[] = {
     S390_FEAT_SIE_GPERE,
     S390_FEAT_SIE_IB,
     S390_FEAT_SIE_CEI,
+    S390_FEAT_BPB,
 };
 
 static uint16_t full_GEN7_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 6a18a41..3cd4fab 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -490,6 +490,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
     }
 
+    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
+       cs->kvm_run->s.regs.bpbc = env->bpbc;
+       cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_BPBC;
+    }
+
     /* Finally the prefix */
     if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
         cs->kvm_run->s.regs.prefix = env->psa;
@@ -600,6 +605,10 @@ int kvm_arch_get_registers(CPUState *cs)
         memcpy(env->gscb, cs->kvm_run->s.regs.gscb, 32);
     }
 
+    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
+       env->bpbc = cs->kvm_run->s.regs.bpbc;
+    }
+
     /* pfault parameters */
     if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
         env->pfault_token = cs->kvm_run->s.regs.pft;
@@ -2278,6 +2287,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         clear_bit(S390_FEAT_CMM_NT, model->features);
     }
 
+    /* stfle.82 is a transparent bit. As there is some state attached
+     * anyway we only enable this bit if the host kernel can handle
+     * migrate and reset */
+    if (!kvm_check_extension(kvm_state, KVM_CAP_S390_BPB)) {
+        clear_bit(S390_FEAT_BPB, model->features);
+    }
+
     /* We emulate a zPCI bus and AEN, therefore we don't need HW support */
     if (pci_available) {
         set_bit(S390_FEAT_ZPCI, model->features);
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index b78f326..4aac456 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -194,6 +194,22 @@ const VMStateDescription vmstate_gscb = {
         }
 };
 
+static bool bpbc_needed(void *opaque)
+{
+    return s390_has_feat(S390_FEAT_BPB);
+}
+
+const VMStateDescription vmstate_bpbc = {
+    .name = "cpu/bpbc",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = bpbc_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(env.bpbc, S390CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_s390_cpu = {
     .name = "cpu",
     .post_load = cpu_post_load,
@@ -228,6 +244,7 @@ const VMStateDescription vmstate_s390_cpu = {
         &vmstate_riccb,
         &vmstate_exval,
         &vmstate_gscb,
+        &vmstate_bpbc,
         NULL
     },
 };
-- 
2.9.4

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

On 17.01.2018 15:18, Christian Borntraeger wrote:
> We need to handle the bpb control on reset and migration. Normally
> stfle.82 is transparent (and the normal guest part works without
> hypervisor activity). To prevent any issues we require full
> host kernel support for this feature.

Actually it is not transparent because we need hypervisor support to get
VSIE running... or what am I missing? (or were you talking about bit 81?)


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Christian Borntraeger]

On 01/17/2018 03:30 PM, David Hildenbrand wrote:
> On 17.01.2018 15:18, Christian Borntraeger wrote:
>> We need to handle the bpb control on reset and migration. Normally
>> stfle.82 is transparent (and the normal guest part works without
>> hypervisor activity). To prevent any issues we require full
>> host kernel support for this feature.
> 
> Actually it is not transparent because we need hypervisor support to get
> VSIE running... or what am I missing? (or were you talking about bit 81?)

When you pass along the bit as a transparent bit (just enable it if the host
has it) it will work in nested guests. Its only that after reset or vsie you
have a short period of time where you work in a stale state.
Anyway we are aware that bit82 should have been a non-transparent bit, consider
it a quirk in the architecture. Thats why I handle this feature not in patch 3.

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Cornelia Huck]

On Wed, 17 Jan 2018 15:18:48 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> We need to handle the bpb control on reset and migration. Normally
> stfle.82 is transparent (and the normal guest part works without
> hypervisor activity). To prevent any issues we require full
> host kernel support for this feature.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  target/s390x/cpu.c              |  1 +
>  target/s390x/cpu.h              |  1 +
>  target/s390x/cpu_features.c     |  1 +
>  target/s390x/cpu_features_def.h |  1 +
>  target/s390x/gen-features.c     |  1 +
>  target/s390x/kvm.c              | 16 ++++++++++++++++
>  target/s390x/machine.c          | 17 +++++++++++++++++
>  7 files changed, 38 insertions(+)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index ae3cee9..1577b2c 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -89,6 +89,7 @@ static void s390_cpu_reset(CPUState *s)
>      CPUS390XState *env = &cpu->env;
>  
>      env->pfault_token = -1UL;
> +    env->bpbc = 0;
>      scc->parent_reset(s);
>      cpu->env.sigp_order = 0;
>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 1a8b6b9..8514905 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -93,6 +93,7 @@ struct CPUS390XState {
>  
>      uint32_t fpc;          /* floating-point control register */
>      uint32_t cc_op;
> +    uint8_t bpbc;          /* branch prediction blocking */
>  
>      float_status fpu_status; /* passed to softfloat lib */
>  
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 31a4676..5d1c210 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -89,6 +89,7 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
> +    FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),

No nice "facility" suffix? :)

>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 4b6d4e9..4487cfd 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -80,6 +80,7 @@ typedef enum {
>      S390_FEAT_MSA_EXT_4,
>      S390_FEAT_EDAT_2,
>      S390_FEAT_DFP_PACKED_CONVERSION,
> +    S390_FEAT_BPB,
>      S390_FEAT_VECTOR,
>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index b24f6ad..95ee870 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -357,6 +357,7 @@ static uint16_t full_GEN7_GA1[] = {
>      S390_FEAT_SIE_GPERE,
>      S390_FEAT_SIE_IB,
>      S390_FEAT_SIE_CEI,
> +    S390_FEAT_BPB,

Will this be provided that far back on real hardware?

>  };
>  
>  static uint16_t full_GEN7_GA2[] = {
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 6a18a41..3cd4fab 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -490,6 +490,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
> +       cs->kvm_run->s.regs.bpbc = env->bpbc;
> +       cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_BPBC;
> +    }
> +
>      /* Finally the prefix */
>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>          cs->kvm_run->s.regs.prefix = env->psa;
> @@ -600,6 +605,10 @@ int kvm_arch_get_registers(CPUState *cs)
>          memcpy(env->gscb, cs->kvm_run->s.regs.gscb, 32);
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
> +       env->bpbc = cs->kvm_run->s.regs.bpbc;
> +    }
> +
>      /* pfault parameters */
>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>          env->pfault_token = cs->kvm_run->s.regs.pft;
> @@ -2278,6 +2287,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>          clear_bit(S390_FEAT_CMM_NT, model->features);
>      }
>  
> +    /* stfle.82 is a transparent bit. As there is some state attached
> +     * anyway we only enable this bit if the host kernel can handle
> +     * migrate and reset */

Having a transparent bit with some state attached seems a bit odd...

> +    if (!kvm_check_extension(kvm_state, KVM_CAP_S390_BPB)) {
> +        clear_bit(S390_FEAT_BPB, model->features);
> +    }
> +
>      /* We emulate a zPCI bus and AEN, therefore we don't need HW support */
>      if (pci_available) {
>          set_bit(S390_FEAT_ZPCI, model->features);

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

On 17.01.2018 15:37, Cornelia Huck wrote:
> On Wed, 17 Jan 2018 15:18:48 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> We need to handle the bpb control on reset and migration. Normally
>> stfle.82 is transparent (and the normal guest part works without
>> hypervisor activity). To prevent any issues we require full
>> host kernel support for this feature.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  target/s390x/cpu.c              |  1 +
>>  target/s390x/cpu.h              |  1 +
>>  target/s390x/cpu_features.c     |  1 +
>>  target/s390x/cpu_features_def.h |  1 +
>>  target/s390x/gen-features.c     |  1 +
>>  target/s390x/kvm.c              | 16 ++++++++++++++++
>>  target/s390x/machine.c          | 17 +++++++++++++++++
>>  7 files changed, 38 insertions(+)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index ae3cee9..1577b2c 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -89,6 +89,7 @@ static void s390_cpu_reset(CPUState *s)
>>      CPUS390XState *env = &cpu->env;
>>  
>>      env->pfault_token = -1UL;
>> +    env->bpbc = 0;
>>      scc->parent_reset(s);
>>      cpu->env.sigp_order = 0;
>>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 1a8b6b9..8514905 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -93,6 +93,7 @@ struct CPUS390XState {
>>  
>>      uint32_t fpc;          /* floating-point control register */
>>      uint32_t cc_op;
>> +    uint8_t bpbc;          /* branch prediction blocking */
>>  
>>      float_status fpu_status; /* passed to softfloat lib */
>>  
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 31a4676..5d1c210 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -89,6 +89,7 @@ static const S390FeatDef s390_features[] = {
>>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
>> +    FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
> 
> No nice "facility" suffix? :)
> 
>>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 4b6d4e9..4487cfd 100644
>> --- a/target/s390x/cpu_features_def.h
>> +++ b/target/s390x/cpu_features_def.h
>> @@ -80,6 +80,7 @@ typedef enum {
>>      S390_FEAT_MSA_EXT_4,
>>      S390_FEAT_EDAT_2,
>>      S390_FEAT_DFP_PACKED_CONVERSION,
>> +    S390_FEAT_BPB,
>>      S390_FEAT_VECTOR,
>>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index b24f6ad..95ee870 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -357,6 +357,7 @@ static uint16_t full_GEN7_GA1[] = {
>>      S390_FEAT_SIE_GPERE,
>>      S390_FEAT_SIE_IB,
>>      S390_FEAT_SIE_CEI,
>> +    S390_FEAT_BPB,
> 
> Will this be provided that far back on real hardware?
> 
>>  };
>>  
>>  static uint16_t full_GEN7_GA2[] = {
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index 6a18a41..3cd4fab 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -490,6 +490,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
>>      }
>>  
>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>> +       cs->kvm_run->s.regs.bpbc = env->bpbc;
>> +       cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_BPBC;
>> +    }
>> +
>>      /* Finally the prefix */
>>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>>          cs->kvm_run->s.regs.prefix = env->psa;
>> @@ -600,6 +605,10 @@ int kvm_arch_get_registers(CPUState *cs)
>>          memcpy(env->gscb, cs->kvm_run->s.regs.gscb, 32);
>>      }
>>  
>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>> +       env->bpbc = cs->kvm_run->s.regs.bpbc;
>> +    }
>> +
>>      /* pfault parameters */
>>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>>          env->pfault_token = cs->kvm_run->s.regs.pft;
>> @@ -2278,6 +2287,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>>          clear_bit(S390_FEAT_CMM_NT, model->features);
>>      }
>>  
>> +    /* stfle.82 is a transparent bit. As there is some state attached
>> +     * anyway we only enable this bit if the host kernel can handle
>> +     * migrate and reset */
> 
> Having a transparent bit with some state attached seems a bit odd...
> 

And exactly for this reason I tend to nack patch nr 3 (if that is of any
weight :) ).

As soon as we enable bits for CPU models, we guarantee that migration
works. While introducing this change we already had one example where
this was not the case. Not good. (and remember having another such
exception)

It is easier to patch a feature in than silently enabling *anything*
somebody thinks is transparent (but its not). Especially not for the
host model. The expanded host model is migration safe.

And as we saw, in the unlikely event of such heavy changes, we need to
touch fw/linux/qemu either way.

But there is more I dislike about the approach in patch 3:

1. feature names. We need aliases. Different QEMU versions on the same
hw might end up not understanding what a feature means. (old one: only
knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)

2. CPU model compatibility checks. We suddenly support _in this QEMU
version_ CPU features for CPU models that were never defined.

E.g. somewhen in the future I could say -z14,stfl_123=on

just because it is a transparent feature but maybe completely fanced by
ibc. Not good.

I can understand that we need something like that for development. I
propose something like a special cpu model for that (similar to
host-passthrough).

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Christian Borntraeger]

On 01/17/2018 03:50 PM, David Hildenbrand wrote:
> On 17.01.2018 15:37, Cornelia Huck wrote:
>> On Wed, 17 Jan 2018 15:18:48 +0100
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> We need to handle the bpb control on reset and migration. Normally
>>> stfle.82 is transparent (and the normal guest part works without
>>> hypervisor activity). To prevent any issues we require full
>>> host kernel support for this feature.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  target/s390x/cpu.c              |  1 +
>>>  target/s390x/cpu.h              |  1 +
>>>  target/s390x/cpu_features.c     |  1 +
>>>  target/s390x/cpu_features_def.h |  1 +
>>>  target/s390x/gen-features.c     |  1 +
>>>  target/s390x/kvm.c              | 16 ++++++++++++++++
>>>  target/s390x/machine.c          | 17 +++++++++++++++++
>>>  7 files changed, 38 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>>> index ae3cee9..1577b2c 100644
>>> --- a/target/s390x/cpu.c
>>> +++ b/target/s390x/cpu.c
>>> @@ -89,6 +89,7 @@ static void s390_cpu_reset(CPUState *s)
>>>      CPUS390XState *env = &cpu->env;
>>>  
>>>      env->pfault_token = -1UL;
>>> +    env->bpbc = 0;
>>>      scc->parent_reset(s);
>>>      cpu->env.sigp_order = 0;
>>>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>>> index 1a8b6b9..8514905 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -93,6 +93,7 @@ struct CPUS390XState {
>>>  
>>>      uint32_t fpc;          /* floating-point control register */
>>>      uint32_t cc_op;
>>> +    uint8_t bpbc;          /* branch prediction blocking */
>>>  
>>>      float_status fpu_status; /* passed to softfloat lib */
>>>  
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 31a4676..5d1c210 100644
>>> --- a/target/s390x/cpu_features.c
>>> +++ b/target/s390x/cpu_features.c
>>> @@ -89,6 +89,7 @@ static const S390FeatDef s390_features[] = {
>>>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>>>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>>>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
>>> +    FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
>>
>> No nice "facility" suffix? :)
>>
>>>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>>>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>>>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
>>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>>> index 4b6d4e9..4487cfd 100644
>>> --- a/target/s390x/cpu_features_def.h
>>> +++ b/target/s390x/cpu_features_def.h
>>> @@ -80,6 +80,7 @@ typedef enum {
>>>      S390_FEAT_MSA_EXT_4,
>>>      S390_FEAT_EDAT_2,
>>>      S390_FEAT_DFP_PACKED_CONVERSION,
>>> +    S390_FEAT_BPB,
>>>      S390_FEAT_VECTOR,
>>>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>> index b24f6ad..95ee870 100644
>>> --- a/target/s390x/gen-features.c
>>> +++ b/target/s390x/gen-features.c
>>> @@ -357,6 +357,7 @@ static uint16_t full_GEN7_GA1[] = {
>>>      S390_FEAT_SIE_GPERE,
>>>      S390_FEAT_SIE_IB,
>>>      S390_FEAT_SIE_CEI,
>>> +    S390_FEAT_BPB,
>>
>> Will this be provided that far back on real hardware?
>>
>>>  };
>>>  
>>>  static uint16_t full_GEN7_GA2[] = {
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index 6a18a41..3cd4fab 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -490,6 +490,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
>>>      }
>>>  
>>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>>> +       cs->kvm_run->s.regs.bpbc = env->bpbc;
>>> +       cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_BPBC;
>>> +    }
>>> +
>>>      /* Finally the prefix */
>>>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>>>          cs->kvm_run->s.regs.prefix = env->psa;
>>> @@ -600,6 +605,10 @@ int kvm_arch_get_registers(CPUState *cs)
>>>          memcpy(env->gscb, cs->kvm_run->s.regs.gscb, 32);
>>>      }
>>>  
>>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>>> +       env->bpbc = cs->kvm_run->s.regs.bpbc;
>>> +    }
>>> +
>>>      /* pfault parameters */
>>>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>>>          env->pfault_token = cs->kvm_run->s.regs.pft;
>>> @@ -2278,6 +2287,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>>>          clear_bit(S390_FEAT_CMM_NT, model->features);
>>>      }
>>>  
>>> +    /* stfle.82 is a transparent bit. As there is some state attached
>>> +     * anyway we only enable this bit if the host kernel can handle
>>> +     * migrate and reset */
>>
>> Having a transparent bit with some state attached seems a bit odd...
>>
> 
> And exactly for this reason I tend to nack patch nr 3 (if that is of any
> weight :) ).

I have communicated the mistake to asll relevant parties - it will not happen again
(famous last words).

> 
> As soon as we enable bits for CPU models, we guarantee that migration
> works. While introducing this change we already had one example where
> this was not the case. Not good. (and remember having another such
> exception)

The point is migration continues to work. In fact I had a different version
of this patch set that did it the other way around. Keep 82 a transparent
and add a new cpu misc facility that takes care of the migration state.
> 
> It is easier to patch a feature in than silently enabling *anything*
> somebody thinks is transparent (but its not). Especially not for the
> host model. The expanded host model is migration safe.

I really do not care about -cpu host (host-passthrough) for migration safety, 
because its not. And as you said: host-model (expanded) will work.

> 
> And as we saw, in the unlikely event of such heavy changes, we need to
> touch fw/linux/qemu either way.
> 
> But there is more I dislike about the approach in patch 3:
> 
> 1. feature names. We need aliases. Different QEMU versions on the same
> hw might end up not understanding what a feature means. (old one: only
> knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)

I plan to keep the old names. e.g. stfle131 is better than sea_esop2.

> 
> 2. CPU model compatibility checks. We suddenly support _in this QEMU
> version_ CPU features for CPU models that were never defined.

we defined it. It just have a canonical name stfle*
> 
> E.g. somewhen in the future I could say -z14,stfl_123=on
> 
> just because it is a transparent feature but maybe completely fanced by
> ibc. Not good.
> 
> I can understand that we need something like that for development. I
> propose something like a special cpu model for that (similar to
> host-passthrough).
> 

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

>> And exactly for this reason I tend to nack patch nr 3 (if that is of any
>> weight :) ).
> 
> I have communicated the mistake to asll relevant parties - it will not happen again
> (famous last words).

An I already saw it happen in the past. (I think I really have to dig
out that one feature to make a point :P ). Mistakes happen, but we don't
have to propagate them to customers if we can catch them early :)

> 
>>
>> As soon as we enable bits for CPU models, we guarantee that migration
>> works. While introducing this change we already had one example where
>> this was not the case. Not good. (and remember having another such
>> exception)
> 
> The point is migration continues to work. In fact I had a different version
> of this patch set that did it the other way around. Keep 82 a transparent
> and add a new cpu misc facility that takes care of the migration state.
>>
>> It is easier to patch a feature in than silently enabling *anything*
>> somebody thinks is transparent (but its not). Especially not for the
>> host model. The expanded host model is migration safe.
> 
> I really do not care about -cpu host (host-passthrough) for migration safety, 
> because its not. And as you said: host-model (expanded) will work.
> 

It will if the world would be perfect.

expand "-cpu host" -> -cpu z14-base,stfle_82=on

stfle_82 would now not be properly migrated. Yes, it might work somehow
right now. But it is not clean.

>>
>> And as we saw, in the unlikely event of such heavy changes, we need to
>> touch fw/linux/qemu either way.
>>
>> But there is more I dislike about the approach in patch 3:
>>
>> 1. feature names. We need aliases. Different QEMU versions on the same
>> hw might end up not understanding what a feature means. (old one: only
>> knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)
> 
> I plan to keep the old names. e.g. stfle131 is better than sea_esop2.


Oh god no. With vx, te, iep one at least has a rough idea what is happening.

-cpu z14-base,stfle123,stfle234,stfle323 ... :(


This all smells like a huge hack for a scenario that happened once. I
prefer to do it the clean way. Enable only what you checked works and
what you can actually give a name.

Especially we will lose the ability to know which bit was valid for
which hardware generation - which is key when working with IBC.

I am not sure if giving all that up is worth it.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Christian Borntraeger]

On 01/17/2018 04:10 PM, David Hildenbrand wrote:
> 
>>> And exactly for this reason I tend to nack patch nr 3 (if that is of any
>>> weight :) ).
>>
>> I have communicated the mistake to asll relevant parties - it will not happen again
>> (famous last words).
> 
> An I already saw it happen in the past. (I think I really have to dig
> out that one feature to make a point :P ). Mistakes happen, but we don't
> have to propagate them to customers if we can catch them early :)
> 
>>
>>>
>>> As soon as we enable bits for CPU models, we guarantee that migration
>>> works. While introducing this change we already had one example where
>>> this was not the case. Not good. (and remember having another such
>>> exception)
>>
>> The point is migration continues to work. In fact I had a different version
>> of this patch set that did it the other way around. Keep 82 a transparent
>> and add a new cpu misc facility that takes care of the migration state.
>>>
>>> It is easier to patch a feature in than silently enabling *anything*
>>> somebody thinks is transparent (but its not). Especially not for the
>>> host model. The expanded host model is migration safe.
>>
>> I really do not care about -cpu host (host-passthrough) for migration safety, 
>> because its not. And as you said: host-model (expanded) will work.
>>
> 
> It will if the world would be perfect.
> 
> expand "-cpu host" -> -cpu z14-base,stfle_82=on
> 
> stfle_82 would now not be properly migrated. Yes, it might work somehow
> right now. But it is not clean.
> 
>>>
>>> And as we saw, in the unlikely event of such heavy changes, we need to
>>> touch fw/linux/qemu either way.
>>>
>>> But there is more I dislike about the approach in patch 3:
>>>
>>> 1. feature names. We need aliases. Different QEMU versions on the same
>>> hw might end up not understanding what a feature means. (old one: only
>>> knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)
>>
>> I plan to keep the old names. e.g. stfle131 is better than sea_esop2.
> 
> 
> Oh god no. With vx, te, iep one at least has a rough idea what is happening.
> 
> -cpu z14-base,stfle123,stfle234,stfle323 ... :(
> 
> 
> This all smells like a huge hack for a scenario that happened once. I
> prefer to do it the clean way. Enable only what you checked works and
> what you can actually give a name.
> 
> Especially we will lose the ability to know which bit was valid for
> which hardware generation - which is key when working with IBC.
> 
> I am not sure if giving all that up is worth it.
> 

I will spin up a second patch that enables stfle81 and name it "ppa15".
We can then discuss patch 3 on the slow path with enough time to think
about this.

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

On 17.01.2018 17:04, Christian Borntraeger wrote:
> 
> 
> On 01/17/2018 04:10 PM, David Hildenbrand wrote:
>>
>>>> And exactly for this reason I tend to nack patch nr 3 (if that is of any
>>>> weight :) ).
>>>
>>> I have communicated the mistake to asll relevant parties - it will not happen again
>>> (famous last words).
>>
>> An I already saw it happen in the past. (I think I really have to dig
>> out that one feature to make a point :P ). Mistakes happen, but we don't
>> have to propagate them to customers if we can catch them early :)
>>
>>>
>>>>
>>>> As soon as we enable bits for CPU models, we guarantee that migration
>>>> works. While introducing this change we already had one example where
>>>> this was not the case. Not good. (and remember having another such
>>>> exception)
>>>
>>> The point is migration continues to work. In fact I had a different version
>>> of this patch set that did it the other way around. Keep 82 a transparent
>>> and add a new cpu misc facility that takes care of the migration state.
>>>>
>>>> It is easier to patch a feature in than silently enabling *anything*
>>>> somebody thinks is transparent (but its not). Especially not for the
>>>> host model. The expanded host model is migration safe.
>>>
>>> I really do not care about -cpu host (host-passthrough) for migration safety, 
>>> because its not. And as you said: host-model (expanded) will work.
>>>
>>
>> It will if the world would be perfect.
>>
>> expand "-cpu host" -> -cpu z14-base,stfle_82=on
>>
>> stfle_82 would now not be properly migrated. Yes, it might work somehow
>> right now. But it is not clean.
>>
>>>>
>>>> And as we saw, in the unlikely event of such heavy changes, we need to
>>>> touch fw/linux/qemu either way.
>>>>
>>>> But there is more I dislike about the approach in patch 3:
>>>>
>>>> 1. feature names. We need aliases. Different QEMU versions on the same
>>>> hw might end up not understanding what a feature means. (old one: only
>>>> knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)
>>>
>>> I plan to keep the old names. e.g. stfle131 is better than sea_esop2.
>>
>>
>> Oh god no. With vx, te, iep one at least has a rough idea what is happening.
>>
>> -cpu z14-base,stfle123,stfle234,stfle323 ... :(
>>
>>
>> This all smells like a huge hack for a scenario that happened once. I
>> prefer to do it the clean way. Enable only what you checked works and
>> what you can actually give a name.
>>
>> Especially we will lose the ability to know which bit was valid for
>> which hardware generation - which is key when working with IBC.
>>
>> I am not sure if giving all that up is worth it.
>>
> 
> I will spin up a second patch that enables stfle81 and name it "ppa15".
> We can then discuss patch 3 on the slow path with enough time to think
> about this.
> 

christian++ :)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Cornelia Huck]

On Wed, 17 Jan 2018 17:04:05 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> I will spin up a second patch that enables stfle81 and name it "ppa15".
> We can then discuss patch 3 on the slow path with enough time to think
> about this.

Sounds good to me. I really don't want to rush this, but still get the
81/82 bits to guests.

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Halil Pasic]

On 01/17/2018 04:10 PM, David Hildenbrand wrote:
>>> As soon as we enable bits for CPU models, we guarantee that migration
>>> works. While introducing this change we already had one example where
>>> this was not the case. Not good. (and remember having another such
>>> exception)
>> The point is migration continues to work. In fact I had a different version
>> of this patch set that did it the other way around. Keep 82 a transparent
>> and add a new cpu misc facility that takes care of the migration state.
>>> It is easier to patch a feature in than silently enabling *anything*
>>> somebody thinks is transparent (but its not). Especially not for the
>>> host model. The expanded host model is migration safe.
>> I really do not care about -cpu host (host-passthrough) for migration safety, 
>> because its not. And as you said: host-model (expanded) will work.
>>
> It will if the world would be perfect.
> 
> expand "-cpu host" -> -cpu z14-base,stfle_82=on
> 
> stfle_82 would now not be properly migrated. Yes, it might work somehow
> right now. But it is not clean.
> 

Based on the code (did not test) I would expect to see this in the
scenario I think you are talking about David:

0. Libvirt uses cpu mode host (not host-passtrough).
1. Source: -"cpu host" expands to "-cpu z14-base,stfle_82=on" as you said
the whole environment (host cpu, kvm, qemu) is supporting 82 inclusive migration
2. Target: libvirt requires "-cpu z14-base,stfle_82=on" when trying to migrate
2.1. If the target environment as a whole does not fully support 82 the model
checking refuses the migration. It does not matter if the reason is QEMU missing
patch #2 or lack of HW support or whatever.
2.2 If the target environment as a whole does fully support 82 the model migration
works, and is safe. If the target QEMU does not have patch #2 we will end up here.
But we may end up here for other reasons.

Both outcomes are works as designed, as far as my understanding of the design goes.
Or am I wrong?

Assumed my reasoning above is correct, I really don't get what is not clean.
Except 'blindly' trusting the hardware that it works as advertised (and
fixing the mess only if it turns out that there is a mess).

On a meta level I think I understand your concerns David to some extent.
But for my taste here you are too concerned about the user shooting herself
into the foot (because equipment malfunction or user error).

Let me also note, that while we might very well end up propagating bugs
to the user, we also give the user the means to mitigate these (e.g.
by turning the buggy features explicitly off like -cpu host,stfle_82=off).

Regards,
Halil

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

On 17.01.2018 17:07, Halil Pasic wrote:
> 
> 
> On 01/17/2018 04:10 PM, David Hildenbrand wrote:
>>>> As soon as we enable bits for CPU models, we guarantee that migration
>>>> works. While introducing this change we already had one example where
>>>> this was not the case. Not good. (and remember having another such
>>>> exception)
>>> The point is migration continues to work. In fact I had a different version
>>> of this patch set that did it the other way around. Keep 82 a transparent
>>> and add a new cpu misc facility that takes care of the migration state.
>>>> It is easier to patch a feature in than silently enabling *anything*
>>>> somebody thinks is transparent (but its not). Especially not for the
>>>> host model. The expanded host model is migration safe.
>>> I really do not care about -cpu host (host-passthrough) for migration safety, 
>>> because its not. And as you said: host-model (expanded) will work.
>>>
>> It will if the world would be perfect.
>>
>> expand "-cpu host" -> -cpu z14-base,stfle_82=on
>>
>> stfle_82 would now not be properly migrated. Yes, it might work somehow
>> right now. But it is not clean.
>>
> 

Please don't only focus on this case. I had different reasons why this
is a bad idea (especially IBC and feature names). And this is still my
position.

> Based on the code (did not test) I would expect to see this in the
> scenario I think you are talking about David:
> 
> 0. Libvirt uses cpu mode host (not host-passtrough).
> 1. Source: -"cpu host" expands to "-cpu z14-base,stfle_82=on" as you said
> the whole environment (host cpu, kvm, qemu) is supporting 82 inclusive migration
> 2. Target: libvirt requires "-cpu z14-base,stfle_82=on" when trying to migrate
> 2.1. If the target environment as a whole does not fully support 82 the model
> checking refuses the migration. It does not matter if the reason is QEMU missing
> patch #2 or lack of HW support or whatever.

Let's assume stfle_999, because this is obviously not a problem for 82.

If source and target blindly forward a feature they think is migration
safe, but is not (again, I am being conservative here as I was given a
counter example in the very same patch set), migration does not fail but
the guest might see a difference, because some state is not properly
migrated.

> 
> Assumed my reasoning above is correct, I really don't get what is not clean.
> Except 'blindly' trusting the hardware that it works as advertised (and
> fixing the mess only if it turns out that there is a mess).
> 
> On a meta level I think I understand your concerns David to some extent.
> But for my taste here you are too concerned about the user shooting herself
> into the foot (because equipment malfunction or user error).

Using the host model in QEMU shoots you in the foot. And it shouldn't.
Better safe than sorry.

> 
> Let me also note, that while we might very well end up propagating bugs
> to the user, we also give the user the means to mitigate these (e.g.
> by turning the buggy features explicitly off like -cpu host,stfle_82=off).

Shoot first and then ask questions? :D

I don't see any valid reason for this risk. Forwarding "transparent"
features from KVM to QEMU - perfect idea. Forwarding "transparent"
features from QEMU to the guest - not a good idea.

New HW -> new features -> new QEMU CPU model.

Patching in CPU features is really "out of order" ( ;) )

> 
> Regards,
> Halil
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] s390x/kvm: Handle bpb feature

[Christian Borntraeger]

On 01/17/2018 03:37 PM, Cornelia Huck wrote:
> On Wed, 17 Jan 2018 15:18:48 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> We need to handle the bpb control on reset and migration. Normally
>> stfle.82 is transparent (and the normal guest part works without
>> hypervisor activity). To prevent any issues we require full
>> host kernel support for this feature.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  target/s390x/cpu.c              |  1 +
>>  target/s390x/cpu.h              |  1 +
>>  target/s390x/cpu_features.c     |  1 +
>>  target/s390x/cpu_features_def.h |  1 +
>>  target/s390x/gen-features.c     |  1 +
>>  target/s390x/kvm.c              | 16 ++++++++++++++++
>>  target/s390x/machine.c          | 17 +++++++++++++++++
>>  7 files changed, 38 insertions(+)
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index ae3cee9..1577b2c 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -89,6 +89,7 @@ static void s390_cpu_reset(CPUState *s)
>>      CPUS390XState *env = &cpu->env;
>>  
>>      env->pfault_token = -1UL;
>> +    env->bpbc = 0;
>>      scc->parent_reset(s);
>>      cpu->env.sigp_order = 0;
>>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 1a8b6b9..8514905 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -93,6 +93,7 @@ struct CPUS390XState {
>>  
>>      uint32_t fpc;          /* floating-point control register */
>>      uint32_t cc_op;
>> +    uint8_t bpbc;          /* branch prediction blocking */
>>  
>>      float_status fpu_status; /* passed to softfloat lib */
>>  
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 31a4676..5d1c210 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -89,6 +89,7 @@ static const S390FeatDef s390_features[] = {
>>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
>> +    FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
> 
> No nice "facility" suffix? :)

assist I think.

> 
>>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 4b6d4e9..4487cfd 100644
>> --- a/target/s390x/cpu_features_def.h
>> +++ b/target/s390x/cpu_features_def.h
>> @@ -80,6 +80,7 @@ typedef enum {
>>      S390_FEAT_MSA_EXT_4,
>>      S390_FEAT_EDAT_2,
>>      S390_FEAT_DFP_PACKED_CONVERSION,
>> +    S390_FEAT_BPB,
>>      S390_FEAT_VECTOR,
>>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index b24f6ad..95ee870 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -357,6 +357,7 @@ static uint16_t full_GEN7_GA1[] = {
>>      S390_FEAT_SIE_GPERE,
>>      S390_FEAT_SIE_IB,
>>      S390_FEAT_SIE_CEI,
>> +    S390_FEAT_BPB,
> 
> Will this be provided that far back on real hardware?

I theory this could be backported to every machine (but its pretty unlikely).
Since its just in the full model (and no default model) and kvm will only provide
it if the host has it, this is the most flexible approach if this is backported
further than we expected.

> 
>>  };
>>  
>>  static uint16_t full_GEN7_GA2[] = {
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index 6a18a41..3cd4fab 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -490,6 +490,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
>>      }
>>  
>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>> +       cs->kvm_run->s.regs.bpbc = env->bpbc;
>> +       cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_BPBC;
>> +    }
>> +
>>      /* Finally the prefix */
>>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>>          cs->kvm_run->s.regs.prefix = env->psa;
>> @@ -600,6 +605,10 @@ int kvm_arch_get_registers(CPUState *cs)
>>          memcpy(env->gscb, cs->kvm_run->s.regs.gscb, 32);
>>      }
>>  
>> +    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
>> +       env->bpbc = cs->kvm_run->s.regs.bpbc;
>> +    }
>> +
>>      /* pfault parameters */
>>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>>          env->pfault_token = cs->kvm_run->s.regs.pft;
>> @@ -2278,6 +2287,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>>          clear_bit(S390_FEAT_CMM_NT, model->features);
>>      }
>>  
>> +    /* stfle.82 is a transparent bit. As there is some state attached
>> +     * anyway we only enable this bit if the host kernel can handle
>> +     * migrate and reset */
> 
> Having a transparent bit with some state attached seems a bit odd...

Yes, its a quirk,but the firmware fixes are already out.

[Qemu-devel] [PATCH 3/3] s390x/cpumodel: fix transparency for non-hyp STFL features

[Christian Borntraeger]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Before cpu-models were introduced to QEMU with 2.8 the so called
non-hypervisor-managed STFL facilities (aka transparent facilities) were
handled transparently.

With the advent cpu models, for host model (means -cpu host), we started
fencing these of using the identified full model. The full models however
did not include all non-hypervisor-managed facilities, thus we end up
fencing some of these off.

New (non-hypervisor managed) facilities can be introduced with hardware
or firmware upgrades. Requiring a code change and thus a QEMU upgrade to
leverage such facilities is not acceptable. Namely the semantic of the host
model is 'give me all you can'.

Let us add non-hypervisor managed STFL facility bits to QEMU and to all
the full models. For now first two doublewords should be sufficient.

With this, when using host model, transparent facilities are presented to
the guest. Regarding default (and base)  models however, nothing really
changes, except that user can specify any non-hypervisor facility now.

Thus the so called transparent facilities, aren't handled transparently
with the default nor with the base models (because of migration
considerations).  For example -cpu z13 will not enable (and mandate) any
of the features added with this change for any compat machine types.

Emerging non-hypervisor managed facilities that are expected to be
present in any sane environment (in the context of the machine type)
should be added to the default model (for non-compat machine types).

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target/s390x/cpu_features.c     |  54 +++++++++++++++++++++
 target/s390x/cpu_features_def.h |  54 +++++++++++++++++++++
 target/s390x/gen-features.c     | 103 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 211 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 5d1c210..407864a 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -51,6 +51,7 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("parseh", S390_FEAT_TYPE_STFL, 26, "Parsing-enhancement facility"),
     FEAT_INIT("mvcos", S390_FEAT_TYPE_STFL, 27, "Move-with-optional-specification facility"),
     FEAT_INIT("tods-base", S390_FEAT_TYPE_STFL, 28, "TOD-clock-steering facility (excluding subfunctions)"),
+    FEAT_INIT("stfle29", S390_FEAT_TYPE_STFL, 29, "Facility that is provided by STFLE facility 29"),
     FEAT_INIT("etf3eh", S390_FEAT_TYPE_STFL, 30, "ETF3-enhancement facility"),
     FEAT_INIT("ectg", S390_FEAT_TYPE_STFL, 31, "Extract-CPU-time facility"),
     FEAT_INIT("csst", S390_FEAT_TYPE_STFL, 32, "Compare-and-swap-and-store facility"),
@@ -60,12 +61,14 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("emon", S390_FEAT_TYPE_STFL, 36, "Enhanced-monitor facility"),
     FEAT_INIT("fpe", S390_FEAT_TYPE_STFL, 37, "Floating-point extension facility"),
     FEAT_INIT("opc", S390_FEAT_TYPE_STFL, 38, "Order Preserving Compression facility"),
+    FEAT_INIT("stfle39", S390_FEAT_TYPE_STFL, 39, "Facility that is provided by STFLE facility 39"),
     FEAT_INIT("sprogp", S390_FEAT_TYPE_STFL, 40, "Set-program-parameters facility"),
     FEAT_INIT("fpseh", S390_FEAT_TYPE_STFL, 41, "Floating-point-support-enhancement facilities"),
     FEAT_INIT("dfp", S390_FEAT_TYPE_STFL, 42, "DFP (decimal-floating-point) facility"),
     FEAT_INIT("dfphp", S390_FEAT_TYPE_STFL, 43, "DFP (decimal-floating-point) facility has high performance"),
     FEAT_INIT("pfpo", S390_FEAT_TYPE_STFL, 44, "PFPO instruction"),
     FEAT_INIT("stfle45", S390_FEAT_TYPE_STFL, 45, "Various facilities introduced with z196"),
+    FEAT_INIT("stfle46", S390_FEAT_TYPE_STFL, 46, "Facility that is provided by STFLE facility 46"),
     FEAT_INIT("cmpsceh", S390_FEAT_TYPE_STFL, 47, "CMPSC-enhancement facility"),
     FEAT_INIT("dfpzc", S390_FEAT_TYPE_STFL, 48, "Decimal-floating-point zoned-conversion facility"),
     FEAT_INIT("stfle49", S390_FEAT_TYPE_STFL, 49, "Various facilities introduced with zEC12"),
@@ -74,10 +77,15 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("iacc2", S390_FEAT_TYPE_STFL, 52, "Interlocked-access facility 2"),
     FEAT_INIT("stfle53", S390_FEAT_TYPE_STFL, 53, "Various facilities introduced with z13"),
     FEAT_INIT("eec", S390_FEAT_TYPE_STFL, 54, "Entropy encoding compression facility"),
+    FEAT_INIT("stfle55", S390_FEAT_TYPE_STFL, 55, "Facility that is provided by STFLE facility 55"),
+    FEAT_INIT("stfle56", S390_FEAT_TYPE_STFL, 56, "Facility that is provided by STFLE facility 56"),
     FEAT_INIT("msa5-base", S390_FEAT_TYPE_STFL, 57, "Message-security-assist-extension-5 facility (excluding subfunctions)"),
     FEAT_INIT("minste2", S390_FEAT_TYPE_STFL, 58, "Miscellaneous-instruction-extensions facility 2"),
     FEAT_INIT("sema", S390_FEAT_TYPE_STFL, 59, "Semaphore-assist facility"),
     FEAT_INIT("tsi", S390_FEAT_TYPE_STFL, 60, "Time-slice Instrumentation facility"),
+    FEAT_INIT("stfle61", S390_FEAT_TYPE_STFL, 61, "Facility that is provided by STFLE facility 61"),
+    FEAT_INIT("stfle62", S390_FEAT_TYPE_STFL, 62, "Facility that is provided by STFLE facility 62"),
+    FEAT_INIT("stfle63", S390_FEAT_TYPE_STFL, 63, "Facility that is provided by STFLE facility 63"),
     FEAT_INIT("ri", S390_FEAT_TYPE_STFL, 64, "CPU runtime-instrumentation facility"),
     FEAT_INIT("zpci", S390_FEAT_TYPE_STFL, 69, "z/PCI facility"),
     FEAT_INIT("aen", S390_FEAT_TYPE_STFL, 71, "General-purpose-adapter-event-notification facility"),
@@ -89,7 +97,53 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
     FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
     FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
+    FEAT_INIT("stfle81", S390_FEAT_TYPE_STFL, 81, "Facility that is provided by STFLE facility 81"),
     FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
+    FEAT_INIT("stfle83", S390_FEAT_TYPE_STFL, 83, "Facility that is provided by STFLE facility 83"),
+    FEAT_INIT("stfle84", S390_FEAT_TYPE_STFL, 84, "Facility that is provided by STFLE facility 84"),
+    FEAT_INIT("stfle85", S390_FEAT_TYPE_STFL, 85, "Facility that is provided by STFLE facility 85"),
+    FEAT_INIT("stfle86", S390_FEAT_TYPE_STFL, 86, "Facility that is provided by STFLE facility 86"),
+    FEAT_INIT("stfle87", S390_FEAT_TYPE_STFL, 87, "Facility that is provided by STFLE facility 87"),
+    FEAT_INIT("stfle88", S390_FEAT_TYPE_STFL, 88, "Facility that is provided by STFLE facility 88"),
+    FEAT_INIT("stfle89", S390_FEAT_TYPE_STFL, 89, "Facility that is provided by STFLE facility 89"),
+    FEAT_INIT("stfle90", S390_FEAT_TYPE_STFL, 90, "Facility that is provided by STFLE facility 90"),
+    FEAT_INIT("stfle91", S390_FEAT_TYPE_STFL, 91, "Facility that is provided by STFLE facility 91"),
+    FEAT_INIT("stfle92", S390_FEAT_TYPE_STFL, 92, "Facility that is provided by STFLE facility 92"),
+    FEAT_INIT("stfle93", S390_FEAT_TYPE_STFL, 93, "Facility that is provided by STFLE facility 93"),
+    FEAT_INIT("stfle94", S390_FEAT_TYPE_STFL, 94, "Facility that is provided by STFLE facility 94"),
+    FEAT_INIT("stfle95", S390_FEAT_TYPE_STFL, 95, "Facility that is provided by STFLE facility 95"),
+    FEAT_INIT("stfle96", S390_FEAT_TYPE_STFL, 96, "Facility that is provided by STFLE facility 96"),
+    FEAT_INIT("stfle97", S390_FEAT_TYPE_STFL, 97, "Facility that is provided by STFLE facility 97"),
+    FEAT_INIT("stfle98", S390_FEAT_TYPE_STFL, 98, "Facility that is provided by STFLE facility 98"),
+    FEAT_INIT("stfle99", S390_FEAT_TYPE_STFL, 99, "Facility that is provided by STFLE facility 99"),
+    FEAT_INIT("stfle100", S390_FEAT_TYPE_STFL, 100, "Facility that is provided by STFLE facility 100"),
+    FEAT_INIT("stfle101", S390_FEAT_TYPE_STFL, 101, "Facility that is provided by STFLE facility 101"),
+    FEAT_INIT("stfle102", S390_FEAT_TYPE_STFL, 102, "Facility that is provided by STFLE facility 102"),
+    FEAT_INIT("stfle103", S390_FEAT_TYPE_STFL, 103, "Facility that is provided by STFLE facility 103"),
+    FEAT_INIT("stfle104", S390_FEAT_TYPE_STFL, 104, "Facility that is provided by STFLE facility 104"),
+    FEAT_INIT("stfle105", S390_FEAT_TYPE_STFL, 105, "Facility that is provided by STFLE facility 105"),
+    FEAT_INIT("stfle106", S390_FEAT_TYPE_STFL, 106, "Facility that is provided by STFLE facility 106"),
+    FEAT_INIT("stfle107", S390_FEAT_TYPE_STFL, 107, "Facility that is provided by STFLE facility 107"),
+    FEAT_INIT("stfle108", S390_FEAT_TYPE_STFL, 108, "Facility that is provided by STFLE facility 108"),
+    FEAT_INIT("stfle109", S390_FEAT_TYPE_STFL, 109, "Facility that is provided by STFLE facility 109"),
+    FEAT_INIT("stfle110", S390_FEAT_TYPE_STFL, 110, "Facility that is provided by STFLE facility 110"),
+    FEAT_INIT("stfle111", S390_FEAT_TYPE_STFL, 111, "Facility that is provided by STFLE facility 111"),
+    FEAT_INIT("stfle112", S390_FEAT_TYPE_STFL, 112, "Facility that is provided by STFLE facility 112"),
+    FEAT_INIT("stfle113", S390_FEAT_TYPE_STFL, 113, "Facility that is provided by STFLE facility 113"),
+    FEAT_INIT("stfle114", S390_FEAT_TYPE_STFL, 114, "Facility that is provided by STFLE facility 114"),
+    FEAT_INIT("stfle115", S390_FEAT_TYPE_STFL, 115, "Facility that is provided by STFLE facility 115"),
+    FEAT_INIT("stfle116", S390_FEAT_TYPE_STFL, 116, "Facility that is provided by STFLE facility 116"),
+    FEAT_INIT("stfle117", S390_FEAT_TYPE_STFL, 117, "Facility that is provided by STFLE facility 117"),
+    FEAT_INIT("stfle118", S390_FEAT_TYPE_STFL, 118, "Facility that is provided by STFLE facility 118"),
+    FEAT_INIT("stfle119", S390_FEAT_TYPE_STFL, 119, "Facility that is provided by STFLE facility 119"),
+    FEAT_INIT("stfle120", S390_FEAT_TYPE_STFL, 120, "Facility that is provided by STFLE facility 120"),
+    FEAT_INIT("stfle121", S390_FEAT_TYPE_STFL, 121, "Facility that is provided by STFLE facility 121"),
+    FEAT_INIT("stfle122", S390_FEAT_TYPE_STFL, 122, "Facility that is provided by STFLE facility 122"),
+    FEAT_INIT("stfle123", S390_FEAT_TYPE_STFL, 123, "Facility that is provided by STFLE facility 123"),
+    FEAT_INIT("stfle124", S390_FEAT_TYPE_STFL, 124, "Facility that is provided by STFLE facility 124"),
+    FEAT_INIT("stfle125", S390_FEAT_TYPE_STFL, 125, "Facility that is provided by STFLE facility 125"),
+    FEAT_INIT("stfle126", S390_FEAT_TYPE_STFL, 126, "Facility that is provided by STFLE facility 126"),
+    FEAT_INIT("stfle127", S390_FEAT_TYPE_STFL, 127, "Facility that is provided by STFLE facility 127"),
     FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
     FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
     FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 4487cfd..fd15362 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -42,6 +42,7 @@ typedef enum {
     S390_FEAT_PARSING_ENH,
     S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
     S390_FEAT_TOD_CLOCK_STEERING,
+    S390_FEAT_STFLE_29,
     S390_FEAT_ETF3_ENH,
     S390_FEAT_EXTRACT_CPU_TIME,
     S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
@@ -51,12 +52,14 @@ typedef enum {
     S390_FEAT_ENHANCED_MONITOR,
     S390_FEAT_FLOATING_POINT_EXT,
     S390_FEAT_ORDER_PRESERVING_COMPRESSION,
+    S390_FEAT_STFLE_39,
     S390_FEAT_SET_PROGRAM_PARAMETERS,
     S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
     S390_FEAT_DFP,
     S390_FEAT_DFP_FAST,
     S390_FEAT_PFPO,
     S390_FEAT_STFLE_45,
+    S390_FEAT_STFLE_46,
     S390_FEAT_CMPSC_ENH,
     S390_FEAT_DFP_ZONED_CONVERSION,
     S390_FEAT_STFLE_49,
@@ -65,10 +68,15 @@ typedef enum {
     S390_FEAT_INTERLOCKED_ACCESS_2,
     S390_FEAT_STFLE_53,
     S390_FEAT_ENTROPY_ENC_COMP,
+    S390_FEAT_STFLE_55,
+    S390_FEAT_STFLE_56,
     S390_FEAT_MSA_EXT_5,
     S390_FEAT_MISC_INSTRUCTION_EXT,
     S390_FEAT_SEMAPHORE_ASSIST,
     S390_FEAT_TIME_SLICE_INSTRUMENTATION,
+    S390_FEAT_STFLE_61,
+    S390_FEAT_STFLE_62,
+    S390_FEAT_STFLE_63,
     S390_FEAT_RUNTIME_INSTRUMENTATION,
     S390_FEAT_ZPCI,
     S390_FEAT_ADAPTER_EVENT_NOTIFICATION,
@@ -80,7 +88,53 @@ typedef enum {
     S390_FEAT_MSA_EXT_4,
     S390_FEAT_EDAT_2,
     S390_FEAT_DFP_PACKED_CONVERSION,
+    S390_FEAT_STFLE_81,
     S390_FEAT_BPB,
+    S390_FEAT_STFLE_83,
+    S390_FEAT_STFLE_84,
+    S390_FEAT_STFLE_85,
+    S390_FEAT_STFLE_86,
+    S390_FEAT_STFLE_87,
+    S390_FEAT_STFLE_88,
+    S390_FEAT_STFLE_89,
+    S390_FEAT_STFLE_90,
+    S390_FEAT_STFLE_91,
+    S390_FEAT_STFLE_92,
+    S390_FEAT_STFLE_93,
+    S390_FEAT_STFLE_94,
+    S390_FEAT_STFLE_95,
+    S390_FEAT_STFLE_96,
+    S390_FEAT_STFLE_97,
+    S390_FEAT_STFLE_98,
+    S390_FEAT_STFLE_99,
+    S390_FEAT_STFLE_100,
+    S390_FEAT_STFLE_101,
+    S390_FEAT_STFLE_102,
+    S390_FEAT_STFLE_103,
+    S390_FEAT_STFLE_104,
+    S390_FEAT_STFLE_105,
+    S390_FEAT_STFLE_106,
+    S390_FEAT_STFLE_107,
+    S390_FEAT_STFLE_108,
+    S390_FEAT_STFLE_109,
+    S390_FEAT_STFLE_110,
+    S390_FEAT_STFLE_111,
+    S390_FEAT_STFLE_112,
+    S390_FEAT_STFLE_113,
+    S390_FEAT_STFLE_114,
+    S390_FEAT_STFLE_115,
+    S390_FEAT_STFLE_116,
+    S390_FEAT_STFLE_117,
+    S390_FEAT_STFLE_118,
+    S390_FEAT_STFLE_119,
+    S390_FEAT_STFLE_120,
+    S390_FEAT_STFLE_121,
+    S390_FEAT_STFLE_122,
+    S390_FEAT_STFLE_123,
+    S390_FEAT_STFLE_124,
+    S390_FEAT_STFLE_125,
+    S390_FEAT_STFLE_126,
+    S390_FEAT_STFLE_127,
     S390_FEAT_VECTOR,
     S390_FEAT_INSTRUCTION_EXEC_PROT,
     S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 95ee870..6c6a6ba 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -350,6 +350,11 @@ static uint16_t base_GEN14_GA1[] = {
  * Automatically includes corresponding base features.
  * Full features are all features this hardware supports even if kvm/QEMU do not
  * support these features yet.
+ *
+ * Non hypervisor managed  bits are supposed to be transparent with host-model.
+ * To achieve this the non-hypervisor managed bits are added to the full model.
+ * Thus full features does not necessarily reflect the hardware for the
+ * non-hypervisor managed bits.
  */
 static uint16_t full_GEN7_GA1[] = {
     S390_FEAT_SIE_F2,
@@ -357,7 +362,105 @@ static uint16_t full_GEN7_GA1[] = {
     S390_FEAT_SIE_GPERE,
     S390_FEAT_SIE_IB,
     S390_FEAT_SIE_CEI,
+
+    /* non-hyp 16-63*/
+    S390_FEAT_EXTENDED_TRANSLATION_2,
+    S390_FEAT_MSA,
+    S390_FEAT_LONG_DISPLACEMENT,
+    S390_FEAT_LONG_DISPLACEMENT_FAST,
+    S390_FEAT_HFP_MADDSUB,
+    S390_FEAT_EXTENDED_IMMEDIATE,
+    S390_FEAT_EXTENDED_TRANSLATION_3,
+    S390_FEAT_HFP_UNNORMALIZED_EXT,
+    S390_FEAT_ETF2_ENH,
+    S390_FEAT_STORE_CLOCK_FAST,
+    S390_FEAT_PARSING_ENH,
+    S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
+    S390_FEAT_TOD_CLOCK_STEERING,
+    S390_FEAT_STFLE_29,
+    S390_FEAT_ETF3_ENH,
+    S390_FEAT_EXTRACT_CPU_TIME,
+    S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
+    S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
+    S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
+    S390_FEAT_EXECUTE_EXT,
+    S390_FEAT_ENHANCED_MONITOR,
+    S390_FEAT_FLOATING_POINT_EXT,
+    S390_FEAT_ORDER_PRESERVING_COMPRESSION,
+    S390_FEAT_STFLE_39,
+    S390_FEAT_SET_PROGRAM_PARAMETERS,
+    S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
+    S390_FEAT_DFP,
+    S390_FEAT_DFP_FAST,
+    S390_FEAT_PFPO,
+    S390_FEAT_STFLE_45,
+    S390_FEAT_STFLE_46,
+    S390_FEAT_CMPSC_ENH,
+    S390_FEAT_DFP_ZONED_CONVERSION,
+    S390_FEAT_STFLE_49,
+    S390_FEAT_CONSTRAINT_TRANSACTIONAL_EXE,
+    S390_FEAT_LOCAL_TLB_CLEARING,
+    S390_FEAT_INTERLOCKED_ACCESS_2,
+    S390_FEAT_STFLE_53,
+    S390_FEAT_ENTROPY_ENC_COMP,
+    S390_FEAT_STFLE_55,
+    S390_FEAT_STFLE_56,
+    S390_FEAT_MSA_EXT_5,
+    S390_FEAT_MISC_INSTRUCTION_EXT,
+    S390_FEAT_SEMAPHORE_ASSIST,
+    S390_FEAT_TIME_SLICE_INSTRUMENTATION,
+    S390_FEAT_STFLE_61,
+    S390_FEAT_STFLE_62,
+    S390_FEAT_STFLE_63,
+    /* non-hyp 80-127*/
+    S390_FEAT_DFP_PACKED_CONVERSION,
+    S390_FEAT_STFLE_81,
     S390_FEAT_BPB,
+    S390_FEAT_STFLE_83,
+    S390_FEAT_STFLE_84,
+    S390_FEAT_STFLE_85,
+    S390_FEAT_STFLE_86,
+    S390_FEAT_STFLE_87,
+    S390_FEAT_STFLE_88,
+    S390_FEAT_STFLE_89,
+    S390_FEAT_STFLE_90,
+    S390_FEAT_STFLE_91,
+    S390_FEAT_STFLE_92,
+    S390_FEAT_STFLE_93,
+    S390_FEAT_STFLE_94,
+    S390_FEAT_STFLE_95,
+    S390_FEAT_STFLE_96,
+    S390_FEAT_STFLE_97,
+    S390_FEAT_STFLE_98,
+    S390_FEAT_STFLE_99,
+    S390_FEAT_STFLE_100,
+    S390_FEAT_STFLE_101,
+    S390_FEAT_STFLE_102,
+    S390_FEAT_STFLE_103,
+    S390_FEAT_STFLE_104,
+    S390_FEAT_STFLE_105,
+    S390_FEAT_STFLE_106,
+    S390_FEAT_STFLE_107,
+    S390_FEAT_STFLE_108,
+    S390_FEAT_STFLE_109,
+    S390_FEAT_STFLE_110,
+    S390_FEAT_STFLE_111,
+    S390_FEAT_STFLE_112,
+    S390_FEAT_STFLE_113,
+    S390_FEAT_STFLE_114,
+    S390_FEAT_STFLE_115,
+    S390_FEAT_STFLE_116,
+    S390_FEAT_STFLE_117,
+    S390_FEAT_STFLE_118,
+    S390_FEAT_STFLE_119,
+    S390_FEAT_STFLE_120,
+    S390_FEAT_STFLE_121,
+    S390_FEAT_STFLE_122,
+    S390_FEAT_STFLE_123,
+    S390_FEAT_STFLE_124,
+    S390_FEAT_STFLE_125,
+    S390_FEAT_STFLE_126,
+    S390_FEAT_STFLE_127,
 };
 
 static uint16_t full_GEN7_GA2[] = {
-- 
2.9.4

Re: [Qemu-devel] [PATCH 3/3] s390x/cpumodel: fix transparency for non-hyp STFL features

[David Hildenbrand]

On 17.01.2018 15:18, Christian Borntraeger wrote:
> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> 
> Before cpu-models were introduced to QEMU with 2.8 the so called
> non-hypervisor-managed STFL facilities (aka transparent facilities) were
> handled transparently.
> 
> With the advent cpu models, for host model (means -cpu host), we started
> fencing these of using the identified full model. The full models however
> did not include all non-hypervisor-managed facilities, thus we end up
> fencing some of these off.
> 
> New (non-hypervisor managed) facilities can be introduced with hardware
> or firmware upgrades. Requiring a code change and thus a QEMU upgrade to
> leverage such facilities is not acceptable. Namely the semantic of the host
> model is 'give me all you can'.
> 
> Let us add non-hypervisor managed STFL facility bits to QEMU and to all
> the full models. For now first two doublewords should be sufficient.
> 
> With this, when using host model, transparent facilities are presented to
> the guest. Regarding default (and base)  models however, nothing really
> changes, except that user can specify any non-hypervisor facility now.
> 
> Thus the so called transparent facilities, aren't handled transparently
> with the default nor with the base models (because of migration
> considerations).  For example -cpu z13 will not enable (and mandate) any
> of the features added with this change for any compat machine types.
> 
> Emerging non-hypervisor managed facilities that are expected to be
> present in any sane environment (in the context of the machine type)
> should be added to the default model (for non-compat machine types).
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  target/s390x/cpu_features.c     |  54 +++++++++++++++++++++
>  target/s390x/cpu_features_def.h |  54 +++++++++++++++++++++
>  target/s390x/gen-features.c     | 103 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 211 insertions(+)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 5d1c210..407864a 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -51,6 +51,7 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("parseh", S390_FEAT_TYPE_STFL, 26, "Parsing-enhancement facility"),
>      FEAT_INIT("mvcos", S390_FEAT_TYPE_STFL, 27, "Move-with-optional-specification facility"),
>      FEAT_INIT("tods-base", S390_FEAT_TYPE_STFL, 28, "TOD-clock-steering facility (excluding subfunctions)"),
> +    FEAT_INIT("stfle29", S390_FEAT_TYPE_STFL, 29, "Facility that is provided by STFLE facility 29"),
>      FEAT_INIT("etf3eh", S390_FEAT_TYPE_STFL, 30, "ETF3-enhancement facility"),
>      FEAT_INIT("ectg", S390_FEAT_TYPE_STFL, 31, "Extract-CPU-time facility"),
>      FEAT_INIT("csst", S390_FEAT_TYPE_STFL, 32, "Compare-and-swap-and-store facility"),
> @@ -60,12 +61,14 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("emon", S390_FEAT_TYPE_STFL, 36, "Enhanced-monitor facility"),
>      FEAT_INIT("fpe", S390_FEAT_TYPE_STFL, 37, "Floating-point extension facility"),
>      FEAT_INIT("opc", S390_FEAT_TYPE_STFL, 38, "Order Preserving Compression facility"),
> +    FEAT_INIT("stfle39", S390_FEAT_TYPE_STFL, 39, "Facility that is provided by STFLE facility 39"),
>      FEAT_INIT("sprogp", S390_FEAT_TYPE_STFL, 40, "Set-program-parameters facility"),
>      FEAT_INIT("fpseh", S390_FEAT_TYPE_STFL, 41, "Floating-point-support-enhancement facilities"),
>      FEAT_INIT("dfp", S390_FEAT_TYPE_STFL, 42, "DFP (decimal-floating-point) facility"),
>      FEAT_INIT("dfphp", S390_FEAT_TYPE_STFL, 43, "DFP (decimal-floating-point) facility has high performance"),
>      FEAT_INIT("pfpo", S390_FEAT_TYPE_STFL, 44, "PFPO instruction"),
>      FEAT_INIT("stfle45", S390_FEAT_TYPE_STFL, 45, "Various facilities introduced with z196"),
> +    FEAT_INIT("stfle46", S390_FEAT_TYPE_STFL, 46, "Facility that is provided by STFLE facility 46"),
>      FEAT_INIT("cmpsceh", S390_FEAT_TYPE_STFL, 47, "CMPSC-enhancement facility"),
>      FEAT_INIT("dfpzc", S390_FEAT_TYPE_STFL, 48, "Decimal-floating-point zoned-conversion facility"),
>      FEAT_INIT("stfle49", S390_FEAT_TYPE_STFL, 49, "Various facilities introduced with zEC12"),
> @@ -74,10 +77,15 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("iacc2", S390_FEAT_TYPE_STFL, 52, "Interlocked-access facility 2"),
>      FEAT_INIT("stfle53", S390_FEAT_TYPE_STFL, 53, "Various facilities introduced with z13"),
>      FEAT_INIT("eec", S390_FEAT_TYPE_STFL, 54, "Entropy encoding compression facility"),
> +    FEAT_INIT("stfle55", S390_FEAT_TYPE_STFL, 55, "Facility that is provided by STFLE facility 55"),
> +    FEAT_INIT("stfle56", S390_FEAT_TYPE_STFL, 56, "Facility that is provided by STFLE facility 56"),
>      FEAT_INIT("msa5-base", S390_FEAT_TYPE_STFL, 57, "Message-security-assist-extension-5 facility (excluding subfunctions)"),
>      FEAT_INIT("minste2", S390_FEAT_TYPE_STFL, 58, "Miscellaneous-instruction-extensions facility 2"),
>      FEAT_INIT("sema", S390_FEAT_TYPE_STFL, 59, "Semaphore-assist facility"),
>      FEAT_INIT("tsi", S390_FEAT_TYPE_STFL, 60, "Time-slice Instrumentation facility"),
> +    FEAT_INIT("stfle61", S390_FEAT_TYPE_STFL, 61, "Facility that is provided by STFLE facility 61"),
> +    FEAT_INIT("stfle62", S390_FEAT_TYPE_STFL, 62, "Facility that is provided by STFLE facility 62"),
> +    FEAT_INIT("stfle63", S390_FEAT_TYPE_STFL, 63, "Facility that is provided by STFLE facility 63"),
>      FEAT_INIT("ri", S390_FEAT_TYPE_STFL, 64, "CPU runtime-instrumentation facility"),
>      FEAT_INIT("zpci", S390_FEAT_TYPE_STFL, 69, "z/PCI facility"),
>      FEAT_INIT("aen", S390_FEAT_TYPE_STFL, 71, "General-purpose-adapter-event-notification facility"),
> @@ -89,7 +97,53 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
> +    FEAT_INIT("stfle81", S390_FEAT_TYPE_STFL, 81, "Facility that is provided by STFLE facility 81"),
>      FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
> +    FEAT_INIT("stfle83", S390_FEAT_TYPE_STFL, 83, "Facility that is provided by STFLE facility 83"),
> +    FEAT_INIT("stfle84", S390_FEAT_TYPE_STFL, 84, "Facility that is provided by STFLE facility 84"),
> +    FEAT_INIT("stfle85", S390_FEAT_TYPE_STFL, 85, "Facility that is provided by STFLE facility 85"),
> +    FEAT_INIT("stfle86", S390_FEAT_TYPE_STFL, 86, "Facility that is provided by STFLE facility 86"),
> +    FEAT_INIT("stfle87", S390_FEAT_TYPE_STFL, 87, "Facility that is provided by STFLE facility 87"),
> +    FEAT_INIT("stfle88", S390_FEAT_TYPE_STFL, 88, "Facility that is provided by STFLE facility 88"),
> +    FEAT_INIT("stfle89", S390_FEAT_TYPE_STFL, 89, "Facility that is provided by STFLE facility 89"),
> +    FEAT_INIT("stfle90", S390_FEAT_TYPE_STFL, 90, "Facility that is provided by STFLE facility 90"),
> +    FEAT_INIT("stfle91", S390_FEAT_TYPE_STFL, 91, "Facility that is provided by STFLE facility 91"),
> +    FEAT_INIT("stfle92", S390_FEAT_TYPE_STFL, 92, "Facility that is provided by STFLE facility 92"),
> +    FEAT_INIT("stfle93", S390_FEAT_TYPE_STFL, 93, "Facility that is provided by STFLE facility 93"),
> +    FEAT_INIT("stfle94", S390_FEAT_TYPE_STFL, 94, "Facility that is provided by STFLE facility 94"),
> +    FEAT_INIT("stfle95", S390_FEAT_TYPE_STFL, 95, "Facility that is provided by STFLE facility 95"),
> +    FEAT_INIT("stfle96", S390_FEAT_TYPE_STFL, 96, "Facility that is provided by STFLE facility 96"),
> +    FEAT_INIT("stfle97", S390_FEAT_TYPE_STFL, 97, "Facility that is provided by STFLE facility 97"),
> +    FEAT_INIT("stfle98", S390_FEAT_TYPE_STFL, 98, "Facility that is provided by STFLE facility 98"),
> +    FEAT_INIT("stfle99", S390_FEAT_TYPE_STFL, 99, "Facility that is provided by STFLE facility 99"),
> +    FEAT_INIT("stfle100", S390_FEAT_TYPE_STFL, 100, "Facility that is provided by STFLE facility 100"),
> +    FEAT_INIT("stfle101", S390_FEAT_TYPE_STFL, 101, "Facility that is provided by STFLE facility 101"),
> +    FEAT_INIT("stfle102", S390_FEAT_TYPE_STFL, 102, "Facility that is provided by STFLE facility 102"),
> +    FEAT_INIT("stfle103", S390_FEAT_TYPE_STFL, 103, "Facility that is provided by STFLE facility 103"),
> +    FEAT_INIT("stfle104", S390_FEAT_TYPE_STFL, 104, "Facility that is provided by STFLE facility 104"),
> +    FEAT_INIT("stfle105", S390_FEAT_TYPE_STFL, 105, "Facility that is provided by STFLE facility 105"),
> +    FEAT_INIT("stfle106", S390_FEAT_TYPE_STFL, 106, "Facility that is provided by STFLE facility 106"),
> +    FEAT_INIT("stfle107", S390_FEAT_TYPE_STFL, 107, "Facility that is provided by STFLE facility 107"),
> +    FEAT_INIT("stfle108", S390_FEAT_TYPE_STFL, 108, "Facility that is provided by STFLE facility 108"),
> +    FEAT_INIT("stfle109", S390_FEAT_TYPE_STFL, 109, "Facility that is provided by STFLE facility 109"),
> +    FEAT_INIT("stfle110", S390_FEAT_TYPE_STFL, 110, "Facility that is provided by STFLE facility 110"),
> +    FEAT_INIT("stfle111", S390_FEAT_TYPE_STFL, 111, "Facility that is provided by STFLE facility 111"),
> +    FEAT_INIT("stfle112", S390_FEAT_TYPE_STFL, 112, "Facility that is provided by STFLE facility 112"),
> +    FEAT_INIT("stfle113", S390_FEAT_TYPE_STFL, 113, "Facility that is provided by STFLE facility 113"),
> +    FEAT_INIT("stfle114", S390_FEAT_TYPE_STFL, 114, "Facility that is provided by STFLE facility 114"),
> +    FEAT_INIT("stfle115", S390_FEAT_TYPE_STFL, 115, "Facility that is provided by STFLE facility 115"),
> +    FEAT_INIT("stfle116", S390_FEAT_TYPE_STFL, 116, "Facility that is provided by STFLE facility 116"),
> +    FEAT_INIT("stfle117", S390_FEAT_TYPE_STFL, 117, "Facility that is provided by STFLE facility 117"),
> +    FEAT_INIT("stfle118", S390_FEAT_TYPE_STFL, 118, "Facility that is provided by STFLE facility 118"),
> +    FEAT_INIT("stfle119", S390_FEAT_TYPE_STFL, 119, "Facility that is provided by STFLE facility 119"),
> +    FEAT_INIT("stfle120", S390_FEAT_TYPE_STFL, 120, "Facility that is provided by STFLE facility 120"),
> +    FEAT_INIT("stfle121", S390_FEAT_TYPE_STFL, 121, "Facility that is provided by STFLE facility 121"),
> +    FEAT_INIT("stfle122", S390_FEAT_TYPE_STFL, 122, "Facility that is provided by STFLE facility 122"),
> +    FEAT_INIT("stfle123", S390_FEAT_TYPE_STFL, 123, "Facility that is provided by STFLE facility 123"),
> +    FEAT_INIT("stfle124", S390_FEAT_TYPE_STFL, 124, "Facility that is provided by STFLE facility 124"),
> +    FEAT_INIT("stfle125", S390_FEAT_TYPE_STFL, 125, "Facility that is provided by STFLE facility 125"),
> +    FEAT_INIT("stfle126", S390_FEAT_TYPE_STFL, 126, "Facility that is provided by STFLE facility 126"),
> +    FEAT_INIT("stfle127", S390_FEAT_TYPE_STFL, 127, "Facility that is provided by STFLE facility 127"),
>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 4487cfd..fd15362 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -42,6 +42,7 @@ typedef enum {
>      S390_FEAT_PARSING_ENH,
>      S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
>      S390_FEAT_TOD_CLOCK_STEERING,
> +    S390_FEAT_STFLE_29,
>      S390_FEAT_ETF3_ENH,
>      S390_FEAT_EXTRACT_CPU_TIME,
>      S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
> @@ -51,12 +52,14 @@ typedef enum {
>      S390_FEAT_ENHANCED_MONITOR,
>      S390_FEAT_FLOATING_POINT_EXT,
>      S390_FEAT_ORDER_PRESERVING_COMPRESSION,
> +    S390_FEAT_STFLE_39,
>      S390_FEAT_SET_PROGRAM_PARAMETERS,
>      S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
>      S390_FEAT_DFP,
>      S390_FEAT_DFP_FAST,
>      S390_FEAT_PFPO,
>      S390_FEAT_STFLE_45,
> +    S390_FEAT_STFLE_46,
>      S390_FEAT_CMPSC_ENH,
>      S390_FEAT_DFP_ZONED_CONVERSION,
>      S390_FEAT_STFLE_49,
> @@ -65,10 +68,15 @@ typedef enum {
>      S390_FEAT_INTERLOCKED_ACCESS_2,
>      S390_FEAT_STFLE_53,
>      S390_FEAT_ENTROPY_ENC_COMP,
> +    S390_FEAT_STFLE_55,
> +    S390_FEAT_STFLE_56,
>      S390_FEAT_MSA_EXT_5,
>      S390_FEAT_MISC_INSTRUCTION_EXT,
>      S390_FEAT_SEMAPHORE_ASSIST,
>      S390_FEAT_TIME_SLICE_INSTRUMENTATION,
> +    S390_FEAT_STFLE_61,
> +    S390_FEAT_STFLE_62,
> +    S390_FEAT_STFLE_63,
>      S390_FEAT_RUNTIME_INSTRUMENTATION,
>      S390_FEAT_ZPCI,
>      S390_FEAT_ADAPTER_EVENT_NOTIFICATION,
> @@ -80,7 +88,53 @@ typedef enum {
>      S390_FEAT_MSA_EXT_4,
>      S390_FEAT_EDAT_2,
>      S390_FEAT_DFP_PACKED_CONVERSION,
> +    S390_FEAT_STFLE_81,
>      S390_FEAT_BPB,
> +    S390_FEAT_STFLE_83,
> +    S390_FEAT_STFLE_84,
> +    S390_FEAT_STFLE_85,
> +    S390_FEAT_STFLE_86,
> +    S390_FEAT_STFLE_87,
> +    S390_FEAT_STFLE_88,
> +    S390_FEAT_STFLE_89,
> +    S390_FEAT_STFLE_90,
> +    S390_FEAT_STFLE_91,
> +    S390_FEAT_STFLE_92,
> +    S390_FEAT_STFLE_93,
> +    S390_FEAT_STFLE_94,
> +    S390_FEAT_STFLE_95,
> +    S390_FEAT_STFLE_96,
> +    S390_FEAT_STFLE_97,
> +    S390_FEAT_STFLE_98,
> +    S390_FEAT_STFLE_99,
> +    S390_FEAT_STFLE_100,
> +    S390_FEAT_STFLE_101,
> +    S390_FEAT_STFLE_102,
> +    S390_FEAT_STFLE_103,
> +    S390_FEAT_STFLE_104,
> +    S390_FEAT_STFLE_105,
> +    S390_FEAT_STFLE_106,
> +    S390_FEAT_STFLE_107,
> +    S390_FEAT_STFLE_108,
> +    S390_FEAT_STFLE_109,
> +    S390_FEAT_STFLE_110,
> +    S390_FEAT_STFLE_111,
> +    S390_FEAT_STFLE_112,
> +    S390_FEAT_STFLE_113,
> +    S390_FEAT_STFLE_114,
> +    S390_FEAT_STFLE_115,
> +    S390_FEAT_STFLE_116,
> +    S390_FEAT_STFLE_117,
> +    S390_FEAT_STFLE_118,
> +    S390_FEAT_STFLE_119,
> +    S390_FEAT_STFLE_120,
> +    S390_FEAT_STFLE_121,
> +    S390_FEAT_STFLE_122,
> +    S390_FEAT_STFLE_123,
> +    S390_FEAT_STFLE_124,
> +    S390_FEAT_STFLE_125,
> +    S390_FEAT_STFLE_126,
> +    S390_FEAT_STFLE_127,
>      S390_FEAT_VECTOR,
>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 95ee870..6c6a6ba 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -350,6 +350,11 @@ static uint16_t base_GEN14_GA1[] = {
>   * Automatically includes corresponding base features.
>   * Full features are all features this hardware supports even if kvm/QEMU do not
>   * support these features yet.
> + *
> + * Non hypervisor managed  bits are supposed to be transparent with host-model.
> + * To achieve this the non-hypervisor managed bits are added to the full model.
> + * Thus full features does not necessarily reflect the hardware for the
> + * non-hypervisor managed bits.
>   */
>  static uint16_t full_GEN7_GA1[] = {
>      S390_FEAT_SIE_F2,
> @@ -357,7 +362,105 @@ static uint16_t full_GEN7_GA1[] = {
>      S390_FEAT_SIE_GPERE,
>      S390_FEAT_SIE_IB,
>      S390_FEAT_SIE_CEI,
> +
> +    /* non-hyp 16-63*/
> +    S390_FEAT_EXTENDED_TRANSLATION_2,
> +    S390_FEAT_MSA,
> +    S390_FEAT_LONG_DISPLACEMENT,
> +    S390_FEAT_LONG_DISPLACEMENT_FAST,
> +    S390_FEAT_HFP_MADDSUB,
> +    S390_FEAT_EXTENDED_IMMEDIATE,
> +    S390_FEAT_EXTENDED_TRANSLATION_3,
> +    S390_FEAT_HFP_UNNORMALIZED_EXT,
> +    S390_FEAT_ETF2_ENH,
> +    S390_FEAT_STORE_CLOCK_FAST,
> +    S390_FEAT_PARSING_ENH,
> +    S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
> +    S390_FEAT_TOD_CLOCK_STEERING,
> +    S390_FEAT_STFLE_29,
> +    S390_FEAT_ETF3_ENH,
> +    S390_FEAT_EXTRACT_CPU_TIME,
> +    S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
> +    S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
> +    S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
> +    S390_FEAT_EXECUTE_EXT,
> +    S390_FEAT_ENHANCED_MONITOR,
> +    S390_FEAT_FLOATING_POINT_EXT,
> +    S390_FEAT_ORDER_PRESERVING_COMPRESSION,
> +    S390_FEAT_STFLE_39,
> +    S390_FEAT_SET_PROGRAM_PARAMETERS,
> +    S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
> +    S390_FEAT_DFP,
> +    S390_FEAT_DFP_FAST,
> +    S390_FEAT_PFPO,
> +    S390_FEAT_STFLE_45,
> +    S390_FEAT_STFLE_46,
> +    S390_FEAT_CMPSC_ENH,
> +    S390_FEAT_DFP_ZONED_CONVERSION,
> +    S390_FEAT_STFLE_49,
> +    S390_FEAT_CONSTRAINT_TRANSACTIONAL_EXE,
> +    S390_FEAT_LOCAL_TLB_CLEARING,
> +    S390_FEAT_INTERLOCKED_ACCESS_2,
> +    S390_FEAT_STFLE_53,
> +    S390_FEAT_ENTROPY_ENC_COMP,
> +    S390_FEAT_STFLE_55,
> +    S390_FEAT_STFLE_56,
> +    S390_FEAT_MSA_EXT_5,
> +    S390_FEAT_MISC_INSTRUCTION_EXT,
> +    S390_FEAT_SEMAPHORE_ASSIST,
> +    S390_FEAT_TIME_SLICE_INSTRUMENTATION,
> +    S390_FEAT_STFLE_61,
> +    S390_FEAT_STFLE_62,
> +    S390_FEAT_STFLE_63,
> +    /* non-hyp 80-127*/
> +    S390_FEAT_DFP_PACKED_CONVERSION,
> +    S390_FEAT_STFLE_81,
>      S390_FEAT_BPB,
> +    S390_FEAT_STFLE_83,
> +    S390_FEAT_STFLE_84,
> +    S390_FEAT_STFLE_85,
> +    S390_FEAT_STFLE_86,
> +    S390_FEAT_STFLE_87,
> +    S390_FEAT_STFLE_88,
> +    S390_FEAT_STFLE_89,
> +    S390_FEAT_STFLE_90,
> +    S390_FEAT_STFLE_91,
> +    S390_FEAT_STFLE_92,
> +    S390_FEAT_STFLE_93,
> +    S390_FEAT_STFLE_94,
> +    S390_FEAT_STFLE_95,
> +    S390_FEAT_STFLE_96,
> +    S390_FEAT_STFLE_97,
> +    S390_FEAT_STFLE_98,
> +    S390_FEAT_STFLE_99,
> +    S390_FEAT_STFLE_100,
> +    S390_FEAT_STFLE_101,
> +    S390_FEAT_STFLE_102,
> +    S390_FEAT_STFLE_103,
> +    S390_FEAT_STFLE_104,
> +    S390_FEAT_STFLE_105,
> +    S390_FEAT_STFLE_106,
> +    S390_FEAT_STFLE_107,
> +    S390_FEAT_STFLE_108,
> +    S390_FEAT_STFLE_109,
> +    S390_FEAT_STFLE_110,
> +    S390_FEAT_STFLE_111,
> +    S390_FEAT_STFLE_112,
> +    S390_FEAT_STFLE_113,
> +    S390_FEAT_STFLE_114,
> +    S390_FEAT_STFLE_115,
> +    S390_FEAT_STFLE_116,
> +    S390_FEAT_STFLE_117,
> +    S390_FEAT_STFLE_118,
> +    S390_FEAT_STFLE_119,
> +    S390_FEAT_STFLE_120,
> +    S390_FEAT_STFLE_121,
> +    S390_FEAT_STFLE_122,
> +    S390_FEAT_STFLE_123,
> +    S390_FEAT_STFLE_124,
> +    S390_FEAT_STFLE_125,
> +    S390_FEAT_STFLE_126,
> +    S390_FEAT_STFLE_127,
>  };
>  
>  static uint16_t full_GEN7_GA2[] = {
> 

As explained in Patch 2, NACK from my side. But as I also said, that
NACK might not have any weight :)

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 3/3] s390x/cpumodel: fix transparency for non-hyp STFL features

[Cornelia Huck]

On Wed, 17 Jan 2018 15:18:49 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> 
> Before cpu-models were introduced to QEMU with 2.8 the so called
> non-hypervisor-managed STFL facilities (aka transparent facilities) were
> handled transparently.
> 
> With the advent cpu models, for host model (means -cpu host), we started

s/the advent/the advent of/

s/means//

> fencing these of using the identified full model. The full models however

s/of/off/

> did not include all non-hypervisor-managed facilities, thus we end up
> fencing some of these off.
> 
> New (non-hypervisor managed) facilities can be introduced with hardware
> or firmware upgrades. Requiring a code change and thus a QEMU upgrade to
> leverage such facilities is not acceptable. Namely the semantic of the host
> model is 'give me all you can'.
> 
> Let us add non-hypervisor managed STFL facility bits to QEMU and to all
> the full models. For now first two doublewords should be sufficient.
> 
> With this, when using host model, transparent facilities are presented to
> the guest. Regarding default (and base)  models however, nothing really
> changes, except that user can specify any non-hypervisor facility now.
> 
> Thus the so called transparent facilities, aren't handled transparently
> with the default nor with the base models (because of migration
> considerations).  For example -cpu z13 will not enable (and mandate) any
> of the features added with this change for any compat machine types.
> 
> Emerging non-hypervisor managed facilities that are expected to be
> present in any sane environment (in the context of the machine type)
> should be added to the default model (for non-compat machine types).

I agree with this change in principle, but would like feedback from
David.

> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  target/s390x/cpu_features.c     |  54 +++++++++++++++++++++
>  target/s390x/cpu_features_def.h |  54 +++++++++++++++++++++
>  target/s390x/gen-features.c     | 103 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 211 insertions(+)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 5d1c210..407864a 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -51,6 +51,7 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("parseh", S390_FEAT_TYPE_STFL, 26, "Parsing-enhancement facility"),
>      FEAT_INIT("mvcos", S390_FEAT_TYPE_STFL, 27, "Move-with-optional-specification facility"),
>      FEAT_INIT("tods-base", S390_FEAT_TYPE_STFL, 28, "TOD-clock-steering facility (excluding subfunctions)"),
> +    FEAT_INIT("stfle29", S390_FEAT_TYPE_STFL, 29, "Facility that is provided by STFLE facility 29"),
>      FEAT_INIT("etf3eh", S390_FEAT_TYPE_STFL, 30, "ETF3-enhancement facility"),
>      FEAT_INIT("ectg", S390_FEAT_TYPE_STFL, 31, "Extract-CPU-time facility"),
>      FEAT_INIT("csst", S390_FEAT_TYPE_STFL, 32, "Compare-and-swap-and-store facility"),
> @@ -60,12 +61,14 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("emon", S390_FEAT_TYPE_STFL, 36, "Enhanced-monitor facility"),
>      FEAT_INIT("fpe", S390_FEAT_TYPE_STFL, 37, "Floating-point extension facility"),
>      FEAT_INIT("opc", S390_FEAT_TYPE_STFL, 38, "Order Preserving Compression facility"),
> +    FEAT_INIT("stfle39", S390_FEAT_TYPE_STFL, 39, "Facility that is provided by STFLE facility 39"),
>      FEAT_INIT("sprogp", S390_FEAT_TYPE_STFL, 40, "Set-program-parameters facility"),
>      FEAT_INIT("fpseh", S390_FEAT_TYPE_STFL, 41, "Floating-point-support-enhancement facilities"),
>      FEAT_INIT("dfp", S390_FEAT_TYPE_STFL, 42, "DFP (decimal-floating-point) facility"),
>      FEAT_INIT("dfphp", S390_FEAT_TYPE_STFL, 43, "DFP (decimal-floating-point) facility has high performance"),
>      FEAT_INIT("pfpo", S390_FEAT_TYPE_STFL, 44, "PFPO instruction"),
>      FEAT_INIT("stfle45", S390_FEAT_TYPE_STFL, 45, "Various facilities introduced with z196"),
> +    FEAT_INIT("stfle46", S390_FEAT_TYPE_STFL, 46, "Facility that is provided by STFLE facility 46"),
>      FEAT_INIT("cmpsceh", S390_FEAT_TYPE_STFL, 47, "CMPSC-enhancement facility"),
>      FEAT_INIT("dfpzc", S390_FEAT_TYPE_STFL, 48, "Decimal-floating-point zoned-conversion facility"),
>      FEAT_INIT("stfle49", S390_FEAT_TYPE_STFL, 49, "Various facilities introduced with zEC12"),
> @@ -74,10 +77,15 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("iacc2", S390_FEAT_TYPE_STFL, 52, "Interlocked-access facility 2"),
>      FEAT_INIT("stfle53", S390_FEAT_TYPE_STFL, 53, "Various facilities introduced with z13"),
>      FEAT_INIT("eec", S390_FEAT_TYPE_STFL, 54, "Entropy encoding compression facility"),
> +    FEAT_INIT("stfle55", S390_FEAT_TYPE_STFL, 55, "Facility that is provided by STFLE facility 55"),
> +    FEAT_INIT("stfle56", S390_FEAT_TYPE_STFL, 56, "Facility that is provided by STFLE facility 56"),
>      FEAT_INIT("msa5-base", S390_FEAT_TYPE_STFL, 57, "Message-security-assist-extension-5 facility (excluding subfunctions)"),
>      FEAT_INIT("minste2", S390_FEAT_TYPE_STFL, 58, "Miscellaneous-instruction-extensions facility 2"),
>      FEAT_INIT("sema", S390_FEAT_TYPE_STFL, 59, "Semaphore-assist facility"),
>      FEAT_INIT("tsi", S390_FEAT_TYPE_STFL, 60, "Time-slice Instrumentation facility"),
> +    FEAT_INIT("stfle61", S390_FEAT_TYPE_STFL, 61, "Facility that is provided by STFLE facility 61"),
> +    FEAT_INIT("stfle62", S390_FEAT_TYPE_STFL, 62, "Facility that is provided by STFLE facility 62"),
> +    FEAT_INIT("stfle63", S390_FEAT_TYPE_STFL, 63, "Facility that is provided by STFLE facility 63"),
>      FEAT_INIT("ri", S390_FEAT_TYPE_STFL, 64, "CPU runtime-instrumentation facility"),
>      FEAT_INIT("zpci", S390_FEAT_TYPE_STFL, 69, "z/PCI facility"),
>      FEAT_INIT("aen", S390_FEAT_TYPE_STFL, 71, "General-purpose-adapter-event-notification facility"),
> @@ -89,7 +97,53 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("msa4-base", S390_FEAT_TYPE_STFL, 77, "Message-security-assist-extension-4 facility (excluding subfunctions)"),
>      FEAT_INIT("edat2", S390_FEAT_TYPE_STFL, 78, "Enhanced-DAT facility 2"),
>      FEAT_INIT("dfppc", S390_FEAT_TYPE_STFL, 80, "Decimal-floating-point packed-conversion facility"),
> +    FEAT_INIT("stfle81", S390_FEAT_TYPE_STFL, 81, "Facility that is provided by STFLE facility 81"),
>      FEAT_INIT("bpb", S390_FEAT_TYPE_STFL, 82, "Branch Prediction Blocking"),
> +    FEAT_INIT("stfle83", S390_FEAT_TYPE_STFL, 83, "Facility that is provided by STFLE facility 83"),
> +    FEAT_INIT("stfle84", S390_FEAT_TYPE_STFL, 84, "Facility that is provided by STFLE facility 84"),
> +    FEAT_INIT("stfle85", S390_FEAT_TYPE_STFL, 85, "Facility that is provided by STFLE facility 85"),
> +    FEAT_INIT("stfle86", S390_FEAT_TYPE_STFL, 86, "Facility that is provided by STFLE facility 86"),
> +    FEAT_INIT("stfle87", S390_FEAT_TYPE_STFL, 87, "Facility that is provided by STFLE facility 87"),
> +    FEAT_INIT("stfle88", S390_FEAT_TYPE_STFL, 88, "Facility that is provided by STFLE facility 88"),
> +    FEAT_INIT("stfle89", S390_FEAT_TYPE_STFL, 89, "Facility that is provided by STFLE facility 89"),
> +    FEAT_INIT("stfle90", S390_FEAT_TYPE_STFL, 90, "Facility that is provided by STFLE facility 90"),
> +    FEAT_INIT("stfle91", S390_FEAT_TYPE_STFL, 91, "Facility that is provided by STFLE facility 91"),
> +    FEAT_INIT("stfle92", S390_FEAT_TYPE_STFL, 92, "Facility that is provided by STFLE facility 92"),
> +    FEAT_INIT("stfle93", S390_FEAT_TYPE_STFL, 93, "Facility that is provided by STFLE facility 93"),
> +    FEAT_INIT("stfle94", S390_FEAT_TYPE_STFL, 94, "Facility that is provided by STFLE facility 94"),
> +    FEAT_INIT("stfle95", S390_FEAT_TYPE_STFL, 95, "Facility that is provided by STFLE facility 95"),
> +    FEAT_INIT("stfle96", S390_FEAT_TYPE_STFL, 96, "Facility that is provided by STFLE facility 96"),
> +    FEAT_INIT("stfle97", S390_FEAT_TYPE_STFL, 97, "Facility that is provided by STFLE facility 97"),
> +    FEAT_INIT("stfle98", S390_FEAT_TYPE_STFL, 98, "Facility that is provided by STFLE facility 98"),
> +    FEAT_INIT("stfle99", S390_FEAT_TYPE_STFL, 99, "Facility that is provided by STFLE facility 99"),
> +    FEAT_INIT("stfle100", S390_FEAT_TYPE_STFL, 100, "Facility that is provided by STFLE facility 100"),
> +    FEAT_INIT("stfle101", S390_FEAT_TYPE_STFL, 101, "Facility that is provided by STFLE facility 101"),
> +    FEAT_INIT("stfle102", S390_FEAT_TYPE_STFL, 102, "Facility that is provided by STFLE facility 102"),
> +    FEAT_INIT("stfle103", S390_FEAT_TYPE_STFL, 103, "Facility that is provided by STFLE facility 103"),
> +    FEAT_INIT("stfle104", S390_FEAT_TYPE_STFL, 104, "Facility that is provided by STFLE facility 104"),
> +    FEAT_INIT("stfle105", S390_FEAT_TYPE_STFL, 105, "Facility that is provided by STFLE facility 105"),
> +    FEAT_INIT("stfle106", S390_FEAT_TYPE_STFL, 106, "Facility that is provided by STFLE facility 106"),
> +    FEAT_INIT("stfle107", S390_FEAT_TYPE_STFL, 107, "Facility that is provided by STFLE facility 107"),
> +    FEAT_INIT("stfle108", S390_FEAT_TYPE_STFL, 108, "Facility that is provided by STFLE facility 108"),
> +    FEAT_INIT("stfle109", S390_FEAT_TYPE_STFL, 109, "Facility that is provided by STFLE facility 109"),
> +    FEAT_INIT("stfle110", S390_FEAT_TYPE_STFL, 110, "Facility that is provided by STFLE facility 110"),
> +    FEAT_INIT("stfle111", S390_FEAT_TYPE_STFL, 111, "Facility that is provided by STFLE facility 111"),
> +    FEAT_INIT("stfle112", S390_FEAT_TYPE_STFL, 112, "Facility that is provided by STFLE facility 112"),
> +    FEAT_INIT("stfle113", S390_FEAT_TYPE_STFL, 113, "Facility that is provided by STFLE facility 113"),
> +    FEAT_INIT("stfle114", S390_FEAT_TYPE_STFL, 114, "Facility that is provided by STFLE facility 114"),
> +    FEAT_INIT("stfle115", S390_FEAT_TYPE_STFL, 115, "Facility that is provided by STFLE facility 115"),
> +    FEAT_INIT("stfle116", S390_FEAT_TYPE_STFL, 116, "Facility that is provided by STFLE facility 116"),
> +    FEAT_INIT("stfle117", S390_FEAT_TYPE_STFL, 117, "Facility that is provided by STFLE facility 117"),
> +    FEAT_INIT("stfle118", S390_FEAT_TYPE_STFL, 118, "Facility that is provided by STFLE facility 118"),
> +    FEAT_INIT("stfle119", S390_FEAT_TYPE_STFL, 119, "Facility that is provided by STFLE facility 119"),
> +    FEAT_INIT("stfle120", S390_FEAT_TYPE_STFL, 120, "Facility that is provided by STFLE facility 120"),
> +    FEAT_INIT("stfle121", S390_FEAT_TYPE_STFL, 121, "Facility that is provided by STFLE facility 121"),
> +    FEAT_INIT("stfle122", S390_FEAT_TYPE_STFL, 122, "Facility that is provided by STFLE facility 122"),
> +    FEAT_INIT("stfle123", S390_FEAT_TYPE_STFL, 123, "Facility that is provided by STFLE facility 123"),
> +    FEAT_INIT("stfle124", S390_FEAT_TYPE_STFL, 124, "Facility that is provided by STFLE facility 124"),
> +    FEAT_INIT("stfle125", S390_FEAT_TYPE_STFL, 125, "Facility that is provided by STFLE facility 125"),
> +    FEAT_INIT("stfle126", S390_FEAT_TYPE_STFL, 126, "Facility that is provided by STFLE facility 126"),
> +    FEAT_INIT("stfle127", S390_FEAT_TYPE_STFL, 127, "Facility that is provided by STFLE facility 127"),

The "stfle<n>" naming is probably the sanest thing to do here :/

>      FEAT_INIT("vx", S390_FEAT_TYPE_STFL, 129, "Vector facility"),
>      FEAT_INIT("iep", S390_FEAT_TYPE_STFL, 130, "Instruction-execution-protection facility"),
>      FEAT_INIT("sea_esop2", S390_FEAT_TYPE_STFL, 131, "Side-effect-access facility and Enhanced-suppression-on-protection facility 2"),

Re: [Qemu-devel] [PATCH/RFC 0/3] s390x/kvm: implement new hardware/firmware features

[no-reply]

Hi,

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

Type: series
Message-id: 20180117141849.65757-1-borntraeger@de.ibm.com
Subject: [Qemu-devel] [PATCH/RFC 0/3] s390x/kvm: implement new hardware/firmware 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
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20180117114430.19110-1-marcandre.lureau@redhat.com -> patchew/20180117114430.19110-1-marcandre.lureau@redhat.com
 t [tag update]            patchew/20180117135015.15051-1-mreitz@redhat.com -> patchew/20180117135015.15051-1-mreitz@redhat.com
 * [new tag]               patchew/20180117141849.65757-1-borntraeger@de.ibm.com -> patchew/20180117141849.65757-1-borntraeger@de.ibm.com
Switched to a new branch 'test'
1c939adbf1 s390x/cpumodel: fix transparency for non-hyp STFL features
ff8e68c658 s390x/kvm: Handle bpb feature
aceb38e1c3 header sync

=== OUTPUT BEGIN ===
Checking PATCH 1/3: header sync...
Checking PATCH 2/3: s390x/kvm: Handle bpb feature...
ERROR: suspect code indent for conditional statements (4, 7)
#82: FILE: target/s390x/kvm.c:493:
+    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
+       cs->kvm_run->s.regs.bpbc = env->bpbc;

ERROR: suspect code indent for conditional statements (4, 7)
#94: FILE: target/s390x/kvm.c:608:
+    if (can_sync_regs(cs, KVM_SYNC_BPBC)) {
+       env->bpbc = cs->kvm_run->s.regs.bpbc;

total: 2 errors, 0 warnings, 98 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.

Checking PATCH 3/3: s390x/cpumodel: fix transparency for non-hyp STFL features...
ERROR: line over 90 characters
#49: FILE: target/s390x/cpu_features.c:54:
+    FEAT_INIT("stfle29", S390_FEAT_TYPE_STFL, 29, "Facility that is provided by STFLE facility 29"),

ERROR: line over 90 characters
#57: FILE: target/s390x/cpu_features.c:64:
+    FEAT_INIT("stfle39", S390_FEAT_TYPE_STFL, 39, "Facility that is provided by STFLE facility 39"),

ERROR: line over 90 characters
#64: FILE: target/s390x/cpu_features.c:71:
+    FEAT_INIT("stfle46", S390_FEAT_TYPE_STFL, 46, "Facility that is provided by STFLE facility 46"),

ERROR: line over 90 characters
#72: FILE: target/s390x/cpu_features.c:80:
+    FEAT_INIT("stfle55", S390_FEAT_TYPE_STFL, 55, "Facility that is provided by STFLE facility 55"),

ERROR: line over 90 characters
#73: FILE: target/s390x/cpu_features.c:81:
+    FEAT_INIT("stfle56", S390_FEAT_TYPE_STFL, 56, "Facility that is provided by STFLE facility 56"),

ERROR: line over 90 characters
#78: FILE: target/s390x/cpu_features.c:86:
+    FEAT_INIT("stfle61", S390_FEAT_TYPE_STFL, 61, "Facility that is provided by STFLE facility 61"),

ERROR: line over 90 characters
#79: FILE: target/s390x/cpu_features.c:87:
+    FEAT_INIT("stfle62", S390_FEAT_TYPE_STFL, 62, "Facility that is provided by STFLE facility 62"),

ERROR: line over 90 characters
#80: FILE: target/s390x/cpu_features.c:88:
+    FEAT_INIT("stfle63", S390_FEAT_TYPE_STFL, 63, "Facility that is provided by STFLE facility 63"),

ERROR: line over 90 characters
#88: FILE: target/s390x/cpu_features.c:100:
+    FEAT_INIT("stfle81", S390_FEAT_TYPE_STFL, 81, "Facility that is provided by STFLE facility 81"),

ERROR: line over 90 characters
#90: FILE: target/s390x/cpu_features.c:102:
+    FEAT_INIT("stfle83", S390_FEAT_TYPE_STFL, 83, "Facility that is provided by STFLE facility 83"),

ERROR: line over 90 characters
#91: FILE: target/s390x/cpu_features.c:103:
+    FEAT_INIT("stfle84", S390_FEAT_TYPE_STFL, 84, "Facility that is provided by STFLE facility 84"),

ERROR: line over 90 characters
#92: FILE: target/s390x/cpu_features.c:104:
+    FEAT_INIT("stfle85", S390_FEAT_TYPE_STFL, 85, "Facility that is provided by STFLE facility 85"),

ERROR: line over 90 characters
#93: FILE: target/s390x/cpu_features.c:105:
+    FEAT_INIT("stfle86", S390_FEAT_TYPE_STFL, 86, "Facility that is provided by STFLE facility 86"),

ERROR: line over 90 characters
#94: FILE: target/s390x/cpu_features.c:106:
+    FEAT_INIT("stfle87", S390_FEAT_TYPE_STFL, 87, "Facility that is provided by STFLE facility 87"),

ERROR: line over 90 characters
#95: FILE: target/s390x/cpu_features.c:107:
+    FEAT_INIT("stfle88", S390_FEAT_TYPE_STFL, 88, "Facility that is provided by STFLE facility 88"),

ERROR: line over 90 characters
#96: FILE: target/s390x/cpu_features.c:108:
+    FEAT_INIT("stfle89", S390_FEAT_TYPE_STFL, 89, "Facility that is provided by STFLE facility 89"),

ERROR: line over 90 characters
#97: FILE: target/s390x/cpu_features.c:109:
+    FEAT_INIT("stfle90", S390_FEAT_TYPE_STFL, 90, "Facility that is provided by STFLE facility 90"),

ERROR: line over 90 characters
#98: FILE: target/s390x/cpu_features.c:110:
+    FEAT_INIT("stfle91", S390_FEAT_TYPE_STFL, 91, "Facility that is provided by STFLE facility 91"),

ERROR: line over 90 characters
#99: FILE: target/s390x/cpu_features.c:111:
+    FEAT_INIT("stfle92", S390_FEAT_TYPE_STFL, 92, "Facility that is provided by STFLE facility 92"),

ERROR: line over 90 characters
#100: FILE: target/s390x/cpu_features.c:112:
+    FEAT_INIT("stfle93", S390_FEAT_TYPE_STFL, 93, "Facility that is provided by STFLE facility 93"),

ERROR: line over 90 characters
#101: FILE: target/s390x/cpu_features.c:113:
+    FEAT_INIT("stfle94", S390_FEAT_TYPE_STFL, 94, "Facility that is provided by STFLE facility 94"),

ERROR: line over 90 characters
#102: FILE: target/s390x/cpu_features.c:114:
+    FEAT_INIT("stfle95", S390_FEAT_TYPE_STFL, 95, "Facility that is provided by STFLE facility 95"),

ERROR: line over 90 characters
#103: FILE: target/s390x/cpu_features.c:115:
+    FEAT_INIT("stfle96", S390_FEAT_TYPE_STFL, 96, "Facility that is provided by STFLE facility 96"),

ERROR: line over 90 characters
#104: FILE: target/s390x/cpu_features.c:116:
+    FEAT_INIT("stfle97", S390_FEAT_TYPE_STFL, 97, "Facility that is provided by STFLE facility 97"),

ERROR: line over 90 characters
#105: FILE: target/s390x/cpu_features.c:117:
+    FEAT_INIT("stfle98", S390_FEAT_TYPE_STFL, 98, "Facility that is provided by STFLE facility 98"),

ERROR: line over 90 characters
#106: FILE: target/s390x/cpu_features.c:118:
+    FEAT_INIT("stfle99", S390_FEAT_TYPE_STFL, 99, "Facility that is provided by STFLE facility 99"),

ERROR: line over 90 characters
#107: FILE: target/s390x/cpu_features.c:119:
+    FEAT_INIT("stfle100", S390_FEAT_TYPE_STFL, 100, "Facility that is provided by STFLE facility 100"),

ERROR: line over 90 characters
#108: FILE: target/s390x/cpu_features.c:120:
+    FEAT_INIT("stfle101", S390_FEAT_TYPE_STFL, 101, "Facility that is provided by STFLE facility 101"),

ERROR: line over 90 characters
#109: FILE: target/s390x/cpu_features.c:121:
+    FEAT_INIT("stfle102", S390_FEAT_TYPE_STFL, 102, "Facility that is provided by STFLE facility 102"),

ERROR: line over 90 characters
#110: FILE: target/s390x/cpu_features.c:122:
+    FEAT_INIT("stfle103", S390_FEAT_TYPE_STFL, 103, "Facility that is provided by STFLE facility 103"),

ERROR: line over 90 characters
#111: FILE: target/s390x/cpu_features.c:123:
+    FEAT_INIT("stfle104", S390_FEAT_TYPE_STFL, 104, "Facility that is provided by STFLE facility 104"),

ERROR: line over 90 characters
#112: FILE: target/s390x/cpu_features.c:124:
+    FEAT_INIT("stfle105", S390_FEAT_TYPE_STFL, 105, "Facility that is provided by STFLE facility 105"),

ERROR: line over 90 characters
#113: FILE: target/s390x/cpu_features.c:125:
+    FEAT_INIT("stfle106", S390_FEAT_TYPE_STFL, 106, "Facility that is provided by STFLE facility 106"),

ERROR: line over 90 characters
#114: FILE: target/s390x/cpu_features.c:126:
+    FEAT_INIT("stfle107", S390_FEAT_TYPE_STFL, 107, "Facility that is provided by STFLE facility 107"),

ERROR: line over 90 characters
#115: FILE: target/s390x/cpu_features.c:127:
+    FEAT_INIT("stfle108", S390_FEAT_TYPE_STFL, 108, "Facility that is provided by STFLE facility 108"),

ERROR: line over 90 characters
#116: FILE: target/s390x/cpu_features.c:128:
+    FEAT_INIT("stfle109", S390_FEAT_TYPE_STFL, 109, "Facility that is provided by STFLE facility 109"),

ERROR: line over 90 characters
#117: FILE: target/s390x/cpu_features.c:129:
+    FEAT_INIT("stfle110", S390_FEAT_TYPE_STFL, 110, "Facility that is provided by STFLE facility 110"),

ERROR: line over 90 characters
#118: FILE: target/s390x/cpu_features.c:130:
+    FEAT_INIT("stfle111", S390_FEAT_TYPE_STFL, 111, "Facility that is provided by STFLE facility 111"),

ERROR: line over 90 characters
#119: FILE: target/s390x/cpu_features.c:131:
+    FEAT_INIT("stfle112", S390_FEAT_TYPE_STFL, 112, "Facility that is provided by STFLE facility 112"),

ERROR: line over 90 characters
#120: FILE: target/s390x/cpu_features.c:132:
+    FEAT_INIT("stfle113", S390_FEAT_TYPE_STFL, 113, "Facility that is provided by STFLE facility 113"),

ERROR: line over 90 characters
#121: FILE: target/s390x/cpu_features.c:133:
+    FEAT_INIT("stfle114", S390_FEAT_TYPE_STFL, 114, "Facility that is provided by STFLE facility 114"),

ERROR: line over 90 characters
#122: FILE: target/s390x/cpu_features.c:134:
+    FEAT_INIT("stfle115", S390_FEAT_TYPE_STFL, 115, "Facility that is provided by STFLE facility 115"),

ERROR: line over 90 characters
#123: FILE: target/s390x/cpu_features.c:135:
+    FEAT_INIT("stfle116", S390_FEAT_TYPE_STFL, 116, "Facility that is provided by STFLE facility 116"),

ERROR: line over 90 characters
#124: FILE: target/s390x/cpu_features.c:136:
+    FEAT_INIT("stfle117", S390_FEAT_TYPE_STFL, 117, "Facility that is provided by STFLE facility 117"),

ERROR: line over 90 characters
#125: FILE: target/s390x/cpu_features.c:137:
+    FEAT_INIT("stfle118", S390_FEAT_TYPE_STFL, 118, "Facility that is provided by STFLE facility 118"),

ERROR: line over 90 characters
#126: FILE: target/s390x/cpu_features.c:138:
+    FEAT_INIT("stfle119", S390_FEAT_TYPE_STFL, 119, "Facility that is provided by STFLE facility 119"),

ERROR: line over 90 characters
#127: FILE: target/s390x/cpu_features.c:139:
+    FEAT_INIT("stfle120", S390_FEAT_TYPE_STFL, 120, "Facility that is provided by STFLE facility 120"),

ERROR: line over 90 characters
#128: FILE: target/s390x/cpu_features.c:140:
+    FEAT_INIT("stfle121", S390_FEAT_TYPE_STFL, 121, "Facility that is provided by STFLE facility 121"),

ERROR: line over 90 characters
#129: FILE: target/s390x/cpu_features.c:141:
+    FEAT_INIT("stfle122", S390_FEAT_TYPE_STFL, 122, "Facility that is provided by STFLE facility 122"),

ERROR: line over 90 characters
#130: FILE: target/s390x/cpu_features.c:142:
+    FEAT_INIT("stfle123", S390_FEAT_TYPE_STFL, 123, "Facility that is provided by STFLE facility 123"),

ERROR: line over 90 characters
#131: FILE: target/s390x/cpu_features.c:143:
+    FEAT_INIT("stfle124", S390_FEAT_TYPE_STFL, 124, "Facility that is provided by STFLE facility 124"),

ERROR: line over 90 characters
#132: FILE: target/s390x/cpu_features.c:144:
+    FEAT_INIT("stfle125", S390_FEAT_TYPE_STFL, 125, "Facility that is provided by STFLE facility 125"),

ERROR: line over 90 characters
#133: FILE: target/s390x/cpu_features.c:145:
+    FEAT_INIT("stfle126", S390_FEAT_TYPE_STFL, 126, "Facility that is provided by STFLE facility 126"),

ERROR: line over 90 characters
#134: FILE: target/s390x/cpu_features.c:146:
+    FEAT_INIT("stfle127", S390_FEAT_TYPE_STFL, 127, "Facility that is provided by STFLE facility 127"),

total: 54 errors, 0 warnings, 294 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