[PATCH RFC 0/5] KVM: s390: Add VSIE Interpretation Extension Facility (vsie_sigpif)

[PATCH RFC 0/5] KVM: s390: Add VSIE Interpretation Extension Facility (vsie_sigpif)

[Christoph Schlameuss]

In the upcoming IBM Z machine generation (gen17) the s390x architecture
adds a new VSIE Interpretation Extension Facility (vsie_sigpif) to
improve guest-3 guest performance.

To exploit the new machine support the guest-1 KVM needs to create and
maintain shadow structures pointing to the original state descriptions
and system control areas.
These pointers are followed by the machines firmware and modifications
of the original SCA in guest-3 are monitored and handled by firmware.

---
Christoph Schlameuss (5):
      KVM: s390: Add vsie_sigpif detection
      KVM: s390: Add ssca_block and ssca_entry structs for vsie_ie
      KVM: s390: Shadow VSIE SCA in guest-1
      KVM: s390: Re-init SSCA on switch to ESCA
      KVM: s390: Add VSIE shadow stat counters

 arch/s390/include/asm/kvm_host.h               |  44 +++-
 arch/s390/include/asm/sclp.h                   |   1 +
 arch/s390/kvm/kvm-s390.c                       |   5 +
 arch/s390/kvm/vsie.c                           | 285 ++++++++++++++++++++++++-
 drivers/s390/char/sclp_early.c                 |   1 +
 tools/testing/selftests/kvm/include/s390/sie.h |   2 +-
 6 files changed, 333 insertions(+), 5 deletions(-)
---
base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
change-id: 20250228-vsieie-07be34fc3984

Best regards,
-- 
Christoph Schlameuss <schlameuss@linux.ibm.com>

[PATCH RFC 1/5] KVM: s390: Add vsie_sigpif detection

[Christoph Schlameuss]

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>
---
 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 9a367866cab0eb5f24960aea9fd552e0a919cffa..149912c3b1ffd0f8ed978b8c06a70efc892b7e01 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -983,6 +983,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 18f37dff03c9924190c1f9ae02d83967ae944f1b..f843cd0f148b0cb52e5e12f6c7d84cce26259442 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -101,6 +101,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 ebecb96bacce7d75563bd3a130a7cc31869dc254..16204c638119fa3a6c36e8e24af2b0b399f8123b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3458,6 +3458,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 d9d6edaf8de828a51b30098c288e76b2a61749cd..a06eb5fd3986910f05ac463640ad929a2579815f 100644
--- a/drivers/s390/char/sclp_early.c
+++ b/drivers/s390/char/sclp_early.c
@@ -60,6 +60,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.48.1

Re: [PATCH RFC 1/5] KVM: s390: Add vsie_sigpif detection

[David Hildenbrand]

On 18.03.25 19:59, Christoph Schlameuss wrote:
> 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>

-- 
Cheers,

David / dhildenb

[PATCH RFC 2/5] KVM: s390: Add ssca_block and ssca_entry structs for vsie_ie

