[PATCH 5/5] kvm-s390: streamline memslot handling - v4

ehrhardt
2009-05-28T15:43:08Z

*updates in v4*
- kickout only scheduled vcpus (its superfluous and wait might hang forever on
not running vcpus)

*updates in v3*
- handling the mmu reload vcpu request can now be handled inside the sigp
handling avoiding an addtional exit
- kvm_arch_set_memory_region now waits for kicked vcpu's to consume the request
bit it set to ensure that after the kvm_arch_set_memory_region call all vcpus
use the updated memory information

*updates in v2*
- added optimization to skip (addtional) kickout of vcpu's that had the request
already set.

This patch relocates the variables kvm-s390 uses to track guest mem addr/size.
As discussed dropping the variables at struct kvm_arch level allows to use the
common vcpu->request based mechanism to reload guest memory if e.g. changes
via set_memory_region.
The kick mechanism introduced in this series is used to ensure running vcpus
leave guest state to catch the update.

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
include/asm/kvm_host.h | 4 ---
kvm/gaccess.h | 23 ++++++++++---------
kvm/intercept.c | 6 ++---
kvm/kvm-s390.c | 57 ++++++++++++++++++++-----------------------------
kvm/kvm-s390.h | 33 +++++++++++++++++++++++++++-
kvm/sigp.c | 4 +--
6 files changed, 74 insertions(+), 53 deletions(-)

[diff]
Index: kvm/arch/s390/kvm/gaccess.h
===================================================================
--- kvm.orig/arch/s390/kvm/gaccess.h
+++ kvm/arch/s390/kvm/gaccess.h
@@ -1,7 +1,7 @@
/*
* gaccess.h - access guest memory
*
- * Copyright IBM Corp. 2008
+ * Copyright IBM Corp. 2008,2009
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2 only)
@@ -16,13 +16,14 @@
#include <linux/compiler.h>
#include <linux/kvm_host.h>
#include <asm/uaccess.h>
+#include "kvm-s390.h"

static inline void __user *__guestaddr_to_user(struct kvm_vcpu *vcpu,
unsigned long guestaddr)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
- unsigned long origin = vcpu->kvm->arch.guest_origin;
- unsigned long memsize = vcpu->kvm->arch.guest_memsize;
+ unsigned long origin = vcpu->arch.sie_block->gmsor;
+ unsigned long memsize = kvm_s390_vcpu_get_memsize(vcpu);

if (guestaddr < 2 * PAGE_SIZE)
guestaddr += prefix;
@@ -158,8 +159,8 @@ static inline int copy_to_guest(struct k
const void *from, unsigned long n)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
- unsigned long origin = vcpu->kvm->arch.guest_origin;
- unsigned long memsize = vcpu->kvm->arch.guest_memsize;
+ unsigned long origin = vcpu->arch.sie_block->gmsor;
+ unsigned long memsize = kvm_s390_vcpu_get_memsize(vcpu);

if ((guestdest < 2 * PAGE_SIZE) && (guestdest + n > 2 * PAGE_SIZE))
goto slowpath;
@@ -209,8 +210,8 @@ static inline int copy_from_guest(struct
unsigned long guestsrc, unsigned long n)
{
unsigned long prefix = vcpu->arch.sie_block->prefix;
- unsigned long origin = vcpu->kvm->arch.guest_origin;
- unsigned long memsize = vcpu->kvm->arch.guest_memsize;
+ unsigned long origin = vcpu->arch.sie_block->gmsor;
+ unsigned long memsize = kvm_s390_vcpu_get_memsize(vcpu);

if ((guestsrc < 2 * PAGE_SIZE) && (guestsrc + n > 2 * PAGE_SIZE))
goto slowpath;
@@ -244,8 +245,8 @@ static inline int copy_to_guest_absolute
unsigned long guestdest,
const void *from, unsigned long n)
{
- unsigned long origin = vcpu->kvm->arch.guest_origin;
- unsigned long memsize = vcpu->kvm->arch.guest_memsize;
+ unsigned long origin = vcpu->arch.sie_block->gmsor;
+ unsigned long memsize = kvm_s390_vcpu_get_memsize(vcpu);

if (guestdest + n > memsize)
return -EFAULT;
@@ -262,8 +263,8 @@ static inline int copy_from_guest_absolu
unsigned long guestsrc,
unsigned long n)
{
- unsigned long origin = vcpu->kvm->arch.guest_origin;
- unsigned long memsize = vcpu->kvm->arch.guest_memsize;
+ unsigned long origin = vcpu->arch.sie_block->gmsor;
+ unsigned long memsize = kvm_s390_vcpu_get_memsize(vcpu);

if (guestsrc + n > memsize)
return -EFAULT;
Index: kvm/arch/s390/kvm/intercept.c
===================================================================
--- kvm.orig/arch/s390/kvm/intercept.c
+++ kvm/arch/s390/kvm/intercept.c
@@ -1,7 +1,7 @@
/*
* intercept.c - in-kernel handling for sie intercepts
*
- * Copyright IBM Corp. 2008
+ * Copyright IBM Corp. 2008,2009
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2 only)
@@ -166,9 +166,9 @@ static int handle_validity(struct kvm_vc

vcpu->stat.exit_validity++;
if ((viwhy == 0x37) && (vcpu->arch.sie_block->prefix
- <= vcpu->kvm->arch.guest_memsize - 2*PAGE_SIZE)){
+ <= kvm_s390_vcpu_get_memsize(vcpu) - 2*PAGE_SIZE)) {
rc = fault_in_pages_writeable((char __user *)
- vcpu->kvm->arch.guest_origin +
+ vcpu->arch.sie_block->gmsor +
vcpu->arch.sie_block->prefix,
2*PAGE_SIZE);
if (rc)
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -1,7 +1,7 @@
/*
* s390host.c -- hosting zSeries kernel virtual machines
*
- * Copyright IBM Corp. 2008
+ * Copyright IBM Corp. 2008,2009
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2 only)
@@ -10,6 +10,7 @@
* Author(s): Carsten Otte <cotte@de.ibm.com>
* Christian Borntraeger <borntraeger@de.ibm.com>
* Heiko Carstens <heiko.carstens@de.ibm.com>
+ * Christian Ehrhardt <ehrhardt@de.ibm.com>
*/

#include <linux/compiler.h>
@@ -278,16 +279,10 @@ static void kvm_s390_vcpu_initial_reset(
vcpu->arch.sie_block->gbea = 1;
}

-/* The current code can have up to 256 pages for virtio */
-#define VIRTIODESCSPACE (256ul * 4096ul)
-
int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
{
atomic_set(&vcpu->arch.sie_block->cpuflags, CPUSTAT_ZARCH);
- vcpu->arch.sie_block->gmslm = vcpu->kvm->arch.guest_memsize +
- vcpu->kvm->arch.guest_origin +
- VIRTIODESCSPACE - 1ul;
- vcpu->arch.sie_block->gmsor = vcpu->kvm->arch.guest_origin;
+ set_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests);
vcpu->arch.sie_block->ecb = 2;
vcpu->arch.sie_block->eca = 0xC1002001U;
hrtimer_init(&vcpu->arch.ckc_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
@@ -493,8 +488,9 @@ rerun_vcpu:
kvm_s390_handle_vcpu_requests(vcpu, VCPUREQUESTLVL_VCPURUN);

/* verify, that memory has been registered */
- if (!vcpu->kvm->arch.guest_memsize) {
+ if (!vcpu->arch.sie_block->gmslm) {
vcpu_put(vcpu);
+ VCPU_EVENT(vcpu, 3, "%s", "no memory registered to run vcpu");
return -EINVAL;
}

@@ -677,6 +673,12 @@ long kvm_arch_vcpu_ioctl(struct file *fi
return -EINVAL;
}

+static int wait_bit_schedule(void *word)
+{
+ schedule();
+ return 0;
+}
+
/* Section: memory related */
int kvm_arch_set_memory_region(struct kvm *kvm,
struct kvm_userspace_memory_region *mem,
@@ -684,6 +686,7 @@ int kvm_arch_set_memory_region(struct kv
int user_alloc)
{
int i;
+ struct kvm_vcpu *vcpu;

/* A few sanity checks. We can have exactly one memory slot which has
to start at guest virtual zero and which has to be located at a
@@ -692,7 +695,7 @@ int kvm_arch_set_memory_region(struct kv
vmas. It is okay to mmap() and munmap() stuff in this slot after
doing this call at any time */

- if (mem->slot || kvm->arch.guest_memsize)
+ if (mem->slot)
return -EINVAL;

if (mem->guest_phys_addr)
@@ -707,36 +710,24 @@ int kvm_arch_set_memory_region(struct kv
if (!user_alloc)
return -EINVAL;

- /* lock all vcpus */
+ /* request update of sie control block for all available vcpus */
for (i = 0; i < KVM_MAX_VCPUS; ++i) {
- if (!kvm->vcpus[i])
+ vcpu = kvm->vcpus[i];
+ if (!vcpu)
+ continue;
+
+ if (!test_and_set_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests))
continue;
- if (!mutex_trylock(&kvm->vcpus[i]->mutex))
- goto fail_out;
- }

- kvm->arch.guest_origin = mem->userspace_addr;
- kvm->arch.guest_memsize = mem->memory_size;
+ if (vcpu->cpu == -1)
+ continue;

- /* update sie control blocks, and unlock all vcpus */
- for (i = 0; i < KVM_MAX_VCPUS; ++i) {
- if (kvm->vcpus[i]) {
- kvm->vcpus[i]->arch.sie_block->gmsor =
- kvm->arch.guest_origin;
- kvm->vcpus[i]->arch.sie_block->gmslm =
- kvm->arch.guest_memsize +
- kvm->arch.guest_origin +
- VIRTIODESCSPACE - 1ul;
- mutex_unlock(&kvm->vcpus[i]->mutex);
- }
+ kvm_s390_inject_sigp_stop(vcpu, ACTION_VCPUREQUEST_ON_STOP);
+ wait_on_bit(&vcpu->requests, KVM_REQ_MMU_RELOAD,
+ wait_bit_schedule, TASK_UNINTERRUPTIBLE);
}

return 0;
-
-fail_out:
- for (; i >= 0; i--)
- mutex_unlock(&kvm->vcpus[i]->mutex);
- return -EINVAL;
}

void kvm_arch_flush_shadow(struct kvm *kvm)
Index: kvm/arch/s390/kvm/kvm-s390.h
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.h
+++ kvm/arch/s390/kvm/kvm-s390.h
@@ -1,7 +1,7 @@
/*
* kvm_s390.h - definition for kvm on s390
*
- * Copyright IBM Corp. 2008
+ * Copyright IBM Corp. 2008,2009
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2 only)
@@ -9,6 +9,7 @@
*
* Author(s): Carsten Otte <cotte@de.ibm.com>
* Christian Borntraeger <borntraeger@de.ibm.com>
+ * Christian Ehrhardt <ehrhardt@de.ibm.com>
*/

#ifndef ARCH_S390_KVM_S390_H
@@ -18,6 +19,9 @@
#include <linux/kvm.h>
#include <linux/kvm_host.h>

+/* The current code can have up to 256 pages for virtio */
+#define VIRTIODESCSPACE (256ul * 4096ul)
+
typedef int (*intercept_handler_t)(struct kvm_vcpu *vcpu);

/* negativ values are error codes, positive values for internal conditions */
@@ -54,6 +58,29 @@ int kvm_s390_inject_vcpu(struct kvm_vcpu
int kvm_s390_inject_program_int(struct kvm_vcpu *vcpu, u16 code);
int kvm_s390_inject_sigp_stop(struct kvm_vcpu *vcpu, int action);

+static inline int kvm_s390_vcpu_get_memsize(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.sie_block->gmslm
+ - vcpu->arch.sie_block->gmsor
+ - VIRTIODESCSPACE + 1ul;
+}
+
+static inline void kvm_s390_vcpu_set_mem(struct kvm_vcpu *vcpu)
+{
+ struct kvm_memory_slot *mem;
+
+ down_read(&vcpu->kvm->slots_lock);
+ mem = &vcpu->kvm->memslots[0];
+
+ vcpu->arch.sie_block->gmsor = mem->userspace_addr;
+ vcpu->arch.sie_block->gmslm =
+ mem->userspace_addr +
+ (mem->npages << PAGE_SHIFT) +
+ VIRTIODESCSPACE - 1ul;
+
+ up_read(&vcpu->kvm->slots_lock);
+}
+
/* interception levels from which handle vcpu requests can be called */
#define VCPUREQUESTLVL_SIGP 1
#define VCPUREQUESTLVL_VCPURUN 2
@@ -65,6 +92,10 @@ static inline unsigned long kvm_s390_han
if (!vcpu->requests)
return 0;

+ /* requests that can be handled at all levels */
+ if (test_and_clear_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests))
+ kvm_s390_vcpu_set_mem(vcpu);
+
return vcpu->requests;
}

Index: kvm/arch/s390/include/asm/kvm_host.h
===================================================================
--- kvm.orig/arch/s390/include/asm/kvm_host.h
+++ kvm/arch/s390/include/asm/kvm_host.h
@@ -1,7 +1,7 @@
/*
* asm-s390/kvm_host.h - definition for kernel virtual machines on s390
*
- * Copyright IBM Corp. 2008
+ * Copyright IBM Corp. 2008,2009
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2 only)
@@ -226,8 +226,6 @@ struct kvm_vm_stat {
};

struct kvm_arch{
- unsigned long guest_origin;
- unsigned long guest_memsize;
struct sca_block *sca;
debug_info_t *dbf;
struct kvm_s390_float_interrupt float_int;
Index: kvm/arch/s390/kvm/sigp.c
===================================================================
--- kvm.orig/arch/s390/kvm/sigp.c
+++ kvm/arch/s390/kvm/sigp.c
@@ -202,9 +202,9 @@ static int __sigp_set_prefix(struct kvm_
/* make sure that the new value is valid memory */
address = address & 0x7fffe000u;
if ((copy_from_guest(vcpu, &tmp,
- (u64) (address + vcpu->kvm->arch.guest_origin) , 1)) ||
+ (u64) (address + vcpu->arch.sie_block->gmsor) , 1)) ||
(copy_from_guest(vcpu, &tmp, (u64) (address +
- vcpu->kvm->arch.guest_origin + PAGE_SIZE), 1))) {
+ vcpu->arch.sie_block->gmsor + PAGE_SIZE), 1))) {
*reg |= SIGP_STAT_INVALID_PARAMETER;
return 1; /* invalid parameter */
}