Use DIAG318 to set Control Program Name & Version Codes

PATCH Superseded Collin Walling 5 participants 16 messages Jun 18, 2020 – Jun 22, 2020

Subsystems (3)

KVM s390 KVM s390

lore

Share & tools

lore

Collin Walling

author

2020-06-18 22:22 UTC #0 lore raw

v8 cover Use DIAG318 to set Control Program Name & Version Codes

Changelog:

• Reset is handled in KVM during initial and clear resets
• Sync/Store register handling
• Removed device IOCTL code
• Added KVM Capability for DIAG318
- this is for determining if the CPU model can enable this feature
• Reverted changes introduced by bullet 3 in v7
• Unshadowing CPNC again, as it makes sense if the guest executing in VSIE sets a unique name/version code; this data should be preserved
- reverts bullet 4 in v3
• Diag318 is no longer reported via VM or CPU events
- no place to put this such that the messages aren't flooding the logs
- not necessary, as this data is primarily for IBM hardware/firmware service events, and is observable via such events (e.g. CPU ring dump)
- was nice for testing purposes
• A copy of the diag318 info (name & version code) is now stored in the kvm_vcpu_arch struct, as opposed to the kvm_arch struct

• Removed diag handler, as it will now take place within userspace
• Removed KVM_S390_VM_MISC_ENABLE_DIAG318 (undoes first bullet in v6)
• Misc clean ups and fixes [removed in v8]
- introduced a new patch to s/diag318/diag_318 and s/byte_134/fac134 to keep things consistent with the rest of the code

• KVM disables diag318 get/set by default [removed in v7]
• added new IOCTL to tell KVM to enable diag318 [removed in v7]
• removed VCPU event message in favor of VM_EVENT only [removed in v8]

• s/cpc/diag318_info in order to make the relevant data more clear
• removed mutex locks for diag318_info get/set

• removed setup.c changes introduced in bullet 1 of v3
• kept diag318_info struct cleanup
• analogous QEMU patches:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00164.html

• kernel patch for diag 0x318 instruction call fixup [removed in v4]
• removed CPU model code
• cleaned up diag318_info struct
• cpnc is no longer unshadowed as it was not needed [removed in v8]
• rebased on 5.1.0-rc3

-------------------------------------------------------------------------------

This instruction call is executed once-and-only-once during Kernel setup. The availability of this instruction depends on Read Info byte 134 (aka fac134), bit 0.

DIAG 0x318's is handled by userspace and may be enabled for a guest even if the host kernel cannot support it.

The diag318_info is composed of a Control Program Name Code (CPNC) and a Control Program Version Code (CPVC). The CPNC is stored in the SIE block, and the CPNC & CPVC pair is stored in the kvm_vcpu_arch struct.

These values are used for problem diagnosis and allows IBM to identify control program information by answering the following question:

"What environment is this guest running in?" (CPNC) "What are more details regarding the OS?" (CPVC)

In the future, we will implement the CPVC to convey more information about the OS (such as Linux version and potentially some value denoting a specific distro + release). For now, we set this field to 0 until we come up with a solid plan.

Collin Walling (2):
s390/setup: diag 318: refactor struct
s390/kvm: diagnose 0x318 sync and reset

 arch/s390/include/asm/diag.h     |  6 ++----
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kernel/setup.c         |  3 +--
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 7 files changed, 24 insertions(+), 9 deletions(-)

Signature · 1 line

2.21.3

Collin Walling

author

2020-06-18 22:22 UTC #1 lore raw

PATCH v8 1/2 s390/setup: diag 318: refactor struct

The diag 318 struct introduced in include/asm/diag.h can be reused in KVM, so let's condense the version code fields in the diag318_info struct for easier usage and simplify it until we can determine how the data should be formatted.

Signed-off-by Collin Walling <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Reviewed-by Cornelia Huck <[email protected]>

Reviewed-by Thomas Huth <[email protected]>

Reviewed-by Janosch Frank <[email protected]>

---
 arch/s390/include/asm/diag.h | 6 ++----
 arch/s390/kernel/setup.c     | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

arch/s390/include/asm/diag.h +2 -4

