[PATCH 0/7] KVM: s390: new guest facilities

[PATCH 0/7] KVM: s390: new guest facilities

[Christian Borntraeger]

Here is support for new guest cpu facilities.

Christian Borntraeger (7):
  KVM: s390: add vector enhancements facility 2 to cpumodel
  KVM: s390: add vector BCD enhancements facility to cpumodel
  KVM: s390: add MSA9 to cpumodel
  KVM: s390: enable MSA9 keywrapping functions depending on cpu model
  KVM: s390: provide query function for instructions returning 32 byte
  KVM: s390: add enhanced sort facilty to cpu model
  KVM: s390: add deflate conversion facilty to cpu model

 Documentation/virtual/kvm/devices/vm.txt |  3 +-
 arch/s390/include/asm/cpacf.h            |  1 +
 arch/s390/include/asm/kvm_host.h         |  1 +
 arch/s390/include/uapi/asm/kvm.h         |  5 +-
 arch/s390/kvm/kvm-s390.c                 | 77 ++++++++++++++++++++++++
 arch/s390/tools/gen_facilities.c         |  3 +
 tools/arch/s390/include/uapi/asm/kvm.h   |  3 +-
 7 files changed, 90 insertions(+), 3 deletions(-)

-- 
2.19.1

[PATCH 1/7] KVM: s390: add vector enhancements facility 2 to cpumodel

[Christian Borntraeger]

If vector support is enabled, the vector enhancements facility 2
might also be enabled.
We can directly forward this facility to the guest if available
and VX is requested by user space.

Please note that user space can and will have the final decision
on the facility bits for a guests.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4638303ba6a8..03ddbe5e62bc 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -657,6 +657,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 				set_kvm_facility(kvm->arch.model.fac_mask, 135);
 				set_kvm_facility(kvm->arch.model.fac_list, 135);
 			}
+			if (test_facility(148)) {
+				set_kvm_facility(kvm->arch.model.fac_mask, 148);
+				set_kvm_facility(kvm->arch.model.fac_list, 148);
+			}
 			r = 0;
 		} else
 			r = -EINVAL;
-- 
2.19.1

[PATCH 2/7] KVM: s390: add vector BCD enhancements facility to cpumodel

[Christian Borntraeger]

If vector support is enabled, the vector BCD enhancements facility
might also be enabled.
We can directly forward this facility to the guest if available
and VX is requested by user space.

Please note that user space can and will have the final decision
on the facility bits for a guests.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 03ddbe5e62bc..d3f3e63bb164 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -661,6 +661,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 				set_kvm_facility(kvm->arch.model.fac_mask, 148);
 				set_kvm_facility(kvm->arch.model.fac_list, 148);
 			}
+			if (test_facility(152)) {
+				set_kvm_facility(kvm->arch.model.fac_mask, 152);
+				set_kvm_facility(kvm->arch.model.fac_list, 152);
+			}
 			r = 0;
 		} else
 			r = -EINVAL;
-- 
2.19.1

[PATCH 3/7] KVM: s390: add MSA9 to cpumodel

[Christian Borntraeger]

This enables stfle.155 and adds the subfunctions for KDSA. Bit 155 is
added to the list of facilities that will be enabled when there is no
cpu model involved as MSA9 requires no additional handling from
userspace, e.g. for migration.

Please note that a cpu model enabled user space can and will have the
final decision on the facility bits for a guests.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 Documentation/virtual/kvm/devices/vm.txt |  3 ++-
 arch/s390/include/asm/cpacf.h            |  1 +
 arch/s390/include/uapi/asm/kvm.h         |  3 ++-
 arch/s390/kvm/kvm-s390.c                 | 13 +++++++++++++
 arch/s390/tools/gen_facilities.c         |  1 +
 tools/arch/s390/include/uapi/asm/kvm.h   |  3 ++-
 6 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
index 95ca68d663a4..4ffb82b02468 100644
--- a/Documentation/virtual/kvm/devices/vm.txt
+++ b/Documentation/virtual/kvm/devices/vm.txt
@@ -141,7 +141,8 @@ struct kvm_s390_vm_cpu_subfunc {
        u8 pcc[16];           # valid with Message-Security-Assist-Extension 4
        u8 ppno[16];          # valid with Message-Security-Assist-Extension 5
        u8 kma[16];           # valid with Message-Security-Assist-Extension 8
-       u8 reserved[1808];    # reserved for future instructions
+       u8 kdsa[16];          # valid with Message-Security-Assist-Extension 9
+       u8 reserved[1792];    # reserved for future instructions
 };
 
 Parameters: address of a buffer to load the subfunction blocks from.
diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
index 3cc52e37b4b2..ce2770743cc5 100644
--- a/arch/s390/include/asm/cpacf.h
+++ b/arch/s390/include/asm/cpacf.h
@@ -28,6 +28,7 @@
 #define CPACF_KMCTR		0xb92d		/* MSA4 */
 #define CPACF_PRNO		0xb93c		/* MSA5 */
 #define CPACF_KMA		0xb929		/* MSA8 */
+#define CPACF_KDSA		0xb93a		/* MSA9 */
 
 /*
  * En/decryption modifier bits
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 16511d97e8dc..09652eabe769 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -152,7 +152,8 @@ struct kvm_s390_vm_cpu_subfunc {
 	__u8 pcc[16];		/* with MSA4 */
 	__u8 ppno[16];		/* with MSA5 */
 	__u8 kma[16];		/* with MSA8 */
-	__u8 reserved[1808];
+	__u8 kdsa[16];		/* with MSA9 */
+	__u8 reserved[1792];
 };
 
 /* kvm attributes for crypto */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d3f3e63bb164..0dad61ccde3d 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -368,6 +368,10 @@ static void kvm_s390_cpu_feat_init(void)
 		__cpacf_query(CPACF_KMA, (cpacf_mask_t *)
 			      kvm_s390_available_subfunc.kma);
 
+	if (test_facility(155)) /* MSA9 */
+		__cpacf_query(CPACF_KDSA, (cpacf_mask_t *)
+			      kvm_s390_available_subfunc.kdsa);
+
 	if (MACHINE_HAS_ESOP)
 		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
 	/*
@@ -1331,6 +1335,9 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "SET: guest KMA    subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[0],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[1]);
+	VM_EVENT(kvm, 3, "SET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
 
 	return 0;
 }
@@ -1499,6 +1506,9 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "GET: guest KMA    subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[0],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[1]);
+	VM_EVENT(kvm, 3, "GET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
 
 	return 0;
 }
@@ -1554,6 +1564,9 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "GET: host  KMA    subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm_s390_available_subfunc.kma)[0],
 		 ((unsigned long *) &kvm_s390_available_subfunc.kma)[1]);
+	VM_EVENT(kvm, 3, "GET: host  KDSA   subfunc 0x%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[0],
+		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[1]);
 
 	return 0;
 }
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index fd788e0f2e5b..e952cb3b75b2 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -93,6 +93,7 @@ static struct facility_def facility_defs[] = {
 			131, /* enhanced-SOP 2 and side-effect */
 			139, /* multiple epoch facility */
 			146, /* msa extension 8 */
+			155, /* msa extension 9 */
 			-1  /* END */
 		}
 	},
diff --git a/tools/arch/s390/include/uapi/asm/kvm.h b/tools/arch/s390/include/uapi/asm/kvm.h
index 16511d97e8dc..09652eabe769 100644
--- a/tools/arch/s390/include/uapi/asm/kvm.h
+++ b/tools/arch/s390/include/uapi/asm/kvm.h
@@ -152,7 +152,8 @@ struct kvm_s390_vm_cpu_subfunc {
 	__u8 pcc[16];		/* with MSA4 */
 	__u8 ppno[16];		/* with MSA5 */
 	__u8 kma[16];		/* with MSA8 */
-	__u8 reserved[1808];
+	__u8 kdsa[16];		/* with MSA9 */
+	__u8 reserved[1792];
 };
 
 /* kvm attributes for crypto */
-- 
2.19.1

[PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

Instead of adding a new machine option to disable/enable the keywrapping
options of pckmo (like for AES and DEA) we can now use the CPU model to
decide.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h | 1 +
 arch/s390/kvm/kvm-s390.c         | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index c47e22bba87f..e224246ff93c 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
 #define ECD_HOSTREGMGMT	0x20000000
 #define ECD_MEF		0x08000000
 #define ECD_ETOKENF	0x02000000
+#define ECD_ECC		0x00200000
 	__u32	ecd;			/* 0x01c8 */
 	__u8	reserved1cc[18];	/* 0x01cc */
 	__u64	pp;			/* 0x01de */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0dad61ccde3d..ff2444d935fd 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
 			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
 	}
+	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
+	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
+	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
+		vcpu->arch.sie_block->ecd |= ECD_ECC;
 	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
 					| SDNXC;
 	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
-- 
2.19.1

[PATCH 5/7] KVM: s390: provide query function for instructions returning 32 byte

[Christian Borntraeger]

Some of the new features have a 32byte response for the query function.
Provide a new wrapper similar to __cpacf_query. We might want to factor
this out if other users come up, as of today there is none. So let us
keep the function within KVM.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index ff2444d935fd..f0404960a715 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -321,6 +321,19 @@ static inline int plo_test_bit(unsigned char nr)
 	return cc == 0;
 }
 
+static inline void __insn32_query(unsigned int opcode, u8 query[32])
+{
+	register unsigned long r0 asm("0") = 0;	/* query function */
+	register unsigned long r1 asm("1") = (unsigned long) query;
+
+	asm volatile(
+		/* Parameter regs are ignored */
+		"	.insn	rrf,%[opc] << 16,2,4,6,0\n"
+		: "=m" (*query)
+		: "d" (r0), "a" (r1), [opc] "i" (opcode)
+		: "cc");
+}
+
 static void kvm_s390_cpu_feat_init(void)
 {
 	int i;
-- 
2.19.1

[PATCH 6/7] KVM: s390: add enhanced sort facilty to cpu model

[Christian Borntraeger]

This enables stfle.150 and adds the subfunctions for SORTL. Bit 150 is
added to the list of facilities that will be enabled when there is no
cpu model involved as sortl requires no additional handling from
userspace, e.g. for migration.

Please note that a cpu model enabled user space can and will have the
final decision on the facility bits for a guests.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/include/uapi/asm/kvm.h |  3 ++-
 arch/s390/kvm/kvm-s390.c         | 20 ++++++++++++++++++++
 arch/s390/tools/gen_facilities.c |  1 +
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 09652eabe769..eba5bac08348 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -153,7 +153,8 @@ struct kvm_s390_vm_cpu_subfunc {
 	__u8 ppno[16];		/* with MSA5 */
 	__u8 kma[16];		/* with MSA8 */
 	__u8 kdsa[16];		/* with MSA9 */
-	__u8 reserved[1792];
+	__u8 sortl[32];		/* with STFLE.150 */
+	__u8 reserved[1760];
 };
 
 /* kvm attributes for crypto */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index f0404960a715..24b6960d01b4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -334,6 +334,8 @@ static inline void __insn32_query(unsigned int opcode, u8 query[32])
 		: "cc");
 }
 
+#define INSN_SORTL 0xb938
+
 static void kvm_s390_cpu_feat_init(void)
 {
 	int i;
@@ -385,6 +387,9 @@ static void kvm_s390_cpu_feat_init(void)
 		__cpacf_query(CPACF_KDSA, (cpacf_mask_t *)
 			      kvm_s390_available_subfunc.kdsa);
 
+	if (test_facility(150)) /* SORTL */
+		__insn32_query(INSN_SORTL, kvm_s390_available_subfunc.sortl);
+
 	if (MACHINE_HAS_ESOP)
 		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
 	/*
@@ -1351,6 +1356,11 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "SET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
+	VM_EVENT(kvm, 3, "SET: guest SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
 
 	return 0;
 }
@@ -1522,6 +1532,11 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "GET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
+	VM_EVENT(kvm, 3, "GET: guest SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
 
 	return 0;
 }
@@ -1580,6 +1595,11 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
 	VM_EVENT(kvm, 3, "GET: host  KDSA   subfunc 0x%16.16lx.%16.16lx",
 		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[0],
 		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[1]);
+	VM_EVENT(kvm, 3, "GET: host  SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[0],
+		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[1],
+		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[2],
+		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[3]);
 
 	return 0;
 }
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index e952cb3b75b2..1ec6bed785e8 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -93,6 +93,7 @@ static struct facility_def facility_defs[] = {
 			131, /* enhanced-SOP 2 and side-effect */
 			139, /* multiple epoch facility */
 			146, /* msa extension 8 */
+			150, /* enhanced sort */
 			155, /* msa extension 9 */
 			-1  /* END */
 		}
-- 
2.19.1

[PATCH 7/7] KVM: s390: add deflate conversion facilty to cpu model

[Christian Borntraeger]

This enables stfle.151 and adds the subfunctions for DFLTCC

This enables stfle.151 and adds the subfunctions for DFLTCC. Bit 151 is
added to the list of facilities that will be enabled when there is no
cpu model involved as DFLTCC requires no additional handling from
userspace, e.g. for migration.

Please note that a cpu model enabled user space can and will have the
final decision on the facility bits for a guests.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
 arch/s390/include/uapi/asm/kvm.h |  3 ++-
 arch/s390/kvm/kvm-s390.c         | 19 +++++++++++++++++++
 arch/s390/tools/gen_facilities.c |  1 +
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index eba5bac08348..47104e5b47fd 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -154,7 +154,8 @@ struct kvm_s390_vm_cpu_subfunc {
 	__u8 kma[16];		/* with MSA8 */
 	__u8 kdsa[16];		/* with MSA9 */
 	__u8 sortl[32];		/* with STFLE.150 */
-	__u8 reserved[1760];
+	__u8 dfltcc[32];	/* with STFLE.151 */
+	__u8 reserved[1728];
 };
 
 /* kvm attributes for crypto */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 24b6960d01b4..7dae78ccefc6 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -335,6 +335,7 @@ static inline void __insn32_query(unsigned int opcode, u8 query[32])
 }
 
 #define INSN_SORTL 0xb938
+#define INSN_DFLTCC 0xb939
 
 static void kvm_s390_cpu_feat_init(void)
 {
@@ -390,6 +391,9 @@ static void kvm_s390_cpu_feat_init(void)
 	if (test_facility(150)) /* SORTL */
 		__insn32_query(INSN_SORTL, kvm_s390_available_subfunc.sortl);
 
+	if (test_facility(151)) /* DFLTCC */
+		__insn32_query(INSN_DFLTCC, kvm_s390_available_subfunc.dfltcc);
+
 	if (MACHINE_HAS_ESOP)
 		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
 	/*
@@ -1361,6 +1365,11 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
+	VM_EVENT(kvm, 3, "SET: guest DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[1],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[2],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[3]);
 
 	return 0;
 }
@@ -1537,6 +1546,11 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
 		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
+	VM_EVENT(kvm, 3, "GET: guest DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[0],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[1],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[2],
+		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[3]);
 
 	return 0;
 }
@@ -1600,6 +1614,11 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
 		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[1],
 		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[2],
 		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[3]);
+	VM_EVENT(kvm, 3, "GET: host  DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
+		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[0],
+		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[1],
+		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[2],
+		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[3]);
 
 	return 0;
 }
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index 1ec6bed785e8..cead9e0dcffb 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -94,6 +94,7 @@ static struct facility_def facility_defs[] = {
 			139, /* multiple epoch facility */
 			146, /* msa extension 8 */
 			150, /* enhanced sort */
+			151, /* deflate conversion */
 			155, /* msa extension 9 */
 			-1  /* END */
 		}
-- 
2.19.1

