| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <60d0d472-ca46-7a34-c1a2-b4342a4eb9f7@linux.ibm.com>
Date: Wed, 15 Jun 2022 12:58:00 +0200
From: Janosch Frank <frankja@...ux.ibm.com>
To: Claudio Imbrenda <imbrenda@...ux.ibm.com>, kvm@...r.kernel.org
Cc: borntraeger@...ibm.com, thuth@...hat.com, pasic@...ux.ibm.com,
david@...hat.com, linux-s390@...r.kernel.org,
linux-kernel@...r.kernel.org, scgl@...ux.ibm.com,
mimu@...ux.ibm.com, nrb@...ux.ibm.com
Subject: Re: [PATCH v11 15/19] KVM: s390: pv: asynchronous destroy for reboot
On 6/3/22 08:56, Claudio Imbrenda wrote:
> Until now, destroying a protected guest was an entirely synchronous
> operation that could potentially take a very long time, depending on
> the size of the guest, due to the time needed to clean up the address
> space from protected pages.
>
> This patch implements an asynchronous destroy mechanism, that allows a
> protected guest to reboot significantly faster than previously.
>
> This is achieved by clearing the pages of the old guest in background.
> In case of reboot, the new guest will be able to run in the same
> address space almost immediately.
>
> The old protected guest is then only destroyed when all of its memory has
> been destroyed or otherwise made non protected.
>
> Two new PV commands are added for the KVM_S390_PV_COMMAND ioctl:
>
> KVM_PV_ASYNC_DISABLE_PREPARE: prepares the current protected VM for
> asynchronous teardown. The current VM will then continue immediately
> as non-protected. If a protected VM had already been set aside without
> starting the teardown process, this call will fail.
>
> KVM_PV_ASYNC_DISABLE: tears down the protected VM previously set aside
> for asynchronous teardown. This PV command should ideally be issued by
> userspace from a separate thread. If a fatal signal is received (or the
> process terminates naturally), the command will terminate immediately
> without completing.
>
> Leftover protected VMs are cleaned up when a KVM VM is torn down
> normally (either via IOCTL or when the process terminates); this
> cleanup has been implemented in a previous patch.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@...ux.ibm.com>
> ---
> arch/s390/kvm/kvm-s390.c | 34 +++++++++-
> arch/s390/kvm/kvm-s390.h | 2 +
> arch/s390/kvm/pv.c | 131 +++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 2 +
> 4 files changed, 166 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 369de8377116..842419092c0c 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2256,9 +2256,13 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>
> static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> {
> + const bool needslock = (cmd->cmd != KVM_PV_ASYNC_DISABLE);
> + void __user *argp = (void __user *)cmd->data;
> int r = 0;
> u16 dummy;
> - void __user *argp = (void __user *)cmd->data;
> +
> + if (needslock)
> + mutex_lock(&kvm->lock);
>
> switch (cmd->cmd) {
> case KVM_PV_ENABLE: {
> @@ -2292,6 +2296,28 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> set_bit(IRQ_PEND_EXT_SERVICE, &kvm->arch.float_int.masked_irqs);
> break;
> }
> + case KVM_PV_ASYNC_DISABLE_PREPARE:
> + r = -EINVAL;
> + if (!kvm_s390_pv_is_protected(kvm) || !async_destroy)
> + break;
> +
> + r = kvm_s390_cpus_from_pv(kvm, &cmd->rc, &cmd->rrc);
> + /*
> + * If a CPU could not be destroyed, destroy VM will also fail.
> + * There is no point in trying to destroy it. Instead return
> + * the rc and rrc from the first CPU that failed destroying.
> + */
> + if (r)
> + break;
> + r = kvm_s390_pv_deinit_vm_async_prepare(kvm, &cmd->rc, &cmd->rrc);
> +
> + /* no need to block service interrupts any more */
> + clear_bit(IRQ_PEND_EXT_SERVICE, &kvm->arch.float_int.masked_irqs);
> + break;
> + case KVM_PV_ASYNC_DISABLE:
> + /* This must not be called while holding kvm->lock */
> + r = kvm_s390_pv_deinit_vm_async(kvm, &cmd->rc, &cmd->rrc);
> + break;
> case KVM_PV_DISABLE: {
> r = -EINVAL;
> if (!kvm_s390_pv_is_protected(kvm))
> @@ -2393,6 +2419,9 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> default:
> r = -ENOTTY;
> }
> + if (needslock)
> + mutex_unlock(&kvm->lock);
> +
> return r;
> }
>
> @@ -2597,9 +2626,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = -EINVAL;
> break;
> }
> - mutex_lock(&kvm->lock);
> + /* must be called without kvm->lock */
> r = kvm_s390_handle_pv(kvm, &args);
> - mutex_unlock(&kvm->lock);
> if (copy_to_user(argp, &args, sizeof(args))) {
> r = -EFAULT;
> break;
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index d3abedafa7a8..d296afb6041c 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -243,6 +243,8 @@ static inline u32 kvm_s390_get_gisa_desc(struct kvm *kvm)
> /* implemented in pv.c */
> int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> +int kvm_s390_pv_deinit_vm_async_prepare(struct kvm *kvm, u16 *rc, u16 *rrc);
> +int kvm_s390_pv_deinit_vm_async(struct kvm *kvm, u16 *rc, u16 *rrc);
> int kvm_s390_pv_deinit_vm(struct kvm *kvm, u16 *rc, u16 *rrc);
> int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc);
> int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index 8471c17d538c..ab06fa366e49 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -279,6 +279,137 @@ int kvm_s390_pv_deinit_vm(struct kvm *kvm, u16 *rc, u16 *rrc)
> return cc ? -EIO : 0;
> }
>
> +/**
> + * kvm_s390_destroy_lower_2g - Destroy the first 2GB of protected guest memory.
> + * @kvm the VM whose memory is to be cleared.
> + * Destroy the first 2GB of guest memory, to avoid prefix issues after reboot.
> + */
> +static void kvm_s390_destroy_lower_2g(struct kvm *kvm)
> +{
> + struct kvm_memory_slot *slot;
> + unsigned long lim;
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&kvm->srcu);
> +
> + /* Take the memslot containing guest absolute address 0 */
> + slot = gfn_to_memslot(kvm, 0);
> + /* Clear all slots that are completely below 2GB */
> + while (slot && slot->base_gfn + slot->npages < SZ_2G / PAGE_SIZE) {
> + lim = slot->userspace_addr + slot->npages * PAGE_SIZE;
> + s390_uv_destroy_range(kvm->mm, slot->userspace_addr, lim);
> + /* Take the next memslot */
> + slot = gfn_to_memslot(kvm, slot->base_gfn + slot->npages);
> + }
> + /* Last slot crosses the 2G boundary, clear only up to 2GB */
> + if (slot && slot->base_gfn < SZ_2G / PAGE_SIZE) {
> + lim = slot->userspace_addr + SZ_2G - slot->base_gfn * PAGE_SIZE;
> + s390_uv_destroy_range(kvm->mm, slot->userspace_addr, lim);
> + }
Any reason why you split that up instead of always calculating a length
and using MIN()?
Powered by blists - more mailing lists