Use DIAG318 to set Control Program Name & Version Codes

PATCH Applied Collin Walling 5 participants 15 messages Jun 22, 2020 – Jun 23, 2020

Subsystems (3)

KVM s390 KVM s390

lore

Share & tools

lore

Collin Walling

author

2020-06-22 15:46 UTC #0 lore raw

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

Changelog:

• No longer unshadowing CPNC in VSIE

• 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 [removed in v9]
• 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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 7 files changed, 22 insertions(+), 9 deletions(-)

Signature · 1 line

2.26.2

Collin Walling

author

2020-06-22 15:46 UTC #1 lore raw

PATCH v9 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.26.2

Collin Walling

author

2020-06-22 15:46 UTC #2 lore raw

PATCH v9 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.

This data is reset on load normal and clear resets.

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

Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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 +1 -0

index 9e9056cebfcf..4f3cbf6003a9 100644
@@ -548,6 +548,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.26.2

Cornelia Huck

Acked-by on v9 2/2

2020-06-22 16:05 UTC #3 lore raw

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

Quoted 8 lines · On Mon, 22 Jun 2020 11:46:36 -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.

Quoted 5 lines

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.
This data is reset on load normal and clear resets.

Looks good to me AFAICS without access to the architecture.

Acked-by Cornelia Huck <[email protected]>

One small thing below.

Quoted 10 lines

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

Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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

Should we document this in Documentation/virt/kvm/api.rst?

(Documentation of KVM caps generally seems to be a bit of a hit-and-miss, though. But we could at least document the s390 ones :)

I also noticed that the new entries for the vcpu resets and pv do not seem to be in proper rst format. Maybe fix that and add the new doc in an add-on series?

Quoted 3 lines

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

Collin Walling

author

2020-06-22 16:13 UTC #4 lore raw

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

Quoted 48 lines · Cornelia Huck

Quoted 8 lines · On Mon, 22 Jun 2020 11:46:36 -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.
Quoted 5 lines
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.
This data is reset on load normal and clear resets.
Looks good to me AFAICS without access to the architecture.
Acked-by Cornelia Huck <[email protected]>
One small thing below.
Quoted 10 lines
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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
Should we document this in Documentation/virt/kvm/api.rst?

Quoted 7 lines

(Documentation of KVM caps generally seems to be a bit of a hit-and-miss, though. But we could at least document the s390 ones :)
I also noticed that the new entries for the vcpu resets and pv do not seem to be in proper rst format. Maybe fix that and add the new doc in an add-on series?

Sure thing. I'll fix up the rst and add docs for the new DIAG cap.

Quoted 4 lines

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

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

Collin Walling

author

2020-06-22 16:23 UTC #5 lore raw

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

Quoted 52 lines · Collin Walling

Quoted 51 lines · Cornelia Huck
Quoted 8 lines · On Mon, 22 Jun 2020 11:46:36 -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.
Quoted 5 lines
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.
This data is reset on load normal and clear resets.
Looks good to me AFAICS without access to the architecture.
Acked-by Cornelia Huck <[email protected]>
One small thing below.
Quoted 10 lines
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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
Should we document this in Documentation/virt/kvm/api.rst?
(Documentation of KVM caps generally seems to be a bit of a hit-and-miss, though. But we could at least document the s390 ones :)

Quoted 13 lines

Quoted 4 lines
I also noticed that the new entries for the vcpu resets and pv do not seem to be in proper rst format. Maybe fix that and add the new doc in an add-on series?
Sure thing. I'll fix up the rst and add docs for the new DIAG cap.
Quoted 4 lines
Quoted 3 lines
include/uapi/linux/kvm.h +0 -0
 #ifdef KVM_CAP_IRQ_ROUTING
 

Mind if I get some early feedback for the first run? How does this sound:

8.24 KVM_CAP_S390_DIAG318
-------------------------

:Architecture: s390

This capability allows for information regarding the control program that may be observed via system/firmware service events. The availability of this capability indicates that KVM handling of the register synchronization, reset, and VSIE shadowing of the DIAGNOSE 0x318 related information is present.

The information associated with the instruction is an 8-byte value consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte Control Program Version Code (CPVC). The CPNC determines what environment the control program is running in (e.g. Linux, z/VM...), and the CPVC is used for extraneous information specific to OS (e.g. Linux version, Linux distribution...)

