Skip to contentSkip to conversation

Use DIAG318 to set Control Program Name & Version Codes

PATCH Superseded Collin Walling 3 participants 9 messages May 15, 2020 – Jun 15, 2020

Subsystems (4)

v7 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

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

Changelog:

v7

• 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
- introduced a new patch to s/diag318/diag_318 and s/byte_134/fac134 to keep things consistent with the rest of the code

v6

• 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

v5

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

v4

• 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

v3

• 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
• 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 SIE blocks, and the CPNC & CPVC pair is stored in the kvm_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 (3):
s390/setup: diag 318: refactor struct
s390: keep diag 318 variables consistent with the rest s390/kvm: diagnose 0x318 get/set handling

 Documentation/virt/kvm/devices/vm.rst | 21 +++++++
 arch/s390/include/asm/diag.h          |  8 +--
 arch/s390/include/asm/kvm_host.h      |  5 +-
 arch/s390/include/asm/sclp.h          |  2 +-
 arch/s390/include/uapi/asm/kvm.h      |  4 ++
 arch/s390/kernel/setup.c              |  9 ++-
 arch/s390/kvm/kvm-s390.c              | 82 +++++++++++++++++++++++++++
 arch/s390/kvm/vsie.c                  |  2 +
 drivers/s390/char/sclp.h              |  2 +-
 drivers/s390/char/sclp_early.c        |  2 +-
 10 files changed, 123 insertions(+), 14 deletions(-)
Signature · 1 line
2.21.3
#1 lore raw

PATCH v7 1/3 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.

---
 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 36445dd40fdb..1aaaf11acc6b 100644
@@ -1028,8 +1028,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
#2 lore raw

PATCH v7 2/3 s390: keep diag 318 variables consistent with the rest

In reply to Collin Walling

Rename diag318 to diag_318 and byte_134 to fac134 in order to keep naming schemes consistent with other diags and the read info struct and make grepping easier.

---
 arch/s390/include/asm/diag.h   | 2 +-
 arch/s390/include/asm/sclp.h   | 2 +-
 arch/s390/kernel/setup.c       | 6 +++---
 drivers/s390/char/sclp.h       | 2 +-
 drivers/s390/char/sclp_early.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)
arch/s390/include/asm/diag.h +1 -1
index ca8f85b53a90..19da822e494c 100644
@@ -295,7 +295,7 @@ struct diag26c_mac_resp {
 } __aligned(8);
 
 #define CPNC_LINUX		0x4