[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.h               | 20 +++++++++++++++++++-
 tools/testing/selftests/kvm/include/s390/sie.h |  2 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 149912c3b1ffd0f8ed978b8c06a70efc892b7e01..0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -87,6 +87,13 @@ struct bsca_entry {
 	__u64	reserved2[2];
 };
 
+struct ssca_entry {
+	__u8	reserved0[8];
+	__u64	ssda;
+	__u64	ossea;
+	__u8	reserved18[8];
+};
+
 union ipte_control {
 	unsigned long val;
 	struct {
@@ -128,6 +135,17 @@ 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 use KVM_S390_ESCA_CPU_SLOTS.
+ * 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_ESCA_CPU_SLOTS];
+};
+
 /*
  * This struct is used to store some machine check info from lowcore
  * for machine checks that happen while the guest is running.
@@ -358,7 +376,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 160acd4a1db92d6129c0f084db82c8c147d5c23e..4ff1c1a354af51d322042c03d59a8cf56685abd3 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.48.1

[PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Christoph Schlameuss]

Introduce a new shadow_sca function into kvm_s390_handle_vsie.
kvm_s390_handle_vsie is called within guest-1 when guest-2 initiates the
VSIE.

shadow_sca and unshadow_sca create and manage ssca_block structs in
guest-1 memory. References to the created ssca_blocks are kept within an
array and limited to the number of cpus. This ensures each VSIE in
execution can have its own SCA. Having the amount of shadowed SCAs
configurable above this is left to another patch.

SCAOL/H in the VSIE control block are overwritten with references to the
shadow SCA. The original SCA pointer is saved in the vsie_page and
restored on VSIE exit. This limits the amount of change in the
preexisting VSIE pin and shadow functions.

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.

The ssca_blocks are also kept within a radix tree to reuse already
existing ssca_blocks efficiently. While the radix tree and array with
references to the ssca_blocks are held in kvm_s390_vsie.
The use of the ssca_blocks is tracked using an ref_count on the block
itself.

No strict mapping between the guest-1 vcpu and guest-3 vcpu is enforced.
Instead each VSIE entry updates the shadow SCA creating a valid mapping
for all cpus currently in VSIE.

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  22 +++-
 arch/s390/kvm/vsie.c             | 264 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 281 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -29,6 +29,7 @@
 #define KVM_S390_BSCA_CPU_SLOTS 64
 #define KVM_S390_ESCA_CPU_SLOTS 248
 #define KVM_MAX_VCPUS 255
+#define KVM_S390_MAX_VCPUS 256
 
 #define KVM_INTERNAL_MEM_SLOTS 1
 
@@ -137,13 +138,23 @@ struct esca_block {
 
 /*
  * The shadow sca / ssca needs to cover both bsca and esca depending on what the
- * guest uses so we use KVM_S390_ESCA_CPU_SLOTS.
+ * 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_ESCA_CPU_SLOTS];
+	struct ssca_entry cpu[KVM_S390_MAX_VCPUS];
+};
+
+/*
+ * Store the vsie ssca block and accompanied management data.
+ */
+struct ssca_vsie {
+	struct ssca_block ssca;			/* 0x0000 */
+	__u8	reserved[0x2200 - 0x2040];	/* 0x2040 */
+	atomic_t ref_count;			/* 0x2200 */
 };
 
 /*
@@ -953,12 +964,19 @@ struct sie_page2 {
 
 struct vsie_page;
 
+/*
+ * vsie_pages, sscas and accompanied management vars
+ */
 struct kvm_s390_vsie {
 	struct mutex mutex;
 	struct radix_tree_root addr_to_page;
 	int page_count;
 	int next;
 	struct vsie_page *pages[KVM_MAX_VCPUS];
+	struct rw_semaphore ssca_lock;
+	struct radix_tree_root osca_to_ssca;
+	int ssca_count;
+	struct ssca_vsie *sscas[KVM_MAX_VCPUS];
 };
 
 struct kvm_s390_gisa_iam {
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index a78df3a4f353093617bc166ed8f4dd332fd6b08e..0327c4964d27e493932a2b90b62c5a27b0a95446 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -25,6 +25,8 @@
 #include "gaccess.h"
 #include "gmap.h"
 
+#define SSCA_PAGEORDER 2
+
 enum vsie_page_flags {
 	VSIE_PAGE_IN_USE = 0,
 };
@@ -63,7 +65,8 @@ struct vsie_page {
 	 * looked up by other CPUs.
 	 */
 	unsigned long flags;			/* 0x0260 */
-	__u8 reserved[0x0700 - 0x0268];		/* 0x0268 */
+	u64 sca_o;				/* 0x0268 */
+	__u8 reserved[0x0700 - 0x0270];		/* 0x0270 */
 	struct kvm_s390_crypto_cb crycb;	/* 0x0700 */
 	__u8 fac[S390_ARCH_FAC_LIST_SIZE_BYTE];	/* 0x0800 */
 };
@@ -595,6 +598,236 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	return rc;
 }
 
+/* fill the shadow system control area */
+static void init_ssca(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
+{
+	u64 sca_o_hva = vsie_page->sca_o;
+	unsigned int bit, cpu_slots;
+	struct ssca_entry *cpu;
+	void *ossea_hva;
+	int is_esca;
+	u64 *mcn;
+
+	/* set original SIE control block address */
+	ssca->ssca.osca = virt_to_phys((void *)sca_o_hva);
+	WARN_ON_ONCE(ssca->ssca.osca & 0x000f);
+
+	/* use ECB of shadow scb to determine SCA type */
+	is_esca = (vsie_page->scb_s.ecb2 & ECB2_ESCA);
+	cpu_slots = is_esca ? KVM_S390_MAX_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
+	mcn = is_esca ? ((struct esca_block *)sca_o_hva)->mcn :
+			&((struct bsca_block *)sca_o_hva)->mcn;
+
+	/*
+	 * For every enabled sigp entry in the original sca we need to populate
+	 * the corresponding shadow sigp entry with the address of the shadow
+	 * state description and the address of the original sigp entry.
+	 */
+	for_each_set_bit_inv(bit, (unsigned long *)mcn, cpu_slots) {
+		cpu = &ssca->ssca.cpu[bit];
+		if (is_esca)
+			ossea_hva = &((struct esca_block *)sca_o_hva)->cpu[bit];
+		else
+			ossea_hva = &((struct bsca_block *)sca_o_hva)->cpu[bit];
+		cpu->ossea = virt_to_phys(ossea_hva);
+		/* cpu->ssda is set in update_entry_ssda() when vsie is entered */
+	}
+}
+
+/* remove running scb pointer to ssca */
+static void update_entry_ssda_remove(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
+{
+	struct ssca_entry *cpu = &ssca->ssca.cpu[vsie_page->scb_s.icpua & 0xff];
+
+	WRITE_ONCE(cpu->ssda, 0);
+}
+
+/* add running scb pointer to ssca */
+static void update_entry_ssda_add(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
+{
+	struct ssca_entry *cpu = &ssca->ssca.cpu[vsie_page->scb_s.icpua & 0xff];
+	phys_addr_t scb_s_hpa = virt_to_phys(&vsie_page->scb_s);
+
+	WRITE_ONCE(cpu->ssda, scb_s_hpa);
+}
+
+/*
+ * Try to find the address of an existing shadow system control area.
+ * @kvm:  pointer to the kvm struct
+ * @sca_o_hva: original system control area address; guest-1 virtual
+ *
+ * Called with ssca_lock held.
+ */
+static struct ssca_vsie *get_existing_ssca(struct kvm *kvm, u64 sca_o_hva)
+{
+	struct ssca_vsie *ssca = radix_tree_lookup(&kvm->arch.vsie.osca_to_ssca, sca_o_hva);
+
+	if (ssca)
+		WARN_ON_ONCE(atomic_inc_return(&ssca->ref_count) < 1);
+	return ssca;
+}
+
+/*
+ * Try to find an currently unused ssca_vsie from the vsie struct.
+ * @kvm:  pointer to the kvm struct
+ *
+ * Called with ssca_lock held.
+ */
+static struct ssca_vsie *get_free_existing_ssca(struct kvm *kvm)
+{
+	struct ssca_vsie *ssca;
+	int i;
+
+	for (i = kvm->arch.vsie.ssca_count - 1; i >= 0; i--) {
+		ssca = kvm->arch.vsie.sscas[i];
+		if (atomic_inc_return(&ssca->ref_count) == 1)
+			return ssca;
+		atomic_dec(&ssca->ref_count);
+	}
+	return ERR_PTR(-EFAULT);
+}
+
+/*
+ * Get a existing or new shadow system control area (ssca).
+ * @kvm:  pointer to the kvm struct
+ * @vsie_page:  the current vsie_page
+ *
+ * May sleep.
+ */
+static struct ssca_vsie *get_ssca(struct kvm *kvm, struct vsie_page *vsie_page)
+{
+	u64 sca_o_hva = vsie_page->sca_o;
+	phys_addr_t sca_o_hpa = virt_to_phys((void *)sca_o_hva);
+	struct ssca_vsie *ssca, *ssca_new = NULL;
+
+	/* get existing ssca */
+	down_read(&kvm->arch.vsie.ssca_lock);
+	ssca = get_existing_ssca(kvm, sca_o_hva);
+	up_read(&kvm->arch.vsie.ssca_lock);
+	if (ssca)
+		return ssca;
+
+	/*
+	 * Allocate new ssca, 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.)
+	 */
+	if (kvm->arch.vsie.ssca_count < atomic_read(&kvm->online_vcpus)) {
+		BUILD_BUG_ON(offsetof(struct ssca_block, cpu) != 64);
+		BUILD_BUG_ON(offsetof(struct ssca_vsie, ref_count) != 0x2200);
+		BUILD_BUG_ON(sizeof(struct ssca_vsie) > ((1UL << SSCA_PAGEORDER)-1) * PAGE_SIZE);
+		ssca_new = (struct ssca_vsie *)__get_free_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
+								SSCA_PAGEORDER);
+		if (!ssca_new) {
+			ssca = ERR_PTR(-ENOMEM);
+			goto out;
+		}
+		init_ssca(vsie_page, ssca_new);
+	}
+
+	/* enter write lock and recheck to make sure ssca has not been created by other cpu */
+	down_write(&kvm->arch.vsie.ssca_lock);
+	ssca = get_existing_ssca(kvm, sca_o_hva);
+	if (ssca)
+		goto out;
+
+	/* check again under write lock if we are still under our ssca_count limit */
+	if (ssca_new && kvm->arch.vsie.ssca_count < atomic_read(&kvm->online_vcpus)) {
+		/* make use of ssca just created */
+		ssca = ssca_new;
+		ssca_new = NULL;
+
+		kvm->arch.vsie.sscas[kvm->arch.vsie.ssca_count] = ssca;
+		kvm->arch.vsie.ssca_count++;
+	} else {
+		/* reuse previously created ssca for different osca */
+		ssca = get_free_existing_ssca(kvm);
+		/* with nr_vcpus sscas one must be free */
+		if (IS_ERR(ssca))
+			goto out;
+
+		radix_tree_delete(&kvm->arch.vsie.osca_to_ssca,
+				  (u64)phys_to_virt(ssca->ssca.osca));
+		memset(ssca, 0, sizeof(struct ssca_vsie));
+		init_ssca(vsie_page, ssca);
+	}
+	atomic_set(&ssca->ref_count, 1);
+
+	/* virt_to_phys(sca_o_hva) == ssca->osca */
+	radix_tree_insert(&kvm->arch.vsie.osca_to_ssca, sca_o_hva, ssca);
+	WRITE_ONCE(ssca->ssca.osca, sca_o_hpa);
+
+out:
+	up_write(&kvm->arch.vsie.ssca_lock);
+	if (ssca_new)
+		free_pages((unsigned long)ssca_new, SSCA_PAGEORDER);
+	return ssca;
+}
+
+/*
+ * Unshadow the sca on vsie exit.
+ * @vcpu:  pointer to the current vcpu struct
+ * @vsie_page:  the current vsie_page
+ */
+static void unshadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
+{
+	struct ssca_vsie *ssca;
+
+	down_read(&vcpu->kvm->arch.vsie.ssca_lock);
+	ssca = radix_tree_lookup(&vcpu->kvm->arch.vsie.osca_to_ssca,
+				 vsie_page->sca_o);
+	if (ssca) {
+		update_entry_ssda_remove(vsie_page, ssca);
+		vsie_page->scb_s.scaoh = vsie_page->sca_o >> 32;
+		vsie_page->scb_s.scaol = vsie_page->sca_o;
+		vsie_page->scb_s.osda = 0;
+		atomic_dec(&ssca->ref_count);
+	}
+	up_read(&vcpu->kvm->arch.vsie.ssca_lock);
+}
+
+/*
+ * Shadow the sca on vsie enter.
+ * @vcpu:  pointer to the current vcpu struct
+ * @vsie_page:  the current vsie_page
+ */
+static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
+{
+	phys_addr_t scb_o_hpa, sca_s_hpa;
+	struct ssca_vsie *ssca;
+	u64 sca_o_hva;
+
+	/* only shadow sca when vsie_sigpif is enabled */
+	if (!vcpu->kvm->arch.use_vsie_sigpif)
+		return 0;
+
+	sca_o_hva = (u64)vsie_page->scb_s.scaoh << 32 | vsie_page->scb_s.scaol;
+	/* skip if the guest does not have an sca */
+	if (!sca_o_hva)
+		return 0;
+
+	scb_o_hpa = virt_to_phys(vsie_page->scb_o);
+	WRITE_ONCE(vsie_page->scb_s.osda, scb_o_hpa);
+	vsie_page->sca_o = sca_o_hva;
+
+	ssca = get_ssca(vcpu->kvm, vsie_page);
+	if (WARN_ON(IS_ERR(ssca)))
+		return PTR_ERR(ssca);
+
+	/* update shadow control block sca references to shadow sca */
+	update_entry_ssda_add(vsie_page, ssca);
+	sca_s_hpa = virt_to_phys(ssca);
+	if (sclp.has_64bscao) {
+		WARN_ON_ONCE(sca_s_hpa & 0x003f);
+		vsie_page->scb_s.scaoh = (u64)sca_s_hpa >> 32;
+	} else {
+		WARN_ON_ONCE(sca_s_hpa & 0x000f);
+	}
+	vsie_page->scb_s.scaol = (u64)sca_s_hpa;
+
+	return 0;
+}
+
 void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, unsigned long start,
 				 unsigned long end)
 {
@@ -699,6 +932,9 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 
 	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
 	if (hpa) {
+		/* with vsie_sigpif scaoh/l was pointing to g1 ssca_block but
+		 * should have been reset in unshadow_sca()
+		 */
 		unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
 		vsie_page->sca_gpa = 0;
 		scb_s->scaol = 0;
@@ -775,6 +1011,9 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		if (rc)
 			goto unpin;
 		vsie_page->sca_gpa = gpa;
+		/* with vsie_sigpif scaoh and scaol will be overwritten
+		 * in shadow_sca to point to g1 ssca_block instead
+		 */
 		scb_s->scaoh = (u32)((u64)hpa >> 32);
 		scb_s->scaol = (u32)(u64)hpa;
 	}
@@ -1490,12 +1729,17 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
 		goto out_unpin_scb;
 	rc = pin_blocks(vcpu, vsie_page);
 	if (rc)
-		goto out_unshadow;
+		goto out_unshadow_scb;
+	rc = shadow_sca(vcpu, vsie_page);
+	if (rc)
+		goto out_unpin_blocks;
 	register_shadow_scb(vcpu, vsie_page);
 	rc = vsie_run(vcpu, vsie_page);
 	unregister_shadow_scb(vcpu);
+	unshadow_sca(vcpu, vsie_page);
+out_unpin_blocks:
 	unpin_blocks(vcpu, vsie_page);
-out_unshadow:
+out_unshadow_scb:
 	unshadow_scb(vcpu, vsie_page);
 out_unpin_scb:
 	unpin_scb(vcpu, vsie_page, scb_addr);
@@ -1510,12 +1754,15 @@ 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);
+	init_rwsem(&kvm->arch.vsie.ssca_lock);
+	INIT_RADIX_TREE(&kvm->arch.vsie.osca_to_ssca, GFP_KERNEL_ACCOUNT);
 }
 
 /* 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;
+	struct ssca_vsie *ssca;
 	int i;
 
 	mutex_lock(&kvm->arch.vsie.mutex);
@@ -1531,6 +1778,17 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
 	}
 	kvm->arch.vsie.page_count = 0;
 	mutex_unlock(&kvm->arch.vsie.mutex);
+
+	down_write(&kvm->arch.vsie.ssca_lock);
+	for (i = 0; i < kvm->arch.vsie.ssca_count; i++) {
+		ssca = kvm->arch.vsie.sscas[i];
+		kvm->arch.vsie.sscas[i] = NULL;
+		radix_tree_delete(&kvm->arch.vsie.osca_to_ssca,
+				  (u64)phys_to_virt(ssca->ssca.osca));
+		free_pages((unsigned long)ssca, SSCA_PAGEORDER);
+	}
+	kvm->arch.vsie.ssca_count = 0;
+	up_write(&kvm->arch.vsie.ssca_lock);
 }
 
 void kvm_s390_vsie_kick(struct kvm_vcpu *vcpu)

-- 
2.48.1

Re: [PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Janosch Frank]

On 3/18/25 7:59 PM, Christoph Schlameuss wrote:
> Introduce a new shadow_sca function into kvm_s390_handle_vsie.
> kvm_s390_handle_vsie is called within guest-1 when guest-2 initiates the
> VSIE.
> 
> shadow_sca and unshadow_sca create and manage ssca_block structs in
> guest-1 memory. References to the created ssca_blocks are kept within an
> array and limited to the number of cpus. This ensures each VSIE in
> execution can have its own SCA. Having the amount of shadowed SCAs
> configurable above this is left to another patch.
> 
> SCAOL/H in the VSIE control block are overwritten with references to the
> shadow SCA. The original SCA pointer is saved in the vsie_page and
> restored on VSIE exit. This limits the amount of change in the
> preexisting VSIE pin and shadow functions.
> 
> 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.
> 
> The ssca_blocks are also kept within a radix tree to reuse already
> existing ssca_blocks efficiently. While the radix tree and array with
> references to the ssca_blocks are held in kvm_s390_vsie.
> The use of the ssca_blocks is tracked using an ref_count on the block
> itself.
> 
> No strict mapping between the guest-1 vcpu and guest-3 vcpu is enforced.
> Instead each VSIE entry updates the shadow SCA creating a valid mapping
> for all cpus currently in VSIE.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> ---
>   arch/s390/include/asm/kvm_host.h |  22 +++-
>   arch/s390/kvm/vsie.c             | 264 ++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 281 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -29,6 +29,7 @@
>   #define KVM_S390_BSCA_CPU_SLOTS 64
>   #define KVM_S390_ESCA_CPU_SLOTS 248
>   #define KVM_MAX_VCPUS 255
> +#define KVM_S390_MAX_VCPUS 256

#define KVM_S390_SSCA_CPU_SLOTS 256

Yes, I'm aware, that ESCA and MAX_VCPUS are pretty confusing.
I'm searching for solutions but they might take a while.

>   
>   #define KVM_INTERNAL_MEM_SLOTS 1
>   
> @@ -137,13 +138,23 @@ struct esca_block {
>   
>   /*
>    * The shadow sca / ssca needs to cover both bsca and esca depending on what the
> - * guest uses so we use KVM_S390_ESCA_CPU_SLOTS.
> + * 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_ESCA_CPU_SLOTS];
> +	struct ssca_entry cpu[KVM_S390_MAX_VCPUS];

This should have been resolved in the previous patch, no?

> +};
> +
> +/*
> + * Store the vsie ssca block and accompanied management data.
> + */
> +struct ssca_vsie {
> +	struct ssca_block ssca;			/* 0x0000 */
> +	__u8	reserved[0x2200 - 0x2040];	/* 0x2040 */
> +	atomic_t ref_count;			/* 0x2200 */
>   };
>   

[...]

>   void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, unsigned long start,
>   				 unsigned long end)
>   {
> @@ -699,6 +932,9 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>   
>   	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
>   	if (hpa) {
> +		/* with vsie_sigpif scaoh/l was pointing to g1 ssca_block but
> +		 * should have been reset in unshadow_sca()
> +		 */

There shouldn't be text in the first or last line of multi-line comments.

>   		unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
>   		vsie_page->sca_gpa = 0;
>   		scb_s->scaol = 0;
> @@ -775,6 +1011,9 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>   		if (rc)
>   			goto unpin;
>   		vsie_page->sca_gpa = gpa;
> +		/* with vsie_sigpif scaoh and scaol will be overwritten
> +		 * in shadow_sca to point to g1 ssca_block instead
> +		 */

Same

>   		scb_s->scaoh = (u32)((u64)hpa >> 32);
>   		scb_s->scaol = (u32)(u64)hpa;
>   	}
> @@ -1490,12 +1729,17 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>   		goto out_unpin_scb;
>   	rc = pin_blocks(vcpu, vsie_page);
>   	if (rc)
> -		goto out_unshadow;
> +		goto out_unshadow_scb;
> +	rc = shadow_sca(vcpu, vsie_page);
> +	if (rc)
> +		goto out_unpin_blocks;
>   	register_shadow_scb(vcpu, vsie_page);
>   	rc = vsie_run(vcpu, vsie_page);
>   	unregister_shadow_scb(vcpu);

For personal preference I'd like to have a \n here to visually separate 
the cleanup from the rest of the code.

> +	unshadow_sca(vcpu, vsie_page);
> +out_unpin_blocks:
>   	unpin_blocks(vcpu, vsie_page);
> -out_unshadow:
> +out_unshadow_scb:
>   	unshadow_scb(vcpu, vsie_page);
>   out_unpin_scb:
>   	unpin_scb(vcpu, vsie_page, scb_addr);
> @@ -1510,12 +1754,15 @@ 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);
> +	init_rwsem(&kvm->arch.vsie.ssca_lock);
> +	INIT_RADIX_TREE(&kvm->arch.vsie.osca_to_ssca, GFP_KERNEL_ACCOUNT);
>   }
>   
>   /* 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;
> +	struct ssca_vsie *ssca;
>   	int i;
>   
>   	mutex_lock(&kvm->arch.vsie.mutex);
> @@ -1531,6 +1778,17 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
>   	}
>   	kvm->arch.vsie.page_count = 0;
>   	mutex_unlock(&kvm->arch.vsie.mutex);
> +
> +	down_write(&kvm->arch.vsie.ssca_lock);
> +	for (i = 0; i < kvm->arch.vsie.ssca_count; i++) {
> +		ssca = kvm->arch.vsie.sscas[i];
> +		kvm->arch.vsie.sscas[i] = NULL;
> +		radix_tree_delete(&kvm->arch.vsie.osca_to_ssca,
> +				  (u64)phys_to_virt(ssca->ssca.osca));
> +		free_pages((unsigned long)ssca, SSCA_PAGEORDER);
> +	}
> +	kvm->arch.vsie.ssca_count = 0;
> +	up_write(&kvm->arch.vsie.ssca_lock);
>   }
>   
>   void kvm_s390_vsie_kick(struct kvm_vcpu *vcpu)
> 

Re: [PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Christoph Schlameuss]

On Wed Mar 19, 2025 at 2:41 PM CET, Janosch Frank wrote:
> On 3/18/25 7:59 PM, Christoph Schlameuss wrote:
>> Introduce a new shadow_sca function into kvm_s390_handle_vsie.
>> kvm_s390_handle_vsie is called within guest-1 when guest-2 initiates the
>> VSIE.
>> 
>> shadow_sca and unshadow_sca create and manage ssca_block structs in
>> guest-1 memory. References to the created ssca_blocks are kept within an
>> array and limited to the number of cpus. This ensures each VSIE in
>> execution can have its own SCA. Having the amount of shadowed SCAs
>> configurable above this is left to another patch.
>> 
>> SCAOL/H in the VSIE control block are overwritten with references to the
>> shadow SCA. The original SCA pointer is saved in the vsie_page and
>> restored on VSIE exit. This limits the amount of change in the
>> preexisting VSIE pin and shadow functions.
>> 
>> 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.
>> 
>> The ssca_blocks are also kept within a radix tree to reuse already
>> existing ssca_blocks efficiently. While the radix tree and array with
>> references to the ssca_blocks are held in kvm_s390_vsie.
>> The use of the ssca_blocks is tracked using an ref_count on the block
>> itself.
>> 
>> No strict mapping between the guest-1 vcpu and guest-3 vcpu is enforced.
>> Instead each VSIE entry updates the shadow SCA creating a valid mapping
>> for all cpus currently in VSIE.
>> 
>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/kvm_host.h |  22 +++-
>>   arch/s390/kvm/vsie.c             | 264 ++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 281 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -29,6 +29,7 @@
>>   #define KVM_S390_BSCA_CPU_SLOTS 64
>>   #define KVM_S390_ESCA_CPU_SLOTS 248
>>   #define KVM_MAX_VCPUS 255
>> +#define KVM_S390_MAX_VCPUS 256
>
> #define KVM_S390_SSCA_CPU_SLOTS 256
>
> Yes, I'm aware, that ESCA and MAX_VCPUS are pretty confusing.
> I'm searching for solutions but they might take a while.
>
>>   
>>   #define KVM_INTERNAL_MEM_SLOTS 1
>>   
>> @@ -137,13 +138,23 @@ struct esca_block {
>>   
>>   /*
>>    * The shadow sca / ssca needs to cover both bsca and esca depending on what the
>> - * guest uses so we use KVM_S390_ESCA_CPU_SLOTS.
>> + * 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_ESCA_CPU_SLOTS];
>> +	struct ssca_entry cpu[KVM_S390_MAX_VCPUS];
>
> This should have been resolved in the previous patch, no?
>

Oops, yes, exactly.

>> +};
>> +
>> +/*
>> + * Store the vsie ssca block and accompanied management data.
>> + */
>> +struct ssca_vsie {
>> +	struct ssca_block ssca;			/* 0x0000 */
>> +	__u8	reserved[0x2200 - 0x2040];	/* 0x2040 */
>> +	atomic_t ref_count;			/* 0x2200 */
>>   };
>>   
>
> [...]
>
>>   void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, unsigned long start,
>>   				 unsigned long end)
>>   {
>> @@ -699,6 +932,9 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>   
>>   	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
>>   	if (hpa) {
>> +		/* with vsie_sigpif scaoh/l was pointing to g1 ssca_block but
>> +		 * should have been reset in unshadow_sca()
>> +		 */
>
> There shouldn't be text in the first or last line of multi-line comments.
>

Will fix. Thx. (checkpatch seems to be fine with this, so I assume just in not
desired?)

>>   		unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa);
>>   		vsie_page->sca_gpa = 0;
>>   		scb_s->scaol = 0;
>> @@ -775,6 +1011,9 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>   		if (rc)
>>   			goto unpin;
>>   		vsie_page->sca_gpa = gpa;
>> +		/* with vsie_sigpif scaoh and scaol will be overwritten
>> +		 * in shadow_sca to point to g1 ssca_block instead
>> +		 */
>
> Same
>
>>   		scb_s->scaoh = (u32)((u64)hpa >> 32);
>>   		scb_s->scaol = (u32)(u64)hpa;
>>   	}
>> @@ -1490,12 +1729,17 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>>   		goto out_unpin_scb;
>>   	rc = pin_blocks(vcpu, vsie_page);
>>   	if (rc)
>> -		goto out_unshadow;
>> +		goto out_unshadow_scb;
>> +	rc = shadow_sca(vcpu, vsie_page);
>> +	if (rc)
>> +		goto out_unpin_blocks;
>>   	register_shadow_scb(vcpu, vsie_page);
>>   	rc = vsie_run(vcpu, vsie_page);
>>   	unregister_shadow_scb(vcpu);
>
> For personal preference I'd like to have a \n here to visually separate 
> the cleanup from the rest of the code.
>