Re: [PATCH 1/7] KVM: s390: add vector enhancements facility 2 to cpumodel

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> If vector support is enabled, the vector enhancements facility 2
> might also be enabled.
> We can directly forward this facility to the guest if available
> and VX is requested by user space.
> 
> Please note that user space can and will have the final decision
> on the facility bits for a guests.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4638303ba6a8..03ddbe5e62bc 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -657,6 +657,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>  				set_kvm_facility(kvm->arch.model.fac_mask, 135);
>  				set_kvm_facility(kvm->arch.model.fac_list, 135);
>  			}
> +			if (test_facility(148)) {
> +				set_kvm_facility(kvm->arch.model.fac_mask, 148);
> +				set_kvm_facility(kvm->arch.model.fac_list, 148);
> +			}
>  			r = 0;
>  		} else
>  			r = -EINVAL;
> 

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 2/7] KVM: s390: add vector BCD enhancements facility to cpumodel

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> If vector support is enabled, the vector BCD enhancements facility
> might also be enabled.
> We can directly forward this facility to the guest if available
> and VX is requested by user space.
> 
> Please note that user space can and will have the final decision
> on the facility bits for a guests.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 03ddbe5e62bc..d3f3e63bb164 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -661,6 +661,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>  				set_kvm_facility(kvm->arch.model.fac_mask, 148);
>  				set_kvm_facility(kvm->arch.model.fac_list, 148);
>  			}
> +			if (test_facility(152)) {
> +				set_kvm_facility(kvm->arch.model.fac_mask, 152);
> +				set_kvm_facility(kvm->arch.model.fac_list, 152);
> +			}
>  			r = 0;
>  		} else
>  			r = -EINVAL;
> 

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 3/7] KVM: s390: add MSA9 to cpumodel

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> This enables stfle.155 and adds the subfunctions for KDSA. Bit 155 is
> added to the list of facilities that will be enabled when there is no
> cpu model involved as MSA9 requires no additional handling from
> userspace, e.g. for migration.
> 
> Please note that a cpu model enabled user space can and will have the
> final decision on the facility bits for a guests.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  Documentation/virtual/kvm/devices/vm.txt |  3 ++-
>  arch/s390/include/asm/cpacf.h            |  1 +
>  arch/s390/include/uapi/asm/kvm.h         |  3 ++-
>  arch/s390/kvm/kvm-s390.c                 | 13 +++++++++++++
>  arch/s390/tools/gen_facilities.c         |  1 +
>  tools/arch/s390/include/uapi/asm/kvm.h   |  3 ++-
>  6 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
> index 95ca68d663a4..4ffb82b02468 100644
> --- a/Documentation/virtual/kvm/devices/vm.txt
> +++ b/Documentation/virtual/kvm/devices/vm.txt
> @@ -141,7 +141,8 @@ struct kvm_s390_vm_cpu_subfunc {
>         u8 pcc[16];           # valid with Message-Security-Assist-Extension 4
>         u8 ppno[16];          # valid with Message-Security-Assist-Extension 5
>         u8 kma[16];           # valid with Message-Security-Assist-Extension 8
> -       u8 reserved[1808];    # reserved for future instructions
> +       u8 kdsa[16];          # valid with Message-Security-Assist-Extension 9
> +       u8 reserved[1792];    # reserved for future instructions
>  };
>  
>  Parameters: address of a buffer to load the subfunction blocks from.
> diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
> index 3cc52e37b4b2..ce2770743cc5 100644
> --- a/arch/s390/include/asm/cpacf.h
> +++ b/arch/s390/include/asm/cpacf.h
> @@ -28,6 +28,7 @@
>  #define CPACF_KMCTR		0xb92d		/* MSA4 */
>  #define CPACF_PRNO		0xb93c		/* MSA5 */
>  #define CPACF_KMA		0xb929		/* MSA8 */
> +#define CPACF_KDSA		0xb93a		/* MSA9 */
>  
>  /*
>   * En/decryption modifier bits
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 16511d97e8dc..09652eabe769 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -152,7 +152,8 @@ struct kvm_s390_vm_cpu_subfunc {
>  	__u8 pcc[16];		/* with MSA4 */
>  	__u8 ppno[16];		/* with MSA5 */
>  	__u8 kma[16];		/* with MSA8 */
> -	__u8 reserved[1808];
> +	__u8 kdsa[16];		/* with MSA9 */
> +	__u8 reserved[1792];
>  };
>  
>  /* kvm attributes for crypto */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d3f3e63bb164..0dad61ccde3d 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -368,6 +368,10 @@ static void kvm_s390_cpu_feat_init(void)
>  		__cpacf_query(CPACF_KMA, (cpacf_mask_t *)
>  			      kvm_s390_available_subfunc.kma);
>  
> +	if (test_facility(155)) /* MSA9 */
> +		__cpacf_query(CPACF_KDSA, (cpacf_mask_t *)
> +			      kvm_s390_available_subfunc.kdsa);
> +
>  	if (MACHINE_HAS_ESOP)
>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>  	/*
> @@ -1331,6 +1335,9 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "SET: guest KMA    subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[0],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[1]);
> +	VM_EVENT(kvm, 3, "SET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
>  
>  	return 0;
>  }
> @@ -1499,6 +1506,9 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "GET: guest KMA    subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[0],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kma)[1]);
> +	VM_EVENT(kvm, 3, "GET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
>  
>  	return 0;
>  }
> @@ -1554,6 +1564,9 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "GET: host  KMA    subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm_s390_available_subfunc.kma)[0],
>  		 ((unsigned long *) &kvm_s390_available_subfunc.kma)[1]);
> +	VM_EVENT(kvm, 3, "GET: host  KDSA   subfunc 0x%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[0],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[1]);
>  
>  	return 0;
>  }
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index fd788e0f2e5b..e952cb3b75b2 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -93,6 +93,7 @@ static struct facility_def facility_defs[] = {
>  			131, /* enhanced-SOP 2 and side-effect */
>  			139, /* multiple epoch facility */
>  			146, /* msa extension 8 */
> +			155, /* msa extension 9 */
>  			-1  /* END */
>  		}
>  	},
> diff --git a/tools/arch/s390/include/uapi/asm/kvm.h b/tools/arch/s390/include/uapi/asm/kvm.h
> index 16511d97e8dc..09652eabe769 100644
> --- a/tools/arch/s390/include/uapi/asm/kvm.h
> +++ b/tools/arch/s390/include/uapi/asm/kvm.h
> @@ -152,7 +152,8 @@ struct kvm_s390_vm_cpu_subfunc {
>  	__u8 pcc[16];		/* with MSA4 */
>  	__u8 ppno[16];		/* with MSA5 */
>  	__u8 kma[16];		/* with MSA8 */
> -	__u8 reserved[1808];
> +	__u8 kdsa[16];		/* with MSA9 */
> +	__u8 reserved[1792];
>  };
>  
>  /* kvm attributes for crypto */
> 

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> Instead of adding a new machine option to disable/enable the keywrapping
> options of pckmo (like for AES and DEA) we can now use the CPU model to
> decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h | 1 +
>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index c47e22bba87f..e224246ff93c 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>  #define ECD_HOSTREGMGMT	0x20000000
>  #define ECD_MEF		0x08000000
>  #define ECD_ETOKENF	0x02000000
> +#define ECD_ECC		0x00200000
>  	__u32	ecd;			/* 0x01c8 */
>  	__u8	reserved1cc[18];	/* 0x01cc */
>  	__u64	pp;			/* 0x01de */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0dad61ccde3d..ff2444d935fd 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>  	}
> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
> +		vcpu->arch.sie_block->ecd |= ECD_ECC;

I see, that's why you sent the patch to remember this.

vsie?

If I am not wrong, you should allow to set ECD_ECC there similarly, if
the configured cpu model has these facilitites.