The CPNC must be stored in the SIE block for the CPU that executes the diag instruction, which is communicated from userspace to KVM via register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are stored together in the kvm_vcpu_arch struct.

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

Cornelia Huck

2020-06-22 16:35 UTC #6 lore raw

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

Quoted 20 lines · On Mon, 22 Jun 2020 12:23:45 -0400 Collin Walling <[email protected]> wrote:

Mind if I get some early feedback for the first run? How does this sound:
8.24 KVM_CAP_S390_DIAG318
-------------------------
:Architecture: s390
This capability allows for information regarding the control program that may be observed via system/firmware service events. The availability of this capability indicates that KVM handling of the register synchronization, reset, and VSIE shadowing of the DIAGNOSE 0x318 related information is present.
The information associated with the instruction is an 8-byte value consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte Control Program Version Code (CPVC). The CPNC determines what environment the control program is running in (e.g. Linux, z/VM...), and the CPVC is used for extraneous information specific to OS (e.g. Linux version, Linux distribution...)

Quoted 4 lines

The CPNC must be stored in the SIE block for the CPU that executes the diag instruction, which is communicated from userspace to KVM via register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are stored together in the kvm_vcpu_arch struct.

Hm... what about replacing that last paragraph with

"If this capability is available, the CPNC and CPVC are available for synchronization between KVM and userspace via the sync regs mechanism (KVM_SYNC_DIAG318)."

Collin Walling

author

2020-06-22 16:45 UTC #7 lore raw

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

Quoted 28 lines · Cornelia Huck

Quoted 20 lines · On Mon, 22 Jun 2020 12:23:45 -0400 Collin Walling <[email protected]> wrote:
Mind if I get some early feedback for the first run? How does this sound:
8.24 KVM_CAP_S390_DIAG318
-------------------------
:Architecture: s390
This capability allows for information regarding the control program that may be observed via system/firmware service events. The availability of this capability indicates that KVM handling of the register synchronization, reset, and VSIE shadowing of the DIAGNOSE 0x318 related information is present.
The information associated with the instruction is an 8-byte value consisting of a one-byte Control Program Name Code (CPNC), and a 7-byte Control Program Version Code (CPVC). The CPNC determines what environment the control program is running in (e.g. Linux, z/VM...), and the CPVC is used for extraneous information specific to OS (e.g. Linux version, Linux distribution...)
Quoted 4 lines
The CPNC must be stored in the SIE block for the CPU that executes the diag instruction, which is communicated from userspace to KVM via register synchronization using the KVM_SYNC_DIAG318 flag. Both codes are stored together in the kvm_vcpu_arch struct.

Quoted 8 lines

Hm... what about replacing that last paragraph with
"If this capability is available, the CPNC and CPVC are available for synchronization between KVM and userspace via the sync regs mechanism (KVM_SYNC_DIAG318)."
?

I like it!

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy

David Hildenbrand

Reviewed-by on v9 2/2

2020-06-23 07:12 UTC #8 lore raw

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

In reply to Collin Walling

Quoted 8 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.

Quoted 8 lines

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.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[email protected]>

LGTM

Reviewed-by David Hildenbrand <[email protected]>

Signature · 3 lines

Thanks,

David / dhildenb

Thomas Huth

2020-06-23 08:42 UTC #9 lore raw

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

In reply to Collin Walling

Quoted 66 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.

This data is reset on load normal and clear resets.

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

Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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 +3 -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

arch/s390/include/uapi/asm/kvm.h struct kvm_sync_regs { · line 262

Quoted 7 lines

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

@@ -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) */

It's been a while since I touched kvm_sync_regs the last time ... but can your really extend this structure right in the middle without breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I think you rather have to add this add the end or e.g. put it into the padding2 region or something like that...? Or do I miss something?

Thomas

Christian Borntraeger

2020-06-23 08:46 UTC #10 lore raw

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

Quoted 64 lines · Thomas Huth

Quoted 63 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.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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 +3 -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)
 

Quoted 16 lines