-union diag318_info {
+union diag_318_info {
 	unsigned long val;
 	struct {
 		unsigned long cpnc : 8;
arch/s390/include/asm/sclp.h +1 -1
index c563f8368b19..a45967cfc1ae 100644
@@ -78,7 +78,7 @@ struct sclp_info {
 	unsigned char has_skey : 1;
 	unsigned char has_kss : 1;
 	unsigned char has_gisaf : 1;
-	unsigned char has_diag318 : 1;
+	unsigned char has_diag_318 : 1;
 	unsigned char has_sipl : 1;
 	unsigned char has_dirq : 1;
 	unsigned int ibc;
arch/s390/kernel/setup.c +3 -3
index 1aaaf11acc6b..8925a1ac14c9 100644
@@ -1026,16 +1026,16 @@ static void __init setup_task_size(void)
  */
 static void __init setup_control_program_code(void)
 {
-	union diag318_info diag318_info = {
+	union diag_318_info diag_318_info = {
 		.cpnc = CPNC_LINUX,
 		.cpvc = 0,
 	};
 
-	if (!sclp.has_diag318)
+	if (!sclp.has_diag_318)
 		return;
 
 	diag_stat_inc(DIAG_STAT_X318);
-	asm volatile("diag %0,0,0x318\n" : : "d" (diag318_info.val));
+	asm volatile("diag %0,0,0x318\n" : : "d" (diag_318_info.val));
 }
 
 /*
drivers/s390/char/sclp.h +1 -1
index 196333013e54..d6a91f3b8e2b 100644
@@ -196,7 +196,7 @@ struct read_info_sccb {
 	u8	_pad_122[124 - 122];	/* 122-123 */
 	u32	hmfai;			/* 124-127 */
 	u8	_pad_128[134 - 128];	/* 128-133 */
-	u8	byte_134;			/* 134 */
+	u8	fac134;			/* 134 */
 	u8	cpudirq;		/* 135 */
 	u16	cbl;			/* 136-137 */
 	u8	_pad_138[4096 - 138];	/* 138-4095 */
drivers/s390/char/sclp_early.c +1 -1
index cc5e84b80c69..388d24c65e74 100644
@@ -46,7 +46,7 @@ static void __init sclp_early_facilities_detect(struct read_info_sccb *sccb)
 	if (sccb->fac91 & 0x40)
 		S390_lowcore.machine_flags |= MACHINE_FLAG_TLB_GUEST;
 	if (sccb->cpuoff > 134)
-		sclp.has_diag318 = !!(sccb->byte_134 & 0x80);
+		sclp.has_diag_318 = !!(sccb->fac134 & 0x80);
 	sclp.rnmax = sccb->rnmax ? sccb->rnmax : sccb->rnmax2;
 	sclp.rzm = sccb->rnsize ? sccb->rnsize : sccb->rnsize2;
 	sclp.rzm <<= 20;
Signature · 1 line
2.21.3
#3 lore raw

Re: [PATCH v7 2/3] s390: keep diag 318 variables consistent with the rest

Quoted 18 lines · Collin Walling

Rename diag318 to diag_318 and byte_134 to fac134 in order to keep naming schemes consistent with other diags and the read info struct and make grepping easier.

---
 arch/s390/include/asm/diag.h   | 2 +-
 arch/s390/include/asm/sclp.h   | 2 +-
 arch/s390/kernel/setup.c       | 6 +++---
 drivers/s390/char/sclp.h       | 2 +-
 drivers/s390/char/sclp_early.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)
arch/s390/include/asm/diag.h +0 -0
index ca8f85b53a90..19da822e494c 100644

arch/s390/include/asm/diag.h struct diag26c_mac_resp { · line 298

Quoted 5 lines
arch/s390/include/asm/diag.h +1 -1
@@ -295,7 +295,7 @@ struct diag26c_mac_resp {
 } __aligned(8);
 
 #define CPNC_LINUX		0x4
-union diag318_info {
+union diag_318_info {
$ grep -r diag.*info arch/s390/include/asm/diag.h
struct diag204_info_blk_hdr {
struct diag204_x_info_blk_hdr {
struct diag204_cpu_info {
struct diag204_x_cpu_info {
	struct diag204_x_cpu_info cpus[];
union diag318_info {

... none of these seem to use an underscore between the "diag" and the number ... so this seems unnecessary to me ... or what do I miss?

Thomas

#4 lore raw

Re: [PATCH v7 2/3] s390: keep diag 318 variables consistent with the rest

Quoted 33 lines · Thomas Huth
Quoted 18 lines · Collin Walling

Rename diag318 to diag_318 and byte_134 to fac134 in order to keep naming schemes consistent with other diags and the read info struct and make grepping easier.

---
 arch/s390/include/asm/diag.h   | 2 +-
 arch/s390/include/asm/sclp.h   | 2 +-
 arch/s390/kernel/setup.c       | 6 +++---
 drivers/s390/char/sclp.h       | 2 +-
 drivers/s390/char/sclp_early.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)
arch/s390/include/asm/diag.h +0 -0
index ca8f85b53a90..19da822e494c 100644

arch/s390/include/asm/diag.h struct diag26c_mac_resp { · line 298

Quoted 5 lines
arch/s390/include/asm/diag.h +1 -1
@@ -295,7 +295,7 @@ struct diag26c_mac_resp {
 } __aligned(8);
 
 #define CPNC_LINUX		0x4
-union diag318_info {
+union diag_318_info {
$ grep -r diag.*info arch/s390/include/asm/diag.h
struct diag204_info_blk_hdr {
struct diag204_x_info_blk_hdr {
struct diag204_cpu_info {
struct diag204_x_cpu_info {
	struct diag204_x_cpu_info cpus[];
union diag318_info {
Quoted 5 lines

... none of these seem to use an underscore between the "diag" and the number ... so this seems unnecessary to me ... or what do I miss?

Thomas

I could have sworn I saw more cases with the underscore. I think I was honing-in on a few cases in QEMU for whatever reason.

Let's just forget this patch was posted :)

Signature · 5 lines
--
Regards,
Collin

Stay safe and stay healthy
#5 lore raw

PATCH v7 3/3 s390/kvm: diagnose 0x318 get/set handling

In reply to Collin Walling

DIAGNOSE 0x318 (diag 318) 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 handling is required, so let's introduce some functions to communicate between userspace and KVM via ioctls. These will be used to get/set the diag 318 related information.

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_arch struct.

---
 Documentation/virt/kvm/devices/vm.rst | 21 +++++++
 arch/s390/include/asm/kvm_host.h      |  5 +-
 arch/s390/include/uapi/asm/kvm.h      |  4 ++
 arch/s390/kvm/kvm-s390.c              | 82 +++++++++++++++++++++++++++
 arch/s390/kvm/vsie.c                  |  2 +
 5 files changed, 113 insertions(+), 1 deletion(-)
Documentation/virt/kvm/devices/vm.rst +21 -0
index 0aa5b1cfd700..52cc906dd7bd 100644
@@ -314,3 +314,24 @@ Allows userspace to query the status of migration mode.
 	     if it is enabled
 :Returns:   -EFAULT if the given address is not accessible from kernel space;
 	    0 in case of success.
+
+6. GROUP: KVM_S390_VM_MISC
+==========================
+
+:Architectures: s390
+
+6.1. KVM_S390_VM_MISC_DIAG_318 (r/w)
+-----------------------------------
+
+Allows userspace to retrieve and set the DIAGNOSE 0x318 information,
+which consists of a 1-byte "Control Program Name Code" and a 7-byte
+"Control Program Version Code" (a 64 bit value all in all). This
+information is set by the guest (usually during IPL). This interface is
+intended to allow retrieving and setting it during migration; while no
+real harm is done if the information is changed outside of migration,
+it is strongly discouraged.
+
+:Parameters: address of a buffer in user space (u64), where the
+	     information is read from or stored into
+:Returns:    -EFAULT if the given address is not accessible from kernel space;
+	     0 in case of success
arch/s390/include/asm/kvm_host.h +4 -1
index d6bcd34f3ec3..c03222051043 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 */
@@ -938,6 +939,7 @@ struct kvm_arch{
 	int user_sigp;
 	int user_stsi;
 	int user_instr0;
+	int use_diag_318;
 	struct s390_io_adapter *adapters[MAX_S390_IO_ADAPTERS];
 	wait_queue_head_t ipte_wq;
 	int ipte_lock_count;
@@ -956,6 +958,7 @@ struct kvm_arch{
 	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
 	struct kvm_s390_gisa_interrupt gisa_int;
 	struct kvm_s390_pv pv;
+	union diag_318_info diag_318_info;
 };
 
 #define KVM_HVA_ERR_BAD		(-1UL)
arch/s390/include/uapi/asm/kvm.h +4 -0
index 436ec7636927..2ce8e6f72206 100644
@@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
 #define KVM_S390_VM_CRYPTO		2
 #define KVM_S390_VM_CPU_MODEL		3
 #define KVM_S390_VM_MIGRATION		4
+#define KVM_S390_VM_MISC		5
 
 /* kvm attributes for mem_ctrl */
 #define KVM_S390_VM_MEM_ENABLE_CMMA	0
@@ -171,6 +172,9 @@ struct kvm_s390_vm_cpu_subfunc {
 #define KVM_S390_VM_MIGRATION_START	1
 #define KVM_S390_VM_MIGRATION_STATUS	2
 
+/* kvm attributes for KVM_S390_VM_MISC */
+#define KVM_S390_VM_MISC_DIAG_318	0
+
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
arch/s390/kvm/kvm-s390.c +82 -0

112 lines not on this page · raw message

arch/s390/kvm/vsie.c +2 -0
index 4f6c22d72072..a6820267e661 100644
@@ -548,6 +548,8 @@ 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;
+	if (sclp.has_diag_318)
+		scb_s->cpnc = scb_o->cpnc;
 
 	prepare_ibc(vcpu, vsie_page);
 	rc = shadow_crycb(vcpu, vsie_page);
Signature · 1 line
2.21.3
#6 lore raw

Re: [PATCH v7 3/3] s390/kvm: diagnose 0x318 get/set handling

Quoted 41 lines · Collin Walling

DIAGNOSE 0x318 (diag 318) 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 handling is required, so let's introduce some functions to communicate between userspace and KVM via ioctls. These will be used to get/set the diag 318 related information.

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_arch struct.

---
 Documentation/virt/kvm/devices/vm.rst | 21 +++++++
 arch/s390/include/asm/kvm_host.h      |  5 +-
 arch/s390/include/uapi/asm/kvm.h      |  4 ++
 arch/s390/kvm/kvm-s390.c              | 82 +++++++++++++++++++++++++++
 arch/s390/kvm/vsie.c                  |  2 +
 5 files changed, 113 insertions(+), 1 deletion(-)
Documentation/virt/kvm/devices/vm.rst +11 -0
index 0aa5b1cfd700..52cc906dd7bd 100644
@@ -314,3 +314,24 @@ Allows userspace to query the status of migration mode.
 	     if it is enabled
 :Returns:   -EFAULT if the given address is not accessible from kernel space;
 	    0 in case of success.
+
+6. GROUP: KVM_S390_VM_MISC
+==========================
+
+:Architectures: s390
+
+6.1. KVM_S390_VM_MISC_DIAG_318 (r/w)
+-----------------------------------
+
+Allows userspace to retrieve and set the DIAGNOSE 0x318 information,
+which consists of a 1-byte "Control Program Name Code" and a 7-byte

Documentation/virt/kvm/devices/vm.rst Allows userspace to query the status of migration mode. · line 337

Quoted 10 lines
Documentation/virt/kvm/devices/vm.rst +10 -0
+"Control Program Version Code" (a 64 bit value all in all). This
+information is set by the guest (usually during IPL). This interface is
+intended to allow retrieving and setting it during migration; while no
+real harm is done if the information is changed outside of migration,
+it is strongly discouraged.
+
+:Parameters: address of a buffer in user space (u64), where the
+	     information is read from or stored into
+:Returns:    -EFAULT if the given address is not accessible from kernel space;
+	     0 in case of success

An alternative would be a new sync_reg value + KVM capability.

#7 lore raw

Re: [PATCH v7 3/3] s390/kvm: diagnose 0x318 get/set handling

Quoted 49 lines · Christian Borntraeger
Quoted 46 lines · Collin Walling

DIAGNOSE 0x318 (diag 318) 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 handling is required, so let's introduce some functions to communicate between userspace and KVM via ioctls. These will be used to get/set the diag 318 related information.

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_arch struct.

---
 Documentation/virt/kvm/devices/vm.rst | 21 +++++++
 arch/s390/include/asm/kvm_host.h      |  5 +-
 arch/s390/include/uapi/asm/kvm.h      |  4 ++
 arch/s390/kvm/kvm-s390.c              | 82 +++++++++++++++++++++++++++
 arch/s390/kvm/vsie.c                  |  2 +
 5 files changed, 113 insertions(+), 1 deletion(-)
Documentation/virt/kvm/devices/vm.rst +16 -0
index 0aa5b1cfd700..52cc906dd7bd 100644
@@ -314,3 +314,24 @@ Allows userspace to query the status of migration mode.
 	     if it is enabled
 :Returns:   -EFAULT if the given address is not accessible from kernel space;
 	    0 in case of success.
+
+6. GROUP: KVM_S390_VM_MISC
+==========================
+
+:Architectures: s390
+
+6.1. KVM_S390_VM_MISC_DIAG_318 (r/w)
+-----------------------------------
+
+Allows userspace to retrieve and set the DIAGNOSE 0x318 information,
+which consists of a 1-byte "Control Program Name Code" and a 7-byte
+"Control Program Version Code" (a 64 bit value all in all). This
+information is set by the guest (usually during IPL). This interface is
+intended to allow retrieving and setting it during migration; while no
+real harm is done if the information is changed outside of migration,
+it is strongly discouraged.
Quoted 10 lines

Documentation/virt/kvm/devices/vm.rst Allows userspace to query the status of migration mode. · line 337

Quoted 5 lines
Documentation/virt/kvm/devices/vm.rst +5 -0
+
+:Parameters: address of a buffer in user space (u64), where the
+	     information is read from or stored into
+:Returns:    -EFAULT if the given address is not accessible from kernel space;
+	     0 in case of success

An alternative would be a new sync_reg value + KVM capability.

I did some investigation into this (new grounds for me)

sync_reg would handle the migration part, and then we'd just need the ioctl so the diag instruction handler can set the data. Does that sound right?

Signature · 4 lines
Regards,
Collin

Stay safe and stay healthy
#8 lore raw

Re: [PATCH v7 3/3] s390/kvm: diagnose 0x318 get/set handling

Quoted 61 lines · Collin Walling
Quoted 49 lines · Christian Borntraeger
Quoted 46 lines · Collin Walling

DIAGNOSE 0x318 (diag 318) 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 handling is required, so let's introduce some functions to communicate between userspace and KVM via ioctls. These will be used to get/set the diag 318 related information.

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_arch struct.

---
 Documentation/virt/kvm/devices/vm.rst | 21 +++++++
 arch/s390/include/asm/kvm_host.h      |  5 +-
 arch/s390/include/uapi/asm/kvm.h      |  4 ++
 arch/s390/kvm/kvm-s390.c              | 82 +++++++++++++++++++++++++++
 arch/s390/kvm/vsie.c                  |  2 +
 5 files changed, 113 insertions(+), 1 deletion(-)
Documentation/virt/kvm/devices/vm.rst +16 -0
index 0aa5b1cfd700..52cc906dd7bd 100644
@@ -314,3 +314,24 @@ Allows userspace to query the status of migration mode.
 	     if it is enabled
 :Returns:   -EFAULT if the given address is not accessible from kernel space;
 	    0 in case of success.
+
+6. GROUP: KVM_S390_VM_MISC
+==========================
+
+:Architectures: s390
+
+6.1. KVM_S390_VM_MISC_DIAG_318 (r/w)
+-----------------------------------
+
+Allows userspace to retrieve and set the DIAGNOSE 0x318 information,
+which consists of a 1-byte "Control Program Name Code" and a 7-byte
+"Control Program Version Code" (a 64 bit value all in all). This
+information is set by the guest (usually during IPL). This interface is
+intended to allow retrieving and setting it during migration; while no
+real harm is done if the information is changed outside of migration,
+it is strongly discouraged.
Quoted 10 lines

Documentation/virt/kvm/devices/vm.rst Allows userspace to query the status of migration mode. · line 337

Quoted 5 lines
Documentation/virt/kvm/devices/vm.rst +5 -0
+
+:Parameters: address of a buffer in user space (u64), where the
+	     information is read from or stored into
+:Returns:    -EFAULT if the given address is not accessible from kernel space;
+	     0 in case of success

An alternative would be a new sync_reg value + KVM capability.

Quoted 5 lines

I did some investigation into this (new grounds for me)

sync_reg would handle the migration part, and then we'd just need the ioctl so the diag instruction handler can set the data. Does that sound right?

We would not even need the ioctl. We could just export/import this all the time. See the sync_regs and store_regs functions.