[PATCH 01/04] kvm-s390: Some codingstyle issues.

[PATCH 01/04] kvm-s390: Some codingstyle issues.

[Carsten Otte]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Just a few codingstyle issues that have already been
commented on but that somehow got forgotten.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
---
 arch/s390/kvm/gaccess.h   |   18 ++++++------------
 arch/s390/kvm/intercept.c |   35 +++++++++++++----------------------
 arch/s390/kvm/kvm-s390.c  |   46 +++++++++++++++++++---------------------------
 arch/s390/kvm/kvm-s390.h  |    6 ++++--
 arch/s390/kvm/priv.c      |   11 ++++++-----
 arch/s390/kvm/sigp.c      |    4 +---
 6 files changed, 49 insertions(+), 71 deletions(-)

Index: linux-host/arch/s390/kvm/gaccess.h
===================================================================
--- linux-host.orig/arch/s390/kvm/gaccess.h
+++ linux-host/arch/s390/kvm/gaccess.h
@@ -42,8 +42,7 @@ static inline int get_guest_u64(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 7)
-		BUG();
+	BUG_ON(guestaddr & 7);
 
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
@@ -56,8 +55,7 @@ static inline int get_guest_u32(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 3)
-		BUG();
+	BUG_ON(guestaddr & 3);
 
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
@@ -70,8 +68,7 @@ static inline int get_guest_u16(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 1)
-		BUG();
+	BUG_ON(guestaddr & 1);
 
 	if (IS_ERR(uptr))
 		return PTR_ERR(uptr);
@@ -95,8 +92,7 @@ static inline int put_guest_u64(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 7)
-		BUG();
+	BUG_ON(guestaddr & 7);
 
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
@@ -109,8 +105,7 @@ static inline int put_guest_u32(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 3)
-		BUG();
+	BUG_ON(guestaddr & 3);
 
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
@@ -123,8 +118,7 @@ static inline int put_guest_u16(struct k
 {
 	void __user *uptr = __guestaddr_to_user(vcpu, guestaddr);
 
-	if (guestaddr & 1)
-		BUG();
+	BUG_ON(guestaddr & 1);
 
 	if (IS_ERR((void __force *) uptr))
 		return PTR_ERR((void __force *) uptr);
Index: linux-host/arch/s390/kvm/intercept.c
===================================================================
--- linux-host.orig/arch/s390/kvm/intercept.c
+++ linux-host/arch/s390/kvm/intercept.c
@@ -45,7 +45,7 @@ static int handle_lctg(struct kvm_vcpu *
 
 	do {
 		rc = get_guest_u64(vcpu, useraddr,
-			&vcpu->arch.sie_block->gcr[reg]);
+				   &vcpu->arch.sie_block->gcr[reg]);
 		if (rc == -EFAULT) {
 			kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
 			break;
@@ -53,9 +53,7 @@ static int handle_lctg(struct kvm_vcpu *
 		useraddr += 8;
 		if (reg == reg3)
 			break;
-		reg = reg + 1;
-		if (reg > 15)
-			reg = 0;
+		reg = (reg + 1) % 16;
 	} while (1);
 	return 0;
 }
@@ -76,11 +74,10 @@ static int handle_lctl(struct kvm_vcpu *
 	if (base2)
 		useraddr += vcpu->arch.guest_gprs[base2];
 
-	reg = reg1;
-
 	VCPU_EVENT(vcpu, 5, "lctl r1:%x, r3:%x,b2:%x,d2:%x", reg1, reg3, base2,
 		   disp2);
 
+	reg = reg1;
 	do {
 		rc = get_guest_u32(vcpu, useraddr, &val);
 		if (rc == -EFAULT) {
@@ -92,9 +89,7 @@ static int handle_lctl(struct kvm_vcpu *
 		useraddr += 4;
 		if (reg == reg3)
 			break;
-		reg = reg + 1;
-		if (reg > 15)
-			reg = 0;
+		reg = (reg + 1) % 16;
 	} while (1);
 	return 0;
 }
@@ -153,26 +148,25 @@ static int handle_validity(struct kvm_vc
 	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);
+					 vcpu->kvm->arch.guest_origin +
+					 vcpu->arch.sie_block->prefix,
+					 PAGE_SIZE);
 		return 0;
 	}
 	VCPU_EVENT(vcpu, 2, "unhandled validity intercept code %d",
-		viwhy);
+		   viwhy);
 	return -ENOTSUPP;
 }
 
 static int handle_instruction(struct kvm_vcpu *vcpu)
 {
-	intercept_handler_t handler =
-		instruction_handlers[vcpu->arch.sie_block->ipa >> 8];
+	intercept_handler_t handler;
 
 	vcpu->stat.exit_instruction++;
-
-	if (!handler)
-		return -ENOTSUPP;
-
-	return handler(vcpu);
+	handler = instruction_handlers[vcpu->arch.sie_block->ipa >> 8];
+	if (handler)
+		return handler(vcpu);
+	return -ENOTSUPP;
 }
 
 static int handle_prog(struct kvm_vcpu *vcpu)
@@ -215,11 +209,8 @@ int kvm_handle_sie_intercept(struct kvm_
 
 	if (code & 3 || code > 0x48)
 		return -ENOTSUPP;
-
 	func = intercept_funcs[code >> 2];
-
 	if (func)
 		return func(vcpu);
-
 	return -ENOTSUPP;
 }
Index: linux-host/arch/s390/kvm/kvm-s390.c
===================================================================
--- linux-host.orig/arch/s390/kvm/kvm-s390.c
+++ linux-host/arch/s390/kvm/kvm-s390.c
@@ -64,7 +64,7 @@ struct kvm_stats_debugfs_item debugfs_en
 	{ "instruction_sigp_set_prefix", VCPU_STAT(instruction_sigp_prefix) },
 	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
 	{ "diagnose_44", VCPU_STAT(diagnose_44) },
-	{ NULL }
+	{ NULL },
 };
 

