* [PATCH 1/6] kvm-s390: Fix memory slot versus run - v3
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-12 15:21 ` [PATCH 2/6] kvm-s390: use hrtimer for clock wakeup from idle - v2 ehrhardt
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Carsten Otte <cotte@de.ibm.com>
This patch fixes an incorrectness in the kvm backend for s390.
In case virtual cpus are being created before the corresponding
memory slot is being registered, we need to update the sie
control blocks for the virtual cpus.
*updates in v3*
In consideration of the s390 memslot constraints locking was changed
to trylock. These locks should never be held, as vcpu's can't run without
the single memslot we just assign when running this code. To ensure this
never deadlocks in case other code changes the code uses trylocks and bail
out if it can't get all locks.
Additionally most of the discussed special conditions for s390 like
only one memslot and no user_alloc are now checked for validity in
kvm_arch_set_memory_region.
Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
kvm-s390.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -657,6 +657,8 @@ int kvm_arch_set_memory_region(struct kv
struct kvm_memory_slot old,
int user_alloc)
{
+ int i;
+
/* 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
page boundary in userland and which has to end at a page boundary.
@@ -664,7 +666,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)
+ if (mem->slot || kvm->arch.guest_memsize)
return -EINVAL;
if (mem->guest_phys_addr)
@@ -676,15 +678,39 @@ int kvm_arch_set_memory_region(struct kv
if (mem->memory_size & (PAGE_SIZE - 1))
return -EINVAL;
+ if (!user_alloc)
+ return -EINVAL;
+
+ /* lock all vcpus */
+ for (i = 0; i < KVM_MAX_VCPUS; ++i) {
+ if (!kvm->vcpus[i])
+ 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;
- /* FIXME: we do want to interrupt running CPUs and update their memory
- configuration now to avoid race conditions. But hey, changing the
- memory layout while virtual CPUs are running is usually bad
- programming practice. */
+ /* 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);
+ }
+ }
return 0;
+
+fail_out:
+ for (; i >= 0; i--)
+ mutex_unlock(&kvm->vcpus[i]->mutex);
+ return -EINVAL;
}
void kvm_arch_flush_shadow(struct kvm *kvm)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/6] kvm-s390: use hrtimer for clock wakeup from idle - v2
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
2009-05-12 15:21 ` [PATCH 1/6] kvm-s390: Fix memory slot versus run " ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-12 15:21 ` [PATCH 3/6] kvm-s390: optimize float int lock: spin_lock_bh --> spin_lock ehrhardt
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Christian Borntraeger <borntraeger@de.ibm.com>
This patch reworks the s390 clock comparator wakeup to hrtimer. The clock
comparator is a per-cpu value that is compared against the TOD clock. If
ckc <= TOD an external interrupt 1004 is triggered. Since the clock comparator
and the TOD clock have a much higher resolution than jiffies we should use
hrtimers to trigger the wakeup. This speeds up guest nanosleep for small
values.
Since hrtimers callbacks run in hard-irq context, I added a tasklet to do
the actual work with enabled interrupts.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
arch/s390/include/asm/kvm_host.h | 5 ++++-
arch/s390/kvm/interrupt.c | 33 +++++++++++++++++++++++----------
arch/s390/kvm/kvm-s390.c | 7 +++++--
arch/s390/kvm/kvm-s390.h | 4 +++-
4 files changed, 35 insertions(+), 14 deletions(-)
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
@@ -13,6 +13,8 @@
#ifndef ASM_KVM_HOST_H
#define ASM_KVM_HOST_H
+#include <linux/hrtimer.h>
+#include <linux/interrupt.h>
#include <linux/kvm_host.h>
#include <asm/debug.h>
#include <asm/cpuid.h>
@@ -210,7 +212,8 @@ struct kvm_vcpu_arch {
s390_fp_regs guest_fpregs;
unsigned int guest_acrs[NUM_ACRS];
struct kvm_s390_local_interrupt local_int;
- struct timer_list ckc_timer;
+ struct hrtimer ckc_timer;
+ struct tasklet_struct tasklet;
union {
cpuid_t cpu_id;
u64 stidp_data;
Index: kvm/arch/s390/kvm/interrupt.c
===================================================================
--- kvm.orig/arch/s390/kvm/interrupt.c
+++ kvm/arch/s390/kvm/interrupt.c
@@ -12,6 +12,8 @@
#include <asm/lowcore.h>
#include <asm/uaccess.h>
+#include <linux/hrtimer.h>
+#include <linux/interrupt.h>
#include <linux/kvm_host.h>
#include <linux/signal.h>
#include "kvm-s390.h"
@@ -361,12 +363,10 @@ int kvm_s390_handle_wait(struct kvm_vcpu
return 0;
}
- sltime = (vcpu->arch.sie_block->ckc - now) / (0xf4240000ul / HZ) + 1;
+ sltime = ((vcpu->arch.sie_block->ckc - now)*125)>>9;
- vcpu->arch.ckc_timer.expires = jiffies + sltime;
-
- add_timer(&vcpu->arch.ckc_timer);
- VCPU_EVENT(vcpu, 5, "enabled wait timer:%llx jiffies", sltime);
+ hrtimer_start(&vcpu->arch.ckc_timer, ktime_set (0, sltime) , HRTIMER_MODE_REL);
+ VCPU_EVENT(vcpu, 5, "enabled wait via clock comparator: %llx ns", sltime);
no_timer:
spin_lock_bh(&vcpu->arch.local_int.float_int->lock);
spin_lock_bh(&vcpu->arch.local_int.lock);
@@ -389,21 +389,34 @@ no_timer:
remove_wait_queue(&vcpu->wq, &wait);
spin_unlock_bh(&vcpu->arch.local_int.lock);
spin_unlock_bh(&vcpu->arch.local_int.float_int->lock);
- del_timer(&vcpu->arch.ckc_timer);
+ hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
return 0;
}
-void kvm_s390_idle_wakeup(unsigned long data)
+void kvm_s390_tasklet(unsigned long parm)
{
- struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
+ struct kvm_vcpu *vcpu = (struct kvm_vcpu *) parm;
- spin_lock_bh(&vcpu->arch.local_int.lock);
+ spin_lock(&vcpu->arch.local_int.lock);
vcpu->arch.local_int.timer_due = 1;
if (waitqueue_active(&vcpu->arch.local_int.wq))
wake_up_interruptible(&vcpu->arch.local_int.wq);
- spin_unlock_bh(&vcpu->arch.local_int.lock);
+ spin_unlock(&vcpu->arch.local_int.lock);
}
+/*
+ * low level hrtimer wake routine. Because this runs in hardirq context
+ * we schedule a tasklet to do the real work.
+ */
+enum hrtimer_restart kvm_s390_idle_wakeup(struct hrtimer *timer)
+{
+ struct kvm_vcpu *vcpu;
+
+ vcpu = container_of(timer, struct kvm_vcpu, arch.ckc_timer);
+ tasklet_schedule(&vcpu->arch.tasklet);
+
+ return HRTIMER_NORESTART;
+}
void kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu)
{
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -15,6 +15,7 @@
#include <linux/compiler.h>
#include <linux/err.h>
#include <linux/fs.h>
+#include <linux/hrtimer.h>
#include <linux/init.h>
#include <linux/kvm.h>
#include <linux/kvm_host.h>
@@ -287,8 +288,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu
vcpu->arch.sie_block->gmsor = vcpu->kvm->arch.guest_origin;
vcpu->arch.sie_block->ecb = 2;
vcpu->arch.sie_block->eca = 0xC1002001U;
- setup_timer(&vcpu->arch.ckc_timer, kvm_s390_idle_wakeup,
- (unsigned long) vcpu);
+ hrtimer_init(&vcpu->arch.ckc_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+ tasklet_init(&vcpu->arch.tasklet, kvm_s390_tasklet,
+ (unsigned long) vcpu);
+ vcpu->arch.ckc_timer.function = kvm_s390_idle_wakeup;
get_cpu_id(&vcpu->arch.cpu_id);
vcpu->arch.cpu_id.version = 0xff;
return 0;
Index: kvm/arch/s390/kvm/kvm-s390.h
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.h
+++ kvm/arch/s390/kvm/kvm-s390.h
@@ -14,6 +14,7 @@
#ifndef ARCH_S390_KVM_S390_H
#define ARCH_S390_KVM_S390_H
+#include <linux/hrtimer.h>
#include <linux/kvm.h>
#include <linux/kvm_host.h>
@@ -46,7 +47,8 @@ static inline int host_intervention(stru
}
int kvm_s390_handle_wait(struct kvm_vcpu *vcpu);
-void kvm_s390_idle_wakeup(unsigned long data);
+enum hrtimer_restart kvm_s390_idle_wakeup(struct hrtimer *timer);
+void kvm_s390_tasklet(unsigned long parm);
void kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu);
int kvm_s390_inject_vm(struct kvm *kvm,
struct kvm_s390_interrupt *s390int);
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/6] kvm-s390: optimize float int lock: spin_lock_bh --> spin_lock
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
2009-05-12 15:21 ` [PATCH 1/6] kvm-s390: Fix memory slot versus run " ehrhardt
2009-05-12 15:21 ` [PATCH 2/6] kvm-s390: use hrtimer for clock wakeup from idle - v2 ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-12 15:21 ` [PATCH 4/6]: kvm-s390: Unlink vcpu on destroy - v2 ehrhardt
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Christian Borntraeger <borntraeger@de.ibm.com>
The floating interrupt lock is only taken in process context. We can
replace all spin_lock_bh with standard spin_lock calls.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
arch/s390/kvm/interrupt.c | 20 ++++++++++----------
arch/s390/kvm/kvm-s390.c | 4 ++--
arch/s390/kvm/priv.c | 4 ++--
arch/s390/kvm/sigp.c | 16 ++++++++--------
4 files changed, 22 insertions(+), 22 deletions(-)
Index: kvm/arch/s390/kvm/interrupt.c
===================================================================
--- kvm.orig/arch/s390/kvm/interrupt.c
+++ kvm/arch/s390/kvm/interrupt.c
@@ -301,13 +301,13 @@ int kvm_cpu_has_interrupt(struct kvm_vcp
}
if ((!rc) && atomic_read(&fi->active)) {
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_for_each_entry(inti, &fi->list, list)
if (__interrupt_is_deliverable(vcpu, inti)) {
rc = 1;
break;
}
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
}
if ((!rc) && (vcpu->arch.sie_block->ckc <
@@ -368,7 +368,7 @@ int kvm_s390_handle_wait(struct kvm_vcpu
hrtimer_start(&vcpu->arch.ckc_timer, ktime_set (0, sltime) , HRTIMER_MODE_REL);
VCPU_EVENT(vcpu, 5, "enabled wait via clock comparator: %llx ns", sltime);
no_timer:
- spin_lock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_lock(&vcpu->arch.local_int.float_int->lock);
spin_lock_bh(&vcpu->arch.local_int.lock);
add_wait_queue(&vcpu->arch.local_int.wq, &wait);
while (list_empty(&vcpu->arch.local_int.list) &&
@@ -377,18 +377,18 @@ no_timer:
!signal_pending(current)) {
set_current_state(TASK_INTERRUPTIBLE);
spin_unlock_bh(&vcpu->arch.local_int.lock);
- spin_unlock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_unlock(&vcpu->arch.local_int.float_int->lock);
vcpu_put(vcpu);
schedule();
vcpu_load(vcpu);
- spin_lock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_lock(&vcpu->arch.local_int.float_int->lock);
spin_lock_bh(&vcpu->arch.local_int.lock);
}
__unset_cpu_idle(vcpu);
__set_current_state(TASK_RUNNING);
remove_wait_queue(&vcpu->wq, &wait);
spin_unlock_bh(&vcpu->arch.local_int.lock);
- spin_unlock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_unlock(&vcpu->arch.local_int.float_int->lock);
hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
return 0;
}
@@ -455,7 +455,7 @@ void kvm_s390_deliver_pending_interrupts
if (atomic_read(&fi->active)) {
do {
deliver = 0;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_for_each_entry_safe(inti, n, &fi->list, list) {
if (__interrupt_is_deliverable(vcpu, inti)) {
list_del(&inti->list);
@@ -466,7 +466,7 @@ void kvm_s390_deliver_pending_interrupts
}
if (list_empty(&fi->list))
atomic_set(&fi->active, 0);
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
if (deliver) {
__do_deliver_interrupt(vcpu, inti);
kfree(inti);
@@ -531,7 +531,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
mutex_lock(&kvm->lock);
fi = &kvm->arch.float_int;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_add_tail(&inti->list, &fi->list);
atomic_set(&fi->active, 1);
sigcpu = find_first_bit(fi->idle_mask, KVM_MAX_VCPUS);
@@ -548,7 +548,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
if (waitqueue_active(&li->wq))
wake_up_interruptible(&li->wq);
spin_unlock_bh(&li->lock);
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
mutex_unlock(&kvm->lock);
return 0;
}
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -323,11 +323,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(st
spin_lock_init(&vcpu->arch.local_int.lock);
INIT_LIST_HEAD(&vcpu->arch.local_int.list);
vcpu->arch.local_int.float_int = &kvm->arch.float_int;
- spin_lock_bh(&kvm->arch.float_int.lock);
+ spin_lock(&kvm->arch.float_int.lock);
kvm->arch.float_int.local_int[id] = &vcpu->arch.local_int;
init_waitqueue_head(&vcpu->arch.local_int.wq);
vcpu->arch.local_int.cpuflags = &vcpu->arch.sie_block->cpuflags;
- spin_unlock_bh(&kvm->arch.float_int.lock);
+ spin_unlock(&kvm->arch.float_int.lock);
rc = kvm_vcpu_init(vcpu, kvm, id);
if (rc)
Index: kvm/arch/s390/kvm/priv.c
===================================================================
--- kvm.orig/arch/s390/kvm/priv.c
+++ kvm/arch/s390/kvm/priv.c
@@ -204,11 +204,11 @@ static void handle_stsi_3_2_2(struct kvm
int cpus = 0;
int n;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
for (n = 0; n < KVM_MAX_VCPUS; n++)
if (fi->local_int[n])
cpus++;
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
/* deal with other level 3 hypervisors */
if (stsi(mem, 3, 2, 2) == -ENOSYS)
Index: kvm/arch/s390/kvm/sigp.c
===================================================================
--- kvm.orig/arch/s390/kvm/sigp.c
+++ kvm/arch/s390/kvm/sigp.c
@@ -52,7 +52,7 @@ static int __sigp_sense(struct kvm_vcpu
if (cpu_addr >= KVM_MAX_VCPUS)
return 3; /* not operational */
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
if (fi->local_int[cpu_addr] == NULL)
rc = 3; /* not operational */
else if (atomic_read(fi->local_int[cpu_addr]->cpuflags)
@@ -64,7 +64,7 @@ static int __sigp_sense(struct kvm_vcpu
*reg |= SIGP_STAT_STOPPED;
rc = 1; /* status stored */
}
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sensed status of cpu %x rc %x", cpu_addr, rc);
return rc;
@@ -86,7 +86,7 @@ static int __sigp_emergency(struct kvm_v
inti->type = KVM_S390_INT_EMERGENCY;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if (li == NULL) {
rc = 3; /* not operational */
@@ -102,7 +102,7 @@ static int __sigp_emergency(struct kvm_v
spin_unlock_bh(&li->lock);
rc = 0; /* order accepted */
unlock:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sent sigp emerg to cpu %x", cpu_addr);
return rc;
}
@@ -123,7 +123,7 @@ static int __sigp_stop(struct kvm_vcpu *
inti->type = KVM_S390_SIGP_STOP;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if (li == NULL) {
rc = 3; /* not operational */
@@ -142,7 +142,7 @@ static int __sigp_stop(struct kvm_vcpu *
spin_unlock_bh(&li->lock);
rc = 0; /* order accepted */
unlock:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sent sigp stop to cpu %x", cpu_addr);
return rc;
}
@@ -188,7 +188,7 @@ static int __sigp_set_prefix(struct kvm_
if (!inti)
return 2; /* busy */
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if ((cpu_addr >= KVM_MAX_VCPUS) || (li == NULL)) {
@@ -220,7 +220,7 @@ static int __sigp_set_prefix(struct kvm_
out_li:
spin_unlock_bh(&li->lock);
out_fi:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
return rc;
}
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 4/6]: kvm-s390: Unlink vcpu on destroy - v2
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
` (2 preceding siblings ...)
2009-05-12 15:21 ` [PATCH 3/6] kvm-s390: optimize float int lock: spin_lock_bh --> spin_lock ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-12 15:21 ` [PATCH 5/6] kvm-s390: Sanity check on validity intercept ehrhardt
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Carsten Otte <cotte@de.ibm.com>
This patch makes sure we do unlink a vcpu's sie control block
from the system control area in kvm_arch_vcpu_destroy. This
prevents illegal accesses to the sie control block from other
virtual cpus after free.
Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
kvm-s390.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -195,6 +195,10 @@ out_nokvm:
void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
{
VCPU_EVENT(vcpu, 3, "%s", "free cpu");
+ if (vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda ==
+ (__u64) vcpu->arch.sie_block)
+ vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda = 0;
+ smp_mb();
free_page((unsigned long)(vcpu->arch.sie_block));
kvm_vcpu_uninit(vcpu);
kfree(vcpu);
@@ -307,8 +311,10 @@ struct kvm_vcpu *kvm_arch_vcpu_create(st
vcpu->arch.sie_block->icpua = id;
BUG_ON(!kvm->arch.sca);
- BUG_ON(kvm->arch.sca->cpu[id].sda);
- kvm->arch.sca->cpu[id].sda = (__u64) vcpu->arch.sie_block;
+ if (!kvm->arch.sca->cpu[id].sda)
+ kvm->arch.sca->cpu[id].sda = (__u64) vcpu->arch.sie_block;
+ else
+ BUG_ON(!kvm->vcpus[id]); /* vcpu does already exist */
vcpu->arch.sie_block->scaoh = (__u32)(((__u64)kvm->arch.sca) >> 32);
vcpu->arch.sie_block->scaol = (__u32)(__u64)kvm->arch.sca;
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 5/6] kvm-s390: Sanity check on validity intercept
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
` (3 preceding siblings ...)
2009-05-12 15:21 ` [PATCH 4/6]: kvm-s390: Unlink vcpu on destroy - v2 ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-12 15:21 ` [PATCH 6/6] kvm-s390: Verify memory in kvm run ehrhardt
2009-05-13 7:44 ` [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 Avi Kivity
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Carsten Otte <cotte@de.ibm.com>
This patch adds a sanity check for the content of the guest
prefix register content before faulting in the cpu lowcore
that it refers to. The guest might end up in an endless loop
where SIE complains about missing lowcore with incorrect
content of the prefix register without this fix.
Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
arch/s390/kvm/intercept.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
Index: kvm/arch/s390/kvm/intercept.c
===================================================================
--- kvm.orig/arch/s390/kvm/intercept.c
+++ kvm/arch/s390/kvm/intercept.c
@@ -154,17 +154,25 @@ static int handle_stop(struct kvm_vcpu *
static int handle_validity(struct kvm_vcpu *vcpu)
{
int viwhy = vcpu->arch.sie_block->ipb >> 16;
+ int rc;
+
vcpu->stat.exit_validity++;
- if (viwhy == 0x37) {
- fault_in_pages_writeable((char __user *)
- vcpu->kvm->arch.guest_origin +
- vcpu->arch.sie_block->prefix,
- PAGE_SIZE);
- return 0;
- }
- VCPU_EVENT(vcpu, 2, "unhandled validity intercept code %d",
- viwhy);
- return -ENOTSUPP;
+ if ((viwhy == 0x37) && (vcpu->arch.sie_block->prefix
+ <= vcpu->kvm->arch.guest_memsize - 2*PAGE_SIZE)){
+ rc = fault_in_pages_writeable((char __user *)
+ vcpu->kvm->arch.guest_origin +
+ vcpu->arch.sie_block->prefix,
+ 2*PAGE_SIZE);
+ if (rc)
+ /* user will receive sigsegv, exit to user */
+ rc = -ENOTSUPP;
+ } else
+ rc = -ENOTSUPP;
+
+ if (rc)
+ VCPU_EVENT(vcpu, 2, "unhandled validity intercept code %d",
+ viwhy);
+ return rc;
}
static int handle_instruction(struct kvm_vcpu *vcpu)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 6/6] kvm-s390: Verify memory in kvm run
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
` (4 preceding siblings ...)
2009-05-12 15:21 ` [PATCH 5/6] kvm-s390: Sanity check on validity intercept ehrhardt
@ 2009-05-12 15:21 ` ehrhardt
2009-05-13 7:44 ` [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 Avi Kivity
6 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-12 15:21 UTC (permalink / raw)
To: Avi Kivity, kvm
Cc: ehrhardt, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
From: Carsten Otte <cotte@de.ibm.com>
This check verifies that the guest we're trying to run in KVM_RUN
has some memory assigned to it. It enters an endless exception
loop if this is not the case.
Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Ehrhardt <ehrhardt@de.ibm.com>
---
arch/s390/kvm/kvm-s390.c | 6 ++++++
1 file changed, 6 insertions(+)
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -478,6 +478,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
vcpu_load(vcpu);
+ /* verify, that memory has been registered */
+ if (!vcpu->kvm->arch.guest_memsize) {
+ vcpu_put(vcpu);
+ return -EINVAL;
+ }
+
if (vcpu->sigset_active)
sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3
2009-05-12 15:21 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes - v3 ehrhardt
` (5 preceding siblings ...)
2009-05-12 15:21 ` [PATCH 6/6] kvm-s390: Verify memory in kvm run ehrhardt
@ 2009-05-13 7:44 ` Avi Kivity
6 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2009-05-13 7:44 UTC (permalink / raw)
To: ehrhardt
Cc: kvm, Christian Borntraeger, Carsten Otte, Heiko Carstens,
Martin Schwidefsky
ehrhardt@linux.vnet.ibm.com wrote:
> From: Christian Ehrhardt <ehrhardt@de.ibm.com>
>
> *updates in v3*
> - fix memory slot vs. run uses trylock to avoid a potential livelock
> - fix memory slot vs. run checks if it is the first and only memslot registered
>
> *updates in v2*
> - hrtimer wakeup use a more accurate calculation
> - unlink vcpu uses smb_mb so the pointer is really zero when the page is freed
>
> This is a collection of fixes for kvm-s390 that originate from several tests
> made in the last few months. They are now tested a while and should be ready
> to be merged.
>
>
Applied all, thanks.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/6] kvm-s390: optimize float int lock: spin_lock_bh --> spin_lock
2009-05-05 14:39 [PATCH 0/6] kvm-s390: collection of kvm-s390 fixes ehrhardt
@ 2009-05-05 14:39 ` ehrhardt
0 siblings, 0 replies; 9+ messages in thread
From: ehrhardt @ 2009-05-05 14:39 UTC (permalink / raw)
To: Avi Kivity, kvm; +Cc: ehrhardt, Christian Borntraeger, Carsten Otte
From: Christian Borntraeger <borntraeger@de.ibm.com>
The floating interrupt lock is only taken in process context. We can
replace all spin_lock_bh with standard spin_lock calls.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/interrupt.c | 20 ++++++++++----------
arch/s390/kvm/kvm-s390.c | 4 ++--
arch/s390/kvm/priv.c | 4 ++--
arch/s390/kvm/sigp.c | 16 ++++++++--------
4 files changed, 22 insertions(+), 22 deletions(-)
Index: kvm/arch/s390/kvm/interrupt.c
===================================================================
--- kvm.orig/arch/s390/kvm/interrupt.c
+++ kvm/arch/s390/kvm/interrupt.c
@@ -301,13 +301,13 @@ int kvm_cpu_has_interrupt(struct kvm_vcp
}
if ((!rc) && atomic_read(&fi->active)) {
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_for_each_entry(inti, &fi->list, list)
if (__interrupt_is_deliverable(vcpu, inti)) {
rc = 1;
break;
}
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
}
if ((!rc) && (vcpu->arch.sie_block->ckc <
@@ -368,7 +368,7 @@ int kvm_s390_handle_wait(struct kvm_vcpu
hrtimer_start(&vcpu->arch.ckc_timer, ktime_set (0, sltime) , HRTIMER_MODE_REL);
VCPU_EVENT(vcpu, 5, "enabled wait via clock comparator: %llx ns", sltime);
no_timer:
- spin_lock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_lock(&vcpu->arch.local_int.float_int->lock);
spin_lock_bh(&vcpu->arch.local_int.lock);
add_wait_queue(&vcpu->arch.local_int.wq, &wait);
while (list_empty(&vcpu->arch.local_int.list) &&
@@ -377,18 +377,18 @@ no_timer:
!signal_pending(current)) {
set_current_state(TASK_INTERRUPTIBLE);
spin_unlock_bh(&vcpu->arch.local_int.lock);
- spin_unlock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_unlock(&vcpu->arch.local_int.float_int->lock);
vcpu_put(vcpu);
schedule();
vcpu_load(vcpu);
- spin_lock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_lock(&vcpu->arch.local_int.float_int->lock);
spin_lock_bh(&vcpu->arch.local_int.lock);
}
__unset_cpu_idle(vcpu);
__set_current_state(TASK_RUNNING);
remove_wait_queue(&vcpu->wq, &wait);
spin_unlock_bh(&vcpu->arch.local_int.lock);
- spin_unlock_bh(&vcpu->arch.local_int.float_int->lock);
+ spin_unlock(&vcpu->arch.local_int.float_int->lock);
hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
return 0;
}
@@ -455,7 +455,7 @@ void kvm_s390_deliver_pending_interrupts
if (atomic_read(&fi->active)) {
do {
deliver = 0;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_for_each_entry_safe(inti, n, &fi->list, list) {
if (__interrupt_is_deliverable(vcpu, inti)) {
list_del(&inti->list);
@@ -466,7 +466,7 @@ void kvm_s390_deliver_pending_interrupts
}
if (list_empty(&fi->list))
atomic_set(&fi->active, 0);
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
if (deliver) {
__do_deliver_interrupt(vcpu, inti);
kfree(inti);
@@ -531,7 +531,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
mutex_lock(&kvm->lock);
fi = &kvm->arch.float_int;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
list_add_tail(&inti->list, &fi->list);
atomic_set(&fi->active, 1);
sigcpu = find_first_bit(fi->idle_mask, KVM_MAX_VCPUS);
@@ -548,7 +548,7 @@ int kvm_s390_inject_vm(struct kvm *kvm,
if (waitqueue_active(&li->wq))
wake_up_interruptible(&li->wq);
spin_unlock_bh(&li->lock);
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
mutex_unlock(&kvm->lock);
return 0;
}
Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -323,11 +323,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(st
spin_lock_init(&vcpu->arch.local_int.lock);
INIT_LIST_HEAD(&vcpu->arch.local_int.list);
vcpu->arch.local_int.float_int = &kvm->arch.float_int;
- spin_lock_bh(&kvm->arch.float_int.lock);
+ spin_lock(&kvm->arch.float_int.lock);
kvm->arch.float_int.local_int[id] = &vcpu->arch.local_int;
init_waitqueue_head(&vcpu->arch.local_int.wq);
vcpu->arch.local_int.cpuflags = &vcpu->arch.sie_block->cpuflags;
- spin_unlock_bh(&kvm->arch.float_int.lock);
+ spin_unlock(&kvm->arch.float_int.lock);
rc = kvm_vcpu_init(vcpu, kvm, id);
if (rc)
Index: kvm/arch/s390/kvm/priv.c
===================================================================
--- kvm.orig/arch/s390/kvm/priv.c
+++ kvm/arch/s390/kvm/priv.c
@@ -204,11 +204,11 @@ static void handle_stsi_3_2_2(struct kvm
int cpus = 0;
int n;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
for (n = 0; n < KVM_MAX_VCPUS; n++)
if (fi->local_int[n])
cpus++;
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
/* deal with other level 3 hypervisors */
if (stsi(mem, 3, 2, 2) == -ENOSYS)
Index: kvm/arch/s390/kvm/sigp.c
===================================================================
--- kvm.orig/arch/s390/kvm/sigp.c
+++ kvm/arch/s390/kvm/sigp.c
@@ -52,7 +52,7 @@ static int __sigp_sense(struct kvm_vcpu
if (cpu_addr >= KVM_MAX_VCPUS)
return 3; /* not operational */
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
if (fi->local_int[cpu_addr] == NULL)
rc = 3; /* not operational */
else if (atomic_read(fi->local_int[cpu_addr]->cpuflags)
@@ -64,7 +64,7 @@ static int __sigp_sense(struct kvm_vcpu
*reg |= SIGP_STAT_STOPPED;
rc = 1; /* status stored */
}
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sensed status of cpu %x rc %x", cpu_addr, rc);
return rc;
@@ -86,7 +86,7 @@ static int __sigp_emergency(struct kvm_v
inti->type = KVM_S390_INT_EMERGENCY;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if (li == NULL) {
rc = 3; /* not operational */
@@ -102,7 +102,7 @@ static int __sigp_emergency(struct kvm_v
spin_unlock_bh(&li->lock);
rc = 0; /* order accepted */
unlock:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sent sigp emerg to cpu %x", cpu_addr);
return rc;
}
@@ -123,7 +123,7 @@ static int __sigp_stop(struct kvm_vcpu *
inti->type = KVM_S390_SIGP_STOP;
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if (li == NULL) {
rc = 3; /* not operational */
@@ -142,7 +142,7 @@ static int __sigp_stop(struct kvm_vcpu *
spin_unlock_bh(&li->lock);
rc = 0; /* order accepted */
unlock:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
VCPU_EVENT(vcpu, 4, "sent sigp stop to cpu %x", cpu_addr);
return rc;
}
@@ -188,7 +188,7 @@ static int __sigp_set_prefix(struct kvm_
if (!inti)
return 2; /* busy */
- spin_lock_bh(&fi->lock);
+ spin_lock(&fi->lock);
li = fi->local_int[cpu_addr];
if ((cpu_addr >= KVM_MAX_VCPUS) || (li == NULL)) {
@@ -220,7 +220,7 @@ static int __sigp_set_prefix(struct kvm_
out_li:
spin_unlock_bh(&li->lock);
out_fi:
- spin_unlock_bh(&fi->lock);
+ spin_unlock(&fi->lock);
return rc;
}
^ permalink raw reply [flat|nested] 9+ messages in thread