>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>  					| SDNXC;
>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH 5/7] KVM: s390: provide query function for instructions returning 32 byte

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> Some of the new features have a 32byte response for the query function.
> Provide a new wrapper similar to __cpacf_query. We might want to factor
> this out if other users come up, as of today there is none. So let us
> keep the function within KVM.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ff2444d935fd..f0404960a715 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -321,6 +321,19 @@ static inline int plo_test_bit(unsigned char nr)
>  	return cc == 0;
>  }
>  
> +static inline void __insn32_query(unsigned int opcode, u8 query[32])
> +{
> +	register unsigned long r0 asm("0") = 0;	/* query function */
> +	register unsigned long r1 asm("1") = (unsigned long) query;
> +
> +	asm volatile(
> +		/* Parameter regs are ignored */
> +		"	.insn	rrf,%[opc] << 16,2,4,6,0\n"
> +		: "=m" (*query)
> +		: "d" (r0), "a" (r1), [opc] "i" (opcode)
> +		: "cc");
> +}
> +
>  static void kvm_s390_cpu_feat_init(void)
>  {
>  	int i;
> 

Are these !cpacf functions, so the generic name "__insn32" is preferred
over e.g. __cpacf_query32 ?

Looks sane to me.

-- 

Thanks,

David / dhildenb

Re: [PATCH 5/7] KVM: s390: provide query function for instructions returning 32 byte

[Christian Borntraeger]

On 17.04.19 17:42, David Hildenbrand wrote:
> On 17.04.19 17:28, Christian Borntraeger wrote:
>> Some of the new features have a 32byte response for the query function.
>> Provide a new wrapper similar to __cpacf_query. We might want to factor
>> this out if other users come up, as of today there is none. So let us
>> keep the function within KVM.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  arch/s390/kvm/kvm-s390.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index ff2444d935fd..f0404960a715 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -321,6 +321,19 @@ static inline int plo_test_bit(unsigned char nr)
>>  	return cc == 0;
>>  }
>>  
>> +static inline void __insn32_query(unsigned int opcode, u8 query[32])
>> +{
>> +	register unsigned long r0 asm("0") = 0;	/* query function */
>> +	register unsigned long r1 asm("1") = (unsigned long) query;
>> +
>> +	asm volatile(
>> +		/* Parameter regs are ignored */
>> +		"	.insn	rrf,%[opc] << 16,2,4,6,0\n"
>> +		: "=m" (*query)
>> +		: "d" (r0), "a" (r1), [opc] "i" (opcode)
>> +		: "cc");
>> +}
>> +
>>  static void kvm_s390_cpu_feat_init(void)
>>  {
>>  	int i;
>>
> 
> Are these !cpacf functions, so the generic name "__insn32" is preferred
> over e.g. __cpacf_query32 ?

yes, not cpacf.

> 
> Looks sane to me.
> 

Re: [PATCH 6/7] KVM: s390: add enhanced sort facilty to cpu model

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> This enables stfle.150 and adds the subfunctions for SORTL. Bit 150 is
> added to the list of facilities that will be enabled when there is no
> cpu model involved as sortl requires no additional handling from
> userspace, e.g. for migration.
> 
> Please note that a cpu model enabled user space can and will have the
> final decision on the facility bits for a guests.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/include/uapi/asm/kvm.h |  3 ++-
>  arch/s390/kvm/kvm-s390.c         | 20 ++++++++++++++++++++
>  arch/s390/tools/gen_facilities.c |  1 +
>  3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 09652eabe769..eba5bac08348 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -153,7 +153,8 @@ struct kvm_s390_vm_cpu_subfunc {
>  	__u8 ppno[16];		/* with MSA5 */
>  	__u8 kma[16];		/* with MSA8 */
>  	__u8 kdsa[16];		/* with MSA9 */
> -	__u8 reserved[1792];
> +	__u8 sortl[32];		/* with STFLE.150 */
> +	__u8 reserved[1760];
>  };
>  
>  /* kvm attributes for crypto */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index f0404960a715..24b6960d01b4 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -334,6 +334,8 @@ static inline void __insn32_query(unsigned int opcode, u8 query[32])
>  		: "cc");
>  }
>  
> +#define INSN_SORTL 0xb938
> +
>  static void kvm_s390_cpu_feat_init(void)
>  {
>  	int i;
> @@ -385,6 +387,9 @@ static void kvm_s390_cpu_feat_init(void)
>  		__cpacf_query(CPACF_KDSA, (cpacf_mask_t *)
>  			      kvm_s390_available_subfunc.kdsa);
>  
> +	if (test_facility(150)) /* SORTL */
> +		__insn32_query(INSN_SORTL, kvm_s390_available_subfunc.sortl);
> +
>  	if (MACHINE_HAS_ESOP)
>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>  	/*
> @@ -1351,6 +1356,11 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "SET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
> +	VM_EVENT(kvm, 3, "SET: guest SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
>  
>  	return 0;
>  }
> @@ -1522,6 +1532,11 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "GET: guest KDSA   subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[0],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.kdsa)[1]);
> +	VM_EVENT(kvm, 3, "GET: guest SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
>  
>  	return 0;
>  }
> @@ -1580,6 +1595,11 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
>  	VM_EVENT(kvm, 3, "GET: host  KDSA   subfunc 0x%16.16lx.%16.16lx",
>  		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[0],
>  		 ((unsigned long *) &kvm_s390_available_subfunc.kdsa)[1]);
> +	VM_EVENT(kvm, 3, "GET: host  SORTL  subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[0],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[1],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[2],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[3]);
>  
>  	return 0;
>  }
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index e952cb3b75b2..1ec6bed785e8 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -93,6 +93,7 @@ static struct facility_def facility_defs[] = {
>  			131, /* enhanced-SOP 2 and side-effect */
>  			139, /* multiple epoch facility */
>  			146, /* msa extension 8 */
> +			150, /* enhanced sort */
>  			155, /* msa extension 9 */
>  			-1  /* END */
>  		}
> 

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 7/7] KVM: s390: add deflate conversion facilty to cpu model

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> This enables stfle.151 and adds the subfunctions for DFLTCC
> 
> This enables stfle.151 and adds the subfunctions for DFLTCC. Bit 151 is
> added to the list of facilities that will be enabled when there is no
> cpu model involved as DFLTCC requires no additional handling from
> userspace, e.g. for migration.
> 
> Please note that a cpu model enabled user space can and will have the
> final decision on the facility bits for a guests.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/include/uapi/asm/kvm.h |  3 ++-
>  arch/s390/kvm/kvm-s390.c         | 19 +++++++++++++++++++
>  arch/s390/tools/gen_facilities.c |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index eba5bac08348..47104e5b47fd 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -154,7 +154,8 @@ struct kvm_s390_vm_cpu_subfunc {
>  	__u8 kma[16];		/* with MSA8 */
>  	__u8 kdsa[16];		/* with MSA9 */
>  	__u8 sortl[32];		/* with STFLE.150 */
> -	__u8 reserved[1760];
> +	__u8 dfltcc[32];	/* with STFLE.151 */
> +	__u8 reserved[1728];
>  };
>  
>  /* kvm attributes for crypto */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 24b6960d01b4..7dae78ccefc6 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -335,6 +335,7 @@ static inline void __insn32_query(unsigned int opcode, u8 query[32])
>  }
>  
>  #define INSN_SORTL 0xb938
> +#define INSN_DFLTCC 0xb939
>  
>  static void kvm_s390_cpu_feat_init(void)
>  {
> @@ -390,6 +391,9 @@ static void kvm_s390_cpu_feat_init(void)
>  	if (test_facility(150)) /* SORTL */
>  		__insn32_query(INSN_SORTL, kvm_s390_available_subfunc.sortl);
>  
> +	if (test_facility(151)) /* DFLTCC */
> +		__insn32_query(INSN_DFLTCC, kvm_s390_available_subfunc.dfltcc);
> +
>  	if (MACHINE_HAS_ESOP)
>  		allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>  	/*
> @@ -1361,6 +1365,11 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
> +	VM_EVENT(kvm, 3, "SET: guest DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[1],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[2],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[3]);
>  
>  	return 0;
>  }
> @@ -1537,6 +1546,11 @@ static int kvm_s390_get_processor_subfunc(struct kvm *kvm,
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[1],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[2],
>  		 ((unsigned long *) &kvm->arch.model.subfuncs.sortl)[3]);
> +	VM_EVENT(kvm, 3, "GET: guest DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[0],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[1],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[2],
> +		 ((unsigned long *) &kvm->arch.model.subfuncs.dfltcc)[3]);
>  
>  	return 0;
>  }
> @@ -1600,6 +1614,11 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm,
>  		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[1],
>  		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[2],
>  		 ((unsigned long *) &kvm_s390_available_subfunc.sortl)[3]);
> +	VM_EVENT(kvm, 3, "GET: host  DFLTCC subfunc 0x%16.16lx.%16.16lx.%16.16lx.%16.16lx",
> +		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[0],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[1],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[2],
> +		 ((unsigned long *) &kvm_s390_available_subfunc.dfltcc)[3]);
>  
>  	return 0;
>  }
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 1ec6bed785e8..cead9e0dcffb 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -94,6 +94,7 @@ static struct facility_def facility_defs[] = {
>  			139, /* multiple epoch facility */
>  			146, /* msa extension 8 */
>  			150, /* enhanced sort */
> +			151, /* deflate conversion */
>  			155, /* msa extension 9 */
>  			-1  /* END */
>  		}
> 

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 5/7] KVM: s390: provide query function for instructions returning 32 byte