@@ -113,8 +113,6 @@ long kvm_arch_dev_ioctl(struct file *fil
 	return -EINVAL;
 }
 
-
-
 int kvm_dev_ioctl_check_extension(long ext)
 {
 	return 0;
@@ -160,7 +158,6 @@ struct kvm *kvm_arch_create_vm(void)
 	int rc;
 	char debug_name[16];
 
-
 	rc = s390_enable_sie();
 	if (rc)
 		goto out_nokvm;
@@ -392,8 +389,7 @@ int kvm_arch_vcpu_ioctl_get_fpu(struct k
 	return 0;
 }
 
-static int kvm_arch_vcpu_ioctl_set_initial_psw(struct kvm_vcpu *vcpu,
-	psw_t psw)
+static int kvm_arch_vcpu_ioctl_set_initial_psw(struct kvm_vcpu *vcpu, psw_t psw)
 {
 	int rc = 0;
 
@@ -475,7 +471,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 		rc = kvm_handle_sie_intercept(vcpu);
 	} while (!signal_pending(current) && !rc);
 
-	if ((rc == 0) && signal_pending(current))
+	if (signal_pending(current) && !rc)
 		rc = -EINTR;
 
 	if (rc == -ENOTSUPP) {
@@ -537,49 +533,47 @@ int __kvm_s390_vcpu_store_status(struct 
 	} else
 		prefix = 0;
 
-
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, fp_regs),
-	  vcpu->arch.guest_fpregs.fprs, 128, prefix))
+			vcpu->arch.guest_fpregs.fprs, 128, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, gp_regs),
-	  vcpu->arch.guest_gprs, 128, prefix))
+			vcpu->arch.guest_gprs, 128, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, psw),
-	  &vcpu->arch.sie_block->gpsw, 16, prefix))
+			&vcpu->arch.sie_block->gpsw, 16, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, pref_reg),
-	  &vcpu->arch.sie_block->prefix, 4, prefix))
+			&vcpu->arch.sie_block->prefix, 4, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu,
-	  addr + offsetof(struct save_area_s390x, fp_ctrl_reg),
-	  &vcpu->arch.guest_fpregs.fpc, 4, prefix))
+			addr + offsetof(struct save_area_s390x, fp_ctrl_reg),
+			&vcpu->arch.guest_fpregs.fpc, 4, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, tod_reg),
-	  &vcpu->arch.sie_block->todpr, 4, prefix))
+			&vcpu->arch.sie_block->todpr, 4, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, timer),
-	  &vcpu->arch.sie_block->cputm, 8, prefix))
+			&vcpu->arch.sie_block->cputm, 8, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, clk_cmp),
-	  &vcpu->arch.sie_block->ckc, 8, prefix))
+			&vcpu->arch.sie_block->ckc, 8, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu, addr + offsetof(struct save_area_s390x, acc_regs),
-	  &vcpu->arch.guest_acrs, 64, prefix))
+			&vcpu->arch.guest_acrs, 64, prefix))
 		return -EFAULT;
 
 	if (__guestcopy(vcpu,
-	  addr + offsetof(struct save_area_s390x, ctrl_regs),
-	  &vcpu->arch.sie_block->gcr, 128, prefix))
+			addr + offsetof(struct save_area_s390x, ctrl_regs),
+			&vcpu->arch.sie_block->gcr, 128, prefix))
 		return -EFAULT;
-
 	return 0;
 }
 
@@ -590,7 +584,6 @@ static int kvm_s390_vcpu_store_status(st
 	vcpu_load(vcpu);
 	rc = __kvm_s390_vcpu_store_status(vcpu, addr);
 	vcpu_put(vcpu);
-
 	return rc;
 }
 
@@ -638,16 +631,16 @@ 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 != 0)
+	if (mem->slot)
 		return -EINVAL;
 
-	if (mem->guest_phys_addr != 0)
+	if (mem->guest_phys_addr)
 		return -EINVAL;
 
-	if (mem->userspace_addr % PAGE_SIZE)
+	if (mem->userspace_addr & (PAGE_SIZE - 1))
 		return -EINVAL;
 
-	if (mem->memory_size % PAGE_SIZE)
+	if (mem->memory_size & (PAGE_SIZE - 1))
 		return -EINVAL;
 
 	kvm->arch.guest_origin = mem->userspace_addr;
@@ -674,7 +667,6 @@ static int __init kvm_s390_init(void)
 static void __exit kvm_s390_exit(void)
 {
 	kvm_exit();
-	return;
 }
 
 module_init(kvm_s390_init);
Index: linux-host/arch/s390/kvm/kvm-s390.h
===================================================================
--- linux-host.orig/arch/s390/kvm/kvm-s390.h
+++ linux-host/arch/s390/kvm/kvm-s390.h
@@ -13,12 +13,13 @@
 
 #ifndef ARCH_S390_KVM_S390_H
 #define ARCH_S390_KVM_S390_H
+
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
 
-typedef  int (*intercept_handler_t)(struct kvm_vcpu *vcpu);
+typedef int (*intercept_handler_t)(struct kvm_vcpu *vcpu);
 
-extern int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu);
+int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu);
 
 #define VM_EVENT(d_kvm, d_loglevel, d_string, d_args...)\
 do { \
@@ -59,4 +60,5 @@ int __kvm_s390_vcpu_store_status(struct 
 				 unsigned long addr);
 /* implemented in diag.c */
 int kvm_s390_handle_diag(struct kvm_vcpu *vcpu);
+
 #endif
Index: linux-host/arch/s390/kvm/priv.c
===================================================================
--- linux-host.orig/arch/s390/kvm/priv.c
+++ linux-host/arch/s390/kvm/priv.c
@@ -175,7 +175,6 @@ static int handle_stfl(struct kvm_vcpu *
 	else
 		VCPU_EVENT(vcpu, 5, "store facility list value %x",
 			   facility_list);
-
 	return 0;
 }
 
@@ -315,8 +314,10 @@ static intercept_handler_t priv_handlers
 
 int kvm_s390_handle_priv(struct kvm_vcpu *vcpu)
 {
-	if (priv_handlers[vcpu->arch.sie_block->ipa & 0x00ff])
-		return priv_handlers[vcpu->arch.sie_block->ipa & 0x00ff]
-			(vcpu);
-		return -ENOTSUPP;
+	intercept_handler_t handler;
+
+	handler = priv_handlers[vcpu->arch.sie_block->ipa & 0x00ff];
+	if (handler)
+		return handler(vcpu);
+	return -ENOTSUPP;
 }
Index: linux-host/arch/s390/kvm/sigp.c
===================================================================
--- linux-host.orig/arch/s390/kvm/sigp.c
+++ linux-host/arch/s390/kvm/sigp.c
@@ -149,9 +149,8 @@ unlock:
 static int __sigp_set_arch(struct kvm_vcpu *vcpu, u32 parameter)
 {
 	int rc;
-	parameter = parameter & 0xff;
 
-	switch (parameter) {
+	switch (parameter & 0xff) {
 	case 0:
 		printk(KERN_WARNING "kvm: request to switch to ESA/390 mode"
 							" not supported");



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[PATCH 02/04] kvm-s390: Fix incorrect return value

[Carsten Otte]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

kvm_arch_vcpu_ioctl_run currently incorrectly always returns 0.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-host/arch/s390/kvm/kvm-s390.c
===================================================================
--- linux-host.orig/arch/s390/kvm/kvm-s390.c
+++ linux-host/arch/s390/kvm/kvm-s390.c
@@ -497,7 +497,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v
 	vcpu_put(vcpu);
 
 	vcpu->stat.exit_userspace++;
-	return 0;
+	return rc;
 }
 
 static int __guestcopy(struct kvm_vcpu *vcpu, u64 guestdest, const void *from,



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[PATCH 03/04] kvm-s390: Improve pgste accesses

[Carsten Otte]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

There is no need to use interlocked updates when the rcp
lock is held. Therefore the simple bitops variants can be
used. This should improve performance.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
---
include/asm-s390/pgtable.h |   12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

Index: kvm/include/asm-s390/pgtable.h
===================================================================
--- kvm.orig/include/asm-s390/pgtable.h
+++ kvm/include/asm-s390/pgtable.h
@@ -553,12 +553,12 @@ static inline void ptep_rcp_copy(pte_t *

skey = page_get_storage_key(page_to_phys(page));
if (skey & _PAGE_CHANGED)
- set_bit(RCP_GC_BIT, pgste);
+ set_bit_simple(RCP_GC_BIT, pgste);
if (skey & _PAGE_REFERENCED)
- set_bit(RCP_GR_BIT, pgste);
- if (test_and_clear_bit(RCP_HC_BIT, pgste))
+ set_bit_simple(RCP_GR_BIT, pgste);
+ if (test_and_clear_bit_simple(RCP_HC_BIT, pgste))
SetPageDirty(page);
- if (test_and_clear_bit(RCP_HR_BIT, pgste))
+ if (test_and_clear_bit_simple(RCP_HR_BIT, pgste))
SetPageReferenced(page);
#endif
}
@@ -732,8 +732,8 @@ static inline int ptep_test_and_clear_yo
young = ((page_get_storage_key(physpage) & _PAGE_REFERENCED) != 0);
rcp_lock(ptep);
if (young)
- set_bit(RCP_GR_BIT, pgste);
- young |= test_and_clear_bit(RCP_HR_BIT, pgste);
+ set_bit_simple(RCP_GR_BIT, pgste);
+ young |= test_and_clear_bit_simple(RCP_HR_BIT, pgste);
rcp_unlock(ptep);
return young;
#endif



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[PATCH 04/04] kvm-s390: rename stfl to kvm_stfl

[Carsten Otte]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Temporarily rename this function to avoid merge conflicts and/or
dependencies. This function will be removed as soon as git-s390
and kvm.git are finally upstream.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
---
 arch/s390/kvm/priv.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: kvm/arch/s390/kvm/priv.c
===================================================================
--- kvm.orig/arch/s390/kvm/priv.c
+++ kvm/arch/s390/kvm/priv.c
@@ -151,7 +151,7 @@ static int handle_chsc(struct kvm_vcpu *
 	return 0;
 }
 
-static unsigned int stfl(void)
+static unsigned int kvm_stfl(void)
 {
 	asm volatile(
 		"	.insn	s,0xb2b10000,0(0)\n" /* stfl */
@@ -162,7 +162,7 @@ static unsigned int stfl(void)
 
 static int handle_stfl(struct kvm_vcpu *vcpu)
 {
-	unsigned int facility_list = stfl();
+	unsigned int facility_list = kvm_stfl();
 	int rc;
 
 	vcpu->stat.instruction_stfl++;



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH 03/04] kvm-s390: Improve pgste accesses

[Martin Schwidefsky]

On Fri, 2008-04-04 at 15:12 +0200, Carsten Otte wrote:
> Index: kvm/include/asm-s390/pgtable.h
> ===================================================================
> --- kvm.orig/include/asm-s390/pgtable.h
> +++ kvm/include/asm-s390/pgtable.h
> @@ -553,12 +553,12 @@ static inline void ptep_rcp_copy(pte_t *
> 
> skey = page_get_storage_key(page_to_phys(page));
> if (skey & _PAGE_CHANGED)
> - set_bit(RCP_GC_BIT, pgste);
> + set_bit_simple(RCP_GC_BIT, pgste);
> if (skey & _PAGE_REFERENCED)
> - set_bit(RCP_GR_BIT, pgste);
> - if (test_and_clear_bit(RCP_HC_BIT, pgste))
> + set_bit_simple(RCP_GR_BIT, pgste);
> + if (test_and_clear_bit_simple(RCP_HC_BIT, pgste))
> SetPageDirty(page);
> - if (test_and_clear_bit(RCP_HR_BIT, pgste))
> + if (test_and_clear_bit_simple(RCP_HR_BIT, pgste))
> SetPageReferenced(page);
> #endif
> }
> @@ -732,8 +732,8 @@ static inline int ptep_test_and_clear_yo
> young = ((page_get_storage_key(physpage) & _PAGE_REFERENCED) != 0);
> rcp_lock(ptep);
> if (young)
> - set_bit(RCP_GR_BIT, pgste);
> - young |= test_and_clear_bit(RCP_HR_BIT, pgste);
> + set_bit_simple(RCP_GR_BIT, pgste);
> + young |= test_and_clear_bit_simple(RCP_HR_BIT, pgste);
> rcp_unlock(ptep);
> return young;
> #endif

Major formatting accident ?

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH 03/04] kvm-s390: Improve pgste accesses

[Carsten Otte]

MAILER-DAEMON@linux.ibm.com wrote:
> Major formatting accident ?
Ooooooooops! How did that happen? Thanks, will resend that patch.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH 03/04] kvm-s390: Improve pgste accesses

[Avi Kivity]

Carsten Otte wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> There is no need to use interlocked updates when the rcp
> lock is held. Therefore the simple bitops variants can be
> used. This should improve performance.
>
> skey = page_get_storage_key(page_to_phys(page));
> if (skey & _PAGE_CHANGED)
> - set_bit(RCP_GC_BIT, pgste);
> + set_bit_simple(RCP_GC_BIT, pgste);
>   

This looks whitespace damaged.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace