kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] kvm: s390: Add PV dump support
@ 2022-02-23  9:19 Janosch Frank
  2022-02-23  9:19 ` [PATCH 1/9] s390x: Add SE hdr query information Janosch Frank
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:19 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Sometimes dumping inside of a VM fails, is unavailable or doesn't
yield the required data. For these occasions we dump the VM from the
outside, writing memory and cpu data to a file.

Up to now PV guests only supported dumping from the inside of the
guest through dumpers like KDUMP. A PV guest can be dumped from the
hypervisor but the data will be stale and / or encrypted.

To get the actual state of the PV VM we need the help of the
Ultravisor who safeguards the VM state. New UV calls have been added
to initialize the dump, dump storage state data, dump cpu data and
complete the dump process.


Notes:
I'm still pondering adding a lock to the dumping PV struct member to
make absolutely sure that we can't run into the validity even if
userspace tries to achieve getting it.

I chose not to document the dump data provided by the Ultravisor since
KVM doesn't interprete it in any way. We're currently searching for a
location and enough cycles to make it available to all.

Janosch Frank (9):
  s390x: Add SE hdr query information
  s390: uv: Add dump fields to query
  KVM: s390: pv: Add query interface
  KVM: s390: pv: Add dump support definitions
  KVM: s390: pv: Add query dump information
  kvm: s390: Add configuration dump functionality
  kvm: s390: Add CPU dump functionality
  Documentation: virt: Protected virtual machine dumps
  Documentation/virt/kvm/api.rst: Add protvirt dump/info api
    descriptions

 Documentation/virt/kvm/api.rst          | 129 +++++++++++-
 Documentation/virt/kvm/index.rst        |   1 +
 Documentation/virt/kvm/s390-pv-dump.rst |  60 ++++++
 arch/s390/boot/uv.c                     |   4 +
 arch/s390/include/asm/kvm_host.h        |   1 +
 arch/s390/include/asm/uv.h              |  45 +++-
 arch/s390/kernel/uv.c                   |  53 +++++
 arch/s390/kvm/kvm-s390.c                | 267 ++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.h                |   3 +
 arch/s390/kvm/pv.c                      | 131 ++++++++++++
 include/uapi/linux/kvm.h                |  53 +++++
 11 files changed, 744 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/virt/kvm/s390-pv-dump.rst

-- 
2.32.0


^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH 1/9] s390x: Add SE hdr query information
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
@ 2022-02-23  9:19 ` Janosch Frank
  2022-03-01 17:22   ` Claudio Imbrenda
  2022-02-23  9:20 ` [PATCH 2/9] s390: uv: Add dump fields to query Janosch Frank
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:19 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

We have information about the supported se header version and pcf bits
so let's expose it via the sysfs files.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/boot/uv.c        |  2 ++
 arch/s390/include/asm/uv.h |  7 ++++++-
 arch/s390/kernel/uv.c      | 20 ++++++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
index e6be155ab2e5..b100b57cf15d 100644
--- a/arch/s390/boot/uv.c
+++ b/arch/s390/boot/uv.c
@@ -41,6 +41,8 @@ void uv_query_info(void)
 		uv_info.max_num_sec_conf = uvcb.max_num_sec_conf;
 		uv_info.max_guest_cpu_id = uvcb.max_guest_cpu_id;
 		uv_info.uv_feature_indications = uvcb.uv_feature_indications;
+		uv_info.supp_se_hdr_ver = uvcb.supp_se_hdr_versions;
+		uv_info.supp_se_hdr_pcf = uvcb.supp_se_hdr_pcf;
 	}
 
 #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 86218382d29c..7d6c78b61bf2 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -107,7 +107,10 @@ struct uv_cb_qui {
 	u8  reserved88[158 - 136];		/* 0x0088 */
 	u16 max_guest_cpu_id;			/* 0x009e */
 	u64 uv_feature_indications;		/* 0x00a0 */
-	u8  reserveda8[200 - 168];		/* 0x00a8 */
+	u64 reserveda8;				/* 0x00a8 */
+	u64 supp_se_hdr_versions;		/* 0x00b0 */
+	u64 supp_se_hdr_pcf;			/* 0x00b8 */
+	u64 reservedc0;				/* 0x00c0 */
 } __packed __aligned(8);
 
 /* Initialize Ultravisor */
@@ -285,6 +288,8 @@ struct uv_info {
 	unsigned int max_num_sec_conf;
 	unsigned short max_guest_cpu_id;
 	unsigned long uv_feature_indications;
+	unsigned long supp_se_hdr_ver;
+	unsigned long supp_se_hdr_pcf;
 };
 
 extern struct uv_info uv_info;
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index a5425075dd25..852840384e75 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -392,6 +392,24 @@ static ssize_t uv_query_facilities(struct kobject *kobj,
 static struct kobj_attribute uv_query_facilities_attr =
 	__ATTR(facilities, 0444, uv_query_facilities, NULL);
 
+static ssize_t uv_query_supp_se_hdr_ver(struct kobject *kobj,
+					struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%lx\n", uv_info.supp_se_hdr_ver);
+}
+
+static struct kobj_attribute uv_query_supp_se_hdr_ver_attr =
+	__ATTR(supp_se_hdr_ver, 0444, uv_query_supp_se_hdr_ver, NULL);
+
+static ssize_t uv_query_supp_se_hdr_pcf(struct kobject *kobj,
+					struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%lx\n", uv_info.supp_se_hdr_pcf);
+}
+
+static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
+	__ATTR(supp_se_hdr_pcf, 0444, uv_query_supp_se_hdr_pcf, NULL);
+
 static ssize_t uv_query_feature_indications(struct kobject *kobj,
 					    struct kobj_attribute *attr, char *buf)
 {
@@ -437,6 +455,8 @@ static struct attribute *uv_query_attrs[] = {
 	&uv_query_max_guest_cpus_attr.attr,
 	&uv_query_max_guest_vms_attr.attr,
 	&uv_query_max_guest_addr_attr.attr,
+	&uv_query_supp_se_hdr_ver_attr.attr,
+	&uv_query_supp_se_hdr_pcf_attr.attr,
 	NULL,
 };
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 2/9] s390: uv: Add dump fields to query
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
  2022-02-23  9:19 ` [PATCH 1/9] s390x: Add SE hdr query information Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-03-01 17:24   ` Claudio Imbrenda
  2022-02-23  9:20 ` [PATCH 3/9] KVM: s390: pv: Add query interface Janosch Frank
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

The new dump feature requires us to know how much memory is needed for
the "dump storage state" and "dump finalize" ultravisor call. These
values are reported via the UV query call.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/boot/uv.c        |  2 ++
 arch/s390/include/asm/uv.h |  5 +++++
 arch/s390/kernel/uv.c      | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
index b100b57cf15d..67c737c1e580 100644
--- a/arch/s390/boot/uv.c
+++ b/arch/s390/boot/uv.c
@@ -43,6 +43,8 @@ void uv_query_info(void)
 		uv_info.uv_feature_indications = uvcb.uv_feature_indications;
 		uv_info.supp_se_hdr_ver = uvcb.supp_se_hdr_versions;
 		uv_info.supp_se_hdr_pcf = uvcb.supp_se_hdr_pcf;
+		uv_info.conf_dump_storage_state_len = uvcb.conf_dump_storage_state_len;
+		uv_info.conf_dump_finalize_len = uvcb.conf_dump_finalize_len;
 	}
 
 #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 7d6c78b61bf2..b79e516d4424 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -111,6 +111,9 @@ struct uv_cb_qui {
 	u64 supp_se_hdr_versions;		/* 0x00b0 */
 	u64 supp_se_hdr_pcf;			/* 0x00b8 */
 	u64 reservedc0;				/* 0x00c0 */
+	u64 conf_dump_storage_state_len;	/* 0x00c8 */
+	u64 conf_dump_finalize_len;		/* 0x00d0 */
+	u8  reservedd8[256 - 216];		/* 0x00d8 */
 } __packed __aligned(8);
 
 /* Initialize Ultravisor */
@@ -290,6 +293,8 @@ struct uv_info {
 	unsigned long uv_feature_indications;
 	unsigned long supp_se_hdr_ver;
 	unsigned long supp_se_hdr_pcf;
+	unsigned long conf_dump_storage_state_len;
+	unsigned long conf_dump_finalize_len;
 };
 
 extern struct uv_info uv_info;
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index 852840384e75..84fe33b6af4d 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -410,6 +410,36 @@ static ssize_t uv_query_supp_se_hdr_pcf(struct kobject *kobj,
 static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
 	__ATTR(supp_se_hdr_pcf, 0444, uv_query_supp_se_hdr_pcf, NULL);
 
+static ssize_t uv_query_dump_cpu_len(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *page)
+{
+	return scnprintf(page, PAGE_SIZE, "%lx\n",
+			uv_info.guest_cpu_stor_len);
+}
+
+static struct kobj_attribute uv_query_dump_cpu_len_attr =
+	__ATTR(uv_query_dump_cpu_len, 0444, uv_query_dump_cpu_len, NULL);
+
+static ssize_t uv_query_dump_storage_state_len(struct kobject *kobj,
+					       struct kobj_attribute *attr, char *page)
+{
+	return scnprintf(page, PAGE_SIZE, "%lx\n",
+			uv_info.conf_dump_storage_state_len);
+}
+
+static struct kobj_attribute uv_query_dump_storage_state_len_attr =
+	__ATTR(dump_storage_state_len, 0444, uv_query_dump_storage_state_len, NULL);
+
+static ssize_t uv_query_dump_finalize_len(struct kobject *kobj,
+					  struct kobj_attribute *attr, char *page)
+{
+	return scnprintf(page, PAGE_SIZE, "%lx\n",
+			uv_info.conf_dump_finalize_len);
+}
+
+static struct kobj_attribute uv_query_dump_finalize_len_attr =
+	__ATTR(dump_finalize_len, 0444, uv_query_dump_finalize_len, NULL);
+
 static ssize_t uv_query_feature_indications(struct kobject *kobj,
 					    struct kobj_attribute *attr, char *buf)
 {
@@ -457,6 +487,9 @@ static struct attribute *uv_query_attrs[] = {
 	&uv_query_max_guest_addr_attr.attr,
 	&uv_query_supp_se_hdr_ver_attr.attr,
 	&uv_query_supp_se_hdr_pcf_attr.attr,
+	&uv_query_dump_storage_state_len_attr.attr,
+	&uv_query_dump_finalize_len_attr.attr,
+	&uv_query_dump_cpu_len_attr.attr,
 	NULL,
 };
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
  2022-02-23  9:19 ` [PATCH 1/9] s390x: Add SE hdr query information Janosch Frank
  2022-02-23  9:20 ` [PATCH 2/9] s390: uv: Add dump fields to query Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-02-23 11:30   ` Thomas Huth
  2022-03-02 12:04   ` Claudio Imbrenda
  2022-02-23  9:20 ` [PATCH 4/9] KVM: s390: pv: Add dump support definitions Janosch Frank
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Some of the query information is already available via sysfs but
having a IOCTL makes the information easier to retrieve.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index faa85397b6fb..837f898ad2ff 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
 	return r;
 }
 