[David Hildenbrand]

On 17.04.19 17:43, Christian Borntraeger wrote:
> 
> 
> On 17.04.19 17:42, David Hildenbrand wrote:
>> On 17.04.19 17:28, Christian Borntraeger wrote:
>>> Some of the new features have a 32byte response for the query function.
>>> Provide a new wrapper similar to __cpacf_query. We might want to factor
>>> this out if other users come up, as of today there is none. So let us
>>> keep the function within KVM.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>> ---
>>>  arch/s390/kvm/kvm-s390.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index ff2444d935fd..f0404960a715 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -321,6 +321,19 @@ static inline int plo_test_bit(unsigned char nr)
>>>  	return cc == 0;
>>>  }
>>>  
>>> +static inline void __insn32_query(unsigned int opcode, u8 query[32])
>>> +{
>>> +	register unsigned long r0 asm("0") = 0;	/* query function */
>>> +	register unsigned long r1 asm("1") = (unsigned long) query;
>>> +
>>> +	asm volatile(
>>> +		/* Parameter regs are ignored */
>>> +		"	.insn	rrf,%[opc] << 16,2,4,6,0\n"
>>> +		: "=m" (*query)
>>> +		: "d" (r0), "a" (r1), [opc] "i" (opcode)
>>> +		: "cc");
>>> +}
>>> +
>>>  static void kvm_s390_cpu_feat_init(void)
>>>  {
>>>  	int i;
>>>
>>
>> Are these !cpacf functions, so the generic name "__insn32" is preferred
>> over e.g. __cpacf_query32 ?
> 
> yes, not cpacf.

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

-- 

Thanks,

David / dhildenb

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 17.04.19 17:28, Christian Borntraeger wrote:
> Instead of adding a new machine option to disable/enable the keywrapping
> options of pckmo (like for AES and DEA) we can now use the CPU model to
> decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h | 1 +
>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index c47e22bba87f..e224246ff93c 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>  #define ECD_HOSTREGMGMT	0x20000000
>  #define ECD_MEF		0x08000000
>  #define ECD_ETOKENF	0x02000000
> +#define ECD_ECC		0x00200000
>  	__u32	ecd;			/* 0x01c8 */
>  	__u8	reserved1cc[18];	/* 0x01cc */
>  	__u64	pp;			/* 0x01de */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0dad61ccde3d..ff2444d935fd 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>  	}
> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>  					| SDNXC;
>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
> 
Just noting here that user space can set pckmo and friends even though
HW does not support it (if I remember correctly), resulting in ECD_ECC
being able to be set from user space although not supported. Is that
sane? Shouldn't we somehow check if the HW actually supports it?

(features should only be used if the indication is on, not because it
"might" work)

-- 

Thanks,

David / dhildenb

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 17.04.19 17:38, David Hildenbrand wrote:
> On 17.04.19 17:28, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index c47e22bba87f..e224246ff93c 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>  #define ECD_HOSTREGMGMT	0x20000000
>>  #define ECD_MEF		0x08000000
>>  #define ECD_ETOKENF	0x02000000
>> +#define ECD_ECC		0x00200000
>>  	__u32	ecd;			/* 0x01c8 */
>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>  	__u64	pp;			/* 0x01de */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0dad61ccde3d..ff2444d935fd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>  	}
>> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
> 
> I see, that's why you sent the patch to remember this.
> 
> vsie?

i split this out because this is different from what we did with KVM_S390_VM_CRYPTO_ENABLE_AES_KW
and friends. (we had no cpu model back then).

But you are right. I will look into vsie, should be straightforward.
> 
> If I am not wrong, you should allow to set ECD_ECC there similarly, if
> the configured cpu model has these facilitites.
> 
>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>  					| SDNXC;
>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>
> 
> 

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 17.04.19 17:47, David Hildenbrand wrote:
> On 17.04.19 17:28, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index c47e22bba87f..e224246ff93c 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>  #define ECD_HOSTREGMGMT	0x20000000
>>  #define ECD_MEF		0x08000000
>>  #define ECD_ETOKENF	0x02000000
>> +#define ECD_ECC		0x00200000
>>  	__u32	ecd;			/* 0x01c8 */
>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>  	__u64	pp;			/* 0x01de */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0dad61ccde3d..ff2444d935fd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>  	}
>> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>  					| SDNXC;
>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>
> Just noting here that user space can set pckmo and friends even though
> HW does not support it (if I remember correctly), resulting in ECD_ECC
> being able to be set from user space although not supported. Is that
> sane? Shouldn't we somehow check if the HW actually supports it?
> 
> (features should only be used if the indication is on, not because it
> "might" work)

For those machines ECD_ECC will be ignored, but yes, adding an additional check
that we have these in the real machine might be safer.

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 17.04.19 17:47, David Hildenbrand wrote:
> On 17.04.19 17:28, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index c47e22bba87f..e224246ff93c 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>  #define ECD_HOSTREGMGMT	0x20000000
>>  #define ECD_MEF		0x08000000
>>  #define ECD_ETOKENF	0x02000000
>> +#define ECD_ECC		0x00200000
>>  	__u32	ecd;			/* 0x01c8 */
>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>  	__u64	pp;			/* 0x01de */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0dad61ccde3d..ff2444d935fd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>  	}
>> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>  					| SDNXC;
>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>
> Just noting here that user space can set pckmo and friends even though
> HW does not support it (if I remember correctly), resulting in ECD_ECC
> being able to be set from user space although not supported. Is that
> sane? Shouldn't we somehow check if the HW actually supports it?

untested patch, something like this on top

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 7dae78ccefc6..dadd1105b5a4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2985,9 +2985,14 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
                VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
                           vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
        }