index 0036eab14391..ca8f85b53a90 100644
@@ -298,10 +298,8 @@ struct diag26c_mac_resp {
 union diag318_info {
 	unsigned long val;
 	struct {
-		unsigned int cpnc : 8;
-		unsigned int cpvc_linux : 24;
-		unsigned char cpvc_distro[3];
-		unsigned char zero;
+		unsigned long cpnc : 8;
+		unsigned long cpvc : 56;
 	};
 };

arch/s390/kernel/setup.c +1 -2

index 5853c9872dfe..878cacfc9c3e 100644
@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void)
 {
 	union diag318_info diag318_info = {
 		.cpnc = CPNC_LINUX,
-		.cpvc_linux = 0,
-		.cpvc_distro = {0},
+		.cpvc = 0,
 	};
 
 	if (!sclp.has_diag318)

Signature · 1 line

2.21.3

Collin Walling

author

2020-06-18 22:22 UTC #4 lore raw

PATCH v8 2/2 s390/kvm: diagnose 0x318 sync and reset

In reply to Collin Walling

DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.

This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.

The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.

The CPNC is shadowed/unshadowed in VSIE.

This data is reset on load normal and clear resets.

Signed-off-by Collin Walling <[email protected]>

---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)

arch/s390/include/asm/kvm_host.h +3 -1

index 3d554887794e..8bdf6f1607ca 100644
@@ -260,7 +260,8 @@ struct kvm_s390_sie_block {
 	__u32	scaol;			/* 0x0064 */
 	__u8	sdf;			/* 0x0068 */
 	__u8    epdx;			/* 0x0069 */
-	__u8    reserved6a[2];		/* 0x006a */
+	__u8	cpnc;			/* 0x006a */
+	__u8	reserved6b;		/* 0x006b */
 	__u32	todpr;			/* 0x006c */
 #define GISA_FORMAT1 0x00000001
 	__u32	gd;			/* 0x0070 */

@@ -745,6 +746,7 @@ struct kvm_vcpu_arch {
 	bool gs_enabled;
 	bool skey_enabled;
 	struct kvm_s390_pv_vcpu pv;
+	union diag318_info diag318_info;
 };
 
 struct kvm_vm_stat {

arch/s390/include/uapi/asm/kvm.h +4 -1

index 436ec7636927..2ae1b660086c 100644
@@ -231,11 +231,13 @@ struct kvm_guest_debug_arch {
 #define KVM_SYNC_GSCB   (1UL << 9)
 #define KVM_SYNC_BPBC   (1UL << 10)
 #define KVM_SYNC_ETOKEN (1UL << 11)
+#define KVM_SYNC_DIAG318 (1UL << 12)
 
 #define KVM_SYNC_S390_VALID_FIELDS \
 	(KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
 	 KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
-	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \
+	 KVM_SYNC_DIAG318)
 
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8

@@ -254,6 +256,7 @@ struct kvm_sync_regs {
 	__u64 pft;	/* pfault token [PFAULT] */
 	__u64 pfs;	/* pfault select [PFAULT] */
 	__u64 pfc;	/* pfault compare [PFAULT] */
+	__u64 diag318;	/* diagnose 0x318 info */
 	union {
 		__u64 vrs[32][2];	/* vector registers (KVM_SYNC_VRS) */
 		__u64 fprs[16];		/* fp registers (KVM_SYNC_FPRS) */

arch/s390/kvm/kvm-s390.c +10 -1

index d0ff26d157bc..b05ad718b64b 100644
@@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_AIS_MIGRATION:
 	case KVM_CAP_S390_VCPU_RESETS:
 	case KVM_CAP_SET_GUEST_DEBUG:
+	case KVM_CAP_S390_DIAG318:
 		r = 1;
 		break;
 	case KVM_CAP_S390_HPAGE_1M:

@@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 				    KVM_SYNC_ACRS |
 				    KVM_SYNC_CRS |
 				    KVM_SYNC_ARCH0 |
-				    KVM_SYNC_PFAULT;
+				    KVM_SYNC_PFAULT |
+				    KVM_SYNC_DIAG318;
 	kvm_s390_set_prefix(vcpu, 0);
 	if (test_kvm_facility(vcpu->kvm, 64))
 		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;

@@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
 		vcpu->arch.sie_block->pp = 0;
 		vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
 		vcpu->arch.sie_block->todpr = 0;
+		vcpu->arch.sie_block->cpnc = 0;
 	}
 }

@@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu)
 
 	regs->etoken = 0;
 	regs->etoken_extension = 0;
+	regs->diag318 = 0;
 }
 
 int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)

@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.

@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)

arch/s390/kvm/vsie.c +3 -0

index 9e9056cebfcf..ba83d0568bc7 100644
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;
+
 	if (scb_s->ihcpu != 0xffffU)
 		scb_o->ihcpu = scb_s->ihcpu;
 }

@@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		scb_s->ecd |= scb_o->ecd & ECD_ETOKENF;
 
 	scb_s->hpid = HPID_VSIE;
+	scb_s->cpnc = scb_o->cpnc;
 
 	prepare_ibc(vcpu, vsie_page);
 	rc = shadow_crycb(vcpu, vsie_page);

include/uapi/linux/kvm.h +1 -0

index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184
 
 #ifdef KVM_CAP_IRQ_ROUTING

Signature · 1 line

2.21.3

Janosch Frank

maintainer Reviewed-by on v8 2/2

2020-06-19 11:02 UTC #5 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 12 lines · Collin Walling

DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.

Quoted 5 lines

The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>

Reviewed-by Janosch Frank <[email protected]>

Could you extend the s390 kvm selftests sync_regs test with diag318 please?

I'd also like to have it added to the kvm unit tests. You can either do that yourself or I'll add it when I go over my pending patches. Since we can't retrieve these values from the VM, a simple check for the sclp feature bit and an execution of the instruction would be enough.

Quoted 148 lines

---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)

arch/s390/include/asm/kvm_host.h +3 -1

index 3d554887794e..8bdf6f1607ca 100644
@@ -260,7 +260,8 @@ struct kvm_s390_sie_block {
 	__u32	scaol;			/* 0x0064 */
 	__u8	sdf;			/* 0x0068 */
 	__u8    epdx;			/* 0x0069 */
-	__u8    reserved6a[2];		/* 0x006a */
+	__u8	cpnc;			/* 0x006a */
+	__u8	reserved6b;		/* 0x006b */
 	__u32	todpr;			/* 0x006c */
 #define GISA_FORMAT1 0x00000001
 	__u32	gd;			/* 0x0070 */

@@ -745,6 +746,7 @@ struct kvm_vcpu_arch {
 	bool gs_enabled;
 	bool skey_enabled;
 	struct kvm_s390_pv_vcpu pv;
+	union diag318_info diag318_info;
 };
 
 struct kvm_vm_stat {

arch/s390/include/uapi/asm/kvm.h +4 -1

index 436ec7636927..2ae1b660086c 100644
@@ -231,11 +231,13 @@ struct kvm_guest_debug_arch {
 #define KVM_SYNC_GSCB   (1UL << 9)
 #define KVM_SYNC_BPBC   (1UL << 10)
 #define KVM_SYNC_ETOKEN (1UL << 11)
+#define KVM_SYNC_DIAG318 (1UL << 12)
 
 #define KVM_SYNC_S390_VALID_FIELDS \
 	(KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
 	 KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
-	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \
+	 KVM_SYNC_DIAG318)
 
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8

@@ -254,6 +256,7 @@ struct kvm_sync_regs {
 	__u64 pft;	/* pfault token [PFAULT] */
 	__u64 pfs;	/* pfault select [PFAULT] */
 	__u64 pfc;	/* pfault compare [PFAULT] */
+	__u64 diag318;	/* diagnose 0x318 info */
 	union {
 		__u64 vrs[32][2];	/* vector registers (KVM_SYNC_VRS) */
 		__u64 fprs[16];		/* fp registers (KVM_SYNC_FPRS) */

arch/s390/kvm/kvm-s390.c +10 -1

index d0ff26d157bc..b05ad718b64b 100644
@@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_AIS_MIGRATION:
 	case KVM_CAP_S390_VCPU_RESETS:
 	case KVM_CAP_SET_GUEST_DEBUG:
+	case KVM_CAP_S390_DIAG318:
 		r = 1;
 		break;
 	case KVM_CAP_S390_HPAGE_1M:

@@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 				    KVM_SYNC_ACRS |
 				    KVM_SYNC_CRS |
 				    KVM_SYNC_ARCH0 |
-				    KVM_SYNC_PFAULT;
+				    KVM_SYNC_PFAULT |
+				    KVM_SYNC_DIAG318;
 	kvm_s390_set_prefix(vcpu, 0);
 	if (test_kvm_facility(vcpu->kvm, 64))
 		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;

@@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
 		vcpu->arch.sie_block->pp = 0;
 		vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
 		vcpu->arch.sie_block->todpr = 0;
+		vcpu->arch.sie_block->cpnc = 0;
 	}
 }

@@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu)
 
 	regs->etoken = 0;
 	regs->etoken_extension = 0;
+	regs->diag318 = 0;
 }
 
 int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)