arch/s390/include/uapi/asm/kvm.h struct kvm_sync_regs { · line 262
Quoted 10 lines
arch/s390/include/uapi/asm/kvm.h +1 -0
 /* 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) */
It's been a while since I touched kvm_sync_regs the last time ... but can your really extend this structure right in the middle without breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I think you rather have to add this add the end or e.g. put it into the padding2 region or something like that...? Or do I miss something?

Argh. You are right. It should go to the end and not in the middle. Will fixup.

Christian Borntraeger

2020-06-23 08:47 UTC #11 lore raw

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

Quoted 84 lines · Christian Borntraeger

Quoted 64 lines · Thomas Huth
Quoted 63 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.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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 +3 -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)
 
Quoted 16 lines
arch/s390/include/uapi/asm/kvm.h struct kvm_sync_regs { · line 262
Quoted 10 lines
arch/s390/include/uapi/asm/kvm.h +1 -0
 /* 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) */
It's been a while since I touched kvm_sync_regs the last time ... but can your really extend this structure right in the middle without breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I think you rather have to add this add the end or e.g. put it into the padding2 region or something like that...? Or do I miss something?

Quoted 2 lines

Argh. You are right. It should go to the end and not in the middle. Will fixup.

Something like this on top.

arch/s390/include/uapi/asm/kvm.h +2 -2

index 2ae1b660086c..7a6b14874d65 100644
@@ -256,7 +256,6 @@ 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) */

@@ -267,7 +266,8 @@ struct kvm_sync_regs {
        __u8 reserved2 : 7;
        __u8 padding1[51];      /* riccb needs to be 64byte aligned */
        __u8 riccb[64];         /* runtime instrumentation controls block */
-       __u8 padding2[192];     /* sdnx needs to be 256byte aligned */
+       __u64 diag318;          /* diagnose 0x318 info */
+       __u8 padding2[184];     /* sdnx needs to be 256byte aligned */
        union {
                __u8 sdnx[SDNXL];  /* state description annex */
                struct {

Thomas Huth

2020-06-23 08:58 UTC #12 lore raw

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

Quoted 105 lines · Christian Borntraeger

Quoted 84 lines · Christian Borntraeger
Quoted 64 lines · Thomas Huth
Quoted 63 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.
This data is reset on load normal and clear resets.
Signed-off-by Collin Walling <[email protected]>
Reviewed-by Janosch Frank <[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             |  1 +
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 19 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 +3 -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)
 
Quoted 16 lines
arch/s390/include/uapi/asm/kvm.h struct kvm_sync_regs { · line 262
Quoted 10 lines
arch/s390/include/uapi/asm/kvm.h +1 -0
 /* 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) */
It's been a while since I touched kvm_sync_regs the last time ... but can your really extend this structure right in the middle without breaking older user spaces (ie. QEMUs) ? This is a uapi header ... so I think you rather have to add this add the end or e.g. put it into the padding2 region or something like that...? Or do I miss something?
Quoted 2 lines
Argh. You are right. It should go to the end and not in the middle. Will fixup.
Something like this on top.
arch/s390/include/uapi/asm/kvm.h +0 -1
index 2ae1b660086c..7a6b14874d65 100644
@@ -256,7 +256,6 @@ 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/include/uapi/asm/kvm.h struct kvm_sync_regs { · line 273

Quoted 9 lines

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

@@ -267,7 +266,8 @@ struct kvm_sync_regs {
        __u8 reserved2 : 7;
        __u8 padding1[51];      /* riccb needs to be 64byte aligned */
        __u8 riccb[64];         /* runtime instrumentation controls block */
-       __u8 padding2[192];     /* sdnx needs to be 256byte aligned */
+       __u64 diag318;          /* diagnose 0x318 info */
+       __u8 padding2[184];     /* sdnx needs to be 256byte aligned */
        union {
                __u8 sdnx[SDNXL];  /* state description annex */
                struct {

Ack!

Thomas

Christian Borntraeger

Applied

2020-06-23 07:13 UTC #13 lore raw

In reply to Collin Walling

Quoted 6 lines · Collin Walling

Changelog:
v9
• No longer unshadowing CPNC in VSIE

applied.

Collin Walling

author

2020-06-23 14:38 UTC #14 lore raw

Quoted 11 lines · Christian Borntraeger

Quoted 6 lines · Collin Walling
Changelog:
v9
• No longer unshadowing CPNC in VSIE
applied.

Thanks!

Signature · 4 lines

Regards,
Collin

Stay safe and stay healthy