-       /* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
-       if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
-           (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
+       /*
+        * if any of 32,33,34,40,41 is active in host AND guest,
+        * we enable pckmo for ecc
+        */
+       if (((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
+            (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0)) &&
+           ((kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
+            (kvm_s390_available_subfunc.pckmo[5] & 0xc0)))
                vcpu->arch.sie_block->ecd |= ECD_ECC;
        vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
                                        | SDNXC;


> 
> (features should only be used if the indication is on, not because it
> "might" work)
> 

Re: [PATCH 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 17.04.19 17:48, Christian Borntraeger wrote:
> 
> 
> On 17.04.19 17:38, David Hildenbrand wrote:
>> On 17.04.19 17:28, Christian Borntraeger wrote:
>>> Instead of adding a new machine option to disable/enable the keywrapping
>>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>>> decide.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>> ---
>>>  arch/s390/include/asm/kvm_host.h | 1 +
>>>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>>>  2 files changed, 5 insertions(+)
>>>
>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>> index c47e22bba87f..e224246ff93c 100644
>>> --- a/arch/s390/include/asm/kvm_host.h
>>> +++ b/arch/s390/include/asm/kvm_host.h
>>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>>  #define ECD_HOSTREGMGMT	0x20000000
>>>  #define ECD_MEF		0x08000000
>>>  #define ECD_ETOKENF	0x02000000
>>> +#define ECD_ECC		0x00200000
>>>  	__u32	ecd;			/* 0x01c8 */
>>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>>  	__u64	pp;			/* 0x01de */
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 0dad61ccde3d..ff2444d935fd 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>>  	}
>>> +	/* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
>>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
>>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
>>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>
>> I see, that's why you sent the patch to remember this.
>>
>> vsie?
> 
> i split this out because this is different from what we did with KVM_S390_VM_CRYPTO_ENABLE_AES_KW
> and friends. (we had no cpu model back then).
> 
> But you are right. I will look into vsie, should be straightforward.


untested but something like this on top.

diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index d62fa148558b..d0510d8c77e0 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
        const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
        unsigned long *b1, *b2;
        u8 ecb3_flags;
+       u8 ecd_flags;
        int apie_h;
        int key_msk = test_kvm_facility(vcpu->kvm, 76);
        int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
@@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
        /* we may only allow it if enabled for guest 2 */
        ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
                     (ECB3_AES | ECB3_DEA);
-       if (!ecb3_flags)
+       ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
+       if (!ecb3_flags && !ecbflags)
                goto end;
 
        /* copy only the wrapping keys */
@@ -329,6 +331,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
                return set_validity_icpt(scb_s, 0x0035U);
 
        scb_s->ecb3 |= ecb3_flags;
+       scb_s->ecd |= ecd_flags;
 
        /* xor both blocks in one run */
        b1 = (unsigned long *) vsie_page->crycb.dea_wrapping_key_mask;

[PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

Instead of adding a new machine option to disable/enable the keywrapping
options of pckmo (like for AES and DEA) we can now use the CPU model to
decide.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
---
v1->v2: - enable vsie
	- also check if the host has the pckmo functions
 arch/s390/include/asm/kvm_host.h | 1 +
 arch/s390/kvm/kvm-s390.c         | 7 +++++++
 arch/s390/kvm/vsie.c             | 5 ++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index c47e22bba87fa..e224246ff93c6 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
 #define ECD_HOSTREGMGMT	0x20000000
 #define ECD_MEF		0x08000000
 #define ECD_ETOKENF	0x02000000
+#define ECD_ECC		0x00200000
 	__u32	ecd;			/* 0x01c8 */
 	__u8	reserved1cc[18];	/* 0x01cc */
 	__u64	pp;			/* 0x01de */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0dad61ccde3d6..9869d785677f1 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
 			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
 	}
+	/*
+	 * if any of 32,33,34,40,41 is active in host AND guest,
+	 * we enable pckmo for ecc
+	 */
+	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
+	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
+		vcpu->arch.sie_block->ecd |= ECD_ECC;
 	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
 					| SDNXC;
 	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index d62fa148558b9..c6983d962abfd 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
 	unsigned long *b1, *b2;
 	u8 ecb3_flags;
+	u32 ecd_flags;
 	int apie_h;
 	int key_msk = test_kvm_facility(vcpu->kvm, 76);
 	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
@@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	/* we may only allow it if enabled for guest 2 */
 	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
 		     (ECB3_AES | ECB3_DEA);
-	if (!ecb3_flags)
+	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
+	if (!ecb3_flags && !ecd_flags)
 		goto end;
 
 	/* copy only the wrapping keys */
@@ -329,6 +331,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		return set_validity_icpt(scb_s, 0x0035U);
 
 	scb_s->ecb3 |= ecb3_flags;
+	scb_s->ecd |= ecd_flags;
 
 	/* xor both blocks in one run */
 	b1 = (unsigned long *) vsie_page->crycb.dea_wrapping_key_mask;
-- 
2.19.1

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 17.04.19 20:29, Christian Borntraeger wrote:
> Instead of adding a new machine option to disable/enable the keywrapping
> options of pckmo (like for AES and DEA) we can now use the CPU model to
> decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
> v1->v2: - enable vsie
> 	- also check if the host has the pckmo functions
>  arch/s390/include/asm/kvm_host.h | 1 +
>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>  arch/s390/kvm/vsie.c             | 5 ++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)

FWIW, I tested this variant successfully with some printk debugging to 
check if the settings are good. The only question is: does anybody cares
about
	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))

being too long? I find this more readable than 

	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] &
	     kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] &
	     kvm_s390_available_subfunc.pckmo[5] & 0xc0))

Christian

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 18.04.19 09:35, Christian Borntraeger wrote:
> On 17.04.19 20:29, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>> v1->v2: - enable vsie
>> 	- also check if the host has the pckmo functions
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> FWIW, I tested this variant successfully with some printk debugging to 
> check if the settings are good. The only question is: does anybody cares
> about
> 	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
> 	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
> 
> being too long? I find this more readable than 
> 
> 	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] &
> 	     kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
> 	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] &
> 	     kvm_s390_available_subfunc.pckmo[5] & 0xc0))
> 

Can you just factor that out into a function / makro?

> Christian
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 17.04.19 20:29, Christian Borntraeger wrote:
> Instead of adding a new machine option to disable/enable the keywrapping
> options of pckmo (like for AES and DEA) we can now use the CPU model to
> decide.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> ---
> v1->v2: - enable vsie
> 	- also check if the host has the pckmo functions
>  arch/s390/include/asm/kvm_host.h | 1 +
>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>  arch/s390/kvm/vsie.c             | 5 ++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index c47e22bba87fa..e224246ff93c6 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>  #define ECD_HOSTREGMGMT	0x20000000
>  #define ECD_MEF		0x08000000
>  #define ECD_ETOKENF	0x02000000
> +#define ECD_ECC		0x00200000
>  	__u32	ecd;			/* 0x01c8 */
>  	__u8	reserved1cc[18];	/* 0x01cc */
>  	__u64	pp;			/* 0x01de */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0dad61ccde3d6..9869d785677f1 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>  	}
> +	/*
> +	 * if any of 32,33,34,40,41 is active in host AND guest,
> +	 * we enable pckmo for ecc
> +	 */
> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))

Maybe some helper like

bool kvm_has_pckmo_subfunc(kvm, nr)
{
	/* magic for one number */
}
...

if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
...

then you can also get rid of the comment.

> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>  					| SDNXC;
>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index d62fa148558b9..c6983d962abfd 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>  	unsigned long *b1, *b2;
>  	u8 ecb3_flags;
> +	u32 ecd_flags;
>  	int apie_h;
>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	/* we may only allow it if enabled for guest 2 */
>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>  		     (ECB3_AES | ECB3_DEA);
> -	if (!ecb3_flags)
> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
> +	if (!ecb3_flags && !ecd_flags)
>  		goto end;

Just so I get it right, there are no *new* wrapping keys? Which wrapping
keys are used then?

>  
>  	/* copy only the wrapping keys */
> @@ -329,6 +331,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  		return set_validity_icpt(scb_s, 0x0035U);
>  
>  	scb_s->ecb3 |= ecb3_flags;
> +	scb_s->ecd |= ecd_flags;
>  
>  	/* xor both blocks in one run */
>  	b1 = (unsigned long *) vsie_page->crycb.dea_wrapping_key_mask;
> 


-- 

Thanks,

David / dhildenb

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 18.04.19 09:54, David Hildenbrand wrote:
> On 17.04.19 20:29, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>> v1->v2: - enable vsie
>> 	- also check if the host has the pckmo functions
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>  3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index c47e22bba87fa..e224246ff93c6 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>  #define ECD_HOSTREGMGMT	0x20000000
>>  #define ECD_MEF		0x08000000
>>  #define ECD_ETOKENF	0x02000000
>> +#define ECD_ECC		0x00200000
>>  	__u32	ecd;			/* 0x01c8 */
>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>  	__u64	pp;			/* 0x01de */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0dad61ccde3d6..9869d785677f1 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>  	}
>> +	/*
>> +	 * if any of 32,33,34,40,41 is active in host AND guest,
>> +	 * we enable pckmo for ecc
>> +	 */
>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
> 
> Maybe some helper like
> 
> bool kvm_has_pckmo_subfunc(kvm, nr)
> {
> 	/* magic for one number */
> }
> ...
> 
> if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
> ...
> 
> then you can also get rid of the comment.