@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.

@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)

arch/s390/kvm/vsie.c +3 -0

index 9e9056cebfcf..ba83d0568bc7 100644
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;
+
 	if (scb_s->ihcpu != 0xffffU)
 		scb_o->ihcpu = scb_s->ihcpu;
 }

@@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		scb_s->ecd |= scb_o->ecd & ECD_ETOKENF;
 
 	scb_s->hpid = HPID_VSIE;
+	scb_s->cpnc = scb_o->cpnc;
 
 	prepare_ibc(vcpu, vsie_page);
 	rc = shadow_crycb(vcpu, vsie_page);

include/uapi/linux/kvm.h +1 -0

index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184
 
 #ifdef KVM_CAP_IRQ_ROUTING

Collin Walling

author

2020-06-19 14:45 UTC #6 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 15 lines · Janosch Frank

Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.

Quoted 8 lines

Quoted 3 lines
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[email protected]>
Could you extend the s390 kvm selftests sync_regs test with diag318 please?

Can do!

Quoted 155 lines

Quoted 148 lines

---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)

arch/s390/include/asm/kvm_host.h +3 -1

index 3d554887794e..8bdf6f1607ca 100644
@@ -260,7 +260,8 @@ struct kvm_s390_sie_block {
 	__u32	scaol;			/* 0x0064 */
 	__u8	sdf;			/* 0x0068 */
 	__u8    epdx;			/* 0x0069 */
-	__u8    reserved6a[2];		/* 0x006a */
+	__u8	cpnc;			/* 0x006a */
+	__u8	reserved6b;		/* 0x006b */
 	__u32	todpr;			/* 0x006c */
 #define GISA_FORMAT1 0x00000001
 	__u32	gd;			/* 0x0070 */

@@ -745,6 +746,7 @@ struct kvm_vcpu_arch {
 	bool gs_enabled;
 	bool skey_enabled;
 	struct kvm_s390_pv_vcpu pv;
+	union diag318_info diag318_info;
 };
 
 struct kvm_vm_stat {

arch/s390/include/uapi/asm/kvm.h +4 -1

index 436ec7636927..2ae1b660086c 100644
@@ -231,11 +231,13 @@ struct kvm_guest_debug_arch {
 #define KVM_SYNC_GSCB   (1UL << 9)
 #define KVM_SYNC_BPBC   (1UL << 10)
 #define KVM_SYNC_ETOKEN (1UL << 11)
+#define KVM_SYNC_DIAG318 (1UL << 12)
 
 #define KVM_SYNC_S390_VALID_FIELDS \
 	(KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
 	 KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
-	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \
+	 KVM_SYNC_DIAG318)
 
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8

@@ -254,6 +256,7 @@ struct kvm_sync_regs {
 	__u64 pft;	/* pfault token [PFAULT] */
 	__u64 pfs;	/* pfault select [PFAULT] */
 	__u64 pfc;	/* pfault compare [PFAULT] */
+	__u64 diag318;	/* diagnose 0x318 info */
 	union {
 		__u64 vrs[32][2];	/* vector registers (KVM_SYNC_VRS) */
 		__u64 fprs[16];		/* fp registers (KVM_SYNC_FPRS) */

arch/s390/kvm/kvm-s390.c +10 -1

index d0ff26d157bc..b05ad718b64b 100644
@@ -545,6 +545,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_AIS_MIGRATION:
 	case KVM_CAP_S390_VCPU_RESETS:
 	case KVM_CAP_SET_GUEST_DEBUG:
+	case KVM_CAP_S390_DIAG318:
 		r = 1;
 		break;
 	case KVM_CAP_S390_HPAGE_1M:

@@ -3267,7 +3268,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 				    KVM_SYNC_ACRS |
 				    KVM_SYNC_CRS |
 				    KVM_SYNC_ARCH0 |
-				    KVM_SYNC_PFAULT;
+				    KVM_SYNC_PFAULT |
+				    KVM_SYNC_DIAG318;
 	kvm_s390_set_prefix(vcpu, 0);
 	if (test_kvm_facility(vcpu->kvm, 64))
 		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;

@@ -3562,6 +3564,7 @@ static void kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
 		vcpu->arch.sie_block->pp = 0;
 		vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
 		vcpu->arch.sie_block->todpr = 0;
+		vcpu->arch.sie_block->cpnc = 0;
 	}
 }

@@ -3579,6 +3582,7 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu)
 
 	regs->etoken = 0;
 	regs->etoken_extension = 0;
+	regs->diag318 = 0;
 }
 
 int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)