Sure. Will insert that.

>> +	unshadow_sca(vcpu, vsie_page);
>> +out_unpin_blocks:
>>   	unpin_blocks(vcpu, vsie_page);
>> -out_unshadow:
>> +out_unshadow_scb:
>>   	unshadow_scb(vcpu, vsie_page);
>>   out_unpin_scb:
>>   	unpin_scb(vcpu, vsie_page, scb_addr);
>> @@ -1510,12 +1754,15 @@ 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);
>> +	init_rwsem(&kvm->arch.vsie.ssca_lock);
>> +	INIT_RADIX_TREE(&kvm->arch.vsie.osca_to_ssca, GFP_KERNEL_ACCOUNT);
>>   }
>>   
>>   /* 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;
>> +	struct ssca_vsie *ssca;
>>   	int i;
>>   
>>   	mutex_lock(&kvm->arch.vsie.mutex);
>> @@ -1531,6 +1778,17 @@ void kvm_s390_vsie_destroy(struct kvm *kvm)
>>   	}
>>   	kvm->arch.vsie.page_count = 0;
>>   	mutex_unlock(&kvm->arch.vsie.mutex);
>> +
>> +	down_write(&kvm->arch.vsie.ssca_lock);
>> +	for (i = 0; i < kvm->arch.vsie.ssca_count; i++) {
>> +		ssca = kvm->arch.vsie.sscas[i];
>> +		kvm->arch.vsie.sscas[i] = NULL;
>> +		radix_tree_delete(&kvm->arch.vsie.osca_to_ssca,
>> +				  (u64)phys_to_virt(ssca->ssca.osca));
>> +		free_pages((unsigned long)ssca, SSCA_PAGEORDER);
>> +	}
>> +	kvm->arch.vsie.ssca_count = 0;
>> +	up_write(&kvm->arch.vsie.ssca_lock);
>>   }
>>   
>>   void kvm_s390_vsie_kick(struct kvm_vcpu *vcpu)
>> 

Re: [PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Janosch Frank]

On 3/19/25 3:41 PM, Christoph Schlameuss wrote:
> On Wed Mar 19, 2025 at 2:41 PM CET, Janosch Frank wrote:
>> On 3/18/25 7:59 PM, Christoph Schlameuss wrote:
>>> Introduce a new shadow_sca function into kvm_s390_handle_vsie.
>>> kvm_s390_handle_vsie is called within guest-1 when guest-2 initiates the
>>> VSIE.
>>>
>>> shadow_sca and unshadow_sca create and manage ssca_block structs in
>>> guest-1 memory. References to the created ssca_blocks are kept within an
>>> array and limited to the number of cpus. This ensures each VSIE in
>>> execution can have its own SCA. Having the amount of shadowed SCAs
>>> configurable above this is left to another patch.
>>>
>>> SCAOL/H in the VSIE control block are overwritten with references to the
>>> shadow SCA. The original SCA pointer is saved in the vsie_page and
>>> restored on VSIE exit. This limits the amount of change in the
>>> preexisting VSIE pin and shadow functions.
>>>
>>> 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.
>>>
>>> The ssca_blocks are also kept within a radix tree to reuse already
>>> existing ssca_blocks efficiently. While the radix tree and array with
>>> references to the ssca_blocks are held in kvm_s390_vsie.
>>> The use of the ssca_blocks is tracked using an ref_count on the block
>>> itself.
>>>
>>> No strict mapping between the guest-1 vcpu and guest-3 vcpu is enforced.
>>> Instead each VSIE entry updates the shadow SCA creating a valid mapping
>>> for all cpus currently in VSIE.
>>>
>>> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
>>> ---
>>>    arch/s390/include/asm/kvm_host.h |  22 +++-
>>>    arch/s390/kvm/vsie.c             | 264 ++++++++++++++++++++++++++++++++++++++-
>>>    2 files changed, 281 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>>> index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
>>> --- a/arch/s390/include/asm/kvm_host.h
>>> +++ b/arch/s390/include/asm/kvm_host.h
>>> @@ -29,6 +29,7 @@
>>>    #define KVM_S390_BSCA_CPU_SLOTS 64
>>>    #define KVM_S390_ESCA_CPU_SLOTS 248
>>>    #define KVM_MAX_VCPUS 255
>>> +#define KVM_S390_MAX_VCPUS 256
>>
>> #define KVM_S390_SSCA_CPU_SLOTS 256
>>
>> Yes, I'm aware, that ESCA and MAX_VCPUS are pretty confusing.
>> I'm searching for solutions but they might take a while.
>>
>>>    
>>>    #define KVM_INTERNAL_MEM_SLOTS 1
>>>    
>>> @@ -137,13 +138,23 @@ struct esca_block {
>>>    
>>>    /*
>>>     * The shadow sca / ssca needs to cover both bsca and esca depending on what the
>>> - * guest uses so we use KVM_S390_ESCA_CPU_SLOTS.
>>> + * 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_ESCA_CPU_SLOTS];
>>> +	struct ssca_entry cpu[KVM_S390_MAX_VCPUS];
>>
>> This should have been resolved in the previous patch, no?
>>
> 
> Oops, yes, exactly.
> 
>>> +};
>>> +
>>> +/*
>>> + * Store the vsie ssca block and accompanied management data.
>>> + */
>>> +struct ssca_vsie {
>>> +	struct ssca_block ssca;			/* 0x0000 */
>>> +	__u8	reserved[0x2200 - 0x2040];	/* 0x2040 */
>>> +	atomic_t ref_count;			/* 0x2200 */
>>>    };
>>>    
>>
>> [...]
>>
>>>    void kvm_s390_vsie_gmap_notifier(struct gmap *gmap, unsigned long start,
>>>    				 unsigned long end)
>>>    {
>>> @@ -699,6 +932,9 @@ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>    
>>>    	hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol;
>>>    	if (hpa) {
>>> +		/* with vsie_sigpif scaoh/l was pointing to g1 ssca_block but
>>> +		 * should have been reset in unshadow_sca()
>>> +		 */
>>
>> There shouldn't be text in the first or last line of multi-line comments.
>>
> 
> Will fix. Thx. (checkpatch seems to be fine with this, so I assume just in not
> desired?)

Might either be a personal preference as well or something that we don't 
really do in s390 KVM code.

Re: [PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Nico Boehr]

On Tue Mar 18, 2025 at 7:59 PM CET, Christoph Schlameuss wrote:
[...]
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
[...]
> +static struct ssca_vsie *get_ssca(struct kvm *kvm, struct vsie_page *vsie_page)
> +{
> +	u64 sca_o_hva = vsie_page->sca_o;
> +	phys_addr_t sca_o_hpa = virt_to_phys((void *)sca_o_hva);
> +	struct ssca_vsie *ssca, *ssca_new = NULL;
> +
> +	/* get existing ssca */
> +	down_read(&kvm->arch.vsie.ssca_lock);
> +	ssca = get_existing_ssca(kvm, sca_o_hva);
> +	up_read(&kvm->arch.vsie.ssca_lock);
> +	if (ssca)
> +		return ssca;

I would assume this is the most common case, no?

And below only happens rarely, right?

> +	/*
> +	 * Allocate new ssca, 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.)
> +	 */
> +	if (kvm->arch.vsie.ssca_count < atomic_read(&kvm->online_vcpus)) {
> +		BUILD_BUG_ON(offsetof(struct ssca_block, cpu) != 64);
> +		BUILD_BUG_ON(offsetof(struct ssca_vsie, ref_count) != 0x2200);
> +		BUILD_BUG_ON(sizeof(struct ssca_vsie) > ((1UL << SSCA_PAGEORDER)-1) * PAGE_SIZE);
> +		ssca_new = (struct ssca_vsie *)__get_free_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
> +								SSCA_PAGEORDER);
> +		if (!ssca_new) {
> +			ssca = ERR_PTR(-ENOMEM);
> +			goto out;
> +		}
> +		init_ssca(vsie_page, ssca_new);
> +	}
> +
> +	/* enter write lock and recheck to make sure ssca has not been created by other cpu */
> +	down_write(&kvm->arch.vsie.ssca_lock);

I am wondering whether it's really worth having this optimization of trying to
avoid taking the lock? Maybe we can accept a bit of contention on the rwlock
since it shouldn't happen very often and keep the code a bit less complex?

Re: [PATCH RFC 3/5] KVM: s390: Shadow VSIE SCA in guest-1

[Christoph Schlameuss]

On Thu Mar 20, 2025 at 4:22 PM CET, Nico Boehr wrote:
> On Tue Mar 18, 2025 at 7:59 PM CET, Christoph Schlameuss wrote:
> [...]
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 0aca5fa01f3d772c3b3dd62a22134c0d4cb9dc22..4ab196caa9e79e4c4d295d23fed65e1a142e6ab1 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
> [...]
>> +static struct ssca_vsie *get_ssca(struct kvm *kvm, struct vsie_page *vsie_page)
>> +{
>> +	u64 sca_o_hva = vsie_page->sca_o;
>> +	phys_addr_t sca_o_hpa = virt_to_phys((void *)sca_o_hva);
>> +	struct ssca_vsie *ssca, *ssca_new = NULL;
>> +
>> +	/* get existing ssca */
>> +	down_read(&kvm->arch.vsie.ssca_lock);
>> +	ssca = get_existing_ssca(kvm, sca_o_hva);
>> +	up_read(&kvm->arch.vsie.ssca_lock);
>> +	if (ssca)
>> +		return ssca;
>
> I would assume this is the most common case, no?
>
> And below only happens rarely, right?
>

By far, yes.

>> +	/*
>> +	 * Allocate new ssca, 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.)
>> +	 */
>> +	if (kvm->arch.vsie.ssca_count < atomic_read(&kvm->online_vcpus)) {
>> +		BUILD_BUG_ON(offsetof(struct ssca_block, cpu) != 64);
>> +		BUILD_BUG_ON(offsetof(struct ssca_vsie, ref_count) != 0x2200);
>> +		BUILD_BUG_ON(sizeof(struct ssca_vsie) > ((1UL << SSCA_PAGEORDER)-1) * PAGE_SIZE);
>> +		ssca_new = (struct ssca_vsie *)__get_free_pages(GFP_KERNEL_ACCOUNT | __GFP_ZERO,
>> +								SSCA_PAGEORDER);
>> +		if (!ssca_new) {
>> +			ssca = ERR_PTR(-ENOMEM);
>> +			goto out;
>> +		}
>> +		init_ssca(vsie_page, ssca_new);
>> +	}
>> +
>> +	/* enter write lock and recheck to make sure ssca has not been created by other cpu */
>> +	down_write(&kvm->arch.vsie.ssca_lock);
>
> I am wondering whether it's really worth having this optimization of trying to
> avoid taking the lock? Maybe we can accept a bit of contention on the rwlock
> since it shouldn't happen very often and keep the code a bit less complex?

With that reasoning I did not try to reduce the section under the write lock
further than it is now. I would hope this is a somewhat good balance. The
allocation really is the "worst" bit I would rather not do under the write lock
if possible.

I can try to make this a bit easier to read.

[PATCH RFC 4/5] KVM: s390: Re-init SSCA on switch to ESCA

[Christoph Schlameuss]

When the original SCA of a VSIE is switched from BSCA to ESCA (adding
the 65th processor) the addresses in the shadow SCA need to change as
well. It is sufficient to check for this on VSIE entry as all CPUs are
kicked out of VSIE for the migration.
This patch adds the necessary code where the original state description
address of the SSCA entry is updated.

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  1 +
 arch/s390/kvm/vsie.c             | 24 ++++++++++++++++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 4ab196caa9e79e4c4d295d23fed65e1a142e6ab1..e44f43906844d3b629e9685637af3f66398a4a8d 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -155,6 +155,7 @@ struct ssca_vsie {
 	struct ssca_block ssca;			/* 0x0000 */
 	__u8	reserved[0x2200 - 0x2040];	/* 0x2040 */
 	atomic_t ref_count;			/* 0x2200 */
+	__u8	is_esca;
 };
 
 /*
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 0327c4964d27e493932a2b90b62c5a27b0a95446..3ddebebf8e9e90be3d5e27b6dc91d91214c3ea34 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -71,6 +71,11 @@ struct vsie_page {
 	__u8 fac[S390_ARCH_FAC_LIST_SIZE_BYTE];	/* 0x0800 */
 };
 
+static inline bool vsie_uses_esca(struct vsie_page *vsie_page)
+{
+	return (vsie_page->scb_s.ecb2 & ECB2_ESCA);
+}
+
 /* trigger a validity icpt for the given scb */
 static int set_validity_icpt(struct kvm_s390_sie_block *scb,
 			     __u16 reason_code)
@@ -605,7 +610,7 @@ static void init_ssca(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
 	unsigned int bit, cpu_slots;
 	struct ssca_entry *cpu;
 	void *ossea_hva;
-	int is_esca;
+	bool is_esca;
 	u64 *mcn;
 
 	/* set original SIE control block address */
@@ -613,11 +618,12 @@ static void init_ssca(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
 	WARN_ON_ONCE(ssca->ssca.osca & 0x000f);
 
 	/* use ECB of shadow scb to determine SCA type */
-	is_esca = (vsie_page->scb_s.ecb2 & ECB2_ESCA);
+	is_esca = vsie_uses_esca(vsie_page);
 	cpu_slots = is_esca ? KVM_S390_MAX_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
 	mcn = is_esca ? ((struct esca_block *)sca_o_hva)->mcn :
 			&((struct bsca_block *)sca_o_hva)->mcn;
 
+	ssca->is_esca = is_esca;
 	/*
 	 * For every enabled sigp entry in the original sca we need to populate
 	 * the corresponding shadow sigp entry with the address of the shadow
@@ -643,10 +649,20 @@ static void update_entry_ssda_remove(struct vsie_page *vsie_page, struct ssca_vs
 }
 
 /* add running scb pointer to ssca */
-static void update_entry_ssda_add(struct vsie_page *vsie_page, struct ssca_vsie *ssca)
+static void update_entry_ssda_add(struct kvm *kvm, struct vsie_page *vsie_page,
+				  struct ssca_vsie *ssca)
 {
 	struct ssca_entry *cpu = &ssca->ssca.cpu[vsie_page->scb_s.icpua & 0xff];
 	phys_addr_t scb_s_hpa = virt_to_phys(&vsie_page->scb_s);
+	bool is_esca = vsie_uses_esca(vsie_page);
+
+	/* update original sca entry addresses after bsca / esca switch */
+	if (!ssca->is_esca && is_esca) {
+		down_write(&kvm->arch.vsie.ssca_lock);
+		if (!ssca->is_esca && is_esca)
+			init_ssca(vsie_page, ssca);
+		up_write(&kvm->arch.vsie.ssca_lock);
+	}
 
 	WRITE_ONCE(cpu->ssda, scb_s_hpa);
 }
@@ -815,7 +831,7 @@ static int shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		return PTR_ERR(ssca);
 
 	/* update shadow control block sca references to shadow sca */
-	update_entry_ssda_add(vsie_page, ssca);
+	update_entry_ssda_add(vcpu->kvm, vsie_page, ssca);
 	sca_s_hpa = virt_to_phys(ssca);
 	if (sclp.has_64bscao) {
 		WARN_ON_ONCE(sca_s_hpa & 0x003f);

-- 
2.48.1

[PATCH RFC 5/5] KVM: s390: Add VSIE shadow stat counters

[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 cpus.
* vsie_shadow_sca shows the number of allocated system control area
  shadows. Should count upwards between 0 and the max number of cpus.
* vsie_shadow_sca_create shows the number of newly allocated system
  control area 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             | 7 ++++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index e44f43906844d3b629e9685637af3f66398a4a8d..909c662ac4e3e1e70a2e3e9054acee14bc20ed02 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -824,6 +824,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_sca;
+	u64 vsie_shadow_sca_create;
+	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 16204c638119fa3a6c36e8e24af2b0b399f8123b..aba798e7814be6011d71a1e1be894e2c0a6b2bb2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -76,6 +76,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_sca),
+	STATS_DESC_COUNTER(VM, vsie_shadow_sca_create),
+	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 3ddebebf8e9e90be3d5e27b6dc91d91214c3ea34..7b599b6eb2ceb4141b8f1489804aef5dcd429ea0 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -678,8 +678,10 @@ static struct ssca_vsie *get_existing_ssca(struct kvm *kvm, u64 sca_o_hva)
 {
 	struct ssca_vsie *ssca = radix_tree_lookup(&kvm->arch.vsie.osca_to_ssca, sca_o_hva);
 
-	if (ssca)
+	if (ssca) {
 		WARN_ON_ONCE(atomic_inc_return(&ssca->ref_count) < 1);
+		kvm->stat.vsie_shadow_sca_reuse++;
+	}
 	return ssca;
 }
 
@@ -755,6 +757,7 @@ static struct ssca_vsie *get_ssca(struct kvm *kvm, struct vsie_page *vsie_page)
 
 		kvm->arch.vsie.sscas[kvm->arch.vsie.ssca_count] = ssca;
 		kvm->arch.vsie.ssca_count++;
+		kvm->stat.vsie_shadow_sca++;
 	} else {
 		/* reuse previously created ssca for different osca */
 		ssca = get_free_existing_ssca(kvm);
@@ -771,6 +774,7 @@ static struct ssca_vsie *get_ssca(struct kvm *kvm, struct vsie_page *vsie_page)
 
 	/* virt_to_phys(sca_o_hva) == ssca->osca */
 	radix_tree_insert(&kvm->arch.vsie.osca_to_ssca, sca_o_hva, ssca);
+	kvm->stat.vsie_shadow_sca_create++;
 	WRITE_ONCE(ssca->ssca.osca, sca_o_hpa);
 
 out:
@@ -1672,6 +1676,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
 		__set_bit(VSIE_PAGE_IN_USE, &vsie_page->flags);
 		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) {

-- 
2.48.1