Will give it a try.
> 
>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>  					| SDNXC;
>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index d62fa148558b9..c6983d962abfd 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>  	unsigned long *b1, *b2;
>>  	u8 ecb3_flags;
>> +	u32 ecd_flags;
>>  	int apie_h;
>>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
>> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  	/* we may only allow it if enabled for guest 2 */
>>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>>  		     (ECB3_AES | ECB3_DEA);
>> -	if (!ecb3_flags)
>> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
>> +	if (!ecb3_flags && !ecd_flags)
>>  		goto end;
> 
> Just so I get it right, there are no *new* wrapping keys? Which wrapping
> keys are used then?

Yes, AES.

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 18.04.19 10:58, Christian Borntraeger wrote:
> 
> 
> On 18.04.19 09:54, David Hildenbrand wrote:
>> On 17.04.19 20:29, Christian Borntraeger wrote:
>>> Instead of adding a new machine option to disable/enable the keywrapping
>>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>>> decide.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>> ---
>>> v1->v2: - enable vsie
>>> 	- also check if the host has the pckmo functions
>>>  arch/s390/include/asm/kvm_host.h | 1 +
>>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>>  3 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>> index c47e22bba87fa..e224246ff93c6 100644
>>> --- a/arch/s390/include/asm/kvm_host.h
>>> +++ b/arch/s390/include/asm/kvm_host.h
>>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>>  #define ECD_HOSTREGMGMT	0x20000000
>>>  #define ECD_MEF		0x08000000
>>>  #define ECD_ETOKENF	0x02000000
>>> +#define ECD_ECC		0x00200000
>>>  	__u32	ecd;			/* 0x01c8 */
>>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>>  	__u64	pp;			/* 0x01de */
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 0dad61ccde3d6..9869d785677f1 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>>  	}
>>> +	/*
>>> +	 * if any of 32,33,34,40,41 is active in host AND guest,
>>> +	 * we enable pckmo for ecc
>>> +	 */
>>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
>>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
>>
>> Maybe some helper like
>>
>> bool kvm_has_pckmo_subfunc(kvm, nr)
>> {
>> 	/* magic for one number */
>> }
>> ...
>>
>> if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
>> ...
>>
>> then you can also get rid of the comment.
> 
> Will give it a try.
>>
>>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>>  					| SDNXC;
>>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>> index d62fa148558b9..c6983d962abfd 100644
>>> --- a/arch/s390/kvm/vsie.c
>>> +++ b/arch/s390/kvm/vsie.c
>>> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>>  	unsigned long *b1, *b2;
>>>  	u8 ecb3_flags;
>>> +	u32 ecd_flags;
>>>  	int apie_h;
>>>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>>>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
>>> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>  	/* we may only allow it if enabled for guest 2 */
>>>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>>>  		     (ECB3_AES | ECB3_DEA);
>>> -	if (!ecb3_flags)
>>> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
>>> +	if (!ecb3_flags && !ecd_flags)
>>>  		goto end;
>>
>> Just so I get it right, there are no *new* wrapping keys? Which wrapping
>> keys are used then?
> 
> Yes, AES.
> 

Hmmmm, so if user space doesn't call KVM_S390_VM_CRYPTO_ENABLE_AES_KW,
the wrapping key is basically uninitialized (kvm_s390_vm_set_crypto()),
but will be used.

I guess you should also check against kvm->arch.crypto.aes_kw before
turning the ecd bit on, just to be sure.

-- 

Thanks,

David / dhildenb

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 18.04.19 11:13, David Hildenbrand wrote:
> On 18.04.19 10:58, Christian Borntraeger wrote:
>>
>>
>> On 18.04.19 09:54, David Hildenbrand wrote:
>>> On 17.04.19 20:29, Christian Borntraeger wrote:
>>>> Instead of adding a new machine option to disable/enable the keywrapping
>>>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>>>> decide.
>>>>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>>> ---
>>>> v1->v2: - enable vsie
>>>> 	- also check if the host has the pckmo functions
>>>>  arch/s390/include/asm/kvm_host.h | 1 +
>>>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>>>  3 files changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>>> index c47e22bba87fa..e224246ff93c6 100644
>>>> --- a/arch/s390/include/asm/kvm_host.h
>>>> +++ b/arch/s390/include/asm/kvm_host.h
>>>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>>>  #define ECD_HOSTREGMGMT	0x20000000
>>>>  #define ECD_MEF		0x08000000
>>>>  #define ECD_ETOKENF	0x02000000
>>>> +#define ECD_ECC		0x00200000
>>>>  	__u32	ecd;			/* 0x01c8 */
>>>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>>>  	__u64	pp;			/* 0x01de */
>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>> index 0dad61ccde3d6..9869d785677f1 100644
>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>>>  	}
>>>> +	/*
>>>> +	 * if any of 32,33,34,40,41 is active in host AND guest,
>>>> +	 * we enable pckmo for ecc
>>>> +	 */
>>>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
>>>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
>>>
>>> Maybe some helper like
>>>
>>> bool kvm_has_pckmo_subfunc(kvm, nr)
>>> {
>>> 	/* magic for one number */
>>> }
>>> ...
>>>
>>> if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
>>> ...
>>>
>>> then you can also get rid of the comment.
>>
>> Will give it a try.
>>>
>>>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>>>  					| SDNXC;
>>>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>>> index d62fa148558b9..c6983d962abfd 100644
>>>> --- a/arch/s390/kvm/vsie.c
>>>> +++ b/arch/s390/kvm/vsie.c
>>>> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>>>  	unsigned long *b1, *b2;
>>>>  	u8 ecb3_flags;
>>>> +	u32 ecd_flags;
>>>>  	int apie_h;
>>>>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>>>>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
>>>> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>  	/* we may only allow it if enabled for guest 2 */
>>>>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>>>>  		     (ECB3_AES | ECB3_DEA);
>>>> -	if (!ecb3_flags)
>>>> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
>>>> +	if (!ecb3_flags && !ecd_flags)
>>>>  		goto end;
>>>
>>> Just so I get it right, there are no *new* wrapping keys? Which wrapping
>>> keys are used then?
>>
>> Yes, AES.
>>
> 
> Hmmmm, so if user space doesn't call KVM_S390_VM_CRYPTO_ENABLE_AES_KW,
> the wrapping key is basically uninitialized (kvm_s390_vm_set_crypto()),
> but will be used.
> 
> I guess you should also check against kvm->arch.crypto.aes_kw before
> turning the ecd bit on, just to be sure.

We should rather initialize the aes value when ecc wrapping is enabled.

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[David Hildenbrand]