@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.

@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)

arch/s390/kvm/vsie.c +3 -0

index 9e9056cebfcf..ba83d0568bc7 100644
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;
+
 	if (scb_s->ihcpu != 0xffffU)
 		scb_o->ihcpu = scb_s->ihcpu;
 }

@@ -548,6 +550,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		scb_s->ecd |= scb_o->ecd & ECD_ETOKENF;
 
 	scb_s->hpid = HPID_VSIE;
+	scb_s->cpnc = scb_o->cpnc;
 
 	prepare_ibc(vcpu, vsie_page);
 	rc = shadow_crycb(vcpu, vsie_page);

include/uapi/linux/kvm.h +1 -0

index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184
 
 #ifdef KVM_CAP_IRQ_ROUTING

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

David Hildenbrand

2020-06-19 14:53 UTC #7 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

In reply to Collin Walling

Quoted 14 lines · Collin Walling

DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.

[...]

Quoted 26 lines

int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)

arch/s390/kvm/kvm-s390.c +5 -0

@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.

@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)

arch/s390/kvm/vsie.c +0 -0

index 9e9056cebfcf..ba83d0568bc7 100644

arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426

Quoted 4 lines

arch/s390/kvm/vsie.c +1 -0

@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;

"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?

Apart from that LGTM.

Signature · 3 lines

Thanks,

David / dhildenb

Collin Walling

author

2020-06-19 15:47 UTC #8 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 47 lines · David Hildenbrand

Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 29 lines
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
arch/s390/kvm/kvm-s390.c +5 -0
@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.
@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)
arch/s390/kvm/vsie.c +0 -0
index 9e9056cebfcf..ba83d0568bc7 100644
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 

Quoted 7 lines

arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 1 line
arch/s390/kvm/vsie.c +1 -0
+	scb_o->cpnc = scb_s->cpnc;
"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?
Apart from that LGTM.

I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)?

* Shadow SCB (load shadow VSIE page; originally CPNC is 0)

* Execute diag318 (under SIE)

* Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set)

Don't we need to preserve the CPNC for the level 3+ guest somehow?

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

David Hildenbrand

2020-06-19 17:18 UTC #9 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 53 lines · Collin Walling

Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
arch/s390/kvm/kvm-s390.c +5 -0
@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.
@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)
arch/s390/kvm/vsie.c +0 -0
index 9e9056cebfcf..ba83d0568bc7 100644
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1 -0
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;
"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?

Quoted 7 lines

Quoted 2 lines
Apart from that LGTM.
I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)?
* Shadow SCB (load shadow VSIE page; originally CPNC is 0)

1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB.

Quoted 2 lines

* Execute diag318 (under SIE)

2. Here the SIE runs using the shadow SCB.

Quoted 3 lines

* Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set)

3. Here you copy back the cpnc from the shadow SCB to the pinned (original) SCB.

If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be dropped, because the values will always match.

If guest3 tries to modify the cpnc (via diag 318), we exit the SIE (intercept) in 2., return to our guest 2. guest 2 will perform the change and adapt the original SCB.

