* [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing
@ 2023-12-19 14:08 Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-12-19 14:08 UTC (permalink / raw)
To: Claudio Imbrenda, Heiko Carstens, Janosch Frank,
David Hildenbrand, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Nina Schoetterl-Glausch
Cc: kvm, Sven Schnelle, linux-kernel, linux-s390
v3 -> v4:
* pick up tags (thanks {David, Janosch, Heiko})
* changes to commit messages
* flip lines and add comment (Janosch)
v2 -> v3:
* pick up tags (thanks Claudio)
* reverse Christmas tree
v1 -> v2:
* pick up tags (thanks {Claudio, David})
* drop Fixes tag on cleanup patch, change message (thanks David)
* drop Fixes tag on second patch since the length of the facility list
copied wasn't initially specified and only clarified in later
revisions
* use READ/WRITE_ONCE (thanks {David, Heiko})
Improve the STFLE vsie implementation.
Firstly, fix a bug concerning the identification if the guest is
intending to use interpretive execution for STFLE for its guest.
Secondly, decrease the amount of guest memory accessed to the
minimum.
Also do some (optional) cleanups.
Nina Schoetterl-Glausch (4):
KVM: s390: vsie: Fix STFLE interpretive execution identification
KVM: s390: vsie: Fix length of facility list shadowed
KVM: s390: cpu model: Use proper define for facility mask size
KVM: s390: Minor refactor of base/ext facility lists
arch/s390/include/asm/facility.h | 6 +++++
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kernel/Makefile | 2 +-
arch/s390/kernel/facility.c | 21 +++++++++++++++
arch/s390/kvm/kvm-s390.c | 44 ++++++++++++++------------------
arch/s390/kvm/vsie.c | 19 ++++++++++++--
6 files changed, 65 insertions(+), 29 deletions(-)
create mode 100644 arch/s390/kernel/facility.c
Range-diff against v3:
1: de77a2c36786 ! 1: 69599bb38487 KVM: s390: vsie: Fix STFLE interpretive execution identification
@@ arch/s390/kvm/vsie.c: static void retry_vsie_icpt(struct vsie_page *vsie_page)
+ __u32 fac = READ_ONCE(vsie_page->scb_o->fac);
if (fac && test_kvm_facility(vcpu->kvm, 7)) {
-+ fac = fac & 0x7ffffff8U;
retry_vsie_icpt(vsie_page);
++ /*
++ * The facility list origin (FLO) is in bits 1 - 28 of the FLD
++ * so we need to mask here before reading.
++ */
++ fac = fac & 0x7ffffff8U;
if (read_guest_real(vcpu, fac, &vsie_page->fac,
sizeof(vsie_page->fac)))
+ return set_validity_icpt(scb_s, 0x1090U);
2: e4b44c4d2400 ! 2: cba3c32a8db7 KVM: s390: vsie: Fix length of facility list shadowed
@@ Commit message
The length of the facility list accessed when interpretively executing
STFLE is the same as the hosts facility list (in case of format-0)
- When shadowing, copy only those bytes.
- The memory following the facility list need not be accessible, in which
- case we'd wrongly inject a validity intercept.
+ The memory following the facility list doesn't need to be accessible.
+ The current VSIE implementation accesses a fixed length that exceeds the
+ guest/host facility list length and can therefore wrongly inject a
+ validity intercept.
+ Instead, find out the host facility list length by running STFLE and
+ copy only as much as necessary when shadowing.
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
+ Acked-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
## arch/s390/include/asm/facility.h ##
@@ arch/s390/include/asm/facility.h: static inline void stfle(u64 *stfle_fac_list,
#endif /* __ASM_FACILITY_H */
## arch/s390/kernel/Makefile ##
-@@ arch/s390/kernel/Makefile: obj-y += sysinfo.o lgr.o os_info.o
+@@ arch/s390/kernel/Makefile: obj-y += sysinfo.o lgr.o os_info.o ctlreg.o
obj-y += runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
obj-y += entry.o reipl.o kdebugfs.o alternative.o
obj-y += nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
@@ arch/s390/kvm/vsie.c: static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie
+ * -> format-0 flcb
+ */
if (fac && test_kvm_facility(vcpu->kvm, 7)) {
- fac = fac & 0x7ffffff8U;
retry_vsie_icpt(vsie_page);
+ /*
+@@ arch/s390/kvm/vsie.c: static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
+ * so we need to mask here before reading.
+ */
+ fac = fac & 0x7ffffff8U;
+ /*
+ * format-0 -> size of nested guest's facility list == guest's size
+ * guest's size == host's size, since STFLE is interpretatively executed
3: 8b02ac33defb ! 3: 4b52e432d736 KVM: s390: cpu model: Use proper define for facility mask size
@@ Commit message
Note that both values are the same, there is no functional change.
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
+ Reviewed-by: David Hildenbrand <david@redhat.com>
+ Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
## arch/s390/include/asm/kvm_host.h ##
4: a592be823576 = 4: 9e551ba53b14 KVM: s390: Minor refactor of base/ext facility lists
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification
2023-12-19 14:08 [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing Nina Schoetterl-Glausch
@ 2023-12-19 14:08 ` Nina Schoetterl-Glausch
2023-12-20 9:56 ` Christian Borntraeger
2023-12-20 10:40 ` Janosch Frank
2023-12-19 14:08 ` [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed Nina Schoetterl-Glausch
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-12-19 14:08 UTC (permalink / raw)
To: Christian Borntraeger, Alexander Gordeev, Janosch Frank,
Claudio Imbrenda, Heiko Carstens, Vasily Gorbik
Cc: Nina Schoetterl-Glausch, David Hildenbrand, linux-kernel, kvm,
Sven Schnelle, linux-s390
STFLE can be interpretively executed.
This occurs when the facility list designation is unequal to zero.
Perform the check before applying the address mask instead of after.
Fixes: 66b630d5b7f2 ("KVM: s390: vsie: support STFLE interpretation")
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 8207a892bbe2..35937911724e 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -984,10 +984,15 @@ static void retry_vsie_icpt(struct vsie_page *vsie_page)
static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
{
struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
- __u32 fac = READ_ONCE(vsie_page->scb_o->fac) & 0x7ffffff8U;
+ __u32 fac = READ_ONCE(vsie_page->scb_o->fac);
if (fac && test_kvm_facility(vcpu->kvm, 7)) {
retry_vsie_icpt(vsie_page);
+ /*
+ * The facility list origin (FLO) is in bits 1 - 28 of the FLD
+ * so we need to mask here before reading.
+ */
+ fac = fac & 0x7ffffff8U;
if (read_guest_real(vcpu, fac, &vsie_page->fac,
sizeof(vsie_page->fac)))
return set_validity_icpt(scb_s, 0x1090U);
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed
2023-12-19 14:08 [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
@ 2023-12-19 14:08 ` Nina Schoetterl-Glausch
2023-12-20 10:45 ` Janosch Frank
2023-12-19 14:08 ` [PATCH v4 3/4] KVM: s390: cpu model: Use proper define for facility mask size Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists Nina Schoetterl-Glausch
3 siblings, 1 reply; 9+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-12-19 14:08 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, David Hildenbrand,
Vasily Gorbik, Nina Schoetterl-Glausch, Alexander Gordeev,
Claudio Imbrenda, Heiko Carstens
Cc: linux-kernel, kvm, Sven Schnelle, linux-s390
The length of the facility list accessed when interpretively executing
STFLE is the same as the hosts facility list (in case of format-0)
The memory following the facility list doesn't need to be accessible.
The current VSIE implementation accesses a fixed length that exceeds the
guest/host facility list length and can therefore wrongly inject a
validity intercept.
Instead, find out the host facility list length by running STFLE and
copy only as much as necessary when shadowing.
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Acked-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
arch/s390/include/asm/facility.h | 6 ++++++
arch/s390/kernel/Makefile | 2 +-
arch/s390/kernel/facility.c | 21 +++++++++++++++++++++
arch/s390/kvm/vsie.c | 12 +++++++++++-
4 files changed, 39 insertions(+), 2 deletions(-)
create mode 100644 arch/s390/kernel/facility.c
diff --git a/arch/s390/include/asm/facility.h b/arch/s390/include/asm/facility.h
index 94b6919026df..796007125dff 100644
--- a/arch/s390/include/asm/facility.h
+++ b/arch/s390/include/asm/facility.h
@@ -111,4 +111,10 @@ static inline void stfle(u64 *stfle_fac_list, int size)
preempt_enable();
}
+/**
+ * stfle_size - Actual size of the facility list as specified by stfle
+ * (number of double words)
+ */
+unsigned int stfle_size(void);
+
#endif /* __ASM_FACILITY_H */
diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
index 353def93973b..7a562b4199c8 100644
--- a/arch/s390/kernel/Makefile
+++ b/arch/s390/kernel/Makefile
@@ -41,7 +41,7 @@ obj-y += sysinfo.o lgr.o os_info.o ctlreg.o
obj-y += runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
obj-y += entry.o reipl.o kdebugfs.o alternative.o
obj-y += nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
-obj-y += smp.o text_amode31.o stacktrace.o abs_lowcore.o
+obj-y += smp.o text_amode31.o stacktrace.o abs_lowcore.o facility.o
extra-y += vmlinux.lds
diff --git a/arch/s390/kernel/facility.c b/arch/s390/kernel/facility.c
new file mode 100644
index 000000000000..f02127219a27
--- /dev/null
+++ b/arch/s390/kernel/facility.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright IBM Corp. 2023
+ */
+
+#include <asm/facility.h>
+
+unsigned int stfle_size(void)
+{
+ static unsigned int size;
+ unsigned int r;
+ u64 dummy;
+
+ r = READ_ONCE(size);
+ if (!r) {
+ r = __stfle_asm(&dummy, 1) + 1;
+ WRITE_ONCE(size, r);
+ }
+ return r;
+}
+EXPORT_SYMBOL(stfle_size);
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 35937911724e..fef42e2a80a2 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -19,6 +19,7 @@
#include <asm/nmi.h>
#include <asm/dis.h>
#include <asm/fpu/api.h>
+#include <asm/facility.h>
#include "kvm-s390.h"
#include "gaccess.h"
@@ -986,6 +987,10 @@ static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
__u32 fac = READ_ONCE(vsie_page->scb_o->fac);
+ /*
+ * Alternate-STFLE-Interpretive-Execution facilities are not supported
+ * -> format-0 flcb
+ */
if (fac && test_kvm_facility(vcpu->kvm, 7)) {
retry_vsie_icpt(vsie_page);
/*
@@ -993,8 +998,13 @@ static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
* so we need to mask here before reading.
*/
fac = fac & 0x7ffffff8U;
+ /*
+ * format-0 -> size of nested guest's facility list == guest's size
+ * guest's size == host's size, since STFLE is interpretatively executed
+ * using a format-0 for the guest, too.
+ */
if (read_guest_real(vcpu, fac, &vsie_page->fac,
- sizeof(vsie_page->fac)))
+ stfle_size() * sizeof(u64)))
return set_validity_icpt(scb_s, 0x1090U);
scb_s->fac = (__u32)(__u64) &vsie_page->fac;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] KVM: s390: cpu model: Use proper define for facility mask size
2023-12-19 14:08 [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed Nina Schoetterl-Glausch
@ 2023-12-19 14:08 ` Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists Nina Schoetterl-Glausch
3 siblings, 0 replies; 9+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-12-19 14:08 UTC (permalink / raw)
To: Janosch Frank, Heiko Carstens, Christian Borntraeger,
Claudio Imbrenda, Alexander Gordeev, Vasily Gorbik
Cc: Nina Schoetterl-Glausch, David Hildenbrand, kvm, Sven Schnelle,
linux-kernel, linux-s390
Use the previously unused S390_ARCH_FAC_MASK_SIZE_U64 instead of
S390_ARCH_FAC_LIST_SIZE_U64 for defining the fac_mask array.
Note that both values are the same, there is no functional change.
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
arch/s390/include/asm/kvm_host.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 67a298b6cf6e..52664105a473 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -818,7 +818,7 @@ struct s390_io_adapter {
struct kvm_s390_cpu_model {
/* facility mask supported by kvm & hosting machine */
- __u64 fac_mask[S390_ARCH_FAC_LIST_SIZE_U64];
+ __u64 fac_mask[S390_ARCH_FAC_MASK_SIZE_U64];
struct kvm_s390_vm_cpu_subfunc subfuncs;
/* facility list requested by guest (in dma page) */
__u64 *fac_list;
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists
2023-12-19 14:08 [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing Nina Schoetterl-Glausch
` (2 preceding siblings ...)
2023-12-19 14:08 ` [PATCH v4 3/4] KVM: s390: cpu model: Use proper define for facility mask size Nina Schoetterl-Glausch
@ 2023-12-19 14:08 ` Nina Schoetterl-Glausch
2023-12-20 10:48 ` Janosch Frank
3 siblings, 1 reply; 9+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-12-19 14:08 UTC (permalink / raw)
To: Claudio Imbrenda, Alexander Gordeev, Janosch Frank, Vasily Gorbik,
Heiko Carstens, Christian Borntraeger
Cc: Nina Schoetterl-Glausch, linux-s390, David Hildenbrand,
Sven Schnelle, linux-kernel, kvm
Directly use the size of the arrays instead of going through the
indirection of kvm_s390_fac_size().
Don't use magic number for the number of entries in the non hypervisor
managed facility bit mask list.
Make the constraint of that number on kvm_s390_fac_base obvious.
Get rid of implicit double anding of stfle_fac_list.
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
Notes:
I think it's nicer this way but it might be needless churn.
arch/s390/kvm/kvm-s390.c | 44 +++++++++++++++++-----------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 7aa0e668488f..ac8d551f8b32 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -224,33 +224,25 @@ static int async_destroy = 1;
module_param(async_destroy, int, 0444);
MODULE_PARM_DESC(async_destroy, "Asynchronous destroy for protected guests");
-/*
- * For now we handle at most 16 double words as this is what the s390 base
- * kernel handles and stores in the prefix page. If we ever need to go beyond
- * this, this requires changes to code, but the external uapi can stay.
- */
-#define SIZE_INTERNAL 16
-
+#define HMFAI_DWORDS 16
/*
* Base feature mask that defines default mask for facilities. Consists of the
* defines in FACILITIES_KVM and the non-hypervisor managed bits.
*/
-static unsigned long kvm_s390_fac_base[SIZE_INTERNAL] = { FACILITIES_KVM };
+static unsigned long kvm_s390_fac_base[HMFAI_DWORDS] = { FACILITIES_KVM };
+static_assert(ARRAY_SIZE(((long[]){ FACILITIES_KVM })) <= HMFAI_DWORDS);
+static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_MASK_SIZE_U64);
+static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= S390_ARCH_FAC_LIST_SIZE_U64);
+static_assert(ARRAY_SIZE(kvm_s390_fac_base) <= ARRAY_SIZE(stfle_fac_list));
+
/*
* Extended feature mask. Consists of the defines in FACILITIES_KVM_CPUMODEL
* and defines the facilities that can be enabled via a cpu model.
*/
-static unsigned long kvm_s390_fac_ext[SIZE_INTERNAL] = { FACILITIES_KVM_CPUMODEL };
-
-static unsigned long kvm_s390_fac_size(void)
-{
- BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_MASK_SIZE_U64);
- BUILD_BUG_ON(SIZE_INTERNAL > S390_ARCH_FAC_LIST_SIZE_U64);
- BUILD_BUG_ON(SIZE_INTERNAL * sizeof(unsigned long) >
- sizeof(stfle_fac_list));
-
- return SIZE_INTERNAL;
-}
+static const unsigned long kvm_s390_fac_ext[] = { FACILITIES_KVM_CPUMODEL };
+static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_MASK_SIZE_U64);
+static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= S390_ARCH_FAC_LIST_SIZE_U64);
+static_assert(ARRAY_SIZE(kvm_s390_fac_ext) <= ARRAY_SIZE(stfle_fac_list));
/* available cpu features supported by kvm */
static DECLARE_BITMAP(kvm_s390_available_cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS);
@@ -3348,13 +3340,16 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm->arch.sie_page2->kvm = kvm;
kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
- for (i = 0; i < kvm_s390_fac_size(); i++) {
+ for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_base); i++) {
kvm->arch.model.fac_mask[i] = stfle_fac_list[i] &
- (kvm_s390_fac_base[i] |
- kvm_s390_fac_ext[i]);
+ kvm_s390_fac_base[i];
kvm->arch.model.fac_list[i] = stfle_fac_list[i] &
kvm_s390_fac_base[i];
}
+ for (i = 0; i < ARRAY_SIZE(kvm_s390_fac_ext); i++) {
+ kvm->arch.model.fac_mask[i] |= stfle_fac_list[i] &
+ kvm_s390_fac_ext[i];
+ }
kvm->arch.model.subfuncs = kvm_s390_available_subfunc;
/* we are always in czam mode - even on pre z14 machines */
@@ -5868,9 +5863,8 @@ static int __init kvm_s390_init(void)
return -EINVAL;
}
- for (i = 0; i < 16; i++)
- kvm_s390_fac_base[i] |=
- stfle_fac_list[i] & nonhyp_mask(i);
+ for (i = 0; i < HMFAI_DWORDS; i++)
+ kvm_s390_fac_base[i] |= nonhyp_mask(i);
r = __kvm_s390_init();
if (r)
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
@ 2023-12-20 9:56 ` Christian Borntraeger
2023-12-20 10:40 ` Janosch Frank
1 sibling, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2023-12-20 9:56 UTC (permalink / raw)
To: Nina Schoetterl-Glausch, Alexander Gordeev, Janosch Frank,
Claudio Imbrenda, Heiko Carstens, Vasily Gorbik
Cc: David Hildenbrand, linux-kernel, kvm, Sven Schnelle, linux-s390
Am 19.12.23 um 15:08 schrieb Nina Schoetterl-Glausch:
> STFLE can be interpretively executed.
> This occurs when the facility list designation is unequal to zero.
> Perform the check before applying the address mask instead of after.
>
> Fixes: 66b630d5b7f2 ("KVM: s390: vsie: support STFLE interpretation")
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
this should not matter in reality but maybe some weird guests puts this at address 0.
Do we want a unit test for that case?
> ---
> arch/s390/kvm/vsie.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 8207a892bbe2..35937911724e 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -984,10 +984,15 @@ static void retry_vsie_icpt(struct vsie_page *vsie_page)
> static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> {
> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> - __u32 fac = READ_ONCE(vsie_page->scb_o->fac) & 0x7ffffff8U;
> + __u32 fac = READ_ONCE(vsie_page->scb_o->fac);
>
> if (fac && test_kvm_facility(vcpu->kvm, 7)) {
> retry_vsie_icpt(vsie_page);
> + /*
> + * The facility list origin (FLO) is in bits 1 - 28 of the FLD
> + * so we need to mask here before reading.
> + */
> + fac = fac & 0x7ffffff8U;
> if (read_guest_real(vcpu, fac, &vsie_page->fac,
> sizeof(vsie_page->fac)))
> return set_validity_icpt(scb_s, 0x1090U);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
2023-12-20 9:56 ` Christian Borntraeger
@ 2023-12-20 10:40 ` Janosch Frank
1 sibling, 0 replies; 9+ messages in thread
From: Janosch Frank @ 2023-12-20 10:40 UTC (permalink / raw)
To: Nina Schoetterl-Glausch, Christian Borntraeger, Alexander Gordeev,
Claudio Imbrenda, Heiko Carstens, Vasily Gorbik
Cc: David Hildenbrand, linux-kernel, kvm, Sven Schnelle, linux-s390
On 12/19/23 15:08, Nina Schoetterl-Glausch wrote:
> STFLE can be interpretively executed.
> This occurs when the facility list designation is unequal to zero.
> Perform the check before applying the address mask instead of after.
>
> Fixes: 66b630d5b7f2 ("KVM: s390: vsie: support STFLE interpretation")
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> arch/s390/kvm/vsie.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 8207a892bbe2..35937911724e 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -984,10 +984,15 @@ static void retry_vsie_icpt(struct vsie_page *vsie_page)
> static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> {
> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> - __u32 fac = READ_ONCE(vsie_page->scb_o->fac) & 0x7ffffff8U;
> + __u32 fac = READ_ONCE(vsie_page->scb_o->fac);
>
> if (fac && test_kvm_facility(vcpu->kvm, 7)) {
> retry_vsie_icpt(vsie_page);
> + /*
> + * The facility list origin (FLO) is in bits 1 - 28 of the FLD
> + * so we need to mask here before reading.
> + */
> + fac = fac & 0x7ffffff8U;
> if (read_guest_real(vcpu, fac, &vsie_page->fac,
> sizeof(vsie_page->fac)))
> return set_validity_icpt(scb_s, 0x1090U);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed
2023-12-19 14:08 ` [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed Nina Schoetterl-Glausch
@ 2023-12-20 10:45 ` Janosch Frank
0 siblings, 0 replies; 9+ messages in thread
From: Janosch Frank @ 2023-12-20 10:45 UTC (permalink / raw)
To: Nina Schoetterl-Glausch, Christian Borntraeger, David Hildenbrand,
Vasily Gorbik, Alexander Gordeev, Claudio Imbrenda,
Heiko Carstens
Cc: linux-kernel, kvm, Sven Schnelle, linux-s390
On 12/19/23 15:08, Nina Schoetterl-Glausch wrote:
> The length of the facility list accessed when interpretively executing
> STFLE is the same as the hosts facility list (in case of format-0)
> The memory following the facility list doesn't need to be accessible.
> The current VSIE implementation accesses a fixed length that exceeds the
> guest/host facility list length and can therefore wrongly inject a
> validity intercept.
> Instead, find out the host facility list length by running STFLE and
> copy only as much as necessary when shadowing.
>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Acked-by: Heiko Carstens <hca@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists
2023-12-19 14:08 ` [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists Nina Schoetterl-Glausch
@ 2023-12-20 10:48 ` Janosch Frank
0 siblings, 0 replies; 9+ messages in thread
From: Janosch Frank @ 2023-12-20 10:48 UTC (permalink / raw)
To: Nina Schoetterl-Glausch, Claudio Imbrenda, Alexander Gordeev,
Vasily Gorbik, Heiko Carstens, Christian Borntraeger
Cc: linux-s390, David Hildenbrand, Sven Schnelle, linux-kernel, kvm
On 12/19/23 15:08, Nina Schoetterl-Glausch wrote:
> Directly use the size of the arrays instead of going through the
> indirection of kvm_s390_fac_size().
> Don't use magic number for the number of entries in the non hypervisor
> managed facility bit mask list.
> Make the constraint of that number on kvm_s390_fac_base obvious.
> Get rid of implicit double anding of stfle_fac_list.
>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
@Nina: I'm currently still recovering from a cold and hence I'm not
fully able to grasp this patch.
May I drop it and we re-visit it next year for 6.9?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-12-20 10:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-19 14:08 [PATCH v4 0/4] KVM: s390: Fix minor bugs in STFLE shadowing Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 1/4] KVM: s390: vsie: Fix STFLE interpretive execution identification Nina Schoetterl-Glausch
2023-12-20 9:56 ` Christian Borntraeger
2023-12-20 10:40 ` Janosch Frank
2023-12-19 14:08 ` [PATCH v4 2/4] KVM: s390: vsie: Fix length of facility list shadowed Nina Schoetterl-Glausch
2023-12-20 10:45 ` Janosch Frank
2023-12-19 14:08 ` [PATCH v4 3/4] KVM: s390: cpu model: Use proper define for facility mask size Nina Schoetterl-Glausch
2023-12-19 14:08 ` [PATCH v4 4/4] KVM: s390: Minor refactor of base/ext facility lists Nina Schoetterl-Glausch
2023-12-20 10:48 ` Janosch Frank
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox