* [PATCH 01/21] KVM: s390: Remove double 64bscao feature check
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 02/21] KVM: s390: vsie: Remove duplicate assertion Christoph Schlameuss
` (19 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Hendrik Brueckner,
Janosch Frank, Christoph Schlameuss
sclp.has_64bscao is already verified in the guard clause a few lines
above this. So we cannot reach this code if it is not true.
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@de.ibm.com>
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 23c817595e28..4b1ef955c7e8 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -455,8 +455,7 @@ static void __init kvm_s390_cpu_feat_init(void)
!test_facility(3) || !nested)
return;
allow_cpu_feat(KVM_S390_VM_CPU_FEAT_SIEF2);
- if (sclp.has_64bscao)
- allow_cpu_feat(KVM_S390_VM_CPU_FEAT_64BSCAO);
+ allow_cpu_feat(KVM_S390_VM_CPU_FEAT_64BSCAO);
if (sclp.has_siif)
allow_cpu_feat(KVM_S390_VM_CPU_FEAT_SIIF);
if (sclp.has_gpere)
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 02/21] KVM: s390: vsie: Remove duplicate assertion
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 01/21] KVM: s390: Remove double 64bscao feature check Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 03/21] KVM: s390: vsie: Convert shift to phys_to_pfn() Christoph Schlameuss
` (18 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
This is covered by a static_assert at the struct vsie_page definition.
Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index eea24562e7db..3e956c7faa4b 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1565,7 +1565,6 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
- BUILD_BUG_ON(sizeof(struct vsie_page) != PAGE_SIZE);
scb_addr = kvm_s390_get_base_disp_s(vcpu, NULL);
/* 512 byte alignment */
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 03/21] KVM: s390: vsie: Convert shift to phys_to_pfn()
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 01/21] KVM: s390: Remove double 64bscao feature check Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 02/21] KVM: s390: vsie: Remove duplicate assertion Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers Christoph Schlameuss
` (17 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Make the code slightly more readable.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 3e956c7faa4b..937f9c99bc00 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -701,7 +701,7 @@ static int pin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t *hpa)
/* Unpins a page previously pinned via pin_guest_page, marking it as dirty. */
static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
{
- kvm_release_page_dirty(pfn_to_page(hpa >> PAGE_SHIFT));
+ kvm_release_page_dirty(pfn_to_page(phys_to_pfn(hpa)));
/* mark the page always as dirty for migration */
mark_page_dirty(kvm, gpa_to_gfn(gpa));
}
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (2 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 03/21] KVM: s390: vsie: Convert shift to phys_to_pfn() Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:24 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function Christoph Schlameuss
` (16 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Introduce some small helper functions to get and set the system control
area origin address from the SIE control block.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 937f9c99bc00..aa0cebece2d5 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -72,6 +72,22 @@ struct vsie_page {
static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
+static unsigned long read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
+{
+ unsigned long vsie_sca = READ_ONCE(scb->scaol) & ~0xfUL;
+
+ if (test_kvm_cpu_feat(kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
+ vsie_sca |= (u64)READ_ONCE(scb->scaoh) << 32;
+
+ return vsie_sca;
+}
+
+static void write_scao(struct kvm_s390_sie_block *scb, unsigned long hpa)
+{
+ scb->scaoh = (u32)((u64)hpa >> 32);
+ scb->scaol = (u32)(u64)hpa;
+}
+
/* trigger a validity icpt for the given scb */
static int set_validity_icpt(struct kvm_s390_sie_block *scb,
__u16 reason_code)
@@ -712,12 +728,11 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
hpa_t hpa;
- hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
+ hpa = read_scao(vcpu->kvm, scb_s);
if (hpa) {
unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
vsie_page->sca_gpa = 0;
- scb_s->scaol = 0;
- scb_s->scaoh = 0;
+ write_scao(scb_s, 0);
}
hpa = scb_s->itdba;
@@ -771,9 +786,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
gpa_t gpa;
int rc = 0;
- gpa = READ_ONCE(scb_o->scaol) & ~0xfUL;
- if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
- gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
+ gpa = read_scao(vcpu->kvm, scb_o);
if (gpa) {
if (gpa < 2 * PAGE_SIZE)
rc = set_validity_icpt(scb_s, 0x0038U);
@@ -790,8 +803,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
if (rc)
goto unpin;
vsie_page->sca_gpa = gpa;
- scb_s->scaoh = (u32)((u64)hpa >> 32);
- scb_s->scaol = (u32)(u64)hpa;
+ write_scao(scb_s, hpa);
}
gpa = READ_ONCE(scb_o->itdba) & ~0xffUL;
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers
2026-07-09 15:15 ` [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers Christoph Schlameuss
@ 2026-07-09 15:24 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:24 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Alexander Gordeev, linux-s390, kvm, Heiko Carstens, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] read_scao() incorrectly truncates the 64-bit Host Physical Address (HPA) when unpinning the shadow SCB if the guest lacks the 64BSCAO feature, leading to host memory corruption and leaked pages.
--
--- Patch [4]: [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers ---
commit ef50a3af344aad608f5c9779f347c6f56923a4f5
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add SCAO read and write helpers
This commit introduces helper functions to get and set the system control
area origin address from the SIE control block.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 937f9c99bc002..aa0cebece2d50 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -72,6 +72,22 @@ struct vsie_page {
>
> static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
>
> +static unsigned long read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
> +{
> + unsigned long vsie_sca = READ_ONCE(scb->scaol) & ~0xfUL;
> +
> + if (test_kvm_cpu_feat(kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
> + vsie_sca |= (u64)READ_ONCE(scb->scaoh) << 32;
> +
> + return vsie_sca;
> +}
> +
> +static void write_scao(struct kvm_s390_sie_block *scb, unsigned long hpa)
> +{
> + scb->scaoh = (u32)((u64)hpa >> 32);
> + scb->scaol = (u32)(u64)hpa;
> +}
> +
[ ... ]
> @@ -712,12 +728,11 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> hpa_t hpa;
>
> - hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
> + hpa = read_scao(vcpu->kvm, scb_s);
[Severity: Critical]
Does this truncate the 64-bit Host Physical Address (HPA) when unpinning
the shadow SCB if the guest lacks the 64BSCAO feature?
When a guest is instantiated without KVM_S390_VM_CPU_FEAT_64BSCAO, KVM will
still pin the guest's SCA page to an HPA that can be > 4GB, writing the full
64-bit address to the shadow SCB via write_scao().
During unpin_blocks(), read_scao() will conditionally mask off the upper 32
bits because the guest lacks the CPU feature.
Could this cause unpin_guest_page() to receive a truncated, invalid address,
causing the host to decrement the reference count of the wrong host page and
permanently leak the originally pinned page?
> if (hpa) {
> unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
> vsie_page->sca_gpa = 0;
> - scb_s->scaol = 0;
> - scb_s->scaoh = 0;
> + write_scao(scb_s, 0);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=4
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (3 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 04/21] KVM: s390: vsie: Add SCAO read and write helpers Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:26 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection Christoph Schlameuss
` (15 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Improve readability as well as allow re-use in coming patches.
In particular add the logic to be able to check the validity of BSCA and
ESCA origin addresses.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 42 ++++++++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index aa0cebece2d5..97581b748a94 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -72,6 +72,11 @@ struct vsie_page {
static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
+static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
+{
+ return (scb->ecb2 & ECB2_ESCA);
+}
+
static unsigned long read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
{
unsigned long vsie_sca = READ_ONCE(scb->scaol) & ~0xfUL;
@@ -98,6 +103,25 @@ static int set_validity_icpt(struct kvm_s390_sie_block *scb,
return 1;
}
+/* The sca header must not cross pages etc. */
+static int validate_scao(struct kvm_vcpu *vcpu, struct kvm_s390_sie_block *scb, gpa_t gpa)
+{
+ int offset;
+
+ if (gpa < 2 * PAGE_SIZE)
+ return set_validity_icpt(scb, 0x0038U);
+ if ((gpa & ~0x1fffUL) == kvm_s390_get_prefix(vcpu))
+ return set_validity_icpt(scb, 0x0011U);
+
+ if (sie_uses_esca(scb))
+ offset = offsetof(struct esca_block, cpu[0]) - 1;
+ else
+ offset = offsetof(struct bsca_block, cpu[0]) - 1;
+ if ((gpa & PAGE_MASK) != ((gpa + offset) & PAGE_MASK))
+ return set_validity_icpt(scb, 0x003bU);
+ return 0;
+}
+
/* mark the prefix as unmapped, this will block the VSIE */
static void prefix_unmapped(struct vsie_page *vsie_page)
{
@@ -788,20 +812,14 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
gpa = read_scao(vcpu->kvm, scb_o);
if (gpa) {
- if (gpa < 2 * PAGE_SIZE)
- rc = set_validity_icpt(scb_s, 0x0038U);
- else if ((gpa & ~0x1fffUL) == kvm_s390_get_prefix(vcpu))
- rc = set_validity_icpt(scb_s, 0x0011U);
- else if ((gpa & PAGE_MASK) !=
- ((gpa + offsetof(struct bsca_block, cpu[0]) - 1) & PAGE_MASK))
- rc = set_validity_icpt(scb_s, 0x003bU);
- if (!rc) {
- rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
- if (rc)
- rc = set_validity_icpt(scb_s, 0x0034U);
- }
+ rc = validate_scao(vcpu, scb_o, gpa);
if (rc)
goto unpin;
+ rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
+ if (rc) {
+ rc = set_validity_icpt(scb_s, 0x0034U);
+ goto unpin;
+ }
vsie_page->sca_gpa = gpa;
write_scao(scb_s, hpa);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function
2026-07-09 15:15 ` [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function Christoph Schlameuss
@ 2026-07-09 15:26 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:26 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Heiko Carstens, linux-s390, Vasily Gorbik, Alexander Gordeev, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The validity intercept for an invalid SCAO is written to the original guest SCB (`scb_o`) instead of the shadow SCB (`scb_s`), causing it to be silently overwritten and lost during unshadowing.
--
--- Patch [5]: [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function ---
commit 617161c9be47e700da98b253966f2dd2c1dfcaeb
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Move SCAO validation into a function
This commit moves the System Control Area Origin (SCAO) validation logic
into a new standalone function to improve readability and allow reuse. It
also adds logic to validate both BSCA and ESCA origin addresses.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -788,20 +812,14 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>
> gpa = read_scao(vcpu->kvm, scb_o);
> if (gpa) {
> - if (gpa < 2 * PAGE_SIZE)
> - rc = set_validity_icpt(scb_s, 0x0038U);
> - else if ((gpa & ~0x1fffUL) == kvm_s390_get_prefix(vcpu))
> - rc = set_validity_icpt(scb_s, 0x0011U);
> - else if ((gpa & PAGE_MASK) !=
> - ((gpa + offsetof(struct bsca_block, cpu[0]) - 1) & PAGE_MASK))
> - rc = set_validity_icpt(scb_s, 0x003bU);
> - if (!rc) {
> - rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
> - if (rc)
> - rc = set_validity_icpt(scb_s, 0x0034U);
> - }
> + rc = validate_scao(vcpu, scb_o, gpa);
[Severity: High]
Does this mistakenly set the validity intercept on the original SCB (scb_o)
instead of the shadow SCB (scb_s)?
The previous code correctly called set_validity_icpt(scb_s, ...) to write
intercepts to the shadow SCB. By passing scb_o to validate_scao(), the
validity intercept is now written to the original SCB.
When unshadowing occurs later in unshadow_scb(), the uninitialized intercept
data from the shadow SCB will overwrite the original SCB, completely erasing
the validity intercept:
unshadow_scb() {
...
scb_o->icptcode = scb_s->icptcode;
scb_o->icptstatus = scb_s->icptstatus;
scb_o->ipa = scb_s->ipa;
scb_o->ipb = scb_s->ipb;
...
}
Should validate_scao() be modified to ensure the validity intercept is
correctly written to the shadow SCB?
> if (rc)
> goto unpin;
> + rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
> + if (rc) {
> + rc = set_validity_icpt(scb_s, 0x0034U);
> + goto unpin;
> + }
> vsie_page->sca_gpa = gpa;
> write_scao(scb_s, hpa);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=5
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (4 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 05/21] KVM: s390: vsie: Move SCAO validation into a function Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:32 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie Christoph Schlameuss
` (14 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss,
Hendrik Brueckner
Add sensing of the VSIE Interpretation Extension Facility as vsie_sigpif
from SCLP. This facility is introduced with IBM Z gen17.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
arch/s390/include/asm/kvm_host.h | 1 +
arch/s390/include/asm/sclp.h | 1 +
arch/s390/kvm/kvm-s390.c | 1 +
drivers/s390/char/sclp_early.c | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index eaa34c5bd3c1..f10f8940bd48 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -672,6 +672,7 @@ struct kvm_arch {
int use_pfmfi;
int use_skf;
int use_zpci_interp;
+ int use_vsie_sigpif;
int user_cpu_state_ctrl;
int user_sigp;
int user_stsi;
diff --git a/arch/s390/include/asm/sclp.h b/arch/s390/include/asm/sclp.h
index f2d490558054..094b378f997b 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -102,6 +102,7 @@ struct sclp_info {
unsigned char has_dirq : 1;
unsigned char has_iplcc : 1;
unsigned char has_zpci_lsi : 1;
+ unsigned char has_vsie_sigpif : 1;
unsigned char has_aisii : 1;
unsigned char has_aeni : 1;
unsigned char has_aisi : 1;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4b1ef955c7e8..6749198bac3f 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3378,6 +3378,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm->arch.use_pfmfi = sclp.has_pfmfi;
kvm->arch.use_skf = sclp.has_skey;
+ kvm->arch.use_vsie_sigpif = sclp.has_vsie_sigpif;
spin_lock_init(&kvm->arch.start_stop_lock);
kvm_s390_vsie_init(kvm);
if (use_gisa)
diff --git a/drivers/s390/char/sclp_early.c b/drivers/s390/char/sclp_early.c
index 22dd797e6229..9ea840d80711 100644
--- a/drivers/s390/char/sclp_early.c
+++ b/drivers/s390/char/sclp_early.c
@@ -56,6 +56,7 @@ static void __init sclp_early_facilities_detect(void)
sclp.has_diag318 = !!(sccb->byte_134 & 0x80);
sclp.has_diag320 = !!(sccb->byte_134 & 0x04);
sclp.has_iplcc = !!(sccb->byte_134 & 0x02);
+ sclp.has_vsie_sigpif = !!(sccb->byte_134 & 0x01);
}
if (sccb->cpuoff > 137) {
sclp.has_sipl = !!(sccb->cbl & 0x4000);
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection
2026-07-09 15:15 ` [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection Christoph Schlameuss
@ 2026-07-09 15:32 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:32 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Vasily Gorbik, Heiko Carstens, kvm, linux-s390, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Truncation of host physical address in read_scao leads to use-after-free and memory leak.
- [High] Lost validity intercept during VSIE SCAO validation failure.
--
--- Patch [6]: [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection ---
commit 5159ce1359458442d26eeb7031f94445e04e2d6a
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add vsie_sigpif detection
Add sensing of the VSIE Interpretation Extension Facility as vsie_sigpif
from SCLP. This facility is introduced with IBM Z gen17.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index eea24562e7db2..97581b748a94d 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -72,6 +72,27 @@ struct vsie_page {
>
> static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
>
> +static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
> +{
> + return (scb->ecb2 & ECB2_ESCA);
> +}
> +
> +static unsigned long read_scao(struct kvm *kvm, struct kvm_s390_sie_block *scb)
> +{
> + unsigned long vsie_sca = READ_ONCE(scb->scaol) & ~0xfUL;
> +
> + if (test_kvm_cpu_feat(kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
> + vsie_sca |= (u64)READ_ONCE(scb->scaoh) << 32;
[Severity: Critical]
Does this code truncate the host physical address if the guest lacks the
KVM_S390_VM_CPU_FEAT_64BSCAO feature?
When read_scao() is called from unpin_blocks() on the shadow SCB (scb_s):
arch/s390/kvm/vsie.c:unpin_blocks() {
...
hpa = read_scao(vcpu->kvm, scb_s);
if (hpa) {
unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
...
}
The shadow SCB contains a 64-bit host physical address. If the L1 guest is
configured without the 64BSCAO feature, read_scao() will silently drop the
upper 32 bits of the HPA.
Could this truncated address then be passed to unpin_guest_page(),
causing it to unpin the wrong host physical page (use-after-free) while
leaking the actually pinned page?
> +
> + return vsie_sca;
> +}
> +
> +static void write_scao(struct kvm_s390_sie_block *scb, unsigned long hpa)
> +{
> + scb->scaoh = (u32)((u64)hpa >> 32);
> + scb->scaol = (u32)(u64)hpa;
> +}
> +
> /* trigger a validity icpt for the given scb */
> static int set_validity_icpt(struct kvm_s390_sie_block *scb,
> __u16 reason_code)
> @@ -82,6 +103,25 @@ static int set_validity_icpt(struct kvm_s390_sie_block *scb,
> return 1;
> }
>
> +/* The sca header must not cross pages etc. */
> +static int validate_scao(struct kvm_vcpu *vcpu, struct kvm_s390_sie_block *scb, gpa_t gpa)
> +{
> + int offset;
> +
> + if (gpa < 2 * PAGE_SIZE)
> + return set_validity_icpt(scb, 0x0038U);
[Severity: High]
Can this validity intercept be lost during VSIE SCAO validation failure?
Looking at pin_blocks(), it calls validate_scao() and passes the guest
SCB (scb_o):
arch/s390/kvm/vsie.c:pin_blocks() {
...
rc = validate_scao(vcpu, scb_o, gpa);
if (rc)
goto unpin;
...
}
When validate_scao() fails, it injects the intercept code into scb_o.
However, on the failure path, unshadow_scb() is called:
arch/s390/kvm/vsie.c:unshadow_scb() {
...
scb_o->icptcode = scb_s->icptcode;
scb_o->icptstatus = scb_s->icptstatus;
scb_o->ipa = scb_s->ipa;
scb_o->ipb = scb_s->ipb;
...
}
Would unshadow_scb() unconditionally overwrite scb_o's intercept fields
with those from the shadow SCB (scb_s), erasing the validity intercept
that was just written to scb_o?
Could this result in the L1 guest receiving an architecturally invalid
exit without an intercept code?
> + if ((gpa & ~0x1fffUL) == kvm_s390_get_prefix(vcpu))
> + return set_validity_icpt(scb, 0x0011U);
> +
> + if (sie_uses_esca(scb))
> + offset = offsetof(struct esca_block, cpu[0]) - 1;
> + else
> + offset = offsetof(struct bsca_block, cpu[0]) - 1;
> + if ((gpa & PAGE_MASK) != ((gpa + offset) & PAGE_MASK))
> + return set_validity_icpt(scb, 0x003bU);
> + return 0;
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=6
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (5 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 06/21] KVM: s390: vsie: Add vsie_sigpif detection Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:24 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 08/21] KVM: s390: vsie: Move pin/unpin_scb methods Christoph Schlameuss
` (13 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Add the required guest-1 structures for the vsie_sigpif to the SIE
control block and vsie_page for use in later patches.
The shadow SCA features the address of the original SCA as well as an
entry for each original SIGP entry. The entries contain the addresses of
the shadow state description and original SIGP entry.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/include/asm/kvm_host_types.h | 25 ++++++++++++++++++++++++-
tools/testing/selftests/kvm/include/s390/sie.h | 2 +-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/s390/include/asm/kvm_host_types.h b/arch/s390/include/asm/kvm_host_types.h
index 3f50942bdfe6..6540146b4b00 100644
--- a/arch/s390/include/asm/kvm_host_types.h
+++ b/arch/s390/include/asm/kvm_host_types.h
@@ -6,6 +6,8 @@
#include <linux/atomic.h>
#include <linux/types.h>
+#define KVM_S390_MAX_VSIE_VCPUS 256
+
#define KVM_S390_BSCA_CPU_SLOTS 64
#define KVM_S390_ESCA_CPU_SLOTS 248
@@ -45,6 +47,13 @@ struct bsca_entry {
__u64 reserved2[2];
};
+struct ssca_entry {
+ __u64 reserved1;
+ __u64 ssda;
+ __u64 ossea;
+ __u64 reserved2;
+};
+
union ipte_control {
unsigned long val;
struct {
@@ -86,6 +95,20 @@ struct esca_block {
struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];
};
+/*
+ * The shadow sca / ssca needs to cover both bsca and esca depending on what the
+ * guest uses so we allocate space for 256 entries that are defined in the
+ * architecture.
+ * The header part of the struct must not cross page boundaries.
+ */
+struct ssca_block {
+ __u64 osca;
+ __u64 reserved08[7];
+ struct ssca_entry cpu[KVM_S390_MAX_VSIE_VCPUS];
+};
+
+static_assert(offsetof(struct ssca_block, cpu) == 64);
+
/*
* This struct is used to store some machine check info from lowcore
* for machine checks that happen while the guest is running.
@@ -315,7 +338,7 @@ struct kvm_s390_sie_block {
__u32 fac; /* 0x01a0 */
__u8 reserved1a4[20]; /* 0x01a4 */
__u64 cbrlo; /* 0x01b8 */
- __u8 reserved1c0[8]; /* 0x01c0 */
+ __u64 osda; /* 0x01c0 */
#define ECD_HOSTREGMGMT 0x20000000
#define ECD_MEF 0x08000000
#define ECD_ETOKENF 0x02000000
diff --git a/tools/testing/selftests/kvm/include/s390/sie.h b/tools/testing/selftests/kvm/include/s390/sie.h
index 160acd4a1db9..4ff1c1a354af 100644
--- a/tools/testing/selftests/kvm/include/s390/sie.h
+++ b/tools/testing/selftests/kvm/include/s390/sie.h
@@ -223,7 +223,7 @@ struct kvm_s390_sie_block {
__u32 fac; /* 0x01a0 */
__u8 reserved1a4[20]; /* 0x01a4 */
__u64 cbrlo; /* 0x01b8 */
- __u8 reserved1c0[8]; /* 0x01c0 */
+ __u64 osda; /* 0x01c0 */
#define ECD_HOSTREGMGMT 0x20000000
#define ECD_MEF 0x08000000
#define ECD_ETOKENF 0x02000000
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie
2026-07-09 15:15 ` [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie Christoph Schlameuss
@ 2026-07-09 15:24 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:24 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik, linux-s390, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch regarding `vsie_page` modifications.
--
--- Patch [7]: [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie ---
commit 62a4a049329b153db252c516ebc9d4ec4ac9f106
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie
This commit adds the ssca_block and ssca_entry structures to represent the
shadow state description and original SIGP entries. It also updates the SIE
control block definition to include the original state description address.
> KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie
>
> Add the required guest-1 structures for the vsie_sigpif to the SIE
> control block and vsie_page for use in later patches.
[Severity: Medium]
Is struct vsie_page supposed to be updated in this patch?
The commit message mentions adding structures to vsie_page, but looking
at arch/s390/kvm/vsie.c where struct vsie_page is defined, there are no
changes to it in this diff.
Could a chunk of the patch have been accidentally omitted during a rebase
or patch split, or does the commit message just need to be updated to
reflect the current patch boundaries?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=7
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 08/21] KVM: s390: vsie: Move pin/unpin_scb methods
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (6 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 07/21] KVM: s390: vsie: Add ssca_block and ssca_entry structs for vsie_ie Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 09/21] KVM: s390: vsie: Move pin/unpin guest page Christoph Schlameuss
` (12 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Move {,un}pin_scb() up in preparation for the next patches.
No change intended.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 66 ++++++++++++++++++++++++++--------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 97581b748a94..d663df90af1f 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -660,6 +660,39 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
return rc;
}
+/* unpin the scb provided by guest 2, marking it as dirty */
+static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
+ gpa_t gpa)
+{
+ hpa_t hpa = virt_to_phys(vsie_page->scb_o);
+
+ if (hpa)
+ unpin_guest_page(vcpu->kvm, gpa, hpa);
+ vsie_page->scb_o = NULL;
+}
+
+/*
+ * Pin the scb at gpa provided by guest 2 at vsie_page->scb_o.
+ *
+ * Returns: - 0 if the scb was pinned.
+ * - > 0 if control has to be given to guest 2
+ */
+static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
+ gpa_t gpa)
+{
+ hpa_t hpa;
+ int rc;
+
+ rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
+ if (rc) {
+ rc = kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
+ WARN_ON_ONCE(rc);
+ return 1;
+ }
+ vsie_page->scb_o = phys_to_virt(hpa);
+ return 0;
+}
+
void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, gpa_t start, gpa_t end)
{
struct vsie_page *cur, *next;
@@ -910,39 +943,6 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
return rc;
}
-/* unpin the scb provided by guest 2, marking it as dirty */
-static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
- gpa_t gpa)
-{
- hpa_t hpa = virt_to_phys(vsie_page->scb_o);
-
- if (hpa)
- unpin_guest_page(vcpu->kvm, gpa, hpa);
- vsie_page->scb_o = NULL;
-}
-
-/*
- * Pin the scb at gpa provided by guest 2 at vsie_page->scb_o.
- *
- * Returns: - 0 if the scb was pinned.
- * - > 0 if control has to be given to guest 2
- */
-static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
- gpa_t gpa)
-{
- hpa_t hpa;
- int rc;
-
- rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
- if (rc) {
- rc = kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
- WARN_ON_ONCE(rc);
- return 1;
- }
- vsie_page->scb_o = phys_to_virt(hpa);
- return 0;
-}
-
/*
* Inject a fault into guest 2.
*
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 09/21] KVM: s390: vsie: Move pin/unpin guest page
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (7 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 08/21] KVM: s390: vsie: Move pin/unpin_scb methods Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 10/21] KVM: s390: vsie: Move release/acquire gmap shadow Christoph Schlameuss
` (11 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Move {,un}pin_guest_page() up in preparation for the next patch.
No change intended.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 52 ++++++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index d663df90af1f..cdf271eda88b 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -150,6 +150,32 @@ static int prefix_is_mapped(struct vsie_page *vsie_page)
return !(atomic_read(&vsie_page->scb_s.prog20) & PROG_REQUEST);
}
+/*
+ * Pin the guest page given by gpa and set hpa to the pinned host address.
+ * Will always be pinned writable.
+ *
+ * Returns: - 0 on success
+ * - -EINVAL if the gpa is not valid guest storage
+ */
+static int pin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t *hpa)
+{
+ struct page *page;
+
+ page = gfn_to_page(kvm, gpa_to_gfn(gpa));
+ if (!page)
+ return -EINVAL;
+ *hpa = (hpa_t)page_to_phys(page) + (gpa & ~PAGE_MASK);
+ return 0;
+}
+
+/* Unpins a page previously pinned via pin_guest_page, marking it as dirty. */
+static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
+{
+ kvm_release_page_dirty(pfn_to_page(phys_to_pfn(hpa)));
+ /* mark the page always as dirty for migration */
+ mark_page_dirty(kvm, gpa_to_gfn(gpa));
+}
+
/* copy the updated intervention request bits into the shadow scb */
static void update_intervention_requests(struct vsie_page *vsie_page)
{
@@ -753,32 +779,6 @@ static int map_prefix(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct
return rc;
}
-/*
- * Pin the guest page given by gpa and set hpa to the pinned host address.
- * Will always be pinned writable.
- *
- * Returns: - 0 on success
- * - -EINVAL if the gpa is not valid guest storage
- */
-static int pin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t *hpa)
-{
- struct page *page;
-
- page = gfn_to_page(kvm, gpa_to_gfn(gpa));
- if (!page)
- return -EINVAL;
- *hpa = (hpa_t)page_to_phys(page) + (gpa & ~PAGE_MASK);
- return 0;
-}
-
-/* Unpins a page previously pinned via pin_guest_page, marking it as dirty. */
-static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
-{
- kvm_release_page_dirty(pfn_to_page(phys_to_pfn(hpa)));
- /* mark the page always as dirty for migration */
- mark_page_dirty(kvm, gpa_to_gfn(gpa));
-}
-
/* unpin all blocks previously pinned by pin_blocks(), marking them dirty */
static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 10/21] KVM: s390: vsie: Move release/acquire gmap shadow
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (8 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 09/21] KVM: s390: vsie: Move pin/unpin guest page Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages Christoph Schlameuss
` (10 subsequent siblings)
20 siblings, 0 replies; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Move {release,acquire}_gmap_shadow() up in preparation for the next patch.
No change intended.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 130 +++++++++++++++++++++++++--------------------------
1 file changed, 65 insertions(+), 65 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index cdf271eda88b..5bdd9385266f 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -150,6 +150,71 @@ static int prefix_is_mapped(struct vsie_page *vsie_page)
return !(atomic_read(&vsie_page->scb_s.prog20) & PROG_REQUEST);
}
+static void release_gmap_shadow(struct vsie_page *vsie_page)
+{
+ struct gmap *gmap = vsie_page->gmap_cache.gmap;
+
+ lockdep_assert_held(&gmap->kvm->arch.gmap->children_lock);
+
+ list_del(&vsie_page->gmap_cache.list);
+ vsie_page->gmap_cache.gmap = NULL;
+ prefix_unmapped(vsie_page);
+
+ if (list_empty(&gmap->scb_users)) {
+ gmap_remove_child(gmap);
+ gmap_put(gmap);
+ }
+}
+
+static struct gmap *acquire_gmap_shadow(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
+{
+ union ctlreg0 cr0;
+ struct gmap *gmap;
+ union asce asce;
+ int edat;
+
+ asce.val = vcpu->arch.sie_block->gcr[1];
+ cr0.val = vcpu->arch.sie_block->gcr[0];
+ edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8);
+ edat += edat && test_kvm_facility(vcpu->kvm, 78);
+
+ scoped_guard(spinlock, &vcpu->kvm->arch.gmap->children_lock) {
+ gmap = vsie_page->gmap_cache.gmap;
+ if (gmap) {
+ /*
+ * ASCE or EDAT could have changed since last icpt, or the gmap
+ * we're holding has been unshadowed. If the gmap is still valid,
+ * we can safely reuse it.
+ */
+ if (gmap_is_shadow_valid(gmap, asce, edat)) {
+ vcpu->kvm->stat.gmap_shadow_reuse++;
+ gmap_get(gmap);
+ return gmap;
+ }
+ /* release the old shadow and mark the prefix as unmapped */
+ release_gmap_shadow(vsie_page);
+ }
+ }
+again:
+ gmap = gmap_create_shadow(vcpu->arch.mc, vcpu->kvm->arch.gmap, asce, edat);
+ if (IS_ERR(gmap))
+ return gmap;
+ scoped_guard(spinlock, &vcpu->kvm->arch.gmap->children_lock) {
+ /* unlikely race condition, remove the previous shadow */
+ if (vsie_page->gmap_cache.gmap)
+ release_gmap_shadow(vsie_page);
+ if (!gmap->parent) {
+ gmap_put(gmap);
+ goto again;
+ }
+ vcpu->kvm->stat.gmap_shadow_create++;
+ list_add(&vsie_page->gmap_cache.list, &gmap->scb_users);
+ vsie_page->gmap_cache.gmap = gmap;
+ prefix_unmapped(vsie_page);
+ }
+ return gmap;
+}
+
/*
* Pin the guest page given by gpa and set hpa to the pinned host address.
* Will always be pinned writable.
@@ -1311,71 +1376,6 @@ static int do_vsie_run(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struc
return rc;
}
-static void release_gmap_shadow(struct vsie_page *vsie_page)
-{
- struct gmap *gmap = vsie_page->gmap_cache.gmap;
-
- lockdep_assert_held(&gmap->kvm->arch.gmap->children_lock);
-
- list_del(&vsie_page->gmap_cache.list);
- vsie_page->gmap_cache.gmap = NULL;
- prefix_unmapped(vsie_page);
-
- if (list_empty(&gmap->scb_users)) {
- gmap_remove_child(gmap);
- gmap_put(gmap);
- }
-}
-
-static struct gmap *acquire_gmap_shadow(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
-{
- union ctlreg0 cr0;
- struct gmap *gmap;
- union asce asce;
- int edat;
-
- asce.val = vcpu->arch.sie_block->gcr[1];
- cr0.val = vcpu->arch.sie_block->gcr[0];
- edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8);
- edat += edat && test_kvm_facility(vcpu->kvm, 78);
-
- scoped_guard(spinlock, &vcpu->kvm->arch.gmap->children_lock) {
- gmap = vsie_page->gmap_cache.gmap;
- if (gmap) {
- /*
- * ASCE or EDAT could have changed since last icpt, or the gmap
- * we're holding has been unshadowed. If the gmap is still valid,
- * we can safely reuse it.
- */
- if (gmap_is_shadow_valid(gmap, asce, edat)) {
- vcpu->kvm->stat.gmap_shadow_reuse++;
- gmap_get(gmap);
- return gmap;
- }
- /* release the old shadow and mark the prefix as unmapped */
- release_gmap_shadow(vsie_page);
- }
- }
-again:
- gmap = gmap_create_shadow(vcpu->arch.mc, vcpu->kvm->arch.gmap, asce, edat);
- if (IS_ERR(gmap))
- return gmap;
- scoped_guard(spinlock, &vcpu->kvm->arch.gmap->children_lock) {
- /* unlikely race condition, remove the previous shadow */
- if (vsie_page->gmap_cache.gmap)
- release_gmap_shadow(vsie_page);
- if (!gmap->parent) {
- gmap_put(gmap);
- goto again;
- }
- vcpu->kvm->stat.gmap_shadow_create++;
- list_add(&vsie_page->gmap_cache.list, &gmap->scb_users);
- vsie_page->gmap_cache.gmap = gmap;
- prefix_unmapped(vsie_page);
- }
- return gmap;
-}
-
/*
* Register the shadow scb at the VCPU, e.g. for kicking out of vsie.
*/
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (9 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 10/21] KVM: s390: vsie: Move release/acquire gmap shadow Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:23 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page Christoph Schlameuss
` (9 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Encapsulate allocation of struct vsie_page and free into its own methods
to allow this from multiple code paths.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 5bdd9385266f..4090b4019f21 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1503,6 +1503,24 @@ static void put_vsie_page(struct vsie_page *vsie_page)
clear_bit(VSIE_PAGE_IN_USE, &vsie_page->flags);
}
+static void free_vsie_page(struct vsie_page *vsie_page)
+{
+ free_page((unsigned long)vsie_page);
+}
+
+static struct vsie_page *alloc_vsie_page(struct kvm *kvm)
+{
+ struct vsie_page *vsie_page;
+
+ vsie_page = (struct vsie_page *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
+ if (!vsie_page)
+ return vsie_page;
+
+ /* Mark it as invalid until it resides in the tree. */
+ vsie_page->scb_gpa = ULONG_MAX;
+ return vsie_page;
+}
+
/*
* Get or create a vsie page for a scb address.
*
@@ -1538,7 +1556,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
mutex_lock(&kvm->arch.vsie.mutex);
if (kvm->arch.vsie.page_count < nr_vcpus) {
- vsie_page = (void *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
+ vsie_page = alloc_vsie_page(kvm);
if (!vsie_page) {
mutex_unlock(&kvm->arch.vsie.mutex);
return ERR_PTR(-ENOMEM);
@@ -1558,9 +1576,9 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
if (vsie_page->scb_gpa != ULONG_MAX)
radix_tree_delete(&kvm->arch.vsie.addr_to_page,
vsie_page->scb_gpa >> 9);
+ /* Mark it as invalid until it resides in the tree. */
+ vsie_page->scb_gpa = ULONG_MAX;
}
- /* Mark it as invalid until it resides in the tree. */
- vsie_page->scb_gpa = ULONG_MAX;
/* Double use of the same address or allocation failure. */
if (radix_tree_insert(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page)) {
@@ -1662,7 +1680,7 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
if (vsie_page->scb_gpa != ULONG_MAX)
radix_tree_delete(&kvm->arch.vsie.addr_to_page,
vsie_page->scb_gpa >> 9);
- free_page((unsigned long)vsie_page);
+ free_vsie_page(vsie_page);
}
kvm->arch.vsie.page_count = 0;
mutex_unlock(&kvm->arch.vsie.mutex);
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages
2026-07-09 15:15 ` [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages Christoph Schlameuss
@ 2026-07-09 15:23 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:23 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Vasily Gorbik, Heiko Carstens, kvm, linux-s390, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unused parameter 'kvm' in newly added function `alloc_vsie_page`.
--
--- Patch [11]: [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages ---
commit c4b88feb6783fb127e24e97af58ad89ffa8ef444
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Create helpers to alloc and free vsie_pages
This commit extracts the allocation and freeing of vsie_page structures
into dedicated helper functions. This encapsulation allows these operations
to be reused across multiple code paths.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 5bdd9385266f0..4090b4019f215 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> +static void free_vsie_page(struct vsie_page *vsie_page)
> +{
> + free_page((unsigned long)vsie_page);
> +}
> +
> +static struct vsie_page *alloc_vsie_page(struct kvm *kvm)
> +{
> + struct vsie_page *vsie_page;
> +
> + vsie_page = (struct vsie_page *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
[Severity: Low]
Does this function actually need the kvm parameter? It appears to be completely
unused inside the function body.
> + if (!vsie_page)
> + return vsie_page;
> +
> + /* Mark it as invalid until it resides in the tree. */
> + vsie_page->scb_gpa = ULONG_MAX;
> + return vsie_page;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=11
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (10 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 11/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:25 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages Christoph Schlameuss
` (8 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
The underlying implementation of radix_tree is xarray since v4.20.
Changing to the xarray API directly relaxes some of the locking
obligations and provides more symmetry with the next vsie changes in
the series.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/vsie.c | 36 ++++++++++++++----------------------
2 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index f10f8940bd48..1439f9496bd0 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -624,7 +624,7 @@ struct vsie_page;
struct kvm_s390_vsie {
struct mutex mutex;
- struct radix_tree_root addr_to_page;
+ struct xarray addr_to_page;
int page_count;
int next;
struct vsie_page *pages[KVM_MAX_VCPUS];
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 4090b4019f21..1734215ae73a 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -59,8 +59,7 @@ struct vsie_page {
gpa_t sdnx_gpa; /* 0x0250 */
/*
* guest address of the original SCB. Remains set for free vsie
- * pages, so we can properly look them up in our addr_to_page
- * radix tree.
+ * pages, so we can properly look them up in our addr_to_page map.
*/
gpa_t scb_gpa; /* 0x0258 */
/* the shadow gmap in use by the vsie_page */
@@ -1533,19 +1532,15 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
struct vsie_page *vsie_page;
int nr_vcpus;
- rcu_read_lock();
- vsie_page = radix_tree_lookup(&kvm->arch.vsie.addr_to_page, addr >> 9);
- rcu_read_unlock();
- if (vsie_page) {
- if (try_get_vsie_page(vsie_page)) {
- if (vsie_page->scb_gpa == addr)
- return vsie_page;
- /*
- * We raced with someone reusing + putting this vsie
- * page before we grabbed it.
- */
- put_vsie_page(vsie_page);
- }
+ vsie_page = xa_load(&kvm->arch.vsie.addr_to_page, addr >> 9);
+ if (vsie_page && try_get_vsie_page(vsie_page)) {
+ if (vsie_page->scb_gpa == addr)
+ return vsie_page;
+ /*
+ * We raced with someone reusing + putting this vsie
+ * page before we grabbed it.
+ */
+ put_vsie_page(vsie_page);
}
/*
@@ -1574,14 +1569,13 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
kvm->arch.vsie.next %= nr_vcpus;
}
if (vsie_page->scb_gpa != ULONG_MAX)
- radix_tree_delete(&kvm->arch.vsie.addr_to_page,
- vsie_page->scb_gpa >> 9);
+ xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
/* Mark it as invalid until it resides in the tree. */
vsie_page->scb_gpa = ULONG_MAX;
}
/* Double use of the same address or allocation failure. */
- if (radix_tree_insert(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page)) {
+ if (xa_store(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page, GFP_KERNEL)) {
put_vsie_page(vsie_page);
mutex_unlock(&kvm->arch.vsie.mutex);
return NULL;
@@ -1660,7 +1654,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
void kvm_s390_vsie_init(struct kvm *kvm)
{
mutex_init(&kvm->arch.vsie.mutex);
- INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
+ xa_init_flags(&kvm->arch.vsie.addr_to_page, XA_FLAGS_ALLOC);
}
/* Destroy the vsie data structures. To be called when a vm is destroyed. */
@@ -1676,10 +1670,8 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
if (vsie_page->gmap_cache.gmap)
release_gmap_shadow(vsie_page);
kvm->arch.vsie.pages[i] = NULL;
- /* free the radix tree entry */
if (vsie_page->scb_gpa != ULONG_MAX)
- radix_tree_delete(&kvm->arch.vsie.addr_to_page,
- vsie_page->scb_gpa >> 9);
+ xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
free_vsie_page(vsie_page);
}
kvm->arch.vsie.page_count = 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page
2026-07-09 15:15 ` [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page Christoph Schlameuss
@ 2026-07-09 15:25 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:25 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Vasily Gorbik, Alexander Gordeev, kvm, linux-s390, Heiko Carstens
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unconditional overwrite and logic error in `get_vsie_page` due to `xa_store()` misuse.
- [Medium] Missing `xa_destroy()` causes memory leak of XArray internal nodes during VM teardown.
- [Medium] Incorrect XArray initialization drops memcg accounting and adds free-slot tracking overhead.
--
--- Patch [12]: [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page ---
commit b9607cfaff7f6d624df41b31c8e80cc0a315a2e3
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Replace radix_tree with xarray addr_to_page
This commit replaces the radix_tree implementation in the s390 vsie with
an xarray for addr_to_page. Changing to the xarray API directly relaxes some
of the locking obligations and provides more symmetry with the next vsie
changes in the series.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 4090b4019f215..1734215ae73a9 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -1574,14 +1569,13 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
> kvm->arch.vsie.next %= nr_vcpus;
> }
> if (vsie_page->scb_gpa != ULONG_MAX)
> - radix_tree_delete(&kvm->arch.vsie.addr_to_page,
> - vsie_page->scb_gpa >> 9);
> + xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
> /* Mark it as invalid until it resides in the tree. */
> vsie_page->scb_gpa = ULONG_MAX;
> }
>
> /* Double use of the same address or allocation failure. */
> - if (radix_tree_insert(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page)) {
> + if (xa_store(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page, GFP_KERNEL)) {
[Severity: High]
Does this unintentionally change the error handling semantics by using
xa_store() instead of xa_insert()?
The original radix_tree_insert() correctly refused to overwrite existing
entries and returned an error (e.g., -EEXIST). However, xa_store()
unconditionally overwrites the entry and returns the previous pointer or an
error pointer on failure.
If an entry already exists, xa_store() returns the old vsie_page pointer,
which evaluates to true. The code incorrectly treats this as an error, calling
put_vsie_page() on the newly inserted page while returning NULL. The new page
remains in the xarray but its scb_gpa is left as ULONG_MAX, breaking lookups
for this address.
Also, should this be using GFP_KERNEL_ACCOUNT to maintain the accounting
properties of the original code?
> put_vsie_page(vsie_page);
> mutex_unlock(&kvm->arch.vsie.mutex);
> return NULL;
[ ... ]
> @@ -1660,7 +1654,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
> void kvm_s390_vsie_init(struct kvm *kvm)
> {
> mutex_init(&kvm->arch.vsie.mutex);
> - INIT_RADIX_TREE(&kvm->arch.vsie.addr_to_page, GFP_KERNEL_ACCOUNT);
> + xa_init_flags(&kvm->arch.vsie.addr_to_page, XA_FLAGS_ALLOC);
[Severity: Medium]
Should this use XA_FLAGS_ACCOUNT (or 0) instead of XA_FLAGS_ALLOC?
The original code initialized the radix tree with GFP_KERNEL_ACCOUNT, ensuring
that memory allocated for internal nodes was accounted to the guest's memory
cgroup. Initializing with XA_FLAGS_ALLOC and storing with GFP_KERNEL drops this
accounting.
Additionally, XA_FLAGS_ALLOC enables internal free-slot tracking
(XA_FLAGS_TRACK_FREE) which is typically meant for ID allocators (like
xa_alloc()). Using this on an xarray indexed by sparse physical addresses adds
unnecessary overhead.
> }
>
> /* Destroy the vsie data structures. To be called when a vm is destroyed. */
> @@ -1676,10 +1670,8 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
> if (vsie_page->gmap_cache.gmap)
> release_gmap_shadow(vsie_page);
> kvm->arch.vsie.pages[i] = NULL;
> - /* free the radix tree entry */
> if (vsie_page->scb_gpa != ULONG_MAX)
> - radix_tree_delete(&kvm->arch.vsie.addr_to_page,
> - vsie_page->scb_gpa >> 9);
> + xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
> free_vsie_page(vsie_page);
> }
> kvm->arch.vsie.page_count = 0;
[Severity: Medium]
Will this leak the xarray internal nodes when tearing down a VM?
If any page was left with scb_gpa == ULONG_MAX (e.g., due to the xa_store()
issue mentioned above), xa_erase() is skipped for that entry.
Because the VM destruction path never calls
xa_destroy(&kvm->arch.vsie.addr_to_page), the xarray internal nodes allocated
for any remaining entries will be permanently leaked. Should xa_destroy() be
called here to guarantee all internal memory is freed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=12
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (11 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 12/21] KVM: s390: vsie: Replace radix_tree with xarray addr_to_page Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:28 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow Christoph Schlameuss
` (7 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
In the main patch of this set we need to pin and unpin multiple
consecutive guest-2 pages in guest-1. As these might not be consecutive
in guest-1 it is necessary to iterate over all pages and store guest and
host addresses for later use.
As the new methods to use the existing {,un}pin_guest_page() methods
these are moved up unchanged in the file to prevent to have to resort to
forward declarations later on.
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 1734215ae73a..644b37582b5c 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -71,6 +71,11 @@ struct vsie_page {
static_assert(sizeof(struct vsie_page) == PAGE_SIZE);
+struct kvm_address_pair {
+ gpa_t gpa;
+ hpa_t hpa;
+};
+
static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
{
return (scb->ecb2 & ECB2_ESCA);
@@ -240,6 +245,40 @@ static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
mark_page_dirty(kvm, gpa_to_gfn(gpa));
}
+/* unpin multiple guest pages pinned with pin_guest_pages() */
+static void unpin_guest_pages(struct kvm *kvm, struct kvm_address_pair *addr, unsigned int nr_pages)
+{
+ int i;
+
+ for (i = 0; i < nr_pages; i++) {
+ unpin_guest_page(kvm, addr[i].gpa, addr[i].hpa);
+ addr[i].gpa = 0;
+ addr[i].hpa = 0;
+ }
+}
+
+/* pin nr_pages consecutive guest pages */
+static int pin_guest_pages(struct kvm *kvm, gpa_t gpa, unsigned int nr_pages,
+ struct kvm_address_pair *addr)
+{
+ hpa_t hpa;
+ int i, rc;
+
+ /* the guest pages may not be mapped continuously, so pin each page */
+ for (i = 0; i < nr_pages; i++) {
+ rc = pin_guest_page(kvm, gpa + PAGE_SIZE * i, &hpa);
+ if (rc)
+ goto err;
+ addr[i].gpa = gpa + PAGE_SIZE * i;
+ addr[i].hpa = hpa;
+ }
+ return i;
+
+err:
+ unpin_guest_pages(kvm, addr, i);
+ return -EFAULT;
+}
+
/* copy the updated intervention request bits into the shadow scb */
static void update_intervention_requests(struct vsie_page *vsie_page)
{
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages
2026-07-09 15:15 ` [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages Christoph Schlameuss
@ 2026-07-09 15:28 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:28 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: linux-s390, Heiko Carstens, kvm, Vasily Gorbik, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect address calculation for unaligned GPAs in `pin_guest_pages`, leading to data corruption for multi-page contiguous buffers.
--
--- Patch [13]: [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages ---
commit b2d504ec12f509941a6b52b4ad1605645f085955
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add helper to pin multiple guest pages
This commit introduces helpers to pin and unpin multiple consecutive guest
pages in guest-1. It moves the existing pin and unpin methods to avoid
forward declarations and iterates over pages to store guest and host addresses.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 1734215ae73a9..644b37582b5c8 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -240,6 +245,40 @@ static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa)
> mark_page_dirty(kvm, gpa_to_gfn(gpa));
> }
>
> +/* unpin multiple guest pages pinned with pin_guest_pages() */
> +static void unpin_guest_pages(struct kvm *kvm, struct kvm_address_pair *addr, unsigned int nr_pages)
> +{
> + int i;
> +
> + for (i = 0; i < nr_pages; i++) {
> + unpin_guest_page(kvm, addr[i].gpa, addr[i].hpa);
> + addr[i].gpa = 0;
> + addr[i].hpa = 0;
> + }
> +}
> +
> +/* pin nr_pages consecutive guest pages */
> +static int pin_guest_pages(struct kvm *kvm, gpa_t gpa, unsigned int nr_pages,
> + struct kvm_address_pair *addr)
> +{
> + hpa_t hpa;
> + int i, rc;
> +
> + /* the guest pages may not be mapped continuously, so pin each page */
> + for (i = 0; i < nr_pages; i++) {
> + rc = pin_guest_page(kvm, gpa + PAGE_SIZE * i, &hpa);
[Severity: Medium]
Could this address calculation cause issues if the initial gpa is unaligned?
If a caller passes an unaligned gpa to pin_guest_pages() (for example, 0x1010),
adding PAGE_SIZE * i propagates this initial page offset to every subsequent
page. This would mean the calculation requests 0x2010, 0x3010, and so on,
skipping the guest memory between 0x2000 and 0x200F.
Would it be safer to align the gpa calculation for the subsequent pages to
ensure a truly contiguous buffer is correctly pinned?
> + if (rc)
> + goto err;
> + addr[i].gpa = gpa + PAGE_SIZE * i;
> + addr[i].hpa = hpa;
> + }
> + return i;
> +
> +err:
> + unpin_guest_pages(kvm, addr, i);
> + return -EFAULT;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=13
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (12 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 13/21] KVM: s390: vsie: Add helper to pin multiple guest pages Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:30 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin Christoph Schlameuss
` (6 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Create a helper method release_gmap_shadow_safe() to simplify releasing
the gmap shadow of a vsie_page.
Will be called from multiple places in upcoming patches.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 644b37582b5c..874ed2a55347 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -170,6 +170,15 @@ static void release_gmap_shadow(struct vsie_page *vsie_page)
}
}
+static void release_gmap_shadow_safe(struct kvm *kvm, struct vsie_page *vsie_page)
+{
+ if (vsie_page->gmap_cache.gmap) {
+ scoped_guard(spinlock, &kvm->arch.gmap->children_lock)
+ if (vsie_page->gmap_cache.gmap)
+ release_gmap_shadow(vsie_page);
+ }
+}
+
static struct gmap *acquire_gmap_shadow(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
{
union ctlreg0 cr0;
@@ -1623,11 +1632,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
mutex_unlock(&kvm->arch.vsie.mutex);
memset(&vsie_page->scb_s, 0, sizeof(struct kvm_s390_sie_block));
- if (vsie_page->gmap_cache.gmap) {
- scoped_guard(spinlock, &kvm->arch.gmap->children_lock)
- if (vsie_page->gmap_cache.gmap)
- release_gmap_shadow(vsie_page);
- }
+ release_gmap_shadow_safe(kvm, vsie_page);
prefix_unmapped(vsie_page);
vsie_page->fault_addr = 0;
vsie_page->scb_s.ihcpu = 0xffffU;
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow
2026-07-09 15:15 ` [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow Christoph Schlameuss
@ 2026-07-09 15:30 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:30 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect replacement of `radix_tree_insert` with `xa_store` inadvertently overwrites active VSIE pages on collision.
--
--- Patch [14]: [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow ---
commit 4c401f4e9eb6b9bc115d89b58dc2831bda3b8f47
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add helper to release gmap shadow
Create a helper method release_gmap_shadow_safe() to simplify releasing
the gmap shadow of a vsie_page.
Will be called from multiple places in upcoming patches.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 644b37582b5c8..874ed2a553479 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -1623,11 +1632,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
[Severity: High]
Does replacing radix_tree_insert() with xa_store() inadvertently overwrite
active VSIE pages on collision?
Looking at get_vsie_page() just above this diff context:
/* Double use of the same address or allocation failure. */
if (xa_store(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page, GFP_KERNEL)) {
put_vsie_page(vsie_page);
mutex_unlock(&kvm->arch.vsie.mutex);
return NULL;
}
xa_store() replaces existing entries unconditionally instead of returning
-EEXIST like radix_tree_insert().
When an existing entry is overwritten, it returns the old entry (a truthy
pointer), triggering the error path but leaving the xarray pointing to the
aborted new page.
> mutex_unlock(&kvm->arch.vsie.mutex);
>
> memset(&vsie_page->scb_s, 0, sizeof(struct kvm_s390_sie_block));
> - if (vsie_page->gmap_cache.gmap) {
> - scoped_guard(spinlock, &kvm->arch.gmap->children_lock)
> - if (vsie_page->gmap_cache.gmap)
> - release_gmap_shadow(vsie_page);
> - }
> + release_gmap_shadow_safe(kvm, vsie_page);
> prefix_unmapped(vsie_page);
> vsie_page->fault_addr = 0;
> vsie_page->scb_s.ihcpu = 0xffffU;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=14
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (13 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 14/21] KVM: s390: vsie: Add helper to release gmap shadow Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:31 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1 Christoph Schlameuss
` (5 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Introduce the struct vsie_sca to hold the SSCA and required management
data along with alloc and free methods to use with the struct.
On this basis we can also add methods {,un}pin_sca() to simplify the
pinning SCAs in g1.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/include/asm/kvm_host_types.h | 1 +
arch/s390/kvm/vsie.c | 79 +++++++++++++++++++++++++++++++++-
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/arch/s390/include/asm/kvm_host_types.h b/arch/s390/include/asm/kvm_host_types.h
index 6540146b4b00..d6025b34a66d 100644
--- a/arch/s390/include/asm/kvm_host_types.h
+++ b/arch/s390/include/asm/kvm_host_types.h
@@ -7,6 +7,7 @@
#include <linux/types.h>
#define KVM_S390_MAX_VSIE_VCPUS 256
+#define KVM_S390_MAX_SCA_PAGES 5
#define KVM_S390_BSCA_CPU_SLOTS 64
#define KVM_S390_ESCA_CPU_SLOTS 248
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 874ed2a55347..4b492af15542 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -29,6 +29,7 @@
enum vsie_page_flags {
VSIE_PAGE_IN_USE = 0,
+ VSIE_PAGE_PINNED = 1,
};
struct vsie_page {
@@ -64,7 +65,8 @@ struct vsie_page {
gpa_t scb_gpa; /* 0x0258 */
/* the shadow gmap in use by the vsie_page */
struct gmap_cache gmap_cache; /* 0x0260 */
- __u8 reserved[0x06f8 - 0x0278]; /* 0x0278 */
+ struct vsie_sca *vsie_sca; /* 0x0278 */
+ __u8 reserved[0x06f8 - 0x0280]; /* 0x0280 */
struct kvm_s390_crypto_cb crycb; /* 0x06f8 */
__u8 fac[8 + S390_ARCH_FAC_LIST_SIZE_BYTE];/* 0x07f8 */
};
@@ -76,6 +78,27 @@ struct kvm_address_pair {
hpa_t hpa;
};
+enum vsie_sca_flags {
+ VSIE_SCA_ESCA = 0,
+ VSIE_SCA_PINNED = 1,
+};
+
+struct vsie_sca {
+ struct ssca_block ssca;
+ struct vsie_page *pages[KVM_S390_MAX_VSIE_VCPUS];
+ struct mutex mutex;
+ atomic_t ref_count;
+ gpa_t sca_gpa;
+ unsigned long flags;
+ u64 mcn[4];
+ unsigned long sca_o_nr_pages;
+ struct kvm_address_pair sca_o_pages[KVM_S390_MAX_SCA_PAGES];
+};
+
+static_assert(!(offsetof(struct vsie_sca, ssca) & 0x3f));
+static_assert((offsetof(struct vsie_sca, ssca) & ~PAGE_MASK) +
+ offsetof(struct ssca_block, cpu[0]) <= PAGE_SIZE);
+
static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
{
return (scb->ecb2 & ECB2_ESCA);
@@ -831,6 +854,60 @@ static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
return 0;
}
+/*
+ * Unpin g2 original sca in g1 memory.
+ *
+ * Called with vsie_sca_lock held.
+ */
+static void unpin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
+{
+ if (!test_bit(VSIE_SCA_PINNED, &vsie_sca->flags))
+ return;
+
+ unpin_guest_pages(kvm, vsie_sca->sca_o_pages, vsie_sca->sca_o_nr_pages);
+ vsie_sca->sca_o_nr_pages = 0;
+
+ __clear_bit(VSIE_SCA_PINNED, &vsie_sca->flags);
+}
+
+/*
+ * Pin g2 original sca in g1 memory.
+ *
+ * Called with vsie_sca_lock held.
+ */
+static int pin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
+{
+ bool is_esca = test_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
+ int nr_pages = KVM_S390_MAX_SCA_PAGES;
+
+ if (test_bit(VSIE_SCA_PINNED, &vsie_sca->flags))
+ return 0;
+
+ if (!is_esca) {
+ nr_pages = 1;
+ if ((vsie_sca->sca_gpa & ~PAGE_MASK) + sizeof(struct bsca_block) > PAGE_SIZE)
+ nr_pages = 2;
+ }
+
+ vsie_sca->sca_o_nr_pages = pin_guest_pages(kvm, vsie_sca->sca_gpa, nr_pages,
+ vsie_sca->sca_o_pages);
+ if (WARN_ON_ONCE(vsie_sca->sca_o_nr_pages != nr_pages))
+ return -EIO;
+ __set_bit(VSIE_SCA_PINNED, &vsie_sca->flags);
+
+ return 0;
+}
+
+static void free_vsie_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
+{
+ free_pages_exact(vsie_sca, sizeof(*vsie_sca));
+}
+
+static struct vsie_sca *alloc_vsie_sca(void)
+{
+ return alloc_pages_exact(sizeof(struct vsie_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+}
+
void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, gpa_t start, gpa_t end)
{
struct vsie_page *cur, *next;
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin
2026-07-09 15:15 ` [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin Christoph Schlameuss
@ 2026-07-09 15:31 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:31 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Vasily Gorbik, kvm, linux-s390, Heiko Carstens, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Guest-triggerable WARN_ON_ONCE and negative error code truncation in pin_sca().
- [High] Unconditional pinning of 5 pages for ESCA breaks valid guest configurations due to over-pinning.
- [High] The dynamically allocated `struct vsie_sca` leaves its embedded `mutex` and `ref_count` uninitialized.
- [High] Missing NULL pointer check in `free_vsie_sca` can crash the kernel on error paths.
- [Low] Unused static functions trigger compiler warnings and break builds with `-Werror`.
--
--- Patch [15]: [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin ---
commit e9a459801d0d13c2945b7f63700e761f8e46bb25
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add struct vsie_sca with pin and unpin
Introduce the struct vsie_sca to hold the SSCA and required management
data along with alloc and free methods to use with the struct.
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -831,6 +854,60 @@ static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
[ ... ]
> +static int pin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
> +{
> + bool is_esca = test_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
> + int nr_pages = KVM_S390_MAX_SCA_PAGES;
[Severity: High]
Will this unconditionally pin 5 pages for ESCA blocks, even if the
16,128-byte block only spans 4 pages depending on its alignment?
If a nested guest allocates an ESCA block adjacent to unbacked memory,
could pin_guest_pages() attempt to pin a 5th unbacked page and fail
with -EFAULT, incorrectly rejecting a valid guest configuration?
> +
> + if (test_bit(VSIE_SCA_PINNED, &vsie_sca->flags))
> + return 0;
> +
> + if (!is_esca) {
> + nr_pages = 1;
> + if ((vsie_sca->sca_gpa & ~PAGE_MASK) + sizeof(struct bsca_block) > PAGE_SIZE)
> + nr_pages = 2;
> + }
> +
> + vsie_sca->sca_o_nr_pages = pin_guest_pages(kvm, vsie_sca->sca_gpa, nr_pages,
> + vsie_sca->sca_o_pages);
> + if (WARN_ON_ONCE(vsie_sca->sca_o_nr_pages != nr_pages))
[Severity: High]
If the guest provides an invalid physical address for sca_gpa,
pin_guest_pages() will legitimately fail and return -EFAULT.
Because sca_o_nr_pages is an unsigned long, will this negative error
code be implicitly cast to a huge positive number?
This would cause the WARN_ON_ONCE() to evaluate to true. Does this allow
a malicious guest to intentionally trigger a host kernel warning,
leading to a denial of service if panic_on_warn is enabled?
> + return -EIO;
> + __set_bit(VSIE_SCA_PINNED, &vsie_sca->flags);
> +
> + return 0;
> +}
> +
> +static void free_vsie_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
> +{
> + free_pages_exact(vsie_sca, sizeof(*vsie_sca));
[Severity: High]
Is it possible for free_vsie_sca() to be called with a NULL pointer
during error cleanup?
Unlike kfree(), free_pages_exact() is not NULL-safe for allocations larger
than PAGE_SIZE. Since struct vsie_sca is roughly 10.5 KB, free_pages_exact()
will attempt to free invalid low-memory addresses starting at 4096 on its
second iteration, which would crash the kernel.
> +}
> +
> +static struct vsie_sca *alloc_vsie_sca(void)
> +{
> + return alloc_pages_exact(sizeof(struct vsie_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
[Severity: High]
Does relying solely on __GFP_ZERO leave the embedded mutex and ref_count
uninitialized?
Locking a zero-filled mutex is a structural defect that triggers lockdep
splats or kernel panics because its internal spinlock and wait queues
are not initialized. Should this function call mutex_init() and
atomic_set() before returning the allocated structure?
[Severity: Low]
This patch introduces alloc_vsie_sca(), free_vsie_sca(), pin_sca(), and
unpin_sca() as static functions, but they don't appear to be invoked
anywhere in this file.
Could this trigger -Wunused-function compiler warnings and break builds
that compile with -Werror?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=15
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (14 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 15/21] KVM: s390: vsie: Add struct vsie_sca with pin and unpin Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:37 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif Christoph Schlameuss
` (4 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Restructure kvm_s390_handle_vsie() to create a guest-1 shadow of the SCA
if guest-2 attempts to enter SIE with an SCA. If the SCA is used the
vsie_pages are stored in a new vsie_sca struct instead of the arch vsie
struct.
When the VSIE-Interpretation-Extension Facility is active the shadow SCA
(ssca_block) will be created and shadows of all CPUs defined in the
configuration are created. SCAOL/H in the VSIE control block are
overwritten with references to the shadow SCA.
The shadow SCA contains the addresses of the original guest-3 SCA as
well as the original VSIE control blocks. With these addresses the
machine can directly monitor the intervention bits within the original
SCA entries, enabling it to handle SENSE_RUNNING and EXTERNAL_CALL sigp
instructions without exiting VSIE.
The original SCA will be pinned in guest-2 memory and only be unpinned
before reuse. This means some pages might still be pinned even after the
guest 3 VM does no longer exist.
The vsie_scas including the ssca_blocks are also kept within a map to
reuse already existing ssca_blocks efficiently. While the map and array
with references to the vsie_scas are held in the arch vsie struct. The
use of vsie_scas is tracked using an ref_count.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/include/asm/kvm_host.h | 21 +-
arch/s390/include/asm/kvm_host_types.h | 2 +
arch/s390/kvm/vsie.c | 575 +++++++++++++++++++++++++++++----
3 files changed, 528 insertions(+), 70 deletions(-)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 1439f9496bd0..c0e5caf0eb16 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -621,13 +621,32 @@ struct sie_page2 {
};
struct vsie_page;
+struct vsie_sca;
+/*
+ * vsie_pages, scas and accompanied management vars
+ */
struct kvm_s390_vsie {
struct mutex mutex;
struct xarray addr_to_page;
int page_count;
int next;
- struct vsie_page *pages[KVM_MAX_VCPUS];
+ struct vsie_page *pages[KVM_S390_MAX_VSIE_VCPUS];
+ /*
+ * The vsie_sca_lock is used to synchronize access to
+ * - the kvm_s390_vsie.scas[]
+ * - the kvm_s390_vsie.osca_to_sca map
+ * - new vsie_sca creation and initialization
+ */
+ struct rw_semaphore vsie_sca_lock;
+ struct xarray osca_to_sca;
+ int sca_count;
+ int sca_next;
+ /*
+ * In addition to the use of the array when entering and exiting vsie the scas[] is
+ * accessed from the gmap_notifier without any lock held.
+ */
+ struct vsie_sca *scas[KVM_S390_MAX_VSIE_VCPUS];
};
struct kvm_s390_gisa_iam {
diff --git a/arch/s390/include/asm/kvm_host_types.h b/arch/s390/include/asm/kvm_host_types.h
index d6025b34a66d..ae0c56393475 100644
--- a/arch/s390/include/asm/kvm_host_types.h
+++ b/arch/s390/include/asm/kvm_host_types.h
@@ -6,6 +6,8 @@
#include <linux/atomic.h>
#include <linux/types.h>
+#define BYTES_PER_LONG 8
+
#define KVM_S390_MAX_VSIE_VCPUS 256
#define KVM_S390_MAX_SCA_PAGES 5
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 4b492af15542..74455c689c51 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -86,6 +86,7 @@ enum vsie_sca_flags {
struct vsie_sca {
struct ssca_block ssca;
struct vsie_page *pages[KVM_S390_MAX_VSIE_VCPUS];
+ /* The mutex is used to synchronize access to the pages[] */
struct mutex mutex;
atomic_t ref_count;
gpa_t sca_gpa;
@@ -120,6 +121,17 @@ static void write_scao(struct kvm_s390_sie_block *scb, unsigned long hpa)
scb->scaol = (u32)(u64)hpa;
}
+static inline bool use_vsie_sigpif(struct kvm *kvm, struct kvm_s390_sie_block *scb)
+{
+ if (!kvm->arch.use_vsie_sigpif)
+ return false;
+ if (!(scb->eca & ECA_SIGPI) && !(scb->ecb & ECB_SRSI))
+ return false;
+ if (!read_scao(kvm, scb))
+ return false;
+ return true;
+}
+
/* trigger a validity icpt for the given scb */
static int set_validity_icpt(struct kvm_s390_sie_block *scb,
__u16 reason_code)
@@ -822,14 +834,18 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
}
/* unpin the scb provided by guest 2, marking it as dirty */
-static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
- gpa_t gpa)
+static void unpin_scb(struct kvm *kvm, struct vsie_page *vsie_page)
{
- hpa_t hpa = virt_to_phys(vsie_page->scb_o);
+ hpa_t hpa;
+ if (!test_bit(VSIE_PAGE_PINNED, &vsie_page->flags))
+ return;
+
+ hpa = virt_to_phys(vsie_page->scb_o);
if (hpa)
- unpin_guest_page(vcpu->kvm, gpa, hpa);
+ unpin_guest_page(kvm, vsie_page->scb_gpa, hpa);
vsie_page->scb_o = NULL;
+ __clear_bit(VSIE_PAGE_PINNED, &vsie_page->flags);
}
/*
@@ -838,19 +854,22 @@ static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
* Returns: - 0 if the scb was pinned.
* - > 0 if control has to be given to guest 2
*/
-static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
- gpa_t gpa)
+static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
{
hpa_t hpa;
int rc;
- rc = pin_guest_page(vcpu->kvm, gpa, &hpa);
+ if (test_bit(VSIE_PAGE_PINNED, &vsie_page->flags))
+ return 0;
+
+ rc = pin_guest_page(vcpu->kvm, vsie_page->scb_gpa, &hpa);
if (rc) {
rc = kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
WARN_ON_ONCE(rc);
return 1;
}
vsie_page->scb_o = phys_to_virt(hpa);
+ __set_bit(VSIE_PAGE_PINNED, &vsie_page->flags);
return 0;
}
@@ -898,6 +917,74 @@ static int pin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
return 0;
}
+static int get_sca_entry_addr(struct kvm *kvm, struct vsie_sca *vsie_sca, u16 cpu_nr, gpa_t *gpa,
+ hpa_t *hpa)
+{
+ hpa_t offset;
+ int pn;
+
+ /*
+ * We cannot simply access the hva since the esca_block has typically
+ * 4 pages (arch max 5 pages) that might not be continuous in g1 memory.
+ * The bsca_block may also be stretched over two pages. Only the header
+ * is guaranteed to be on the same page.
+ */
+ if (test_bit(VSIE_SCA_ESCA, &vsie_sca->flags))
+ offset = offsetof(struct esca_block, cpu[cpu_nr]);
+ else
+ offset = offsetof(struct bsca_block, cpu[cpu_nr]);
+ pn = ((vsie_sca->sca_gpa & ~PAGE_MASK) + offset) >> PAGE_SHIFT;
+ if (WARN_ON_ONCE(pn >= vsie_sca->sca_o_nr_pages))
+ return -EINVAL;
+
+ if (gpa)
+ *gpa = vsie_sca->sca_o_pages[pn].gpa + offset;
+ if (hpa)
+ *hpa = vsie_sca->sca_o_pages[pn].hpa + offset;
+ return 0;
+}
+
+static void put_vsie_sca(struct vsie_sca *vsie_sca)
+{
+ if (!vsie_sca)
+ return;
+
+ WARN_ON_ONCE(atomic_dec_return(&vsie_sca->ref_count) < 0);
+}
+
+/*
+ * Try to find the address of an existing shadow system control area.
+ * @sca_o_gpa: original system control area address; guest-2 physical
+ *
+ * Called with lock on vsie_sca_lock.
+ */
+static struct vsie_sca *get_existing_vsie_sca(struct kvm *kvm, hpa_t sca_o_gpa)
+{
+ struct vsie_sca *vsie_sca = xa_load(&kvm->arch.vsie.osca_to_sca, sca_o_gpa >> 6);
+
+ WARN_ON_ONCE(vsie_sca && atomic_inc_return(&vsie_sca->ref_count) < 1);
+ return vsie_sca;
+}
+
+/* Try to find and get a currently unused vsie_sca from the vsie struct. */
+static struct vsie_sca *get_reuseable_vsie_sca(struct kvm *kvm)
+{
+ struct vsie_sca *vsie_sca;
+ int i, ref_count;
+
+ for (i = 0; i < kvm->arch.vsie.sca_count; i++) {
+ vsie_sca = READ_ONCE(kvm->arch.vsie.scas[kvm->arch.vsie.sca_next]);
+ kvm->arch.vsie.sca_next++;
+ kvm->arch.vsie.sca_next %= kvm->arch.vsie.sca_count;
+ ref_count = atomic_inc_return(&vsie_sca->ref_count);
+ WARN_ON_ONCE(ref_count < 1);
+ if (ref_count == 1)
+ return vsie_sca;
+ atomic_dec(&vsie_sca->ref_count);
+ }
+ return ERR_PTR(-EAGAIN);
+}
+
static void free_vsie_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
{
free_pages_exact(vsie_sca, sizeof(*vsie_sca));
@@ -905,7 +992,131 @@ static void free_vsie_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
static struct vsie_sca *alloc_vsie_sca(void)
{
- return alloc_pages_exact(sizeof(struct vsie_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+ struct vsie_sca *vsie_sca;
+
+ vsie_sca = alloc_pages_exact(sizeof(*vsie_sca), GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+ if (vsie_sca)
+ mutex_init(&vsie_sca->mutex);
+
+ return vsie_sca;
+}
+
+/* Clear the vsie_sca struct but keep the vsie_page references and ref_count */
+static void clear_vsie_sca(struct vsie_sca *vsie_sca)
+{
+ static_assert(offsetof(struct vsie_sca, pages) < offsetof(struct vsie_sca, ref_count));
+
+ unsigned long offset = offsetof(struct vsie_sca, ref_count) + sizeof(vsie_sca->ref_count);
+ void *vsie_sca_data = (void *)vsie_sca + offset;
+
+ memset(&vsie_sca->ssca, 0, sizeof(vsie_sca->ssca));
+ memset(vsie_sca_data, 0, sizeof(struct vsie_sca) - offset);
+}
+
+/* Pin and get an existing or new guest-3 system control area.*/
+static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_block *scb_o,
+ gpa_t sca_gpa)
+{
+ struct vsie_sca *vsie_sca, *vsie_sca_new = NULL;
+ struct vsie_page *vsie_page_n;
+ struct kvm *kvm = vcpu->kvm;
+ unsigned int max_vsie_sca;
+ int rc, cpu_nr;
+
+ rc = validate_scao(vcpu, scb_o, sca_gpa);
+ if (rc)
+ return ERR_PTR(-EINVAL);
+
+ down_read(&kvm->arch.vsie.vsie_sca_lock);
+ vsie_sca = get_existing_vsie_sca(kvm, sca_gpa);
+ up_read(&kvm->arch.vsie.vsie_sca_lock);
+ if (vsie_sca)
+ return vsie_sca;
+
+ /*
+ * Allocate new vsie_sca, it will likely be needed below.
+ * We want at least #online_vcpus shadows, so every VCPU can execute the
+ * VSIE in parallel. (Worst case all single core VMs.)
+ */
+ max_vsie_sca = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
+
+ if (kvm->arch.vsie.sca_count < max_vsie_sca) {
+ vsie_sca_new = alloc_vsie_sca();
+ if (!vsie_sca_new)
+ return ERR_PTR(-ENOMEM);
+ }
+
+ /*
+ * Now we're taking the vsie_sca_lock in write mode so that we can manipulate
+ * the radix tree and recheck for existing SCAs with exclusive access.
+ *
+ * In the next lines we try three things to get an SCA:
+ * - Retry getting an existing vsie_sca
+ * - Using our newly allocated vsie_sca if we're under the limit
+ * - Reusing an vsie_sca including ssca to shadow a different osca
+ */
+ down_write(&kvm->arch.vsie.vsie_sca_lock);
+ vsie_sca = get_existing_vsie_sca(kvm, sca_gpa);
+ if (vsie_sca)
+ goto out;
+
+ /* check again under write lock if we are still under our vsie_sca limit */
+ if (vsie_sca_new && kvm->arch.vsie.sca_count < max_vsie_sca) {
+ /* make use of vsie_sca just created */
+ vsie_sca = vsie_sca_new;
+ vsie_sca_new = NULL;
+
+ kvm->arch.vsie.scas[kvm->arch.vsie.sca_count] = vsie_sca;
+ kvm->arch.vsie.sca_count++;
+ atomic_set(&vsie_sca->ref_count, 1);
+ } else {
+ /* reuse previously created vsie_sca allocation for different osca */
+ vsie_sca = get_reuseable_vsie_sca(kvm);
+ /* with nr_vcpus scas one must be reusable */
+ if (IS_ERR(vsie_sca))
+ goto out;
+ WARN_ON_ONCE(atomic_read(&vsie_sca->ref_count) != 1);
+
+ xa_erase(&kvm->arch.vsie.osca_to_sca, vsie_sca->sca_gpa >> 6);
+ for (cpu_nr = 0; cpu_nr < KVM_S390_MAX_VSIE_VCPUS; cpu_nr++) {
+ vsie_page_n = vsie_sca->pages[cpu_nr];
+ if (!vsie_page_n)
+ continue;
+
+ /* unpin but keep the vsie_page for reuse */
+ unpin_scb(kvm, vsie_page_n);
+ release_gmap_shadow_safe(kvm, vsie_page_n);
+ memset(vsie_page_n, 0, sizeof(struct vsie_page));
+ vsie_page_n->scb_gpa = ULONG_MAX;
+ }
+ unpin_sca(kvm, vsie_sca);
+ clear_vsie_sca(vsie_sca);
+ }
+
+ if (sie_uses_esca(scb_o))
+ __set_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
+ vsie_sca->sca_gpa = sca_gpa;
+
+ /*
+ * The pinned original sca will only be unpinned lazily to limit the
+ * required amount of pins/unpins on each vsie entry/exit.
+ * The unpin is done in the reuse vsie_sca allocation path above and
+ * kvm_s390_vsie_destroy().
+ */
+ rc = pin_sca(kvm, vsie_sca);
+ if (rc) {
+ vsie_sca = ERR_PTR(rc);
+ goto out;
+ }
+
+ WARN_ON_ONCE(xa_store(&kvm->arch.vsie.osca_to_sca,
+ vsie_sca->sca_gpa >> 6, vsie_sca, GFP_KERNEL));
+
+out:
+ up_write(&kvm->arch.vsie.vsie_sca_lock);
+ if (vsie_sca_new)
+ free_vsie_sca(kvm, vsie_sca_new);
+ return vsie_sca;
}
void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, gpa_t start, gpa_t end)
@@ -974,11 +1185,13 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
hpa_t hpa;
- hpa = read_scao(vcpu->kvm, scb_s);
- if (hpa) {
- unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
- vsie_page->sca_gpa = 0;
- write_scao(scb_s, 0);
+ if (!vsie_page->vsie_sca) {
+ hpa = read_scao(vcpu->kvm, scb_s);
+ if (hpa) {
+ unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
+ vsie_page->sca_gpa = 0;
+ write_scao(scb_s, 0);
+ }
}
hpa = scb_s->itdba;
@@ -1017,9 +1230,6 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
* This works as long as the data lies in one page. If blocks ever exceed one
* page, we have to fall back to shadowing.
*
- * As we reuse the sca, the vcpu pointers contained in it are invalid. We must
- * therefore not enable any facilities that access these pointers (e.g. SIGPIF).
- *
* Returns: - 0 if all blocks were pinned.
* - > 0 if control has to be given to guest 2
* - -ENOMEM if out of memory
@@ -1032,8 +1242,8 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
gpa_t gpa;
int rc = 0;
- gpa = read_scao(vcpu->kvm, scb_o);
- if (gpa) {
+ gpa = vsie_page->sca_gpa;
+ if (gpa && !vsie_page->vsie_sca) {
rc = validate_scao(vcpu, scb_o, gpa);
if (rc)
goto unpin;
@@ -1042,7 +1252,6 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
rc = set_validity_icpt(scb_s, 0x0034U);
goto unpin;
}
- vsie_page->sca_gpa = gpa;
write_scao(scb_s, hpa);
}
@@ -1645,17 +1854,42 @@ static struct vsie_page *alloc_vsie_page(struct kvm *kvm)
return vsie_page;
}
+static int vsie_page_init(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, unsigned long scb_gpa)
+{
+ struct kvm *kvm = vcpu->kvm;
+ int rc;
+
+ if (vsie_page->scb_gpa != ULONG_MAX)
+ xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
+ vsie_page->scb_gpa = scb_gpa;
+ rc = pin_scb(vcpu, vsie_page);
+ if (rc)
+ return -ENOMEM;
+
+ vsie_page->sca_gpa = read_scao(kvm, vsie_page->scb_o);
+ WARN_ON_ONCE(xa_store(&kvm->arch.vsie.addr_to_page, scb_gpa >> 9, vsie_page, GFP_KERNEL));
+
+ return 0;
+}
+
/*
* Get or create a vsie page for a scb address.
*
+ * Original control blocks are pinned when the vsie_page pointing to them is
+ * returned.
+ * Newly created vsie_pages only have vsie_page->scb_gpa and vsie_page->sca_gpa
+ * set.
+ *
* Returns: - address of a vsie page (cached or new one)
* - NULL if the same scb address is already used by another VCPU
* - ERR_PTR(-ENOMEM) if out of memory
*/
-static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
+static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr)
{
- struct vsie_page *vsie_page;
- int nr_vcpus;
+ struct vsie_page *vsie_page, *vsie_page_new = NULL;
+ struct kvm *kvm = vcpu->kvm;
+ unsigned int max_vsie_page;
+ int rc, pages_idx;
vsie_page = xa_load(&kvm->arch.vsie.addr_to_page, addr >> 9);
if (vsie_page && try_get_vsie_page(vsie_page)) {
@@ -1668,59 +1902,205 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
put_vsie_page(vsie_page);
}
- /*
- * We want at least #online_vcpus shadows, so every VCPU can execute
- * the VSIE in parallel.
- */
- nr_vcpus = atomic_read(&kvm->online_vcpus);
+ max_vsie_page = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
+
+ /* allocate new vsie_page - we will likely need it */
+ if (kvm->arch.vsie.page_count < max_vsie_page) {
+ vsie_page_new = alloc_vsie_page(kvm);
+ if (!vsie_page_new)
+ return ERR_PTR(-ENOMEM);
+ __set_bit(VSIE_PAGE_IN_USE, &vsie_page_new->flags);
+ }
mutex_lock(&kvm->arch.vsie.mutex);
- if (kvm->arch.vsie.page_count < nr_vcpus) {
- vsie_page = alloc_vsie_page(kvm);
- if (!vsie_page) {
+ vsie_page = xa_load(&kvm->arch.vsie.addr_to_page, addr >> 9);
+ if (vsie_page && try_get_vsie_page(vsie_page)) {
+ if (vsie_page->scb_gpa == addr) {
mutex_unlock(&kvm->arch.vsie.mutex);
- return ERR_PTR(-ENOMEM);
+ if (vsie_page_new)
+ free_vsie_page(vsie_page_new);
+ return vsie_page;
}
- __set_bit(VSIE_PAGE_IN_USE, &vsie_page->flags);
- kvm->arch.vsie.pages[kvm->arch.vsie.page_count] = vsie_page;
+ /*
+ * We raced with someone reusing + putting this vsie
+ * page before we grabbed it.
+ */
+ put_vsie_page(vsie_page);
+ }
+
+ if (kvm->arch.vsie.page_count < max_vsie_page) {
+ pages_idx = kvm->arch.vsie.page_count;
+ vsie_page = vsie_page_new;
+ vsie_page_new = NULL;
+ WRITE_ONCE(kvm->arch.vsie.pages[kvm->arch.vsie.page_count], vsie_page);
kvm->arch.vsie.page_count++;
} else {
/* reuse an existing entry that belongs to nobody */
while (true) {
- vsie_page = kvm->arch.vsie.pages[kvm->arch.vsie.next];
+ pages_idx = kvm->arch.vsie.next;
+ kvm->arch.vsie.next++;
+ kvm->arch.vsie.next %= kvm->arch.vsie.page_count;
+ vsie_page = kvm->arch.vsie.pages[pages_idx];
if (try_get_vsie_page(vsie_page))
break;
- kvm->arch.vsie.next++;
- kvm->arch.vsie.next %= nr_vcpus;
}
- if (vsie_page->scb_gpa != ULONG_MAX)
- xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
- /* Mark it as invalid until it resides in the tree. */
- vsie_page->scb_gpa = ULONG_MAX;
+
+ unpin_scb(kvm, vsie_page);
}
- /* Double use of the same address or allocation failure. */
- if (xa_store(&kvm->arch.vsie.addr_to_page, addr >> 9, vsie_page, GFP_KERNEL)) {
+ rc = vsie_page_init(vcpu, vsie_page, addr);
+ mutex_unlock(&kvm->arch.vsie.mutex);
+ if (vsie_page_new)
+ free_vsie_page(vsie_page_new);
+ if (WARN_ON_ONCE(rc)) {
+ unpin_scb(kvm, vsie_page);
+ vsie_page->scb_gpa = ULONG_MAX;
put_vsie_page(vsie_page);
- mutex_unlock(&kvm->arch.vsie.mutex);
- return NULL;
+ return ERR_PTR(rc);
}
- vsie_page->scb_gpa = addr;
- mutex_unlock(&kvm->arch.vsie.mutex);
memset(&vsie_page->scb_s, 0, sizeof(struct kvm_s390_sie_block));
release_gmap_shadow_safe(kvm, vsie_page);
prefix_unmapped(vsie_page);
vsie_page->fault_addr = 0;
vsie_page->scb_s.ihcpu = 0xffffU;
+
return vsie_page;
}
+static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie_sca *vsie_sca,
+ gpa_t scb_gpa, u16 cpu_nr)
+{
+ struct vsie_page *vsie_page, *vsie_page_new = NULL;
+ int rc;
+
+ vsie_page = vsie_sca->pages[cpu_nr];
+ if (!vsie_page) {
+ vsie_page_new = alloc_vsie_page(vcpu->kvm);
+ if (!vsie_page_new)
+ return ERR_PTR(-ENOMEM);
+ vsie_page_new->vsie_sca = vsie_sca;
+ __set_bit(VSIE_PAGE_IN_USE, &vsie_page_new->flags);
+
+ /* be careful to not loose a page here if we raced */
+ scoped_guard(mutex, &vsie_sca->mutex) {
+ vsie_page = vsie_sca->pages[cpu_nr];
+ if (!vsie_page) {
+ WRITE_ONCE(vsie_sca->pages[cpu_nr], vsie_page_new);
+ vsie_page = vsie_page_new;
+ }
+ }
+ }
+ if (vsie_page != vsie_page_new) {
+ if (vsie_page_new)
+ free_vsie_page(vsie_page_new);
+
+ /* not a new vsie_page so get it */
+ if (!try_get_vsie_page(vsie_page))
+ return ERR_PTR(-EAGAIN);
+ vsie_page->vsie_sca = vsie_sca;
+ }
+ if (vsie_page->scb_gpa != scb_gpa || vsie_page->sca_gpa != vsie_sca->sca_gpa) {
+ scoped_guard(mutex, &vcpu->kvm->arch.vsie.mutex) {
+ unpin_scb(vcpu->kvm, vsie_page);
+ rc = vsie_page_init(vcpu, vsie_page, scb_gpa);
+ }
+ if (WARN_ON_ONCE(rc)) {
+ put_vsie_page(vsie_page);
+ return ERR_PTR(rc);
+ }
+ }
+
+ return vsie_page;
+}
+
+static void vsie_sca_update(struct vsie_sca *vsie_sca, unsigned int cpu_nr,
+ struct vsie_page *vsie_page_n, hpa_t sca_o_entry_hpa)
+{
+ vsie_sca->ssca.cpu[cpu_nr].ssda = virt_to_phys(&vsie_page_n->scb_s);
+ vsie_sca->ssca.cpu[cpu_nr].ossea = sca_o_entry_hpa;
+ WRITE_ONCE(vsie_sca->pages[cpu_nr], vsie_page_n);
+}
+
+/* Fill the shadow system control area used for VSIE SIGPI. */
+static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
+ struct vsie_sca *vsie_sca)
+{
+ hpa_t sca_o_entry_hpa, sca_o_hpa = vsie_sca->sca_o_pages[0].hpa;
+ bool is_esca = sie_uses_esca(vsie_page->scb_o);
+ unsigned int cpu_nr, cpu_slots;
+ struct vsie_page *vsie_page_n;
+ hva_t sca_o_entry_hva;
+ unsigned long *mcn;
+ gpa_t scb_o_gpa;
+ int rc;
+
+ if (is_esca)
+ mcn = phys_to_virt(sca_o_hpa) + offsetof(struct esca_block, mcn);
+ else
+ mcn = phys_to_virt(sca_o_hpa) + offsetof(struct bsca_block, mcn);
+
+ /* pin and make shadow for ALL scb in the sca */
+ cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
+ for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
+ rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
+ if (rc)
+ goto err;
+
+ if (vsie_page->scb_o->icpua == cpu_nr) {
+ vsie_sca_update(vsie_sca, cpu_nr, vsie_page, sca_o_entry_hpa);
+ } else {
+ sca_o_entry_hva = (hva_t)phys_to_virt(sca_o_entry_hpa);
+ if (is_esca)
+ scb_o_gpa = ((struct esca_entry *)sca_o_entry_hva)->sda;
+ else
+ scb_o_gpa = ((struct bsca_entry *)sca_o_entry_hva)->sda;
+ vsie_page_n = get_vsie_page_cpu_nr(vcpu, vsie_sca, scb_o_gpa, cpu_nr);
+ if (!vsie_page_n)
+ rc = -EAGAIN;
+ if (IS_ERR(vsie_page_n))
+ rc = PTR_ERR(vsie_page_n);
+ if (rc)
+ goto err;
+ rc = shadow_scb(vcpu, vsie_page_n);
+ vsie_sca_update(vsie_sca, cpu_nr, vsie_page_n, sca_o_entry_hpa);
+ put_vsie_page(vsie_page_n);
+ if (rc)
+ goto err;
+ }
+ }
+ vsie_sca->ssca.osca = sca_o_hpa;
+
+ return 0;
+
+err:
+ for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
+ vsie_sca->ssca.cpu[cpu_nr].ssda = 0;
+ vsie_sca->ssca.cpu[cpu_nr].ossea = 0;
+ }
+ return rc;
+}
+
+/* Shadow or reshadow the SCA on VSIE enter. */
+static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct vsie_sca *vsie_sca)
+{
+ int rc = 0;
+
+ guard(rwsem_write)(&vcpu->kvm->arch.vsie.vsie_sca_lock);
+ if (!vsie_sca->ssca.osca)
+ rc = _shadow_sca(vcpu, vsie_page, vsie_sca);
+
+ return rc;
+}
+
int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
{
+ struct kvm_s390_sie_block *scb_o;
+ struct vsie_sca *vsie_sca = NULL;
struct vsie_page *vsie_page;
- unsigned long scb_addr;
- int rc;
+ gpa_t scb_addr;
+ hpa_t scb_hpa;
+ int rc = 0;
vcpu->stat.instruction_sie++;
if (!test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_SIEF2))
@@ -1731,7 +2111,7 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
scb_addr = kvm_s390_get_base_disp_s(vcpu, NULL);
/* 512 byte alignment */
- if (unlikely(scb_addr & 0x1ffUL))
+ if (unlikely(!scb_addr || scb_addr & 0x1ffUL))
return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
if (kvm_s390_vcpu_has_irq(vcpu, 0) || kvm_s390_vcpu_sie_inhibited(vcpu)) {
@@ -1739,34 +2119,65 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
return 0;
}
- vsie_page = get_vsie_page(vcpu->kvm, scb_addr);
- if (IS_ERR(vsie_page)) {
- return PTR_ERR(vsie_page);
- } else if (!vsie_page) {
+ rc = pin_guest_page(vcpu->kvm, scb_addr, &scb_hpa);
+ if (rc)
+ return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
+ scb_o = (struct kvm_s390_sie_block *)phys_to_virt(scb_hpa);
+
+ if (!use_vsie_sigpif(vcpu->kvm, scb_o)) {
+ /* get the vsie_page with pinned scb_o */
+ vsie_page = get_vsie_page(vcpu, scb_addr);
+ if (IS_ERR(vsie_page)) {
+ rc = PTR_ERR(vsie_page);
+ goto out_unpin;
+ }
+ } else {
+ /* get the vsie_sca with pinned sca_gpa */
+ vsie_sca = get_vsie_sca(vcpu, scb_o, read_scao(vcpu->kvm, scb_o));
+ if (IS_ERR(vsie_sca)) {
+ rc = PTR_ERR(vsie_sca);
+ goto out_unpin;
+ }
+ vsie_page = get_vsie_page_cpu_nr(vcpu, vsie_sca, scb_addr, scb_o->icpua);
+ if (IS_ERR(vsie_page)) {
+ rc = PTR_ERR(vsie_page);
+ goto out_put_sca;
+ }
+ }
+ if (!vsie_page) {
/* double use of sie control block - simply do nothing */
kvm_s390_rewind_psw(vcpu, 4);
- return 0;
+ rc = 0;
+ goto out_put_sca;
}
- rc = pin_scb(vcpu, vsie_page, scb_addr);
- if (rc)
- goto out_put;
rc = shadow_scb(vcpu, vsie_page);
if (rc)
- goto out_unpin_scb;
+ goto out_put;
+ if (vsie_sca) {
+ /* pin and shadow the sca including all scb_o in the g3 conf */
+ rc = shadow_sca(vcpu, vsie_page, vsie_sca);
+ if (rc)
+ goto out_unshadow;
+ }
+
rc = pin_blocks(vcpu, vsie_page);
if (rc)
goto out_unshadow;
register_shadow_scb(vcpu, vsie_page);
+
rc = vsie_run(vcpu, vsie_page);
+
unregister_shadow_scb(vcpu);
unpin_blocks(vcpu, vsie_page);
out_unshadow:
unshadow_scb(vcpu, vsie_page);
-out_unpin_scb:
- unpin_scb(vcpu, vsie_page, scb_addr);
out_put:
put_vsie_page(vsie_page);
+out_put_sca:
+ put_vsie_sca(vsie_sca);
+out_unpin:
+ unpin_guest_page(vcpu->kvm, scb_addr, scb_hpa);
return rc < 0 ? rc : 0;
}
@@ -1776,27 +2187,53 @@ void kvm_s390_vsie_init(struct kvm *kvm)
{
mutex_init(&kvm->arch.vsie.mutex);
xa_init_flags(&kvm->arch.vsie.addr_to_page, XA_FLAGS_ALLOC);
+ init_rwsem(&kvm->arch.vsie.vsie_sca_lock);
+ xa_init_flags(&kvm->arch.vsie.osca_to_sca, XA_FLAGS_ALLOC);
+}
+
+static void kvm_s390_vsie_destroy_page(struct kvm *kvm, struct vsie_page *vsie_page)
+{
+ scoped_guard(spinlock, &kvm->arch.gmap->children_lock)
+ if (vsie_page->gmap_cache.gmap)
+ release_gmap_shadow(vsie_page);
+ /* free the radix tree entry */
+ if (vsie_page->scb_gpa != ULONG_MAX)
+ xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
+ free_vsie_page(vsie_page);
}
/* Destroy the vsie data structures. To be called when a vm is destroyed. */
void kvm_s390_vsie_destroy(struct kvm *kvm)
{
struct vsie_page *vsie_page;
- int i;
+ struct vsie_sca *vsie_sca;
+ int i, cpu_nr;
+
+ guard(mutex)(&kvm->arch.vsie.mutex);
- mutex_lock(&kvm->arch.vsie.mutex);
for (i = 0; i < kvm->arch.vsie.page_count; i++) {
vsie_page = kvm->arch.vsie.pages[i];
- scoped_guard(spinlock, &kvm->arch.gmap->children_lock)
- if (vsie_page->gmap_cache.gmap)
- release_gmap_shadow(vsie_page);
kvm->arch.vsie.pages[i] = NULL;
- if (vsie_page->scb_gpa != ULONG_MAX)
- xa_erase(&kvm->arch.vsie.addr_to_page, vsie_page->scb_gpa >> 9);
- free_vsie_page(vsie_page);
+ kvm_s390_vsie_destroy_page(kvm, vsie_page);
}
kvm->arch.vsie.page_count = 0;
- mutex_unlock(&kvm->arch.vsie.mutex);
+ for (i = 0; i < kvm->arch.vsie.sca_count; i++) {
+ vsie_sca = kvm->arch.vsie.scas[i];
+ kvm->arch.vsie.scas[i] = NULL;
+
+ for (cpu_nr = 0; cpu_nr < KVM_S390_MAX_VSIE_VCPUS; cpu_nr++) {
+ vsie_page = vsie_sca->pages[cpu_nr];
+ vsie_sca->pages[cpu_nr] = NULL;
+ if (!vsie_page)
+ continue;
+ unpin_scb(kvm, vsie_page);
+ kvm_s390_vsie_destroy_page(kvm, vsie_page);
+ }
+
+ unpin_sca(kvm, vsie_sca);
+ free_vsie_sca(kvm, vsie_sca);
+ }
+ kvm->arch.vsie.sca_count = 0;
}
void kvm_s390_vsie_kick(struct kvm_vcpu *vcpu)
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1
2026-07-09 15:15 ` [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1 Christoph Schlameuss
@ 2026-07-09 15:37 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:37 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Heiko Carstens, Vasily Gorbik, kvm, Alexander Gordeev, linux-s390
Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [Critical] Incorrect pointer arithmetic in `get_sca_entry_addr()` computes wildly out-of-bounds host and guest physical addresses.
- [Critical] Incomplete error rollback in `get_vsie_page_cpu_nr()` leads to host NULL pointer dereference and XArray corruption.
- [Critical] Guest-controlled `icpua` field from the SIE control block is used directly as an array index without bounds checking, leading to arbitrary OOB reads and writes.
- [Critical] Concurrent `memset` of a shared `vsie_page` during SCA reuse causes data corruption and potential Host DoS.
- [Critical] Shadow SCBs are left unprotected during guest execution because `VSIE_PAGE_IN_USE` is prematurely cleared.
- [High] Unvalidated guest-provided `sda` in the SCA allows out-of-bounds reads of host physical memory.
- [High] Failure in `pin_sca()` permanently leaks the `vsie_sca` reference, allowing a malicious guest to exhaust the VSIE SCA pool.
- [High] Stale `vsie_sca` pointers in cached `vsie_page`s break SCB execution when a guest toggles SIGPIF off, and enable Use-After-Free.
- [High] KVM caches the guest's SCA address in `vsie_page->sca_gpa` and ignores dynamic updates to the SCB's SCAO field.
- [High] Mismatched locking causes a data race on the `vsie_sca->pages[]` array.
- [Medium] Multiple guest-triggerable `WARN_ON_ONCE` assertions enable host syslog flooding and Denial of Service.
- [Medium] XArray internal nodes and elements are permanently leaked on VM destruction and SCA reuse due to missing `xa_destroy()` and incomplete `xa_erase()` cleanup.
--
--- Patch [16]: [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1 ---
commit 9e60c38b165cf3838037fe34e3e571d246e77218
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Shadow VSIE SCA in guest-1
This commit restructures the vsie handler to create a guest-1 shadow of the
SCA if guest-2 attempts to enter SIE with an SCA. It adds the vsie_sca
struct and manages its lifecycle and lookup via a map in the arch vsie
struct.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 4b492af155429..74455c689c51b 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -694,6 +694,22 @@ static int shadow_scb_minimal(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
[ ... ]
> scb_s->ecb2 |= scb_o->ecb2 & ECB2_ESCA;
>
> if (vsie_page->vsie_sca) {
> scb_s->eca |= scb_o->eca & ECA_SIGPI;
> scb_s->ecb |= scb_o->ecb & ECB_SRSI;
> write_scao(scb_s, virt_to_phys(&vsie_page->vsie_sca->ssca));
> scb_s->osda = virt_to_phys(scb_o);
> }
[Severity: High]
Does this code use a stale vsie_sca pointer if the guest executes the same
SCB with SIGPIF disabled?
When get_vsie_page() retrieves a cached vsie_page, it does not clear
vsie_page->vsie_sca. If the guest previously executed this SCB with SIGPIF
enabled, but now executes it with SIGPIF disabled, will shadow_scb_minimal()
forcefully inject SIGPIF configuration using a stale pointer? Since vsie_sca
can be recycled, could this cause cross-guest isolation breaches?
[ ... ]
> @@ -927,6 +927,21 @@ static int pin_sca(struct kvm *kvm, struct vsie_sca *vsie_sca)
[ ... ]
> vsie_sca->sca_o_nr_pages = pin_guest_pages(kvm, vsie_sca->sca_gpa, nr_pages,
> vsie_sca->sca_o_pages);
> if (WARN_ON_ONCE(vsie_sca->sca_o_nr_pages != nr_pages))
> return -EIO;
[Severity: Medium]
Is this WARN_ON_ONCE triggerable by a malicious guest?
If the guest provides an unmapped sca_gpa, pin_guest_pages() will return
-EFAULT. This causes an underflow when compared to nr_pages, triggering the
warning. Should guest-driven errors return directly without asserting to
prevent syslog flooding?
[ ... ]
> +static int get_sca_entry_addr(struct kvm *kvm, struct vsie_sca *vsie_sca, u16 cpu_nr, gpa_t *gpa,
> + hpa_t *hpa)
> +{
> + hpa_t offset;
> + int pn;
> +
> + /*
> + * We cannot simply access the hva since the esca_block has typically
> + * 4 pages (arch max 5 pages) that might not be continuous in g1 memory.
> + * The bsca_block may also be stretched over two pages. Only the header
> + * is guaranteed to be on the same page.
> + */
> + if (test_bit(VSIE_SCA_ESCA, &vsie_sca->flags))
> + offset = offsetof(struct esca_block, cpu[cpu_nr]);
> + else
> + offset = offsetof(struct bsca_block, cpu[cpu_nr]);
> + pn = ((vsie_sca->sca_gpa & ~PAGE_MASK) + offset) >> PAGE_SHIFT;
> + if (WARN_ON_ONCE(pn >= vsie_sca->sca_o_nr_pages))
> + return -EINVAL;
> +
> + if (gpa)
> + *gpa = vsie_sca->sca_o_pages[pn].gpa + offset;
> + if (hpa)
> + *hpa = vsie_sca->sca_o_pages[pn].hpa + offset;
> + return 0;
> +}
[Severity: Critical]
Does this address arithmetic overshoot the pinned page boundaries?
The offset calculated here is the full structural offset from the start of
the SCA (which can be up to 5 * PAGE_SIZE). When calculating the hpa, this
full offset is added to the base physical address of the pn-th page, rather
than just the offset within that specific page. Can this instruct the
hardware to access unpinned, arbitrary host physical memory?
[ ... ]
> +static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_block *scb_o,
> + gpa_t sca_gpa)
> +{
[ ... ]
> + } else {
> + /* reuse previously created vsie_sca allocation for different osca */
> + vsie_sca = get_reuseable_vsie_sca(kvm);
> + /* with nr_vcpus scas one must be reusable */
> + if (IS_ERR(vsie_sca))
> + goto out;
> + WARN_ON_ONCE(atomic_read(&vsie_sca->ref_count) != 1);
> +
> + xa_erase(&kvm->arch.vsie.osca_to_sca, vsie_sca->sca_gpa >> 6);
> + for (cpu_nr = 0; cpu_nr < KVM_S390_MAX_VSIE_VCPUS; cpu_nr++) {
> + vsie_page_n = vsie_sca->pages[cpu_nr];
> + if (!vsie_page_n)
> + continue;
> +
> + /* unpin but keep the vsie_page for reuse */
> + unpin_scb(kvm, vsie_page_n);
> + release_gmap_shadow_safe(kvm, vsie_page_n);
> + memset(vsie_page_n, 0, sizeof(struct vsie_page));
[Severity: Critical]
Is it safe to memset the vsie_page while it is still in the addr_to_page
XArray?
The vsie_page is zeroed out here, clearing its VSIE_PAGE_IN_USE flag, but it
is never removed from the addr_to_page XArray. A concurrent VCPU executing
get_vsie_page() can perform a lockless xa_load(), observe the in-use flag as
0, and claim the page. Could this cause the first thread to clear scb_o
while the second thread actively executes shadow_scb() using the now-NULL
pointer?
> + vsie_page_n->scb_gpa = ULONG_MAX;
[Severity: Medium]
Does this code leak XArray entries?
Because scb_gpa is set to ULONG_MAX without calling xa_erase(), does this
leave dangling pointers and leak XArray internal nodes?
> + }
> + unpin_sca(kvm, vsie_sca);
> + clear_vsie_sca(vsie_sca);
> + }
> +
> + if (sie_uses_esca(scb_o))
> + __set_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
> + vsie_sca->sca_gpa = sca_gpa;
> +
> + /*
> + * The pinned original sca will only be unpinned lazily to limit the
> + * required amount of pins/unpins on each vsie entry/exit.
> + * The unpin is done in the reuse vsie_sca allocation path above and
> + * kvm_s390_vsie_destroy().
> + */
> + rc = pin_sca(kvm, vsie_sca);
> + if (rc) {
> + vsie_sca = ERR_PTR(rc);
> + goto out;
> + }
[Severity: High]
Does this error path leak the vsie_sca reference?
The ref_count is initialized to 1 earlier. If pin_sca() fails (e.g., due to
an unmapped guest sca_gpa), the function returns ERR_PTR(rc) and the caller
bypasses put_vsie_sca(). Since get_reuseable_vsie_sca() expects a ref_count
of 0 for reuse, will these stranded objects permanently exhaust the finite
pool of vsie_sca structures?
[ ... ]
> @@ -1032,8 +1242,8 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> gpa_t gpa;
> int rc = 0;
>
> - gpa = read_scao(vcpu->kvm, scb_o);
> - if (gpa) {
> + gpa = vsie_page->sca_gpa;
> + if (gpa && !vsie_page->vsie_sca) {
> rc = validate_scao(vcpu, scb_o, gpa);
[Severity: High]
Will this ignore dynamic updates to the guest's SCAO field?
By reading the cached vsie_page->sca_gpa instead of dynamically calling
read_scao(), KVM seems to lock the SCA address permanently after it is first
set in vsie_page_init(). If a guest updates the SCAO field for a reused SCB,
will KVM silently continue pinning and shadowing the old address?
[ ... ]
> +static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie_sca *vsie_sca,
> + gpa_t scb_gpa, u16 cpu_nr)
> +{
> + struct vsie_page *vsie_page, *vsie_page_new = NULL;
> + int rc;
> +
> + vsie_page = vsie_sca->pages[cpu_nr];
[Severity: Critical]
Is the cpu_nr index validated against the bounds of the pages array?
The cpu_nr parameter comes directly from the guest-controlled icpua field.
Since it is not bounds-checked against KVM_S390_MAX_VSIE_VCPUS, can a
malicious guest provide an out-of-bounds or negative index, causing KVM to
read from and later overwrite arbitrary host memory with a newly allocated
page pointer?
> + if (!vsie_page) {
> + vsie_page_new = alloc_vsie_page(vcpu->kvm);
> + if (!vsie_page_new)
> + return ERR_PTR(-ENOMEM);
> + vsie_page_new->vsie_sca = vsie_sca;
> + __set_bit(VSIE_PAGE_IN_USE, &vsie_page_new->flags);
> +
> + /* be careful to not loose a page here if we raced */
> + scoped_guard(mutex, &vsie_sca->mutex) {
> + vsie_page = vsie_sca->pages[cpu_nr];
> + if (!vsie_page) {
> + WRITE_ONCE(vsie_sca->pages[cpu_nr], vsie_page_new);
[Severity: High]
Does this create a data race on the vsie_sca->pages array?
This code writes to the array while holding vsie_sca->mutex. However,
vsie_sca_update() also writes to this array but holds vsie_sca_lock instead
of vsie_sca->mutex. Could this lock mismatch corrupt array pointers?
> + vsie_page = vsie_page_new;
> + }
> + }
> + }
[ ... ]
> + if (vsie_page->scb_gpa != scb_gpa || vsie_page->sca_gpa != vsie_sca->sca_gpa) {
> + scoped_guard(mutex, &vcpu->kvm->arch.vsie.mutex) {
> + unpin_scb(vcpu->kvm, vsie_page);
> + rc = vsie_page_init(vcpu, vsie_page, scb_gpa);
> + }
> + if (WARN_ON_ONCE(rc)) {
> + put_vsie_page(vsie_page);
> + return ERR_PTR(rc);
> + }
> + }
[Severity: Critical]
Does this error path leave vsie_page->scb_gpa with a stale value?
If vsie_page_init() fails to pin the SCB, it returns an error but leaves
vsie_page->scb_gpa updated to the unpinnable address. By returning ERR_PTR
here without rolling back scb_gpa to ULONG_MAX, if this page is encountered
again, vsie_page_init() will see the stale value and mistakenly call
xa_erase(). Can this arbitrarily delete a valid entry belonging to another
VCPU's SCB?
Also, since the error is driven by guest-controlled input (an unpinnable
SCB address), does WARN_ON_ONCE(rc) allow a guest to spam the host syslog?
[ ... ]
> +static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
> + struct vsie_sca *vsie_sca)
> +{
[ ... ]
> + for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
> + rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
> + if (rc)
> + goto err;
> +
> + if (vsie_page->scb_o->icpua == cpu_nr) {
> + vsie_sca_update(vsie_sca, cpu_nr, vsie_page, sca_o_entry_hpa);
> + } else {
> + sca_o_entry_hva = (hva_t)phys_to_virt(sca_o_entry_hpa);
> + if (is_esca)
> + scb_o_gpa = ((struct esca_entry *)sca_o_entry_hva)->sda;
> + else
> + scb_o_gpa = ((struct bsca_entry *)sca_o_entry_hva)->sda;
[Severity: High]
Are alignment checks missing when reading the guest-provided sda?
The guest-provided sda is read from the SCA and passed to
get_vsie_page_cpu_nr(), which maps a single 4096-byte page. If KVM does not
enforce 512-byte alignment on sda, could a malicious guest provide an address
near the end of the page? This would cause the 512-byte shadow_scb() read to
cross the boundary of the pinned page into adjacent, unpinned host physical
memory.
> + vsie_page_n = get_vsie_page_cpu_nr(vcpu, vsie_sca, scb_o_gpa, cpu_nr);
> + if (!vsie_page_n)
> + rc = -EAGAIN;
> + if (IS_ERR(vsie_page_n))
> + rc = PTR_ERR(vsie_page_n);
> + if (rc)
> + goto err;
> + rc = shadow_scb(vcpu, vsie_page_n);
> + vsie_sca_update(vsie_sca, cpu_nr, vsie_page_n, sca_o_entry_hpa);
> + put_vsie_page(vsie_page_n);
> + if (rc)
> + goto err;
> + }
> + }
[Severity: Critical]
Does calling put_vsie_page() prematurely expose the shadow SCB?
put_vsie_page() clears the VSIE_PAGE_IN_USE flag. Because these pages reside
in the global addr_to_page pool, another host VCPU looking for a free page
can observe the in-use flag as 0 and claim it. Can this concurrent VCPU call
vsie_page_init() and overwrite the memory of the shadow SCB while the
hardware is actively using it for nested execution?
[ ... ]
> void kvm_s390_vsie_destroy(struct kvm *kvm)
> {
> struct vsie_page *vsie_page;
> - int i;
> + struct vsie_sca *vsie_sca;
> + int i, cpu_nr;
> +
> + guard(mutex)(&kvm->arch.vsie.mutex);
[ ... ]
> + for (i = 0; i < kvm->arch.vsie.sca_count; i++) {
> + vsie_sca = kvm->arch.vsie.scas[i];
> + kvm->arch.vsie.scas[i] = NULL;
> +
> + for (cpu_nr = 0; cpu_nr < KVM_S390_MAX_VSIE_VCPUS; cpu_nr++) {
> + vsie_page = vsie_sca->pages[cpu_nr];
> + vsie_sca->pages[cpu_nr] = NULL;
> + if (!vsie_page)
> + continue;
> + unpin_scb(kvm, vsie_page);
> + kvm_s390_vsie_destroy_page(kvm, vsie_page);
> + }
> +
> + unpin_sca(kvm, vsie_sca);
> + free_vsie_sca(kvm, vsie_sca);
> + }
> + kvm->arch.vsie.sca_count = 0;
> }
[Severity: Medium]
Does this teardown leak XArray entries?
When freeing vsie_sca objects, there are no calls to xa_erase() to remove
them from the osca_to_sca XArray. Furthermore, xa_destroy() is missing for
both addr_to_page and osca_to_sca. Will this permanently leak dynamically
allocated XArray internal nodes every time a VM is destroyed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=16
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (15 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 16/21] KVM: s390: vsie: Shadow VSIE SCA in guest-1 Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:35 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration Christoph Schlameuss
` (3 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
As we are shadowing the SCA we need to add and remove the pointers to
the shadowed control blocks and sca entries whenever the mcn /
processors in the configuration changes.
It is not expected that the mcn changes frequently for a already running
guest-3 configuration. So we can simply fully re-init the ssca whenever
the mcn changes.
To detect the mcn change we store the expected mcn in the struct
vsie_sca when running init_ssca().
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 68 ++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 56 insertions(+), 12 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 74455c689c51..de8931e714ca 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -2014,6 +2014,43 @@ static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie
return vsie_page;
}
+/*
+ * Copy the mcn from the osca to the vsie_sca to be able to detect mcn changes later on.
+ *
+ * @vsie_sca: vsie_sca to copy mcn to.
+ * @sca: Pointer to a struct bsca_block or struct esca_block to read from.
+ */
+static void sca_mcn_copy(struct vsie_sca *vsie_sca, void *sca)
+{
+ int offset = offsetof(struct bsca_block, mcn);
+ int size = BYTES_PER_LONG;
+
+ if (test_bit(VSIE_SCA_ESCA, &vsie_sca->flags)) {
+ offset = offsetof(struct esca_block, mcn);
+ size = BYTES_PER_LONG * 4;
+ }
+ memcpy(&vsie_sca->mcn, sca + offset, size);
+}
+
+/*
+ * Compare the mcn from the given sca to the vsie_sca to be able to detect mcn changes.
+ *
+ * @vsie_sca: vsie_sca to compare mcn to.
+ * @sca: Pointer to a struct bsca_block or struct esca_block to compare to.
+ */
+static bool sca_mcn_equals(struct vsie_sca *vsie_sca, void *sca)
+{
+ int offset = offsetof(struct bsca_block, mcn);
+ int size = BYTES_PER_LONG;
+
+ if (test_bit(VSIE_SCA_ESCA, &vsie_sca->flags)) {
+ size = BYTES_PER_LONG * 4;
+ offset = offsetof(struct esca_block, mcn);
+ }
+
+ return !memcmp(&vsie_sca->mcn, sca + offset, size);
+}
+
static void vsie_sca_update(struct vsie_sca *vsie_sca, unsigned int cpu_nr,
struct vsie_page *vsie_page_n, hpa_t sca_o_entry_hpa)
{
@@ -2031,18 +2068,14 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
unsigned int cpu_nr, cpu_slots;
struct vsie_page *vsie_page_n;
hva_t sca_o_entry_hva;
- unsigned long *mcn;
gpa_t scb_o_gpa;
int rc;
- if (is_esca)
- mcn = phys_to_virt(sca_o_hpa) + offsetof(struct esca_block, mcn);
- else
- mcn = phys_to_virt(sca_o_hpa) + offsetof(struct bsca_block, mcn);
+ sca_mcn_copy(vsie_sca, phys_to_virt(sca_o_hpa));
/* pin and make shadow for ALL scb in the sca */
cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
- for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
+ for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {
rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
if (rc)
goto err;
@@ -2074,7 +2107,7 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
return 0;
err:
- for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
+ for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {
vsie_sca->ssca.cpu[cpu_nr].ssda = 0;
vsie_sca->ssca.cpu[cpu_nr].ossea = 0;
}
@@ -2084,13 +2117,24 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
/* Shadow or reshadow the SCA on VSIE enter. */
static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct vsie_sca *vsie_sca)
{
- int rc = 0;
+ struct kvm_s390_sie_block *sca_o = phys_to_virt(vsie_sca->sca_o_pages[0].hpa);
+ bool do_init_ssca;
- guard(rwsem_write)(&vcpu->kvm->arch.vsie.vsie_sca_lock);
- if (!vsie_sca->ssca.osca)
- rc = _shadow_sca(vcpu, vsie_page, vsie_sca);
+ scoped_guard(rwsem_read, &vcpu->kvm->arch.vsie.vsie_sca_lock) {
+ do_init_ssca = !vsie_sca->ssca.osca;
+ do_init_ssca = do_init_ssca || !sca_mcn_equals(vsie_sca, sca_o);
+ if (!do_init_ssca)
+ return 0;
+ }
- return rc;
+ scoped_guard(rwsem_write, &vcpu->kvm->arch.vsie.vsie_sca_lock) {
+ do_init_ssca = !vsie_sca->ssca.osca;
+ do_init_ssca = do_init_ssca || !sca_mcn_equals(vsie_sca, sca_o);
+ if (do_init_ssca)
+ return _shadow_sca(vcpu, vsie_page, vsie_sca);
+ }
+
+ return 0;
}
int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif
2026-07-09 15:15 ` [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif Christoph Schlameuss
@ 2026-07-09 15:35 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:35 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: kvm, linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] If `_shadow_sca()` encounters a transient error, it prematurely updates the cached MCN but zeroes out the shadow SCA, permanently breaking VSIE SIGP handling.
- [High] Dynamic MCN updates fail to clear shadow SCA entries for removed vCPUs, leaving stale pointers that the hardware may access.
- [High] Mismatch between cached `vsie_sca->flags` and actual guest SCA format leads to copying garbage MCN data from wrong offsets.
- [Low] Incorrect pointer type `struct kvm_s390_sie_block *` is used for the guest SCA page.
Pre-existing issues:
- [Critical] get_sca_entry_addr() incorrectly calculates the host physical address (HPA) of SCA entries located past the first page, leading to arbitrary host physical memory access.
--
--- Patch [17]: [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif ---
commit c4d503d35381889298738f17763bcbfdd845d493
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif
This commit enables dynamically adding and removing guest-3 CPUs in KVM
by shadowing the SCA and re-initializing the shadow SCA whenever the guest's
multi-cpu configuration (MCN) changes. It introduces caching and comparison
of the expected MCN to detect when the layout changes.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 74455c689c51b..de8931e714ca2 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> +static void sca_mcn_copy(struct vsie_sca *vsie_sca, void *sca)
> +{
> + int offset = offsetof(struct bsca_block, mcn);
> + int size = BYTES_PER_LONG;
> +
> + if (test_bit(VSIE_SCA_ESCA, &vsie_sca->flags)) {
[Severity: High]
If a guest switches a control block from BSCA to ESCA (or vice versa) while
keeping the same physical sca_gpa, will the cached VSIE_SCA_ESCA flag in
vsie_sca->flags be stale?
Later in _shadow_sca(), the layout is determined dynamically using
sie_uses_esca(vsie_page->scb_o). If these disagree, could sca_mcn_copy() read
garbage memory from the wrong MCN offset?
> + offset = offsetof(struct esca_block, mcn);
> + size = BYTES_PER_LONG * 4;
> + }
> + memcpy(&vsie_sca->mcn, sca + offset, size);
> +}
[ ... ]
> static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
> struct vsie_sca *vsie_sca)
> {
[ ... ]
> - if (is_esca)
> - mcn = phys_to_virt(sca_o_hpa) + offsetof(struct esca_block, mcn);
> - else
> - mcn = phys_to_virt(sca_o_hpa) + offsetof(struct bsca_block, mcn);
> + sca_mcn_copy(vsie_sca, phys_to_virt(sca_o_hpa));
[Severity: High]
If _shadow_sca() encounters an error later in this function, could this early
copy of the MCN cause permanent failures?
If _shadow_sca() returns an error, vsie_sca->mcn will already be updated to
the new value, while the error path zeroes out the shadow SCA but leaves
vsie_sca->ssca.osca unmodified.
On the next VSIE entry retry, shadow_sca() will see that sca_mcn_equals() is
true, and skip re-initializing the shadow SCA. Does this leave the guest
with a broken SIGP configuration permanently?
>
> /* pin and make shadow for ALL scb in the sca */
> cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
> - for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
> + for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {
[Severity: High]
Does this loop exclusively iterate over the bits set in the new MCN?
If a vCPU was removed by the guest (its MCN bit transitions from 1 to 0), the
loop skips that CPU index entirely. Does this mean the ssda and ossea
pointers for the removed CPU are never cleared, leaving stale pointers that
the hardware might still access?
> rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
[Severity: Critical]
This is a pre-existing issue, but looking at get_sca_entry_addr() called
here, is there a problem with how it calculates the host physical address
for SCA entries past the first page?
arch/s390/kvm/vsie.c:get_sca_entry_addr() {
...
if (hpa)
*hpa = vsie_sca->sca_o_pages[pn].hpa + offset;
...
}
Because offset is the total byte offset from the start of the SCA block, and
sca_o_pages[pn].hpa is already the physical address of the specific pn-th
page, does adding offset again push the resulting HPA massively out of
bounds?
> if (rc)
> goto err;
[ ... ]
> /* Shadow or reshadow the SCA on VSIE enter. */
> static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct vsie_sca *vsie_sca)
> {
> - int rc = 0;
> + struct kvm_s390_sie_block *sca_o = phys_to_virt(vsie_sca->sca_o_pages[0].hpa);
[Severity: Low]
This isn't a bug, but sca_o is typed as a State Control Block
(struct kvm_s390_sie_block *) when the memory appears to actually be a System
Control Area (struct bsca_block or struct esca_block). Could this static
typing make the code confusing to read?
> + bool do_init_ssca;
>
> - guard(rwsem_write)(&vcpu->kvm->arch.vsie.vsie_sca_lock);
> - if (!vsie_sca->ssca.osca)
> - rc = _shadow_sca(vcpu, vsie_page, vsie_sca);
> + scoped_guard(rwsem_read, &vcpu->kvm->arch.vsie.vsie_sca_lock) {
> + do_init_ssca = !vsie_sca->ssca.osca;
> + do_init_ssca = do_init_ssca || !sca_mcn_equals(vsie_sca, sca_o);
> + if (!do_init_ssca)
> + return 0;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=17
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (16 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with vsie sigpif Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:35 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters Christoph Schlameuss
` (2 subsequent siblings)
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Introduce two new module parameters allowing to keep more shadow
structures
* vsie_shadow_scb_max
Override the maximum number of VSIE control blocks / vsie_pages to
shadow in guest-1. KVM will either use this value or the number of
current VCPUs. Either way the number will be capped to 256.
This is the number of guest-3 control blocks / CPUs to keep shadowed
to minimize the repeated shadowing effort.
* vsie_shadow_sca_max
Override the maximum number of VSIE system control areas to
shadow in guest-1. KVM will use a minimum of the current number of
vCPUs and a maximum of 256 or this value if it is lower.
This is the number of guest-3 system control areas / VMs to keep
shadowed to minimize repeated shadowing effort.
Set to 0 to disable the SCA shadowing and VSIE SIGPI.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index de8931e714ca..c392f7afe130 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -100,6 +100,16 @@ static_assert(!(offsetof(struct vsie_sca, ssca) & 0x3f));
static_assert((offsetof(struct vsie_sca, ssca) & ~PAGE_MASK) +
offsetof(struct ssca_block, cpu[0]) <= PAGE_SIZE);
+/* maximum vsie shadow scb */
+unsigned int vsie_shadow_scb_max = 1;
+module_param(vsie_shadow_scb_max, uint, 0644);
+MODULE_PARM_DESC(vsie_shadow_scb_max, "Maximum number of VSIE shadow control blocks to keep. Values smaller number VCPUs uses number of VCPUs; maximum 256");
+
+/* maximum vsie shadow sca */
+unsigned int vsie_shadow_sca_max = 1;
+module_param(vsie_shadow_sca_max, uint, 0644);
+MODULE_PARM_DESC(vsie_shadow_sca_max, "Maximum number of VSIE shadow system control areas to keep. Values smaller number of VCPUs uses number of VCPUs; maximum 256");
+
static inline bool sie_uses_esca(struct kvm_s390_sie_block *scb)
{
return (scb->ecb2 & ECB2_ESCA);
@@ -1038,7 +1048,8 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_
* We want at least #online_vcpus shadows, so every VCPU can execute the
* VSIE in parallel. (Worst case all single core VMs.)
*/
- max_vsie_sca = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
+ max_vsie_sca = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_sca_max),
+ KVM_S390_MAX_VSIE_VCPUS);
if (kvm->arch.vsie.sca_count < max_vsie_sca) {
vsie_sca_new = alloc_vsie_sca();
@@ -1902,7 +1913,8 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
put_vsie_page(vsie_page);
}
- max_vsie_page = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
+ max_vsie_page = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_scb_max),
+ KVM_S390_MAX_VSIE_VCPUS);
/* allocate new vsie_page - we will likely need it */
if (kvm->arch.vsie.page_count < max_vsie_page) {
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration
2026-07-09 15:15 ` [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration Christoph Schlameuss
@ 2026-07-09 15:35 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:35 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: linux-s390, Heiko Carstens, Alexander Gordeev, kvm, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using the `MIN()` and `MAX()` macros on concurrently mutable module parameters causes a TOCTOU vulnerability leading to out-of-bounds array writes.
- [Medium] Setting `vsie_shadow_sca_max` to 0 fails to disable SCA shadowing, breaking the documented administrative feature.
--
--- Patch [18]: [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration ---
commit 20f6b4acf8a001837c97129c0036793d506ee32c
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add VSIE max shadow configuration
This commit introduces vsie_shadow_scb_max and vsie_shadow_sca_max
module parameters to control the maximum number of shadow structures
kept by KVM.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index de8931e714ca2..c392f7afe130c 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -1038,7 +1048,8 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_
> * We want at least #online_vcpus shadows, so every VCPU can execute the
> * VSIE in parallel. (Worst case all single core VMs.)
> */
> - max_vsie_sca = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
> + max_vsie_sca = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_sca_max),
> + KVM_S390_MAX_VSIE_VCPUS);
[Severity: Medium]
Does setting vsie_shadow_sca_max to 0 actually disable SCA shadowing
as documented in the commit message?
Since online_vcpus is at least 1 for an active VM, MAX(online_vcpus, 0)
will always evaluate to online_vcpus. It appears max_vsie_sca silently
defaults to the number of active VCPUs instead of 0, which would prevent
the feature from being disabled.
[Severity: High]
Could the use of MIN() and MAX() macros on a mutable module parameter lead
to a time-of-check to time-of-use out-of-bounds write?
The MIN() and MAX() macros evaluate their arguments multiple times. If
vsie_shadow_sca_max is concurrently modified via sysfs from a value < 256
to a value > 256 during evaluation, could this macro return the new
out-of-bounds value?
If max_vsie_sca exceeds 256, it could allow kvm->arch.vsie.sca_count to
exceed array capacities, leading to an out-of-bounds write here:
kvm->arch.vsie.scas[kvm->arch.vsie.sca_count] = vsie_sca;
[ ... ]
> @@ -1902,7 +1913,8 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
> put_vsie_page(vsie_page);
> }
>
> - max_vsie_page = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
> + max_vsie_page = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_scb_max),
> + KVM_S390_MAX_VSIE_VCPUS);
[Severity: High]
Does this have the same double-evaluation vulnerability as max_vsie_sca?
If vsie_shadow_scb_max is modified during macro evaluation, max_vsie_page
could bypass the KVM_S390_MAX_VSIE_VCPUS limit, leading to out-of-bounds
writes on the kvm->arch.vsie.pages array.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=18
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (17 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:30 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks Christoph Schlameuss
2026-07-09 15:15 ` [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI Christoph Schlameuss
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Add new stat counters to VSIE shadowing to be able to verify and monitor
the functionality.
* vsie_shadow_scb shows the number of allocated SIE control block
shadows. Should count upwards between 0 and the max number of active
CPUs or configured value.
* vsie_shadow_sca shows the number of allocated system control area
shadows. Should count upwards between 0 and the max number of active
CPUs or configured value.
* vsie_shadow_scb_reuse shows the number of reused SIE control block
shadows.
* vsie_shadow_sca_reuse shows the number of reused system control area
shadows.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/include/asm/kvm_host.h | 4 ++++
arch/s390/kvm/kvm-s390.c | 4 ++++
arch/s390/kvm/vsie.c | 6 ++++++
3 files changed, 14 insertions(+)
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index c0e5caf0eb16..575fd67941f7 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -466,6 +466,10 @@ struct kvm_vm_stat {
u64 gmap_shadow_r3_entry;
u64 gmap_shadow_sg_entry;
u64 gmap_shadow_pg_entry;
+ u64 vsie_shadow_scb;
+ u64 vsie_shadow_scb_reuse;
+ u64 vsie_shadow_sca;
+ u64 vsie_shadow_sca_reuse;
};
struct kvm_arch_memory_slot {
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 6749198bac3f..481e566fe228 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -86,6 +86,10 @@ const struct kvm_stats_desc kvm_vm_stats_desc[] = {
STATS_DESC_COUNTER(VM, gmap_shadow_r3_entry),
STATS_DESC_COUNTER(VM, gmap_shadow_sg_entry),
STATS_DESC_COUNTER(VM, gmap_shadow_pg_entry),
+ STATS_DESC_COUNTER(VM, vsie_shadow_scb),
+ STATS_DESC_COUNTER(VM, vsie_shadow_scb_reuse),
+ STATS_DESC_COUNTER(VM, vsie_shadow_sca),
+ STATS_DESC_COUNTER(VM, vsie_shadow_sca_reuse),
};
const struct kvm_stats_header kvm_vm_stats_header = {
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index c392f7afe130..80050376eaab 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -1080,6 +1080,7 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_
kvm->arch.vsie.scas[kvm->arch.vsie.sca_count] = vsie_sca;
kvm->arch.vsie.sca_count++;
atomic_set(&vsie_sca->ref_count, 1);
+ kvm->stat.vsie_shadow_sca++;
} else {
/* reuse previously created vsie_sca allocation for different osca */
vsie_sca = get_reuseable_vsie_sca(kvm);
@@ -1102,6 +1103,7 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_
}
unpin_sca(kvm, vsie_sca);
clear_vsie_sca(vsie_sca);
+ kvm->stat.vsie_shadow_sca_reuse++;
}
if (sie_uses_esca(scb_o))
@@ -1946,6 +1948,7 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
vsie_page_new = NULL;
WRITE_ONCE(kvm->arch.vsie.pages[kvm->arch.vsie.page_count], vsie_page);
kvm->arch.vsie.page_count++;
+ kvm->stat.vsie_shadow_scb++;
} else {
/* reuse an existing entry that belongs to nobody */
while (true) {
@@ -1958,6 +1961,7 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
}
unpin_scb(kvm, vsie_page);
+ kvm->stat.vsie_shadow_scb_reuse++;
}
rc = vsie_page_init(vcpu, vsie_page, addr);
@@ -2002,6 +2006,7 @@ static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie
vsie_page = vsie_page_new;
}
}
+ vcpu->kvm->stat.vsie_shadow_scb++;
}
if (vsie_page != vsie_page_new) {
if (vsie_page_new)
@@ -2011,6 +2016,7 @@ static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie
if (!try_get_vsie_page(vsie_page))
return ERR_PTR(-EAGAIN);
vsie_page->vsie_sca = vsie_sca;
+ vcpu->kvm->stat.vsie_shadow_scb_reuse++;
}
if (vsie_page->scb_gpa != scb_gpa || vsie_page->sca_gpa != vsie_sca->sca_gpa) {
scoped_guard(mutex, &vcpu->kvm->arch.vsie.mutex) {
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters
2026-07-09 15:15 ` [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters Christoph Schlameuss
@ 2026-07-09 15:30 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:30 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: Heiko Carstens, Alexander Gordeev, linux-s390, kvm, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The vsie_shadow_scb stat counter overcounts allocations during races, and vsie_shadow_scb_reuse incorrectly counts cache hits rather than resource repurposing.
--
--- Patch [19]: [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters ---
commit b0e742d7460b00786ba09124150753300ba413ad
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Add VSIE shadow stat counters
This patch adds new statistics counters to VSIE shadowing to monitor
functionality, specifically tracking the allocations and reuses of SIE
control block and system control area shadows.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index c392f7afe130c..80050376eaab1 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -2002,6 +2006,7 @@ static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie
> vsie_page = vsie_page_new;
> }
> }
> + vcpu->kvm->stat.vsie_shadow_scb++;
[Severity: Medium]
Does this increment the vsie_shadow_scb counter even when we lose the race
and subsequently free the newly allocated page? It looks like this might
cause the counter to overcount and increment beyond the configured maximum
value when concurrent allocations occur.
> }
> if (vsie_page != vsie_page_new) {
> if (vsie_page_new)
> @@ -2011,6 +2016,7 @@ static struct vsie_page *get_vsie_page_cpu_nr(struct kvm_vcpu *vcpu, struct vsie
> if (!try_get_vsie_page(vsie_page))
> return ERR_PTR(-EAGAIN);
> vsie_page->vsie_sca = vsie_sca;
> + vcpu->kvm->stat.vsie_shadow_scb_reuse++;
[Severity: Medium]
Does this increment vsie_shadow_scb_reuse on every cache hit rather than
just on evictions or repurposing?
The guest-2 path appears to only count actual resource repurposing, whereas
counting every cache hit here will cause the metric to increase continuously
during normal execution.
> }
> if (vsie_page->scb_gpa != scb_gpa || vsie_page->sca_gpa != vsie_sca->sca_gpa) {
> scoped_guard(mutex, &vcpu->kvm->arch.vsie.mutex) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=19
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (18 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 19/21] KVM: s390: vsie: Add VSIE shadow stat counters Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:47 ` sashiko-bot
2026-07-09 15:15 ` [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI Christoph Schlameuss
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Instead of creating a full shadow for vSIE control blocks only create a
minimal shadow for SIGP purposes for currently not running guest-3
blocks.
The shadow will still be recreated fully when an vSIE is entered with
that scb later.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/vsie.c | 54 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 80050376eaab..feac755d56cb 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -348,8 +348,7 @@ static void update_intervention_requests(struct vsie_page *vsie_page)
static int prepare_cpuflags(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
{
struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
- struct kvm_s390_sie_block *scb_o = vsie_page->scb_o;
- int newflags, cpuflags = atomic_read(&scb_o->cpuflags);
+ int newflags, cpuflags = atomic_read(&vsie_page->scb_o->cpuflags);
/* we don't allow ESA/390 guests unless explicitly enabled */
if (!(cpuflags & CPUSTAT_ZARCH) && !vcpu->kvm->arch.allow_vsie_esamode)
@@ -692,6 +691,39 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
scb_o->ihcpu = scb_s->ihcpu;
}
+static int shadow_scb_minimal(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
+{
+ struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
+ struct kvm_s390_sie_block *scb_o = vsie_page->scb_o;
+ int rc;
+
+ /* make sure we don't have any leftovers when reusing the scb */
+ scb_s->icptcode = 0;
+ scb_s->eca = 0;
+ scb_s->ecb = 0;
+ scb_s->ecb2 = 0;
+ scb_s->ecb3 = 0;
+ scb_s->ecd = 0;
+ scb_s->fac = 0;
+ scb_s->fpf = 0;
+
+ rc = prepare_cpuflags(vcpu, vsie_page);
+ if (rc)
+ return rc;
+
+ scb_s->icpua = scb_o->icpua;
+ scb_s->ecb2 |= scb_o->ecb2 & ECB2_ESCA;
+
+ if (vsie_page->vsie_sca) {
+ scb_s->eca |= scb_o->eca & ECA_SIGPI;
+ scb_s->ecb |= scb_o->ecb & ECB_SRSI;
+ write_scao(scb_s, virt_to_phys(&vsie_page->vsie_sca->ssca));
+ scb_s->osda = virt_to_phys(scb_o);
+ }
+
+ return 0;
+}
+
/*
* Setup the shadow scb by copying and checking the relevant parts of the g2
* provided scb.
@@ -711,17 +743,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
unsigned long new_mso = 0;
int rc;
- /* make sure we don't have any leftovers when reusing the scb */
- scb_s->icptcode = 0;
- scb_s->eca = 0;
- scb_s->ecb = 0;
- scb_s->ecb2 = 0;
- scb_s->ecb3 = 0;
- scb_s->ecd = 0;
- scb_s->fac = 0;
- scb_s->fpf = 0;
-
- rc = prepare_cpuflags(vcpu, vsie_page);
+ rc = shadow_scb_minimal(vcpu, vsie_page);
if (rc)
goto out;
@@ -751,8 +773,6 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
if (!(atomic_read(&scb_s->cpuflags) & CPUSTAT_KSS))
scb_s->ictl |= ICTL_ISKE | ICTL_SSKE | ICTL_RRBE;
- scb_s->icpua = scb_o->icpua;
-
if (!(atomic_read(&scb_s->cpuflags) & CPUSTAT_ZARCH))
new_prefix &= GUEST_PREFIX_MASK_ESA;
else
@@ -2091,7 +2111,7 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
sca_mcn_copy(vsie_sca, phys_to_virt(sca_o_hpa));
- /* pin and make shadow for ALL scb in the sca */
+ /* pin and make minimal shadow for ALL scb in the sca */
cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {
rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
@@ -2113,7 +2133,7 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
rc = PTR_ERR(vsie_page_n);
if (rc)
goto err;
- rc = shadow_scb(vcpu, vsie_page_n);
+ rc = shadow_scb_minimal(vcpu, vsie_page_n);
vsie_sca_update(vsie_sca, cpu_nr, vsie_page_n, sca_o_entry_hpa);
put_vsie_page(vsie_page_n);
if (rc)
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks
2026-07-09 15:15 ` [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks Christoph Schlameuss
@ 2026-07-09 15:47 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:47 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, kvm, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Critical] Out-of-bounds array access using guest-controlled `icpua`.
- [Critical] Host WARN and VM crash via invalid SCA physical address (DoS).
- [High] Lost validity intercept for side VCPUs causing silent execution divergence.
- [High] SCA corruption and livelock on concurrent VSIE entry.
- [High] Pinned SCB guest pages are leaked when the VM is destroyed.
Pre-existing issues:
- [High] Guest 2 SCB memory corruption when prepare_cpuflags fails.
--
--- Patch [20]: [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks ---
commit 2f62e0dfe18aabd20de0781221d96146e1f85a9f
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks
Instead of creating a full shadow for vSIE control blocks, this creates a
minimal shadow for SIGP purposes for currently not running guest-3 blocks.
The full shadow is recreated when vSIE is entered with that SCB later.
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 80050376eaab1..feac755d56cb5 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -692,6 +691,39 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> scb_o->ihcpu = scb_s->ihcpu;
> }
>
> +static int shadow_scb_minimal(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
> +{
[ ... ]
> + rc = prepare_cpuflags(vcpu, vsie_page);
> + if (rc)
> + return rc;
> +
> + scb_s->icpua = scb_o->icpua;
[Severity: Critical]
Does this code allow an out-of-bounds array access if the guest provides an
invalid icpua?
If a malicious guest provides an SCB with an icpua value greater than 255,
it appears kvm_s390_handle_vsie() passes this unvalidated scb_o->icpua to
get_vsie_page_cpu_nr().
Inside get_vsie_page_cpu_nr(), the cpu_nr is used to index directly into the
fixed-size vsie_sca->pages array:
vsie_page = vsie_sca->pages[cpu_nr];
Could this result in an out-of-bounds read and a subsequent out-of-bounds
write of a kernel heap pointer when vsie_page_new is evaluated?
> + scb_s->ecb2 |= scb_o->ecb2 & ECB2_ESCA;
[ ... ]
> @@ -711,17 +743,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
[ ... ]
> - rc = prepare_cpuflags(vcpu, vsie_page);
> + rc = shadow_scb_minimal(vcpu, vsie_page);
[Severity: High]
This isn't a bug introduced by this patch, but does the error path here
corrupt guest memory when prepare_cpuflags() fails?
If shadow_scb_minimal() returns an error, the code jumps to the out label
and calls unshadow_scb().
Because the failure occurs before shadow_scb() initializes scb_s with guest
state from scb_o, scb_s might contain uninitialized zeros or stale data.
Does unshadow_scb() then copy this uninitialized data back into the guest's
scb_o, corrupting the guest's memory?
> if (rc)
> goto out;
[ ... ]
> @@ -2113,7 +2133,7 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
> rc = PTR_ERR(vsie_page_n);
> if (rc)
> goto err;
> - rc = shadow_scb(vcpu, vsie_page_n);
> + rc = shadow_scb_minimal(vcpu, vsie_page_n);
[Severity: High]
Does this error path lose the validity intercept for side VCPUs?
If a side CPU's SCB is invalid, shadow_scb_minimal() sets a validity
intercept in vsie_page_n->scb_s.icptcode and returns an error.
When _shadow_sca() propagates this error and jumps to the err label, it
doesn't seem to call unshadow_scb() on vsie_page_n. Later,
kvm_s390_handle_vsie() jumps to out_unshadow, which only unshadows the main
CPU's vsie_page.
Does this cause the guest to silently skip the SIE instruction without
notification, leading to execution divergence?
[Severity: High]
Can a concurrent VSIE entry cause SCA corruption and a livelock here?
If VCPU 1 is running, its vsie_page has VSIE_PAGE_IN_USE set. If VCPU 2
enters SIE and observes an MCN change, _shadow_sca() attempts to get
VCPU 1's page via get_vsie_page_cpu_nr(), which fails and returns -EAGAIN.
When _shadow_sca() jumps to the err label, it zeroes out
vsie_sca->ssca.cpu[cpu_nr] for all CPUs:
err:
for_each_set_bit_inv(cpu_nr, ...) {
vsie_sca->ssca.cpu[cpu_nr].ssda = 0;
vsie_sca->ssca.cpu[cpu_nr].ossea = 0;
}
Does this corrupt the SCA actively used by VCPU 1's hardware, and cause
VCPU 2 to continuously retry in a tight loop until VCPU 1 exits?
[Severity: Critical]
Does get_vsie_page_cpu_nr() trigger a host warning and VM crash if the guest
provides an invalid SCA physical address?
If a malicious guest provides an invalid physical address in its SCA for a
side CPU, pin_scb() detects this, injects a PGM_ADDRESSING exception, and
returns 1.
It appears vsie_page_init() transforms this return code into -ENOMEM.
When get_vsie_page_cpu_nr() sees this, it hits WARN_ON_ONCE(rc):
get_vsie_page_cpu_nr() {
...
if (WARN_ON_ONCE(rc)) {
put_vsie_page(vsie_page);
return ERR_PTR(rc);
}
}
Does this propagate the -ENOMEM up to the KVM_RUN ioctl, causing QEMU to
terminate the VM instead of resuming the guest?
> vsie_sca_update(vsie_sca, cpu_nr, vsie_page_n, sca_o_entry_hpa);
> put_vsie_page(vsie_page_n);
> if (rc)
> goto err;
[Severity: High]
Do we leak pinned SCB guest pages when the VM is destroyed?
The vsie_page_init() function pins the guest's SCB page, leaving the
vsie_page in the global kvm->arch.vsie.pages cache with a pinned host
physical page.
When the VM is destroyed, kvm_s390_vsie_destroy() frees each vsie_page in
the global cache via kvm_s390_vsie_destroy_page(). However, it appears to
miss calling unpin_scb() for these main CPU pages.
Could this permanently leak the pinned pages, potentially allowing an
unprivileged user to trigger a host physical memory leak by repeatedly
creating and destroying VMs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=20
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI
2026-07-09 15:15 [PATCH 00/21] KVM: s390: vsie: Add VSIE SIGP Interpretation (vsie_sigpi) Christoph Schlameuss
` (19 preceding siblings ...)
2026-07-09 15:15 ` [PATCH 20/21] KVM: s390: vsie: Create minimal scb shadows for not running g3 blocks Christoph Schlameuss
@ 2026-07-09 15:15 ` Christoph Schlameuss
2026-07-09 15:44 ` sashiko-bot
20 siblings, 1 reply; 37+ messages in thread
From: Christoph Schlameuss @ 2026-07-09 15:15 UTC (permalink / raw)
To: kvm, linux-s390
Cc: David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Nico Boehr, Sven Schnelle, Paolo Bonzini,
Shuah Khan, Eric Farman, David Hildenbrand, Christoph Schlameuss
Finally enable code paths actually configuring and using VSIE SIGPI.
SIGP can be globally disabled in the KVM module configuration but will
be enabled by default if the machine (IBM z16 onwards) supports it.
Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 481e566fe228..854de405d08c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -247,6 +247,11 @@ static int async_destroy = 1;
module_param(async_destroy, int, 0444);
MODULE_PARM_DESC(async_destroy, "Asynchronous destroy for protected guests");
+/* allow vsie sigpi */
+static bool vsie_sigpi = true;
+module_param(vsie_sigpi, bool, 0444);
+MODULE_PARM_DESC(vsie_sigpi, "VSIE SIGPI");
+
#define HMFAI_DWORDS 16
/*
* Base feature mask that defines default mask for facilities. Consists of the
@@ -476,6 +481,8 @@ static void __init kvm_s390_cpu_feat_init(void)
allow_cpu_feat(KVM_S390_VM_CPU_FEAT_KSS);
if (sclp.has_astfleie2)
allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ASTFLEIE2);
+ if (sclp.has_vsie_sigpif && vsie_sigpi)
+ allow_cpu_feat(KVM_S390_VM_CPU_FEAT_SIGPIF);
/*
* KVM_S390_VM_CPU_FEAT_SKEY: Wrong shadow of PTE.I bits will make
* all skey handling functions read/set the skey from the PGSTE
@@ -490,9 +497,6 @@ static void __init kvm_s390_cpu_feat_init(void)
* For KVM_S390_VM_CPU_FEAT_SKEY, KVM_S390_VM_CPU_FEAT_CMMA and
* KVM_S390_VM_CPU_FEAT_PFMFI, all PTE.I and PGSTE bits have to be
* correctly shadowed. We can do that for the PGSTE but not for PTE.I.
- *
- * KVM_S390_VM_CPU_FEAT_SIGPIF: Wrong SCB addresses in the SCA. We
- * cannot easily shadow the SCA because of the ipte lock.
*/
}
@@ -3382,7 +3386,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm->arch.use_pfmfi = sclp.has_pfmfi;
kvm->arch.use_skf = sclp.has_skey;
- kvm->arch.use_vsie_sigpif = sclp.has_vsie_sigpif;
+ kvm->arch.use_vsie_sigpif = sclp.has_vsie_sigpif && vsie_sigpi;
spin_lock_init(&kvm->arch.start_stop_lock);
kvm_s390_vsie_init(kvm);
if (use_gisa)
--
2.55.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI
2026-07-09 15:15 ` [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI Christoph Schlameuss
@ 2026-07-09 15:44 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-09 15:44 UTC (permalink / raw)
To: Christoph Schlameuss
Cc: linux-s390, Alexander Gordeev, Vasily Gorbik, Heiko Carstens, kvm
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] The guest-controlled `icpua` field is used as an array index into `vsie_sca->pages` without bounds checking, allowing an attacker to corrupt arbitrary host kernel memory.
- [High] A guest can permanently leak vsie_sca structures by providing an invalid SCAO, eventually exhausting the vsie_sca pool and breaking nested virtualization.
- [High] A malicious guest can intentionally trigger host kernel warnings (WARN_ON_ONCE) by supplying unmapped guest physical addresses in the System Control Area (SCA) structures.
--
--- Patch [21]: [PATCH 21/21] KVM: s390: vsie: Enable VSIE SIGPI ---
commit ea08ed257ee1485d9e9b61914b578f7cc9773f7a
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>
KVM: s390: vsie: Enable VSIE SIGPI
Finally enable code paths actually configuring and using VSIE SIGPI.
SIGP can be globally disabled in the KVM module configuration but will
be enabled by default if the machine (IBM z16 onwards) supports it.
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 481e566fe228b..854de405d08c7 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
> @@ -476,6 +481,8 @@ static void __init kvm_s390_cpu_feat_init(void)
> allow_cpu_feat(KVM_S390_VM_CPU_FEAT_KSS);
> if (sclp.has_astfleie2)
> allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ASTFLEIE2);
> + if (sclp.has_vsie_sigpif && vsie_sigpi)
> + allow_cpu_feat(KVM_S390_VM_CPU_FEAT_SIGPIF);
[Severity: High]
Does enabling this feature expose a resource leak when providing an invalid
SCAO address?
Looking at get_vsie_sca() in arch/s390/kvm/vsie.c, if a guest provides an
unmapped SCAO address, pin_sca() fails and returns an error:
arch/s390/kvm/vsie.c:get_vsie_sca() {
...
rc = pin_sca(kvm, vsie_sca);
if (rc) {
vsie_sca = ERR_PTR(rc);
goto out;
}
...
}
It appears the newly added vsie_sca object is left in the kvm->arch.vsie.scas
array with an undecremented ref_count, permanently leaking it from the
reusable pool.
[Severity: High]
Can a guest intentionally trigger host kernel warnings by supplying unmapped
guest physical addresses in the System Control Area structures?
When a guest provides an unbacked SCA address, pin_guest_pages() in pin_sca()
returns -EFAULT, which directly triggers the host WARN_ON_ONCE:
arch/s390/kvm/vsie.c:pin_sca() {
...
vsie_sca->sca_o_nr_pages = pin_guest_pages(kvm, vsie_sca->sca_gpa, nr_pages,
vsie_sca->sca_o_pages);
if (WARN_ON_ONCE(vsie_sca->sca_o_nr_pages != nr_pages))
return -EIO;
...
}
Similarly, in get_vsie_page_cpu_nr(), if the SCB address is unmapped,
vsie_page_init() returns -ENOMEM and also triggers a WARN_ON_ONCE:
arch/s390/kvm/vsie.c:get_vsie_page_cpu_nr() {
...
scoped_guard(mutex, &vcpu->kvm->arch.vsie.mutex) {
unpin_scb(vcpu->kvm, vsie_page);
rc = vsie_page_init(vcpu, vsie_page, scb_gpa);
}
if (WARN_ON_ONCE(rc)) {
...
}
[Severity: Critical]
Does this enablement allow an out-of-bounds array access in
get_vsie_page_cpu_nr()?
In get_vsie_page_cpu_nr(), the guest-controlled cpu_nr (which comes from
icpua and can be up to 65535) is used as an index to read from and write to
the pages array, which is only sized to 256:
arch/s390/kvm/vsie.c:get_vsie_page_cpu_nr() {
...
vsie_page = vsie_sca->pages[cpu_nr];
if (!vsie_page) {
vsie_page_new = alloc_vsie_page(vcpu->kvm);
...
scoped_guard(mutex, &vsie_sca->mutex) {
vsie_page = vsie_sca->pages[cpu_nr];
if (!vsie_page) {
WRITE_ONCE(vsie_sca->pages[cpu_nr], vsie_page_new);
...
}
Could this corrupt adjacent host kernel memory structures?
> /*
> * KVM_S390_VM_CPU_FEAT_SKEY: Wrong shadow of PTE.I bits will make
> * all skey handling functions read/set the skey from the PGSTE
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=21
^ permalink raw reply [flat|nested] 37+ messages in thread