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

[Qemu-devel] [PATCH v2 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

v1->v2: - style and comment fixes
	- drop transparent facility patch
	- add patch to introduce facility 81

Christian Borntraeger (3):
  header sync
  s390x/kvm: Handle bpb feature
  s390x/kvm: provide stfle.81

 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     |  2 ++
 target/s390x/cpu_features_def.h |  2 ++
 target/s390x/gen-features.c     |  2 ++
 target/s390x/kvm.c              | 14 ++++++++++++++
 target/s390x/machine.c          | 17 +++++++++++++++++
 9 files changed, 46 insertions(+), 7 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v2 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 v2 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              | 14 ++++++++++++++
 target/s390x/machine.c          | 17 +++++++++++++++++
 7 files changed, 36 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..8736001 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,11 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         clear_bit(S390_FEAT_CMM_NT, model->features);
     }
 
+    /* bpb needs kernel support for migration, VSIE 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

[Qemu-devel] [PATCH v2 3/3] s390x/kvm: provide stfle.81

[Christian Borntraeger]

stfle.81 (ppa15) is a transparent facility that can be passed to the
guest without the need to implement hypervisor support. As this feature
can be provided by firmware we add it to all full models.

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

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 5d1c210..1550ab0 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("ppa15", S390_FEAT_TYPE_STFL, 81, "PPA15 is installed"),
     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"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 4487cfd..4d93087 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_PPA15,
     S390_FEAT_BPB,
     S390_FEAT_VECTOR,
     S390_FEAT_INSTRUCTION_EXEC_PROT,
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 95ee870..50bd3f4 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_PPA15,
     S390_FEAT_BPB,
 };
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH v2 1/3] header sync

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On 01/17/2018 01:37 PM, Christian Borntraeger wrote:
> 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.
> - *

Is dropping a license clause intentional?  If so, the commit message may
want to go into more details why it is okay.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 1/3] header sync

[Christian Borntraeger]

It is just a header sync and mimics the kernel.
The previous header sync added the the commit that added the SPDX tag and this
header sync added the commit that removed the now redundant licence text.

On 01/17/2018 08:46 PM, Eric Blake wrote:
> On 01/17/2018 01:37 PM, Christian Borntraeger wrote:
>> 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(-)
>>tag
>> 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.
>> - *
> 
> Is dropping a license clause intentional?  If so, the commit message may
> want to go into more details why it is okay.
> 

Re: [Qemu-devel] [PATCH v2 1/3] header sync

[David Hildenbrand]

On 17.01.2018 20:37, Christian Borntraeger wrote:
> 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
>  
> 

Acked-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

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

[David Hildenbrand]

On 17.01.2018 20:37, 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.
> 
> 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              | 14 ++++++++++++++
>  target/s390x/machine.c          | 17 +++++++++++++++++
>  7 files changed, 36 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;

As this is effectively only 1 bit, I wonder if we should also make that
clear in QEMU

bool bpbc, VMSTATE_BOOL ...

Apart from that, looks sane.

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 3/3] s390x/kvm: provide stfle.81

[David Hildenbrand]

On 17.01.2018 20:37, Christian Borntraeger wrote:
> stfle.81 (ppa15) is a transparent facility that can be passed to the
> guest without the need to implement hypervisor support. As this feature
> can be provided by firmware we add it to all full models.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  target/s390x/cpu_features.c     | 1 +
>  target/s390x/cpu_features_def.h | 1 +
>  target/s390x/gen-features.c     | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 5d1c210..1550ab0 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("ppa15", S390_FEAT_TYPE_STFL, 81, "PPA15 is installed"),
>      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"),
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 4487cfd..4d93087 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_PPA15,
>      S390_FEAT_BPB,
>      S390_FEAT_VECTOR,
>      S390_FEAT_INSTRUCTION_EXEC_PROT,
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 95ee870..50bd3f4 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_PPA15,
>      S390_FEAT_BPB,

Is the order in this list still correct?

Apart from that, looks nice.

>  };
>  
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 3/3] s390x/kvm: provide stfle.81

[Christian Borntraeger]

On 01/17/2018 10:13 PM, David Hildenbrand wrote:
> On 17.01.2018 20:37, Christian Borntraeger wrote:
>> stfle.81 (ppa15) is a transparent facility that can be passed to the
>> guest without the need to implement hypervisor support. As this feature
>> can be provided by firmware we add it to all full models.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  target/s390x/cpu_features.c     | 1 +
>>  target/s390x/cpu_features_def.h | 1 +
>>  target/s390x/gen-features.c     | 1 +
>>  3 files changed, 3 insertions(+)
>>
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 5d1c210..1550ab0 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("ppa15", S390_FEAT_TYPE_STFL, 81, "PPA15 is installed"),
>>      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"),
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 4487cfd..4d93087 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_PPA15,
>>      S390_FEAT_BPB,
>>      S390_FEAT_VECTOR,
>>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 95ee870..50bd3f4 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_PPA15,
>>      S390_FEAT_BPB,
> 

> Is the order in this list still correct?

Is the order in this array important as well? It seems to work fine. At least PPA15 is correctly
ordered vs BPB.

Re: [Qemu-devel] [PATCH v2 3/3] s390x/kvm: provide stfle.81

[David Hildenbrand]

On 17.01.2018 22:37, Christian Borntraeger wrote:
> 
> 
> On 01/17/2018 10:13 PM, David Hildenbrand wrote:
>> On 17.01.2018 20:37, Christian Borntraeger wrote:
>>> stfle.81 (ppa15) is a transparent facility that can be passed to the
>>> guest without the need to implement hypervisor support. As this feature
>>> can be provided by firmware we add it to all full models.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  target/s390x/cpu_features.c     | 1 +
>>>  target/s390x/cpu_features_def.h | 1 +
>>>  target/s390x/gen-features.c     | 1 +
>>>  3 files changed, 3 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 5d1c210..1550ab0 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("ppa15", S390_FEAT_TYPE_STFL, 81, "PPA15 is installed"),
>>>      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"),
>>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>>> index 4487cfd..4d93087 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_PPA15,
>>>      S390_FEAT_BPB,
>>>      S390_FEAT_VECTOR,
>>>      S390_FEAT_INSTRUCTION_EXEC_PROT,
>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>> index 95ee870..50bd3f4 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_PPA15,
>>>      S390_FEAT_BPB,
>>
> 
>> Is the order in this list still correct?
> 
> Is the order in this array important as well? It seems to work fine. At least PPA15 is correctly
> ordered vs BPB.
> 

Purely style. (when introducing, I tried to keep features in the same
order as defined in target/s390x/cpu_features_def.h )

So feel free to keep the order (or I'll clean this up again one day).

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

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

[Thomas Huth]

On 17.01.2018 22:12, David Hildenbrand wrote:
> On 17.01.2018 20:37, 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.
>>
>> 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              | 14 ++++++++++++++
>>  target/s390x/machine.c          | 17 +++++++++++++++++
>>  7 files changed, 36 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;
> 
> As this is effectively only 1 bit, I wonder if we should also make that
> clear in QEMU
> 
> bool bpbc, VMSTATE_BOOL ...
> 
> Apart from that, looks sane.

I had the same thought when reading the patch ... apart from that, also
looks good to me.

 Thomas