(yep, it's confusing)

Or did I miss anything?

Signature · 3 lines

Thanks,

David / dhildenb

Collin Walling

author

2020-06-19 17:55 UTC #10 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 81 lines · David Hildenbrand

Quoted 53 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
arch/s390/kvm/kvm-s390.c +5 -0
@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
 			kvm_clear_async_pf_completion_queue(vcpu);
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+		vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+		vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+	}
 	/*
 	 * If userspace sets the riccb (e.g. after migration) to a valid state,
 	 * we should enable RI here instead of doing the lazy enablement.
@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
 	kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
 	kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+	kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
 	if (MACHINE_HAS_GS) {
 		__ctl_set_bit(2, 4);
 		if (vcpu->arch.gs_enabled)
arch/s390/kvm/vsie.c +0 -0
index 9e9056cebfcf..ba83d0568bc7 100644
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1 -0
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		break;
 	}
 
+	scb_o->cpnc = scb_s->cpnc;
"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?
Quoted 7 lines
Quoted 2 lines
Apart from that LGTM.
I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)?
* Shadow SCB (load shadow VSIE page; originally CPNC is 0)
1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set)
3. Here you copy back the cpnc from the shadow SCB to the pinned (original) SCB.
If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be dropped, because the values will always match.

Quoted 8 lines

If guest3 tries to modify the cpnc (via diag 318), we exit the SIE (intercept) in 2., return to our guest 2. guest 2 will perform the change and adapt the original SCB.
(yep, it's confusing)
Or did I miss anything?

Ah, I see. So the shadowing isn't necessarily for SIE block values, but for storing the register / PSW / clock states, as well as facility bits for the level 3+ guests? Looking at what the vsie code does, that seems to make sense.

So we don't need to shadow OR unshadow the CPNC, then?

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

David Hildenbrand

2020-06-19 18:13 UTC #11 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 90 lines

Am 19.06.2020 um 19:56 schrieb Collin Walling <[email protected]>:
Quoted 85 lines · David Hildenbrand
Quoted 54 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
arch/s390/kvm/kvm-s390.c +5 -0
@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
       if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
           kvm_clear_async_pf_completion_queue(vcpu);
   }
+    if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+        vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+        vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+    }
   /*
    * If userspace sets the riccb (e.g. after migration) to a valid state,
    * we should enable RI here instead of doing the lazy enablement.
@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
   kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
   kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
   kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+    kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
   if (MACHINE_HAS_GS) {
       __ctl_set_bit(2, 4);
       if (vcpu->arch.gs_enabled)
arch/s390/kvm/vsie.c +0 -0
index 9e9056cebfcf..ba83d0568bc7 100644
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1 -0
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
       break;
   }
 
+    scb_o->cpnc = scb_s->cpnc;
"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?
Quoted 7 lines
Quoted 2 lines
Apart from that LGTM.
I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)?
* Shadow SCB (load shadow VSIE page; originally CPNC is 0)
1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set)
3. Here you copy back the cpnc from the shadow SCB to the pinned (original) SCB.
If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be dropped, because the values will always match.
If guest3 tries to modify the cpnc (via diag 318), we exit the SIE (intercept) in 2., return to our guest 2. guest 2 will perform the change and adapt the original SCB.

Quoted 8 lines

Quoted 4 lines
(yep, it's confusing)
Or did I miss anything?
Ah, I see. So the shadowing isn't necessarily for SIE block values, but for storing the register / PSW / clock states, as well as facility bits for the level 3+ guests? Looking at what the

We have to forward all values the SIE has to see and copy back only what could have been changed by the SIE.

Quoted 4 lines

vsie code does, that seems
to make sense.
So we don't need to shadow OR unshadow the CPNC, then?

I think you have to shadow (forward the value) but not unshadow (value cannot change).

Cheers!

Quoted 7 lines

Signature · 5 lines

-- 
Regards,
Collin

Stay safe and stay healthy

Collin Walling

author

2020-06-19 18:46 UTC #12 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 101 lines · David Hildenbrand

Quoted 90 lines
Am 19.06.2020 um 19:56 schrieb Collin Walling <[email protected]>:
Quoted 85 lines · David Hildenbrand
Quoted 54 lines · Collin Walling
Quoted 52 lines · David Hildenbrand
Quoted 14 lines · Collin Walling
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
[...]
Quoted 26 lines
int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
arch/s390/kvm/kvm-s390.c +5 -0
@@ -4194,6 +4198,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
       if (vcpu->arch.pfault_token == KVM_S390_PFAULT_TOKEN_INVALID)
           kvm_clear_async_pf_completion_queue(vcpu);
   }
+    if (kvm_run->kvm_dirty_regs & KVM_SYNC_DIAG318) {
+        vcpu->arch.diag318_info.val = kvm_run->s.regs.diag318;
+        vcpu->arch.sie_block->cpnc = vcpu->arch.diag318_info.cpnc;
+    }
   /*
    * If userspace sets the riccb (e.g. after migration) to a valid state,
    * we should enable RI here instead of doing the lazy enablement.
@@ -4295,6 +4303,7 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
   kvm_run->s.regs.pp = vcpu->arch.sie_block->pp;
   kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea;
   kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC;
+    kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val;
   if (MACHINE_HAS_GS) {
       __ctl_set_bit(2, 4);
       if (vcpu->arch.gs_enabled)
arch/s390/kvm/vsie.c +0 -0
index 9e9056cebfcf..ba83d0568bc7 100644
arch/s390/kvm/vsie.c static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) · line 426
Quoted 4 lines
arch/s390/kvm/vsie.c +1 -0
@@ -423,6 +423,8 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
       break;
   }
 
+    scb_o->cpnc = scb_s->cpnc;
"This is a privileged s390x instruction that must be intercepted", how can the cpnc change, then, while in SIE?
Quoted 7 lines
Quoted 2 lines
Apart from that LGTM.
I thought shadow/unshadow was a load/store (respectively) when executing in SIE for a level 3+ guest (where LPAR is level 1)?
* Shadow SCB (load shadow VSIE page; originally CPNC is 0)
1. Here, you copy the cpnc from the pinned (original) SCB to the shadow SCB.
Quoted 2 lines
* Execute diag318 (under SIE)
2. Here the SIE runs using the shadow SCB.
Quoted 3 lines
* Unshadow SCB (store in original VSIE page; CPNC is whatever code the guest decided to set)
3. Here you copy back the cpnc from the shadow SCB to the pinned (original) SCB.
If 2. cannot modify the cpnc residing in the shadow SCB, 3. can be dropped, because the values will always match.
If guest3 tries to modify the cpnc (via diag 318), we exit the SIE (intercept) in 2., return to our guest 2. guest 2 will perform the change and adapt the original SCB.
Quoted 8 lines
Quoted 4 lines
(yep, it's confusing)
Or did I miss anything?
Ah, I see. So the shadowing isn't necessarily for SIE block values, but for storing the register / PSW / clock states, as well as facility bits for the level 3+ guests? Looking at what the
We have to forward all values the SIE has to see and copy back only what could have been changed by the SIE.

Quoted 9 lines

Quoted 4 lines
vsie code does, that seems
to make sense.
So we don't need to shadow OR unshadow the CPNC, then?
I think you have to shadow (forward the value) but not unshadow (value cannot change).
Cheers!

Gotcha. Very tricky. I'll have to study on it some more. Thanks for the info!

Take care.

Quoted 8 lines

Quoted 7 lines
Signature · 5 lines
-- 
Regards,
Collin

Stay safe and stay healthy

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

Cornelia Huck

2020-06-22 10:25 UTC #13 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

In reply to Collin Walling

Quoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:

DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)

(...)

include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034

Quoted 9 lines

include/uapi/linux/kvm.h +1 -0

index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184

Do we strictly need this new cap, or would checking against the sync regs capabilities be enough?

Quoted 3 lines

include/uapi/linux/kvm.h +0 -0
 #ifdef KVM_CAP_IRQ_ROUTING
 

Christian Borntraeger

2020-06-22 14:51 UTC #14 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

Quoted 31 lines · Cornelia Huck

Quoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)
(...)

Quoted 12 lines

include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034
Quoted 9 lines
include/uapi/linux/kvm.h +1 -0
index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184
Do we strictly need this new cap, or would checking against the sync regs capabilities be enough?

We could check the sync_regs valid field to decide about the sync. We do that for ETOKEN as well and QEMU also uses it in handle_diag_318.

I think what this is used for is actually to tell the QEMU CPU model if this is there. And for that the sync_reg validity seems wrong. So better keep the CAP?

Christian Borntraeger

2020-06-22 14:56 UTC #2 lore raw

Re: [PATCH v8 1/2] s390/setup: diag 318: refactor struct

In reply to Collin Walling

Quoted 10 lines · Collin Walling

The diag 318 struct introduced in include/asm/diag.h can be reused in KVM, so let's condense the version code fields in the diag318_info struct for easier usage and simplify it until we can determine how the data should be formatted.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Reviewed-by Cornelia Huck <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by Janosch Frank <[email protected]>

Series looks good to me. Can you respin the 2nd patch regarding the VSIE things and I can then apply it.

Quoted 37 lines

---
 arch/s390/include/asm/diag.h | 6 ++----
 arch/s390/kernel/setup.c     | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

arch/s390/include/asm/diag.h +2 -4

index 0036eab14391..ca8f85b53a90 100644
@@ -298,10 +298,8 @@ struct diag26c_mac_resp {
 union diag318_info {
 	unsigned long val;
 	struct {
-		unsigned int cpnc : 8;
-		unsigned int cpvc_linux : 24;
-		unsigned char cpvc_distro[3];
-		unsigned char zero;
+		unsigned long cpnc : 8;
+		unsigned long cpvc : 56;
 	};
 };

arch/s390/kernel/setup.c +1 -2

index 5853c9872dfe..878cacfc9c3e 100644
@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void)
 {
 	union diag318_info diag318_info = {
 		.cpnc = CPNC_LINUX,
-		.cpvc_linux = 0,
-		.cpvc_distro = {0},
+		.cpvc = 0,
 	};
 
 	if (!sclp.has_diag318)

Cornelia Huck

2020-06-22 15:03 UTC #15 lore raw

Re: [PATCH v8 2/2] s390/kvm: diagnose 0x318 sync and reset

In reply to Christian Borntraeger

Quoted 45 lines · On Mon, 22 Jun 2020 16:50:41 +0200 Christian Borntraeger…

Quoted 31 lines · Cornelia Huck
Quoted 25 lines · On Thu, 18 Jun 2020 18:22:22 -0400 Collin Walling <[email protected]> wrote:
DIAGNOSE 0x318 (diag318) sets information regarding the environment the VM is running in (Linux, z/VM, etc) and is observed via firmware/service events.
This is a privileged s390x instruction that must be intercepted by SIE. Userspace handles the instruction as well as migration. Data is communicated via VCPU register synchronization.
The Control Program Name Code (CPNC) is stored in the SIE block. The CPNC along with the Control Program Version Code (CPVC) are stored in the kvm_vcpu_arch struct.
The CPNC is shadowed/unshadowed in VSIE.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
---
 arch/s390/include/asm/kvm_host.h |  4 +++-
 arch/s390/include/uapi/asm/kvm.h |  5 ++++-
 arch/s390/kvm/kvm-s390.c         | 11 ++++++++++-
 arch/s390/kvm/vsie.c             |  3 +++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 21 insertions(+), 3 deletions(-)
(...)
Quoted 12 lines
include/uapi/linux/kvm.h struct kvm_ppc_resize_hpt { · line 1034
Quoted 9 lines
include/uapi/linux/kvm.h +1 -0
index 4fdf30316582..35cdb4307904 100644
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_PPC_SECURE_GUEST 181
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
+#define KVM_CAP_S390_DIAG318 184  
Do we strictly need this new cap, or would checking against the sync regs capabilities be enough?

Quoted 7 lines

We could check the sync_regs valid field to decide about the sync. We do that for ETOKEN as well and QEMU also uses it in handle_diag_318.
I think what this is used for is actually to tell the QEMU CPU model if this is there. And for that the sync_reg validity seems wrong. So better keep the CAP?

Ok, makes sense.

Collin Walling

author

2020-06-22 15:37 UTC #3 lore raw

Re: [PATCH v8 1/2] s390/setup: diag 318: refactor struct

In reply to Christian Borntraeger

Quoted 7 lines · Christian Borntraeger

Quoted 5 lines · Collin Walling
The diag 318 struct introduced in include/asm/diag.h can be reused in KVM, so let's condense the version code fields in the diag318_info struct for easier usage and simplify it until we can determine how the data should be formatted.

Quoted 10 lines

Quoted 5 lines
Signed-off-by Collin Walling <[email protected]>
Reviewed-by David Hildenbrand <[email protected]>
Reviewed-by Cornelia Huck <[email protected]>
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by Janosch Frank <[email protected]>
Series looks good to me. Can you respin the 2nd patch regarding the VSIE things and I can then apply it.

Will do. Thanks.

Quoted 37 lines

---
 arch/s390/include/asm/diag.h | 6 ++----
 arch/s390/kernel/setup.c     | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

arch/s390/include/asm/diag.h +2 -4

index 0036eab14391..ca8f85b53a90 100644
@@ -298,10 +298,8 @@ struct diag26c_mac_resp {
 union diag318_info {
 	unsigned long val;
 	struct {
-		unsigned int cpnc : 8;
-		unsigned int cpvc_linux : 24;
-		unsigned char cpvc_distro[3];
-		unsigned char zero;
+		unsigned long cpnc : 8;
+		unsigned long cpvc : 56;
 	};
 };

arch/s390/kernel/setup.c +1 -2

index 5853c9872dfe..878cacfc9c3e 100644
@@ -1021,8 +1021,7 @@ static void __init setup_control_program_code(void)
 {
 	union diag318_info diag318_info = {
 		.cpnc = CPNC_LINUX,
-		.cpvc_linux = 0,
-		.cpvc_distro = {0},
+		.cpvc = 0,
 	};
 
 	if (!sclp.has_diag318)

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy