On 5/9/22 19:51, Claudio Imbrenda wrote:
On Thu, 28 Apr 2022 13:00:59 +0000
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
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@xxxxxxxxxxxxx>
---
[...]
+/*
+ * 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))
why not use the IS_ALIGNED macro?
Habits.
I'll fix this one and the one above.
+ 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;
This will be converted to min() to make it more readable.
+ 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 */
are you getting a page or a block of size conf_dump_storage_state_len ?
Will be changed to:
/* Get 1MB worth of guest storage state data */
I think that comment has historical reasons, we once discussed to always
write one page instead of the "one 1MB worth of guest storage state".
+ 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;
same here ^ (should it be -= uv_info.conf_dump_storage_state_len ?)
Yes
+
+ /* KVM Buffer full, time to copy to the process */
+ if (!buff_user_len ||
+ uvcb.dump_area_origin == (uintptr_t)buff_kvm +
buff_kvm_size) { +
why not ... || size_done == DUMP_BUFF_LEN ?
+ if (copy_to_user(buff_user, buff_kvm,
+ uvcb.dump_area_origin -
(uintptr_t)buff_kvm)) {
aren't you trying to copy size_done bytes?
+ 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 (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 ret;
+}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2eba89d7ec29..b34850907291 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1645,6 +1645,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,
@@ -1688,6 +1702,7 @@ enum pv_cmd_id {
KVM_PV_PREP_RESET,
KVM_PV_UNSHARE_ALL,
KVM_PV_INFO,
+ KVM_PV_DUMP,
};
struct kvm_pv_cmd {