On 18.04.19 12:17, Christian Borntraeger wrote:
> 
> 
> On 18.04.19 11:13, David Hildenbrand wrote:
>> On 18.04.19 10:58, Christian Borntraeger wrote:
>>>
>>>
>>> On 18.04.19 09:54, David Hildenbrand wrote:
>>>> On 17.04.19 20:29, Christian Borntraeger wrote:
>>>>> Instead of adding a new machine option to disable/enable the keywrapping
>>>>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>>>>> decide.
>>>>>
>>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>>>> ---
>>>>> v1->v2: - enable vsie
>>>>> 	- also check if the host has the pckmo functions
>>>>>  arch/s390/include/asm/kvm_host.h | 1 +
>>>>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>>>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>>>>  3 files changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>>>> index c47e22bba87fa..e224246ff93c6 100644
>>>>> --- a/arch/s390/include/asm/kvm_host.h
>>>>> +++ b/arch/s390/include/asm/kvm_host.h
>>>>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>>>>  #define ECD_HOSTREGMGMT	0x20000000
>>>>>  #define ECD_MEF		0x08000000
>>>>>  #define ECD_ETOKENF	0x02000000
>>>>> +#define ECD_ECC		0x00200000
>>>>>  	__u32	ecd;			/* 0x01c8 */
>>>>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>>>>  	__u64	pp;			/* 0x01de */
>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>> index 0dad61ccde3d6..9869d785677f1 100644
>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>>>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>>>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>>>>  	}
>>>>> +	/*
>>>>> +	 * if any of 32,33,34,40,41 is active in host AND guest,
>>>>> +	 * we enable pckmo for ecc
>>>>> +	 */
>>>>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
>>>>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
>>>>
>>>> Maybe some helper like
>>>>
>>>> bool kvm_has_pckmo_subfunc(kvm, nr)
>>>> {
>>>> 	/* magic for one number */
>>>> }
>>>> ...
>>>>
>>>> if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
>>>> ...
>>>>
>>>> then you can also get rid of the comment.
>>>
>>> Will give it a try.
>>>>
>>>>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>>>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>>>>  					| SDNXC;
>>>>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>>>> index d62fa148558b9..c6983d962abfd 100644
>>>>> --- a/arch/s390/kvm/vsie.c
>>>>> +++ b/arch/s390/kvm/vsie.c
>>>>> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>>>>  	unsigned long *b1, *b2;
>>>>>  	u8 ecb3_flags;
>>>>> +	u32 ecd_flags;
>>>>>  	int apie_h;
>>>>>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>>>>>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
>>>>> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>>  	/* we may only allow it if enabled for guest 2 */
>>>>>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>>>>>  		     (ECB3_AES | ECB3_DEA);
>>>>> -	if (!ecb3_flags)
>>>>> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
>>>>> +	if (!ecb3_flags && !ecd_flags)
>>>>>  		goto end;
>>>>
>>>> Just so I get it right, there are no *new* wrapping keys? Which wrapping
>>>> keys are used then?
>>>
>>> Yes, AES.
>>>
>>
>> Hmmmm, so if user space doesn't call KVM_S390_VM_CRYPTO_ENABLE_AES_KW,
>> the wrapping key is basically uninitialized (kvm_s390_vm_set_crypto()),
>> but will be used.
>>
>> I guess you should also check against kvm->arch.crypto.aes_kw before
>> turning the ecd bit on, just to be sure.
> 
> We should rather initialize the aes value when ecc wrapping is enabled.
> 

If you don't glue this to KVM_S390_VM_CRYPTO_ENABLE_AES_KW, you will
have to find another way to reset AES keys during resets.

-- 

Thanks,

David / dhildenb

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 18.04.19 12:31, David Hildenbrand wrote:
> On 18.04.19 12:17, Christian Borntraeger wrote:
>>
>>
>> On 18.04.19 11:13, David Hildenbrand wrote:
>>> On 18.04.19 10:58, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 18.04.19 09:54, David Hildenbrand wrote:
>>>>> On 17.04.19 20:29, Christian Borntraeger wrote:
>>>>>> Instead of adding a new machine option to disable/enable the keywrapping
>>>>>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>>>>>> decide.
>>>>>>
>>>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>>>>>> ---
>>>>>> v1->v2: - enable vsie
>>>>>> 	- also check if the host has the pckmo functions
>>>>>>  arch/s390/include/asm/kvm_host.h | 1 +
>>>>>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>>>>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>>>>>  3 files changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>>>>> index c47e22bba87fa..e224246ff93c6 100644
>>>>>> --- a/arch/s390/include/asm/kvm_host.h
>>>>>> +++ b/arch/s390/include/asm/kvm_host.h
>>>>>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>>>>>>  #define ECD_HOSTREGMGMT	0x20000000
>>>>>>  #define ECD_MEF		0x08000000
>>>>>>  #define ECD_ETOKENF	0x02000000
>>>>>> +#define ECD_ECC		0x00200000
>>>>>>  	__u32	ecd;			/* 0x01c8 */
>>>>>>  	__u8	reserved1cc[18];	/* 0x01cc */
>>>>>>  	__u64	pp;			/* 0x01de */
>>>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>>>> index 0dad61ccde3d6..9869d785677f1 100644
>>>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>>>> @@ -2933,6 +2933,13 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>>>>>>  		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>>>>>>  			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>>>>>>  	}
>>>>>> +	/*
>>>>>> +	 * if any of 32,33,34,40,41 is active in host AND guest,
>>>>>> +	 * we enable pckmo for ecc
>>>>>> +	 */
>>>>>> +	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
>>>>>> +	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
>>>>>
>>>>> Maybe some helper like
>>>>>
>>>>> bool kvm_has_pckmo_subfunc(kvm, nr)
>>>>> {
>>>>> 	/* magic for one number */
>>>>> }
>>>>> ...
>>>>>
>>>>> if (kvm_has_pckmo_subfunc(kvm, 32) || kvm_has_pckmo_subfunc(kvm, 33))
>>>>> ...
>>>>>
>>>>> then you can also get rid of the comment.
>>>>
>>>> Will give it a try.
>>>>>
>>>>>> +		vcpu->arch.sie_block->ecd |= ECD_ECC;
>>>>>>  	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>>>>>>  					| SDNXC;
>>>>>>  	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>>>>> index d62fa148558b9..c6983d962abfd 100644
>>>>>> --- a/arch/s390/kvm/vsie.c
>>>>>> +++ b/arch/s390/kvm/vsie.c
>>>>>> @@ -288,6 +288,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>>>  	const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>>>>>  	unsigned long *b1, *b2;
>>>>>>  	u8 ecb3_flags;
>>>>>> +	u32 ecd_flags;
>>>>>>  	int apie_h;
>>>>>>  	int key_msk = test_kvm_facility(vcpu->kvm, 76);
>>>>>>  	int fmt_o = crycbd_o & CRYCB_FORMAT_MASK;
>>>>>> @@ -320,7 +321,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>>>  	/* we may only allow it if enabled for guest 2 */
>>>>>>  	ecb3_flags = scb_o->ecb3 & vcpu->arch.sie_block->ecb3 &
>>>>>>  		     (ECB3_AES | ECB3_DEA);
>>>>>> -	if (!ecb3_flags)
>>>>>> +	ecd_flags = scb_o->ecd & vcpu->arch.sie_block->ecd & ECD_ECC;
>>>>>> +	if (!ecb3_flags && !ecd_flags)
>>>>>>  		goto end;
>>>>>
>>>>> Just so I get it right, there are no *new* wrapping keys? Which wrapping
>>>>> keys are used then?
>>>>
>>>> Yes, AES.
>>>>
>>>
>>> Hmmmm, so if user space doesn't call KVM_S390_VM_CRYPTO_ENABLE_AES_KW,
>>> the wrapping key is basically uninitialized (kvm_s390_vm_set_crypto()),
>>> but will be used.
>>>
>>> I guess you should also check against kvm->arch.crypto.aes_kw before
>>> turning the ecd bit on, just to be sure.
>>
>> We should rather initialize the aes value when ecc wrapping is enabled.
>>
> 
> If you don't glue this to KVM_S390_VM_CRYPTO_ENABLE_AES_KW, you will
> have to find another way to reset AES keys during resets.

I think it is better to add 2 new attributes as we can have pckmo for ecc but
not for aes (e.g. ECB3_AES == 0 but ECD_ECC ==1).

Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

[Christian Borntraeger]

On 18.04.19 12:40, Christian Borntraeger wrote:
>>
>> If you don't glue this to KVM_S390_VM_CRYPTO_ENABLE_AES_KW, you will
>> have to find another way to reset AES keys during resets.
> 
> I think it is better to add 2 new attributes as we can have pckmo for ecc but
> not for aes (e.g. ECB3_AES == 0 but ECD_ECC ==1).


On the other hand: MSA9 requires MSA3 and MSA4 to be present. So simply checking for
kvm->arch.crypto.aes_kw ==1 as you suggested is very likely good enough.