[PATCH 0/6] kvm-s390: Multiple fixes for kvm on s390

[PATCH 0/6] kvm-s390: Multiple fixes for kvm on s390

[Christian Borntraeger]

Hello Avi,

here is a number of fixes and changes to the kvm on s390 port. This 
also includes the resend of the Kconfig patch. The patch desription has an
additional comment dealing with your comment.

I think all patches are ok for 2.6.27. Patches 4 and 6 are fixing potential 
host BUGs.

1. Advertise KVM_CAP_USER_MEMORY
2. Fix guest kconfig
3. Change guestaddr type in gaccess
4. Fix program check on interrupt delivery handling
5. Fix instruction naming for lctlg
6. Fix possible host kernel bug on lctl(g) handling


Review is appreciated, if ok please apply.

Christian

[PATCH 1/6] kvm-s390: Advertise KVM_CAP_USER_MEMORY

[Christian Borntraeger]

From: Carsten Otte <cotte@de.ibm.com>

KVM_CAP_USER_MEMORY is used by s390, therefore, we should advertise it.

Signed-off-by: Carsten Otte <cotte@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -112,7 +112,12 @@ long kvm_arch_dev_ioctl(struct file *fil
 
 int kvm_dev_ioctl_check_extension(long ext)
 {
-	return 0;
+	switch (ext) {
+	case KVM_CAP_USER_MEMORY:
+		return 1;
+	default:
+		return 0;
+	}
 }
 
 /* Section: vm related */

[PATCH 2/6] kvm-s390: Fix guest kconfig

[Christian Borntraeger]

Cornelia Huck noticed that a modular virtio without kvm guest support
leads to a build error in the s390 virtio transport:

CONFIG_VIRTIO=m leads to 
ERROR: "vmem_add_mapping" [drivers/s390/kvm/kvm_virtio.ko] undefined!
ERROR: "max_pfn" [drivers/s390/kvm/kvm_virtio.ko] undefined!
ERROR: "vmem_remove_mapping" [drivers/s390/kvm/kvm_virtio.ko] undefined!

The virtio transport only works with kvm guest support and only as a
builtin. Lets change the build process of drivers/s390/kvm/kvm_virtio.c
to depend on kvm guest support, which is also a bool.

CONFIG_S390_GUEST already selects CONFIG_VIRTIO, that should prevent
CONFIG_S390_GUEST=y CONFIG_VIRTIO=n situations.

CC: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 drivers/s390/kvm/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: kvm/drivers/s390/kvm/Makefile
===================================================================
--- kvm.orig/drivers/s390/kvm/Makefile
+++ kvm/drivers/s390/kvm/Makefile
@@ -6,4 +6,4 @@
 # it under the terms of the GNU General Public License (version 2 only)
 # as published by the Free Software Foundation.
 
-obj-$(CONFIG_VIRTIO) += kvm_virtio.o
+obj-$(CONFIG_S390_GUEST) += kvm_virtio.o

[PATCH 3/6] kvm-s390: Change guestaddr type in gaccess

[Christian Borntraeger]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

All registers are unsigned long types. This patch changes all occurences
of guestaddr in gaccess from u64 to unsigned long.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---

 arch/s390/kvm/gaccess.h     |   62 +++++++++++++++++++++++---------------------
 arch/s390/kvm/sigp.c        |    5 ++-
 include/asm-s390/kvm_host.h |    2 -
 3 files changed, 37 insertions(+), 32 deletions(-)

Index: kvm/arch/s390/kvm/gaccess.h
===================================================================
--- kvm.orig/arch/s390/kvm/gaccess.h
+++ kvm/arch/s390/kvm/gaccess.h
@@ -18,11 +18,11 @@
 #include <asm/uaccess.h>
 
 static inline void __user *__guestaddr_to_user(struct kvm_vcpu *vcpu,
-					       u64 guestaddr)
+					       unsigned long guestaddr)
 {
-	u64 prefix  = vcpu->arch.sie_block->prefix;
-	u64 origin  = vcpu->kvm->arch.guest_origin;
-	u64 memsize = vcpu->kvm->arch.guest_memsize;
+	unsigned long prefix  = vcpu->arch.sie_block->prefix;
+	unsigned long origin  = vcpu->kvm->arch.guest_origin;
+	unsigned long memsize = vcpu->kvm->arch.guest_memsize;
 
 	if (guestaddr < 2 * PAGE_SIZE)
 		guestaddr += prefix;
@@ -37,7 +37,7 @@ static inline void __user *__guestaddr_t
 	return (void __user *) guestaddr;
 }
 