+static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
+{
+	u32 len;
+
+	switch (info->header.id) {
+	case KVM_PV_INFO_VM: {
+		len =  sizeof(info->header) + sizeof(info->vm);
+
+		if (info->header.len < len)
+			return -EINVAL;
+
+		memcpy(info->vm.inst_calls_list,
+		       uv_info.inst_calls_list,
+		       sizeof(uv_info.inst_calls_list));
+
+		/* It's max cpuidm not max cpus so it's off by one */
+		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
+		info->vm.max_guests = uv_info.max_num_sec_conf;
+		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
+		info->vm.feature_indication = uv_info.uv_feature_indications;
+
+		return 0;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
 static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
 {
 	int r = 0;
@@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
 			     cmd->rc, cmd->rrc);
 		break;
 	}
+	case KVM_PV_INFO: {
+		struct kvm_s390_pv_info info = {};
+
+		if (copy_from_user(&info, argp, sizeof(info.header)))
+			return -EFAULT;
+
+		if (info.header.len < sizeof(info.header))
+			return -EINVAL;
+
+		r = kvm_s390_handle_pv_info(&info);
+		if (r)
+			return r;
+
+		r = copy_to_user(argp, &info, sizeof(info));
+
+		if (r)
+			return -EFAULT;
+		return 0;
+	}
 	default:
 		r = -ENOTTY;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index dbc550bbd9fa..96fceb204a92 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
 	__u64 tweak;
 };
 
+enum pv_cmd_info_id {
+	KVM_PV_INFO_VM,
+};
+
+struct kvm_s390_pv_info_vm {
+	__u64 inst_calls_list[4];
+	__u64 max_cpus;
+	__u64 max_guests;
+	__u64 max_guest_addr;
+	__u64 feature_indication;
+};
+
+struct kvm_s390_pv_info_header {
+	__u32 id;
+	__u32 len;
+};
+
+struct kvm_s390_pv_info {
+	struct kvm_s390_pv_info_header header;
+	struct kvm_s390_pv_info_vm vm;
+};
+
 enum pv_cmd_id {
 	KVM_PV_ENABLE,
 	KVM_PV_DISABLE,
@@ -1650,6 +1672,7 @@ enum pv_cmd_id {
 	KVM_PV_VERIFY,
 	KVM_PV_PREP_RESET,
 	KVM_PV_UNSHARE_ALL,
+	KVM_PV_INFO,
 };
 
 struct kvm_pv_cmd {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 4/9] KVM: s390: pv: Add dump support definitions
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (2 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 3/9] KVM: s390: pv: Add query interface Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-02-23  9:20 ` [PATCH 5/9] KVM: s390: pv: Add query dump information Janosch Frank
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Lets add the constants and structure definitions needed for the dump
support.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/include/asm/uv.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index b79e516d4424..b593fbcfd448 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -50,6 +50,10 @@
 #define UVC_CMD_SET_UNSHARE_ALL		0x0340
 #define UVC_CMD_PIN_PAGE_SHARED		0x0341
 #define UVC_CMD_UNPIN_PAGE_SHARED	0x0342
+#define UVC_CMD_DUMP_INIT		0x0400
+#define UVC_CMD_DUMP_CONF_STOR_STATE	0x0401
+#define UVC_CMD_DUMP_CPU		0x0402
+#define UVC_CMD_DUMP_COMPLETE		0x0403
 #define UVC_CMD_SET_SHARED_ACCESS	0x1000
 #define UVC_CMD_REMOVE_SHARED_ACCESS	0x1001
 
@@ -76,6 +80,10 @@ enum uv_cmds_inst {
 	BIT_UVC_CMD_UNSHARE_ALL = 20,
 	BIT_UVC_CMD_PIN_PAGE_SHARED = 21,
 	BIT_UVC_CMD_UNPIN_PAGE_SHARED = 22,
+	BIT_UVC_CMD_DUMP_INIT = 24,
+	BIT_UVC_CMD_DUMP_CONFIG_STOR_STATE = 25,
+	BIT_UVC_CMD_DUMP_CPU = 26,
+	BIT_UVC_CMD_DUMP_COMPLETE = 27,
 };
 
 enum uv_feat_ind {
@@ -224,6 +232,31 @@ struct uv_cb_share {
 	u64 reserved28;
 } __packed __aligned(8);
 
+struct uv_cb_dump_cpu {
+	struct uv_cb_header header;
+	u64 reserved08[2];
+	u64 cpu_handle;
+	u64 dump_area_origin;
+	u64 reserved28[5];
+} __packed __aligned(8);
+
+struct uv_cb_dump_stor_state {
+	struct uv_cb_header header;
+	u64 reserved08[2];
+	u64 config_handle;
+	u64 dump_area_origin;
+	u64 gaddr;
+	u64 reserved28[4];
+} __packed __aligned(8);
+
+struct uv_cb_dump_complete {
+	struct uv_cb_header header;
+	u64 reserved08[2];
+	u64 config_handle;
+	u64 dump_area_origin;
+	u64 reserved30[5];
+} __packed __aligned(8);
+
 static inline int __uv_call(unsigned long r1, unsigned long r2)
 {
 	int cc;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 5/9] KVM: s390: pv: Add query dump information
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (3 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 4/9] KVM: s390: pv: Add dump support definitions Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-03-01 17:34   ` Claudio Imbrenda
  2022-02-23  9:20 ` [PATCH 6/9] kvm: s390: Add configuration dump functionality Janosch Frank
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

The dump API requires userspace to provide buffers into which we will
store data. The dump information added in this patch tells userspace
how big those buffers need to be.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 11 +++++++++++
 include/uapi/linux/kvm.h | 12 +++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 837f898ad2ff..8de53803c1ca 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2240,6 +2240,17 @@ static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
 
 		return 0;
 	}
+	case KVM_PV_INFO_DUMP: {
+		len =  sizeof(info->header) + sizeof(info->dump);
+
+		if (info->header.len < len)
+			return -EINVAL;
+
+		info->dump.dump_cpu_buffer_len = uv_info.guest_cpu_stor_len;
+		info->dump.dump_config_mem_buffer_per_1m = uv_info.conf_dump_storage_state_len;
+		info->dump.dump_config_finalize_len = uv_info.conf_dump_finalize_len;
+		return 0;
+	}
 	default:
 		return -EINVAL;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 96fceb204a92..d58cd5a40e62 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1644,6 +1644,13 @@ struct kvm_s390_pv_unp {
 
 enum pv_cmd_info_id {
 	KVM_PV_INFO_VM,
+	KVM_PV_INFO_DUMP,
+};
+
+struct kvm_s390_pv_info_dump {
+	__u64 dump_cpu_buffer_len;
+	__u64 dump_config_mem_buffer_per_1m;
+	__u64 dump_config_finalize_len;
 };
 
 struct kvm_s390_pv_info_vm {
@@ -1661,7 +1668,10 @@ struct kvm_s390_pv_info_header {
 
 struct kvm_s390_pv_info {
 	struct kvm_s390_pv_info_header header;
-	struct kvm_s390_pv_info_vm vm;
+	union {
+		struct kvm_s390_pv_info_dump dump;
+		struct kvm_s390_pv_info_vm vm;
+	};
 };
 
 enum pv_cmd_id {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 6/9] kvm: s390: Add configuration dump functionality
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (4 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 5/9] KVM: s390: pv: Add query dump information Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-02-23 18:13   ` kernel test robot
  2022-02-23 19:25   ` kernel test robot
  2022-02-23  9:20 ` [PATCH 7/9] kvm: s390: Add CPU " Janosch Frank
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Sometimes dumping inside of a VM fails, is unavailable or doesn't
yield the required data. For these occasions we dump the VM from the
outside, writing memory and cpu data to a file.

Up to now PV guests only supported dumping from the inside of the
guest through dumpers like KDUMP. A PV guest can be dumped from the
hypervisor but the data will be stale and / or encrypted.

To get the actual state of the PV VM we need the help of the
Ultravisor who safeguards the VM state. New UV calls have been added
to initialize the dump, dump storage state data, dump cpu data and
complete the dump process. We expose these calls in this patch via a
new UV ioctl command.

The sensitive parts of the dump data are encrypted, the dump key is
derived from the Customer Communication Key (CCK). This ensures that
only the owner of the VM who has the CCK can decrypt the dump data.

The memory is dumped / read via a normal export call and a re-import
after the dump initialization is not needed (no re-encryption with a
dump key).

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |   1 +
 arch/s390/kvm/kvm-s390.c         | 137 +++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.h         |   2 +
 arch/s390/kvm/pv.c               | 115 ++++++++++++++++++++++++++
 include/uapi/linux/kvm.h         |  16 ++++
 5 files changed, 271 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index a22c9266ea05..659bf4be6f04 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -921,6 +921,7 @@ struct kvm_s390_pv {
 	u64 guest_len;
 	unsigned long stor_base;
 	void *stor_var;
+	bool dumping;
 };
 
 struct kvm_arch{
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8de53803c1ca..074d50d58430 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -606,6 +606,26 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_PROTECTED:
 		r = is_prot_virt_host();
 		break;
+	case KVM_CAP_S390_PROTECTED_DUMP: {
+		u64 pv_cmds_dump[] = {
+			BIT_UVC_CMD_DUMP_INIT,
+			BIT_UVC_CMD_DUMP_CONFIG_STOR_STATE,
+			BIT_UVC_CMD_DUMP_CONFIG_STOR_STATE,
+			BIT_UVC_CMD_DUMP_CPU,
+		};
+		int i;
+
+		if (!is_prot_virt_host())
+			return 0;
+
+		r = 1;
+		for (i = 0; i < ARRAY_SIZE(pv_cmds_dump); i++) {
+			if (!test_bit_inv(pv_cmds_dump[i],
+					  (unsigned long *)&uv_info.inst_calls_list))
+				return 0;
+		}
+		break;
+	}
 	default:
 		r = 0;
 	}
@@ -2256,6 +2276,92 @@ static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
 	}
 }
 
+static int kvm_s390_pv_dmp(struct kvm *kvm, struct kvm_pv_cmd *cmd,
+			   struct kvm_s390_pv_dmp dmp)
+{
+	int r = -EINVAL;
+	void __user *result_buff = (void __user *)dmp.buff_addr;
+
+	switch (dmp.subcmd) {
+	case KVM_PV_DUMP_INIT: {
+		if (kvm->arch.pv.dumping)
+			break;
+
+		r = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm),
+				  UVC_CMD_DUMP_INIT, &cmd->rc, &cmd->rrc);
+		KVM_UV_EVENT(kvm, 3, "PROTVIRT DUMP INIT: rc %x rrc %x",
+			     cmd->rc, cmd->rrc);
+		if (!r)
+			kvm->arch.pv.dumping = true;
+		else
+			r = -EINVAL;
+		break;
+	}
+	case KVM_PV_DUMP_CONFIG_STOR_STATE: {
+		if (!kvm->arch.pv.dumping)
+			break;
+
+		/*
+		 * GADDR is an output parameter since we might stop
+		 * early. As dmp will be copied back in our caller, we
+		 * don't need to do it ourselves.
+		 */
+		r = kvm_s390_pv_dump_stor_state(kvm, result_buff, &dmp.gaddr, dmp.buff_len,
+						&cmd->rc, &cmd->rrc);
+		break;
+	}
+	case KVM_PV_DUMP_COMPLETE: {
+		struct uv_cb_dump_complete complete = {
+			.header.len = sizeof(complete),
+			.header.cmd = UVC_CMD_DUMP_COMPLETE,
+			.config_handle = kvm_s390_pv_get_handle(kvm),
+		};
+		u64 *compl_data;
+
+		r = -EINVAL;
+		if (!kvm->arch.pv.dumping)
+			break;
+
+		if (dmp.buff_len < uv_info.conf_dump_finalize_len)
+			break;
+
+		/* Allocate dump area */
+		r = -ENOMEM;
+		compl_data = vzalloc(uv_info.conf_dump_finalize_len);
+		if (!compl_data)
+			break;
+		complete.dump_area_origin = (u64)compl_data;
+
+		r = uv_call(0, (u64)&complete);
+		cmd->rc = complete.header.rc;
+		cmd->rrc = complete.header.rrc;
+		KVM_UV_EVENT(kvm, 3, "PROTVIRT DUMP COMPLETE: rc %x rrc %x",
+			     complete.header.rc, complete.header.rrc);
+
+		if (!r) {
+			/*
+			 * kvm_s390_pv_dealloc_vm() will also (mem)set
+			 * this to false on a reboot or other destroy
+			 * operation for this vm.
+			 */
+			kvm->arch.pv.dumping = false;
+			r = copy_to_user(result_buff, compl_data, uv_info.conf_dump_finalize_len);
+			if (r)
+				r = -EFAULT;
+		}
+		vfree(compl_data);
+		if (r > 0)
+			r = -EINVAL;
+		break;
+	}
+	default:
+		r = -ENOTTY;
+		break;
+	}
+
+	return r;
+}
+
 static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
 {
 	int r = 0;
@@ -2411,6 +2517,28 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
 			return -EFAULT;
 		return 0;
 	}
+	case KVM_PV_DUMP: {
+		struct kvm_s390_pv_dmp dmp;
+
+		r = -EINVAL;
+		if (!kvm_s390_pv_is_protected(kvm))
+			break;
+
+		r = -EFAULT;
+		if (copy_from_user(&dmp, argp, sizeof(dmp)))
+			break;
+
+		r = kvm_s390_pv_dmp(kvm, cmd, dmp);
+		if (r)
+			break;
+
+		if (copy_to_user(argp, &dmp, sizeof(dmp))) {
+			r = -EFAULT;
+			break;
+		}
+
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
@@ -4508,6 +4636,15 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 	struct kvm_run *kvm_run = vcpu->run;
 	int rc;
 
+	/*
+	 * Running a VM while dumping always has the potential to
+	 * produce inconsistent dump data. But for PV vcpus a SIE
+	 * entry while dumping could also lead to a validity which we
+	 * absolutely want to avoid.
+	 */
+	if (vcpu->kvm->arch.pv.dumping)
+		return -EINVAL;
+
 	if (kvm_run->immediate_exit)
 		return -EINTR;
 
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 098831e815e6..dbae30aa338c 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -241,6 +241,8 @@ int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
 int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
 		       unsigned long tweak, u16 *rc, u16 *rrc);
 int kvm_s390_pv_set_cpu_state(struct kvm_vcpu *vcpu, u8 state);