-static inline int get_guest_u64(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int get_guest_u64(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u64 *result)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -47,10 +47,10 @@ static inline int get_guest_u64(struct k
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
 
-	return get_user(*result, (u64 __user *) uptr);
+	return get_user(*result, (unsigned long __user *) uptr);
 }
 
-static inline int get_guest_u32(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int get_guest_u32(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u32 *result)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -63,7 +63,7 @@ static inline int get_guest_u32(struct k
 	return get_user(*result, (u32 __user *) uptr);
 }
 
-static inline int get_guest_u16(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int get_guest_u16(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u16 *result)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -76,7 +76,7 @@ static inline int get_guest_u16(struct k
 	return get_user(*result, (u16 __user *) uptr);
 }
 
-static inline int get_guest_u8(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int get_guest_u8(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 			       u8 *result)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -87,7 +87,7 @@ static inline int get_guest_u8(struct kv
 	return get_user(*result, (u8 __user *) uptr);
 }
 
-static inline int put_guest_u64(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int put_guest_u64(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u64 value)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -100,7 +100,7 @@ static inline int put_guest_u64(struct k
 	return put_user(value, (u64 __user *) uptr);
 }
 
-static inline int put_guest_u32(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int put_guest_u32(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u32 value)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -113,7 +113,7 @@ static inline int put_guest_u32(struct k
 	return put_user(value, (u32 __user *) uptr);
 }
 
-static inline int put_guest_u16(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int put_guest_u16(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 				u16 value)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -126,7 +126,7 @@ static inline int put_guest_u16(struct k
 	return put_user(value, (u16 __user *) uptr);
 }
 
-static inline int put_guest_u8(struct kvm_vcpu *vcpu, u64 guestaddr,
+static inline int put_guest_u8(struct kvm_vcpu *vcpu, unsigned long guestaddr,
 			       u8 value)
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
@@ -138,7 +138,8 @@ static inline int put_guest_u8(struct kv
 }
 
 
-static inline int __copy_to_guest_slow(struct kvm_vcpu *vcpu, u64 guestdest,
+static inline int __copy_to_guest_slow(struct kvm_vcpu *vcpu,
+				       unsigned long guestdest,
 				       const void *from, unsigned long n)
 {
 	int rc;
@@ -153,12 +154,12 @@ static inline int __copy_to_guest_slow(s
 	return 0;
 }
 
-static inline int copy_to_guest(struct kvm_vcpu *vcpu, u64 guestdest,
+static inline int copy_to_guest(struct kvm_vcpu *vcpu, unsigned long guestdest,
 				const void *from, unsigned long n)
 {
-	u64 prefix  = vcpu->arch.sie_block->prefix;
-	u64 origin  = vcpu->kvm->arch.guest_origin;
-	u64 memsize = vcpu->kvm->arch.guest_memsize;
+	unsigned long prefix  = vcpu->arch.sie_block->prefix;
+	unsigned long origin  = vcpu->kvm->arch.guest_origin;
+	unsigned long memsize = vcpu->kvm->arch.guest_memsize;
 
 	if ((guestdest < 2 * PAGE_SIZE) && (guestdest + n > 2 * PAGE_SIZE))
 		goto slowpath;
@@ -189,7 +190,8 @@ slowpath:
 }
 
 static inline int __copy_from_guest_slow(struct kvm_vcpu *vcpu, void *to,
-					 u64 guestsrc, unsigned long n)
+					 unsigned long guestsrc,
+					 unsigned long n)
 {
 	int rc;
 	unsigned long i;
@@ -204,11 +206,11 @@ static inline int __copy_from_guest_slow
 }
 
 static inline int copy_from_guest(struct kvm_vcpu *vcpu, void *to,
-				  u64 guestsrc, unsigned long n)
+				  unsigned long guestsrc, unsigned long n)
 {
-	u64 prefix  = vcpu->arch.sie_block->prefix;
-	u64 origin  = vcpu->kvm->arch.guest_origin;
-	u64 memsize = vcpu->kvm->arch.guest_memsize;
+	unsigned long prefix  = vcpu->arch.sie_block->prefix;
+	unsigned long origin  = vcpu->kvm->arch.guest_origin;
+	unsigned long memsize = vcpu->kvm->arch.guest_memsize;
 
 	if ((guestsrc < 2 * PAGE_SIZE) && (guestsrc + n > 2 * PAGE_SIZE))
 		goto slowpath;
@@ -238,11 +240,12 @@ slowpath:
 	return __copy_from_guest_slow(vcpu, to, guestsrc, n);
 }
 
-static inline int copy_to_guest_absolute(struct kvm_vcpu *vcpu, u64 guestdest,
+static inline int copy_to_guest_absolute(struct kvm_vcpu *vcpu,
+					 unsigned long guestdest,
 					 const void *from, unsigned long n)
 {
-	u64 origin  = vcpu->kvm->arch.guest_origin;
-	u64 memsize = vcpu->kvm->arch.guest_memsize;
+	unsigned long origin  = vcpu->kvm->arch.guest_origin;
+	unsigned long memsize = vcpu->kvm->arch.guest_memsize;
 
 	if (guestdest + n > memsize)
 		return -EFAULT;
@@ -256,10 +259,11 @@ static inline int copy_to_guest_absolute
 }
 
 static inline int copy_from_guest_absolute(struct kvm_vcpu *vcpu, void *to,
-					   u64 guestsrc, unsigned long n)
+					   unsigned long guestsrc,
+					   unsigned long n)
 {
-	u64 origin  = vcpu->kvm->arch.guest_origin;
-	u64 memsize = vcpu->kvm->arch.guest_memsize;
+	unsigned long origin  = vcpu->kvm->arch.guest_origin;
+	unsigned long memsize = vcpu->kvm->arch.guest_memsize;
 
 	if (guestsrc + n > memsize)
 		return -EFAULT;
Index: kvm/arch/s390/kvm/sigp.c
===================================================================
--- kvm.orig/arch/s390/kvm/sigp.c
+++ kvm/arch/s390/kvm/sigp.c
@@ -43,7 +43,8 @@
 #define SIGP_STAT_RECEIVER_CHECK    0x00000001UL
 
 
-static int __sigp_sense(struct kvm_vcpu *vcpu, u16 cpu_addr, u64 *reg)
+static int __sigp_sense(struct kvm_vcpu *vcpu, u16 cpu_addr,
+			unsigned long *reg)
 {
 	struct kvm_s390_float_interrupt *fi = &vcpu->kvm->arch.float_int;
 	int rc;
@@ -167,7 +168,7 @@ static int __sigp_set_arch(struct kvm_vc
 }
 
 static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 cpu_addr, u32 address,
-			     u64 *reg)
+			     unsigned long *reg)
 {
 	struct kvm_s390_float_interrupt *fi = &vcpu->kvm->arch.float_int;
 	struct kvm_s390_local_interrupt *li;
Index: kvm/include/asm-s390/kvm_host.h
===================================================================
--- kvm.orig/include/asm-s390/kvm_host.h
+++ kvm/include/asm-s390/kvm_host.h
@@ -231,5 +231,5 @@ struct kvm_arch{
 	struct kvm_s390_float_interrupt float_int;
 };
 
-extern int sie64a(struct kvm_s390_sie_block *, __u64 *);
+extern int sie64a(struct kvm_s390_sie_block *, unsigned long *);
 #endif

[PATCH 4/6] kvm-s390: Fix program check on interrupt delivery handling

[Christian Borntraeger]

From: Christian Borntraeger <borntraeger@de.ibm.com>

The current interrupt handling on s390 misbehaves on an error case. On s390
each cpu has the prefix area (lowcore) for interrupt delivery. This memory
must always be available. If we fail to access the prefix area for a guest
on interrupt delivery the configuration is completely unusable. There is no
point in sending another program interrupt to an inaccessible lowcore.
Furthermore, we should not bug the host kernel, because this can be triggered
by userspace. I think the guest kernel itself can not trigger the problem, as
SET PREFIX and SIGNAL PROCESSOR SET PREFIX both check that the memory is 
available and sane. As this is a userspace bug (e.g. setting the wrong guest
offset, unmapping guest memory) we should kill the userspace process instead
of BUGing the host kernel.
In the long term we probably should notify the userspace process about this 
problem.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/interrupt.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

Index: kvm/arch/s390/kvm/interrupt.c
===================================================================
--- kvm.orig/arch/s390/kvm/interrupt.c
+++ kvm/arch/s390/kvm/interrupt.c
@@ -13,6 +13,7 @@
 #include <asm/lowcore.h>
 #include <asm/uaccess.h>
 #include <linux/kvm_host.h>
+#include <linux/signal.h>
 #include "kvm-s390.h"
 #include "gaccess.h"
 
@@ -246,15 +247,10 @@ static void __do_deliver_interrupt(struc
 	default:
 		BUG();
 	}
-
 	if (exception) {
-		VCPU_EVENT(vcpu, 1, "%s", "program exception while delivering"
-			   " interrupt");
-		kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
-		if (inti->type == KVM_S390_PROGRAM_INT) {
-			printk(KERN_WARNING "kvm: recursive program check\n");
-			BUG();
-		}
+		printk("kvm: The guest lowcore is not mapped during interrupt "
+			"delivery, killing userspace\n");
+		do_exit(SIGKILL);
 	}
 }
 
@@ -277,14 +273,11 @@ static int __try_deliver_ckc_interrupt(s
 		__LC_EXT_NEW_PSW, sizeof(psw_t));
 	if (rc == -EFAULT)
 		exception = 1;
-
 	if (exception) {
-		VCPU_EVENT(vcpu, 1, "%s", "program exception while delivering" \
-			   " ckc interrupt");
-		kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
-		return 0;
+		printk("kvm: The guest lowcore is not mapped during interrupt "
+			"delivery, killing userspace\n");
+		do_exit(SIGKILL);
 	}
-
 	return 1;
 }
 

[PATCH 5/6] kvm-s390: Fix instruction naming for lctlg

[Christian Borntraeger]

From: Christian Borntraeger <borntraeger@de.ibm.com>

Lets fix the name for the lctlg instruction...

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/intercept.c   |    8 ++++----
 arch/s390/kvm/kvm-s390.c    |    2 +-
 include/asm-s390/kvm_host.h |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

Index: kvm/arch/s390/kvm/intercept.c
===================================================================
--- kvm.orig/arch/s390/kvm/intercept.c
+++ kvm/arch/s390/kvm/intercept.c
@@ -20,7 +20,7 @@
 #include "kvm-s390.h"
 #include "gaccess.h"
 
-static int handle_lctg(struct kvm_vcpu *vcpu)
+static int handle_lctlg(struct kvm_vcpu *vcpu)
 {
 	int reg1 = (vcpu->arch.sie_block->ipa & 0x00f0) >> 4;
 	int reg3 = vcpu->arch.sie_block->ipa & 0x000f;
@@ -30,7 +30,7 @@ static int handle_lctg(struct kvm_vcpu *
 	u64 useraddr;
 	int reg, rc;
 
-	vcpu->stat.instruction_lctg++;
+	vcpu->stat.instruction_lctlg++;
 	if ((vcpu->arch.sie_block->ipb & 0xff) != 0x2f)
 		return -ENOTSUPP;
 
@@ -40,7 +40,7 @@ static int handle_lctg(struct kvm_vcpu *
 
 	reg = reg1;
 
-	VCPU_EVENT(vcpu, 5, "lctg r1:%x, r3:%x,b2:%x,d2:%x", reg1, reg3, base2,
+	VCPU_EVENT(vcpu, 5, "lctlg r1:%x, r3:%x,b2:%x,d2:%x", reg1, reg3, base2,
 		   disp2);
 
 	do {
@@ -99,7 +99,7 @@ static intercept_handler_t instruction_h
 	[0xae] = kvm_s390_handle_sigp,
 	[0xb2] = kvm_s390_handle_priv,
 	[0xb7] = handle_lctl,
-	[0xeb] = handle_lctg,
+	[0xeb] = handle_lctlg,
 };
 
 static int handle_noop(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
@@ -39,7 +39,7 @@ struct kvm_stats_debugfs_item debugfs_en
 	{ "exit_instruction", VCPU_STAT(exit_instruction) },
 	{ "exit_program_interruption", VCPU_STAT(exit_program_interruption) },
 	{ "exit_instr_and_program_int", VCPU_STAT(exit_instr_and_program) },
-	{ "instruction_lctg", VCPU_STAT(instruction_lctg) },
+	{ "instruction_lctlg", VCPU_STAT(instruction_lctlg) },
 	{ "instruction_lctl", VCPU_STAT(instruction_lctl) },
 	{ "deliver_emergency_signal", VCPU_STAT(deliver_emergency_signal) },
 	{ "deliver_service_signal", VCPU_STAT(deliver_service_signal) },
Index: kvm/include/asm-s390/kvm_host.h
===================================================================
--- kvm.orig/include/asm-s390/kvm_host.h
+++ kvm/include/asm-s390/kvm_host.h
@@ -111,7 +111,7 @@ struct kvm_vcpu_stat {
 	u32 exit_validity;
 	u32 exit_instruction;
 	u32 instruction_lctl;
-	u32 instruction_lctg;
+	u32 instruction_lctlg;
 	u32 exit_program_interruption;
 	u32 exit_instr_and_program;
 	u32 deliver_emergency_signal;

[PATCH] kvm-s390: Fix possible host kernel bug on lctl(g) handling

[Christian Borntraeger]

From: Christian Borntraeger <borntraeger@de.ibm.com>

The lctl(g) instructions require a specific alignment for the parameters.
The architecture requires a specification program check if these alignments
are not used. Enforcing this alignment also removes a possible host BUG,
since the get_guest functions check for proper alignment and emits a BUG.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/intercept.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: kvm/arch/s390/kvm/intercept.c
===================================================================
--- kvm.orig/arch/s390/kvm/intercept.c
+++ kvm/arch/s390/kvm/intercept.c
@@ -38,6 +38,9 @@ static int handle_lctlg(struct kvm_vcpu 
 	if (base2)
 		useraddr += vcpu->arch.guest_gprs[base2];
 
+	if (useraddr & 7)
+		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
+
 	reg = reg1;
 
 	VCPU_EVENT(vcpu, 5, "lctlg r1:%x, r3:%x,b2:%x,d2:%x", reg1, reg3, base2,
@@ -74,6 +77,9 @@ static int handle_lctl(struct kvm_vcpu *
 	if (base2)
 		useraddr += vcpu->arch.guest_gprs[base2];
 
+	if (useraddr & 3)
+		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
+
 	VCPU_EVENT(vcpu, 5, "lctl r1:%x, r3:%x,b2:%x,d2:%x", reg1, reg3, base2,
 		   disp2);
 

Re: [PATCH 0/6] kvm-s390: Multiple fixes for kvm on s390

[Avi Kivity]

Christian Borntraeger wrote:
> Hello Avi,
>
> here is a number of fixes and changes to the kvm on s390 port. This 
> also includes the resend of the Kconfig patch. The patch desription has an
> additional comment dealing with your comment.
>
> I think all patches are ok for 2.6.27. Patches 4 and 6 are fixing potential 
> host BUGs.
>
> 1. Advertise KVM_CAP_USER_MEMORY
> 2. Fix guest kconfig
> 3. Change guestaddr type in gaccess
> 4. Fix program check on interrupt delivery handling
> 5. Fix instruction naming for lctlg
> 6. Fix possible host kernel bug on lctl(g) handling
>
>
>   

Applied all, thanks.

-- 
error compiling committee.c: too many arguments to function