+int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
+				u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc);
 
 static inline u64 kvm_s390_pv_get_handle(struct kvm *kvm)
 {
diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 7f7c0d6af2ce..3c1c257ed38e 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -303,3 +303,118 @@ int kvm_s390_pv_set_cpu_state(struct kvm_vcpu *vcpu, u8 state)
 		return -EINVAL;
 	return 0;
 }
+
+/* Size of the cache for the storage state dump data. 1MB for now */
+#define DUMP_BUFF_LEN HPAGE_SIZE
+
+/*
+ * kvm_s390_pv_dump_stor_state
+ *
+ * @kvm: pointer to the guest's KVM struct
+ * @buff_user: Userspace pointer where we will write the results to
+ * @gaddr: Starting absolute guest address for which the storage state
+ *         is requested. This value will be updated with the last
+ *         address for which data was written when returning to
+ *         userspace.
+ * @buff_user_len: Length of the buff_user buffer
+ * @rc: Pointer to where the uvcb return code is stored
+ * @rrc: Pointer to where the uvcb return reason code is stored
+ *
+ * Return:
+ *  0 on success
+ *  -ENOMEM if allocating the cache fails
+ *  -EINVAL if gaddr is not aligned to 1MB
+ *  -EINVAL if buff_user_len is not aligned to uv_info.conf_dump_storage_state_len
+ *  -EINVAL if the UV call fails, rc and rrc will be set in this case
+ *  -EFAULT if copying the result to buff_user failed
+ */
+int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
+				u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc)
+{
+	struct uv_cb_dump_stor_state uvcb = {
+		.header.cmd = UVC_CMD_DUMP_CONF_STOR_STATE,
+		.header.len = sizeof(uvcb),
+		.config_handle = kvm->arch.pv.handle,
+		.gaddr = *gaddr,
+		.dump_area_origin = 0,
+	};
+	size_t buff_kvm_size;
+	size_t size_done = 0;
+	u8 *buff_kvm = NULL;
+	int cc, ret;
+
+	ret = -EINVAL;
+	/* UV call processes 1MB guest storage chunks at a time */
+	if (*gaddr & ~HPAGE_MASK)
+		goto out;
+
+	/*
+	 * We provide the storage state for 1MB chunks of guest
+	 * storage. The buffer will need to be aligned to
+	 * conf_dump_storage_state_len so we don't end on a partial
+	 * chunk.
+	 */
+	if (!buff_user_len ||
+	    buff_user_len & (uv_info.conf_dump_storage_state_len - 1))
+		goto out;
+
+	/*
+	 * Allocate a buffer from which we will later copy to the user process.
+	 *
+	 * We don't want userspace to dictate our buffer size so we limit it to DUMP_BUFF_LEN.
+	 */
+	ret = -ENOMEM;
+	buff_kvm_size = buff_user_len <= DUMP_BUFF_LEN ? buff_user_len : DUMP_BUFF_LEN;
+	buff_kvm = vzalloc(buff_kvm_size);
+	if (!buff_kvm)
+		goto out;
+
+	ret = 0;
+	uvcb.dump_area_origin = (u64)buff_kvm;
+	/* We will loop until the user buffer is filled or an error occurs */
+	do {
+		/* Get a page of data */
+		cc = uv_call_sched(0, (u64)&uvcb);
+
+		/* All or nothing */
+		if (cc) {
+			ret = -EINVAL;
+			break;
+		}
+
+		size_done += uv_info.conf_dump_storage_state_len;
+		uvcb.dump_area_origin += uv_info.conf_dump_storage_state_len;
+		uvcb.gaddr += HPAGE_SIZE;
+		buff_user_len -= PAGE_SIZE;
+
+		/* KVM Buffer full, time to copy to the process */
+		if (!buff_user_len ||
+		    uvcb.dump_area_origin == (uintptr_t)buff_kvm + buff_kvm_size) {
+
+			if (copy_to_user(buff_user, buff_kvm,
+					 uvcb.dump_area_origin - (uintptr_t)buff_kvm)) {
+				ret = -EFAULT;
+				break;
+			}
+
+			buff_user += size_done;
+			size_done = 0;
+			uvcb.dump_area_origin = (u64)buff_kvm;
+		}
+	} while (buff_user_len);
+
+	/* Report back where we ended dumping */
+	*gaddr = uvcb.gaddr;
+
+	/* Lets only log errors, we don't want to spam */
+out:
+	if (cc || ret)
+		KVM_UV_EVENT(kvm, 3,
+			     "PROTVIRT DUMP STORAGE STATE: addr %llx ret %d, uvcb rc %x rrc %x",
+			     uvcb.gaddr, ret, uvcb.header.rc, uvcb.header.rrc);
+	*rc = uvcb.header.rc;
+	*rrc = uvcb.header.rrc;
+	vfree(buff_kvm);
+
+	return cc ? -EINVAL : 0;
+}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index d58cd5a40e62..913c5907b9f1 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1141,6 +1141,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_XSAVE2 208
 #define KVM_CAP_SYS_ATTRIBUTES 209
 #define KVM_CAP_S390_MEM_OP_EXTENSION 210
+#define KVM_CAP_S390_PROTECTED_DUMP 211
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1642,6 +1643,20 @@ struct kvm_s390_pv_unp {
 	__u64 tweak;
 };
 
+enum pv_cmd_dmp_id {
+	KVM_PV_DUMP_INIT,
+	KVM_PV_DUMP_CONFIG_STOR_STATE,
+	KVM_PV_DUMP_COMPLETE,
+};
+
+struct kvm_s390_pv_dmp {
+	__u64 subcmd;
+	__u64 buff_addr;
+	__u64 buff_len;
+	__u64 gaddr;		/* For dump storage state */
+	__u64 reserved[4];
+};
+
 enum pv_cmd_info_id {
 	KVM_PV_INFO_VM,
 	KVM_PV_INFO_DUMP,
@@ -1683,6 +1698,7 @@ enum pv_cmd_id {
 	KVM_PV_PREP_RESET,
 	KVM_PV_UNSHARE_ALL,
 	KVM_PV_INFO,
+	KVM_PV_DUMP,
 };
 
 struct kvm_pv_cmd {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 7/9] kvm: s390: Add CPU dump functionality
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (5 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 6/9] kvm: s390: Add configuration dump functionality Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-02-23 19:46   ` kernel test robot
  2022-02-23  9:20 ` [PATCH 8/9] Documentation: virt: Protected virtual machine dumps Janosch Frank
  2022-02-23  9:20 ` [PATCH 9/9] Documentation/virt/kvm/api.rst: Add protvirt dump/info api descriptions Janosch Frank
  8 siblings, 1 reply; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

The previous patch introduced the per-VM dump functions now let's
focus on dumping the VCPU state via the newly introduced
KVM_S390_PV_CPU_COMMAND ioctl which mirrors the VM UV ioctl and can be
extended with new commands later.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 72 ++++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.h |  1 +
 arch/s390/kvm/pv.c       | 16 +++++++++
 include/uapi/linux/kvm.h |  4 +++
 4 files changed, 93 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 074d50d58430..d73cbd910a79 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -5084,6 +5084,51 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
 	return -ENOIOCTLCMD;
 }
 
+int kvm_s390_handle_pv_vcpu_dump(struct kvm_vcpu *vcpu, struct kvm_pv_cmd *cmd)
+{
+	struct kvm_s390_pv_dmp dmp;
+	void *data;
+	int ret;
+
+	/* Dump initialization is a prerequisite */
+	if (!vcpu->kvm->arch.pv.dumping)
+		return -EINVAL;
+
+	if (copy_from_user(&dmp, (__u8 __user *)cmd->data, sizeof(dmp)))
+		return -EFAULT;
+
+	/* We only handle this subcmd right now */
+	if (dmp.subcmd != KVM_PV_DUMP_CPU)
+		return -EINVAL;
+
+	/* CPU dump length is the same as create cpu storage donation. */
+	if (dmp.buff_len != uv_info.guest_cpu_stor_len)
+		return -EINVAL;
+
+	data = vzalloc(uv_info.guest_cpu_stor_len);
+	if (!data)
+		return -ENOMEM;
+
+	ret = kvm_s390_pv_dump_cpu(vcpu, data, &cmd->rc, &cmd->rrc);
+
+	VCPU_EVENT(vcpu, 3, "PROTVIRT DUMP CPU %d rc %x rrc %x",
+		   vcpu->vcpu_id, cmd->rc, cmd->rrc);
+
+	if (ret) {
+		vfree(data);
+		return -EINVAL;
+	}
+
+	/* On success copy over the dump data */
+	if (copy_to_user((__u8 __user *)dmp.buff_addr, data, uv_info.guest_cpu_stor_len)) {
+		vfree(data);
+		return -EFAULT;
+	}
+
+	vfree(data);
+	return 0;
+}
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
@@ -5248,6 +5293,33 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 					   irq_state.len);
 		break;
 	}
+	case KVM_S390_PV_CPU_COMMAND: {
+		struct kvm_pv_cmd cmd;
+
+		r = -EINVAL;
+		if (!is_prot_virt_host())
+			break;
+
+		r = -EFAULT;
+		if (copy_from_user(&cmd, argp, sizeof(cmd)))
+			break;
+
+		r = -EINVAL;
+		if (cmd.flags)
+			break;
+
+		/* We only handle this cmd right now */
+		if (cmd.cmd != KVM_PV_DUMP)
+			break;
+
+		r = kvm_s390_handle_pv_vcpu_dump(vcpu, &cmd);
+
+		/* Always copy over UV rc / rrc data */
+		if (copy_to_user((__u8 __user *)argp, &cmd.rc,
+				 sizeof(cmd.rc) + sizeof(cmd.rrc)))
+			r = -EFAULT;
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index dbae30aa338c..5746a7679f21 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -241,6 +241,7 @@ int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
 int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
 		       unsigned long tweak, u16 *rc, u16 *rrc);
 int kvm_s390_pv_set_cpu_state(struct kvm_vcpu *vcpu, u8 state);
+int kvm_s390_pv_dump_cpu(struct kvm_vcpu *vcpu, void *buff, u16 *rc, u16 *rrc);
 int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
 				u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc);
 
diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 3c1c257ed38e..f5a9ff0164b2 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -304,6 +304,22 @@ int kvm_s390_pv_set_cpu_state(struct kvm_vcpu *vcpu, u8 state)
 	return 0;
 }
 
+int kvm_s390_pv_dump_cpu(struct kvm_vcpu *vcpu, void *buff, u16 *rc, u16 *rrc)
+{
+	struct uv_cb_dump_cpu uvcb = {
+		.header.cmd = UVC_CMD_DUMP_CPU,
+		.header.len = sizeof(uvcb),
+		.cpu_handle = vcpu->arch.pv.handle,
+		.dump_area_origin = (u64)buff,
+	};
+	int cc;
+
+	cc = uv_call_sched(0, (u64)&uvcb);
+	*rc = uvcb.header.rc;
+	*rrc = uvcb.header.rrc;
+	return cc;
+}
+
 /* Size of the cache for the storage state dump data. 1MB for now */
 #define DUMP_BUFF_LEN HPAGE_SIZE
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 913c5907b9f1..129d21f403c3 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1647,6 +1647,7 @@ enum pv_cmd_dmp_id {
 	KVM_PV_DUMP_INIT,
 	KVM_PV_DUMP_CONFIG_STOR_STATE,
 	KVM_PV_DUMP_COMPLETE,
+	KVM_PV_DUMP_CPU,
 };
 
 struct kvm_s390_pv_dmp {
@@ -2104,4 +2105,7 @@ struct kvm_stats_desc {
 /* Available with KVM_CAP_XSAVE2 */
 #define KVM_GET_XSAVE2		  _IOR(KVMIO,  0xcf, struct kvm_xsave)
 
+/* Available with KVM_CAP_S390_PROTECTED_DUMP */
+#define KVM_S390_PV_CPU_COMMAND	_IOWR(KVMIO, 0xd0, struct kvm_pv_cmd)
+
 #endif /* __LINUX_KVM_H */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 8/9] Documentation: virt: Protected virtual machine dumps
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (6 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 7/9] kvm: s390: Add CPU " Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  2022-02-23  9:20 ` [PATCH 9/9] Documentation/virt/kvm/api.rst: Add protvirt dump/info api descriptions Janosch Frank
  8 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Let's add a documentation file which describes the dump process. Since
we only copy the UV dump data from the UV to userspace we'll not go
into detail here and let the party which processes the data describe
its structure.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 Documentation/virt/kvm/index.rst        |  1 +
 Documentation/virt/kvm/s390-pv-dump.rst | 60 +++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/virt/kvm/s390-pv-dump.rst

diff --git a/Documentation/virt/kvm/index.rst b/Documentation/virt/kvm/index.rst
index b6833c7bb474..32f3eed5fadb 100644
--- a/Documentation/virt/kvm/index.rst
+++ b/Documentation/virt/kvm/index.rst
@@ -20,6 +20,7 @@ KVM
    s390-diag
    s390-pv
    s390-pv-boot
+   s390-pv-dump
    timekeeping
    vcpu-requests
 
diff --git a/Documentation/virt/kvm/s390-pv-dump.rst b/Documentation/virt/kvm/s390-pv-dump.rst
new file mode 100644
index 000000000000..6fe7560e10b1
--- /dev/null
+++ b/Documentation/virt/kvm/s390-pv-dump.rst
@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================================
+s390 (IBM Z) Protected Virtualization dumps
+===========================================
+
+Summary
+-------
+
+Dumping a VM is an essential tool for debugging problems inside
+it. This is especially true when a protected VM runs into trouble as
+there's no way to access its memory and registers from the outside
+while it's running.
+
+However when dumping a protected VM we need to maintain its
+confidentiality until the dump is in the hands of the VM owner who
+should be the only one capable of analysing it.
+
+The confidentiality of the VM dump is ensured by the Ultravisor who
+provides an interface to KVM over which encrypted CPU and memory data
+can be requested. The encryption is based on the Customer
+Communication Key which is the key that's used to encrypt VM data in a
+way that the customer is able to decrypt.
+
+
+Dump process
+------------
+
+A dump is done in 3 steps:
+
+Initiation
+This step initializes the dump process, generates cryptographic seeds
+and extracts dump keys with which the VM dump data will be encrypted.
+
+Data gathering
+Currently there are two types of data that can be gathered from a VM:
+the memory and the vcpu state.
+
+The vcpu state contains all the important registers, general, floating
+point, vector, control and tod/timers of a vcpu. The vcpu dump can
+contain incomplete data if a vcpu is dumped while an instruction is
+emulated with help of the hypervisor. This is indicated by a flag bit
+in the dump data. For the same reason it is very important to not only
+write out the encrypted vcpu state, but also the unencrypted state
+from the hypervisor.
+
+The memory state is further divided into the encrypted memory and its
+encryption tweaks / status flags. The encrypted memory can simply be
+read once it has been exported. The time of the export does not matter
+as no re-encryption is needed. Memory that has been swapped out and
+hence was exported can be read from the swap and written to the dump
+target without need for any special actions.
+
+The tweaks / status flags for the exported pages need to be requested
+from the Ultravisor.
+
+Finalization
+The finalization step will provide the data needed to be able to
+decrypt the vcpu and memory data and end the dump process. When this
+step completes successfully a new dump initiation can be started.
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* [PATCH 9/9] Documentation/virt/kvm/api.rst: Add protvirt dump/info api descriptions
  2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
                   ` (7 preceding siblings ...)
  2022-02-23  9:20 ` [PATCH 8/9] Documentation: virt: Protected virtual machine dumps Janosch Frank
@ 2022-02-23  9:20 ` Janosch Frank
  8 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23  9:20 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, imbrenda, david, borntraeger

Time to add the dump API changes to the api documentation file.
Also some minor cleanup.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 Documentation/virt/kvm/api.rst | 129 ++++++++++++++++++++++++++++++++-
 1 file changed, 127 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 622667cc87ef..f73a5d774191 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5062,7 +5062,7 @@ into ESA mode. This reset is a superset of the initial reset.
 	__u32 reserved[3];
   };
 
-cmd values:
+**cmd values:**
 
 KVM_PV_ENABLE
   Allocate memory and register the VM with the Ultravisor, thereby
@@ -5078,7 +5078,6 @@ KVM_PV_ENABLE
   =====      =============================
 
 KVM_PV_DISABLE
-
   Deregister the VM from the Ultravisor and reclaim the memory that
   had been donated to the Ultravisor, making it usable by the kernel
   again.  All registered VCPUs are converted back to non-protected
@@ -5095,6 +5094,93 @@ KVM_PV_VM_VERIFY
   Verify the integrity of the unpacked image. Only if this succeeds,
   KVM is allowed to start protected VCPUs.
 
+KVM_PV_INFO
+  :Capability: KVM_CAP_S390_PROTECTED_DUMP
+
+  Presents an API that provides Ultravisor related data to userspace
+  like the size of buffers needed for other commands, the UV feature
+  indications and the installed UV calls.
+
+**subcommands:**
+
+  KVM_PV_INFO_VM
+    This subcommand provides basic Ultravisor information for PV
+    hosts. These values are likely also exported as files in the sysfs
+    firmware UV query interface but they are more easily available to
+    programs in this API.
+
+    The installed calls and feature_indication members provide the
+    installed UV calls and the UV's other feature indications.
+
+    The max_* members provide information about the maximum number of PV
+    vcpus, PV guests and PV guest memory size.
+
+    ::
+
+      struct kvm_s390_pv_info_vm {
+        __u64 inst_calls_list[4];
+        __u64 max_cpus;
+        __u64 max_guests;
+        __u64 max_guest_addr;
+        __u64 feature_indication;
+      };
+
+
+  KVM_PV_INFO_DUMP
+    This subcommand provides information related to dumping PV guests.
+
+    ::
+
+      struct kvm_s390_pv_info_dump {
+        __u64 dump_cpu_buffer_len;
+        __u64 dump_config_mem_buffer_per_1m;
+        __u64 dump_config_finalize_len;
+      };
+
+KVM_PV_DUMP
+  :Capability: KVM_CAP_S390_PROTECTED_DUMP
+
+  Presents an API that provides calls which facilitate dumping a
+  protected VM.
+
+  ::
+
+    struct kvm_s390_pv_dmp {
+      __u64 subcmd;
+      __u64 buff_addr;
+      __u64 buff_len;
+      __u64 gaddr;		/* For dump storage state */
+    };
+
+  **subcommands:**
+
+  KVM_PV_DUMP_INIT
+    Initializes the dump process of a protected VM. If this call does
+    not succeed all other subcommands will fail with -EINVAL. This
+    subcommand will return -EINVAL if a dump process has not yet been
+    completed.
+
+    Not all PV vms can be dumped, the owner needs to set `dump
+    allowed` PCF bit 34 in the SE header to allow dumping.
+
+  KVM_PV_DUMP_CONFIG_STOR_STATE
+    Stores `buff_len` bytes of tweak component values starting with
+    the 1MB block specified by the absolute guest address
+    (`gaddr`). `buff_len` needs to be `conf_dump_storage_state_len`
+    aligned and at least >= the `conf_dump_storage_state_len` value
+    provided by the dump uv_info data.
+
+  KVM_PV_DUMP_COMPLETE
+    If the subcommand succeeds it completes the dump process and lets
+    KVM_PV_DUMP_INIT be called again.
+
+    On success `conf_dump_finalize_len` bytes of completion data will be
+    stored to the `buff_addr`. The completion data contains a key
+    derivation seed, IV, tweak nonce and encryption keys as well as an
+    authentication tag all of which are needed to decrypt the dump at a
+    later time.
+
+
 4.126 KVM_X86_SET_MSR_FILTER
 ----------------------------
 
@@ -5643,6 +5729,32 @@ The offsets of the state save areas in struct kvm_xsave follow the contents
 of CPUID leaf 0xD on the host.
 
 
+4.135 KVM_S390_PV_CPU_COMMAND
+-----------------------------
+
+:Capability: KVM_CAP_S390_PROTECTED_DUMP
+:Architectures: s390
+:Type: vcpu ioctl
+:Parameters: none
+:Returns: 0 on success, < 0 on error
+
+This ioctl closely mirrors `KVM_S390_PV_COMMAND` but handles requests
+for vcpus. It re-uses the kvm_s390_pv_dmp struct and hence also shares
+the command ids.
+
+**command:**
+
+KVM_PV_DUMP
+  Presents an API that provides calls which facilitate dumping a vcpu
+  of a protected VM.
+
+**subcommand:**
+
+KVM_PV_DUMP_CPU
+  Provides encrypted dump data like register values.
+  The length of the returned data is provided by uv_info.guest_cpu_stor_len.
+
+
 5. The kvm_run structure
 ========================
 
@@ -7629,3 +7741,16 @@ The argument to KVM_ENABLE_CAP is also a bitmask, and must be a subset
 of the result of KVM_CHECK_EXTENSION.  KVM will forward to userspace
 the hypercalls whose corresponding bit is in the argument, and return
 ENOSYS for the others.
+
+8.35 KVM_CAP_S390_PROTECTED_DUMP
+--------------------------------
+
+:Capability: KVM_CAP_S390_PROTECTED_DUMP
+:Architectures: s390
+:Type: vm
+
+This capability indicates that KVM and the Ultravisor support dumping
+PV guests. The `KVM_PV_DUMP` command is available for the
+`KVM_S390_PV_COMMAND` ioctl and the `KVM_PV_INFO` command provides
+dump related UV data. Also the vcpu ioctl `KVM_S390_PV_CPU_COMMAND` is
+available and supports the `KVM_PV_DUMP_CPU` subcommand.
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-02-23  9:20 ` [PATCH 3/9] KVM: s390: pv: Add query interface Janosch Frank
@ 2022-02-23 11:30   ` Thomas Huth
  2022-02-23 12:47     ` Janosch Frank
  2022-03-01 17:32     ` Claudio Imbrenda
  2022-03-02 12:04   ` Claudio Imbrenda
  1 sibling, 2 replies; 23+ messages in thread
From: Thomas Huth @ 2022-02-23 11:30 UTC (permalink / raw)
  To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, borntraeger

On 23/02/2022 10.20, Janosch Frank wrote:
> Some of the query information is already available via sysfs but
> having a IOCTL makes the information easier to retrieve.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
>   include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
>   2 files changed, 70 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index faa85397b6fb..837f898ad2ff 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>   	return r;
>   }
>   
> +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
> +{
> +	u32 len;
> +
> +	switch (info->header.id) {
> +	case KVM_PV_INFO_VM: {
> +		len =  sizeof(info->header) + sizeof(info->vm);
> +
> +		if (info->header.len < len)
> +			return -EINVAL;
> +
> +		memcpy(info->vm.inst_calls_list,
> +		       uv_info.inst_calls_list,
> +		       sizeof(uv_info.inst_calls_list));
> +
> +		/* It's max cpuidm not max cpus so it's off by one */
> +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
> +		info->vm.max_guests = uv_info.max_num_sec_conf;
> +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
> +		info->vm.feature_indication = uv_info.uv_feature_indications;
> +
> +		return 0;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>   static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>   {
>   	int r = 0;
> @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>   			     cmd->rc, cmd->rrc);
>   		break;
>   	}
> +	case KVM_PV_INFO: {
> +		struct kvm_s390_pv_info info = {};
> +
> +		if (copy_from_user(&info, argp, sizeof(info.header)))
> +			return -EFAULT;
> +
> +		if (info.header.len < sizeof(info.header))
> +			return -EINVAL;
> +
> +		r = kvm_s390_handle_pv_info(&info);
> +		if (r)
> +			return r;
> +
> +		r = copy_to_user(argp, &info, sizeof(info));

sizeof(info) is currently OK ... but this might break if somebody later 
extends the kvm_s390_pv_info struct, I guess? ==> Maybe also better use 
sizeof(info->header) + sizeof(info->vm) here, too? Or let 
kvm_s390_handle_pv_info() return the amount of bytes that should be copied here?

  Thomas


> +		if (r)
> +			return -EFAULT;
> +		return 0;
> +	}
>   	default:
>   		r = -ENOTTY;
>   	}
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index dbc550bbd9fa..96fceb204a92 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
>   	__u64 tweak;
>   };
>   
> +enum pv_cmd_info_id {
> +	KVM_PV_INFO_VM,
> +};
> +
> +struct kvm_s390_pv_info_vm {
> +	__u64 inst_calls_list[4];
> +	__u64 max_cpus;
> +	__u64 max_guests;
> +	__u64 max_guest_addr;
> +	__u64 feature_indication;
> +};
> +
> +struct kvm_s390_pv_info_header {
> +	__u32 id;
> +	__u32 len;
> +};
> +
> +struct kvm_s390_pv_info {
> +	struct kvm_s390_pv_info_header header;
> +	struct kvm_s390_pv_info_vm vm;
> +};
> +
>   enum pv_cmd_id {
>   	KVM_PV_ENABLE,
>   	KVM_PV_DISABLE,
> @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
>   	KVM_PV_VERIFY,
>   	KVM_PV_PREP_RESET,
>   	KVM_PV_UNSHARE_ALL,
> +	KVM_PV_INFO,
>   };
>   
>   struct kvm_pv_cmd {


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-02-23 11:30   ` Thomas Huth
@ 2022-02-23 12:47     ` Janosch Frank
  2022-03-01 17:32     ` Claudio Imbrenda
  1 sibling, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-02-23 12:47 UTC (permalink / raw)
  To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, borntraeger

On 2/23/22 12:30, Thomas Huth wrote:
> On 23/02/2022 10.20, Janosch Frank wrote:
>> Some of the query information is already available via sysfs but
>> having a IOCTL makes the information easier to retrieve.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>    arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>    include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
>>    2 files changed, 70 insertions(+)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index faa85397b6fb..837f898ad2ff 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>>    	return r;
>>    }
>>    
>> +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
>> +{
>> +	u32 len;
>> +
>> +	switch (info->header.id) {
>> +	case KVM_PV_INFO_VM: {
>> +		len =  sizeof(info->header) + sizeof(info->vm);
>> +
>> +		if (info->header.len < len)
>> +			return -EINVAL;
>> +
>> +		memcpy(info->vm.inst_calls_list,
>> +		       uv_info.inst_calls_list,
>> +		       sizeof(uv_info.inst_calls_list));
>> +
>> +		/* It's max cpuidm not max cpus so it's off by one */
>> +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
>> +		info->vm.max_guests = uv_info.max_num_sec_conf;
>> +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
>> +		info->vm.feature_indication = uv_info.uv_feature_indications;
>> +
>> +		return 0;
>> +	}
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>    static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>    {
>>    	int r = 0;
>> @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>    			     cmd->rc, cmd->rrc);
>>    		break;
>>    	}
>> +	case KVM_PV_INFO: {
>> +		struct kvm_s390_pv_info info = {};
>> +
>> +		if (copy_from_user(&info, argp, sizeof(info.header)))
>> +			return -EFAULT;
>> +
>> +		if (info.header.len < sizeof(info.header))
>> +			return -EINVAL;
>> +
>> +		r = kvm_s390_handle_pv_info(&info);
>> +		if (r)
>> +			return r;
>> +
>> +		r = copy_to_user(argp, &info, sizeof(info));
> 
> sizeof(info) is currently OK ... but this might break if somebody later
> extends the kvm_s390_pv_info struct, I guess? ==> Maybe also better use
> sizeof(info->header) + sizeof(info->vm) here, too? Or let
> kvm_s390_handle_pv_info() return the amount of bytes that should be copied here?

I'll have to take another look at this for multiple reasons.
Maybe we'll use the same semantics as the UV QUI command.

> 
>    Thomas
> 
> 
>> +		if (r)
>> +			return -EFAULT;
>> +		return 0;
>> +	}
>>    	default:
>>    		r = -ENOTTY;
>>    	}
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index dbc550bbd9fa..96fceb204a92 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
>>    	__u64 tweak;
>>    };
>>    
>> +enum pv_cmd_info_id {
>> +	KVM_PV_INFO_VM,
>> +};
>> +
>> +struct kvm_s390_pv_info_vm {
>> +	__u64 inst_calls_list[4];
>> +	__u64 max_cpus;
>> +	__u64 max_guests;
>> +	__u64 max_guest_addr;
>> +	__u64 feature_indication;
>> +};
>> +
>> +struct kvm_s390_pv_info_header {
>> +	__u32 id;
>> +	__u32 len;
>> +};
>> +
>> +struct kvm_s390_pv_info {
>> +	struct kvm_s390_pv_info_header header;
>> +	struct kvm_s390_pv_info_vm vm;
>> +};
>> +
>>    enum pv_cmd_id {
>>    	KVM_PV_ENABLE,
>>    	KVM_PV_DISABLE,
>> @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
>>    	KVM_PV_VERIFY,
>>    	KVM_PV_PREP_RESET,
>>    	KVM_PV_UNSHARE_ALL,
>> +	KVM_PV_INFO,
>>    };
>>    
>>    struct kvm_pv_cmd {
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 6/9] kvm: s390: Add configuration dump functionality
  2022-02-23  9:20 ` [PATCH 6/9] kvm: s390: Add configuration dump functionality Janosch Frank
@ 2022-02-23 18:13   ` kernel test robot
  2022-02-23 19:25   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-02-23 18:13 UTC (permalink / raw)
  To: Janosch Frank, kvm
  Cc: llvm, kbuild-all, linux-s390, imbrenda, david, borntraeger

Hi Janosch,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kvms390/next]
[also build test WARNING on next-20220222]
[cannot apply to kvm/master s390/features v5.17-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git next
config: s390-randconfig-r044-20220223 (https://download.01.org/0day-ci/archive/20220224/202202240208.nprfvRMA-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/63a2029ece7e8fee92aa5ad277e2cbd8b13b7e6b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
        git checkout 63a2029ece7e8fee92aa5ad277e2cbd8b13b7e6b
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash arch/s390/kvm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> arch/s390/kvm/pv.c:369:2: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!buff_kvm)
           ^~~~~~~~~~~~~~
   include/linux/compiler.h:56:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:419:9: note: uninitialized use occurs here
           return cc ? -EINVAL : 0;
                  ^~
   arch/s390/kvm/pv.c:369:2: note: remove the 'if' if its condition is always false
           if (!buff_kvm)
           ^~~~~~~~~~~~~~
   include/linux/compiler.h:56:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   arch/s390/kvm/pv.c:357:2: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!buff_user_len ||
           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:419:9: note: uninitialized use occurs here
           return cc ? -EINVAL : 0;
                  ^~
   arch/s390/kvm/pv.c:357:2: note: remove the 'if' if its condition is always false
           if (!buff_user_len ||
           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   arch/s390/kvm/pv.c:348:2: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (*gaddr & ~HPAGE_MASK)
           ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:419:9: note: uninitialized use occurs here
           return cc ? -EINVAL : 0;
                  ^~
   arch/s390/kvm/pv.c:348:2: note: remove the 'if' if its condition is always false
           if (*gaddr & ~HPAGE_MASK)
           ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   arch/s390/kvm/pv.c:344:8: note: initialize the variable 'cc' to silence this warning
           int cc, ret;
                 ^
                  = 0
   15 warnings generated.


vim +369 arch/s390/kvm/pv.c

   309	
   310	/*
   311	 * kvm_s390_pv_dump_stor_state
   312	 *
   313	 * @kvm: pointer to the guest's KVM struct
   314	 * @buff_user: Userspace pointer where we will write the results to
   315	 * @gaddr: Starting absolute guest address for which the storage state
   316	 *         is requested. This value will be updated with the last
   317	 *         address for which data was written when returning to
   318	 *         userspace.
   319	 * @buff_user_len: Length of the buff_user buffer
   320	 * @rc: Pointer to where the uvcb return code is stored
   321	 * @rrc: Pointer to where the uvcb return reason code is stored
   322	 *
   323	 * Return:
   324	 *  0 on success
   325	 *  -ENOMEM if allocating the cache fails
   326	 *  -EINVAL if gaddr is not aligned to 1MB
   327	 *  -EINVAL if buff_user_len is not aligned to uv_info.conf_dump_storage_state_len
   328	 *  -EINVAL if the UV call fails, rc and rrc will be set in this case
   329	 *  -EFAULT if copying the result to buff_user failed
   330	 */
   331	int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
   332					u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc)
   333	{
   334		struct uv_cb_dump_stor_state uvcb = {
   335			.header.cmd = UVC_CMD_DUMP_CONF_STOR_STATE,
   336			.header.len = sizeof(uvcb),
   337			.config_handle = kvm->arch.pv.handle,
   338			.gaddr = *gaddr,
   339			.dump_area_origin = 0,
   340		};
   341		size_t buff_kvm_size;
   342		size_t size_done = 0;
   343		u8 *buff_kvm = NULL;
   344		int cc, ret;
   345	
   346		ret = -EINVAL;
   347		/* UV call processes 1MB guest storage chunks at a time */
   348		if (*gaddr & ~HPAGE_MASK)
   349			goto out;
   350	
   351		/*
   352		 * We provide the storage state for 1MB chunks of guest
   353		 * storage. The buffer will need to be aligned to
   354		 * conf_dump_storage_state_len so we don't end on a partial
   355		 * chunk.
   356		 */
   357		if (!buff_user_len ||
   358		    buff_user_len & (uv_info.conf_dump_storage_state_len - 1))
   359			goto out;
   360	
   361		/*
   362		 * Allocate a buffer from which we will later copy to the user process.
   363		 *
   364		 * We don't want userspace to dictate our buffer size so we limit it to DUMP_BUFF_LEN.
   365		 */
   366		ret = -ENOMEM;
   367		buff_kvm_size = buff_user_len <= DUMP_BUFF_LEN ? buff_user_len : DUMP_BUFF_LEN;
   368		buff_kvm = vzalloc(buff_kvm_size);
 > 369		if (!buff_kvm)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 6/9] kvm: s390: Add configuration dump functionality
  2022-02-23  9:20 ` [PATCH 6/9] kvm: s390: Add configuration dump functionality Janosch Frank
  2022-02-23 18:13   ` kernel test robot
@ 2022-02-23 19:25   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-02-23 19:25 UTC (permalink / raw)
  To: Janosch Frank, kvm
  Cc: llvm, kbuild-all, linux-s390, imbrenda, david, borntraeger

Hi Janosch,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kvms390/next]
[also build test WARNING on next-20220222]
[cannot apply to kvm/master s390/features v5.17-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git next
config: s390-randconfig-c005-20220223 (https://download.01.org/0day-ci/archive/20220224/202202240358.kEydOdbz-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/63a2029ece7e8fee92aa5ad277e2cbd8b13b7e6b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
        git checkout 63a2029ece7e8fee92aa5ad277e2cbd8b13b7e6b
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash arch/s390/kvm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from arch/s390/kvm/pv.c:9:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   arch/s390/kvm/pv.c:369:6: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!buff_kvm)
               ^~~~~~~~~
   arch/s390/kvm/pv.c:411:6: note: uninitialized use occurs here
           if (cc || ret)
               ^~
   arch/s390/kvm/pv.c:369:2: note: remove the 'if' if its condition is always false
           if (!buff_kvm)
           ^~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:357:6: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!buff_user_len ||
               ^~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:411:6: note: uninitialized use occurs here
           if (cc || ret)
               ^~
   arch/s390/kvm/pv.c:357:2: note: remove the 'if' if its condition is always false
           if (!buff_user_len ||
           ^~~~~~~~~~~~~~~~~~~~~
>> arch/s390/kvm/pv.c:357:6: warning: variable 'cc' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
           if (!buff_user_len ||
               ^~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:411:6: note: uninitialized use occurs here
           if (cc || ret)
               ^~
   arch/s390/kvm/pv.c:357:6: note: remove the '||' if its condition is always false
           if (!buff_user_len ||
               ^~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:348:6: warning: variable 'cc' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (*gaddr & ~HPAGE_MASK)
               ^~~~~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:411:6: note: uninitialized use occurs here
           if (cc || ret)
               ^~
   arch/s390/kvm/pv.c:348:2: note: remove the 'if' if its condition is always false
           if (*gaddr & ~HPAGE_MASK)
           ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/s390/kvm/pv.c:344:8: note: initialize the variable 'cc' to silence this warning
           int cc, ret;
                 ^
                  = 0
   16 warnings generated.


vim +357 arch/s390/kvm/pv.c

   309	
   310	/*
   311	 * kvm_s390_pv_dump_stor_state
   312	 *
   313	 * @kvm: pointer to the guest's KVM struct
   314	 * @buff_user: Userspace pointer where we will write the results to
   315	 * @gaddr: Starting absolute guest address for which the storage state
   316	 *         is requested. This value will be updated with the last
   317	 *         address for which data was written when returning to
   318	 *         userspace.
   319	 * @buff_user_len: Length of the buff_user buffer
   320	 * @rc: Pointer to where the uvcb return code is stored
   321	 * @rrc: Pointer to where the uvcb return reason code is stored
   322	 *
   323	 * Return:
   324	 *  0 on success
   325	 *  -ENOMEM if allocating the cache fails
   326	 *  -EINVAL if gaddr is not aligned to 1MB
   327	 *  -EINVAL if buff_user_len is not aligned to uv_info.conf_dump_storage_state_len
   328	 *  -EINVAL if the UV call fails, rc and rrc will be set in this case
   329	 *  -EFAULT if copying the result to buff_user failed
   330	 */
   331	int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
   332					u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc)
   333	{
   334		struct uv_cb_dump_stor_state uvcb = {
   335			.header.cmd = UVC_CMD_DUMP_CONF_STOR_STATE,
   336			.header.len = sizeof(uvcb),
   337			.config_handle = kvm->arch.pv.handle,
   338			.gaddr = *gaddr,
   339			.dump_area_origin = 0,
   340		};
   341		size_t buff_kvm_size;
   342		size_t size_done = 0;
   343		u8 *buff_kvm = NULL;
   344		int cc, ret;
   345	
   346		ret = -EINVAL;
   347		/* UV call processes 1MB guest storage chunks at a time */
   348		if (*gaddr & ~HPAGE_MASK)
   349			goto out;
   350	
   351		/*
   352		 * We provide the storage state for 1MB chunks of guest
   353		 * storage. The buffer will need to be aligned to
   354		 * conf_dump_storage_state_len so we don't end on a partial
   355		 * chunk.
   356		 */
 > 357		if (!buff_user_len ||

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 7/9] kvm: s390: Add CPU dump functionality
  2022-02-23  9:20 ` [PATCH 7/9] kvm: s390: Add CPU " Janosch Frank
@ 2022-02-23 19:46   ` kernel test robot
  0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2022-02-23 19:46 UTC (permalink / raw)
  To: Janosch Frank, kvm
  Cc: llvm, kbuild-all, linux-s390, imbrenda, david, borntraeger

Hi Janosch,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kvms390/next]
[also build test WARNING on next-20220222]
[cannot apply to kvm/master s390/features v5.17-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git next
config: s390-randconfig-r044-20220223 (https://download.01.org/0day-ci/archive/20220224/202202240337.wksMLgVn-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/8e5a2c1d95d561cf1530c7a3427a1a367ab67364
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Janosch-Frank/kvm-s390-Add-PV-dump-support/20220223-172213
        git checkout 8e5a2c1d95d561cf1530c7a3427a1a367ab67364
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash arch/s390/kvm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/s390/kvm/kvm-s390.c:23:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from arch/s390/kvm/kvm-s390.c:23:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from arch/s390/kvm/kvm-s390.c:23:
   In file included from include/linux/kvm_host.h:41:
   In file included from include/linux/kvm_para.h:5:
   In file included from include/uapi/linux/kvm_para.h:37:
   In file included from arch/s390/include/asm/kvm_para.h:25:
   In file included from arch/s390/include/asm/diag.h:12:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> arch/s390/kvm/kvm-s390.c:5088:5: warning: no previous prototype for function 'kvm_s390_handle_pv_vcpu_dump' [-Wmissing-prototypes]
   int kvm_s390_handle_pv_vcpu_dump(struct kvm_vcpu *vcpu, struct kvm_pv_cmd *cmd)
       ^
   arch/s390/kvm/kvm-s390.c:5088:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int kvm_s390_handle_pv_vcpu_dump(struct kvm_vcpu *vcpu, struct kvm_pv_cmd *cmd)
   ^
   static 
   13 warnings generated.


vim +/kvm_s390_handle_pv_vcpu_dump +5088 arch/s390/kvm/kvm-s390.c

  5087	
> 5088	int kvm_s390_handle_pv_vcpu_dump(struct kvm_vcpu *vcpu, struct kvm_pv_cmd *cmd)
  5089	{
  5090		struct kvm_s390_pv_dmp dmp;
  5091		void *data;
  5092		int ret;
  5093	
  5094		/* Dump initialization is a prerequisite */
  5095		if (!vcpu->kvm->arch.pv.dumping)
  5096			return -EINVAL;
  5097	
  5098		if (copy_from_user(&dmp, (__u8 __user *)cmd->data, sizeof(dmp)))
  5099			return -EFAULT;
  5100	
  5101		/* We only handle this subcmd right now */
  5102		if (dmp.subcmd != KVM_PV_DUMP_CPU)
  5103			return -EINVAL;
  5104	
  5105		/* CPU dump length is the same as create cpu storage donation. */
  5106		if (dmp.buff_len != uv_info.guest_cpu_stor_len)
  5107			return -EINVAL;
  5108	
  5109		data = vzalloc(uv_info.guest_cpu_stor_len);
  5110		if (!data)
  5111			return -ENOMEM;
  5112	
  5113		ret = kvm_s390_pv_dump_cpu(vcpu, data, &cmd->rc, &cmd->rrc);
  5114	
  5115		VCPU_EVENT(vcpu, 3, "PROTVIRT DUMP CPU %d rc %x rrc %x",
  5116			   vcpu->vcpu_id, cmd->rc, cmd->rrc);
  5117	
  5118		if (ret) {
  5119			vfree(data);
  5120			return -EINVAL;
  5121		}
  5122	
  5123		/* On success copy over the dump data */
  5124		if (copy_to_user((__u8 __user *)dmp.buff_addr, data, uv_info.guest_cpu_stor_len)) {
  5125			vfree(data);
  5126			return -EFAULT;
  5127		}
  5128	
  5129		vfree(data);
  5130		return 0;
  5131	}
  5132	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 1/9] s390x: Add SE hdr query information
  2022-02-23  9:19 ` [PATCH 1/9] s390x: Add SE hdr query information Janosch Frank
@ 2022-03-01 17:22   ` Claudio Imbrenda
  0 siblings, 0 replies; 23+ messages in thread
From: Claudio Imbrenda @ 2022-03-01 17:22 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, david, borntraeger

On Wed, 23 Feb 2022 09:19:59 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> We have information about the supported se header version and pcf bits
> so let's expose it via the sysfs files.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  arch/s390/boot/uv.c        |  2 ++
>  arch/s390/include/asm/uv.h |  7 ++++++-
>  arch/s390/kernel/uv.c      | 20 ++++++++++++++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
> index e6be155ab2e5..b100b57cf15d 100644
> --- a/arch/s390/boot/uv.c
> +++ b/arch/s390/boot/uv.c
> @@ -41,6 +41,8 @@ void uv_query_info(void)
>  		uv_info.max_num_sec_conf = uvcb.max_num_sec_conf;
>  		uv_info.max_guest_cpu_id = uvcb.max_guest_cpu_id;
>  		uv_info.uv_feature_indications = uvcb.uv_feature_indications;
> +		uv_info.supp_se_hdr_ver = uvcb.supp_se_hdr_versions;
> +		uv_info.supp_se_hdr_pcf = uvcb.supp_se_hdr_pcf;
>  	}
>  
>  #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 86218382d29c..7d6c78b61bf2 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -107,7 +107,10 @@ struct uv_cb_qui {
>  	u8  reserved88[158 - 136];		/* 0x0088 */
>  	u16 max_guest_cpu_id;			/* 0x009e */
>  	u64 uv_feature_indications;		/* 0x00a0 */
> -	u8  reserveda8[200 - 168];		/* 0x00a8 */
> +	u64 reserveda8;				/* 0x00a8 */
> +	u64 supp_se_hdr_versions;		/* 0x00b0 */
> +	u64 supp_se_hdr_pcf;			/* 0x00b8 */
> +	u64 reservedc0;				/* 0x00c0 */
>  } __packed __aligned(8);
>  
>  /* Initialize Ultravisor */
> @@ -285,6 +288,8 @@ struct uv_info {
>  	unsigned int max_num_sec_conf;
>  	unsigned short max_guest_cpu_id;
>  	unsigned long uv_feature_indications;
> +	unsigned long supp_se_hdr_ver;
> +	unsigned long supp_se_hdr_pcf;
>  };
>  
>  extern struct uv_info uv_info;
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index a5425075dd25..852840384e75 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -392,6 +392,24 @@ static ssize_t uv_query_facilities(struct kobject *kobj,
>  static struct kobj_attribute uv_query_facilities_attr =
>  	__ATTR(facilities, 0444, uv_query_facilities, NULL);
>  
> +static ssize_t uv_query_supp_se_hdr_ver(struct kobject *kobj,
> +					struct kobj_attribute *attr, char *buf)
> +{
> +	return sysfs_emit(buf, "%lx\n", uv_info.supp_se_hdr_ver);
> +}
> +
> +static struct kobj_attribute uv_query_supp_se_hdr_ver_attr =
> +	__ATTR(supp_se_hdr_ver, 0444, uv_query_supp_se_hdr_ver, NULL);
> +
> +static ssize_t uv_query_supp_se_hdr_pcf(struct kobject *kobj,
> +					struct kobj_attribute *attr, char *buf)
> +{
> +	return sysfs_emit(buf, "%lx\n", uv_info.supp_se_hdr_pcf);
> +}
> +
> +static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
> +	__ATTR(supp_se_hdr_pcf, 0444, uv_query_supp_se_hdr_pcf, NULL);
> +
>  static ssize_t uv_query_feature_indications(struct kobject *kobj,
>  					    struct kobj_attribute *attr, char *buf)
>  {
> @@ -437,6 +455,8 @@ static struct attribute *uv_query_attrs[] = {
>  	&uv_query_max_guest_cpus_attr.attr,
>  	&uv_query_max_guest_vms_attr.attr,
>  	&uv_query_max_guest_addr_attr.attr,
> +	&uv_query_supp_se_hdr_ver_attr.attr,
> +	&uv_query_supp_se_hdr_pcf_attr.attr,
>  	NULL,
>  };
>  


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 2/9] s390: uv: Add dump fields to query
  2022-02-23  9:20 ` [PATCH 2/9] s390: uv: Add dump fields to query Janosch Frank
@ 2022-03-01 17:24   ` Claudio Imbrenda
  0 siblings, 0 replies; 23+ messages in thread
From: Claudio Imbrenda @ 2022-03-01 17:24 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, david, borntraeger

On Wed, 23 Feb 2022 09:20:00 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> The new dump feature requires us to know how much memory is needed for
> the "dump storage state" and "dump finalize" ultravisor call. These
> values are reported via the UV query call.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

I would squash this into the previous patch, though

> ---
>  arch/s390/boot/uv.c        |  2 ++
>  arch/s390/include/asm/uv.h |  5 +++++
>  arch/s390/kernel/uv.c      | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
> index b100b57cf15d..67c737c1e580 100644
> --- a/arch/s390/boot/uv.c
> +++ b/arch/s390/boot/uv.c
> @@ -43,6 +43,8 @@ void uv_query_info(void)
>  		uv_info.uv_feature_indications = uvcb.uv_feature_indications;
>  		uv_info.supp_se_hdr_ver = uvcb.supp_se_hdr_versions;
>  		uv_info.supp_se_hdr_pcf = uvcb.supp_se_hdr_pcf;
> +		uv_info.conf_dump_storage_state_len = uvcb.conf_dump_storage_state_len;
> +		uv_info.conf_dump_finalize_len = uvcb.conf_dump_finalize_len;
>  	}
>  
>  #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 7d6c78b61bf2..b79e516d4424 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -111,6 +111,9 @@ struct uv_cb_qui {
>  	u64 supp_se_hdr_versions;		/* 0x00b0 */
>  	u64 supp_se_hdr_pcf;			/* 0x00b8 */
>  	u64 reservedc0;				/* 0x00c0 */
> +	u64 conf_dump_storage_state_len;	/* 0x00c8 */
> +	u64 conf_dump_finalize_len;		/* 0x00d0 */
> +	u8  reservedd8[256 - 216];		/* 0x00d8 */
>  } __packed __aligned(8);
>  
>  /* Initialize Ultravisor */
> @@ -290,6 +293,8 @@ struct uv_info {
>  	unsigned long uv_feature_indications;
>  	unsigned long supp_se_hdr_ver;
>  	unsigned long supp_se_hdr_pcf;
> +	unsigned long conf_dump_storage_state_len;
> +	unsigned long conf_dump_finalize_len;
>  };
>  
>  extern struct uv_info uv_info;
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 852840384e75..84fe33b6af4d 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -410,6 +410,36 @@ static ssize_t uv_query_supp_se_hdr_pcf(struct kobject *kobj,
>  static struct kobj_attribute uv_query_supp_se_hdr_pcf_attr =
>  	__ATTR(supp_se_hdr_pcf, 0444, uv_query_supp_se_hdr_pcf, NULL);
>  
> +static ssize_t uv_query_dump_cpu_len(struct kobject *kobj,
> +				     struct kobj_attribute *attr, char *page)
> +{
> +	return scnprintf(page, PAGE_SIZE, "%lx\n",
> +			uv_info.guest_cpu_stor_len);
> +}
> +
> +static struct kobj_attribute uv_query_dump_cpu_len_attr =
> +	__ATTR(uv_query_dump_cpu_len, 0444, uv_query_dump_cpu_len, NULL);
> +
> +static ssize_t uv_query_dump_storage_state_len(struct kobject *kobj,
> +					       struct kobj_attribute *attr, char *page)
> +{
> +	return scnprintf(page, PAGE_SIZE, "%lx\n",
> +			uv_info.conf_dump_storage_state_len);
> +}
> +
> +static struct kobj_attribute uv_query_dump_storage_state_len_attr =
> +	__ATTR(dump_storage_state_len, 0444, uv_query_dump_storage_state_len, NULL);
> +
> +static ssize_t uv_query_dump_finalize_len(struct kobject *kobj,
> +					  struct kobj_attribute *attr, char *page)
> +{
> +	return scnprintf(page, PAGE_SIZE, "%lx\n",
> +			uv_info.conf_dump_finalize_len);
> +}
> +
> +static struct kobj_attribute uv_query_dump_finalize_len_attr =
> +	__ATTR(dump_finalize_len, 0444, uv_query_dump_finalize_len, NULL);
> +
>  static ssize_t uv_query_feature_indications(struct kobject *kobj,
>  					    struct kobj_attribute *attr, char *buf)
>  {
> @@ -457,6 +487,9 @@ static struct attribute *uv_query_attrs[] = {
>  	&uv_query_max_guest_addr_attr.attr,
>  	&uv_query_supp_se_hdr_ver_attr.attr,
>  	&uv_query_supp_se_hdr_pcf_attr.attr,
> +	&uv_query_dump_storage_state_len_attr.attr,
> +	&uv_query_dump_finalize_len_attr.attr,
> +	&uv_query_dump_cpu_len_attr.attr,
>  	NULL,
>  };
>  


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-02-23 11:30   ` Thomas Huth
  2022-02-23 12:47     ` Janosch Frank
@ 2022-03-01 17:32     ` Claudio Imbrenda
  2022-03-02  9:03       ` Janosch Frank
  1 sibling, 1 reply; 23+ messages in thread
From: Claudio Imbrenda @ 2022-03-01 17:32 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Janosch Frank, kvm, linux-s390, david, borntraeger

On Wed, 23 Feb 2022 12:30:36 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 23/02/2022 10.20, Janosch Frank wrote:
> > Some of the query information is already available via sysfs but
> > having a IOCTL makes the information easier to retrieve.

why not exporting this via sysfs too?

something like a sysfs file called "query_ultravisor_information_raw"

that way you don't even have a problem with sizes

> > 
> > Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> > ---
> >   arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
> >   include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
> >   2 files changed, 70 insertions(+)
> > 
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index faa85397b6fb..837f898ad2ff 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
> >   	return r;
> >   }
> >   
> > +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
> > +{
> > +	u32 len;
> > +
> > +	switch (info->header.id) {
> > +	case KVM_PV_INFO_VM: {
> > +		len =  sizeof(info->header) + sizeof(info->vm);
> > +
> > +		if (info->header.len < len)
> > +			return -EINVAL;
> > +
> > +		memcpy(info->vm.inst_calls_list,
> > +		       uv_info.inst_calls_list,
> > +		       sizeof(uv_info.inst_calls_list));
> > +
> > +		/* It's max cpuidm not max cpus so it's off by one */
> > +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
> > +		info->vm.max_guests = uv_info.max_num_sec_conf;
> > +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
> > +		info->vm.feature_indication = uv_info.uv_feature_indications;
> > +
> > +		return 0;
> > +	}
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> >   static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> >   {
> >   	int r = 0;
> > @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> >   			     cmd->rc, cmd->rrc);
> >   		break;
> >   	}
> > +	case KVM_PV_INFO: {
> > +		struct kvm_s390_pv_info info = {};
> > +
> > +		if (copy_from_user(&info, argp, sizeof(info.header)))
> > +			return -EFAULT;
> > +
> > +		if (info.header.len < sizeof(info.header))
> > +			return -EINVAL;
> > +
> > +		r = kvm_s390_handle_pv_info(&info);
> > +		if (r)
> > +			return r;
> > +
> > +		r = copy_to_user(argp, &info, sizeof(info));  
> 
> sizeof(info) is currently OK ... but this might break if somebody later 
> extends the kvm_s390_pv_info struct, I guess? ==> Maybe also better use 
> sizeof(info->header) + sizeof(info->vm) here, too? Or let 
> kvm_s390_handle_pv_info() return the amount of bytes that should be copied here?
> 
>   Thomas
> 
> 
> > +		if (r)
> > +			return -EFAULT;
> > +		return 0;
> > +	}
> >   	default:
> >   		r = -ENOTTY;
> >   	}
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index dbc550bbd9fa..96fceb204a92 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
> >   	__u64 tweak;
> >   };
> >   
> > +enum pv_cmd_info_id {
> > +	KVM_PV_INFO_VM,
> > +};
> > +
> > +struct kvm_s390_pv_info_vm {
> > +	__u64 inst_calls_list[4];
> > +	__u64 max_cpus;
> > +	__u64 max_guests;
> > +	__u64 max_guest_addr;
> > +	__u64 feature_indication;
> > +};
> > +
> > +struct kvm_s390_pv_info_header {
> > +	__u32 id;
> > +	__u32 len;
> > +};
> > +
> > +struct kvm_s390_pv_info {
> > +	struct kvm_s390_pv_info_header header;
> > +	struct kvm_s390_pv_info_vm vm;
> > +};
> > +
> >   enum pv_cmd_id {
> >   	KVM_PV_ENABLE,
> >   	KVM_PV_DISABLE,
> > @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
> >   	KVM_PV_VERIFY,
> >   	KVM_PV_PREP_RESET,
> >   	KVM_PV_UNSHARE_ALL,
> > +	KVM_PV_INFO,
> >   };
> >   
> >   struct kvm_pv_cmd {  
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 5/9] KVM: s390: pv: Add query dump information
  2022-02-23  9:20 ` [PATCH 5/9] KVM: s390: pv: Add query dump information Janosch Frank
@ 2022-03-01 17:34   ` Claudio Imbrenda
  0 siblings, 0 replies; 23+ messages in thread
From: Claudio Imbrenda @ 2022-03-01 17:34 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, david, borntraeger

On Wed, 23 Feb 2022 09:20:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> The dump API requires userspace to provide buffers into which we will
> store data. The dump information added in this patch tells userspace
> how big those buffers need to be.

isn't this information already exported in sysfs?

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 11 +++++++++++
>  include/uapi/linux/kvm.h | 12 +++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 837f898ad2ff..8de53803c1ca 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2240,6 +2240,17 @@ static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
>  
>  		return 0;
>  	}
> +	case KVM_PV_INFO_DUMP: {
> +		len =  sizeof(info->header) + sizeof(info->dump);
> +
> +		if (info->header.len < len)
> +			return -EINVAL;
> +
> +		info->dump.dump_cpu_buffer_len = uv_info.guest_cpu_stor_len;
> +		info->dump.dump_config_mem_buffer_per_1m = uv_info.conf_dump_storage_state_len;
> +		info->dump.dump_config_finalize_len = uv_info.conf_dump_finalize_len;
> +		return 0;
> +	}
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 96fceb204a92..d58cd5a40e62 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1644,6 +1644,13 @@ struct kvm_s390_pv_unp {
>  
>  enum pv_cmd_info_id {
>  	KVM_PV_INFO_VM,
> +	KVM_PV_INFO_DUMP,
> +};
> +
> +struct kvm_s390_pv_info_dump {
> +	__u64 dump_cpu_buffer_len;
> +	__u64 dump_config_mem_buffer_per_1m;
> +	__u64 dump_config_finalize_len;
>  };
>  
>  struct kvm_s390_pv_info_vm {
> @@ -1661,7 +1668,10 @@ struct kvm_s390_pv_info_header {
>  
>  struct kvm_s390_pv_info {
>  	struct kvm_s390_pv_info_header header;
> -	struct kvm_s390_pv_info_vm vm;
> +	union {
> +		struct kvm_s390_pv_info_dump dump;
> +		struct kvm_s390_pv_info_vm vm;
> +	};
>  };
>  
>  enum pv_cmd_id {


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-03-01 17:32     ` Claudio Imbrenda
@ 2022-03-02  9:03       ` Janosch Frank
  0 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-03-02  9:03 UTC (permalink / raw)
  To: Claudio Imbrenda, Thomas Huth; +Cc: kvm, linux-s390, david, borntraeger

On 3/1/22 18:32, Claudio Imbrenda wrote:
> On Wed, 23 Feb 2022 12:30:36 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 23/02/2022 10.20, Janosch Frank wrote:
>>> Some of the query information is already available via sysfs but
>>> having a IOCTL makes the information easier to retrieve.
> 
> why not exporting this via sysfs too?
> 
> something like a sysfs file called "query_ultravisor_information_raw"
> 
> that way you don't even have a problem with sizes

I'd like to avoid having to rely on reading more files. IOCTLs are the 
most used way to communicate with KVM and don't require larger changes 
to QEMU.

This is information that's meant for the VMM so in this case KVM can 
control what it reports. If I'd add a raw interface then we can't 
control what's being passed to userspace.


Btw. the UV driver by Steffen provides raw QUI data but we can't 
guarantee that it will always be available to QEMU.

> 
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>    arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>>    include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
>>>    2 files changed, 70 insertions(+)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index faa85397b6fb..837f898ad2ff 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>>>    	return r;
>>>    }
>>>    
>>> +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
>>> +{
>>> +	u32 len;
>>> +
>>> +	switch (info->header.id) {
>>> +	case KVM_PV_INFO_VM: {
>>> +		len =  sizeof(info->header) + sizeof(info->vm);
>>> +
>>> +		if (info->header.len < len)
>>> +			return -EINVAL;
>>> +
>>> +		memcpy(info->vm.inst_calls_list,
>>> +		       uv_info.inst_calls_list,
>>> +		       sizeof(uv_info.inst_calls_list));
>>> +
>>> +		/* It's max cpuidm not max cpus so it's off by one */
>>> +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
>>> +		info->vm.max_guests = uv_info.max_num_sec_conf;
>>> +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
>>> +		info->vm.feature_indication = uv_info.uv_feature_indications;
>>> +
>>> +		return 0;
>>> +	}
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +}
>>> +
>>>    static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>>    {
>>>    	int r = 0;
>>> @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>>    			     cmd->rc, cmd->rrc);
>>>    		break;
>>>    	}
>>> +	case KVM_PV_INFO: {
>>> +		struct kvm_s390_pv_info info = {};
>>> +
>>> +		if (copy_from_user(&info, argp, sizeof(info.header)))
>>> +			return -EFAULT;
>>> +
>>> +		if (info.header.len < sizeof(info.header))
>>> +			return -EINVAL;
>>> +
>>> +		r = kvm_s390_handle_pv_info(&info);
>>> +		if (r)
>>> +			return r;
>>> +
>>> +		r = copy_to_user(argp, &info, sizeof(info));
>>
>> sizeof(info) is currently OK ... but this might break if somebody later
>> extends the kvm_s390_pv_info struct, I guess? ==> Maybe also better use
>> sizeof(info->header) + sizeof(info->vm) here, too? Or let
>> kvm_s390_handle_pv_info() return the amount of bytes that should be copied here?
>>
>>    Thomas
>>
>>
>>> +		if (r)
>>> +			return -EFAULT;
>>> +		return 0;
>>> +	}
>>>    	default:
>>>    		r = -ENOTTY;
>>>    	}
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index dbc550bbd9fa..96fceb204a92 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
>>>    	__u64 tweak;
>>>    };
>>>    
>>> +enum pv_cmd_info_id {
>>> +	KVM_PV_INFO_VM,
>>> +};
>>> +
>>> +struct kvm_s390_pv_info_vm {
>>> +	__u64 inst_calls_list[4];
>>> +	__u64 max_cpus;
>>> +	__u64 max_guests;
>>> +	__u64 max_guest_addr;
>>> +	__u64 feature_indication;
>>> +};
>>> +
>>> +struct kvm_s390_pv_info_header {
>>> +	__u32 id;
>>> +	__u32 len;
>>> +};
>>> +
>>> +struct kvm_s390_pv_info {
>>> +	struct kvm_s390_pv_info_header header;
>>> +	struct kvm_s390_pv_info_vm vm;
>>> +};
>>> +
>>>    enum pv_cmd_id {
>>>    	KVM_PV_ENABLE,
>>>    	KVM_PV_DISABLE,
>>> @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
>>>    	KVM_PV_VERIFY,
>>>    	KVM_PV_PREP_RESET,
>>>    	KVM_PV_UNSHARE_ALL,
>>> +	KVM_PV_INFO,
>>>    };
>>>    
>>>    struct kvm_pv_cmd {
>>
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-02-23  9:20 ` [PATCH 3/9] KVM: s390: pv: Add query interface Janosch Frank
  2022-02-23 11:30   ` Thomas Huth
@ 2022-03-02 12:04   ` Claudio Imbrenda
  2022-03-02 12:41     ` Janosch Frank
  1 sibling, 1 reply; 23+ messages in thread
From: Claudio Imbrenda @ 2022-03-02 12:04 UTC (permalink / raw)
  To: Janosch Frank; +Cc: kvm, linux-s390, david, borntraeger

On Wed, 23 Feb 2022 09:20:01 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Some of the query information is already available via sysfs but
> having a IOCTL makes the information easier to retrieve.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index faa85397b6fb..837f898ad2ff 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>  	return r;
>  }
>  
> +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
> +{
> +	u32 len;
> +
> +	switch (info->header.id) {
> +	case KVM_PV_INFO_VM: {
> +		len =  sizeof(info->header) + sizeof(info->vm);
> +
> +		if (info->header.len < len)
> +			return -EINVAL;

so if userspace gives a smaller buffer, we fail?
this means that if the struct grows in the future, existing software
will break?

> +
> +		memcpy(info->vm.inst_calls_list,
> +		       uv_info.inst_calls_list,
> +		       sizeof(uv_info.inst_calls_list));
> +
> +		/* It's max cpuidm not max cpus so it's off by one */
> +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
> +		info->vm.max_guests = uv_info.max_num_sec_conf;
> +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
> +		info->vm.feature_indication = uv_info.uv_feature_indications;
> +
> +		return 0;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>  {
>  	int r = 0;
> @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>  			     cmd->rc, cmd->rrc);
>  		break;
>  	}
> +	case KVM_PV_INFO: {
> +		struct kvm_s390_pv_info info = {};
> +
> +		if (copy_from_user(&info, argp, sizeof(info.header)))
> +			return -EFAULT;
> +
> +		if (info.header.len < sizeof(info.header))
> +			return -EINVAL;
> +
> +		r = kvm_s390_handle_pv_info(&info);
> +		if (r)
> +			return r;
> +
> +		r = copy_to_user(argp, &info, sizeof(info));
> +
> +		if (r)
> +			return -EFAULT;
> +		return 0;
> +	}
>  	default:
>  		r = -ENOTTY;
>  	}
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index dbc550bbd9fa..96fceb204a92 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
>  	__u64 tweak;
>  };
>  
> +enum pv_cmd_info_id {
> +	KVM_PV_INFO_VM,
> +};
> +
> +struct kvm_s390_pv_info_vm {
> +	__u64 inst_calls_list[4];
> +	__u64 max_cpus;
> +	__u64 max_guests;
> +	__u64 max_guest_addr;
> +	__u64 feature_indication;
> +};
> +
> +struct kvm_s390_pv_info_header {
> +	__u32 id;
> +	__u32 len;
> +};
> +
> +struct kvm_s390_pv_info {
> +	struct kvm_s390_pv_info_header header;
> +	struct kvm_s390_pv_info_vm vm;
> +};
> +
>  enum pv_cmd_id {
>  	KVM_PV_ENABLE,
>  	KVM_PV_DISABLE,
> @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
>  	KVM_PV_VERIFY,
>  	KVM_PV_PREP_RESET,
>  	KVM_PV_UNSHARE_ALL,
> +	KVM_PV_INFO,
>  };
>  
>  struct kvm_pv_cmd {


^ permalink raw reply	[flat|nested] 23+ messages in thread
* Re: [PATCH 3/9] KVM: s390: pv: Add query interface
  2022-03-02 12:04   ` Claudio Imbrenda
@ 2022-03-02 12:41     ` Janosch Frank
  0 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-03-02 12:41 UTC (permalink / raw)
  To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, borntraeger

On 3/2/22 13:04, Claudio Imbrenda wrote:
> On Wed, 23 Feb 2022 09:20:01 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Some of the query information is already available via sysfs but
>> having a IOCTL makes the information easier to retrieve.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   arch/s390/kvm/kvm-s390.c | 47 ++++++++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/kvm.h | 23 ++++++++++++++++++++
>>   2 files changed, 70 insertions(+)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index faa85397b6fb..837f898ad2ff 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2217,6 +2217,34 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>>   	return r;
>>   }
>>   
>> +static int kvm_s390_handle_pv_info(struct kvm_s390_pv_info *info)
>> +{
>> +	u32 len;
>> +
>> +	switch (info->header.id) {
>> +	case KVM_PV_INFO_VM: {
>> +		len =  sizeof(info->header) + sizeof(info->vm);
>> +
>> +		if (info->header.len < len)
>> +			return -EINVAL;
> 
> so if userspace gives a smaller buffer, we fail?
> this means that if the struct grows in the future, existing software
> will break?

I've already answered this.
If we extend the struct, we can make this the lower bound and store a 
maximum of info->header.len or the length of the extended struct.

I.e. it would work like the QUI 0x100 rc.
Or we can add a new IOCTL which gives the KVM_PV_INFO_VM + new values.


The more interesting question is how we indicate more data and new IOCTL 
commands. Do we always bind them to a capability? Should we add a query 
in front of the KVM_PV_INFO_VM call which tells us the available calls?

> 
>> +
>> +		memcpy(info->vm.inst_calls_list,
>> +		       uv_info.inst_calls_list,
>> +		       sizeof(uv_info.inst_calls_list));
>> +
>> +		/* It's max cpuidm not max cpus so it's off by one */
>> +		info->vm.max_cpus = uv_info.max_guest_cpu_id + 1;
>> +		info->vm.max_guests = uv_info.max_num_sec_conf;
>> +		info->vm.max_guest_addr = uv_info.max_sec_stor_addr;
>> +		info->vm.feature_indication = uv_info.uv_feature_indications;
>> +
>> +		return 0;
>> +	}
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>   static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>   {
>>   	int r = 0;
>> @@ -2353,6 +2381,25 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>>   			     cmd->rc, cmd->rrc);
>>   		break;
>>   	}
>> +	case KVM_PV_INFO: {
>> +		struct kvm_s390_pv_info info = {};
>> +
>> +		if (copy_from_user(&info, argp, sizeof(info.header)))
>> +			return -EFAULT;
>> +
>> +		if (info.header.len < sizeof(info.header))
>> +			return -EINVAL;
>> +
>> +		r = kvm_s390_handle_pv_info(&info);
>> +		if (r)
>> +			return r;
>> +
>> +		r = copy_to_user(argp, &info, sizeof(info));
>> +
>> +		if (r)
>> +			return -EFAULT;
>> +		return 0;
>> +	}
>>   	default:
>>   		r = -ENOTTY;
>>   	}
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index dbc550bbd9fa..96fceb204a92 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1642,6 +1642,28 @@ struct kvm_s390_pv_unp {
>>   	__u64 tweak;
>>   };
>>   
>> +enum pv_cmd_info_id {
>> +	KVM_PV_INFO_VM,
>> +};
>> +
>> +struct kvm_s390_pv_info_vm {
>> +	__u64 inst_calls_list[4];
>> +	__u64 max_cpus;
>> +	__u64 max_guests;
>> +	__u64 max_guest_addr;
>> +	__u64 feature_indication;
>> +};
>> +
>> +struct kvm_s390_pv_info_header {
>> +	__u32 id;
>> +	__u32 len;
>> +};
>> +
>> +struct kvm_s390_pv_info {
>> +	struct kvm_s390_pv_info_header header;
>> +	struct kvm_s390_pv_info_vm vm;
>> +};
>> +
>>   enum pv_cmd_id {
>>   	KVM_PV_ENABLE,
>>   	KVM_PV_DISABLE,
>> @@ -1650,6 +1672,7 @@ enum pv_cmd_id {
>>   	KVM_PV_VERIFY,
>>   	KVM_PV_PREP_RESET,
>>   	KVM_PV_UNSHARE_ALL,
>> +	KVM_PV_INFO,
>>   };
>>   
>>   struct kvm_pv_cmd {
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH 0/9] kvm: s390: Add PV dump support
@ 2022-04-28 13:00 Janosch Frank
  0 siblings, 0 replies; 23+ messages in thread
From: Janosch Frank @ 2022-04-28 13:00 UTC (permalink / raw)
  To: kvm; +Cc: linux-s390, borntraeger, imbrenda

Sometimes dumping inside of a VM fails, is unavailable or doesn't
yield the required data. For these occasions we dump the VM from the
outside, writing memory and cpu data to a file.

Up to now PV guests only supported dumping from the inside of the
guest through dumpers like KDUMP. A PV guest can be dumped from the
hypervisor but the data will be stale and / or encrypted.

To get the actual state of the PV VM we need the help of the
Ultravisor who safeguards the VM state. New UV calls have been added
to initialize the dump, dump storage state data, dump cpu data and
complete the dump process.

I chose not to document the dump data provided by the Ultravisor since
KVM doesn't interprete it in any way. We're currently searching for a
location and enough cycles to make it available to all.

v4:
	* Rebased and fixed up conflicts due to the Documentation
          changes and new KVM capabilities
	* Fixed the dump facility check, now we check for all 4 calls

v3:
	* Added Rev-by
	* Renamed the query function's len variables to len_min

v2:
	* Added vcpu SIE blocking to avoid validities
	* Moved the KVM CAP to patch #7
	* Renamed len to len_max and introduced len_written for extendability
	* Added Rev-bys


Janosch Frank (9):
  s390x: Add SE hdr query information
  s390: uv: Add dump fields to query
  KVM: s390: pv: Add query interface
  KVM: s390: pv: Add dump support definitions
  KVM: s390: pv: Add query dump information
  kvm: s390: Add configuration dump functionality
  kvm: s390: Add CPU dump functionality
  Documentation: virt: Protected virtual machine dumps
  Documentation/virt/kvm/api.rst: Add protvirt dump/info api
    descriptions

 Documentation/virt/kvm/api.rst               | 152 ++++++++-
 Documentation/virt/kvm/s390/index.rst        |   1 +
 Documentation/virt/kvm/s390/s390-pv-dump.rst |  60 ++++
 arch/s390/boot/uv.c                          |   4 +
 arch/s390/include/asm/kvm_host.h             |   1 +
 arch/s390/include/asm/uv.h                   |  45 ++-
 arch/s390/kernel/uv.c                        |  53 ++++
 arch/s390/kvm/kvm-s390.c                     | 306 +++++++++++++++++++
 arch/s390/kvm/kvm-s390.h                     |   3 +
 arch/s390/kvm/pv.c                           | 131 ++++++++
 include/uapi/linux/kvm.h                     |  55 ++++
 11 files changed, 808 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/virt/kvm/s390/s390-pv-dump.rst

-- 
2.32.0


^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-04-28 13:05 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-23  9:19 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank
2022-02-23  9:19 ` [PATCH 1/9] s390x: Add SE hdr query information Janosch Frank
2022-03-01 17:22   ` Claudio Imbrenda
2022-02-23  9:20 ` [PATCH 2/9] s390: uv: Add dump fields to query Janosch Frank
2022-03-01 17:24   ` Claudio Imbrenda
2022-02-23  9:20 ` [PATCH 3/9] KVM: s390: pv: Add query interface Janosch Frank
2022-02-23 11:30   ` Thomas Huth
2022-02-23 12:47     ` Janosch Frank
2022-03-01 17:32     ` Claudio Imbrenda
2022-03-02  9:03       ` Janosch Frank
2022-03-02 12:04   ` Claudio Imbrenda
2022-03-02 12:41     ` Janosch Frank
2022-02-23  9:20 ` [PATCH 4/9] KVM: s390: pv: Add dump support definitions Janosch Frank
2022-02-23  9:20 ` [PATCH 5/9] KVM: s390: pv: Add query dump information Janosch Frank
2022-03-01 17:34   ` Claudio Imbrenda
2022-02-23  9:20 ` [PATCH 6/9] kvm: s390: Add configuration dump functionality Janosch Frank
2022-02-23 18:13   ` kernel test robot
2022-02-23 19:25   ` kernel test robot
2022-02-23  9:20 ` [PATCH 7/9] kvm: s390: Add CPU " Janosch Frank
2022-02-23 19:46   ` kernel test robot
2022-02-23  9:20 ` [PATCH 8/9] Documentation: virt: Protected virtual machine dumps Janosch Frank
2022-02-23  9:20 ` [PATCH 9/9] Documentation/virt/kvm/api.rst: Add protvirt dump/info api descriptions Janosch Frank
2022-04-28 13:00 [PATCH 0/9] kvm: s390: Add PV dump support Janosch Frank

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).